diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java index 67a9129cdf..53968dfa44 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java @@ -32,6 +32,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelException; import org.keycloak.models.UserModel; import org.keycloak.storage.ReadOnlyException; +import org.keycloak.utils.StringUtil; /** *

The default implementation for {@link UserProfile}. Should be reused as much as possible by the different implementations @@ -106,23 +107,29 @@ public final class DefaultUserProfile implements UserProfile { try { for (Map.Entry> attribute : attributes.getWritable().entrySet()) { String name = attribute.getKey(); - List currentValue = user.getAttributeStream(name).filter(Objects::nonNull).collect(Collectors.toList()); - List updatedValue = attribute.getValue().stream().filter(Objects::nonNull).collect(Collectors.toList()); + List currentValue = user.getAttributeStream(name) + .filter(Objects::nonNull).collect(Collectors.toList()); + List updatedValue = attribute.getValue().stream() + .filter(StringUtil::isNotBlank).collect(Collectors.toList()); - if (!CollectionUtil.collectionEquals(currentValue, updatedValue)) { - if (!removeAttributes && updatedValue.isEmpty()) { - continue; - } + if (CollectionUtil.collectionEquals(currentValue, updatedValue)) { + continue; + } - user.setAttribute(name, updatedValue); + boolean ignoreEmptyValue = !removeAttributes && updatedValue.isEmpty(); - if (UserModel.EMAIL.equals(name) && metadata.getContext().isResetEmailVerified()) { - user.setEmailVerified(false); - } + if (isCustomAttribute(name) && ignoreEmptyValue) { + continue; + } - for (AttributeChangeListener listener : changeListener) { - listener.onChange(name, user, currentValue); - } + user.setAttribute(name, updatedValue); + + if (UserModel.EMAIL.equals(name) && metadata.getContext().isResetEmailVerified()) { + user.setEmailVerified(false); + } + + for (AttributeChangeListener listener : changeListener) { + listener.onChange(name, user, currentValue); } } @@ -157,6 +164,10 @@ public final class DefaultUserProfile implements UserProfile { return user; } + private boolean isCustomAttribute(String name) { + return !getAttributes().isRootAttribute(name); + } + @Override public Attributes getAttributes() { return attributes; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileWithUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileWithUserProfileTest.java index 340eaba06b..8828d1dd2f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileWithUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileWithUserProfileTest.java @@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.keycloak.testsuite.forms.VerifyProfileTest.PERMISSIONS_ALL; @@ -298,6 +299,7 @@ public class RequiredActionUpdateProfileWithUserProfileTest extends RequiredActi assertThat(StringUtils.isEmpty(user.getLastName()), is(true)); Assert.assertEquals("new@email.com", user.getEmail()); Assert.assertEquals(USERNAME1, user.getUsername()); + assertNull(user.getLastName()); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java index 692e741f1c..7ffc0efe1d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java @@ -1584,4 +1584,51 @@ public class UserProfileTest extends AbstractUserProfileTest { assertEquals("new-email@test.com", userAttributes.getFirstValue(UserModel.EMAIL)); assertNull(userAttributes.getFirstValue("test-attribute")); } + + @Test + public void testRemoveEmptyRootAttribute() { + getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testRemoveEmptyRootAttribute); + } + + private static void testRemoveEmptyRootAttribute(KeycloakSession session) { + Map> attributes = new HashMap<>(); + + attributes.put(UserModel.USERNAME, List.of(org.keycloak.models.utils.KeycloakModelUtils.generateId())); + attributes.put(UserModel.EMAIL, List.of("")); + attributes.put(UserModel.FIRST_NAME, List.of("")); + attributes.put("test-attribute", List.of("")); + + UserProfileProvider provider = getDynamicUserProfileProvider(session); + + provider.setConfiguration("{\"attributes\": [" + + "{\"name\": \"test-attribute\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}}," + + "{\"name\": \"firstName\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}}," + + "{\"name\": \"lastName\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}}," + + "{\"name\": \"email\", \"permissions\": {\"edit\": [\"admin\", \"user\"]}}]}"); + + UserProfile profile = provider.create(UserProfileContext.USER_API, attributes); + UserModel user = profile.create(); + assertNull(user.getEmail()); + assertNull(user.getFirstName()); + assertNull(user.getLastName()); + + attributes.remove(UserModel.EMAIL); + attributes.put(UserModel.FIRST_NAME, List.of("myfname")); + profile = provider.create(UserProfileContext.USER_API, attributes); + Attributes upAttributes = profile.getAttributes(); + assertRemoveEmptyRootAttribute(attributes, user, upAttributes); + + profile = provider.create(UserProfileContext.USER_API, attributes, user); + profile.update(false); + upAttributes = profile.getAttributes(); + assertRemoveEmptyRootAttribute(attributes, user, upAttributes); + } + + private static void assertRemoveEmptyRootAttribute(Map> attributes, UserModel user, Attributes upAttributes) { + assertNull(upAttributes.getFirstValue(UserModel.LAST_NAME)); + assertNull(user.getLastName()); + assertNull(upAttributes.getFirstValue(UserModel.EMAIL)); + assertNull(user.getEmail()); + assertEquals(upAttributes.getFirstValue(UserModel.FIRST_NAME), attributes.get(UserModel.FIRST_NAME).get(0)); + } }