From 3f2b1607a2c906b786c916f55617391ea0305f76 Mon Sep 17 00:00:00 2001 From: Michael Myers Date: Tue, 15 Oct 2024 14:24:41 -0500 Subject: [PATCH] Use challenge response when adding MFA device This PR changes the requirement for the "Reauthenticate" step when adding an MFA device. Currently, we rely on the number of devices returned when we fetch the devices. However, in the future with SSOasMFA, these devices won't always be enabled. This will allow us to check the backend if any of the available devices are "enabled" and if we've received a valid challenge for a device. if not, we forward the user through the privilege token flow instead. This will be expanded by checking for SSO challenges as well and can now be expanded for any other type of auth. --- .../teleport/src/Account/Account.test.tsx | 6 ++++++ .../ChangePasswordWizard.tsx | 4 +--- .../Account/ManageDevices/useManageDevices.ts | 12 ++++++++++-- web/packages/teleport/src/services/auth/auth.ts | 17 ++++++++++++++++- .../teleport/src/services/auth/makeMfa.ts | 1 + .../teleport/src/services/auth/types.ts | 1 + 6 files changed, 35 insertions(+), 6 deletions(-) diff --git a/web/packages/teleport/src/Account/Account.test.tsx b/web/packages/teleport/src/Account/Account.test.tsx index fb3d4534d651f..6fb23549a0e36 100644 --- a/web/packages/teleport/src/Account/Account.test.tsx +++ b/web/packages/teleport/src/Account/Account.test.tsx @@ -243,6 +243,9 @@ 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({ webauthnPublicKey: null, totpChallenge: true }); jest .spyOn(auth, 'createNewWebAuthnDevice') .mockResolvedValueOnce(dummyCredential); @@ -322,6 +325,9 @@ 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({ webauthnPublicKey: null, totpChallenge: false }); jest .spyOn(auth, 'createPrivilegeTokenWithWebauthn') .mockResolvedValueOnce('webauthn-privilege-token'); diff --git a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx index d5e17e71be03a..458357e83ed54 100644 --- a/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx +++ b/web/packages/teleport/src/Account/ChangePasswordWizard/ChangePasswordWizard.tsx @@ -22,7 +22,7 @@ import { ButtonPrimary, ButtonSecondary } from 'design/Button'; import Dialog from 'design/Dialog'; import Flex from 'design/Flex'; import { RadioGroup } from 'design/RadioGroup'; -import { StepComponentProps, StepSlider } from 'design/StepSlider'; +import { StepComponentProps, StepSlider, StepHeader } from 'design/StepSlider'; import React, { useState } from 'react'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; @@ -36,8 +36,6 @@ import { Auth2faType } from 'shared/services'; import Box from 'design/Box'; -import { StepHeader } from 'design/StepSlider'; - import { ChangePasswordReq } from 'teleport/services/auth'; import auth, { MfaChallengeScope } from 'teleport/services/auth/auth'; import { MfaDevice } from 'teleport/services/mfa'; diff --git a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts index 12bfaf6743f04..95929bf9d9714 100644 --- a/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts +++ b/web/packages/teleport/src/Account/ManageDevices/useManageDevices.ts @@ -23,6 +23,7 @@ import Ctx from 'teleport/teleportContext'; import cfg from 'teleport/config'; import auth, { DeviceUsage } from 'teleport/services/auth'; import { MfaDevice } from 'teleport/services/mfa'; +import { MfaChallengeScope } from 'teleport/services/auth/auth'; export default function useManageDevices(ctx: Ctx) { const [devices, setDevices] = useState([]); @@ -45,9 +46,16 @@ export default function useManageDevices(ctx: Ctx) { ); } - function onAddDevice(usage: DeviceUsage) { + async function onAddDevice(usage: DeviceUsage) { setNewDeviceUsage(usage); - if (devices.length === 0) { + const response = await auth.getChallenge({ + 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) { createRestrictedTokenAttempt.run(() => auth.createRestrictedPrivilegeToken().then(token => { setToken(token); diff --git a/web/packages/teleport/src/services/auth/auth.ts b/web/packages/teleport/src/services/auth/auth.ts index 5059ac5646e6c..4dec9b2ade0f0 100644 --- a/web/packages/teleport/src/services/auth/auth.ts +++ b/web/packages/teleport/src/services/auth/auth.ts @@ -37,8 +37,8 @@ import { ChangePasswordReq, CreateNewHardwareDeviceRequest, DeviceUsage, + CreateAuthenticateChallengeRequest, } from './types'; -import { CreateAuthenticateChallengeRequest } from './types'; const auth = { checkWebauthnSupport() { @@ -258,6 +258,21 @@ const auth = { return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken }); }, + async getChallenge( + req: CreateAuthenticateChallengeRequest, + abortSignal?: AbortSignal + ) { + return api + .post( + cfg.api.mfaAuthnChallengePath, + { + challenge_scope: req.scope, + }, + abortSignal + ) + .then(makeMfaAuthenticateChallenge); + }, + async fetchWebAuthnChallenge( req: CreateAuthenticateChallengeRequest, abortSignal?: AbortSignal diff --git a/web/packages/teleport/src/services/auth/makeMfa.ts b/web/packages/teleport/src/services/auth/makeMfa.ts index 20957e7a773d9..0637967483911 100644 --- a/web/packages/teleport/src/services/auth/makeMfa.ts +++ b/web/packages/teleport/src/services/auth/makeMfa.ts @@ -70,6 +70,7 @@ export function makeMfaAuthenticateChallenge(json): MfaAuthenticateChallenge { } return { + totpChallenge: json.totp_challenge, webauthnPublicKey: webauthnPublicKey, }; } diff --git a/web/packages/teleport/src/services/auth/types.ts b/web/packages/teleport/src/services/auth/types.ts index 078464ed8a52a..11057cd185645 100644 --- a/web/packages/teleport/src/services/auth/types.ts +++ b/web/packages/teleport/src/services/auth/types.ts @@ -33,6 +33,7 @@ export type AuthnChallengeRequest = { }; export type MfaAuthenticateChallenge = { + totpChallenge: boolean; webauthnPublicKey: PublicKeyCredentialRequestOptions; };