georchestra · groldan · Apr 14, 2023 · Apr 14, 2023 · Apr 5, 2023
diff --git a/gateway/docker-compose.yml → docker-compose.yml b/gateway/docker-compose.yml → docker-compose.yml
diff --git a/docs/authzn.adoc b/docs/authzn.adoc
@@ -165,6 +165,10 @@ Both standard and non-standard claims can be used to set the `GeorchestraUser`'s
 
 |===
 |Property name | Default value | Description
+|`georchestra.gateway.security.oidc.claims.id.path`
+|Defaults to the standard "sub" claim (subject identifier)
+|JSONPath expression to extract the user identifier from the OIDC claims map
+
 |`georchestra.gateway.security.oidc.claims.organization.path`
 |
 |JSONPath expression to extract the organization short name from the OIDC claims map
@@ -193,6 +197,7 @@ Take as example the following claims provided by an OIDC ID Token:
 [source,json]
 ----
 {
+    "icuid": "abc123",
     "family_name": "Doe",
     "given_name": "John",
     "locale": "en-US",
@@ -208,7 +213,8 @@ Take as example the following claims provided by an OIDC ID Token:
 }
 ----
 
-The following configuration properties can be used to extract the role names from the `groups` claim,
+The following configuration properties can be used to extract the user id from the
+`icuid` claim, the role names from the `groups` claim,
 and the organization's short name from the `PartyOrganisationID` claim:
 
 [source,yaml]
@@ -219,6 +225,8 @@ georchestra:
       oidc:
         # Configure mappings of custom IDToken claims to roles and org name
         claims:
+          # JSONPath expression to extract the user id from a non-standard claim. Otherwise defaults to the "sub" claim (subject identifier)
+          id.path: "$.icuid"
           # JSONPath expression to extract the organization identifier conveyed as
           # the sec-org request header to backend georchestra services
           organization.path: "$.PartyOrganisationID"

diff --git a/docs/roles-mappings.adoc b/docs/roles-mappings.adoc
@@ -3,7 +3,7 @@
 The Gateway sends a `sec-roles` HTTP request header to the backend services
 with the role names provided by the authentication provider.
 
-Sometimes, these roles are unsuficcient as some services may require additional
+Sometimes, these roles are insufficient as some services may require additional
 or different role names.
 
 For example, let's say the OpenID Connect provider gives us a role namded `GDI.ADMIN`, and 

