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

JwtAuthenticationProvider should use provided authentication details #11822

Closed
ch4mpy opened this issue Sep 15, 2022 · 0 comments
Closed

JwtAuthenticationProvider should use provided authentication details #11822

ch4mpy opened this issue Sep 15, 2022 · 0 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: bug A general bug
Milestone

Comments

@ch4mpy
Copy link
Contributor

ch4mpy commented Sep 15, 2022

Describe the bug
JwtAuthenticationProvider delegates the instantiation of an Authentication to a Converter<Jwt, ? extends AbstractAuthenticationToken> but it then alters returned value details property.

For framework user providing a Converter<Jwt, AbstractAuthenticationToken> bean which sets authentication details, this means he won't get the details he had set when later accessing the authentication (from security expressions for instance).

Please note that JwtReactiveAuthenticationManager, the reactive pendant of JwtAuthenticationProvider written by @rwinch , does not set details after the Authentication is returned by the converter.

To Reproduce

  • Create a DemoAuthentication extends AbstractAuthenticationToken class which sets details in constructor and make it immutable (override setDetails to throw an exception)
  • Configure a resource-server with JWT decoder and a Converter<Jwt, DemoAuthentication>

An exception will be thrown by JwtAuthenticationProvider as soon as a request is submitted with a valid JWT.

Expected behavior
This kind of side effect on Authentication instance is a problem when the framework user has provided a jwtAuthenticationConverter which intentionally set authentication details.

Ideally, details property would be set by the jwtAuthenticationConverter (not the JwtAuthenticationProvider).

Other sollution with minimum code impact would be preventing JwtAuthenticationProvider to touch authentication details when it is not null.

Sample
https://github.com/ch4mpy/spring-security-11822

  • edit application.properties to point to your favorite authorization-server
  • get a JWT access-token
  • edit TOKEN value in SpringSecurity11822ApplicationTests
  • run SpringSecurity11822ApplicationTests
@ch4mpy ch4mpy added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 15, 2022
@ch4mpy ch4mpy changed the title JwtAuthenticationProvider should not agressively set authentication details JwtAuthenticationProvider should not aggressively set authentication details Sep 15, 2022
ch4mpy added a commit to ch4mpy/spring-addons that referenced this issue Sep 16, 2022
ch4mpy added a commit to ch4mpy/spring-addons that referenced this issue Sep 16, 2022
@sjohnr sjohnr added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: breaks-passivity A change that breaks passivity with the previous release and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 16, 2022
ch4mpy added a commit to ch4mpy/spring-security that referenced this issue Sep 26, 2022
Prevent JwtAuthenticationProvider from setting authentication details
when jwtAuthenticationConverter returned an authentication instance
with non null details.
@sjohnr sjohnr closed this as completed in 7ad4ebd Dec 13, 2022
@sjohnr sjohnr added this to the 6.1.0-M1 milestone Dec 13, 2022
@sjohnr sjohnr changed the title JwtAuthenticationProvider should not aggressively set authentication details JwtAuthenticationProvider should use provided authentication details Jan 17, 2023
@sjohnr sjohnr added type: bug A general bug and removed type: enhancement A general enhancement type: breaks-passivity A change that breaks passivity with the previous release labels Jan 17, 2023
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) type: bug A general bug
Projects
None yet
2 participants