From ed3fc2fcfbb1b72ca6146484afebdf1416ad442e Mon Sep 17 00:00:00 2001 From: Giuseppe Graziano Date: Thu, 15 Jan 2026 08:48:31 +0100 Subject: [PATCH] Fix duplicate address claim in IDToken (#45423) Closes #45250 Signed-off-by: Giuseppe Graziano (cherry picked from commit db1f75a1cfa77cf24cf79b1fc6f9d4a573a00ce6) --- .../org/keycloak/representations/IDToken.java | 30 +++--- .../keycloak/representations/UserInfo.java | 40 +++---- .../protocol/oidc/mappers/AddressMapper.java | 11 +- .../oauth/OIDCProtocolMappersTest.java | 101 +++++++++--------- .../testsuite/oidc/OIDCScopeTest.java | 2 +- 5 files changed, 86 insertions(+), 98 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/IDToken.java b/core/src/main/java/org/keycloak/representations/IDToken.java index 17383d851dc..603f2680fb3 100755 --- a/core/src/main/java/org/keycloak/representations/IDToken.java +++ b/core/src/main/java/org/keycloak/representations/IDToken.java @@ -18,7 +18,6 @@ package org.keycloak.representations; import java.util.Map; -import java.util.Optional; import org.keycloak.TokenCategory; import org.keycloak.util.JsonSerialization; @@ -130,9 +129,6 @@ public class IDToken extends JsonWebToken { @JsonProperty(PHONE_NUMBER_VERIFIED) protected Boolean phoneNumberVerified; - @JsonProperty(ADDRESS) - protected Map address; - @JsonProperty(UPDATED_AT) protected Long updatedAt; @@ -332,28 +328,30 @@ public class IDToken extends JsonWebToken { this.phoneNumberVerified = phoneNumberVerified; } - @JsonProperty("address") + @JsonIgnore public Map getAddressClaimsMap() { - return address; + Object value = getOtherClaims().get(ADDRESS); + return value instanceof Map ? (Map) value : null; } @JsonIgnore public AddressClaimSet getAddress() { - return Optional.ofNullable(address).map(a -> { - return JsonSerialization.mapper.convertValue(a, AddressClaimSet.class); - }) - .orElse(null); - } + Object value = getOtherClaims().get(ADDRESS); + if (value == null) { + return null; + } - public void setAddress(Map address) { - this.address = address; + return JsonSerialization.mapper.convertValue(value, AddressClaimSet.class); } @JsonIgnore public void setAddress(AddressClaimSet address) { - this.address = Optional.ofNullable(address) - .map(a -> JsonSerialization.mapper.convertValue(a, Map.class)) - .orElse(null); + getOtherClaims().put(ADDRESS, JsonSerialization.mapper.convertValue(address, Map.class)); + } + + @JsonIgnore + public void setAddress(Map address) { + getOtherClaims().put(ADDRESS, address); } public Long getUpdatedAt() { diff --git a/core/src/main/java/org/keycloak/representations/UserInfo.java b/core/src/main/java/org/keycloak/representations/UserInfo.java index f7e421747c6..1b02897c994 100755 --- a/core/src/main/java/org/keycloak/representations/UserInfo.java +++ b/core/src/main/java/org/keycloak/representations/UserInfo.java @@ -16,10 +16,8 @@ */ package org.keycloak.representations; -import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.Optional; import org.keycloak.json.StringOrArrayDeserializer; import org.keycloak.json.StringOrArraySerializer; @@ -99,9 +97,6 @@ public class UserInfo { @JsonProperty("phone_number_verified") protected Boolean phoneNumberVerified; - @JsonProperty("address") - protected Map address; - @JsonProperty("updated_at") protected Long updatedAt; @@ -280,28 +275,30 @@ public class UserInfo { this.phoneNumberVerified = phoneNumberVerified; } - @JsonProperty("address") + @JsonIgnore public Map getAddressClaimsMap() { - return address; + Object value = getOtherClaims().get(IDToken.ADDRESS); + return value instanceof Map ? (Map) value : null; } @JsonIgnore public AddressClaimSet getAddress() { - return Optional.ofNullable(address).map(a -> { - return JsonSerialization.mapper.convertValue(a, AddressClaimSet.class); - }) - .orElse(null); - } + Object value = getOtherClaims().get(IDToken.ADDRESS); + if (value == null) { + return null; + } - public void setAddress(Map address) { - this.address = address; + return JsonSerialization.mapper.convertValue(value, AddressClaimSet.class); } @JsonIgnore public void setAddress(AddressClaimSet address) { - this.address = Optional.ofNullable(address) - .map(a -> JsonSerialization.mapper.convertValue(a, Map.class)) - .orElse(null); + getOtherClaims().put(IDToken.ADDRESS, JsonSerialization.mapper.convertValue(address, Map.class)); + } + + @JsonIgnore + public void setAddress(Map address) { + getOtherClaims().put(IDToken.ADDRESS, address); } public Long getUpdatedAt() { @@ -342,13 +339,4 @@ public class UserInfo { public void setOtherClaims(String name, Object value) { otherClaims.put(name, value); } - - @Override - public String toString() { - try { - return JsonSerialization.writeValueAsString(this); - } catch (IOException e) { - throw new RuntimeException(e); - } - } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java index 741c8c08c01..7b20c35d446 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java @@ -124,12 +124,10 @@ public class AddressMapper extends AbstractOIDCProtocolMapper implements OIDCAcc @Override protected void setClaim(IDToken token, ProtocolMapperModel mappingModel, UserSessionModel userSession) { UserModel user = userSession.getUser(); - Map addressSet = Optional.ofNullable(token.getAddressClaimsMap()).orElseGet(() -> { - return Optional.ofNullable(token.getOtherClaims().get(IDToken.ADDRESS)) - .filter(Map.class::isInstance) - .map(o -> (HashMap) o) - .orElseGet(HashMap::new); - }); + Map addressSet = Optional.ofNullable(token.getAddressClaimsMap()).orElseGet(() -> Optional.ofNullable(token.getOtherClaims().get(IDToken.ADDRESS)) + .filter(Map.class::isInstance) + .map(o -> (HashMap) o) + .orElseGet(HashMap::new)); Optional.ofNullable(getUserModelAttributeValue(user, mappingModel, STREET)) .ifPresent(street -> addressSet.put(AddressClaimSet.STREET_ADDRESS, street)); Optional.ofNullable(getUserModelAttributeValue(user, mappingModel, AddressClaimSet.LOCALITY)) @@ -145,7 +143,6 @@ public class AddressMapper extends AbstractOIDCProtocolMapper implements OIDCAcc if (!addressSet.isEmpty()) { token.setAddress(addressSet); - token.getOtherClaims().put(IDToken.ADDRESS, addressSet); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java index 54e24f220bd..66f03546c8e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java @@ -72,7 +72,9 @@ import org.keycloak.testsuite.util.UserInfoClientUtil; import org.keycloak.testsuite.util.oauth.AccessTokenResponse; import org.keycloak.testsuite.util.oauth.AuthorizationEndpointResponse; import org.keycloak.testsuite.util.userprofile.UserProfileUtil; +import org.keycloak.util.JsonSerialization; +import com.fasterxml.jackson.core.JsonParser; import org.hamcrest.CoreMatchers; import org.junit.After; import org.junit.Before; @@ -198,6 +200,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { @Test public void testAddressMappingWithAdditionalMapper() { + //throws an exception if the json contains a duplicate claim + JsonSerialization.mapper.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION); // prepare test { UserResource userResource = findUserByUsernameId(adminClient.realm("test"), "test-user@localhost"); @@ -215,13 +219,13 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { ProtocolMapperRepresentation addressMapper = createAddressMapper(true, true, true, true); ProtocolMapperRepresentation addressTypeMapper = createClaimMapper("additional-address-field", - "address_type", - "address.type", - "String", - true, - true, - true, - false); + "address_type", + "address.type", + "String", + true, + true, + true, + false); ClientResource app = findClientResourceByClientId(adminClient.realm("test"), "test-app"); @@ -256,6 +260,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { } events.clear(); + JsonSerialization.mapper.disable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION); } @Test @@ -276,13 +281,13 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { userResource.update(user); ProtocolMapperRepresentation addressTypeMapper = createClaimMapper("additional-address-field", - "address_type", - "address.type", - "String", - true, - true, - true, - false); + "address_type", + "address.type", + "String", + true, + true, + true, + false); ProtocolMapperRepresentation addressMapper = createAddressMapper(true, true, true, true); @@ -331,13 +336,13 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { userResource.update(user); ProtocolMapperRepresentation addressTypeMapper = createClaimMapper("additional-address-field", - "address_type", - "address.type", - "String", - true, - true, - true, - false); + "address_type", + "address.type", + "String", + true, + true, + true, + false); ProtocolMapperRepresentation addressMapper = createAddressMapper(true, true, true, true); ClientResource app = findClientResourceByClientId(adminClient.realm("test"), "test-app"); @@ -525,7 +530,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { || model.getName().equals("hard-app") || model.getName().equals("test-script-mapper") || model.getName().equals("json-attribute-mapper") - ) { + ) { app.getProtocolMappers().delete(model.getId()); } } @@ -746,7 +751,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { for (ProtocolMapperRepresentation model : clientRepresentation.getProtocolMappers()) { if (model.getName().equals("empty") || model.getName().equals("null") - ) { + ) { app.getProtocolMappers().delete(model.getId()); } } @@ -1044,17 +1049,17 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { List realmRoleMappings = (List) roleMappings.get("realm"); List testAppMappings = (List) roleMappings.get(clientId); assertRolesString(realmRoleMappings, - "pref.admin", // from direct assignment to /roleRichGroup/level2group - "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.customer-user-premium", // from client role customer-admin-composite-role - realm role for test-app - "pref.realm-composite-role", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.sample-realm-role" // from realm role realm-composite-role + "pref.admin", // from direct assignment to /roleRichGroup/level2group + "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.customer-user-premium", // from client role customer-admin-composite-role - realm role for test-app + "pref.realm-composite-role", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.sample-realm-role" // from realm role realm-composite-role ); assertRolesString(testAppMappings, - "ta.customer-user", // from direct assignment to /roleRichGroup/level2group - "ta.customer-admin-composite-role", // from direct assignment to /roleRichGroup/level2group - "ta.customer-admin", // from client role customer-admin-composite-role - client role for test-app - "ta.sample-client-role" // from realm role realm-composite-role - client role for test-app + "ta.customer-user", // from direct assignment to /roleRichGroup/level2group + "ta.customer-admin-composite-role", // from direct assignment to /roleRichGroup/level2group + "ta.customer-admin", // from client role customer-admin-composite-role - client role for test-app + "ta.sample-client-role" // from realm role realm-composite-role - client role for test-app ); // Revert @@ -1089,11 +1094,11 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { List realmRoleMappings = (List) roleMappings.get("realm"); List testAppAuthzMappings = (List) roleMappings.get(clientId); assertRolesString(realmRoleMappings, - "pref.admin", // from direct assignment to /roleRichGroup/level2group - "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.customer-user-premium", // from client role customer-admin-composite-role - realm role for test-app - "pref.realm-composite-role", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.sample-realm-role" // from realm role realm-composite-role + "pref.admin", // from direct assignment to /roleRichGroup/level2group + "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.customer-user-premium", // from client role customer-admin-composite-role - realm role for test-app + "pref.realm-composite-role", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.sample-realm-role" // from realm role realm-composite-role ); assertNull(testAppAuthzMappings); // There is no client role defined for test-app-authz @@ -1122,12 +1127,12 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { List realmRoleMappings = (List) roleMappings.get("realm"); List testAppScopeMappings = (List) roleMappings.get(clientId); assertRolesString(realmRoleMappings, - "pref.admin", // from direct assignment to /roleRichGroup/level2group - "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.customer-user-premium" + "pref.admin", // from direct assignment to /roleRichGroup/level2group + "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.customer-user-premium" ); assertRolesString(testAppScopeMappings, - "test-app-allowed-by-scope", // from direct assignment to roleRichUser, present as scope allows it + "test-app-allowed-by-scope", // from direct assignment to roleRichUser, present as scope allows it "test-app-disallowed-by-scope" ); @@ -1156,15 +1161,15 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { List realmRoleMappings = (List) roleMappings.get("realm"); List testAppScopeMappings = (List) roleMappings.get(clientId); assertRolesString(realmRoleMappings, - "pref.admin", // from direct assignment to /roleRichGroup/level2group - "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup - "pref.customer-user-premium" + "pref.admin", // from direct assignment to /roleRichGroup/level2group + "pref.user", // from parent group of /roleRichGroup/level2group, i.e. from /roleRichGroup + "pref.customer-user-premium" ); assertRolesString(testAppScopeMappings, - "test-app-allowed-by-scope", // from direct assignment to roleRichUser, present as scope allows it - "test-app-disallowed-by-scope", // from direct assignment to /roleRichGroup/level2group, present as scope allows it - "customer-admin-composite-role", // from the other application - "customer-admin" + "test-app-allowed-by-scope", // from direct assignment to roleRichUser, present as scope allows it + "test-app-disallowed-by-scope", // from direct assignment to /roleRichGroup/level2group, present as scope allows it + "customer-admin-composite-role", // from the other application + "customer-admin" ); // Revert diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCScopeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCScopeTest.java index c28bd0d201a..63389c1ea52 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCScopeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCScopeTest.java @@ -173,7 +173,7 @@ public class OIDCScopeTest extends AbstractOIDCScopeTest { } } - + @Test public void testBuiltinOptionalScopes() throws Exception { // Login. Assert that just 'profile' and 'email' data are there. 'Address' and 'phone' not