mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-21 15:04:25 +00:00
fix(login): treat user-verified passkey as fulfilling MFA in session validity check (#12575)
# Which Problems Are Solved
Users with a passkey plus an additional configured second factor (TOTP /
OTP Email / OTP SMS) get stuck on the passkey verification page: the
WebAuthn ceremony succeeds, but the continue button just spins and the
user is redirected back to `/passkey` indefinitely. Re-registering the
passkey or switching to a private window does not help.
Since the MFA hardening of `isSessionValid` (56f4798), a session is only
considered valid if one of the user's *configured* MFA methods (TOTP,
OTP Email, OTP SMS, U2F) was verified within the session. A passkey
login sets `factors.webAuthN.verifiedAt` with `userVerified: true`, but
only the U2F method reuses that factor — so a passkey session for a user
whose second factor is TOTP or OTP is deemed invalid at flow completion.
This contradicts the login flow itself: `checkMFAFactors` intentionally
never prompts for a second factor after a passkey login ("escape further
checks if user has authenticated with passkey"). The result is a loop:
1. Passkey is verified on `/passkey` → `sendPasskey` →
`completeAuthFlow` → `loginWithOIDCAndSession`
2. `isSessionValid` returns `false` because the configured TOTP/OTP
factor was never checked in this session
3. The re-authentication branch calls `sendLoginname`, which resolves
the user's preferred method (passkey) and returns a redirect to
`/passkey` — back to step 1, with no way forward
# How the Problems Are Solved
Treat a user-verified passkey authentication as fulfilling the MFA
requirement in `isSessionValid`, consistent with the existing passkey
escape in `checkMFAFactors`. A passkey with user verification is
inherently multi-factor (possession + biometrics/PIN).
- `isSessionValid` now derives `hasAuthenticatedWithPasskey` from
`factors.webAuthN.verifiedAt && factors.webAuthN.userVerified` and
accepts it alongside the configured-factor checks (`totpValid ||
otpEmailValid || otpSmsValid || u2fValid`).
- A presence-only WebAuthn assertion (`userVerified: false`, i.e. a
U2F-style second-factor check) does **not** count, so password-based
sessions still require their configured second factor — the original
security fix remains intact.
- Adds regression tests: a user-verified passkey session with a
configured-but-unverified TOTP factor is valid; a presence-only WebAuthn
session with an unverified TOTP factor remains invalid.
This commit is contained in:
@@ -956,6 +956,92 @@ describe("isSessionValid", () => {
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return true when authenticated with user-verified passkey even if TOTP is configured but not verified", async () => {
|
||||
// Regression: a user with passkey + password + TOTP got stuck in a redirect loop on
|
||||
// /passkey — the flow never prompts for TOTP after a passkey login (checkMFAFactors
|
||||
// escapes), but isSessionValid deemed the session invalid because the configured
|
||||
// TOTP factor was not verified. A user-verified passkey is inherently multi-factor.
|
||||
const verifiedTimestamp = createMockTimestamp();
|
||||
const session = createMockSession({
|
||||
factors: {
|
||||
user: {
|
||||
id: mockUserId,
|
||||
organizationId: mockOrganizationId,
|
||||
loginName: "[email protected]",
|
||||
displayName: "Test User",
|
||||
verifiedAt: verifiedTimestamp,
|
||||
},
|
||||
webAuthN: {
|
||||
verifiedAt: verifiedTimestamp,
|
||||
userVerified: true,
|
||||
},
|
||||
// No password factor, TOTP configured but not verified in this session
|
||||
},
|
||||
});
|
||||
|
||||
vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({
|
||||
authMethodTypes: [
|
||||
AuthenticationMethodType.PASSWORD,
|
||||
AuthenticationMethodType.PASSKEY,
|
||||
AuthenticationMethodType.TOTP,
|
||||
],
|
||||
} as any);
|
||||
|
||||
vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({
|
||||
forceMfa: false,
|
||||
forceMfaLocalOnly: false,
|
||||
} as any);
|
||||
|
||||
vi.mocked(verifyHelperModule.shouldEnforceMFA).mockReturnValue(false);
|
||||
|
||||
const result = await isSessionValid({ serviceConfig: { baseUrl: mockServiceUrl }, session });
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("should return false when WebAuthn was presence-only (no userVerified) and configured TOTP is not verified", async () => {
|
||||
// A presence-only WebAuthn assertion (U2F-style, userVerified=false) is not a passkey
|
||||
// login and must not bypass the configured-MFA requirement.
|
||||
const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const verifiedTimestamp = createMockTimestamp();
|
||||
const session = createMockSession({
|
||||
factors: {
|
||||
user: {
|
||||
id: mockUserId,
|
||||
organizationId: mockOrganizationId,
|
||||
loginName: "[email protected]",
|
||||
displayName: "Test User",
|
||||
verifiedAt: verifiedTimestamp,
|
||||
},
|
||||
password: {
|
||||
verifiedAt: verifiedTimestamp,
|
||||
},
|
||||
webAuthN: {
|
||||
verifiedAt: verifiedTimestamp,
|
||||
userVerified: false,
|
||||
},
|
||||
// TOTP configured but not verified in this session
|
||||
},
|
||||
});
|
||||
|
||||
vi.mocked(zitadelModule.listAuthenticationMethodTypes).mockResolvedValue({
|
||||
authMethodTypes: [AuthenticationMethodType.PASSWORD, AuthenticationMethodType.TOTP],
|
||||
} as any);
|
||||
|
||||
vi.mocked(zitadelModule.getLoginSettings).mockResolvedValue({
|
||||
forceMfa: true,
|
||||
forceMfaLocalOnly: false,
|
||||
} as any);
|
||||
|
||||
vi.mocked(verifyHelperModule.shouldEnforceMFA).mockReturnValue(true);
|
||||
|
||||
const result = await isSessionValid({ serviceConfig: { baseUrl: mockServiceUrl }, session });
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(consoleSpy).toHaveBeenCalledWith("[Session] MFA is required but not valid");
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("IDP authentication", () => {
|
||||
|
||||
@@ -126,14 +126,22 @@ export async function isSessionValid({
|
||||
method === AuthenticationMethodType.U2F,
|
||||
);
|
||||
|
||||
// A user-verified passkey (possession + user verification) is inherently multi-factor.
|
||||
// Mirrors the passkey escape in checkMFAFactors, which never prompts for an additional
|
||||
// second factor after a passkey login. Without this, users with a passkey plus a
|
||||
// configured TOTP/OTP factor get stuck in a redirect loop on /passkey, because the
|
||||
// flow never asks for the second factor but this check would deem the session invalid.
|
||||
const hasAuthenticatedWithPasskey = !!session.factors.webAuthN?.verifiedAt && !!session.factors.webAuthN?.userVerified;
|
||||
|
||||
if (mfaMethods && mfaMethods.length > 0) {
|
||||
// User has MFA methods configured — they must be verified regardless of policy
|
||||
// User has MFA methods configured — they must be verified regardless of policy,
|
||||
// unless the session was already authenticated with a user-verified passkey
|
||||
const totpValid = mfaMethods.includes(AuthenticationMethodType.TOTP) && !!session.factors.totp?.verifiedAt;
|
||||
const otpEmailValid = mfaMethods.includes(AuthenticationMethodType.OTP_EMAIL) && !!session.factors.otpEmail?.verifiedAt;
|
||||
const otpSmsValid = mfaMethods.includes(AuthenticationMethodType.OTP_SMS) && !!session.factors.otpSms?.verifiedAt;
|
||||
const u2fValid = mfaMethods.includes(AuthenticationMethodType.U2F) && !!session.factors.webAuthN?.verifiedAt;
|
||||
|
||||
mfaValid = totpValid || otpEmailValid || otpSmsValid || u2fValid;
|
||||
mfaValid = hasAuthenticatedWithPasskey || totpValid || otpEmailValid || otpSmsValid || u2fValid;
|
||||
} else if (isMfaRequired) {
|
||||
// No MFA methods configured, but MFA is forced by policy — check for any verified MFA factors
|
||||
const otpEmail = session.factors.otpEmail?.verifiedAt;
|
||||
|
||||
Reference in New Issue
Block a user