Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions web/packages/teleport/src/DesktopSession/DesktopSession.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { AuthenticatedWebSocket } from 'teleport/lib/AuthenticatedWebSocket';
import { adaptWebSocketToTdpTransport } from 'teleport/lib/tdp';
import { shouldShowMfaPrompt, useMfaEmitter } from 'teleport/lib/useMfa';
import { getHostName } from 'teleport/services/api';
import auth from 'teleport/services/auth';

export function DesktopSession() {
const ctx = useTeleport();
Expand Down Expand Up @@ -72,10 +73,29 @@ export function DesktopSession() {
}, [ctx.userService])
);

const hasAnotherSession = useCallback(
() => ctx.desktopService.checkDesktopIsActive(clusterId, desktopName),
[clusterId, ctx.desktopService, desktopName]
);
// Returns an active session only if per-session MFA is disabled.
// This improves the user experience by preventing multiple confirmation prompts:
// - one from the active desktop alert,
// - another from the per-session MFA prompt.
// The check for another session was added to prevent a situation where a user could be tricked
// into clicking a link that would DOS another user's active session.
// https://github.com/gravitational/webapps/pull/1297
// Showing only the MFA prompt is enough for security.
const hasAnotherSession = useCallback(async (): Promise<boolean> => {
const [mfaRequiredResponse, desktopActive] = await Promise.all([
auth.checkMfaRequired(clusterId, {
windows_desktop: {
desktop_name: desktopName,
login: username,
},
}),
ctx.desktopService.checkDesktopIsActive(clusterId, desktopName),
]);
if (mfaRequiredResponse.required) {
return false;
}
return desktopActive;
}, [clusterId, ctx.desktopService, desktopName, username]);

useEffect(() => {
fetchAcl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default {
HttpResponse.json({})
),
mfaRequired: [
http.post(cfg.getMfaRequiredUrl(), () =>
http.post(cfg.getMfaRequiredUrl(cfg.proxyCluster), () =>
HttpResponse.json({ required: false })
),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ export function useConnectionDiagnostic() {
try {
if (!mfaAuthnResponse) {
const mfaReq = getMfaRequest(req, resourceSpec);
const sessionMfa = await auth.checkMfaRequired(mfaReq);
const sessionMfa = await auth.checkMfaRequired(
ctx.storeUser.getClusterId(),
mfaReq
);
if (sessionMfa.required) {
setShowMfaDialog(true);
return { mfaRequired: true };
Expand Down
3 changes: 1 addition & 2 deletions web/packages/teleport/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -823,8 +823,7 @@ const cfg = {
return generatePath(cfg.api.connectionDiagnostic, { clusterId });
},

getMfaRequiredUrl() {
const clusterId = cfg.proxyCluster;
getMfaRequiredUrl(clusterId: string) {
return generatePath(cfg.api.mfaRequired, { clusterId });
},

Expand Down
5 changes: 3 additions & 2 deletions web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,11 @@ const auth = {
};

function checkMfaRequired(
clusterId: string,
params: IsMfaRequiredRequest,
abortSignal?
abortSignal?: AbortSignal
): Promise<IsMfaRequiredResponse> {
return api.post(cfg.getMfaRequiredUrl(), params, abortSignal);
return api.post(cfg.getMfaRequiredUrl(clusterId), params, abortSignal);
}

function base64EncodeUnicode(str: string) {
Expand Down
Loading