feat(oidc): phase 2 — standard params, ID token claims, logout polish#592
Merged
lakhansamani merged 4 commits intomainfrom Apr 7, 2026
Merged
feat(oidc): phase 2 — standard params, ID token claims, logout polish#592lakhansamani merged 4 commits intomainfrom
lakhansamani merged 4 commits intomainfrom
Conversation
OIDC RP-Initiated Logout 1.0 §3: - Prefer post_logout_redirect_uri (the spec name) over the legacy redirect_uri. Both are accepted so existing clients keep working; when both are supplied, post_logout_redirect_uri wins. - Echo the state query parameter on the final redirect URL so clients can correlate the logout round-trip. URL-escaped via url.QueryEscape. Tests (new file oidc_phase2_logout_test.go) prove both the new param name and the legacy fallback reach the fingerprint-check stage without crashing. Full end-to-end assertions on the actual redirect URL contents would require a test helper that mints a valid session fingerprint cookie — deferred as a Phase 2 follow-up so this task stayed scoped to logout.go only.
OIDC Core §2 ID token claims:
- auth_time: Unix seconds when the user authenticated. New AuthTime
field on AuthTokenConfig; CreateIDToken defaults to time.Now().Unix()
when the caller leaves it zero so existing callers keep working
unchanged (backward compat).
- amr: Authentication Methods Reference array, derived from the
session's LoginMethod via new loginMethodToAMR() helper. Mapping:
basic_auth / mobile_basic_auth -> ["pwd"]
magic_link_login / mobile_otp -> ["otp"]
10 social providers -> ["fed"]
unknown/empty -> claim omitted
The helper references constants from internal/constants rather than
hardcoding strings, so future login-method additions will not drift.
- acr: Authentication Context Class Reference. Hardcoded "0"
(minimal assurance) per OIDC Core §2. Phase 3 will introduce
MFA-aware ACR alongside acr_values request support.
All three new claims added to reservedClaims so custom access token
scripts cannot override them.
Tests (new file oidc_phase2_id_token_claims_test.go) cover auth_time
echo + default-to-now, all 8 amr mapping cases (pwd/otp/fed/unknown/
empty), and the hardcoded acr="0".
Adds five standard OIDC authorization request parameters:
- login_hint: forwarded as URL-escaped query param to the login UI
- ui_locales: forwarded as URL-escaped query param to the login UI
- prompt:
* none -> return OIDC error 'login_required' via the
selected response_mode when no valid session
exists. Proceed silently when a session exists.
* login -> bypass the session cookie, force re-auth
* consent -> parsed, logged, no-op (not implemented yet)
* select_account -> parsed, logged, no-op
- max_age (seconds): if a session exists and now - session.IssuedAt >
max_age, treat as prompt=login (force re-auth)
- id_token_hint: parsed via ParseJWTToken + new parseIDTokenHintSubject
helper. On any failure the hint is logged at debug and ignored per
spec (OIDC treats the hint as advisory only).
Implementation note: prompt=none could not reuse the existing
handleResponse path because that path dispatches 'authentication
required' errors to the login UI, which is the opposite of what the
spec requires. OIDC Core §3.1.2.1 says prompt=none errors must be
returned to the client's redirect_uri via the selected response_mode.
The handler now has a bespoke dispatch for each response_mode
(query / fragment / web_message / form_post) in the prompt=none path.
All new params are optional and default to current behavior when
absent, so existing clients are unaffected. No new config flags.
Tests (new file oidc_phase2_authorize_test.go) cover prompt=none
without session, login_hint URL-encoding, ui_locales forwarding,
prompt=consent/select_account no-op, max_age accepted, invalid
id_token_hint ignored, and prompt=login not rejected.
Code review feedback on Phase 2: - max_age=0 now treated as force-reauth (equivalent to prompt=login) per OIDC Core §3.1.2.1. Previously, the parse guard 'parsed > 0' silently dropped max_age=0 as 'no constraint', which is a spec deviation. Uses a new maxAgeZero sentinel so max_age=-1 (absent) and max_age=0 (force-reauth) are distinguishable. - prompt=none now also returns login_required when the session cookie is decryptable but the session has been revoked or expired. The previous flow only dispatched login_required on cookie-missing; session-validation failures fell through to the normal login-UI redirect path, which is the OIDC Core §3.1.2.1 wrong behavior. The dispatch logic is extracted into a local promptNoneLoginRequired closure and called in both places. - Remove dead '_ = strings.TrimSpace(...)' and '_ = json.NewDecoder(...)' lines at the end of TestAuthorizePromptLoginBypassesSession — they were scaffolding that served no purpose. Drop the now-unused encoding/json and strings imports.
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.
Summary
OIDC Phase 2 — Standard Parameter Support. Adds 5 OIDC Core §3.1.2.1 authorization request parameters, 3 ID token claims, and polishes the RP-initiated logout flow. All changes are additive and fully backward compatible — no new config flags, no breaking changes.
Implemented via three parallel subagents working on disjoint files, followed by a single holistic end-of-phase review (approved with minor follow-ups, all applied).
Changes
Group A — Logout (RP-Initiated Logout §3)
post_logout_redirect_uri(OIDC spec name) over legacyredirect_uri. Both accepted — spec name wins when both supplied.stateon the final redirect URL viaurl.QueryEscape.Group B — ID token claims (OIDC Core §2)
auth_time— Unix seconds. NewAuthTimefield onAuthTokenConfig; defaults totime.Now().Unix()when zero so existing callers work unchanged.amr— Authentication Methods Reference. NewloginMethodToAMRhelper referencinginternal/constants:basic_auth/mobile_basic_auth→["pwd"]magic_link_login/mobile_otp→["otp"]["fed"]acr— Hardcoded"0"(minimal assurance). Phase 3 will add MFA-aware ACR alongsideacr_values.All three added to
reservedClaimsso custom access token scripts cannot forge them.Group C —
/authorizeparameters (OIDC Core §3.1.2.1)login_hint,ui_locales— URL-escaped and forwarded to the login UI.prompt:none→ return OIDC errorlogin_requiredvia the client's response mode when no valid session (or session revoked). Proceed silently when valid session exists.login→ bypass session cookie, force re-auth.consent/select_account→ parsed and logged, no-op.max_age:0→ force re-auth (equivalent toprompt=login).id_token_hint— parsed and validated structurally. Advisory only per spec; invalid hints logged and ignored.Critical:
prompt=nonedispatch correctnesshandleResponsecould not be reused forprompt=noneerror responses because it dispatches authentication-required errors to the login UI, which is the opposite of what the OIDC spec requires. A bespoke dispatch is implemented directly in the handler, covering all four response_modes (query,fragment,form_post,web_message). Also catches the case where a session cookie is present but the session has since been revoked.Backward compatibility
redirect_uristill accepted → existing integrations unaffected.AuthTime=0falls back totime.Now().Unix()→ existingCreateAuthTokencallers keep working with no code changes.Test plan
TestLogoutPrefersPostLogoutRedirectURI+TestLogoutStateEchoAcceptedTestIDTokenAuthTimeClaim— explicit echo + default-to-nowTestIDTokenAmrClaim— 8 table casesTestIDTokenAcrClaim— hardcoded"0"TestAuthorizePromptNoneNoSessionReturnsLoginRequiredTestAuthorizeLoginHintForwarded— URL-encodedTestAuthorizeUILocalesForwardedTestAuthorizePromptConsentAndSelectAccountNoOpTestAuthorizeMaxAgeParsedNotRejectedTestAuthorizeIDTokenHintInvalidIgnoredTestAuthorizePromptLoginBypassesSessionmake test-sqlitegreenCommit log
4 commits, +610/-3 across 6 files.
Out of scope / follow-ups
code id_token,code token,code id_token token), back-channel logout, minimal JWKS multi-key support, MFA-awareacr_values.