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

fix(jwt): provide algorithm when verifying signature and issuer #353

Merged
merged 7 commits into from
Oct 17, 2022

Conversation

antoineauger
Copy link
Contributor

@antoineauger antoineauger commented Oct 12, 2022

I just discovered this project yesterday and setup/used it with success today.
This is the perfect fit for one of our internal project, so a huge thanks 🙏🏻

This MR fixes the errors returned by some endpoints when using a non-default algorithm, for instance RS384:

{
  "tokenProvider" : {
    "keyProvider" : {
      "algorithm" : "RS384"
    }
  }
}

Before this MR, if the token generated with algorithms was different than RS256, the following error was returned by the /userinfo and /introspect endpoints :

{
  "error_description": "signed jwt rejected: another algorithm expected, or no matching key(s) found",
  "error": "invalid_token"
}

🛠️ with ❤️ by Siemens

@ybelMekk ybelMekk added the bug Something isn't working label Oct 12, 2022
@ybelMekk
Copy link
Contributor

ybelMekk commented Oct 12, 2022

Hi and thank you for opening this PR.

Seems like Im the guilty one for this 🐛

Looking forward too.. 👀

@antoineauger antoineauger force-pushed the fix/missing-algorithm branch from 40dc72b to f6d2c49 Compare October 14, 2022 13:49
@antoineauger antoineauger force-pushed the fix/missing-algorithm branch from f6d2c49 to 92f4952 Compare October 14, 2022 15:00
@antoineauger antoineauger marked this pull request as ready for review October 14, 2022 15:17
@antoineauger antoineauger requested a review from a team as a code owner October 14, 2022 15:17
@antoineauger
Copy link
Contributor Author

antoineauger commented Oct 14, 2022

This is ready for review 🙏🏻

I also fixed some minor typos, reformat files and clean some unused imports 😉

/cc @ybelMekk

Copy link
Contributor

@ybelMekk ybelMekk left a comment

Choose a reason for hiding this comment

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

Apart from the printlns, this PR LGTM 🥇

@antoineauger
Copy link
Contributor Author

Apart from the printlns, this PR LGTM 1st_place_medal

@ybelMekk I removed the unwanted printlns 😁

Thanks for the review 🏓

@ybelMekk
Copy link
Contributor

ybelMekk commented Oct 17, 2022

@tommytroen you have the last say 😄

@tommytroen
Copy link
Collaborator

LGTM 🏆

@tommytroen tommytroen merged commit 1e8d271 into navikt:master Oct 17, 2022
@antoineauger antoineauger deleted the fix/missing-algorithm branch October 17, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants