Skip to content

Allow for multiple Jwt to GrantedAuthorizies converters #7596

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
rolaca11 opened this issue Nov 2, 2019 · 7 comments
Closed

Allow for multiple Jwt to GrantedAuthorizies converters #7596

rolaca11 opened this issue Nov 2, 2019 · 7 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@rolaca11
Copy link

rolaca11 commented Nov 2, 2019

It is quite hard to have multiple converters extract granted authorities from JWT tokens.

In the case of an authorization server created with the latest version of spring-security supporting one, the Jwt tokens generated by the auth server contains a claim named authorities, which is an array of strings based on what authorities a user has.

The new version of OAuth Resource server ignores this claim and the only granted authorities of a user are ROLE_USER and SCOPE_<scope> which in my opinion are inadequate.

One could write one's own converter, but to use it, a new instance of JwtAuthenticationConverter has to be created. Also, this solution means that the previously existing ROLE_USER and SCOPE_<scope> would not be available without code duplicating or having to deal with a SCOPE_ prefix before all roles, which is not desirable.

The solution could be an implementation of JwtAuthenticationConverter which holds a DelegatingJwtGrantedAuthoritiesConverter able to iterate over multiple JwtGrantedAuthoritiesConverters, merging the results of one another.

This solution could be wired into the JwtConfigurer as well, allowing the developer to add multiple converters without the need to replace default instances.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 2, 2019
@jzheaux jzheaux self-assigned this Nov 4, 2019
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 4, 2019
jzheaux added a commit that referenced this issue Nov 4, 2019
To confirm that resource server only produces SCOPE_<scope>
authorities by default.

Issue gh-7596
@jzheaux
Copy link
Contributor

jzheaux commented Nov 4, 2019

Thanks for your interest in making Spring Security better, @rolaca11.

It seems to me that supporting the authorities claim is already pretty simple:

JwtGrantedAuthoritiesConverter authorities = new JwtGrantedAuthoritiesConverter();
authorities.setAuthoritiesClaimName("authorities");

http
    .oauth2ResourceServer()
        .jwt()
            .jwtAuthenticationConverter(jwt -> 
                new JwtAuthenticationToken(jwt, authorities.convert(jwt)));

So, I'm not really seeing the benefit of adding complexity to the DSL.

There may be value in a DelegatingJwtGrantedAuthoritiesConverter, though. Can you explain more about what you are trying to achieve in your application? Is it simply that you have JWTs that have both an authorities claim and a scope claim?

Also, I'm a little confused by this statement:

the only granted authorities of a user are ROLE_USER and SCOPE_

Spring Security's Resource Server support only adds SCOPE_<scope> authorities. Can you clarify how you are getting a ROLE_USER authority?

@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Nov 4, 2019
@rolaca11
Copy link
Author

rolaca11 commented Nov 5, 2019

Hi,

Is it simply that you have JWTs that have both an authorities claim and a scope claim?

Yes and originally partly no. Partly no, because I didn't realize you can set a claimName to the JwtGrantedAuthoritiesConverter, so that was my mistake. However, I do want to extract my granted authorities from both the scope and authorities.

Another concern is that no matter what, the prefix extracted by an instance of JwtGrantedAuthoritiesConverter will have the same prefix. So if I have authorities from scope, I'd like the extracted authority from that to be SCOPE_ (the default behavior). If I'd like to have my authorities extracted from authorities to have a different scope, I'd need to use another instance of the converter, where I set the prefix to be something different.

Can you clarify how you are getting a ROLE_USER authority?

Of course, the ROLE_USER authority comes from the fact that OAuth2UserAuthority is instantiated with a constructor only accepting attributes. This constructor defaults the authority name to ROLE_USER. I can't find the usage of this at the moment, because I'm not in front of my computer.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 5, 2019
@jzheaux
Copy link
Contributor

jzheaux commented Nov 11, 2019

If I'd like to have my authorities extracted from authorities to have a different scope, I'd need to use another instance of the converter, where I set the prefix to be something different.

That makes sense, yes. So then you could do:

JwtGrantedAuthoritiesConverter authorities = new JwtGrantedAuthoritiesConverter();
authorities.setAuthorityPrefix("ROLE_");
authorities.setAuthoritiesClaimName("authorities");
// ... etc.

However, I do want to extract my granted authorities from both the scope and authorities.

Okay, great. Yes, I think there's value in making this a bit simpler, I think in the same way that it's done with DelegatingOAuth2TokenValidator.

Would you be interested in changing your PR to only introduce DelegatingJwtGrantedAuthoritiesConverter and its corresponding tests? I'd like to hold off for now on changing the DSL, but I think adding the class is helpful.

At that point, you could do:

JwtGrantedAuthoritiesConverter composite =
    new DelegatingJwtGrantedAuthoritiesConverter(authorities(), scope());

http
    .oauth2ResourceServer()
        .jwt()
             .jwtAuthoritiesConverter(jwt -> new JwtAuthenticationToken(jwt, composite.convert(jwt)));

// ...

JwtGrantedAuthoritiesConverter authorities() {
    JwtGrantedAuthoritiesConverter authorities = new JwtGrantedAuthoritiesConverter();
    authorities.setAuthorityPrefix("ROLE_");
    authorities.setAuthoritiesClaimName("authorities");
    return authorities;
}

JwtGrantedAuthoritiesConverter scope() {
    return new JwtGrantedAuthoritiesConverter();
}

Of course, the ROLE_USER authority comes from the fact that OAuth2UserAuthority is instantiated with a constructor only accepting attributes.

It sounds like you might be mixing Spring Security's OAuth 2.0 Client and Resource Server support together in your application. OAuth2UserAuthority isn't used by Resource Server.

@rolaca11
Copy link
Author

Would you be interested in changing your PR to only introduce DelegatingJwtGrantedAuthoritiesConverter and its corresponding tests?

Of course, however I'll need some time with it, because I'll be busy with other stuff this week

@jzheaux jzheaux added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Nov 11, 2019
@jzheaux jzheaux added this to the 5.3.x milestone Nov 11, 2019
@NeluAkejelu
Copy link

JwtGrantedAuthoritiesConverter composite =
new DelegatingJwtGrantedAuthoritiesConverter(authorities(), scope());

Please any information on the status of this addition DelegatingJwtGrantedAuthoritiesConverter ? I need to implement something similar that allows to extract my granted authorities from both the scope and authorities.

@jzheaux
Copy link
Contributor

jzheaux commented Nov 24, 2020

Thanks for the ping, @NeluAkejelu. My apologies as this one fell off my radar. Please see the related PR to see if it suits your needs.

@NeluAkejelu
Copy link

@jzheaux Thanks. I'll take a look.

rolaca11 pushed a commit to rolaca11/spring-security that referenced this issue Nov 24, 2020
jzheaux added a commit that referenced this issue Nov 24, 2020
- Adjusted internal logic to follow DelegatingOAuth2TokenValidator
- Changed JavaDoc to align more closely with
JwtGrantedAuthoritiesConverter
- Polished test names to follow Spring Security naming convention
- Updated test class name to follow Spring Security naming convention
- Polished tests to use TestJwts
- Added tests to address additional use cases

Closes gh-7596
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: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants