chore: migrate from lestrrat-go/jwx v2 (EOL) to v3#4391
Open
reinkrul wants to merge 13 commits into
Open
Conversation
GetHTTPStatusCode only did a direct type assertion for
HTTPStatusCodeError, so any fmt.Errorf("...: %w", err) wrap between an
error's origin and the Echo error handler silently discarded a
deliberately-set status code, falling back to 500. This affected
RequestServiceAccessToken when the remote presentation definition
endpoint returns an OAuth2 error (e.g. invalid_scope), among other
paths, per #2943.
jwx v2 is deprecated (EOL as of v2.1.7); migrate all direct usage to v3. - Rewrite imports v2 -> v3 across the codebase. - jwa algorithm constants -> function calls (jwa.ES256 -> jwa.ES256()). - Adapt Get()/accessor call sites to v3 signatures ((T, bool) returns). - jwk.FromRaw -> jwk.Import, (jwk.Key).Raw -> jwk.Export. - Replace the removed raw jws.NewVerifier with jwsbb.Verify for detached signature verification (crypto, vcr JSON-LD proof). - didkey resolver: use crypto/ecdh for X25519 (jwx/x25519 removed in v3). - Faithfully replace token.AsMap/PrivateClaims with JSON-based extraction so null-valued claims are still tolerated (v3 per-claim Get errors on null). - azure keyvault: validate key type before jwk.Export for a clear error. - Point go-did at its jwx-v3 branch (pending a tagged go-did release). Assisted by AI
7 tasks
reinkrul
commented
Jul 3, 2026
Addresses review feedback on the jwx v3 migration: - Add crypto/jwx.AsMap(joseObject) helper that converts a jwt.Token's claims to a map via JSON (preserving null-valued claims, unlike a per-field Get loop which errors on null). Replaces the duplicated marshal/unmarshal blocks in auth/client/iam, auth/api/iam/jar, auth/services/oauth, and the vcr test helper, and the equivalent loop in spi.FromJWK. The helper checks both the marshal and unmarshal errors, so those are no longer ignored. - Keep the per-field Get loop for JOSE *headers* (DecryptJWE, ExtractProtectedHeaders): a JSON round-trip flattens rich header values (e.g. the x5c certificate chain) and breaks callers that rely on their concrete Go types. - Remaining discarded returns are single-value accessors that returned no error/bool in v2 (e.g. token.Issuer()), so ignoring the new bool is faithful. Assisted by AI
reinkrul
requested review from
Dirklectisch,
JorisHeadease,
gerardsn,
stevenvegt and
woutslakhorst
as code owners
Replaces the interim jwx-v3-migration branch pseudo-version with the tagged go-did v0.22.0 release (jwx v3). Clears the merge blocker for the jwx v3 migration. Assisted by AI
Contributor
20 new issues
|
; Conflicts: ; auth/oauth/types.go ; go.mod ; go.sum
Contributor
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (37) 🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
The jwx v3 migration extracts JAR request-object claims via jwx.AsMap
(json.Marshal + Unmarshal into a map), which decodes JSON arrays as
[]interface{} rather than the []string that jwx v2's AsMap produced.
oauthParameters.get only matched []string, so a single-value "aud" claim
resolved to "" and audience validation failed (surfaced by the e2e OAuth
flow: "invalid audience, expected: ..., was: ").
Handle []interface{} single-element arrays too, fulfilling get's documented
"array values with len == 1 are treated as single string values" contract
regardless of whether the value came from query params ([]string) or JSON
claims ([]interface{}). Add a unit test covering the accessor.
Assisted by AI
JorisHeadease
requested changes
Jul 8, 2026
| return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "request signature validation failed", InternalError: err} | ||
| } | ||
| claimsAsMap, err := token.AsMap(ctx) | ||
| claimsAsMap, err := jwx.AsMap(token) |
Contributor
There was a problem hiding this comment.
The audience check in handleAuthorizeRequestFromHolder now always fails, breaking the JAR authorization flow. This is what's causing the e2e failure on this PR (invalid audience, expected: https://nodeA/oauth2/subjectA, was: ).
jar.validate now uses the new jwx.AsMap() helper, which does a JSON round-trip. jwx v3 always marshals aud as a JSON array (FlattenAudience is not enabled anywhere in the codebase), so after json.Unmarshal the claim comes back as []interface{}{"..."}. The old v2 token.AsMap(ctx) returned the audience as []string.oauthParameters.get() in params.go only handles string and []string, so params.get(jwt.AudienceKey) returns "" and the check at openid4vp.go:99 rejects every request.
The existing unit test doesn't catch this because it puts aud into the oauthParameters map directly as []string, bypassing the JWT parse path.
Contributor
|
There are also a lot of resolved comments about not ignoring the errors, but the code still ignores them |
Member
Author
I checked them on case-by-case basis, and where they're ignored, they were ignored before. The semantic didn't change, as previously the single call basically did what is now done in 2 calls (meaning nil/error-behavior will be the same). Edit: I added an explicit check for 2 of |
For the raw/detached signature verification (ParseJWS and the JSON-LD proof verifier), use the jws-level jws.VerifierFor(alg) instead of reaching into the low-level jwsbb.Verify building block. jws.VerifierFor is the intended v3 replacement for the v2 jws.NewVerifier(alg) pattern (which no longer registers standard algorithms in v3). For standard algorithms it delegates to jwsbb.Verify anyway, so the result is identical, but it stays at the public jws API level and honours any verifier registered via jws.RegisterVerifier (jwsbb.Verify bypasses the registry by design). Assisted by AI
The es256k registration file is only compiled with -tags jwx_es256k, so the jwa constant->function change was missed during the v2->v3 migration. Assisted by AI
Two follow-ups to the jwx v2->v3 migration:
- Null-valued protected header members were rejected. The header extraction
loops (DecryptJWE, ExtractProtectedHeaders) use per-field Get to preserve
rich types like the x5c certificate chain, but v3's Get errors on a null
value, so a JWS/JWE with a null-valued custom header was rejected where v2's
Headers.AsMap returned nil. Add jwx.HeadersAsMap, which keeps concrete Go
types and stores a null member as nil (the only failure mode of Get on an
existing key into interface{}). Use it in both loops.
- Move and fix the previously-uncompilable ES256K test. It lived in package
jwx but called crypto.ParseJWT (an import cycle), passed the wrong argument
count, and signed with a P-256 key for a secp256k1 algorithm. It never
compiled because CI only builds -tags jwx_es256k, never tests it. Moved to
package crypto and signed with a real secp256k1 key.
Assisted by AI
AsMap was misleadingly generic: it took `any` but is really about a JWT token's claims. Rename to ClaimsAsMap and type the parameter as jwt.Token for compile-time safety and clearer intent. The one non-token caller (spi.FromJWK, a jwk.Key) is not "claims", so it now does its own json.Marshal/Unmarshal inline. A jwk.Key has no null-valued members, so it does not need the null-tolerant helper. Assisted by AI
Its only callers are DecryptJWE and ExtractProtectedHeaders in the crypto package, so move it out of the exported crypto/jwx API and unexport it. Drop its unit test. Assisted by AI
Assisted by AI
JorisHeadease
approved these changes
Jul 10, 2026
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
2 participants
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.
Migrates all direct usage of
github.com/lestrrat-go/jwxfrom the EOL v2 series to v3. Closes part of #4383 (themastertarget).Dependency
Requires go-did v0.22.0 (jwx v3) — pinned in
go.mod. This was the last thing keeping jwx v2 in the tree (see nuts-foundation/go-did#155); the earlier merge blocker (branch pseudo-version) is now resolved.Why
jwx v2 is deprecated (EOL as of v2.1.7) — no further bug/security fixes. jwx is a core JWT/JWS/JWK/crypto dependency, so staying on it is a security liability. We target v3, not v4: v4 hard-depends on
encoding/json/v2behindGOEXPERIMENT=jsonv2, which we won't bake into a production build until that graduates from the Go experiment (tracked as a follow-up in #4383).Key changes
go.mod/go.sum.jwaalgorithm constants → function calls (jwa.ES256→jwa.ES256());jwavalues are now comparable structs.Get()/accessor call sites adapted to v3 signatures (single-value accessors now return(T, bool);Get(name, &dst) error).jwk.FromRaw→jwk.Import;(jwk.Key).Raw(&x)→jwk.Export(key, &x).didkeyresolver: X25519 now built withcrypto/ecdh(jwx dropped itsx25519package in v3).Notable non-mechanical fixes (root-cause, behavior-preserving)
jws.NewVerifier; migrated the detached-signature verification incrypto/jwx.goandvcr/signature/proof/jsonld.gotojwsbb.Verify(...).AsMap/PrivateClaimsremoval: replaced the interim per-claimGetloops with JSON-based extraction inauth/client/iam,auth/api/iam/jar.go, andauth/services/oauth/authz_server.go. v3's per-claimGeterrors on a null-valued claim, which would have rejected otherwise-valid external tokens; JSON extraction preserves v2'sAsMap/PrivateClaimstolerance.parseKeynow checks the key type/curve beforejwk.Export, so an unsupported (e.g. RSA) key returns the intendedonly ES256 keys are supportedrather than a leaky jwx-internal export error.Behavior changes surfaced by v3 (kept intentionally)
TestInsecure1024BitRS512JWTstill rejects the weak key.alg:none: v3 rejects an empty compact signature on any other alg at parse time. Production already handlesalg:none; affected test fixtures updated. Any external issuer emitting unsigned credentials must setalg:none.jku/ remote JWK fetching is introduced anywhere (v3 flipped the fetch whitelist default to allow-all — audited, we don't use it).Testing
go build ./...clean;go test ./...green (107 packages).Follow-up
v6.2andv5.4(Migrate from lestrrat-go/jwx/v2 (EOL) to v3 #4383).encoding/json/v2leavesGOEXPERIMENT.Assisted by AI