Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not ignore all JWKS parsing errors #541

Open
1 of 2 tasks
morigs opened this issue Feb 8, 2024 · 3 comments
Open
1 of 2 tasks

Do not ignore all JWKS parsing errors #541

morigs opened this issue Feb 8, 2024 · 3 comments

Comments

@morigs
Copy link
Contributor

morigs commented Feb 8, 2024

Preflight Checklist

  • I could not find a solution in the existing issues, docs, nor discussions
  • I have joined the ZITADEL chat

Describe your problem

Currently, jsonWebKeySet.UnmarshalJSON ignores all parsing errors. Comment says that this is done to ignore "unknown key types (kty)" but in fact it ignores all errors (for instance, invalid base64). This makes debugging such errors extremely hard

Describe your ideal solution

Returning error describing why key verification really failed instead of "missing keys".
Or at least log occurring errors somewhere as warnings

Version

3.11.1

Environment

Self-hosted

Additional Context

No response

@livio-a livio-a added the auth label Feb 12, 2024
@livio-a livio-a moved this to 🧐 Investigating in Product Management Feb 12, 2024
@livio-a
Copy link
Member

livio-a commented Feb 12, 2024

Hey @morigs

Thanks for the issue. We'll look into it.

@livio-a livio-a moved this from 🧐 Investigating to 🐛 Bugs/Small Issues in Product Management Mar 20, 2024
@livio-a livio-a added the good first issue Good for newcomers label Mar 20, 2024
@livio-a
Copy link
Member

livio-a commented Jul 10, 2024

Need to recheck if github.com/go-jose/go-jose/v4's latest release has an impact / would fix this

@muhlemmer
Copy link
Collaborator

@livio-a it seems that the error fix you were looking for got reverted upstream, because it became a breaking change: #625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🐛 Bugs/Small Issues
Development

No branches or pull requests

3 participants