Skip to content

fix(settings): divert passkey AAL2 sessions without TOTP to inline TO…#20680

Merged
vbudhram merged 1 commit into
mainfrom
fix-amo
Jun 3, 2026
Merged

fix(settings): divert passkey AAL2 sessions without TOTP to inline TO…#20680
vbudhram merged 1 commit into
mainfrom
fix-amo

Conversation

@vbudhram
Copy link
Copy Markdown
Contributor

@vbudhram vbudhram commented Jun 3, 2026

Because

  • A passkey or passwordless+passkey session is session-AAL2, so an AAL2 RP (e.g. AMO) is satisfied at the OAuth grant, but those RPs require account-level 2FA (TOTP), which a passkey does not provide.
  • With no TOTP, the grant keeps succeeding while the RP keeps rejecting, leaving the user looping on the FxA cached-signin screen.

This pull request

  • Removes the isPasskeySession gate on the inline-TOTP-setup divert in utils.ts, so it fires for any session satisfying session-AAL2 without account 2FA (fresh passkey ceremony or cached passkey session); the divert stays gated on wantsTwoStepAuthentication() + accountHasTotp === false.
  • Updates SigninCached/index.tsx to forward accountHasTotp: data.totpIsActive (already returned by the cached-signin handler, no new network call).
  • Adds totpIsActive to CachedSigninHandlerResponse (interfaces.ts) and the mock (mocks.tsx).

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-13883

Checklist

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

Other information

How to test (local, passkeys enabled):

  1. Create a passwordless or password account; register a passkey; sign out.
  2. Sign in with the passkey (session-AAL2, no TOTP).
  3. Hit a profile-AAL2 RP (123done "Sign In (Require Profile AAL2)").
  4. Expect a divert to /inline_totp_setup (not a cached-signin loop). With TOTP enrolled, sign-in completes with no divert.

…TP setup

Because:
 - A passkey or passwordless+passkey session is session-AAL2, so AAL2 RPs (e.g. AMO) are satisfied at the OAuth grant. Those RPs require account-level 2FA (TOTP), which a passkey does not provide, so with no TOTP the grant keeps succeeding while the RP keeps rejecting, looping the user on the cached-signin screen.

This commit:
 - Removes the isPasskeySession gate on the inline-TOTP-setup divert so it fires for any session satisfying session-AAL2 without account 2FA (fresh passkey ceremony or cached passkey session).
 - SigninCached forwards accountHasTotp from the cached-signin response (totpIsActive) so the divert runs on the cached path.
 - Adds a unit case for the cached (non-ceremony) divert plus functional tests for the cached passkey AAL2 session with and without TOTP.
Copilot AI review requested due to automatic review settings June 3, 2026 14:34
@vbudhram vbudhram requested a review from a team as a code owner June 3, 2026 14:34
@vbudhram vbudhram self-assigned this Jun 3, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an OAuth AAL2 relier (e.g., AMO) “cached sign-in loop” for passkey-based sessions by ensuring users without account-level 2FA (TOTP) are diverted to inline TOTP enrollment even when the session is reused (cached) and not the result of a fresh passkey ceremony.

Changes:

  • Removes the isPasskeySession gating so the inline-TOTP-setup divert triggers for any OAuth-web integration that wants 2FA when accountHasTotp === false.
  • Plumbs totpIsActive through the cached-signin response and forwards it as accountHasTotp into handleNavigation.
  • Adds/updates unit + functional tests covering the cached-passkey-session divert behavior (and the “already has TOTP” non-divert case).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/fxa-settings/src/pages/Signin/utils.ts Broadens the inline TOTP setup divert condition by removing the passkey-session-only gate.
packages/fxa-settings/src/pages/Signin/utils.test.ts Adds a unit test ensuring cached passkey sessions without TOTP still divert to inline TOTP setup.
packages/fxa-settings/src/pages/Signin/mocks.tsx Updates cached-signin mock response to include totpIsActive.
packages/fxa-settings/src/pages/Signin/interfaces.ts Extends CachedSigninHandlerResponse to include totpIsActive in data.
packages/fxa-settings/src/pages/Signin/components/SigninCached/index.tsx Forwards data.totpIsActive as accountHasTotp into navigation handling.
packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts Adds functional coverage for cached passkey sessions (with/without TOTP) against AMO-style profile AAL2 behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the fix makes sense. Appreciate the added tests! :shipit:

@vbudhram vbudhram merged commit ef2f079 into main Jun 3, 2026
22 checks passed
@vbudhram vbudhram deleted the fix-amo branch June 3, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants