From bd465bc782cec81f9ef53f953c8b704df06ca1ff Mon Sep 17 00:00:00 2001 From: joerger Date: Thu, 21 Nov 2024 09:35:51 -0800 Subject: [PATCH 1/6] Replace fetchWebAuthnChallenge with getChallenge. --- .../ChangePasswordWizard.test.tsx | 37 ++++++---- .../ChangePasswordWizard.tsx | 5 +- .../teleport/src/services/auth/auth.ts | 71 ++++++++++--------- 3 files changed, 65 insertions(+), 48 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx index 30935e14eb399..d966ba7aae147 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx @@ -30,10 +30,23 @@ import { } from './ChangePasswordWizard'; import { ChangePasswordWizard } from '.'; - -const dummyCredential: Credential = { - id: 'cred-id', - type: 'public-key', +import { MfaChallengeResponse } from 'teleport/services/mfa'; + +const dummyChallengeResponse: MfaChallengeResponse = { + webauthn_response: { + id: 'cred-id', + type: 'public-key', + extensions: { + appid: true, + }, + rawId: 'rawId', + response: { + authenticatorData: 'authenticatorData', + clientDataJSON: 'clientDataJSON', + signature: 'signature', + userHandle: 'userHandle', + }, + }, }; let user: UserEvent; let onSuccess: jest.Mock; @@ -73,8 +86,8 @@ beforeEach(() => { onSuccess = jest.fn(); jest - .spyOn(auth, 'fetchWebAuthnChallenge') - .mockResolvedValueOnce(dummyCredential); + .spyOn(auth, 'getWebAuthnChallengeResponse') + .mockResolvedValueOnce(dummyChallengeResponse); jest.spyOn(auth, 'changePassword').mockResolvedValueOnce(undefined); }); @@ -89,7 +102,7 @@ describe('with passwordless reauthentication', () => { ); await user.click(reauthenticateStep.getByText('Passkey')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.fetchWebAuthnChallenge).toHaveBeenCalledWith({ + expect(auth.getChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'required', }); @@ -113,7 +126,7 @@ describe('with passwordless reauthentication', () => { oldPassword: '', newPassword: 'new-pass1234', secondFactorToken: '', - credential: dummyCredential, + credential: dummyChallengeResponse.webauthn_response, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -180,7 +193,7 @@ describe('with WebAuthn MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('MFA Device')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.fetchWebAuthnChallenge).toHaveBeenCalledWith({ + expect(auth.getChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'discouraged', }); @@ -208,7 +221,7 @@ describe('with WebAuthn MFA reauthentication', () => { oldPassword: 'current-pass', newPassword: 'new-pass1234', secondFactorToken: '', - credential: dummyCredential, + credential: dummyChallengeResponse.webauthn_response, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -282,7 +295,7 @@ describe('with OTP MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('Authenticator App')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.fetchWebAuthnChallenge).not.toHaveBeenCalled(); + expect(auth.getChallenge).not.toHaveBeenCalled(); } it('changes password', async () => { @@ -406,7 +419,7 @@ describe('without reauthentication', () => { 'new-pass1234' ); await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.fetchWebAuthnChallenge).not.toHaveBeenCalled(); + expect(auth.getChallenge).not.toHaveBeenCalled(); expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: 'current-pass', newPassword: 'new-pass1234', diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 458357e83ed54..320a1a38c3b17 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -172,12 +172,13 @@ export function ReauthenticateStep({ const [reauthenticateAttempt, reauthenticate] = useAsync( async (m: ReauthenticationMethod) => { if (m === 'passwordless' || m === 'mfaDevice') { - const res = await auth.fetchWebAuthnChallenge({ + const res = await auth.getChallenge({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: m === 'passwordless' ? 'required' : 'discouraged', }); - onAuthenticated(res); + const challengeResponse = await auth.getWebAuthnChallengeResponse(res); + onAuthenticated(challengeResponse.webauthn_response); } next(); } diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 0d126151e20bf..d7222ffb5ad52 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -18,7 +18,12 @@ import api from 'teleport/services/api'; import cfg from 'teleport/config'; -import { DeviceType, DeviceUsage } from 'teleport/services/mfa'; +import { + DeviceType, + DeviceUsage, + MfaAuthenticateChallenge, + MfaChallengeResponse, +} from 'teleport/services/mfa'; import { CaptureEvent, userEventService } from 'teleport/services/userEvent'; @@ -235,11 +240,12 @@ const auth = { headlessSSOAccept(transactionId: string) { return auth - .fetchWebAuthnChallenge({ scope: MfaChallengeScope.HEADLESS_LOGIN }) + .getChallenge({ scope: MfaChallengeScope.HEADLESS_LOGIN }) + .then(auth.getWebAuthnChallengeResponse) .then(res => { const request = { action: 'accept', - webauthnAssertionResponse: makeWebauthnAssertionResponse(res), + webauthnAssertionResponse: res.webauthn_response, }; return api.put(cfg.getHeadlessSsoPath(transactionId), request); @@ -266,47 +272,45 @@ const auth = { .post( cfg.api.mfaAuthnChallengePath, { + is_mfa_required_req: req.isMfaRequiredRequest, challenge_scope: req.scope, + challenge_allow_reuse: req.allowReuse, + user_verification_requirement: req.userVerificationRequirement, }, abortSignal ) .then(parseMfaChallengeJson); }, - async fetchWebAuthnChallenge( - req: CreateAuthenticateChallengeRequest, - abortSignal?: AbortSignal - ) { - return auth - .checkWebauthnSupport() - .then(() => - api - .post( - cfg.api.mfaAuthnChallengePath, - { - is_mfa_required_req: req.isMfaRequiredRequest, - challenge_scope: req.scope, - challenge_allow_reuse: req.allowReuse, - user_verification_requirement: req.userVerificationRequirement, - }, - abortSignal - ) - .then(parseMfaChallengeJson) - ) - .then(res => - navigator.credentials.get({ - publicKey: res.webauthnPublicKey, + // TODO(Joerger): Replace with getMfaResponse + async getWebAuthnChallengeResponse( + challenge: MfaAuthenticateChallenge + ): Promise { + return auth.checkWebauthnSupport().then(() => + navigator.credentials + .get({ + publicKey: challenge.webauthnPublicKey, }) - ); + .then(cred => { + return makeWebauthnAssertionResponse(cred); + }) + .then(resp => { + return { webauthn_response: resp }; + }) + ); }, + // TODO(Joerger): Combine with otp endpoint. createPrivilegeTokenWithWebauthn() { // Creating privilege tokens always expects the MANAGE_DEVICES webauthn scope. return auth - .fetchWebAuthnChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) + .getChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) + .then(auth.getWebAuthnChallengeResponse) .then(res => api.post(cfg.api.createPrivilegeTokenPath, { - webauthnAssertionResponse: makeWebauthnAssertionResponse(res), + webauthnAssertionResponse: makeWebauthnAssertionResponse( + res.webauthn_response + ), }) ); }, @@ -315,6 +319,7 @@ const auth = { return api.post(cfg.api.createPrivilegeTokenPath, {}); }, + // TODO(Joerger): Get rid of one-shot webauthn flow in favor of two-shot mfa flow. async getWebauthnResponse( scope: MfaChallengeScope, allowReuse?: boolean, @@ -349,11 +354,9 @@ const auth = { } return auth - .fetchWebAuthnChallenge( - { scope, allowReuse, isMfaRequiredRequest }, - abortSignal - ) - .then(res => makeWebauthnAssertionResponse(res)); + .getChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal) + .then(auth.getWebAuthnChallengeResponse) + .then(res => makeWebauthnAssertionResponse(res.webauthn_response)); }, getWebauthnResponseForAdminAction(allowReuse?: boolean) { From cda50c085279741848427d41dd4fbf345a9f3508 Mon Sep 17 00:00:00 2001 From: joerger Date: Thu, 21 Nov 2024 09:43:14 -0800 Subject: [PATCH 2/6] Replace getWebauthnResponse with getMfaChallengeResponse. --- .../ChangePasswordWizard.test.tsx | 13 ++--- .../ChangePasswordWizard.tsx | 9 ++-- .../Account/ManageDevices/useManageDevices.ts | 5 +- .../src/Console/DocumentSsh/useGetScpUrl.ts | 11 ++-- web/packages/teleport/src/Users/useUsers.ts | 10 ++-- .../ReAuthenticate/useReAuthenticate.ts | 7 ++- web/packages/teleport/src/services/api/api.ts | 52 ++++++++++--------- .../teleport/src/services/apps/apps.ts | 19 ++++--- .../teleport/src/services/auth/auth.ts | 51 ++++++++++++------ .../src/services/integrations/integrations.ts | 22 +++++--- .../teleport/src/services/mfa/makeMfa.ts | 8 +++ .../teleport/src/services/user/user.ts | 15 ++---- 12 files changed, 129 insertions(+), 93 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx index d966ba7aae147..492685b5e1597 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx @@ -24,13 +24,14 @@ import { userEvent, UserEvent } from '@testing-library/user-event'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; +import { MfaChallengeResponse } from 'teleport/services/mfa'; + import { ChangePasswordWizardProps, createReauthOptions, } from './ChangePasswordWizard'; import { ChangePasswordWizard } from '.'; -import { MfaChallengeResponse } from 'teleport/services/mfa'; const dummyChallengeResponse: MfaChallengeResponse = { webauthn_response: { @@ -86,7 +87,7 @@ beforeEach(() => { onSuccess = jest.fn(); jest - .spyOn(auth, 'getWebAuthnChallengeResponse') + .spyOn(auth, 'getMfaChallengeResponse') .mockResolvedValueOnce(dummyChallengeResponse); jest.spyOn(auth, 'changePassword').mockResolvedValueOnce(undefined); }); @@ -102,7 +103,7 @@ describe('with passwordless reauthentication', () => { ); await user.click(reauthenticateStep.getByText('Passkey')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getChallenge).toHaveBeenCalledWith({ + expect(auth.getMfaChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'required', }); @@ -193,7 +194,7 @@ describe('with WebAuthn MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('MFA Device')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getChallenge).toHaveBeenCalledWith({ + expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'discouraged', }); @@ -295,7 +296,7 @@ describe('with OTP MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('Authenticator App')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getChallenge).not.toHaveBeenCalled(); + expect(auth.getMfaChallengeResponse).not.toHaveBeenCalled(); } it('changes password', async () => { @@ -419,7 +420,7 @@ describe('without reauthentication', () => { 'new-pass1234' ); await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.getChallenge).not.toHaveBeenCalled(); + expect(auth.getMfaChallengeResponse).not.toHaveBeenCalled(); expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: 'current-pass', newPassword: 'new-pass1234', diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 320a1a38c3b17..1a152f2ddcfbf 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -172,13 +172,16 @@ export function ReauthenticateStep({ const [reauthenticateAttempt, reauthenticate] = useAsync( async (m: ReauthenticationMethod) => { if (m === 'passwordless' || m === 'mfaDevice') { - const res = await auth.getChallenge({ + const challenge = await auth.getMfaChallenge({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: m === 'passwordless' ? 'required' : 'discouraged', }); - const challengeResponse = await auth.getWebAuthnChallengeResponse(res); - onAuthenticated(challengeResponse.webauthn_response); + + const response = await auth.getMfaChallengeResponse(challenge); + + // TODO(Joerger): handle non-webauthn response. + onAuthenticated(response.webauthn_response); } next(); } diff --git a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts index 287bb9a77afc6..7c6ec1917673f 100644 --- a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts +++ b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts @@ -48,14 +48,15 @@ export default function useManageDevices(ctx: Ctx) { async function onAddDevice(usage: DeviceUsage) { setNewDeviceUsage(usage); - const response = await auth.getChallenge({ + const challenge = await auth.getMfaChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES, }); // If the user doesn't receieve any challenges from the backend, that means // they have no valid devices to be challenged and should instead use a privilege token // to add a new device. // TODO (avatus): add SSO challenge here as well when we add SSO for MFA - if (!response.webauthnPublicKey?.challenge && !response.totpChallenge) { + // TODO(Joerger): privilege token is no longer required to add first device. + if (!challenge) { createRestrictedTokenAttempt.run(() => auth.createRestrictedPrivilegeToken().then(token => { setToken(token); diff --git a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts index 220b925d3996f..a984a6f983986 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts +++ b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts @@ -35,15 +35,18 @@ export default function useGetScpUrl(addMfaToScpUrls: boolean) { return cfg.getScpUrl(params); } try { - let webauthn = await auth.getWebauthnResponse( - MfaChallengeScope.USER_SESSION - ); + const challenge = await auth.getMfaChallenge({ + scope: MfaChallengeScope.USER_SESSION, + }); + + const response = await auth.getMfaChallengeResponse(challenge); + setAttempt({ status: 'success', statusText: '', }); return cfg.getScpUrl({ - webauthn, + webauthn: response.webauthn_response, ...params, }); } catch (error) { diff --git a/web/packages/teleport/src/Users/useUsers.ts b/web/packages/teleport/src/Users/useUsers.ts index 5d60ed265b840..d23da34ff659a 100644 --- a/web/packages/teleport/src/Users/useUsers.ts +++ b/web/packages/teleport/src/Users/useUsers.ts @@ -87,16 +87,12 @@ export default function useUsers({ } async function onCreate(u: User) { - const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true); + const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); return ctx.userService - .createUser(u, ExcludeUserField.Traits, webauthnResponse) + .createUser(u, ExcludeUserField.Traits, mfaResponse) .then(result => setUsers([result, ...users])) .then(() => - ctx.userService.createResetPasswordToken( - u.name, - 'invite', - webauthnResponse - ) + ctx.userService.createResetPasswordToken(u.name, 'invite', mfaResponse) ); } diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index d6500860c8ae4..8bb9e13a7661d 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -58,11 +58,10 @@ export default function useReAuthenticate(props: Props) { if ('onMfaResponse' in props) { auth - .getWebauthnResponse(props.challengeScope) - .then(webauthnResponse => - props.onMfaResponse({ webauthn_response: webauthnResponse }) - ) + .getMfaChallenge({ scope: props.challengeScope }) + .then(auth.getMfaChallengeResponse) .catch(handleError); + return; } diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index c48136d0023ae..83b2bf84099b4 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -21,7 +21,7 @@ import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import websession from 'teleport/services/websession'; import { storageService } from '../storageService'; -import { WebauthnAssertionResponse } from '../mfa'; +import { MfaChallengeResponse, WebauthnAssertionResponse } from '../mfa'; import parseError, { ApiError } from './parseError'; @@ -32,7 +32,7 @@ const api = { return api.fetchJsonWithMfaAuthnRetry(url, { signal: abortSignal }); }, - post(url, data?, abortSignal?, webauthnResponse?: WebauthnAssertionResponse) { + post(url, data?, abortSignal?, mfaResponse?: MfaChallengeResponse) { return api.fetchJsonWithMfaAuthnRetry( url, { @@ -40,11 +40,11 @@ const api = { method: 'POST', signal: abortSignal, }, - webauthnResponse + mfaResponse ); }, - postFormData(url, formData, webauthnResponse?: WebauthnAssertionResponse) { + postFormData(url, formData, mfaResponse?: MfaChallengeResponse) { if (formData instanceof FormData) { return api.fetchJsonWithMfaAuthnRetry( url, @@ -60,21 +60,21 @@ const api = { // 2) https://stackoverflow.com/a/64653976 }, }, - webauthnResponse + mfaResponse ); } throw new Error('data for body is not a type of FormData'); }, - delete(url, data?, webauthnResponse?: WebauthnAssertionResponse) { + delete(url, data?, mfaResponse?: MfaChallengeResponse) { return api.fetchJsonWithMfaAuthnRetry( url, { body: JSON.stringify(data), method: 'DELETE', }, - webauthnResponse + mfaResponse ); }, @@ -82,7 +82,7 @@ const api = { url, headers?: Record, signal?, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ) { return api.fetchJsonWithMfaAuthnRetry( url, @@ -91,19 +91,19 @@ const api = { headers, signal, }, - webauthnResponse + mfaResponse ); }, // TODO (avatus) add abort signal to this - put(url, data, webauthnResponse?: WebauthnAssertionResponse) { + put(url, data, mfaResponse?: MfaChallengeResponse) { return api.fetchJsonWithMfaAuthnRetry( url, { body: JSON.stringify(data), method: 'PUT', }, - webauthnResponse + mfaResponse ); }, @@ -111,7 +111,7 @@ const api = { url, data, headers?: Record, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ) { return api.fetchJsonWithMfaAuthnRetry( url, @@ -120,7 +120,7 @@ const api = { method: 'PUT', headers, }, - webauthnResponse + mfaResponse ); }, @@ -140,9 +140,9 @@ const api = { async fetchJsonWithMfaAuthnRetry( url: string, customOptions: RequestInit, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ): Promise { - const response = await api.fetch(url, customOptions, webauthnResponse); + const response = await api.fetch(url, customOptions, mfaResponse); let json; try { @@ -174,26 +174,27 @@ const api = { const isAdminActionMfaError = isAdminActionRequiresMfaError( parseError(json) ); - const shouldRetry = isAdminActionMfaError && !webauthnResponse; + const shouldRetry = isAdminActionMfaError && !mfaResponse; if (!shouldRetry) { throw new ApiError(parseError(json), response, undefined, json.messages); } - let webauthnResponseForRetry; + let mfaResponseForRetry; try { - webauthnResponseForRetry = await auth.getWebauthnResponse( - MfaChallengeScope.ADMIN_ACTION - ); + const challenge = await auth.getMfaChallenge({ + scope: MfaChallengeScope.ADMIN_ACTION, + }); + mfaResponseForRetry = await auth.getMfaChallengeResponse(challenge); } catch { throw new Error( - 'Failed to fetch webauthn credentials, please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.' + 'Failed to fetch MFA challenge. Please connect a registered hardware key and try again. If you do not have a hardware key registered, you can add one from your account settings page.' ); } return api.fetchJsonWithMfaAuthnRetry( url, customOptions, - webauthnResponseForRetry + mfaResponseForRetry ); }, @@ -242,7 +243,7 @@ const api = { fetch( url: string, customOptions: RequestInit = {}, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ) { url = window.location.origin + url; const options = { @@ -255,9 +256,10 @@ const api = { ...getAuthHeaders(), }; - if (webauthnResponse) { + if (mfaResponse) { options.headers[MFA_HEADER] = JSON.stringify({ - webauthnAssertionResponse: webauthnResponse, + // TODO(Joerger): Handle non-webauthn response. + webauthnAssertionResponse: mfaResponse.webauthn_response, }); } diff --git a/web/packages/teleport/src/services/apps/apps.ts b/web/packages/teleport/src/services/apps/apps.ts index a301147f8730f..d64f37414a872 100644 --- a/web/packages/teleport/src/services/apps/apps.ts +++ b/web/packages/teleport/src/services/apps/apps.ts @@ -49,20 +49,23 @@ const service = { }; // Prompt for MFA if per-session MFA is required for this app. - const webauthnResponse = await auth.getWebauthnResponse( - MfaChallengeScope.USER_SESSION, - false, - { + const challenge = await auth.getMfaChallenge({ + scope: MfaChallengeScope.USER_SESSION, + allowReuse: false, + isMfaRequiredRequest: { app: resolveApp, - } - ); + }, + }); + + const resp = await auth.getMfaChallengeResponse(challenge); const createAppSession = { ...resolveApp, arn: params.arn, - mfa_response: webauthnResponse + // TODO(Joerger): Handle non-webauthn response. + mfa_response: resp ? JSON.stringify({ - webauthnAssertionResponse: webauthnResponse, + webauthnAssertionResponse: resp.webauthn_response, }) : null, }; diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index d7222ffb5ad52..75423dbbc55a3 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -240,8 +240,8 @@ const auth = { headlessSSOAccept(transactionId: string) { return auth - .getChallenge({ scope: MfaChallengeScope.HEADLESS_LOGIN }) - .then(auth.getWebAuthnChallengeResponse) + .getMfaChallenge({ scope: MfaChallengeScope.HEADLESS_LOGIN }) + .then(auth.getMfaChallengeResponse) .then(res => { const request = { action: 'accept', @@ -264,7 +264,9 @@ const auth = { return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken }); }, - async getChallenge( + // getChallenge gets an MFA challenge for the provided parameters. If is_mfa_required_req + // is provided and it is found that MFA is not required, returns null instead. + async getMfaChallenge( req: CreateAuthenticateChallengeRequest, abortSignal?: AbortSignal ) { @@ -282,7 +284,14 @@ const auth = { .then(parseMfaChallengeJson); }, - // TODO(Joerger): Replace with getMfaResponse + // getChallengeResponse gets an MFA challenge response for the provided parameters. + // If is_mfa_required_req is provided and it is found that MFA is not required, returns null instead. + async getMfaChallengeResponse(challenge: MfaAuthenticateChallenge) { + // TODO(Joerger): Handle sso and otp. We need to provide some global context which we + // can use to display the MFA component currently found in useMFA. + return auth.getWebAuthnChallengeResponse(challenge); + }, + async getWebAuthnChallengeResponse( challenge: MfaAuthenticateChallenge ): Promise { @@ -304,10 +313,11 @@ const auth = { createPrivilegeTokenWithWebauthn() { // Creating privilege tokens always expects the MANAGE_DEVICES webauthn scope. return auth - .getChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) - .then(auth.getWebAuthnChallengeResponse) + .getMfaChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES }) + .then(auth.getMfaChallengeResponse) .then(res => api.post(cfg.api.createPrivilegeTokenPath, { + // TODO(Joerger): Handle non-webauthn challenges. webauthnAssertionResponse: makeWebauthnAssertionResponse( res.webauthn_response ), @@ -319,7 +329,7 @@ const auth = { return api.post(cfg.api.createPrivilegeTokenPath, {}); }, - // TODO(Joerger): Get rid of one-shot webauthn flow in favor of two-shot mfa flow. + // TODO(Joerger): Remove once /e is no longer using it. async getWebauthnResponse( scope: MfaChallengeScope, allowReuse?: boolean, @@ -354,25 +364,32 @@ const auth = { } return auth - .getChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal) - .then(auth.getWebAuthnChallengeResponse) + .getMfaChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal) + .then(challenge => auth.getMfaChallengeResponse(challenge)) .then(res => makeWebauthnAssertionResponse(res.webauthn_response)); }, - getWebauthnResponseForAdminAction(allowReuse?: boolean) { + getMfaChallengeResponseForAdminAction(allowReuse?: boolean) { // If the client is checking if MFA is required for an admin action, // but we know admin action MFA is not enforced, return early. if (!cfg.isAdminActionMfaEnforced()) { return; } - return auth.getWebauthnResponse( - MfaChallengeScope.ADMIN_ACTION, - allowReuse, - { - admin_action: {}, - } - ); + return auth + .getMfaChallenge({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse: allowReuse, + isMfaRequiredRequest: { + admin_action: {}, + }, + }) + .then(auth.getMfaChallengeResponse); + }, + + // TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated. + getWebauthnResponseForAdminAction(allowReuse?: boolean) { + return auth.getMfaChallengeResponseForAdminAction(allowReuse); }, }; diff --git a/web/packages/teleport/src/services/integrations/integrations.ts b/web/packages/teleport/src/services/integrations/integrations.ts index dfdb2e0ad347b..f02dcdd19adc8 100644 --- a/web/packages/teleport/src/services/integrations/integrations.ts +++ b/web/packages/teleport/src/services/integrations/integrations.ts @@ -20,7 +20,7 @@ import api from 'teleport/services/api'; import cfg from 'teleport/config'; import makeNode from '../nodes/makeNode'; -import auth from '../auth/auth'; +import auth, { MfaChallengeScope } from '../auth/auth'; import { App } from '../apps'; import makeApp from '../apps/makeApps'; @@ -271,14 +271,22 @@ export const integrationService = { integrationName, req: AwsOidcDeployServiceRequest ): Promise { - const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true); + const challenge = await auth.getMfaChallenge({ + scope: MfaChallengeScope.ADMIN_ACTION, + allowReuse: true, + isMfaRequiredRequest: { + admin_action: {}, + }, + }); + + const response = await auth.getMfaChallengeResponse(challenge); return api .post( cfg.getAwsDeployTeleportServiceUrl(integrationName), req, null, - webauthnResponse + response ) .then(resp => resp.serviceDashboardUrl); }, @@ -293,14 +301,14 @@ export const integrationService = { integrationName, req: AwsOidcDeployDatabaseServicesRequest ): Promise { - const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true); + const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); return api .post( cfg.getAwsRdsDbsDeployServicesUrl(integrationName), req, null, - webauthnResponse + mfaResponse ) .then(resp => resp.clusterDashboardUrl); }, @@ -309,13 +317,13 @@ export const integrationService = { integrationName: string, req: EnrollEksClustersRequest ): Promise { - const webauthnResponse = await auth.getWebauthnResponseForAdminAction(true); + const mfaResponse = await auth.getMfaChallengeResponseForAdminAction(true); return api.post( cfg.getEnrollEksClusterUrl(integrationName), req, null, - webauthnResponse + mfaResponse ); }, diff --git a/web/packages/teleport/src/services/mfa/makeMfa.ts b/web/packages/teleport/src/services/mfa/makeMfa.ts index 0ec7c28f88071..505a972fe33e5 100644 --- a/web/packages/teleport/src/services/mfa/makeMfa.ts +++ b/web/packages/teleport/src/services/mfa/makeMfa.ts @@ -64,6 +64,14 @@ export function parseMfaRegistrationChallengeJson( export function parseMfaChallengeJson( challenge: MfaAuthenticateChallengeJson ): MfaAuthenticateChallenge { + if ( + !challenge.sso_challenge && + !challenge.webauthn_challenge && + !challenge.totp_challenge + ) { + return null; + } + // WebAuthn challenge contains Base64URL(byte) fields that needs to // be converted to ArrayBuffer expected by navigator.credentials.get: // - challenge diff --git a/web/packages/teleport/src/services/user/user.ts b/web/packages/teleport/src/services/user/user.ts index fc897d31fbcdc..85c6f70607d6b 100644 --- a/web/packages/teleport/src/services/user/user.ts +++ b/web/packages/teleport/src/services/user/user.ts @@ -20,7 +20,7 @@ import api from 'teleport/services/api'; import cfg from 'teleport/config'; import session from 'teleport/services/websession'; -import { WebauthnAssertionResponse } from '../mfa'; +import { MfaChallengeResponse, WebauthnAssertionResponse } from '../mfa'; import makeUserContext from './makeUserContext'; import { makeResetToken } from './makeResetToken'; @@ -86,14 +86,14 @@ const service = { createUser( user: User, excludeUserField: ExcludeUserField, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ) { return api .post( cfg.getUsersUrl(), withExcludedField(user, excludeUserField), null, - webauthnResponse + mfaResponse ) .then(makeUser); }, @@ -101,15 +101,10 @@ const service = { createResetPasswordToken( name: string, type: ResetPasswordType, - webauthnResponse?: WebauthnAssertionResponse + mfaResponse?: MfaChallengeResponse ) { return api - .post( - cfg.api.resetPasswordTokenPath, - { name, type }, - null, - webauthnResponse - ) + .post(cfg.api.resetPasswordTokenPath, { name, type }, null, mfaResponse) .then(makeResetToken); }, From 7515ca6072320590522139142bee15b2bc00f902 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 26 Nov 2024 12:49:27 -0800 Subject: [PATCH 3/6] Update getMfaChallengeResponse to take DeviceType and otp code. --- .../ChangePasswordWizard.tsx | 5 ++- .../src/Console/DocumentSsh/useGetScpUrl.ts | 5 ++- .../ReAuthenticate/useReAuthenticate.ts | 2 +- .../teleport/src/services/auth/auth.ts | 40 +++++++++++++++---- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 1a152f2ddcfbf..cbaed3f4de01c 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -178,7 +178,10 @@ export function ReauthenticateStep({ m === 'passwordless' ? 'required' : 'discouraged', }); - const response = await auth.getMfaChallengeResponse(challenge); + const response = await auth.getMfaChallengeResponse( + challenge, + 'webauthn' + ); // TODO(Joerger): handle non-webauthn response. onAuthenticated(response.webauthn_response); diff --git a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts index a984a6f983986..478ccbcc5fa59 100644 --- a/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts +++ b/web/packages/teleport/src/Console/DocumentSsh/useGetScpUrl.ts @@ -39,7 +39,10 @@ export default function useGetScpUrl(addMfaToScpUrls: boolean) { scope: MfaChallengeScope.USER_SESSION, }); - const response = await auth.getMfaChallengeResponse(challenge); + const response = await auth.getMfaChallengeResponse( + challenge, + 'webauthn' + ); setAttempt({ status: 'success', diff --git a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts index 8bb9e13a7661d..7b2746de0a25c 100644 --- a/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts +++ b/web/packages/teleport/src/components/ReAuthenticate/useReAuthenticate.ts @@ -59,7 +59,7 @@ export default function useReAuthenticate(props: Props) { if ('onMfaResponse' in props) { auth .getMfaChallenge({ scope: props.challengeScope }) - .then(auth.getMfaChallengeResponse) + .then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn')) .catch(handleError); return; diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 75423dbbc55a3..d1581c78e6067 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -241,7 +241,7 @@ const auth = { headlessSSOAccept(transactionId: string) { return auth .getMfaChallenge({ scope: MfaChallengeScope.HEADLESS_LOGIN }) - .then(auth.getMfaChallengeResponse) + .then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn')) .then(res => { const request = { action: 'accept', @@ -286,19 +286,43 @@ const auth = { // getChallengeResponse gets an MFA challenge response for the provided parameters. // If is_mfa_required_req is provided and it is found that MFA is not required, returns null instead. - async getMfaChallengeResponse(challenge: MfaAuthenticateChallenge) { - // TODO(Joerger): Handle sso and otp. We need to provide some global context which we - // can use to display the MFA component currently found in useMFA. - return auth.getWebAuthnChallengeResponse(challenge); + async getMfaChallengeResponse( + challenge: MfaAuthenticateChallenge, + mfaType?: DeviceType, + totpCode?: string + ): Promise { + // TODO(Joerger): If mfaType is not provided by a parent component, use some global context + // to display a component, similar to the one used in useMfa. For now we just default to + // whichever method we can succeed with first. + if (!mfaType) { + if (totpCode) { + mfaType == 'totp'; + } else if (challenge.webauthnPublicKey) { + mfaType == 'webauthn'; + } + } + + if (mfaType === 'webauthn') { + return auth.getWebAuthnChallengeResponse(challenge.webauthnPublicKey); + } + + if (mfaType === 'totp') { + return { + totp_code: totpCode, + }; + } + + // No viable challenge, return empty response. + return null; }, async getWebAuthnChallengeResponse( - challenge: MfaAuthenticateChallenge + webauthnPublicKey: PublicKeyCredentialRequestOptions ): Promise { return auth.checkWebauthnSupport().then(() => navigator.credentials .get({ - publicKey: challenge.webauthnPublicKey, + publicKey: webauthnPublicKey, }) .then(cred => { return makeWebauthnAssertionResponse(cred); @@ -365,7 +389,7 @@ const auth = { return auth .getMfaChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal) - .then(challenge => auth.getMfaChallengeResponse(challenge)) + .then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn')) .then(res => makeWebauthnAssertionResponse(res.webauthn_response)); }, From 4668f74a4787ce134c2b3f102b6f246261195922 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 27 Nov 2024 11:25:53 -0800 Subject: [PATCH 4/6] lint fix. --- web/packages/teleport/src/services/api/api.ts | 2 +- web/packages/teleport/src/services/auth/auth.ts | 4 ++-- web/packages/teleport/src/services/user/user.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index 83b2bf84099b4..1048c3333e11c 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/web/packages/teleport/src/services/api/api.ts @@ -21,7 +21,7 @@ import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import websession from 'teleport/services/websession'; import { storageService } from '../storageService'; -import { MfaChallengeResponse, WebauthnAssertionResponse } from '../mfa'; +import { MfaChallengeResponse } from '../mfa'; import parseError, { ApiError } from './parseError'; diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index d1581c78e6067..f5570d8ce8e8e 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -296,9 +296,9 @@ const auth = { // whichever method we can succeed with first. if (!mfaType) { if (totpCode) { - mfaType == 'totp'; + mfaType = 'totp'; } else if (challenge.webauthnPublicKey) { - mfaType == 'webauthn'; + mfaType = 'webauthn'; } } diff --git a/web/packages/teleport/src/services/user/user.ts b/web/packages/teleport/src/services/user/user.ts index 85c6f70607d6b..ce2d07c22034e 100644 --- a/web/packages/teleport/src/services/user/user.ts +++ b/web/packages/teleport/src/services/user/user.ts @@ -20,7 +20,7 @@ import api from 'teleport/services/api'; import cfg from 'teleport/config'; import session from 'teleport/services/websession'; -import { MfaChallengeResponse, WebauthnAssertionResponse } from '../mfa'; +import { MfaChallengeResponse } from '../mfa'; import makeUserContext from './makeUserContext'; import { makeResetToken } from './makeResetToken'; From 62b78de117cf157c41099b6ce1ef59b16ea069a8 Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 3 Dec 2024 12:48:48 -0800 Subject: [PATCH 5/6] Fix tests. --- .../teleport/src/Account/Account.test.tsx | 4 +-- .../ChangePasswordWizard.test.tsx | 14 ++++---- .../ChangePasswordWizard.tsx | 21 +++++------ web/packages/teleport/src/lib/useMfa.ts | 15 ++++---- .../teleport/src/services/api/api.test.ts | 36 ++++++++++--------- .../teleport/src/services/auth/auth.ts | 11 +++--- .../teleport/src/services/auth/types.ts | 4 +-- 7 files changed, 54 insertions(+), 51 deletions(-) diff --git a/web/packages/teleport/src/Account/Account.test.tsx b/web/packages/teleport/src/Account/Account.test.tsx index 7dcf86f471adb..3ca45002ffaee 100644 --- a/web/packages/teleport/src/Account/Account.test.tsx +++ b/web/packages/teleport/src/Account/Account.test.tsx @@ -243,7 +243,7 @@ test('adding an MFA device', async () => { const user = userEvent.setup(); const ctx = createTeleportContext(); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([testPasskey]); - jest.spyOn(auth, 'getChallenge').mockResolvedValue({ + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ webauthnPublicKey: null, totpChallenge: true, ssoChallenge: null, @@ -327,7 +327,7 @@ test('removing an MFA method', async () => { const user = userEvent.setup(); const ctx = createTeleportContext(); jest.spyOn(ctx.mfaService, 'fetchDevices').mockResolvedValue([testMfaMethod]); - jest.spyOn(auth, 'getChallenge').mockResolvedValue({ + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValue({ webauthnPublicKey: null, totpChallenge: false, ssoChallenge: null, diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx index 492685b5e1597..d23469ead98fd 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.test.tsx @@ -86,6 +86,7 @@ beforeEach(() => { user = userEvent.setup(); onSuccess = jest.fn(); + jest.spyOn(auth, 'getMfaChallenge').mockResolvedValueOnce(undefined); jest .spyOn(auth, 'getMfaChallengeResponse') .mockResolvedValueOnce(dummyChallengeResponse); @@ -107,6 +108,7 @@ describe('with passwordless reauthentication', () => { scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'required', }); + expect(auth.getMfaChallengeResponse).toHaveBeenCalled(); } it('changes password', async () => { @@ -127,7 +129,7 @@ describe('with passwordless reauthentication', () => { oldPassword: '', newPassword: 'new-pass1234', secondFactorToken: '', - credential: dummyChallengeResponse.webauthn_response, + webauthnResponse: dummyChallengeResponse.webauthn_response, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -194,7 +196,7 @@ describe('with WebAuthn MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('MFA Device')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getMfaChallengeResponse).toHaveBeenCalledWith({ + expect(auth.getMfaChallenge).toHaveBeenCalledWith({ scope: MfaChallengeScope.CHANGE_PASSWORD, userVerificationRequirement: 'discouraged', }); @@ -222,7 +224,7 @@ describe('with WebAuthn MFA reauthentication', () => { oldPassword: 'current-pass', newPassword: 'new-pass1234', secondFactorToken: '', - credential: dummyChallengeResponse.webauthn_response, + webauthnResponse: dummyChallengeResponse.webauthn_response, }); expect(onSuccess).toHaveBeenCalled(); }); @@ -296,7 +298,7 @@ describe('with OTP MFA reauthentication', () => { ); await user.click(reauthenticateStep.getByText('Authenticator App')); await user.click(reauthenticateStep.getByText('Next')); - expect(auth.getMfaChallengeResponse).not.toHaveBeenCalled(); + expect(auth.getMfaChallenge).not.toHaveBeenCalled(); } it('changes password', async () => { @@ -420,11 +422,11 @@ describe('without reauthentication', () => { 'new-pass1234' ); await user.click(changePasswordStep.getByText('Save Changes')); - expect(auth.getMfaChallengeResponse).not.toHaveBeenCalled(); + expect(auth.getMfaChallenge).not.toHaveBeenCalled(); expect(auth.changePassword).toHaveBeenCalledWith({ oldPassword: 'current-pass', newPassword: 'new-pass1234', - credential: undefined, + webauthnResponse: undefined, secondFactorToken: '', }); expect(onSuccess).toHaveBeenCalled(); diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index cbaed3f4de01c..4303e01768e78 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -38,7 +38,7 @@ import Box from 'design/Box'; import { ChangePasswordReq } from 'teleport/services/auth'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; -import { MfaDevice } from 'teleport/services/mfa'; +import { MfaDevice, WebauthnAssertionResponse } from 'teleport/services/mfa'; export interface ChangePasswordWizardProps { /** MFA type setting, as configured in the cluster's configuration. */ @@ -66,7 +66,8 @@ export function ChangePasswordWizard({ const [reauthMethod, setReauthMethod] = useState( reauthOptions[0]?.value ); - const [credential, setCredential] = useState(); + const [webauthnResponse, setWebauthnResponse] = + useState(); const reauthRequired = reauthOptions.length > 0; return ( @@ -84,9 +85,9 @@ export function ChangePasswordWizard({ // Step properties reauthOptions={reauthOptions} reauthMethod={reauthMethod} - credential={credential} onReauthMethodChange={setReauthMethod} - onAuthenticated={setCredential} + webauthnResponse={webauthnResponse} + onWebauthnResponse={setWebauthnResponse} onClose={onClose} onSuccess={onSuccess} /> @@ -154,7 +155,7 @@ interface ReauthenticateStepProps { reauthOptions: ReauthenticationOption[]; reauthMethod: ReauthenticationMethod; onReauthMethodChange(method: ReauthenticationMethod): void; - onAuthenticated(res: Credential): void; + onWebauthnResponse(res: WebauthnAssertionResponse): void; onClose(): void; } @@ -166,7 +167,7 @@ export function ReauthenticateStep({ reauthOptions, reauthMethod, onReauthMethodChange, - onAuthenticated, + onWebauthnResponse, onClose, }: ChangePasswordWizardStepProps) { const [reauthenticateAttempt, reauthenticate] = useAsync( @@ -184,7 +185,7 @@ export function ReauthenticateStep({ ); // TODO(Joerger): handle non-webauthn response. - onAuthenticated(response.webauthn_response); + onWebauthnResponse(response.webauthn_response); } next(); } @@ -232,7 +233,7 @@ export function ReauthenticateStep({ } interface ChangePasswordStepProps { - credential: Credential; + webauthnResponse: WebauthnAssertionResponse; reauthMethod: ReauthenticationMethod; onClose(): void; onSuccess(): void; @@ -243,7 +244,7 @@ export function ChangePasswordStep({ prev, stepIndex, flowLength, - credential, + webauthnResponse, reauthMethod, onClose, onSuccess, @@ -282,7 +283,7 @@ export function ChangePasswordStep({ oldPassword, newPassword, secondFactorToken: authCode, - credential, + webauthnResponse, }); } diff --git a/web/packages/teleport/src/lib/useMfa.ts b/web/packages/teleport/src/lib/useMfa.ts index d5c82e678d3b9..664016790e002 100644 --- a/web/packages/teleport/src/lib/useMfa.ts +++ b/web/packages/teleport/src/lib/useMfa.ts @@ -20,14 +20,12 @@ import { useState, useEffect, useCallback } from 'react'; import { EventEmitterMfaSender } from 'teleport/lib/EventEmitterMfaSender'; import { TermEvent } from 'teleport/lib/term/enums'; -import { - parseMfaChallengeJson as parseMfaChallenge, - makeWebauthnAssertionResponse, -} from 'teleport/services/mfa/makeMfa'; +import { parseMfaChallengeJson as parseMfaChallenge } from 'teleport/services/mfa/makeMfa'; import { MfaAuthenticateChallengeJson, SSOChallenge, } from 'teleport/services/mfa'; +import auth from 'teleport/services/auth/auth'; export function useMfa(emitterSender: EventEmitterMfaSender): MfaState { const [state, setState] = useState<{ @@ -86,16 +84,17 @@ export function useMfa(emitterSender: EventEmitterMfaSender): MfaState { return; } - navigator.credentials - .get({ publicKey: state.webauthnPublicKey }) + auth + .getMfaChallengeResponse({ + webauthnPublicKey: state.webauthnPublicKey, + }) .then(res => { setState(prevState => ({ ...prevState, errorText: '', webauthnPublicKey: null, })); - const credential = makeWebauthnAssertionResponse(res); - emitterSender.sendWebAuthn(credential); + emitterSender.sendWebAuthn(res.webauthn_response); }) .catch((err: Error) => { setErrorText(err.message); diff --git a/web/packages/teleport/src/services/api/api.test.ts b/web/packages/teleport/src/services/api/api.test.ts index b38849561f087..b9689eeb4210b 100644 --- a/web/packages/teleport/src/services/api/api.test.ts +++ b/web/packages/teleport/src/services/api/api.test.ts @@ -16,6 +16,8 @@ * along with this program. If not, see . */ +import { MfaChallengeResponse } from '../mfa'; + import api, { MFA_HEADER, defaultRequestOptions, @@ -26,18 +28,20 @@ import api, { describe('api.fetch', () => { const mockedFetch = jest.spyOn(global, 'fetch').mockResolvedValue({} as any); // we don't care about response - const webauthnResp = { - id: 'some-id', - type: 'some-type', - extensions: { - appid: false, - }, - rawId: 'some-raw-id', - response: { - authenticatorData: 'authen-data', - clientDataJSON: 'client-data-json', - signature: 'signature', - userHandle: 'user-handle', + const mfaResp: MfaChallengeResponse = { + webauthn_response: { + id: 'some-id', + type: 'some-type', + extensions: { + appid: false, + }, + rawId: 'some-raw-id', + response: { + authenticatorData: 'authen-data', + clientDataJSON: 'client-data-json', + signature: 'signature', + userHandle: 'user-handle', + }, }, }; @@ -88,7 +92,7 @@ describe('api.fetch', () => { }); test('with webauthnResponse', async () => { - await api.fetch('/something', undefined, webauthnResp); + await api.fetch('/something', undefined, mfaResp); expect(mockedFetch).toHaveBeenCalledTimes(1); const firstCall = mockedFetch.mock.calls[0]; @@ -100,14 +104,14 @@ describe('api.fetch', () => { ...defaultRequestOptions.headers, ...getAuthHeaders(), [MFA_HEADER]: JSON.stringify({ - webauthnAssertionResponse: webauthnResp, + webauthnAssertionResponse: mfaResp.webauthn_response, }), }, }); }); test('with customOptions and webauthnResponse', async () => { - await api.fetch('/something', customOpts, webauthnResp); + await api.fetch('/something', customOpts, mfaResp); expect(mockedFetch).toHaveBeenCalledTimes(1); const firstCall = mockedFetch.mock.calls[0]; @@ -120,7 +124,7 @@ describe('api.fetch', () => { ...customOpts.headers, ...getAuthHeaders(), [MFA_HEADER]: JSON.stringify({ - webauthnAssertionResponse: webauthnResp, + webauthnAssertionResponse: mfaResp.webauthn_response, }), }, }); diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index f5570d8ce8e8e..f0f30f7356b6d 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -212,14 +212,13 @@ const auth = { oldPassword, newPassword, secondFactorToken, - credential, + webauthnResponse, }: ChangePasswordReq) { const data = { old_password: base64EncodeUnicode(oldPassword), new_password: base64EncodeUnicode(newPassword), second_factor_token: secondFactorToken, - webauthnAssertionResponse: - credential && makeWebauthnAssertionResponse(credential), + webauthnAssertionResponse: webauthnResponse, }; return api.put(cfg.api.changeUserPasswordPath, data); @@ -342,9 +341,7 @@ const auth = { .then(res => api.post(cfg.api.createPrivilegeTokenPath, { // TODO(Joerger): Handle non-webauthn challenges. - webauthnAssertionResponse: makeWebauthnAssertionResponse( - res.webauthn_response - ), + webauthnAssertionResponse: res.webauthn_response, }) ); }, @@ -390,7 +387,7 @@ const auth = { return auth .getMfaChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal) .then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn')) - .then(res => makeWebauthnAssertionResponse(res.webauthn_response)); + .then(res => res.webauthn_response); }, getMfaChallengeResponseForAdminAction(allowReuse?: boolean) { diff --git a/web/packages/teleport/src/services/auth/types.ts b/web/packages/teleport/src/services/auth/types.ts index ae9818ef2ebb7..7c74f666d9db1 100644 --- a/web/packages/teleport/src/services/auth/types.ts +++ b/web/packages/teleport/src/services/auth/types.ts @@ -18,7 +18,7 @@ import { EventMeta } from 'teleport/services/userEvent'; -import { DeviceUsage } from '../mfa'; +import { DeviceUsage, WebauthnAssertionResponse } from '../mfa'; import { IsMfaRequiredRequest, MfaChallengeScope } from './auth'; @@ -74,7 +74,7 @@ export type ChangePasswordReq = { oldPassword: string; newPassword: string; secondFactorToken: string; - credential?: Credential; + webauthnResponse?: WebauthnAssertionResponse; }; export type CreateNewHardwareDeviceRequest = { From 831528ddd17f4609791b18739e8aa126f66b91ef Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 4 Dec 2024 16:29:03 -0800 Subject: [PATCH 6/6] Fix lint. --- .../ChangePasswordWizard.story.tsx | 21 ++++++++++++------- .../ChangePasswordWizard.tsx | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx index 07283ae6c1c8d..3ec850a0aeba7 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.story.tsx @@ -23,10 +23,11 @@ import Dialog from 'design/Dialog'; import { createTeleportContext } from 'teleport/mocks/contexts'; import { ContextProvider } from 'teleport'; -import { MfaDevice } from 'teleport/services/mfa'; +import { MfaDevice, WebauthnAssertionResponse } from 'teleport/services/mfa'; import { ChangePasswordStep, + ChangePasswordWizardStepProps, ReauthenticateStep, createReauthOptions, } from './ChangePasswordWizard'; @@ -107,12 +108,16 @@ const stepProps = { flowLength: 2, refCallback: () => {}, - // Other props - reauthOptions: defaultReauthOptions, - reauthMethod: defaultReauthOptions[0].value, - credential: { id: '', type: '' }, - onReauthMethodChange() {}, - onAuthenticated() {}, + // Shared props + reauthMethod: 'mfaDevice', onClose() {}, onSuccess() {}, -}; + + // ReauthenticateStepProps + reauthOptions: defaultReauthOptions, + onReauthMethodChange() {}, + onWebauthnResponse() {}, + + // ChangePasswordStepProps + webauthnResponse: {} as WebauthnAssertionResponse, +} satisfies ChangePasswordWizardStepProps; diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index 4303e01768e78..3a66e65a070ce 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -147,7 +147,7 @@ const wizardFlows = { withoutReauthentication: [ChangePasswordStep], }; -type ChangePasswordWizardStepProps = StepComponentProps & +export type ChangePasswordWizardStepProps = StepComponentProps & ReauthenticateStepProps & ChangePasswordStepProps;