Skip to content

feat(oidc): token verification + multi-key hardening#594

Merged
lakhansamani merged 1 commit intofeat/oidc-phase3-extensionsfrom
feat/oidc-token-hardening
Apr 8, 2026
Merged

feat(oidc): token verification + multi-key hardening#594
lakhansamani merged 1 commit intofeat/oidc-phase3-extensionsfrom
feat/oidc-token-hardening

Conversation

@lakhansamani
Copy link
Copy Markdown
Contributor

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

Severity ID File:line Fix
Critical C2 internal/token/jwt.go SignJWTToken, parseJWTWithKidSelection kid JOSE header now set to ClientID on signing; verifier selects primary/secondary by kid and falls back to try-both only for legacy kid-less tokens.
High H1 / Logic #7 internal/token/jwt.go parseJWTWithKidSelection (~L174-200) Secondary-key fallback gated on errors.Is(err, jwt.ErrTokenSignatureInvalid). Non-signature errors short-circuit. Errors wrapped with %w so callers can use errors.Is (Logic #13). Successful secondary verify logs "token verified by secondary key" at debug — no token contents logged.
Medium M4 internal/token/audience.go, internal/token/jwt.go ValidateJWTClaims / ValidateJWTTokenWithoutNonce New AudienceMatches helper supports string, []string, []interface{}, and jwt.ClaimStrings (plus nil/empty). Replaces != comparison for aud.
Medium M5 internal/token/jwt.go ValidateJWTClaims sub now strictly equals User.ID for 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.
Logic #4 internal/token/jwt.go ParseJWTToken (~L229-238) iat is now optional per RFC 7519 §4.1.6 / OIDC Core for non-self-issued tokens; missing iat no longer errors.
Enhancement ParseIDTokenHint internal/token/id_token_hint.go (new) Exposes (*provider).ParseIDTokenHint for OIDC Core §3.1.2.1 / RP-Initiated Logout §2 — verifies signature only, skips exp/nbf/iat validation, reuses kid-aware key selection.
Enhancement AMR/ACR constants internal/constants/amr.go, internal/constants/acr.go (new); internal/token/auth_token.go loginMethodToAMR / CreateIDToken RFC 8176 amr values (pwd, otp, fed, mfa) and OIDC Core §2 baseline acr value ("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.
  • Unit tests cover: 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 / empty / legacy fallback.

Notes

  • ParseIDTokenHint is a method on the concrete *provider but is not added to the Provider interface — internal/token/provider.go is out of scope for this PR. It will be wired into the interface alongside the id_token_hint callers in a follow-up.
  • The M5 carve-out for verification tokens is necessary because signup / magic-link / forgot-password / invite / OTP tokens are issued before a stable user ID exists. The OIDC-§2 prohibition applies to ID-token sub; verification tokens are not OIDC ID tokens.

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.
@lakhansamani lakhansamani merged commit 91501db into feat/oidc-phase3-extensions Apr 8, 2026
@lakhansamani lakhansamani deleted the feat/oidc-token-hardening branch April 8, 2026 05:02
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.

1 participant