Skip to content

Fix Jwt & OAuth2 authenticators seperation issue#10811

Merged
kokosing merged 1 commit intotrinodb:masterfrom
lukasz-walkiewicz:fix-jwt-oauth-separation
Jan 31, 2022
Merged

Fix Jwt & OAuth2 authenticators seperation issue#10811
kokosing merged 1 commit intotrinodb:masterfrom
lukasz-walkiewicz:fix-jwt-oauth-separation

Conversation

@lukasz-walkiewicz
Copy link
Copy Markdown
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.

@lukasz-walkiewicz
Copy link
Copy Markdown
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
Copy Markdown
Member

Typo "seperation"

Copy link
Copy Markdown
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.

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
@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

Development

Successfully merging this pull request may close these issues.

5 participants