Skip to content

fix(oauth): fail prompt=none step-up instead of challenging - #21103

Open
StaberindeZA wants to merge 1 commit into
mainfrom
fxa-12860
Open

fix(oauth): fail prompt=none step-up instead of challenging#21103
StaberindeZA wants to merge 1 commit into
mainfrom
fxa-12860

Conversation

@StaberindeZA

@StaberindeZA StaberindeZA commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Because

  • RFC 9470 section 5 requires the authorization request to fail with unmet_authentication_requirements when the requested authentication level cannot be reached. FxA had no such path.
  • A prompt=none step-up request routed the user to an interactive 2FA page, which the parameter forbids, and the RP was never told its request failed.

This pull request

  • Adds UNMET_AUTHENTICATION_REQUIREMENTS to OAUTH_ERRORS, carrying response_error_code: 'unmet_authentication_requirements'.
  • Fails prompt=none step-up requests in getOAuthNavigationTarget instead of routing to an interactive challenge, on errno 170 only.
  • Gates that on a new isPromptNoneRequest navigation option set only by the Authorization container, the one caller that relays a returned error to the RP.
  • Documents errno 170 and the step-up parameters on POST /oauth/authorization.
  • Pins that the id_token acr claim reflects the achieved AAL.

Issue that this pull request solves

Closes: FXA-12860

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: the prompt=none short-circuit in packages/fxa-settings/src/pages/Signin/utils.ts, and where isPromptNoneRequest is set in packages/fxa-settings/src/pages/Authorization/container.tsx.
  • Suggested review order: Signin/utils.tsAuthorization/container.tsx → tests.
  • Risky or complex parts: the gate is on the caller's flag rather than integration.wantsPromptNone(), because prompt=none can outlive the authorization route in the query string. Every other handleNavigation caller renders errors in-FxA, so returning one to them would dead-end the user instead of completing enrolment.

Screenshots (Optional)

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

Other information (Optional)

Scoped to prompt=none. The ticket also describes returning this error when a user declines to enrol a second factor, which is not in this PR:

  • Inline TOTP setup is handled upstream — refactor(settings): scope inline TOTP MFA guard to enrolment step #21097 rescoped its MFA guard to the enrolment step, so dismissing it returns to the intro screen rather than leaving the flow. That is an in-flow back action, not a decline.
  • Recovery setup was implemented and then removed. MfaGuardCore only renders its modal when the mfa:2fa JWT is absent, and that JWT is already minted during TOTP setup — so onDismiss there is reachable only via a stale-JWT re-prompt mid-enrolment, where treating it as a decline would discard a confirmed phone number and generated backup codes.
  • Abandoning an interactive 2FA challenge (/signin_totp_code, /signin_token_code) still dead-ends without notifying the RP. Those surfaces have no decline control and none was added.

The decline half wants its own ticket with UX input. FXA-14408 covers the adjacent prompt=none gap for unverified sessions, where interaction_required is the correct code rather than unmet_authentication_requirements.

@StaberindeZA
StaberindeZA force-pushed the fxa-12860 branch 3 times, most recently from 0c49c2c to 77a6f72 Compare August 25, 2026 22:36
@StaberindeZA StaberindeZA changed the title feat(oauth): return unmet_authentication_requirements to the RP fix(oauth): return an error to the RP when step-up cannot complete Aug 25, 2026
@StaberindeZA
StaberindeZA force-pushed the fxa-12860 branch 4 times, most recently from d753561 to e4872c6 Compare August 27, 2026 15:29
@StaberindeZA StaberindeZA changed the title fix(oauth): return an error to the RP when step-up cannot complete fix(oauth): fail prompt=none step-up instead of challenging Aug 27, 2026
@StaberindeZA
StaberindeZA force-pushed the fxa-12860 branch 2 times, most recently from 10c2a66 to 3cad712 Compare August 27, 2026 15:55
@StaberindeZA
StaberindeZA marked this pull request as ready for review August 27, 2026 17:02
@StaberindeZA
StaberindeZA requested a review from a team as a code owner August 27, 2026 17:02
Copilot AI balanced review requested due to automatic review settings August 27, 2026 17:02

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

Adds RFC 9470-compliant handling for prompt=none OAuth step-up failures.

Changes:

  • Maps errno 170 to unmet_authentication_requirements.
  • Redirects eligible failures to the RP.
  • Adds documentation and unit/functional coverage.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Signin/utils.ts Maps step-up failures to the OAuth error.
Signin/utils.test.ts Tests prompt-none navigation behavior.
Signin/interfaces.ts Adds the prompt-none navigation flag.
Authorization/container.tsx Supplies the flag and relays errors.
Authorization/container.test.tsx Tests RP error redirection.
oauth-web-integration.test.ts Tests error URL and state handling.
oauth-errors.ts Defines the new OAuth error.
grant.spec.ts Verifies achieved AAL claims.
oauth-api.ts Documents step-up behavior and errno 170.
stepUpAuth.spec.ts Adds end-to-end prompt-none coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/fxa-settings/src/pages/Authorization/container.tsx Outdated
Comment thread packages/fxa-auth-server/docs/swagger/oauth-api.ts Outdated
Because:

* RFC 9470 section 5 requires the authorization request to fail with
  unmet_authentication_requirements when the requested authentication
  level cannot be reached; FxA had no such path
* a prompt=none step-up routed the user to an interactive 2FA page,
  which the parameter forbids, and the RP was never told

This commit:

* adds an UNMET_AUTHENTICATION_REQUIREMENTS oauth error carrying the
  RFC error code, relayed by the authorization route's existing
  getRedirectWithErrorUrl path
* fails prompt=none step-up requests instead of routing to a challenge,
  for errno 170 only — an unverified session is interaction_required
* documents errno 170 and the step-up parameters on /oauth/authorization
* pins that the id_token acr claim reflects the achieved AAL

Closes #FXA-12860
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