Expected Behavior
Should be possible to configure scope delimiter if server sends scopes as comma-delimited string (e.g. GitHub does this).
Current Behavior
Delimiter is hard coded here https://github.com/spring-projects/spring-security/blob/de104e22b7855172876d7be6dc6ef882755da60a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/DefaultMapOAuth2AccessTokenResponseConverter.java#L80-L86
The following makes redefining this behaviour inefficient:
* getScopes is private static method
* DefaultMapOAuth2AccessTokenResponseConverter is final class
* DefaultAuthorizationCodeTokenResponseClient is final class
Context
GitHub OAuth returns comma delimited scopes instead of space delimited as assumed by spring security.
Redefining default behaviour is quire cumbersome and inefficient as it creates redundant instance of OAuth2AccessTokenResponseClient per application and redundant instance of OAuth2AccessTokenResponse for each login event:
public class AdvancedAccessTokenResponseClient implements OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> {
private OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> delegate =
new DefaultAuthorizationCodeTokenResponseClient();
@Override
public OAuth2AccessTokenResponse getTokenResponse(OAuth2AuthorizationCodeGrantRequest authorizationGrantRequest) {
OAuth2AccessTokenResponse tokenResponse = delegate.getTokenResponse(authorizationGrantRequest);
Set<String> scopes = tokenResponse.getAccessToken().getScopes();
Set<String> scopesNew = scopes.stream().flatMap(s -> Arrays.stream(s.split(","))).collect(Collectors.toSet());
if (scopesNew.equals(scopes))
return tokenResponse;
return OAuth2AccessTokenResponse.withResponse(tokenResponse).scopes(scopesNew).build();
}
}
Comment From: sjohnr
Thanks @bfanyuk. Do you have a scenario or use case in mind that runs into an issue? Are you using the scopes programmatically or basing authorization rules for your client application on them?
Comment From: bfanyuk
Hello, @sjohnr! Thanks for checking this out. I would like to hide certain endpoints based on scopes like this ...
@PreAuthorize("hasAuthority('SCOPE_email')")
fun getEmail(): String { }
... or this ...
@Bean
fun securityFilterChain(http: HttpSecurity): SecurityFilterChain {
http {
authorizeHttpRequests {
authorize("/email", hasAuthority("SCOPE_email"))
}
}
return http.build()
}
E.g. what is described here fits the most.
Comment From: sjohnr
@bfanyuk RFC 6749 defines scope as space-delimited so GitHub would appear to be off-spec here. Having said that, I'm wondering if you have tried a GrantedAuthoritiesMapper as a workaround yet?
@Bean
public GrantedAuthoritiesMapper userAuthoritiesMapper() {
return (authorities) -> authorities.stream().flatMap(this::mapAuthority).toList();
}
private Stream<GrantedAuthority> mapAuthority(GrantedAuthority authority) {
if (authority.getAuthority().startsWith("SCOPE_")) {
var scopes = authority.getAuthority().substring("SCOPE_".length());
return Stream.of(StringUtils.delimitedListToStringArray(scopes, ","))
.map((scope) -> new SimpleGrantedAuthority("SCOPE_" + scope));
}
return Stream.of(authority);
}
This works in an OAuth2 Client application to map authorities during login. You can of course optimize this if intermediary streams are not desired.
Regarding adding the ability to delimit scopes with comma, it is a very deeply nested component in order to achieve this customization and so I don't know that it would make things easier for users to be able to customize this. Also, since Spring Security is operating to spec and the workaround is quite easy, I don't feel it's worth pursuing a change in the framework.
I'll leave this open for a bit longer so you can try out the above workaround, but I plan to close this issue. If you feel strongly about it and want to contribute a PR, I can work with you on it instead of closing the issue (but I'd again re-iterate that I don't feel it would provide much value).