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

Introduce ReactiveJwtAuthenticationConverter #6273

Closed
jzheaux opened this issue Dec 11, 2018 · 4 comments
Closed

Introduce ReactiveJwtAuthenticationConverter #6273

jzheaux opened this issue Dec 11, 2018 · 4 comments
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Dec 11, 2018

Users often need to consult external resources in a non-blocking way to collect granted authorities in Resource Server.

This is currently possible by implementing a Converter<Jwt, ? extends Mono<? extends AbstractAuthenticationToken>>.

On the Servlet stack, this task can be accomplished more easily by extending JwtAuthenticationConverter:

public class MyAuthenticationConverter
        extends JwtAuthenticationConverter {

    @Override
    public Collection<GrantedAuthority> extractAuthorities(Jwt jwt) {
        // ... extract
    }
}

It would be nice to have the same simplicity on the reactive side:

public class MyReactiveAuthenticationConverter 
        extends ReactiveJwtAuthenticationConverter {

    @Override
    public Flux<GrantedAuthority> extractAuthorities(Jwt jwt) {
        // ... extract in a non-blocking way
    }
}
@jzheaux jzheaux added Reactive in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Dec 11, 2018
@jzheaux jzheaux added this to the 5.2.x milestone Dec 11, 2018
@jzheaux jzheaux changed the title Introduce ReactiveJwtAuthenticationConverter Introduce ReactiveJwtAuthenticationConverter Dec 11, 2018
@edeandrea
Copy link
Contributor

Would you like a PR for this?

@jzheaux
Copy link
Contributor Author

jzheaux commented Dec 12, 2018

Have I told you have lucky we are to have great contributors like you, @edeandrea?

Yes, a PR would be much appreciated, thank you.

I'm actually thinking that ReactiveJwtAuthenticationConverter would be abstract, and not have a default implementation for extractAuthorities.

The reason for this is that the user already gets the default scope-extraction logic by using JwtAuthenticationConverter, which they are already getting by default. The reason they'd use ReactiveJwtAuthenticationConverter is that they want to change that behavior.

@edeandrea
Copy link
Contributor

edeandrea commented Dec 12, 2018

That would be fine if I wanted to completely alter the authorities, but what if I want to merge the default behavior with my own set of authorities (which is actually my use case for bringing this up in the first place). Then I'd have to re-write that default logic myself rather than just calling super.

My thought would be to refactor JwtAuthenticationConverter and extract the logic that creates the authorities into its own Converter<Jwt, Collection<GrantedAuthority>> and have JwtAuthenticationConverter use that. Then ReactiveJwtAuthenticationConverter can use it too by wrapping in a Flux.fromIterable.

I'll code it out & submit a PR so you can see. If we need to tweak from there we can.

@edeandrea
Copy link
Contributor

I opened #6277 for this. We can take the discussion over there if any re-factoring is needed.

jzheaux pushed a commit that referenced this issue Dec 17, 2018
Some changes based on PR comments

Fixes gh-6273
jzheaux added a commit that referenced this issue Aug 2, 2019
Rework the implementation so that it is clearer that authorities are
derived from a single claim.

Issue: gh-6273
kostya05983 pushed a commit to kostya05983/spring-security that referenced this issue Aug 26, 2019
Rework the implementation so that it is clearer that authorities are
derived from a single claim.

Issue: spring-projectsgh-6273
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)
Projects
None yet
Development

No branches or pull requests

2 participants