Support for OIDC scopes (#357)

geosolutions-it · Jul 11, 2024 · 64fe0d1 · 64fe0d1
1 parent be62769
commit 64fe0d1
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 20 deletions.
diff --git a/...n/java/it/geosolutions/geostore/services/rest/security/keycloak/KeyCloakLoginService.java b/...n/java/it/geosolutions/geostore/services/rest/security/keycloak/KeyCloakLoginService.java
@@ -101,7 +101,7 @@ public Response doInternalRedirect(
             token = getAccessToken();
             refreshToken = getRefreshAccessToken();
         }
-        return buildCallbackResponse(token, refreshToken, provider);
+        return buildCallbackResponse(response, token, refreshToken, provider);
     }
 
     private KeycloakTokenDetails getDetails() {

diff --git a/...src/main/java/it/geosolutions/geostore/services/rest/security/oauth2/DiscoveryClient.java b/...src/main/java/it/geosolutions/geostore/services/rest/security/oauth2/DiscoveryClient.java
@@ -27,10 +27,7 @@
  */
 package it.geosolutions.geostore.services.rest.security.oauth2;
 
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.stream.Collectors;
+import java.util.*;
 import org.springframework.web.client.RestTemplate;
 
 /**
@@ -103,20 +100,22 @@ public void autofill(OAuth2Configuration conf) {
                     .ifPresent(uri -> conf.setIdTokenUri((String) uri));
             Optional.ofNullable(response.get(getEndSessionEndpoint()))
                     .ifPresent(uri -> conf.setLogoutUri((String) uri));
-            Optional.ofNullable(response.get(getScopesSupported()))
-                    .ifPresent(
-                            s -> {
-                                @SuppressWarnings("unchecked")
-                                List<String> scopes = (List<String>) s;
-                                conf.setScopes(collectScopes(scopes));
-                            });
             Optional.ofNullable(response.get(getRevocationEndpoint()))
                     .ifPresent(s -> conf.setRevokeEndpoint((String) s));
+            if (conf.getScopes() == null || conf.getScopes().isEmpty()) {
+                Optional.ofNullable(response.get(getScopesSupported()))
+                        .ifPresent(
+                                s -> {
+                                    @SuppressWarnings("unchecked")
+                                    List<String> scopes = (List<String>) s;
+                                    conf.setScopes(collectScopes(scopes));
+                                });
+            }
         }
     }
 
     private String collectScopes(List<String> scopes) {
-        return scopes.stream().collect(Collectors.joining(","));
+        return String.join(",", scopes);
     }
 
     protected String getUserinfoEndpointAttrName() {

diff --git a/.../main/java/it/geosolutions/geostore/services/rest/security/oauth2/Oauth2LoginService.java b/.../main/java/it/geosolutions/geostore/services/rest/security/oauth2/Oauth2LoginService.java
@@ -43,11 +43,11 @@ public Response doInternalRedirect(
             HttpServletRequest request, HttpServletResponse response, String provider) {
         String token = getAccessToken();
         String refreshToken = getRefreshAccessToken();
-        return buildCallbackResponse(token, refreshToken, provider);
+        return buildCallbackResponse(response, token, refreshToken, provider);
     }
 
     protected Response.ResponseBuilder getCallbackResponseBuilder(
-            String token, String refreshToken, String provider) {
+            HttpServletResponse response, String token, String refreshToken, String provider) {
         Response.ResponseBuilder result = new ResponseBuilderImpl();
         IdPConfiguration configuration = configuration(provider);
         LOGGER.info("Callback Provider: {}", provider);
@@ -84,7 +84,7 @@ protected Response.ResponseBuilder getCallbackResponseBuilder(
                                                 + e.getMessage());
             }
         } else {
-            LOGGER.error("No access token found on callback request.");
+            LOGGER.error("No access token found on callback request: {}", response.getStatus());
             result =
                     Response.status(Response.Status.INTERNAL_SERVER_ERROR)
                             .entity("No access token found.");
@@ -104,8 +104,10 @@ protected TokenStorage tokenStorage() {
         return GeoStoreContext.bean(TokenStorage.class);
     }
 
-    protected Response buildCallbackResponse(String token, String refreshToken, String provider) {
-        Response.ResponseBuilder result = getCallbackResponseBuilder(token, refreshToken, provider);
+    protected Response buildCallbackResponse(
+            HttpServletResponse response, String token, String refreshToken, String provider) {
+        Response.ResponseBuilder result =
+                getCallbackResponseBuilder(response, token, refreshToken, provider);
         return result.build();
     }
 

diff --git a/...ions/geostore/services/rest/security/oauth2/openid_connect/OpenIdConnectLoginService.java b/...ions/geostore/services/rest/security/oauth2/openid_connect/OpenIdConnectLoginService.java
@@ -98,6 +98,6 @@ public Response doInternalRedirect(
         }
         assert requestAttributes != null;
         requestAttributes.setAttribute(PROVIDER_KEY, provider, 0);
-        return buildCallbackResponse(token, refreshToken, provider);
+        return buildCallbackResponse(response, token, refreshToken, provider);
     }
 }
diff --git a/...impl/src/test/java/it/geosolutions/geostore/rest/security/oauth2/DiscoveryClientTest.java b/...impl/src/test/java/it/geosolutions/geostore/rest/security/oauth2/DiscoveryClientTest.java
@@ -2,12 +2,14 @@
 
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.common.ConsoleNotifier;
 import it.geosolutions.geostore.services.rest.security.oauth2.DiscoveryClient;
 import it.geosolutions.geostore.services.rest.security.oauth2.OAuth2Configuration;
+import java.util.Arrays;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.springframework.http.MediaType;
@@ -43,6 +45,7 @@ public void testDiscovery() {
         DiscoveryClient discoveryClient =
                 new DiscoveryClient(authService + "/.well-known/openid-configuration");
         OAuth2Configuration configuration = new OAuth2Configuration();
+        configuration.setScopes("openid,groups");
         discoveryClient.autofill(configuration);
         assertEquals("https://oauth2.googleapis.com/token", configuration.getAccessTokenUri());
         assertEquals(
@@ -53,6 +56,15 @@ public void testDiscovery() {
                 "https://openidconnect.googleapis.com/v1/userinfo",
                 configuration.getCheckTokenEndpointUrl());
         assertEquals("https://www.googleapis.com/oauth2/v3/certs", configuration.getIdTokenUri());
-        assertEquals("openid,email,profile", configuration.getScopes());
+        // Split the strings into arrays
+        String[] expectedScopes = "openid,groups".split(",");
+        String[] actualScopes = configuration.getScopes().split(",");
+
+        // Sort the arrays
+        Arrays.sort(expectedScopes);
+        Arrays.sort(actualScopes);
+
+        // Compare the sorted arrays
+        assertArrayEquals("The scopes match", expectedScopes, actualScopes);
     }
 }