From 48ab2b1688d8f2242b54d2d05c32026f5437e37e Mon Sep 17 00:00:00 2001 From: rmartinc Date: Fri, 8 Sep 2023 16:34:42 +0200 Subject: [PATCH] FullNameLDAPStoreMapper removes values for other attributes Closes https://github.com/keycloak/keycloak/issues/22526 --- .../ldap/mappers/FullNameLDAPStorageMapper.java | 4 ++-- .../ldap/LDAPProvidersFullNameMapperTest.java | 13 ++++++++++--- .../testsuite/federation/ldap/LDAPTestAsserts.java | 11 ++++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java index 98bb9afe4a..b7285d8291 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java @@ -142,7 +142,7 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper { @Override public void setAttribute(String name, List values) { - String valueToSet = (values != null && values.size() > 0) ? values.get(0) : null; + String valueToSet = (values != null && !values.isEmpty()) ? values.get(0) : null; if (UserModel.FIRST_NAME.equals(name)) { this.firstName = valueToSet; setFullNameToLDAPObject(); @@ -150,7 +150,7 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper { this.lastName = valueToSet; setFullNameToLDAPObject(); } - super.setSingleAttribute(name, valueToSet); + super.setAttribute(name, values); } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersFullNameMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersFullNameMapperTest.java index 030039cb87..d3128b4424 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersFullNameMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersFullNameMapperTest.java @@ -17,6 +17,8 @@ package org.keycloak.testsuite.federation.ldap; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; @@ -33,6 +35,7 @@ import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPTestUtils; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.stream.Collectors; @@ -93,10 +96,10 @@ public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest { ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(appRealm); LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel); - LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "fullname", "James", "Dee", "fullname@email.org", null, "4578"); + LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "fullname", "James", "Dee", "fullname@email.org", null, "4578", "9876"); // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName - LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578"); + LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578", "9876"); }); // Assert user will be changed in LDAP too @@ -107,6 +110,7 @@ public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest { UserModel fullnameUser = session.users().getUserByUsername(appRealm, "fullname"); fullnameUser.setFirstName("James2"); fullnameUser.setLastName("Dee2"); + fullnameUser.setAttribute("postal_code", Arrays.asList("1234", "2345", "3456")); }); // Assert changed user available in Keycloak @@ -115,7 +119,7 @@ public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest { RealmModel appRealm = ctx.getRealm(); // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName - LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James2", "Dee2", "fullname@email.org", "4578"); + LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James2", "Dee2", "fullname@email.org", "1234", "2345", "3456"); // Remove "fullnameUser" to assert he is removed from LDAP. UserModel fullnameUser = session.users().getUserByUsername(appRealm, "fullname"); @@ -145,6 +149,7 @@ public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest { fullnameUser.setAttribute("myAttribute", Collections.singletonList("test")); fullnameUser.setAttribute("myEmptyAttribute", new ArrayList<>()); fullnameUser.setAttribute("myNullAttribute", null); + fullnameUser.setAttribute("myAttrThreeValues", Arrays.asList("one", "two", "three")); }); // Assert changed user available in Keycloak @@ -159,6 +164,8 @@ public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest { assertThat(fullnameUser.getAttributeStream("myAttribute").collect(Collectors.toList()), contains("test")); assertThat(fullnameUser.getAttributeStream("myEmptyAttribute").collect(Collectors.toList()), is(empty())); assertThat(fullnameUser.getAttributeStream("myNullAttribute").collect(Collectors.toList()), is(empty())); + MatcherAssert.assertThat(Arrays.asList("one", "two", "three"), + Matchers.containsInAnyOrder(fullnameUser.getAttributeStream("myAttrThreeValues").toArray(String[]::new))); // Remove "fullnameUser" to assert he is removed from LDAP. session.users().removeUser(appRealm, fullnameUser); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestAsserts.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestAsserts.java index 3ffe719be9..faee7c26c2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestAsserts.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPTestAsserts.java @@ -17,6 +17,10 @@ package org.keycloak.testsuite.federation.ldap; +import java.util.Arrays; +import java.util.Collections; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Assert; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -31,19 +35,20 @@ import org.keycloak.storage.user.SynchronizationResult; */ public class LDAPTestAsserts { - public static UserModel assertUserImported(UserProvider userProvider, RealmModel realm, String username, String expectedFirstName, String expectedLastName, String expectedEmail, String expectedPostalCode) { + public static UserModel assertUserImported(UserProvider userProvider, RealmModel realm, String username, String expectedFirstName, String expectedLastName, String expectedEmail, String... expectedPostalCode) { UserModel user = userProvider.getUserByUsername(realm, username); assertLoaded(user, username, expectedFirstName, expectedLastName, expectedEmail, expectedPostalCode); return user; } - public static void assertLoaded(UserModel user, String username, String expectedFirstName, String expectedLastName, String expectedEmail, String expectedPostalCode) { + public static void assertLoaded(UserModel user, String username, String expectedFirstName, String expectedLastName, String expectedEmail, String... expectedPostalCode) { Assert.assertNotNull(user); Assert.assertEquals(expectedFirstName, user.getFirstName()); Assert.assertEquals(expectedLastName, user.getLastName()); Assert.assertEquals(expectedEmail, user.getEmail()); - Assert.assertEquals(expectedPostalCode, user.getFirstAttribute("postal_code")); + MatcherAssert.assertThat(expectedPostalCode == null? Collections.emptyList() : Arrays.asList(expectedPostalCode), + Matchers.containsInAnyOrder(user.getAttributeStream("postal_code").toArray(String[]::new))); }