Spring-security-oauth: checkUserScopes is not working as expected

[rwinch]

In the class DefaultOAuth2RequestFactory, the method checkUserScopes is comparing client scopes against client authorities, and i think that would be more reasonable comparing against user's authorities instead.

Related StackOverflow Question

Copied from SEC-3064

[saintcoder]

FYI - checkUserScopes() methods does compare user authorities before the approval step. However after approval it checks with client authorities (instead of user authorities) and fails. This is because the user in security context changes from user to client after approval.

Also related StackOverflow question

[codyfyi]

Yes this is still a problem. The DefaultSecurityContextAccessor#getAuthorities is called in the checkUserScopes method but due to the security context, no scopes are returned.

This results in:
{"error":"invalid_scope","error_description":"Empty scope (either the client or the user is not allowed the requested scopes)"}

[TiBeN]

I have the same issue. Is there a workaround ?

[TiBeN]

False positive for me, it works. (i use the password credentials). I had to inject a DefaultOAuth2RequestFactory into a TokenEndpoint Bean and simply declare a TokenEndpointAuthenticationFilter as a bean, it is automatically added to the Security filter chain.

[nick-pww]

Just ran into this same issue, and it is definitely confusing. It doesn't look like the feature is doing anything like what is described in the tutorial. In the DefaultOAuth2RequestFactory the following method always tries to extract scopes from the Client authentication details.

private Set<String> checkUserScopes(Set<String> scopes, ClientDetails clientDetails) {
        if (!securityContextAccessor.isUser()) {
            return scopes;
        }
        Set<String> result = new LinkedHashSet<String>();
        Set<String> authorities = AuthorityUtils.authorityListToSet(securityContextAccessor.getAuthorities());
        for (String scope : scopes) {
            if (authorities.contains(scope) || authorities.contains(scope.toUpperCase())
                    || authorities.contains("ROLE_" + scope.toUpperCase())) {
                result.add(scope);
            }
        }
        return result;
    }

To me, this is not expected behavior, and would not begin to think of having authorities on clients , or at least not combining authorities with scopes. Also note that it is passing in ClientDetails but never uses it. I don't see how this code would ever work as described with the default Basic auth setup where clients are authed first, and then the user details are loaded.

As is, this would only return scopes from the request that match scopes on the client, not the user. Nowhere in the code that I could find actually looks at the users authorities and maps them to scopes.

I was able to come up with an hacky work around (mostly because of assumed types and having to cast) by registering my own TokenEnhancer:

public class UserRolesToScopeEnhancer implements TokenEnhancer {

    @Override
    public OAuth2AccessToken enhance(OAuth2AccessToken token, OAuth2Authentication auth) {
        Collection<? extends GrantedAuthority> auths = auth.getUserAuthentication().getAuthorities();
        Set<String> authStrings = Collections.unmodifiableSet(auths.stream().map(r -> r.getAuthority()).collect(Collectors.toSet()));
        ((DefaultOAuth2AccessToken)token).setScope(authStrings);
        return token;
    }

}

I'm more then willing to try and come up with a patch for this, but am unsure of the desired intentions here as documentation and code really do not line up, and changing this would could certainly break existing code and apps.

[abccbaandy]

Still don't know how checkUserScopes work, it use SecurityContextHolder but client is in SecurityContext not user.
This make password grant type strange, because I need check if user's scope is valid.

[xak2000]

The DefaultSecurityContextAccessor.getUserAuthentication() method is also wrong, as it returns not user, but client authentication, if Authentication is not a OAuth2Authentication instance (and it's actually a UsernamePasswordAuthenticationToken instance because SecurityContextHolder contains a client authentication at this point).

DefaultOAuth2RequestFactory.checkUserScopes() uses DefaultSecurityContextAccessor.isUser() to check if current authentication is user, but this method always returns true for the reasons mentioned above.

And then it uses DefaultSecurityContextAccessor.getAuthorities(), which actually returns client authorities. So, in presence of these 2 bugs, virtually checkUserScopes() method filters scopes by client authorities.

I don't think it's even possible to check user authorities at this point, because TokenGranter isn't even selected yet and user isn't authenticated yet at this point. So, I honestly don't understand the intention of DefaultOAuth2RequestFactory.checkUserScopes() method, because it is just impossible to implement it (no authenticated user exist at this point in time).

[abdurrehmansyeds]

I was struggling with same issue, since by default the securityContext has client details I extended the DefaultOauth2RequestFactory and have set the User authentication manually in SecurityContext
@Override public TokenRequest createTokenRequest(Map<String, String> requestParameters, ClientDetails authenticatedClient) { SecurityContextHolder.getContext() .setAuthentication(new UsernamePasswordAuthenticationToken(requestParameters.get("username"), null, userDetailsService.loadUserByUsername(requestParameters.get("username")).getAuthorities())); } return super.createTokenRequest(requestParameters, authenticatedClient);

With this code in place the SecurityContext will always be populated by User authentication rather than Client authentication
you can do this for specific grant type aswell
if (requestParameters.get("grant_type").equals("password")) { //same code as above }