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

Add JWT unit tests #467

Merged
merged 5 commits into from
Dec 25, 2023
Merged

Add JWT unit tests #467

merged 5 commits into from
Dec 25, 2023

Conversation

kjosh
Copy link
Contributor

@kjosh kjosh commented Dec 19, 2023

I wrote some unit tests for the JWT vulnerabilities (#398), testing the controller functions and some validator exploits to the best of my ability.

I refactored some relatively minor things to make the controller easier to test:

  • created a method to create a token with a JWK header in the existing IJWTTokenGenerator, so I can also use it in the test without too much duplication
  • removed the static initialization logic in JWTAlgorithmKMS, manage it's singleton lifecycle via Spring instead, mainly so I can inject it into the controller and spy on it in the test
  • replace the String type of the RequestEntity parameters with Void, this should be the correct type for GET-Requests and allows creation of test objects

Copy link
Member

@preetkaran20 preetkaran20 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR. PR looks good to me. merging it.


private static volatile boolean initDone = false;
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for making this class unit testable. !!!

@preetkaran20 preetkaran20 merged commit 1145456 into SasanLabs:master Dec 25, 2023
1 check passed
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.

2 participants