From 5911b8508ed9f87f62d7c2281cd741be382d5188 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 4 Dec 2024 13:02:25 -0800 Subject: [PATCH] Reuse useReAuthenticate in useMfa. --- .../DocumentKubeExec/DocumentKubeExec.tsx | 4 +- .../src/Console/DocumentSsh/DocumentSsh.tsx | 6 +- .../src/DesktopSession/DesktopSession.tsx | 8 +- .../AuthnDialog/AuthnDialog.story.tsx | 35 ++-- .../AuthnDialog/AuthnDialog.test.tsx | 41 ++-- .../components/AuthnDialog/AuthnDialog.tsx | 25 +-- .../ReAuthenticate/useReAuthenticate.ts | 9 +- web/packages/teleport/src/lib/useMfa.ts | 179 +++--------------- 8 files changed, 106 insertions(+), 201 deletions(-) diff --git a/web/packages/teleport/src/Console/DocumentKubeExec/DocumentKubeExec.tsx b/web/packages/teleport/src/Console/DocumentKubeExec/DocumentKubeExec.tsx index 3b405e034f04c..4495f5fc6d40d 100644 --- a/web/packages/teleport/src/Console/DocumentKubeExec/DocumentKubeExec.tsx +++ b/web/packages/teleport/src/Console/DocumentKubeExec/DocumentKubeExec.tsx @@ -43,7 +43,7 @@ export default function DocumentKubeExec({ doc, visible }: Props) { useEffect(() => { // when switching tabs or closing tabs, focus on visible terminal terminalRef.current?.focus(); - }, [visible, mfa.requested]); + }, [visible, mfa.challenge]); const theme = useTheme(); const terminal = ( @@ -63,7 +63,7 @@ export default function DocumentKubeExec({ doc, visible }: Props) { )} - {mfa.requested && } + {mfa.challenge && } {status === 'waiting-for-exec-data' && ( diff --git a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx index 8a81c4ec6031b..33f77c58916fe 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx +++ b/web/packages/teleport/src/Console/DocumentSsh/DocumentSsh.tsx @@ -58,7 +58,7 @@ function DocumentSsh({ doc, visible }: PropTypes) { getDownloader, getUploader, fileTransferRequests, - } = useFileTransfer(tty, session, doc, mfa.addMfaToScpUrls); + } = useFileTransfer(tty, session, doc, mfa.mfaRequired); const theme = useTheme(); function handleCloseFileTransfer() { @@ -72,7 +72,7 @@ function DocumentSsh({ doc, visible }: PropTypes) { useEffect(() => { // when switching tabs or closing tabs, focus on visible terminal terminalRef.current?.focus(); - }, [visible, mfa.requested]); + }, [visible, mfa.challenge]); const onSearchClose = useCallback(() => { setShowSearch(false); @@ -136,7 +136,7 @@ function DocumentSsh({ doc, visible }: PropTypes) { )} - {mfa.requested && } + {mfa.challenge && } {status === 'initialized' && terminal} ); diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx index b1f188f2997c9..93a087c1c1f5e 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -186,9 +186,9 @@ const MfaDialog = ({ mfa }: { mfa: MfaState }) => { { - mfa.setErrorText( - 'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.' - ); + mfa.submitAttempt.status = 'failed'; + mfa.submitAttempt.statusText = + 'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.'; }} /> ); @@ -294,7 +294,7 @@ const nextScreenState = ( // Otherwise, calculate a new screen state. const showAnotherSessionActive = showAnotherSessionActiveDialog; - const showMfa = webauthn.requested; + const showMfa = webauthn.challenge; const showAlert = fetchAttempt.status === 'failed' || // Fetch attempt failed tdpConnection.status === 'failed' || // TDP connection failed diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx index b0226bd71faaa..a256779afb366 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.story.tsx @@ -31,18 +31,20 @@ export const LoadedWithMultipleOptions = () => { ...defaultProps, mfa: { ...defaultProps.mfa, - ssoChallenge: { - redirectUrl: 'hi', - requestId: '123', - channelId: '123', - device: { - connectorId: '123', - connectorType: 'saml', - displayName: 'Okta', + challenge: { + ssoChallenge: { + redirectUrl: 'hi', + requestId: '123', + channelId: '123', + device: { + connectorId: '123', + connectorType: 'saml', + displayName: 'Okta', + }, + }, + webauthnPublicKey: { + challenge: new ArrayBuffer(1), }, - }, - webauthnPublicKey: { - challenge: new ArrayBuffer(1), }, }, }; @@ -54,8 +56,10 @@ export const LoadedWithSingleOption = () => { ...defaultProps, mfa: { ...defaultProps.mfa, - webauthnPublicKey: { - challenge: new ArrayBuffer(1), + challenge: { + webauthnPublicKey: { + challenge: new ArrayBuffer(1), + }, }, }, }; @@ -67,7 +71,10 @@ export const Error = () => { ...defaultProps, mfa: { ...defaultProps.mfa, - errorText: 'Something went wrong', + submitAttempt: { + status: 'failed', + statusText: 'Something went wrong', + }, }, }; return ; diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx index e4752390357e7..56ffcdff8be48 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.test.tsx @@ -17,15 +17,15 @@ */ import React from 'react'; -import { render, screen, fireEvent } from 'design/utils/testing'; +import { render, screen, fireEvent, waitFor } from 'design/utils/testing'; import { makeDefaultMfaState, MfaState } from 'teleport/lib/useMfa'; -import { SSOChallenge } from 'teleport/services/mfa'; +import { SsoChallenge } from 'teleport/services/mfa'; import AuthnDialog from './AuthnDialog'; -const mockSsoChallenge: SSOChallenge = { +const mockSsoChallenge: SsoChallenge = { redirectUrl: 'url', requestId: '123', device: { @@ -52,7 +52,11 @@ describe('AuthnDialog', () => { }); test('renders single option dialog', () => { - const mfa = makeMockState({ ssoChallenge: mockSsoChallenge }); + const mfa = makeMockState({ + challenge: { + ssoChallenge: mockSsoChallenge, + }, + }); render(); expect(screen.getByText('Verify Your Identity')).toBeInTheDocument(); @@ -65,9 +69,11 @@ describe('AuthnDialog', () => { test('renders multi option dialog', () => { const mfa = makeMockState({ - ssoChallenge: mockSsoChallenge, - webauthnPublicKey: { - challenge: new ArrayBuffer(1), + challenge: { + ssoChallenge: mockSsoChallenge, + webauthnPublicKey: { + challenge: new ArrayBuffer(1), + }, }, }); render(); @@ -84,7 +90,10 @@ describe('AuthnDialog', () => { test('displays error text when provided', () => { const errorText = 'Authentication failed'; - const mfa = makeMockState({ errorText }); + const mfa = makeMockState({ + challenge: {}, + submitAttempt: { status: 'failed', statusText: errorText }, + }); render(); expect(screen.getByTestId('danger-alert')).toBeInTheDocument(); @@ -93,23 +102,27 @@ describe('AuthnDialog', () => { test('sso button renders with callback', async () => { const mfa = makeMockState({ - ssoChallenge: mockSsoChallenge, - onSsoAuthenticate: jest.fn(), + challenge: { + ssoChallenge: mockSsoChallenge, + }, + submitWithMfa: jest.fn(), }); render(); const ssoButton = screen.getByText('Okta'); fireEvent.click(ssoButton); - expect(mfa.onSsoAuthenticate).toHaveBeenCalledTimes(1); + expect(mfa.submitWithMfa).toHaveBeenCalledTimes(1); }); test('webauthn button renders with callback', async () => { const mfa = makeMockState({ - webauthnPublicKey: { challenge: new ArrayBuffer(0) }, - onWebauthnAuthenticate: jest.fn(), + challenge: { + webauthnPublicKey: { challenge: new ArrayBuffer(0) }, + }, + submitWithMfa: jest.fn(), }); render(); const webauthn = screen.getByText('Passkey or MFA Device'); fireEvent.click(webauthn); - expect(mfa.onWebauthnAuthenticate).toHaveBeenCalledTimes(1); + expect(mfa.submitWithMfa).toHaveBeenCalledTimes(1); }); }); diff --git a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx index a8b87df47de90..3c4a2ed481f85 100644 --- a/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx +++ b/web/packages/teleport/src/components/AuthnDialog/AuthnDialog.tsx @@ -29,7 +29,8 @@ import { SSOIcon } from 'shared/components/ButtonSso/ButtonSso'; import { MfaState } from 'teleport/lib/useMfa'; export default function AuthnDialog({ mfa, onCancel }: Props) { - let hasMultipleOptions = mfa.ssoChallenge && mfa.webauthnPublicKey; + let hasMultipleOptions = + mfa.challenge.ssoChallenge && mfa.challenge.webauthnPublicKey; return ( ({ width: '400px' })} open={true}> @@ -40,9 +41,9 @@ export default function AuthnDialog({ mfa, onCancel }: Props) { - {mfa.errorText && ( + {mfa.submitAttempt.status === 'failed' && ( - {mfa.errorText} + {mfa.submitAttempt.statusText} )} @@ -52,28 +53,28 @@ export default function AuthnDialog({ mfa, onCancel }: Props) { - {mfa.ssoChallenge && ( + {mfa.challenge.ssoChallenge && ( mfa.submitWithMfa('sso')} gap={2} block > - {mfa.ssoChallenge.device.displayName || - mfa.ssoChallenge.device.connectorId} + {mfa.challenge.ssoChallenge.device.displayName || + mfa.challenge.ssoChallenge.device.connectorId} )} - {mfa.webauthnPublicKey && ( + {mfa.challenge.webauthnPublicKey && ( mfa.submitWithMfa('webauthn')} gap={2} block > diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index 1e7d1337db3f2..57bd0d513d1cc 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -38,12 +38,16 @@ import { // token, and after successfully obtaining the token, the function // `onAuthenticated` will be called with this token. export default function useReAuthenticate(props: ReauthProps): ReauthState { + if (!props.getMfaChallenge) { + props.getMfaChallenge = () => + auth.getMfaChallenge({ scope: props.challengeScope }); + } + // Note that attempt state "success" is not used or required. // After the user submits, the control is passed back // to the caller who is responsible for rendering the `ReAuthenticate` // component. const { attempt, setAttempt } = useAttempt(''); - const [challenge, setMfaChallenge] = useState(null); // Provide a custom error handler to catch a webauthn frontend error that occurs @@ -79,7 +83,7 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState { return challenge; } - return auth.getMfaChallenge({ scope: props.challengeScope }).then(chal => { + return props.getMfaChallenge().then(chal => { setMfaChallenge(chal); return chal; }); @@ -133,6 +137,7 @@ export default function useReAuthenticate(props: ReauthProps): ReauthState { export type ReauthProps = { challengeScope?: MfaChallengeScope; + getMfaChallenge?(): Promise; onMfaResponse?(res: MfaChallengeResponse): void; // TODO(Joerger): Remove in favor of onMfaResponse, make onMfaResponse required. onAuthenticated?(privilegeTokenId: string): void; diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index e7970d00f34d7..16b477705fdd1 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -16,183 +16,62 @@ * along with this program. If not, see . */ -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect } from 'react'; import { EventEmitterMfaSender } from 'teleport/lib/EventEmitterMfaSender'; import { TermEvent } from 'teleport/lib/term/enums'; import { parseMfaChallengeJson as parseMfaChallenge } from 'teleport/services/mfa/makeMfa'; -import { - MfaAuthenticateChallengeJson, - SsoChallenge, -} from 'teleport/services/mfa'; -import auth from 'teleport/services/auth/auth'; +import { DeviceType, MfaAuthenticateChallenge } from 'teleport/services/mfa'; +import useReAuthenticate from 'teleport/components/ReAuthenticate/useReAuthenticate'; +import { Attempt } from 'shared/hooks/useAttemptNext'; export function useMfa(emitterSender: EventEmitterMfaSender): MfaState { - const [state, setState] = useState<{ - errorText: string; - addMfaToScpUrls: boolean; - webauthnPublicKey: PublicKeyCredentialRequestOptions; - ssoChallenge: SsoChallenge; - totpChallenge: boolean; - }>({ - addMfaToScpUrls: false, - errorText: '', - webauthnPublicKey: null, - ssoChallenge: null, - totpChallenge: false, - }); - - function clearChallenges() { - setState(prevState => ({ - ...prevState, - totpChallenge: false, - webauthnPublicKey: null, - ssoChallenge: null, - })); - } - - function onSsoAuthenticate() { - if (!state.ssoChallenge) { - setState(prevState => ({ - ...prevState, - errorText: 'Invalid or missing SSO challenge', - })); - return; - } - - auth.openSsoChallengeRedirect(state.ssoChallenge); - } + const [challenge, setChallenge] = useState(); - function onWebauthnAuthenticate() { - if (!window.PublicKeyCredential) { - const errorText = - 'This browser does not support WebAuthn required for hardware tokens, \ - please try the latest version of Chrome, Firefox or Safari.'; - - setState({ - ...state, - errorText, - }); - return; - } - - auth - .getMfaChallengeResponse({ - webauthnPublicKey: state.webauthnPublicKey, - }) - .then(res => { - setState(prevState => ({ - ...prevState, - errorText: '', - webauthnPublicKey: null, - })); - emitterSender.sendChallengeResponse(res); - }) - .catch((err: Error) => { - setErrorText(err.message); - }); - } - - const waitForSsoChallengeResponse = useCallback( - async ( - ssoChallenge: SsoChallenge, - abortSignal: AbortSignal - ): Promise => { - try { - const resp = await auth.waitForSsoChallengeResponse( - ssoChallenge, - abortSignal - ); - emitterSender.sendChallengeResponse(resp); - clearChallenges(); - } catch (error) { - if (error.name !== 'AbortError') { - throw error; - } - } + const { attempt, submitWithMfa } = useReAuthenticate({ + getMfaChallenge: async () => challenge, + onMfaResponse: res => { + setChallenge(null); + emitterSender.sendChallengeResponse(res); }, - [emitterSender] - ); + }); + + const [mfaRequired, setMfaRequired] = useState(false); useEffect(() => { - let ssoChallengeAbortController: AbortController | undefined; const challengeHandler = (challengeJson: string) => { - const challenge = JSON.parse( - challengeJson - ) as MfaAuthenticateChallengeJson; - - const { webauthnPublicKey, ssoChallenge, totpChallenge } = - parseMfaChallenge(challenge); - - setState(prevState => ({ - ...prevState, - addMfaToScpUrls: true, - ssoChallenge, - webauthnPublicKey, - totpChallenge, - })); - - if (ssoChallenge) { - ssoChallengeAbortController = new AbortController(); - void waitForSsoChallengeResponse( - ssoChallenge, - ssoChallengeAbortController.signal - ); - } + const challenge = JSON.parse(challengeJson); + setChallenge(parseMfaChallenge(challenge)); + setMfaRequired(true); }; emitterSender?.on(TermEvent.MFA_CHALLENGE, challengeHandler); - return () => { - ssoChallengeAbortController?.abort(); emitterSender?.removeListener(TermEvent.MFA_CHALLENGE, challengeHandler); }; - }, [emitterSender, waitForSsoChallengeResponse]); - - function setErrorText(newErrorText: string) { - setState(prevState => ({ ...prevState, errorText: newErrorText })); - } - - // if any challenge exists, requested is true - const requested = !!( - state.webauthnPublicKey || - state.totpChallenge || - state.ssoChallenge - ); + }, [emitterSender]); return { - requested, - onWebauthnAuthenticate, - onSsoAuthenticate, - addMfaToScpUrls: state.addMfaToScpUrls, - setErrorText, - errorText: state.errorText, - webauthnPublicKey: state.webauthnPublicKey, - ssoChallenge: state.ssoChallenge, + challenge, + submitAttempt: attempt, + submitWithMfa, + mfaRequired, }; } export type MfaState = { - onWebauthnAuthenticate: () => void; - onSsoAuthenticate: () => void; - setErrorText: (errorText: string) => void; - errorText: string; - requested: boolean; - addMfaToScpUrls: boolean; - webauthnPublicKey: PublicKeyCredentialRequestOptions; - ssoChallenge: SsoChallenge; + challenge: MfaAuthenticateChallenge; + mfaRequired: boolean; + submitAttempt: Attempt; + submitWithMfa: (mfaType?: DeviceType, totp_code?: string) => Promise; }; // used for testing export function makeDefaultMfaState(): MfaState { return { - onWebauthnAuthenticate: () => null, - onSsoAuthenticate: () => null, - setErrorText: () => null, - errorText: '', - requested: false, - addMfaToScpUrls: false, - webauthnPublicKey: null, - ssoChallenge: null, + challenge: null, + submitAttempt: {} as Attempt, + mfaRequired: true, + submitWithMfa: (mfaType?: DeviceType, totp_code?: string) => null, }; }