diff --git a/lib/web/desktop.go b/lib/web/desktop.go index 55046dc796adf..93be476734baa 100644 --- a/lib/web/desktop.go +++ b/lib/web/desktop.go @@ -374,6 +374,9 @@ func (h *Handler) performSessionMFACeremony( return mfa.PromptFunc(func(ctx context.Context, chal *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) { codec := tdpMFACodec{} + if chal.WebauthnChallenge == nil { + return nil, trace.AccessDenied("Desktop access requires WebAuthn MFA, please register a WebAuthn device to connect") + } // Send the challenge over the socket. msg, err := codec.Encode( &client.MFAAuthenticateChallenge{ diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx index 73505665e47e0..ba4055ced10f7 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -26,13 +26,14 @@ import Dialog, { DialogHeader, DialogTitle, } from 'design/Dialog'; +import { Attempt as AsyncAttempt } from 'shared/hooks/useAsync'; import { Attempt } from 'shared/hooks/useAttemptNext'; import AuthnDialog from 'teleport/components/AuthnDialog'; import TdpClientCanvas from 'teleport/components/TdpClientCanvas'; import { TdpClientCanvasRef } from 'teleport/components/TdpClientCanvas/TdpClientCanvas'; import { useListener } from 'teleport/lib/tdp/client'; -import type { MfaState } from 'teleport/lib/useMfa'; +import { MfaState, shouldShowMfaPrompt } from 'teleport/lib/useMfa'; import TopBar from './TopBar'; import useDesktopSession, { @@ -257,7 +258,7 @@ export function DesktopSession(props: State) { {screenState.screen === 'anotherSessionActive' && ( )} - {screenState.screen === 'mfa' && } + {screenState.screen === 'mfa' && } {screenState.screen === 'alert dialog' && ( )} @@ -282,17 +283,6 @@ export function DesktopSession(props: State) { ); } -const MfaDialog = ({ mfa }: { mfa: MfaState }) => { - return ( - - ); -}; - const AlertDialog = ({ screenState }: { screenState: ScreenState }) => ( ({ width: '484px' })} open={true}> @@ -300,9 +290,13 @@ const AlertDialog = ({ screenState }: { screenState: ScreenState }) => ( <> - {screenState.alertMessage || invalidStateMessage}} - /> + {typeof screenState.alertMessage === 'object' ? ( + + {screenState.alertMessage.title} + + ) : ( + {screenState.alertMessage} + )} Refresh the page to reconnect. @@ -376,7 +370,7 @@ const nextScreenState = ( tdpConnection: Attempt, wsConnection: WebsocketAttempt, showAnotherSessionActiveDialog: boolean, - webauthn: MfaState + mfa: MfaState ): ScreenState => { // We always want to show the user the first alert that caused the session to fail/end, // so if we're already showing an alert, don't change the screen. @@ -393,10 +387,11 @@ const nextScreenState = ( // Otherwise, calculate a new screen state. const showAnotherSessionActive = showAnotherSessionActiveDialog; - const showMfa = webauthn.challenge; + const showMfa = shouldShowMfaPrompt(mfa); const showAlert = fetchAttempt.status === 'failed' || // Fetch attempt failed tdpConnection.status === 'failed' || // TDP connection closed by the remote side. + mfa.attempt.status === 'error' || // MFA was canceled wsConnection.status === 'closed'; // Websocket closed, means unexpected close. const atLeastOneAttemptProcessing = @@ -431,6 +426,7 @@ const nextScreenState = ( tdpConnection, wsConnection, showAnotherSessionActiveDialog, + mfa.attempt, prevState ), canvasState: { shouldConnect: false, shouldDisplay: false }, @@ -461,9 +457,17 @@ const calculateAlertMessage = ( tdpConnection: Attempt, wsConnection: WebsocketAttempt, showAnotherSessionActiveDialog: boolean, + mfaAttempt: AsyncAttempt, prevState: ScreenState -): string => { +) => { let message = ''; + // Errors, except for dialog cancellations, are handled within the MFA dialog. + if (mfaAttempt.status === 'error') { + return { + title: 'This session requires multi factor authentication', + message: mfaAttempt.statusText, + }; + } if (fetchAttempt.status === 'failed') { message = fetchAttempt.statusText || 'fetch attempt failed'; } else if (tdpConnection.status === 'failed') { @@ -494,7 +498,7 @@ type ScreenState = { | 'processing' | 'canvas'; - alertMessage?: string; + alertMessage?: string | { title: string; message: string }; canvasState: { shouldConnect: boolean; shouldDisplay: boolean; diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx index e8a53eb3365a4..740f3a995820e 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx @@ -23,12 +23,11 @@ import { Cross, FingerprintSimple } from 'design/Icon'; import { guessProviderType } from 'shared/components/ButtonSso'; import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso'; -import { MfaCanceledError, MfaState } from 'teleport/lib/useMfa'; +import { MfaState, shouldShowMfaPrompt } from 'teleport/lib/useMfa'; import { MFA_OPTION_TOTP } from 'teleport/services/mfa'; export type Props = { mfaState: MfaState; - replaceErrorText?: string; // onClose is an optional function to perform additional operations // upon closing the dialog. e.g. close a shell session onClose?: () => void; @@ -36,13 +35,9 @@ export type Props = { export default function AuthnDialog({ mfaState: { options, challenge, submit, attempt, cancelAttempt }, - replaceErrorText, onClose = () => {}, }: Props) { - const showError = - attempt.status === 'error' && !(attempt.error instanceof MfaCanceledError); - - if (!challenge && !showError) return; + if (!shouldShowMfaPrompt({ challenge, attempt })) return; // TODO(Joerger): TOTP should be pretty easy to support here with a small button -> form flow. const onlyTotpAvailable = @@ -72,7 +67,7 @@ export default function AuthnDialog({ )} {attempt.status === 'error' && ( - {replaceErrorText || attempt.statusText} + {attempt.statusText} )} diff --git a/web/packages/teleport/src/lib/useMfa.test.tsx b/web/packages/teleport/src/lib/useMfa.test.tsx index 38e8aafdfe139..afe86ca77aae4 100644 --- a/web/packages/teleport/src/lib/useMfa.test.tsx +++ b/web/packages/teleport/src/lib/useMfa.test.tsx @@ -118,6 +118,15 @@ describe('useMfa', () => { await waitFor(() => expect(mfa.current.mfaRequired).toEqual(false)); }); + test('returns empty challenge response when mfa challenge and request are absent', async () => { + const { result: mfa } = renderHook(() => useMfa()); + + const resp = await act(() => mfa.current.getChallengeResponse()); + + expect(resp).toBeUndefined(); + expect(mfa.current.mfaRequired).toEqual(false); + }); + test('adaptable mfa requirement', async () => { jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(mockChallenge); jest @@ -175,7 +184,7 @@ describe('useMfa', () => { throw err; }); - const { result: mfa } = renderHook(() => useMfa({})); + const { result: mfa } = renderHook(() => useMfa({ req: mockChallengeReq })); await act(async () => { await expect(mfa.current.getChallengeResponse()).rejects.toThrow(err); diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index 73fcffb947cca..9e14e40c319dd 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -84,23 +84,20 @@ export function useMfa(props?: MfaProps): MfaState { } // Caller didn't pass a challenge and the mfa required is true or unknown. - if (!challenge) { - let req = props.req; - + if (!challenge && props?.req) { // We already know MFA is required, skip the extra check. - if (mfaRequired === true) req.isMfaRequiredRequest = null; - + if (mfaRequired === true) props.req.isMfaRequiredRequest = null; challenge = await auth.getMfaChallenge(props.req); + } - // An empty challenge means either mfa is not required, or the user has no mfa devices. - if (!challenge) { - setMfaRequired(false); - return; - } - - setMfaRequired(true); + // An empty challenge means either mfa is not required, or the user has no mfa devices. + if (!challenge) { + setMfaRequired(false); + return; } + setMfaRequired(true); + // Prepare a new promise to collect the mfa response retrieved // through the submit function. let resolve: (value: MfaChallengeResponse) => void; @@ -218,6 +215,17 @@ export type MfaState = { cancelAttempt: () => void; }; +/** Indicates if an MFA dialog should be visible. */ +export function shouldShowMfaPrompt( + mfa: Pick +): boolean { + return ( + !!mfa.challenge || + (mfa.attempt.status === 'error' && + !(mfa.attempt.error instanceof MfaCanceledError)) + ); +} + // used for testing export function makeDefaultMfaState(): MfaState { return {