Skip to content

Introduce JWTProfile for declarative JWT validation#4191

Open
stevenvegt wants to merge 8 commits intomasterfrom
tighten-jwt-checks
Open

Introduce JWTProfile for declarative JWT validation#4191
stevenvegt wants to merge 8 commits intomasterfrom
tighten-jwt-checks

Conversation

@stevenvegt
Copy link
Copy Markdown
Member

@stevenvegt stevenvegt commented Apr 15, 2026

Summary

JWT validation was scattered across the codebase with ad-hoc checks after ParseJWT calls. Each JWT type (access token, bearer token, JAR, OpenID4VCI proof, VC, VP) had its own validation pattern — some re-parsed the JWS to read headers, some duplicated iss-to-kid binding, and critically the v1 access token introspection did not call jwt.Validate at all, so tokens without exp/iat were silently accepted with no max-lifetime enforcement. Clock skew tolerance was also inconsistent: the operator-configured auth.clockskew only reached two of the five ParseJWT call sites; the rest defaulted to 0.

This PR introduces a declarative JWTProfile type and threads it through ParseJWT. All caller sites now share one validation shape, one skew policy, and one place to reason about what each JWT type requires.

Changes

Core: crypto/jwt_profile.go + crypto/jwx.go

JWTProfile declares:

  • Typ — required typ header value
  • RequiredClaims — must be present and non-empty
  • MaxValidity — max allowed exp - iat delta (auto-requires exp/iat via jwx's WithMaxDelta)
  • ClockSkew — acceptable clock skew; zero falls back to DefaultJWTClockSkew (5s)
  • Validators — additional checks (e.g. IssuerKidValidator)

Fluent overrides WithMaxValidity(d) and WithClockSkew(d) let callers override per-call without mutating the shared profile globals.

ParseJWT's signature becomes (tokenString, keyFunc, profile, at *time.Time):

  • Typ is checked early, before signature verification.
  • MaxValidity and ClockSkew flow into jwx's validation pass.
  • Presence + non-empty is checked post-parse; errors mirror the jwx format ("<claim>" not satisfied: ...) for consistency.
  • Custom validators run last.
  • The variadic jwt.ParseOption is gone; historical verification uses the explicit at *time.Time (only VC/VP signature verification passes non-nil).

Profiles

  • v1AccessTokenProfile (auth/services/oauth) — typ: "at+jwt", requires iss/sub/service/exp/iat, iss-to-kid binding, max lifetime tracks configured accessTokenLifeSpan
  • v1BearerTokenProfile (auth/services/oauth) — requires iss/sub/exp/iat/aud, max 5s (aud set by claimsFromRequest, required because the builder always sets it)
  • jarProfile (auth/api/iam) — requires iss/iat/exp, max 5m; iat/exp are set in Sign() (not Create()) because the jarRequest is persisted between two HTTP requests — setting them at Create would eat into the validity window
  • openID4VCIProofProfile (vcr/issuer) — typ: "openid4vci-proof+jwt", requires iat/aud (aud comparison against i.issuerIdentifierURL still runs post-parse)
  • vcJWTProfile (vcr/verifier) — iss-to-kid binding for JWT VCs
  • vpJWTProfile (vcr/verifier) — requires nbf as issuance anchor

Clock skew unification

  • auth.clockskew config now reaches every ParseJWT call site via profile.WithClockSkew(s.clockSkew) (previously only two call sites honored it).
  • DefaultJWTClockSkew (5s) applies whenever a profile doesn't set its own — no more silent zero-skew defaults for JAR/VC/VP paths.
  • OpenID4VCI proof's hardcoded WithAcceptableSkew(5s) is gone (covered by the default).

VP hardening

The VP builder (vcr/holder/presenter.go) now always sets iat and exp (default 15m when caller doesn't pass Expires). Previously VPs created via /internal/vcr/v2/holder/vp could lack an expiration.

Removed

  • Ad-hoc jwt.Validate call and manual iss/sub/service checks in IntrospectAccessToken
  • Manual JWT validity too long check in validateAccessTokenRequest
  • jws.ParseString re-parse in vcr/issuer/openid.go that existed only to check typ
  • Redundant strings.Split(keyID, "#")[0] != issuer check in the VC signature path (covered by IssuerKidValidator)
  • Redundant jwt.WithRequiredClaim loop in ParseJWT (presence check is already done post-parse; jwx's WithMaxDelta auto-requires its time claims)

Test plan

  • go test ./... passes
  • TestService_IntrospectAccessToken covers: missing/wrong typ, VP JWT replayed as access token, missing service/iss/exp/iat, iss/kid DID mismatch, excessive lifetime (both the strict 60s and configurable accessTokenLifeSpan paths), expired token
  • New TestParseJWT cases for default clock skew and profile-level override
  • JAR Create/Sign/Parse tests verify iat/exp are set at sign time, caller's map is not mutated, and the 5m max is enforced on receive
  • Existing test assertions updated to the jwx-style error format ("<claim>" not satisfied: ...)

🤖 Generated with Claude Code

IntrospectAccessToken was not calling jwt.Validate, so tokens without
exp/iat claims were silently accepted, and there was no max lifetime
enforcement. This adds:

- WithRequiredClaim for exp and iat
- WithMaxDelta to reject tokens with lifetime exceeding 60s
- WithAcceptableSkew for clock drift tolerance
- Default base validators (IsExpirationValid, IsIssuedAtValid, IsNbfValid)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@qltysh
Copy link
Copy Markdown

qltysh Bot commented Apr 15, 2026

6 new issues

Tool Category Rule Count
qlty Structure Function with many returns (count = 8): validate 6

@qltysh
Copy link
Copy Markdown

qltysh Bot commented Apr 15, 2026

Qlty


Coverage Impact

⬇️ Merging this pull request will decrease total coverage on master by 0.04%.

Modified Files with Diff Coverage (7)

RatingFile% DiffUncovered Line #s
Coverage rating: B Coverage rating: C
crypto/jwx.go51.1%183-184, 187-190...
Coverage rating: B Coverage rating: B
vcr/issuer/openid.go100.0%
Coverage rating: B Coverage rating: B
auth/api/iam/jar.go100.0%
Coverage rating: B Coverage rating: B
vcr/holder/presenter.go100.0%
Coverage rating: B Coverage rating: B
vcr/verifier/signature_verifier.go100.0%
Coverage rating: B Coverage rating: B
auth/services/oauth/authz_server.go100.0%
New Coverage rating: F
crypto/jwt_profile.go19.0%56-59, 71-83
Total57.9%
🤖 Increase coverage with AI coding...
In the `tighten-jwt-checks` branch, add test coverage for this new code:

- `crypto/jwt_profile.go` -- Lines 56-59 and 71-83
- `crypto/jwx.go` -- Lines 183-184, 187-190, 215-216, 226-227, and 244-261

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Each kind of JWT in this project (access tokens, bearer tokens, JAR, OpenID4VCI
proof, VC, VP) had its own ad-hoc validation pattern, often duplicating logic
like iss-to-kid binding or re-parsing the JWS to access protected headers. This
introduces a JWTProfile type that captures the validation requirements
declaratively, and threads it through ParseJWT.

A JWTProfile defines:
- Typ: required value of the JWT typ header
- RequiredClaims: claims that must be present and non-empty
- MaxValidity: max allowed exp - iat delta
- Validators: additional checks (e.g. IssuerKidValidator)

ParseJWT now applies the profile alongside signature verification: typ is
checked early (before crypto), required-claims and max-validity options are
forwarded to jwt.ParseString's internal validation, then non-empty checks and
custom validators run on the parsed token.

Profiles defined:
- v1AccessTokenProfile (auth/services/oauth)
- v1BearerTokenProfile (auth/services/oauth)
- jarProfile (auth/api/iam)
- openID4VCIProofProfile (vcr/issuer)
- vcJWTProfile, vpJWTProfile (vcr/verifier)

Additional VP hardening: the VP builder now always sets iat and exp (default
15min when caller doesn't specify), so verifiers can assume their presence.
The VP profile requires nbf as the issuance-time anchor.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stevenvegt stevenvegt changed the title Add jwt.Validate to v1 access token introspection Introduce JWTProfile for declarative JWT validation Apr 16, 2026
Comment thread auth/api/iam/jar.go Outdated
Comment thread auth/services/oauth/authz_server.go
Comment thread auth/services/oauth/authz_server.go Outdated
Comment thread auth/services/oauth/authz_server.go Outdated
Comment thread crypto/jwt_profile.go Outdated
Comment thread auth/services/oauth/authz_server.go
Comment thread crypto/jwx.go Outdated
Comment thread vcr/verifier/signature_verifier.go
stevenvegt and others added 5 commits April 16, 2026 11:32
Introspection used a hardcoded 60s cap (secureAccessTokenLifeSpan) while
issuance used the configurable s.accessTokenLifeSpan. In non-strict mode
this meant an operator with accesstokenlifespan>60 would issue tokens
that introspect would reject as exceeding 60s.

Adds JWTProfile.WithMaxValidity(d) to derive a profile copy with the
caller's configured lifespan, and threads s.accessTokenLifeSpan through
IntrospectAccessToken. Also adds paired tests proving the cap raises
the ceiling and still enforces a max.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bundle of small review fixes from PR #4191:

1. JAR (auth/api/iam/jar.go) now has MaxValidity=5m. iat/exp are set in
   Sign(), not Create(), because the jarRequest is persisted in
   authzRequestObjectStore between an initial HTTP request (Create) and
   the counterparty's later fetch (Sign). Setting timestamps at Create
   would eat into the validity window.

2. ParseJWT (crypto/jwx.go) drops the per-claim jwt.WithRequiredClaim
   loop: WithMaxDelta already auto-requires its time claims, and the
   post-parse loop already checks presence and emptiness. Error message
   now mirrors jwx's format (%q not satisfied: ...) so callers see one
   consistent style.

3. Copyright year 2024 -> 2026 in crypto/jwt_profile.go.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ParseJWT previously accepted a variadic jwt.ParseOption which the auth
package used for skew and the VCR verifier used for WithClock. This
left other callers (jar.go, signature_verifier for VCs) at skew=0,
risking spurious rejections on small clock drift.

Unifies the plumbing:
- JWTProfile gains a ClockSkew field and a WithClockSkew(d) method,
  mirroring WithMaxValidity. Zero value falls back to the new
  DefaultJWTClockSkew (5s).
- ParseJWT's signature changes to (..., profile *JWTProfile, at *time.Time).
  The variadic is removed; historical verification (WithClock) now
  goes through the at parameter. Only VCR signature verification uses
  a non-nil at today.
- Auth callers (authz_server parseAndValidateJwtBearerToken /
  IntrospectAccessToken) switch to profile.WithClockSkew(s.clockSkew),
  keeping the operator-configured auth.clockskew in effect.
- vcr/issuer/openid.go drops its hardcoded WithAcceptableSkew(5s);
  covered by the default.
- auth/api/iam/jar.go and vcr/verifier/signature_verifier.go (VC/VP)
  now get the 5s default instead of 0.

The JWT_validity_too_long test bumps exp from +10s to +20s: with the
new 5s default skew, jwx's WithMaxDelta treats skew as slack
(delta <= max + skew), so the 5s max plus 5s skew accepts up to 10s.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both JWT types always carry an audience: the v1 bearer token builder
(claimsFromRequest in relying_party.go) sets aud to the authorization
server endpoint, and the OpenID4VCI proof spec §7.2.1 mandates aud ==
credential issuer identifier. Adding aud to RequiredClaims surfaces a
missing claim as a clean profile-level error instead of reaching the
downstream endpoint/issuer comparison with an absent value.

Profiles where aud is conditional (JAR) or absent (v1 access token,
VC/VP) are unchanged.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stevenvegt stevenvegt requested a review from reinkrul April 16, 2026 11:17
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