feat(oidc): token verification + multi-key hardening#594
Merged
lakhansamani merged 1 commit intofeat/oidc-phase3-extensionsfrom Apr 8, 2026
Merged
feat(oidc): token verification + multi-key hardening#594lakhansamani merged 1 commit intofeat/oidc-phase3-extensionsfrom
lakhansamani merged 1 commit intofeat/oidc-phase3-extensionsfrom
Conversation
Token-layer hardening on top of the phase 3 JWKS multi-key work. Addresses the OIDC review findings listed below. - C2: SignJWTToken now stamps the `kid` JOSE header with ClientID so verifiers (including /.well-known/jwks.json) can pick the right published JWK during manual key rotation. ParseJWTToken honors the header — primary vs secondary vs legacy try-both fallback — via the new parseJWTWithKidSelection helper. (internal/token/jwt.go SignJWTToken, parseJWTWithKidSelection) - H1 / Logic #7: Secondary-key fallback no longer fires for every error. It is now gated on errors.Is(err, jwt.ErrTokenSignatureInvalid) so malformed tokens, alg mismatches, and claim errors short-circuit. Both the primary and combined errors are wrapped with %w so callers can use errors.Is (Logic #13). Successful secondary verification logs a single debug line ("token verified by secondary key") as a rotation signal — no token contents are logged. (internal/token/jwt.go parseJWTWithKidSelection lines ~174-200) - Logic #4: `iat` is optional per RFC 7519 §4.1.6 and OIDC Core for non-self-issued tokens. ParseJWTToken no longer treats a missing iat as an error; present iat is still normalized to int64. (internal/token/jwt.go ParseJWTToken lines ~229-238) - M4: `aud` comparison now accepts arrays. New helper token.AudienceMatches handles string, []string, []interface{}, and jwt.ClaimStrings (plus nil/empty). Used in both ValidateJWTClaims and ValidateJWTTokenWithoutNonce. (internal/token/audience.go, internal/token/jwt.go ValidateJWTClaims) - M5: ValidateJWTClaims now requires sub == User.ID for OIDC-visible tokens (access, refresh, ID). The legacy email-as-sub fallback is restricted to verification token types (signup, magic-link, forgot-password, invite, OTP) where the user ID does not yet exist. OIDC Core §2 requires sub to never be reassigned. (internal/token/jwt.go ValidateJWTClaims, verificationTokenTypes) - New helper ParseIDTokenHint: verifies the signature of an id_token_hint (OIDC Core §3.1.2.1, RP-Initiated Logout §2) without running the exp/nbf/iat validators, as the OP SHOULD accept expired hints. Reuses the kid-aware key selection from ParseJWTToken. (internal/token/id_token_hint.go) - AMR / ACR / ResponseType constants: new internal/constants/amr.go and internal/constants/acr.go define the RFC 8176 amr values (AMRPassword, AMROTP, AMRFederated, AMRMFA) and the OIDC Core §2 baseline acr value (ACRBaseline = "0"). Replaces the string literals in auth_token.go loginMethodToAMR (~L65-83) and CreateIDToken (~L478). Tests: unit tests for AudienceMatches (4 shapes + nil/empty), kid header stamping, legacy kid-less fallback, direct secondary-kid selection, signature-only fallback (no retry on malformed), wrapped error propagation via errors.Is, optional iat, strict sub for OIDC tokens, email-as-sub allowed for verification tokens, ParseIDTokenHint accept-expired / reject-bad-sig / reject-malformed / legacy fallback. make test-sqlite: all packages PASS.
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
Token-layer hardening on top of the phase 3 JWKS multi-key work. Addresses the OIDC review findings for token signing, verification, and claim validation.
Findings fixed
internal/token/jwt.goSignJWTToken, parseJWTWithKidSelectionkidJOSE header now set toClientIDon signing; verifier selects primary/secondary bykidand falls back to try-both only for legacy kid-less tokens.internal/token/jwt.goparseJWTWithKidSelection (~L174-200)errors.Is(err, jwt.ErrTokenSignatureInvalid). Non-signature errors short-circuit. Errors wrapped with%wso callers can useerrors.Is(Logic #13). Successful secondary verify logs"token verified by secondary key"at debug — no token contents logged.internal/token/audience.go,internal/token/jwt.goValidateJWTClaims / ValidateJWTTokenWithoutNonceAudienceMatcheshelper supportsstring,[]string,[]interface{}, andjwt.ClaimStrings(plus nil/empty). Replaces!=comparison foraud.internal/token/jwt.goValidateJWTClaimssubnow strictly equalsUser.IDfor OIDC-visible tokens (access, refresh, ID). Email-as-sub retained only for the internal verification token types (signup, magic-link, forgot-password, invite, OTP) where no user ID exists yet.internal/token/jwt.goParseJWTToken (~L229-238)iatis now optional per RFC 7519 §4.1.6 / OIDC Core for non-self-issued tokens; missingiatno longer errors.internal/token/id_token_hint.go(new)(*provider).ParseIDTokenHintfor OIDC Core §3.1.2.1 / RP-Initiated Logout §2 — verifies signature only, skips exp/nbf/iat validation, reuses kid-aware key selection.internal/constants/amr.go,internal/constants/acr.go(new);internal/token/auth_token.gologinMethodToAMR / CreateIDTokenamrvalues (pwd,otp,fed,mfa) and OIDC Core §2 baselineacrvalue ("0") are now named constants. String literals replaced.Test plan
go build ./...clean.go test ./internal/token/...— all new and existing unit tests pass.make test-sqlite— all packages pass:internal/crypto,internal/http_handlers,internal/integration_tests,internal/memory_store,internal/memory_store/db,internal/metrics,internal/parsers,internal/storage,internal/token,internal/utils,internal/validators.errors.Is, optional iat, strict sub for OIDC tokens, email-as-sub allowed for verification tokens, ParseIDTokenHint accept-expired / reject-bad-sig / reject-malformed / empty / legacy fallback.Notes
ParseIDTokenHintis a method on the concrete*providerbut is not added to theProviderinterface —internal/token/provider.gois out of scope for this PR. It will be wired into the interface alongside theid_token_hintcallers in a follow-up.sub; verification tokens are not OIDC ID tokens.