Skip to content

Commit

Permalink
Use challenge response when adding MFA device
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
avatus committed Oct 17, 2024
1 parent 3e75921 commit 3f2b160
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 6 deletions.
6 changes: 6 additions & 0 deletions web/packages/teleport/src/Account/Account.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<MfaDevice[]>([]);
Expand All @@ -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);
Expand Down
17 changes: 16 additions & 1 deletion web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ import {
ChangePasswordReq,
CreateNewHardwareDeviceRequest,
DeviceUsage,
CreateAuthenticateChallengeRequest,
} from './types';
import { CreateAuthenticateChallengeRequest } from './types';

const auth = {
checkWebauthnSupport() {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions web/packages/teleport/src/services/auth/makeMfa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function makeMfaAuthenticateChallenge(json): MfaAuthenticateChallenge {
}

return {
totpChallenge: json.totp_challenge,
webauthnPublicKey: webauthnPublicKey,
};
}
Expand Down
1 change: 1 addition & 0 deletions web/packages/teleport/src/services/auth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export type AuthnChallengeRequest = {
};

export type MfaAuthenticateChallenge = {
totpChallenge: boolean;
webauthnPublicKey: PublicKeyCredentialRequestOptions;
};

Expand Down

0 comments on commit 3f2b160

Please sign in to comment.