Add updates to spring security config

cBioPortal · Jan 17, 2024 · 3a3cdc8 · 3a3cdc8
1 parent cf65392
commit 3a3cdc8
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 169 deletions.
diff --git a/src/main/java/org/cbioportal/security/CustomJwtGrantedAuthoritiesConverter.java b/src/main/java/org/cbioportal/security/CustomJwtGrantedAuthoritiesConverter.java
diff --git a/src/main/java/org/cbioportal/security/config/OAuth2SecurityConfig.java b/src/main/java/org/cbioportal/security/config/OAuth2SecurityConfig.java
@@ -1,7 +1,7 @@
 package org.cbioportal.security.config;
 
-import org.json.simple.JSONArray;
-import org.json.simple.JSONObject;
+import org.cbioportal.security.util.ClaimRoleExtractorUtil;
+import org.cbioportal.security.util.GrantedAuthorityUtil;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -12,20 +12,20 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
 import org.springframework.security.core.GrantedAuthority;
-import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
 import org.springframework.security.oauth2.client.oidc.web.logout.OidcClientInitiatedLogoutSuccessHandler;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
-import org.springframework.security.oauth2.core.oidc.OidcIdToken;
-import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
 import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
 import org.springframework.security.oauth2.core.user.OAuth2UserAuthority;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.logout.LogoutSuccessHandler;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -38,15 +38,9 @@ public class OAuth2SecurityConfig {
 
     // TODO - add this to portal.properties.EXAMPLE
     // TODO - discuss changing this to /logout (Spring Security default) with Aaron
-    @Value("${oauth2.logout.url:/j_spring_security_logout}")
+    @Value("${oauth2.logout.url:/logout}")
     private String logoutUrl;
 
-    // FIXME - we cannot rely on the idp being named 'keycloak' here. In principle
-    // it can be any name, and there can be multiple idp's configured.
-    @Value("${spring.security.oauth2.client.registration.keycloak.client-id:cbioportal_oauth2}")
-    private String clientId;
-
-    // TODO - implement configured retrieval of roles based on this property
     @Value("${spring.security.oauth2.client.jwt-roles-path:resource_access::cbioportal::roles}")
     private String jwtRolesPath;
 
@@ -69,7 +63,6 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
                 )
             )
             .logout(logout -> logout
-                .logoutUrl(logoutUrl)
                 .logoutSuccessHandler(oidcLogoutSuccessHandler())
             );
         return http.build();
@@ -80,77 +73,34 @@ private GrantedAuthoritiesMapper userAuthoritiesMapper() {
             Set<GrantedAuthority> mappedAuthorities = new HashSet<>();
 
             authorities.forEach(authority -> {
-                if (OidcUserAuthority.class.isInstance(authority)) {
-                    OidcUserAuthority oidcUserAuthority = (OidcUserAuthority) authority;
-
-                    OidcIdToken idToken = oidcUserAuthority.getIdToken();
-                    OidcUserInfo userInfo = oidcUserAuthority.getUserInfo();
-
-                    // Map the claims found in idToken and/or userInfo
-                    // to one or more GrantedAuthority's and add it to mappedAuthorities
-                    getRolesFromIdToken(idToken.getClaims()).forEach(role -> {
-                        mappedAuthorities.add(role);
-                    });
-                    getRolesFromUserInfo(userInfo.getClaims()).forEach(role -> {
-                        mappedAuthorities.add(role);
-                    });
-                    // This section is not needed for the Keycloak OIDC client, but is included for interoperability with pure OAuth2 providers
-                } else if (OAuth2UserAuthority.class.isInstance(authority)) {
-                    OAuth2UserAuthority oauth2UserAuthority = (OAuth2UserAuthority) authority;
-
-                    Map<String, Object> userAttributes = oauth2UserAuthority.getAttributes();
-                    getRolesFromIdToken(userAttributes).forEach(role -> {
-                        mappedAuthorities.add(role);
-                    });
-
+                List<Map<String, Object>> claims = new ArrayList<>();
+                if (authority instanceof OidcUserAuthority oidcUserAuthority && !Objects.isNull(oidcUserAuthority.getUserInfo())) {
+                    claims.add(oidcUserAuthority.getUserInfo().getClaims());
+                    claims.add(oidcUserAuthority.getIdToken().getClaims());
+                } else if (authority instanceof OAuth2UserAuthority oauth2UserAuthority) {
+                    claims.add(oauth2UserAuthority.getAttributes());
                 }
-            });
-
+                if(!claims.isEmpty()) {
+                    var roles = claims.stream()
+                        .filter(claim -> !Objects.isNull(claim))
+                        .map(claim -> ClaimRoleExtractorUtil.extractClientRoles(claim, jwtRolesPath))
+                        .flatMap(Collection::stream)
+                        .collect(Collectors.toSet());
+
+                    mappedAuthorities.addAll(GrantedAuthorityUtil.generateGrantedAuthoritiesFromRoles(roles));
+                }
+                });
             return mappedAuthorities;
         };
     }
 