diff --git a/...va/org/georchestra/gateway/security/oauth2/OpenIdConnectCustomClaimsConfigProperties.java b/...va/org/georchestra/gateway/security/oauth2/OpenIdConnectCustomClaimsConfigProperties.java
@@ -44,9 +44,14 @@
 @Slf4j(topic = "org.georchestra.gateway.security.oauth2")
 public @Data class OpenIdConnectCustomClaimsConfigProperties {
 
+    private JsonPathExtractor id = new JsonPathExtractor();
     private RolesMapping roles = new RolesMapping();
     private JsonPathExtractor organization = new JsonPathExtractor();
 
+    public Optional<JsonPathExtractor> id() {
+        return Optional.ofNullable(id);
+    }
+
     public Optional<RolesMapping> roles() {
         return Optional.ofNullable(roles);
     }

diff --git a/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java b/gateway/src/main/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapper.java
@@ -49,6 +49,8 @@
  * <p>
  * {@link StandardClaimAccessor standard claims} map as follow:
  * <ul>
+ * <li>{@link StandardClaimAccessor#getSubject() subject} to
+ * {@link GeorchestraUser#getId() id}
  * <li>{@link StandardClaimAccessor#getPreferredUsername preferredUsername} or
  * {@link StandardClaimAccessor#getEmail email} to
  * {@link GeorchestraUser#setUsername username}, in that order of precedence.
@@ -164,6 +166,11 @@ public class OpenIdConnectUserMapper extends OAuth2UserMapper {
     @VisibleForTesting
     void applyNonStandardClaims(Map<String, Object> claims, GeorchestraUser target) {
 
+        nonStandardClaimsConfig.id().map(jsonEvaluator -> jsonEvaluator.extract(claims))//
+                .map(List::stream)//
+                .flatMap(Stream::findFirst)//
+                .ifPresent(target::setId);
+
         nonStandardClaimsConfig.roles().ifPresent(rolesMapper -> rolesMapper.apply(claims, target));
         nonStandardClaimsConfig.organization().map(jsonEvaluator -> jsonEvaluator.extract(claims))//
                 .map(List::stream)//
@@ -173,6 +180,7 @@ void applyNonStandardClaims(Map<String, Object> claims, GeorchestraUser target)
 
     @VisibleForTesting
     void applyStandardClaims(StandardClaimAccessor standardClaims, GeorchestraUser target) {
+        String subjectId = standardClaims.getSubject();
         String preferredUsername = standardClaims.getPreferredUsername();
         String givenName = standardClaims.getGivenName();
         String familyName = standardClaims.getFamilyName();
@@ -183,6 +191,7 @@ void applyStandardClaims(StandardClaimAccessor standardClaims, GeorchestraUser t
         AddressStandardClaim address = standardClaims.getAddress();
         String formattedAddress = address == null ? null : address.getFormatted();
 
+        apply(target::setId, subjectId);
         apply(target::setUsername, preferredUsername, email);
         apply(target::setFirstName, givenName);
         apply(target::setLastName, familyName);

diff --git a/gateway/src/main/resources/application.yml b/gateway/src/main/resources/application.yml
@@ -136,4 +136,4 @@ logging:
 
 ---
 spring.config.activate.on-profile: dev
-spring.config.import: file:./datadir/default.properties,file:./datadir/gateway/gateway.yaml
+spring.config.import: file:../datadir/default.properties,file:../datadir/gateway/gateway.yaml
diff --git a/...ay/src/test/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapperTest.java b/...ay/src/test/java/org/georchestra/gateway/security/oauth2/OpenIdConnectUserMapperTest.java
@@ -19,6 +19,7 @@
 
 package org.georchestra.gateway.security.oauth2;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -55,6 +56,7 @@ void setUp() throws Exception {
     @Test
     void applyStandardClaims() {
         StandardClaimAccessor standardClaims = mock(StandardClaimAccessor.class);
+        when(standardClaims.getSubject()).thenReturn("b7f3dd13-f9cc-4573-8482-b4fccf8e1977");
         when(standardClaims.getPreferredUsername()).thenReturn("tesuser");
         when(standardClaims.getGivenName()).thenReturn("John");
         when(standardClaims.getFamilyName()).thenReturn("Doe");
@@ -68,6 +70,7 @@ void applyStandardClaims() {
         GeorchestraUser target = new GeorchestraUser();
         mapper.applyStandardClaims(standardClaims, target);
 
+        assertEquals(standardClaims.getSubject(), target.getId());
         assertEquals(standardClaims.getPreferredUsername(), target.getUsername());
         assertEquals(standardClaims.getGivenName(), target.getFirstName());
         assertEquals(standardClaims.getFamilyName(), target.getLastName());
@@ -92,8 +95,7 @@ void applyNonStandardClaims_jsonPath_nested_array_single_value_to_roles() throws
                         + "] ] " //
                         + "}";
 
-        @SuppressWarnings("unchecked")
-        Map<String, Object> claims = (Map<String, Object>) JSONUtils.parseJSON(json.replaceAll("'", "\""));
+        Map<String, Object> claims = sampleClaims(json);
 
         GeorchestraUser target = new GeorchestraUser();
         mapper.applyNonStandardClaims(claims, target);
@@ -121,8 +123,7 @@ void applyNonStandardClaims_jsonPath_nested_array_multiple_values_to_roles() thr
                         + "] ] " //
                         + "}";
 
-        @SuppressWarnings("unchecked")
-        Map<String, Object> claims = (Map<String, Object>) JSONUtils.parseJSON(json.replaceAll("'", "\""));
+        Map<String, Object> claims = sampleClaims(json);
 
         GeorchestraUser target = new GeorchestraUser();
         mapper.applyNonStandardClaims(claims, target);
@@ -154,8 +155,7 @@ void applyNonStandardClaims_jsonPath_multiple_json_paths() throws ParseException
                         + "] ], " //
                         + "'PartyOrganisationID': '6007280321'" + "}";
 
-        @SuppressWarnings("unchecked")
-        Map<String, Object> claims = (Map<String, Object>) JSONUtils.parseJSON(json.replaceAll("'", "\""));
+        Map<String, Object> claims = sampleClaims(json);
 
         GeorchestraUser target = new GeorchestraUser();
         mapper.applyNonStandardClaims(claims, target);
@@ -171,10 +171,8 @@ void applyNonStandardClaims_jsonPath_to_organization() throws ParseException {
         final String jsonPath = "$.PartyOrganisationID";
         nonStandardClaimsConfig.getOrganization().getPath().add(jsonPath);
 
-        final String json = "{'PartyOrganisationID': '6007280321'}";
-
-        @SuppressWarnings("unchecked")
-        Map<String, Object> claims = (Map<String, Object>) JSONUtils.parseJSON(json.replaceAll("'", "\""));
+        Map<String, Object> claims = sampleClaims();
+        assertThat(claims.get("PartyOrganisationID")).isEqualTo("6007280321");
 
         GeorchestraUser target = new GeorchestraUser();
         target.setOrganization("unexpected");
@@ -185,4 +183,76 @@ void applyNonStandardClaims_jsonPath_to_organization() throws ParseException {
         assertEquals(expected, actual);
     }
 
+    @Test
+    void applyNonStandardClaim_jsonPath_to_userId() throws Exception {
+        final String icuid = "50334123";
+        Map<String, Object> claims = sampleClaims();
+        assertThat(claims.get("icuid")).isEqualTo(icuid);
+
+        final String jsonPath = "$.icuid";
+        nonStandardClaimsConfig.getId().getPath().add(jsonPath);
+
+        GeorchestraUser target = new GeorchestraUser();
+        mapper.applyNonStandardClaims(claims, target);
+        assertEquals(icuid, target.getId());
+    }
+
+    private Map<String, Object> sampleClaims() throws ParseException {
+        String json = SAMPLE_CLAIMS;
+        return sampleClaims(json);
+    }
+
+    private Map<String, Object> sampleClaims(String json) throws ParseException {
+        @SuppressWarnings("unchecked")
+        Map<String, Object> claims = (Map<String, Object>) JSONUtils.parseJSON(json.replaceAll("'", "\""));
+        return claims;
+    }
+
+    /**
+     * Sample value for IDToken's "claims": {...}
+     */
+    private static final String SAMPLE_CLAIMS = //
+            "{\n" //
+                    + "          'at_hash': 'YuZBluv2Ehrn_nEqNi0NzA',\n" //
+                    + "          'icuid': '50334123',\n" //
+                    + "          'sub': 'b7f3dd13-f9cc-4573-8482-b4fccf8e1977',\n" //
+                    + "          'groups_json': [\n" //
+                    + "            [\n" //
+                    + "              {\n" //
+                    + "                'parameter': [\n" //
+                    + "                  \n" //
+                    + "                ],\n" //
+                    + "                'name': 'GDI Planer (extern)',\n" //
+                    + "                'targetSystem': 'gdi'\n" //
+                    + "              },\n" //
+                    + "              {\n" //
+                    + "                'parameter': [\n" //
+                    + "                  \n" //
+                    + "                ],\n" //
+                    + "                'name': 'GDI Editor (extern)',\n" //
+                    + "                'targetSystem': 'gdi'\n" //
+                    + "              }\n" //
+                    + "            ]\n" //
+                    + "          ],\n" //
+                    + "          'email_verified': false,\n" //
+                    + "          'iss': 'https://test.login/auth/realms/external-customer-k2',\n" //
+                    + "          'typ': 'ID',\n" //
+                    + "          'preferred_username': 'gabriel.roldan@test.com',\n" //
+                    + "          'given_name': 'Gabriel',\n" //
+                    + "          'nonce': 'p1239kUkQjqBNA7YHBjAiiYy7ULhGq-K01NiF-fm_CEI',\n" //
+                    + "          'sid': 'f123a5b6-a326-4cbe-8af0-75e6f633f0b9',\n" //
+                    + "          'PartyOrganisationID': '6007280321',\n" //
+                    + "          'aud': [\n" //
+                    + "            'gdi'\n" //
+                    + "          ],\n" //
+                    + "          'azp': 'gdi',\n" //
+                    + "          'auth_time': 1681387195,\n" //
+                    + "          'name': 'Gabriel Roldan',\n" //
+                    + "          'exp': '2023-04-13T12:04:57Z',\n" //
+                    + "          'session_state': 'f123a5b6-a326-4cbe-8af0-75e6f633f0b9',\n" //
+                    + "          'iat': '2023-04-13T11:59:57Z',\n" //
+                    + "          'family_name': 'Roldan',\n" //
+                    + "          'jti': 'dc886b41-9d9c-4652-b9c7-8f160c037ccc',\n" //
+                    + "          'email': 'gabriel.roldan@test.com'\n" //
+                    + "        }";
 }