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.

Sign up for GitHub

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

Jump to bottom

Fix Jwt & OAuth2 authenticators seperation issue #10811

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Fix Jwt & OAuth2 authenticators seperation issue #10811

merged 1 commit into from
Jan 31, 2022

Conversation

lukasz-walkiewicz
Copy link
Member

Jwt & OAuth2 use the same classes in order to resolve JSON Web Keys (JWK)
served over HTTP. Using both authenticators simultaneously resulted in a
duplicated binding error which this commit fixes through a better
binding seperation.

@cla-bot cla-bot bot added the cla-signed label Jan 26, 2022
@lukasz-walkiewicz
Copy link
Member Author

One important thing to notice: I've renamed @ForJwk annotation to @ForJwt in JWT and replaced @ForJwk occurrences with @ForOAuth2. I think it would make sense to change the prefix for the http client configuration as well (jwk -> jwt and oauth2-jwk -> oauth2 for JWT and OAuth2 respectively) but it's a breaking change so there would need to be considerable advance warning.

@electrum
Copy link
Member

Typo "seperation"

11xor6
11xor6 approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@11xor6 11xor6 left a comment

Choose a reason for hiding this comment

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

LGTM, but the "retry" commit should be dropped.

@lukasz-walkiewicz
Fix Jwt & OAuth2 authenticators separation issue
0d5d0bc 
Jwt & OAuth2 use the same classes in order to resolve JSON Web Keys (JWK)
served over HTTP. Using both authenticators simultaneously resulted in a
duplicated binding error which this commit fixes through a better
binding separation.
@kokosing kokosing merged commit 31e2fff into trinodb:master Jan 31, 2022
@github-actions github-actions bot added this to the 370 milestone Jan 31, 2022
@kokosing kokosing mentioned this pull request Jan 31, 2022
Closed
@mosabua mosabua mentioned this pull request Jan 31, 2022
Merged
@lukasz-walkiewicz lukasz-walkiewicz deleted the fix-jwt-oauth-separation branch February 2, 2022 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@11xor6 11xor6 11xor6 approved these changes

@kokosing kokosing kokosing approved these changes

@mat3e mat3e mat3e approved these changes

@dain dain Awaiting requested review from dain

Assignees
No one assigned
Labels
cla-signed
Milestone
370
Development

Successfully merging this pull request may close these issues.
5 participants
@lukasz-walkiewicz @electrum @11xor6 @kokosing @mat3e