docs(oauth): document step-up claims and parameters in the OpenAPI spec - #21121
Draft
StaberindeZA wants to merge 1 commit into
Draft
docs(oauth): document step-up claims and parameters in the OpenAPI spec#21121StaberindeZA wants to merge 1 commit into
StaberindeZA wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Documents OAuth step-up parameters, claims, errors, and token behavior in the generated OpenAPI reference.
Changes:
- Expands shared descriptions for step-up parameters and claims.
- Adds step-up guidance and examples to OAuth endpoints.
- Annotates introspection claim schemas.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/fxa-auth-server/docs/swagger/shared/descriptions.ts |
Defines RP-facing claim and parameter descriptions. |
packages/fxa-auth-server/docs/swagger/oauth-server-api.ts |
Updates endpoint notes, errors, and examples. |
packages/fxa-auth-server/lib/routes/oauth/introspect.js |
Attaches descriptions to introspection claims. |
Suppressed comments (4)
packages/fxa-auth-server/docs/swagger/oauth-server-api.ts:97
- This documents
prompt=noneas already redirecting withunmet_authentication_requirements, but the linked #21103 that adds this behavior is still open and blocked; the current settings flow routes errno 170 to an interactive challenge. Block this PR on #21103 or qualify/remove this claim until that change lands so the deployed OpenAPI reference does not advertise unavailable behavior.
retrying. Where it cannot challenge — the RP also sent \`prompt=none\` — the UI instead
redirects to the registered \`redirect_uri\` with
\`error=unmet_authentication_requirements\`.
packages/fxa-auth-server/docs/swagger/oauth-server-api.ts:134
- This documents
prompt=noneas already redirecting withunmet_authentication_requirements, but the linked #21103 that adds this behavior is still open and blocked; the current settings flow routes errno 170 to an interactive challenge. Block this PR on #21103 or qualify/remove this claim until that change lands so the deployed OpenAPI reference does not advertise unavailable behavior.
retrying. Where it cannot challenge — the RP also sent \`prompt=none\` — the UI instead
redirects to the registered \`redirect_uri\` with
\`error=unmet_authentication_requirements\`.
packages/fxa-auth-server/docs/swagger/oauth-server-api.ts:138
- The claim distribution omits
amrfrom ID tokens, althoughgenerateIdTokencopiesgrant.amrinto the ID-token claims and the integration test asserts it. This contrast with introspection makes the new RP-facing documentation misleading; distinguish the three token surfaces explicitly.
The resulting grant carries \`acr\` and \`auth_time\` on the ID token and the JWT access
token, and \`acr\`, \`auth_time\` and \`amr\` via the \`/v1/introspect\` endpoint. These are not carried by
refresh tokens, so an elevated authentication does not survive a token refresh.
packages/fxa-auth-server/docs/swagger/oauth-server-api.ts:101
- The claim distribution omits
amrfrom ID tokens, althoughgenerateIdTokencopiesgrant.amrinto the ID-token claims and the integration test asserts it. This contrast with introspection makes the new RP-facing documentation misleading; distinguish the three token surfaces explicitly.
The resulting grant carries \`acr\` and \`auth_time\` on the ID token and the JWT access
token, and \`acr\`, \`auth_time\` and \`amr\` via the \`/v1/introspect\` endpoint. These are not carried by
refresh tokens, so an elevated authentication does not survive a token refresh.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| accessType: | ||
| 'If specified, a value of `offline` will cause the connecting client to be granted a refresh token alongside its access token.', | ||
| acr: | ||
| 'The authentication context class reference reached by the session that produced this grant, as a string of the form `AAL2`. Reflects the assurance level actually achieved, which may be higher than the level requested.', |
Comment on lines
+87
to
+91
| **Step-up authentication.** A client may require the user to have completed a second factor, | ||
| or to have authenticated recently, by supplying \`acr_values\` and/or \`max_age\` | ||
| ([RFC 9470](https://datatracker.ietf.org/doc/html/rfc9470) section 5). \`acr_values=AAL2\` | ||
| requires an authenticator assurance level of 2; \`AAL2\` is the only value recognized and any | ||
| other value in the list is ignored. \`max_age\` bounds, in seconds, how old the session's most |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
acr_values/max_ageon the authorization endpoints andacr/auth_time/amron introspection, but the generated OpenAPI reference does not mention any of it. The/v1/introspectexample response is stale relative to the Joi schema.swagger.json. Anything not annotated here is invisible to them.acrandamrwere added to the introspect response schema without.description(), so they render bare.This pull request
shared/descriptions.ts— addsacr,amr,authTimeconstants; expandsacrValues(onlyAAL2is recognised, other tokens ignored) andmaxAge(five-second leeway, measured against the session's most recent authentication event). Fixes anassuarancetypo in the existingacrValuestext.oauth-server-api.ts— step-up notes onGET/POST /v1/authorizationandPOST /v1/token;/v1/introspectnotes plusacr/auth_time/amrin the example response; errno 170 added to the error table, where it was missing alongside the pre-existing 119 and 120.introspect.js— wires the bareacrandamrJoi keys to the new description constants.Two things the notes state explicitly, because both are easy to get wrong and neither was written down:
refresh_tokengrant produces a token with noacr/auth_time, and that token still introspects asactive: true— so a check that only readsactivewill not notice the elevation is gone.auth_timeis in seconds whileiat/expon/v1/introspectare in milliseconds. Intentional and retained for back-compat, but a ~1000x trap.Issue that this pull request solves
Closes: FXA-12864
Checklist
Put an
xin the boxes that applyHow to review (Optional)
shared/descriptions.ts, since those strings are the RP-facing contract.descriptions.ts→oauth-server-api.ts→introspect.js..description()to two existing Joi keys, which does not alter validation.lib/routes/oauth/{introspect,authorization,index}.spec.tsandlib/oauth/grant.spec.tspass (89 tests).Other information (Optional)
Overlaps with #21103. An earlier revision of this branch also edited
OAUTH_AUTHORIZATION_POSTinoauth-api.ts. #21103 rewrites the same block and does it better — it correctly notes theunmet_authentication_requirementsredirect comes from the hosted UI rather than the endpoint, and scopes it toprompt=none. That hunk has been dropped here, so this branch no longer touchesoauth-api.tsand the two should merge cleanly in either order. Theprompt=nonebehaviour is mirrored in the/v1/notes for consistency, which reads as forward-looking until #21103 lands.The prose half of FXA-12864 is mozilla/ecosystem-platform#803.
🤖 Generated with Claude Code