Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow up on Web MFA TODOs #50417

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ export function AddAuthDeviceWizard({
const reauthState = useReAuthenticate({
challengeScope: MfaChallengeScope.MANAGE_DEVICES,
onMfaResponse: mfaResponse =>
// TODO(Joerger): Instead of getting a privilege token, we should get
// // a register challenge with the mfa response directly. For good UX, this would
// // require some refactoring to the flow so the user can choose a device type before
// // completing an mfa check and getting an otp/webauthn register challenge, or
// // allowing the backend to return a flexible register challenge
// await auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken);
auth.createPrivilegeToken(mfaResponse).then(setPrivilegeToken),
});

Expand Down Expand Up @@ -188,7 +182,6 @@ export function CreateDeviceStep({
if (usage === 'passwordless' || newMfaDeviceType === 'webauthn') {
createPasskeyAttempt.run(async () => {
const credential = await auth.createNewWebAuthnDevice({
// TODO(Joerger): Skip privilege token step, just pass in mfa response.
tokenId: privilegeToken,
deviceUsage: usage,
});
Expand Down
59 changes: 0 additions & 59 deletions web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ const auth = {
});
},

// TODO(Joerger): Delete once no longer used by /e
async getSsoChallengeResponse(
challenge: SsoChallenge
): Promise<MfaChallengeResponse> {
Expand Down Expand Up @@ -386,63 +385,10 @@ const auth = {
};
},

// TODO(Joerger): Delete once no longer used by /e
createPrivilegeTokenWithWebauthn() {
return auth
.getMfaChallenge({ scope: MfaChallengeScope.MANAGE_DEVICES })
.then(auth.getMfaChallengeResponse)
.then(mfaResp => auth.createPrivilegeToken(mfaResp));
},

// TODO(Joerger): Delete once no longer used by /e
createPrivilegeTokenWithTotp(secondFactorToken: string) {
return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken });
},

createRestrictedPrivilegeToken() {
return api.post(cfg.api.createPrivilegeTokenPath, {});
},

// TODO(Joerger): Remove once /e is no longer using it.
async getWebauthnResponse(
scope: MfaChallengeScope,
allowReuse?: boolean,
isMfaRequiredRequest?: IsMfaRequiredRequest,
abortSignal?: AbortSignal
) {
// TODO(Joerger): DELETE IN 16.0.0
// the create mfa challenge endpoint below supports
// MFARequired requests without the extra roundtrip.
if (isMfaRequiredRequest) {
try {
const isMFARequired = await checkMfaRequired(
isMfaRequiredRequest,
abortSignal
);
if (!isMFARequired.required) {
return;
}
} catch (err) {
if (
err?.response?.status === 400 &&
err?.message.includes('missing target for MFA check')
) {
// checking MFA requirement for admin actions is not supported by old
// auth servers, we expect an error instead. In this case, assume MFA is
// not required. Callers should fallback to retrying with MFA if needed.
return;
}

throw err;
}
}

return auth
.getMfaChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal)
.then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn'))
.then(res => res.webauthn_response);
},

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.
Expand All @@ -460,11 +406,6 @@ const auth = {
})
.then(auth.getMfaChallengeResponse);
},

// TODO(Joerger): Delete in favor of getMfaChallengeResponseForAdminAction once /e is updated.
getWebauthnResponseForAdminAction(allowReuse?: boolean) {
return auth.getMfaChallengeResponseForAdminAction(allowReuse);
},
};

function checkMfaRequired(
Expand Down
Loading