Skip to content

feat(settings): support step-up auth in sign-in flow - #20992

Open
StaberindeZA wants to merge 2 commits into
mainfrom
fxa-12856
Open

feat(settings): support step-up auth in sign-in flow#20992
StaberindeZA wants to merge 2 commits into
mainfrom
fxa-12856

Conversation

@StaberindeZA

Copy link
Copy Markdown
Contributor

Because

  • RFC 9470 step-up requires the authorization endpoint to re-challenge when a session doesn't meet the requested authentication level or is older than max_age; max_age was rejected with a 400 and no freshness check existed
  • an RP requesting step-up (acr_values=AAL2 / max_age) must be challenged for a second factor without prompting for a password, and passkey users were wrongly forced into TOTP enrollment they don't need

This pull request

  • accepts max_age in both /authorization payload schemas
  • enforces max_age freshness in validateRequestedGrant, with a small leeway so a just-completed challenge (incl. max_age=0) is not looped
  • throws INSUFFICIENT_ACR_VALUES (errno 170) for both acr_values and max_age failures, the signal the frontend routes to step-up (was 120, which the frontend ignored)
  • decouples max_age from the password prompt and forwards max_age to the backend so it can enforce authentication freshness
  • does not force TOTP enrollment for passkey sessions (already AAL2)

Issue that this pull request solves

Closes: FXA-12856, FXA-12861

Checklist

Put an x in the boxes that apply

  • 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.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

Because:

* RFC 9470 step-up requires the authorization endpoint to re-challenge
  when a session doesn't meet the requested authentication level or is
  older than max_age; max_age was rejected with a 400 and no freshness
  check existed

This commit:

* accepts max_age in both /authorization payload schemas
* enforces max_age freshness in validateRequestedGrant, with a small
  leeway so a just-completed challenge (incl. max_age=0) is not looped
* throws INSUFFICIENT_ACR_VALUES (errno 170) for both acr_values and
  max_age failures, the signal the frontend routes to step-up (was 120,
  which the frontend ignored)
* adds unit tests for the acr and max_age paths

Closes FXA-12856
@StaberindeZA
StaberindeZA requested a review from a team as a code owner August 7, 2026 14:48
Copilot AI review requested due to automatic review settings August 7, 2026 14:48
@StaberindeZA
StaberindeZA changed the base branch from main to fxa-14308 August 7, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 implements RFC 9470 step-up authentication support across FxA Settings (frontend) and the auth-server (backend), enabling RPs to request re-challenge via acr_values and/or max_age without forcing password re-entry, and fixing passkey sessions being misrouted into TOTP enrollment.

Changes:

  • Frontend: adjusts sign-in navigation for step-up vs Settings-originated AAL upgrades, avoids forcing TOTP enrollment for passkey sessions, and forwards max_age to the backend.
  • Backend: accepts max_age in /authorization payload schemas and enforces session freshness in validateRequestedGrant with a small leeway to avoid challenge loops.
  • Error signaling: uses errno 170 (INSUFFICIENT_ACR_VALUES) for ACR/max_age failures so the frontend step-up handlers trigger correctly; client typing updated to include max_age.

React review (fxa-check-react)

No React/TSX issues found in the changed TSX file.

Clean categories: Component Design & Hooks, Performance, Accessibility (a11y), State Management, Testing Patterns (React/TSX)

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/fxa-settings/src/pages/Signin/utils.ts Updates navigation logic for step-up vs Settings AAL upgrades and prevents passkey sessions from being forced into TOTP setup.
packages/fxa-settings/src/pages/Signin/utils.test.ts Adds/updates routing tests covering passkey AAL2 behavior and step-up vs Settings-originated AAL upgrades.
packages/fxa-settings/src/models/integrations/oauth-web-integration.ts Stops treating max_age=0 as a password-prompt trigger; keeps prompt=login as the only password re-entry signal.
packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts Adds coverage for updated wantsLogin() behavior around max_age.
packages/fxa-settings/src/lib/oauth/hooks.tsx Forwards max_age to auth-server when creating OAuth codes.
packages/fxa-auth-server/test/remote/oauth_api.in.spec.ts Updates expectations to errno 170 and the new message for insufficient ACR values.
packages/fxa-auth-server/lib/routes/oauth/authorization.js Extends Joi validation schemas to accept max_age.
packages/fxa-auth-server/lib/oauth/grant.spec.ts Updates ACR mismatch assertions to errno 170 and adds max_age freshness/leeway tests.
packages/fxa-auth-server/lib/oauth/grant.js Enforces max_age freshness with leeway and switches ACR failures to AppError.insufficientACRValues (errno 170).
packages/fxa-auth-server/docs/swagger/shared/descriptions.ts Documents the new maxAge swagger description for max_age.
packages/fxa-auth-client/lib/client.ts Adds max_age typing and forwards it in createOAuthCode request options.
Suppressed comments (1)

packages/fxa-settings/src/pages/Signin/utils.test.ts:702

  • This assertion can miss a call to navigate('/settings', { ... }) because it only checks for the exact single-arg call shape. Consider asserting based on the first argument across all calls so the test remains correct if navigate options are added later.
        expect(mockNavigate).not.toHaveBeenCalledWith('/settings');

Comment on lines +603 to +606
expect(mockNavigate).not.toHaveBeenCalledWith(
'/inline_totp_setup?client_id=abc',
expect.objectContaining({ replace: true })
expect.anything()
);
Because:

* an RP requesting step-up (acr_values=AAL2 / max_age) must be challenged
  for a second factor without prompting for a password, and passkey users
  were wrongly forced into TOTP enrollment they don't need

This commit:

* decouples max_age from the password prompt and forwards max_age to the
  backend so it can enforce authentication freshness
* does not force TOTP enrollment for passkey sessions (already AAL2)

Closes #FXA-12861
Base automatically changed from fxa-14308 to main August 10, 2026 16:14
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.

2 participants