added ROLE_CLIENT to assertion client authentication, cleaned up roles on client secret authentication, closes mitreid-connect#728, closes mitreid-connect#401

Author: jricher

openid-connect-common/src/main/java/org/mitre/oauth2/service/impl/DefaultClientUserDetailsService.java

@@ -18,8 +18,8 @@
 
 import java.math.BigInteger;
 import java.security.SecureRandom;
-import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 
 import org.mitre.oauth2.model.ClientDetailsEntity;
 import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod;
@@ -44,6 +44,8 @@
 @Service("clientUserDetailsService")
 public class DefaultClientUserDetailsService implements UserDetailsService {
 
+private static GrantedAuthority ROLE_CLIENT = new SimpleGrantedAuthority("ROLE_CLIENT");
+
 @Autowired
 private ClientDetailsEntityService clientDetailsService;
 
@@ -70,14 +72,8 @@ public UserDetails loadUserByUsername(String clientId) throws UsernameNotFoundE
 boolean accountNonExpired = true;
 boolean credentialsNonExpired = true;
 boolean accountNonLocked = true;
-Collection<GrantedAuthority> authorities = client.getAuthorities();
-if (authorities == null || authorities.isEmpty()) {
-// automatically inject ROLE_CLIENT if none exists ...
-// TODO: this should probably happen on the client service side instead to keep it in the real data model
-authorities = new ArrayList<GrantedAuthority>();
-GrantedAuthority roleClient = new SimpleGrantedAuthority("ROLE_CLIENT");
-authorities.add(roleClient);
-}
+Collection<GrantedAuthority> authorities = new HashSet<GrantedAuthority>(client.getAuthorities());
+authorities.add(ROLE_CLIENT);
 
 return new User(clientId, password, enabled, accountNonExpired, credentialsNonExpired, accountNonLocked, authorities);
 } else {

openid-connect-server/src/main/java/org/mitre/openid/connect/assertion/JwtBearerAuthenticationProvider.java

@@ -21,6 +21,8 @@
 
 import java.text.ParseException;
 import java.util.Date;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.mitre.jwt.signer.service.JwtSigningAndValidationService;
 import org.mitre.jwt.signer.service.impl.JWKSetCacheService;
@@ -36,6 +38,8 @@
 import org.springframework.security.authentication.AuthenticationServiceException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 import org.springframework.security.oauth2.common.exceptions.InvalidClientException;
 
@@ -52,6 +56,8 @@ public class JwtBearerAuthenticationProvider implements AuthenticationProvider {
 
 private static final Logger logger = LoggerFactory.getLogger(JwtBearerAuthenticationProvider.class);
 
+private static final GrantedAuthority ROLE_CLIENT = new SimpleGrantedAuthority("ROLE_CLIENT");
+
 // map of verifiers, load keys for clients
 @Autowired
 private JWKSetCacheService validators;
@@ -182,7 +188,12 @@ public Authentication authenticate(Authentication authentication) throws Authent
 }
 
 // IFF we managed to get all the way down here, the token is valid
-return new JwtBearerAssertionAuthenticationToken(client.getClientId(), jwt, client.getAuthorities());
+
+// add in the ROLE_CLIENT authority
+Set<GrantedAuthority> authorities = new HashSet<>(client.getAuthorities());
+authorities.add(ROLE_CLIENT);
+
+return new JwtBearerAssertionAuthenticationToken(client.getClientId(), jwt, authorities);
 
 } catch (InvalidClientException e) {
 throw new UsernameNotFoundException("Could not find client: " + jwtAuth.getClientId());