-
     // See: https://docs.spring.io/spring-security/reference/5.7-SNAPSHOT/servlet/oauth2/login/advanced.html#oauth2login-advanced-oidc-logout
     private LogoutSuccessHandler oidcLogoutSuccessHandler() {
         OidcClientInitiatedLogoutSuccessHandler oidcLogoutSuccessHandler =
             new OidcClientInitiatedLogoutSuccessHandler(this.clientRegistrationRepository);
-        oidcLogoutSuccessHandler.setPostLogoutRedirectUri("{baseUrl}");
+        oidcLogoutSuccessHandler.setPostLogoutRedirectUri("{baseUrl}/login?logout_success");
         return oidcLogoutSuccessHandler;
-
-    }
-
-    private List<GrantedAuthority> getRolesFromUserInfo(Map<String, Object> claims) {
-        if (claims.containsKey("resource_access")) {
-            Map<String, Object> resourceAccess = (Map<String, Object>) claims.get("resource_access");
-            if (resourceAccess.containsKey(clientId)) {
-                Map<String, Object> cbioportalClient = (Map<String, Object>) resourceAccess.get(clientId);
-                if (cbioportalClient.containsKey("roles")) {
-                    List<String> roles = (List<String>) cbioportalClient.get("roles");
-                    return roles.stream()
-                        .map(r -> new SimpleGrantedAuthority(r))
-                        .collect(Collectors.toList());
-                }
-            }
-        }
-        return List.of();
     }
 
 
-    private List<GrantedAuthority> getRolesFromIdToken(Map<String, Object> claims) {
-        if (claims.containsKey("resource_access")) {
-            JSONObject resourceAccess = (JSONObject) claims.get("resource_access");
-            if (resourceAccess.containsKey(clientId)) {
-                JSONObject cbioportalClient = (JSONObject) resourceAccess.get(clientId);
-                if (cbioportalClient.containsKey("roles")) {
-                    JSONArray roles = (JSONArray) cbioportalClient.get("roles");
-                    return (List<GrantedAuthority>) roles.stream()
-                        .map(r -> new SimpleGrantedAuthority((String) r))
-                        .collect(Collectors.toList());
-                }
-            }
-        }
-        return List.of();
-    }
-
 }
diff --git a/src/main/java/org/cbioportal/security/config/OptionalOAuth2SecurityConfig.java b/src/main/java/org/cbioportal/security/config/OptionalOAuth2SecurityConfig.java
@@ -1,7 +1,5 @@
 package org.cbioportal.security.config;
 
-import org.cbioportal.security.CustomJwtGrantedAuthoritiesConverter;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.security.SecurityProperties;
 import org.springframework.context.annotation.Bean;
@@ -10,7 +8,6 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
-import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter;
 import org.springframework.security.web.SecurityFilterChain;
 
 
@@ -21,9 +18,6 @@
 @ConditionalOnProperty(value = "authenticate", havingValue = "optional_oauth2")
 public class OptionalOAuth2SecurityConfig {
 
-    @Value("${spring.security.oauth2.roles-path.client-id:}")
-    private String clientId;
-
     @Bean
     public SecurityFilterChain optionalOAuth2filterChain(HttpSecurity http) throws Exception {
         return http
@@ -36,14 +30,4 @@ public SecurityFilterChain optionalOAuth2filterChain(HttpSecurity http) throws E
             .build();
     }
 
-    @Bean
-    public JwtAuthenticationConverter jwtAuthenticationConverter() {
-        CustomJwtGrantedAuthoritiesConverter grantedAuthoritiesConverter = new CustomJwtGrantedAuthoritiesConverter();
-        grantedAuthoritiesConverter.setClientId(this.clientId);
-
-        JwtAuthenticationConverter jwtAuthenticationConverter = new JwtAuthenticationConverter();
-        jwtAuthenticationConverter.setJwtGrantedAuthoritiesConverter(grantedAuthoritiesConverter);
-        return jwtAuthenticationConverter;
-    }
-
 }
