chore: migrate from lestrrat-go/jwx v2 (EOL) to v3 (V6.2 backport of #4391)#4411
Open
reinkrul wants to merge 2 commits into
Open
chore: migrate from lestrrat-go/jwx v2 (EOL) to v3 (V6.2 backport of #4391)#4411reinkrul wants to merge 2 commits into
reinkrul wants to merge 2 commits into
Conversation
* fix: unwrap error chain when resolving HTTP status code
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.
* chore: migrate from lestrrat-go/jwx v2 (EOL) to v3
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
* refactor(jwx): centralize JWT claims-to-map into jwx.AsMap; check errors
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
* chore: pin go-did to tagged release v0.22.0
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
* fix(iam): handle []interface{} array claims in oauthParameters.get
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
* refactor(jwx): use jws.VerifierFor over the low-level jwsbb.Verify
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
* fix(jwx): jwa.ES256K() function call under jwx_es256k build tag
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
* fix(jwx): tolerate null-valued JOSE header members; restore ES256K test
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
* refactor(jwx): rename AsMap to ClaimsAsMap, type it as jwt.Token
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
* refactor(jwx): make headersAsMap an unexported crypto-package helper
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
* docs(jwx): drop headers caveat from ClaimsAsMap doc
Assisted by AI
* JWK missing checks
(cherry picked from commit 9bf07c1)
reinkrul
requested review from
gerardsn,
stevenvegt and
woutslakhorst
as code owners
Contributor
22 new issues
|
…ix v3 error-string drift in openid4vci_test.go The RSA512->HS256 swap in authz_server_test.go's "wrong signing algorithm" test was copied from master, but master's swap was caused by an unrelated, already-excluded commit (RS256 becoming a supported algorithm, #4252) rather than by jwx v3 itself. jwx v3 signs fine with a 512-bit RSA key (verified empirically), so V6.2's original RSA/RS256 test works unchanged aside from the jwa.RS256 -> jwa.RS256() syntax translation - restored it to keep RS256 (still unsupported on V6.2) under test instead of an unrelated algorithm. Also fixes an untouched-by-conflict v2-style error string assertion in auth/api/iam/openid4vci_test.go, caught by a full -count=1 test run. Assisted by AI
Contributor
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (36) 🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
JorisHeadease
approved these changes
Jul 15, 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.
Backport of #4391 to V6.2.
Scope note
This is a targeted backport of the jwx v2 → v3 migration only. V6.2 has diverged from master in ways that made a direct cherry-pick non-mechanical; the following were deliberately kept out of scope:
crypto/jwt_profile.go, from Introduce JWTProfile for declarative JWT validation #4191) doesn't exist on V6.2.ParseJWTkeeps its V6.2 signature (options ...jwt.ParseOption); the profile-based required-claims/max-validity/clock-skew logic from master was dropped, along with the tests that exercise it.SupportedAlgorithms(from feat(crypto): accept RS256 in SupportedAlgorithms #4252, unrelated to jwx) was not added — V6.2's algorithm allowlist is unchanged apart from the v2→v3 syntax translation. As a corollary,authz_server_test.go's "wrong signing algorithm" test andcrypto/jwx_test.go's "unsupported algorithm" test keep their original RSA/RS256 fixtures (RS256 stays unsupported here) rather than master's HS256 swap — master only switched because feat(crypto): accept RS256 in SupportedAlgorithms #4252 made RS256 supported there, not because of anything in jwx v3 itself (verified empirically: jwx v3 signs fine with a 512-bit RSA key).vc.CreateJWTVerifiablePresentation-based rewrite ofbuildJWTPresentation(a separate, unrelated presenter refactor) doesn't exist on V6.2 either. The two master-only tests that depend on it (#3957VP-type marshalling,#4299holder-as-iss) were dropped; V6.2's original manual claims-map construction inpresenter.gowas kept, just with imports bumped to v3.vcr/issuer/openid.go's rawjws.ParseString/.Type()re-parse,vcr/holder/presenter.go's manual claims map,authz_server.go's RFC003 max-validity check, an error-string assertion inauth/api/iam/openid4vci_test.go) still used v2-style single-return accessors or stale error strings under the hood; these were caught by a fullgo build ./.../go test -count=1 ./...pass and translated to v3's(T, bool)shape / updated wording.Dependency
go-didbumped v0.17.0 → v0.22.0 (same target as master), required for jwx v3. The intermediate go-did changes (v0.18.0–v0.21.0, mostly additive: JWT VP creation support, iss/iat claim handling) are pulled in as a side effect of the version bump but are not otherwise used by V6.2 code.Test changes
crypto/jwx_test.go: RS256/RSA-2048 kept unsupported; error-string alignment for v3's more verbose parse errors.vcr/issuer/openid_test.go,auth/api/iam/openid4vci_test.go: updated expected error strings to v3's more verbose parse-error wrapping.Testing
go build ./...andgo build -tags jwx_es256k ./...clean;go test -count=1 ./...andgo test -tags jwx_es256k ./crypto/...green (fresh run, no cache).Verification methodology
Beyond building/testing, I diffed every file changed by this backport against the corresponding change in #4391 (master), file by file: 93 of 103 shared files are byte-identical in their change-set to master's; the other 10 differ only for the reasons documented above.
Assisted by AI