Skip to content

Conversation

@kw510
Copy link

@kw510 kw510 commented May 16, 2025

  • Update dependencies to use v3 of lestrrat-go/jwx and related packages

  • Refactor JWT handling in tests and main code for compatibility with new API.

@plutov
Copy link

plutov commented Jun 1, 2025

Yes please, this brings issues!

@plutov
Copy link

plutov commented Jun 2, 2025

@kw510 did you manage to make tests work? I did similar PR, but jwt.Sign complains. #102

@kw510
Copy link
Author

kw510 commented Jun 2, 2025

@plutov Heya! Just had a look and you had the same issue I had, but i did get the tests working in the end 🙈

I fixed it by this replacement :)

- // NewSignatureAlgorithm creates a new SignatureAlgorithm object with the given name.
- jwa.NewSignatureAlgorithm
+ // LookupSignatureAlgorithm returns the SignatureAlgorithm object for the given name.
+ jwa.LookupSignatureAlgorithm

kw510 added 2 commits June 3, 2025 14:05
- Upgrade Go version to 1.24.3
- Update lestrrat-go/jwx/v3 to version 3.0.2
- Upgrade golang.org/x/crypto to version 0.38.0 and golang.org/x/sys to version 0.33.0
- Refactor JWT claim extraction in jwtauth.go and jwtauth_test.go to use transform.AsMap for improved readability and error handling.
@plutov
Copy link

plutov commented Jun 3, 2025

@kw510 please also change the version in the example file.

Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Minor feedback

Thank you 👍

- Downgrade Go version in go.mod to 1.23.0
- Update RSA private and public keys in jwtauth_test.go for improved security and compatibility
- Adjust token string in TestSimpleRSAVerifyOnly for consistency
@kw510 kw510 requested a review from VojtechVitek June 3, 2025 20:53
@plutov
Copy link

plutov commented Jul 9, 2025

@VojtechVitek @kw510 let's merge it ? ;)

Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

Let me test this some more before we merge 👍

Can anyone else help test this PR?

@plutov
Copy link

plutov commented Jul 21, 2025

Hey @VojtechVitek I tested it on a project where it was originally failing, but now it works well. Looks good from my side!

@plutov
Copy link

plutov commented Aug 12, 2025

@VojtechVitek how is testing going? Why this merge takes so long?

@hacdias
Copy link

hacdias commented Oct 22, 2025

Same question as @plutov: I've been holding off updating some libraries because of this PR. What's the status?

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.

4 participants