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

refactor[authentication]: reworks the token verification using JWKS url #6

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

sradigan
Copy link

@sradigan sradigan commented Jan 2, 2024

Description

This branch makes the requirement of a confidential client optional. This is to reduce attack surface by removing secrets from multiple install locations and simplify deployment to many resources by not requiring secrets to be deployed.

Note: This removes the token introspection code which makes a call to the identity provider to validated the token. Instead, this code relies on the local decoding and validation of the token. There is notably an omission of validation of aud (audience) on the tokens. Validating audience can be added in if desired, but it is commonly optional and would optimally leverage a configuration flag.

Fixes no issue allocated

Type of change

  • New feature (non-breaking change which adds functionality)
    • Rational: This is the addition of keycloak public client usage
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
    • Rational: The only notable "breaking" change here is that the token validation doesn't rely on a call out to the keycloak server and instead does token validation locally.
  • This change requires a documentation update
    • Rational: Please just let me know if this is something you'd like documented in terms of usage. It is just the omission of the client secret and a different flow when describing the client creation.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

This code was tested (prior to the latest merge from master) on live systems for login in a testing event. For more info, see this commit. I cherry picked the changes against the updated master and re-wrote the commit message to be in-line with the contributions guide.

No tests exist in the repo right now, so no additional testing was written for this

  • Running existing tests
  • Created new tests

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have rebased my branch to include the latest changes from master

Token verification is performed locally using the identity provider
public keys retrieved from the JWKS url. This enables the use of public
clients since no confidential client secrets are required for validation.
@kha7iq
Copy link
Owner

kha7iq commented Jan 3, 2024

@sradigan Thank you for your contribution.
Allow me some time to go through it and test it. Will merge after that.

@sradigan
Copy link
Author

sradigan commented Jan 3, 2024

Sounds good! I'm happy to make updates if needed!

@kha7iq
Copy link
Owner

kha7iq commented Jan 3, 2024

Everything looks good, thank you once again for your time and work

@kha7iq kha7iq merged commit 7f02e1a into kha7iq:master Jan 3, 2024
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.

None yet

2 participants