Skip to content

JwtBearerTokenAuthenticationConverter Should Be In the Sample That Uses It #7354

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

Closed
jgrandja opened this issue Sep 4, 2019 · 2 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid

Comments

@jgrandja
Copy link
Contributor

jgrandja commented Sep 4, 2019

JwtBearerTokenAuthenticationConverter is only used in a sample. Could we not extract this to the sample instead?

Related #7346

@jgrandja jgrandja added this to the 5.2.0.RC1 milestone Sep 4, 2019
@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Sep 4, 2019
@jzheaux jzheaux changed the title Remove JwtBearerTokenAuthenticationConverter JwtBearerTokenAuthenticationConverter Should Be In the Sample That Uses It Sep 4, 2019
@jzheaux
Copy link
Contributor

jzheaux commented Sep 4, 2019

Samples

JwtBearerTokenAuthenticationConverter is only used in a sample.

I believe that there are several classes in Spring Security that are only used in the samples (or aren't used in the codebase at all, are just features available for applications).

I'll list a few here, and we can see if JwtBearerTokenAuthenticationConverter is somehow different from them:

org.springframework.security.oauth2.client.web.reactive.function.client.ServletOAuth2AuthorizedClientExchangeFilterFunction
org.springframework.security.oauth2.server.resource.web.HeaderBearerTokenResolver
org.springframework.security.web.authentication.DelegatingAuthenticationFailureHandler

I've tried to list some from separate projects to hopefully capture any nuances to how we decide whether or not to keep this class where it is at.

Additional Context

It might be helpful if I expanded on some of the rationale already shared in #7346.

I believe JwtBearerTokenAuthenticationConverter is a feature that is useful for simplifying the authentication outcome for oauth2ResourceServer().

When using this class instead of the default JwtAuthenticationConverter, then controllers, for example, can address the principal's attributes instead of their claims, something that has been a source of confusion in the past.

For example:

public String method(@AuthenticationPrincipal(expression="attributes['sub']") String subject)

is more semantically correct than

public String method(@AuthenticationPrincipal(expression="claims['sub']") String subject)

since claims refer to a token in an unauthenticated state.

attributes is also a nicer term that transfers easily between different attribute-holding authentication
mechanisms.

It has the additional benefit that the expression of an authenticated principal in Resource Server becomes unified:

public String method(@AuthenticationPrincipal OAuth2AuthenticatedPrincipal)

I affirm that while JwtAuthenticationConverter is very convenient, it is better practice to use JwtBearerTokenAuthenticationConverter for the above reasons. It will be easier to advocate this class's usage if it is a first-class citizen in the codebase.

Secondary use-case

A secondary use-case is when applications support more than one mode of Resource Server authentication, as is demonstrated in the sample you refer to. In that case, it becomes critical to have a unified view of an authentication and its attributes, and this adapter removes boilerplate for the application.

For example:

public String method(@CurrentSecurityContext(expression="authentication.tokenAttributes['sub']") String subject)

becomes

public String method(@AuthenticationPrincipal(expression="attributes['sub'] String subject)

Additionally

public String method(AbstractOAuth2TokenAuthenticationToken<?> token)

becomes

public String method(BearerTokenAuthentication token)

While this last example may seem minor, there is more than just line length going on here. I believe there is value in drawing a distinction between XYZAuthentication instances as representing authentication results and XYZAuthenticationToken instances representing authentication requests. This is a pattern that is being followed, for example, by the SAML PR.

This is a move forward in this same direction. By using this converter, the application will use an Authentication with a clear naming convention that features it as an authentication result, not a request.

@jzheaux jzheaux modified the milestones: 5.2.0.RC1, 5.2.0 Sep 5, 2019
@jzheaux
Copy link
Contributor

jzheaux commented Sep 30, 2019

I'll just add here one more nice thing about this class is that it will eventually simplify how an application customizes the Principal in a way comparable to the rest of Spring Security since it changes the principal from a concrete class, Jwt, to the OAuth2AuthenticatedPrincipal interface.

@jzheaux jzheaux closed this as completed Sep 30, 2019
@jzheaux jzheaux added the status: invalid An issue that we don't feel is valid label Sep 30, 2019
@jzheaux jzheaux removed this from the 5.2.0 milestone Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

2 participants