Skip to content

Conversation

@0xjac
Copy link
Contributor

@0xjac 0xjac commented Jul 17, 2025

This safely ignores unknown key type errors on JWKS while returning all other errors. Returned errors are wrap to easily identify which key in the set is problematic if any.

Jose v4.0.3 was handling this correctly according to spec, but it was reverted in v4.0.4 as the implementation was a breaking change due to the custom UnmarshalJSON on the key set. For details see:

Jose v4.0.4 also provided a handy static error to check for unknown web key types. Sadly this was removed: a prefix match on the error message is the best option until Jose improves it's error handling.

Hopefully, Jose will not change the error message in a patch or minor version release. But just in case, test cases have been added to detect it.

Closes #541

Definition of Ready

  • I am happy with the code
  • Short description of the feature/issue is added in the pr description
  • PR is linked to the corresponding user story
  • Acceptance criteria are met
  • All open todos and follow ups are defined in a new ticket and justified
  • Deviations from the acceptance criteria and design are agreed with the PO and documented.
  • No debug or dead code
  • My code has no repetitions
  • Critical parts are tested automatically
  • Where possible E2E tests are implemented
  • Documentation/examples are up-to-date
  • All non-functional requirements are met
  • Functionality of the acceptance criteria is checked manually on the dev system.

@elinashoko
Copy link

Hey @0xjac thank you for your contribution! We're a lil backed up with contributions atm but we'll have a look as soon as we can.

@elinashoko elinashoko moved this to 📋 Sprint Backlog in Product Management Jul 18, 2025
@0xjac 0xjac mentioned this pull request Jul 22, 2025
13 tasks
This safely ignores unknown key type errors on JWKS while returning all
other errors. Returned errors are wrap to easily identify which key in
the set is problematic if any.

Jose v4.0.3 was handling this correctly according to spec, but it was
reverted in v4.0.4 as the implementation was a breaking change due to
the custom UnmarshalJSON on the key set. For details see:
- go-jose/go-jose#136
- go-jose/go-jose#137

Jose v4.0.4 also provided a handy static error to check for unknown web
key types. Sadly this was removed: a prefix match on the error message
is the best option until Jose improves it's error handling.

Hopefully, Jose will not change the error message in a patch or minor
version release. But just in case, test cases have been added to detect
it.

Closes zitadel#541
@0xjac 0xjac force-pushed the fix/541-dont-ignore-jwks-errs branch from d2630bc to d2b458e Compare September 25, 2025 14:55
@0xjac
Copy link
Contributor Author

0xjac commented Sep 25, 2025

note: rebased onto main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 📋 Sprint Backlog

Development

Successfully merging this pull request may close these issues.

Do not ignore all JWKS parsing errors

3 participants