diff --git a/src/main/java/org/cbioportal/security/config/Saml2SecurityConfig.java b/src/main/java/org/cbioportal/security/config/Saml2SecurityConfig.java
@@ -2,7 +2,6 @@
 
 import org.cbioportal.security.util.GrantedAuthorityUtil;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -38,9 +37,6 @@
 @ConditionalOnProperty(value = "authenticate", havingValue = "saml")
 public class Saml2SecurityConfig {
 
-    @Value("${saml.logout.url:/saml/logout}")
-    private String logoutUrl;
-
     @Autowired(required = false)
     private RelyingPartyRegistrationRepository relyingPartyRegistrationRepository;
 
@@ -57,7 +53,7 @@ public SecurityFilterChain samlFilterChain(HttpSecurity http) throws Exception {
             // FIXME - csrf should be enabled
             .csrf(AbstractHttpConfigurer::disable)
             .authorizeHttpRequests(auth ->
-                auth.requestMatchers("/api/health", "/images/**", "/js/**").permitAll()
+                auth.requestMatchers("/api/health", "/images/**", "/js/**", "/login").permitAll()
                     .anyRequest().authenticated())
             .exceptionHandling(eh ->
                 eh.defaultAuthenticationEntryPointFor(
@@ -69,7 +65,7 @@ public SecurityFilterChain samlFilterChain(HttpSecurity http) throws Exception {
             // described at https://docs.spring.io/spring-security/reference/6.1/servlet/saml2/logout.html
             // Logout Service POST Binding URL: http://localhost:8080/logout/saml2/slo
             .logout(logout -> logout
-                .logoutUrl(logoutUrl)
+                .logoutUrl("/logout")
                 .logoutSuccessHandler(logoutSuccessHandler())
             )
             .build();
@@ -102,6 +98,7 @@ public LogoutSuccessHandler logoutSuccessHandler() {
             new DefaultRelyingPartyRegistrationResolver(relyingPartyRegistrationRepository);
         OpenSaml4LogoutRequestResolver logoutRequestResolver =
             new OpenSaml4LogoutRequestResolver(relyingPartyRegistrationResolver);
+
         return new Saml2RelyingPartyInitiatedLogoutSuccessHandler(logoutRequestResolver);
     }
 

diff --git a/src/main/java/org/cbioportal/security/token/oauth2/OAuth2TokenAuthenticationProvider.java b/src/main/java/org/cbioportal/security/token/oauth2/OAuth2TokenAuthenticationProvider.java
@@ -34,21 +34,20 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import org.cbioportal.security.util.ClaimRoleExtractorUtil;
+import org.cbioportal.security.util.GrantedAuthorityUtil;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.BadCredentialsException;
 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.jwt.Jwt;
 import org.springframework.security.jwt.JwtHelper;
 
 import java.io.IOException;
-import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.stream.StreamSupport;
+import java.util.Collection;
 
 public class OAuth2TokenAuthenticationProvider implements AuthenticationProvider {
 
@@ -74,34 +73,18 @@ public Authentication authenticate(Authentication authentication) throws Authent
         // request an access token from the OAuth2 identity provider
         final String accessToken = tokenRefreshRestTemplate.getAccessToken(offlineToken);
 
-        Set<GrantedAuthority> authorities = extractAuthorities(accessToken);
+        Collection<GrantedAuthority> authorities = extractAuthorities(accessToken);
         String username = getUsername(accessToken);
 
         return new OAuth2BearerAuthenticationToken(username, authorities);
     }
 
     // Read roles/authorities from JWT token.
-    private Set<GrantedAuthority> extractAuthorities(final String token) throws BadCredentialsException {
+    private Collection<GrantedAuthority> extractAuthorities(final String token) throws BadCredentialsException {
         try {
             final Jwt tokenDecoded = JwtHelper.decode(token);
             final String claims = tokenDecoded.getClaims();
-            JsonNode claimsMap = new ObjectMapper().readTree(claims);
-            JsonNode rolesArrayCursor = claimsMap;
-            for (String keyName: jwtRolesPath.split("::")) {
-                if (rolesArrayCursor.has(keyName)) {
-                    rolesArrayCursor = rolesArrayCursor.get(keyName);
-                } else {
-                    throw new BadCredentialsException("Cannot find user roles in JWT access token with path '"
-                        + jwtRolesPath + "''. Please ensure the dat.oauth2.jwtRolesPath property is correct.");
-                }
-            }
-            final JsonNode rolesArrayPointer = rolesArrayCursor;
-            final Iterable<JsonNode> roles = () -> rolesArrayPointer.elements();
-
-            return StreamSupport.stream(roles.spliterator(), false)
-                .map(role -> role.toString().replaceAll("\"", ""))
-                .map(role -> new SimpleGrantedAuthority(role))
-                .collect(Collectors.toSet());
+            return GrantedAuthorityUtil.generateGrantedAuthoritiesFromRoles(ClaimRoleExtractorUtil.extractClientRoles(claims, jwtRolesPath));
 
         } catch (Exception e) {
             throw new BadCredentialsException("Authorities could not be extracted from access token.");