spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.72k stars 5.86k forks source link

Access to accessToken in GrantedAuthoritiesMapper #14461

Open Xyaren opened 8 months ago

Xyaren commented 8 months ago

Expected Behavior

When using oauth2login it is should be possible to use the access token in GrantedAuthoritiesMapper in order to map access token claims to authorities for use in hasRole/hasAuthority. This is espacially usefull when some claims do only appear in the access token.

Current Behavior GrantedAuthoritiesMapper has currently no access to the accessToken.

Context https://docs.spring.io/spring-security/reference/reactive/oauth2/login/advanced.html#webflux-oauth2-login-advanced-map-authorities-grantedauthoritiesmapper

My (dirty) workaround involves extending the OidcUserService and returning a new OidcUser Object.

jzheaux commented 8 months ago

Hi, @Xyaren, thanks for reaching out. What is making you feel like extending OidcUserService is a dirty workaround? I'm thinking you might do something like this:

@Component
public class MyOidcUserService implements OAuth2UserService<OidcUserRequest, OidcUser> {
    private final OidcUserService delegate = new OidcUserService();

    @Override
    public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
        OidcUser user = this.delegate.loadUser(userRequest);
        Collection<GrantedAuthority> authorities = new ArrayList<>(user.getAuthorities());
        // ... add custom authorities
        return new DefaultOidcUser(authorities, user.getIdToken(), user.getUserInfo());
    }
}

which doesn't seem dirty to me. I want to make sure I'm not missing something; can you clarify please?

Xyaren commented 8 months ago

Hi, thanks for the quick response! I do have a similar workaround, that I'll attach at the end.

It feels odd wrapping the original service, tear apart the response and stitch it together again in a new instance of the same object. I also was unsure about "reconstructing" the OidcUser due to no access to nameAttributeKey which is required for the all-arg constructor. Therefore I went for extending the default implementation and wrapping the result. It there anything speaking against having the Access token available within the OidcUser ? This would allow users to use a GrantedAuthoritiesMapper to map from any of the sources.

import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;

import packages.from.my.company.that.i.dont.want.to.name.JwtToAuthorityExtractor;
import lombok.AllArgsConstructor;
import lombok.experimental.Delegate;
import org.jetbrains.annotations.NotNull;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest;
import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
import org.springframework.security.oauth2.jwt.SupplierJwtDecoder;

@AllArgsConstructor
public class ExtendedOidcUserService extends OidcUserService {

    private SupplierJwtDecoder supplierJwtDecoder;

    @Override
    public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
        var oidcUser = super.loadUser(userRequest);
        var accessToken = userRequest.getAccessToken();

        var authorityExtractor = new JwtToAuthorityExtractor(
            userRequest.getClientRegistration().getClientId(),
            Set.of(),
            false);
        var jwt = supplierJwtDecoder.decode(accessToken.getTokenValue());
        Set<GrantedAuthority> additionalAuthorities = authorityExtractor.authorities(jwt).collect(Collectors.toSet());
        return new ExtendedOidcUser(oidcUser, additionalAuthorities);
    }

    static class ExtendedOidcUser implements OidcUser {
        @Delegate
        private final OidcUser oidcUser;
        private final Collection<? extends GrantedAuthority> authorities;

        public ExtendedOidcUser(OidcUser oidcUser, Collection<? extends GrantedAuthority> additionalAuthorities) {
            this.oidcUser = oidcUser;
            this.authorities = joinAuthorities(oidcUser, additionalAuthorities);
        }

        @SuppressWarnings({"LombokGetterMayBeUsed", "RedundantSuppression"})
        public Collection<? extends GrantedAuthority> getAuthorities() {
            return authorities;
        }

        @NotNull
        private static HashSet<GrantedAuthority> joinAuthorities(
            OidcUser oidcUser,
            Collection<? extends GrantedAuthority> additionalAuthorities) {

            HashSet<GrantedAuthority> totalAuthorities = new HashSet<>(
                oidcUser.getAuthorities().size() + additionalAuthorities.size());

            totalAuthorities.addAll(oidcUser.getAuthorities());
            totalAuthorities.addAll(additionalAuthorities);
            return totalAuthorities;
        }
    }
}