diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index c6ef656f92..2e82897741 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -196,14 +196,22 @@ public class UserAdapter implements UserModel, JpaModel { @Override public void removeAttribute(String name) { - List toRemove = new ArrayList<>(); + List customAttributesToRemove = new ArrayList<>(); for (UserAttributeEntity attr : user.getAttributes()) { if (attr.getName().equals(name)) { - toRemove.add(attr); + customAttributesToRemove.add(attr); } } - if (toRemove.isEmpty()) { + if (customAttributesToRemove.isEmpty()) { + // make sure root user attributes are set to null + if (UserModel.FIRST_NAME.equals(name)) { + setFirstName(null); + } else if (UserModel.LAST_NAME.equals(name)) { + setLastName(null); + } else if (UserModel.EMAIL.equals(name)) { + setEmail(null); + } return; } @@ -213,7 +221,7 @@ public class UserAdapter implements UserModel, JpaModel { query.setParameter("userId", user.getId()); query.executeUpdate(); // KEYCLOAK-3494 : Also remove attributes from local user entity - user.getAttributes().removeAll(toRemove); + user.getAttributes().removeAll(customAttributesToRemove); } @Override 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 240f849901..420f817a10 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 @@ -119,8 +119,7 @@ public final class DefaultUserProfile implements UserProfile { String name = attribute.getKey(); List currentValue = user.getAttributeStream(name) .filter(Objects::nonNull).collect(Collectors.toList()); - List updatedValue = attribute.getValue().stream() - .filter(StringUtil::isNotBlank).collect(Collectors.toList()); + List updatedValue = attribute.getValue(); if (CollectionUtil.collectionEquals(currentValue, updatedValue)) { continue; @@ -132,7 +131,11 @@ public final class DefaultUserProfile implements UserProfile { continue; } - user.setAttribute(name, updatedValue); + if (updatedValue.stream().allMatch(StringUtil::isBlank)) { + user.removeAttribute(name); + } else { + user.setAttribute(name, updatedValue.stream().filter(StringUtil::isNotBlank).collect(Collectors.toList())); + } if (UserModel.EMAIL.equals(name) && metadata.getContext().isResetEmailVerified()) { user.setEmailVerified(false); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java index 1841d9f5fb..b8202b1d66 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java @@ -59,6 +59,7 @@ import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginUpdateProfileEditUsernameAllowedPage; import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.userprofile.UserProfileContext; +import org.keycloak.utils.StringUtil; import org.openqa.selenium.htmlunit.HtmlUnitDriver; /** @@ -513,6 +514,10 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe for (String value : values) { updateProfilePage.clickRemoveAttributeValue(attribute + "-0"); } + // make sure the last attribute is set with a value + if (StringUtil.isBlank(updateProfilePage.getAttribute(attribute + "-0"))) { + updateProfilePage.setAttribute(attribute + "-0", values.get(values.size() - 1)); + } updateProfilePage.update("f", "l", "e@keycloak.org"); userRep = ActionUtil.findUserWithAdminClient(adminClient, "john-doh@localhost"); assertThat(userRep.getAttributes().get(attribute), Matchers.containsInAnyOrder(values.get(values.size() - 1))); 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 d19d74e528..574155bcef 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 @@ -235,6 +235,48 @@ public class UserProfileTest extends AbstractUserProfileTest { profile.validate(); } + @Test + public void testEmptyAttributeRemoved() { + getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testEmptyAttributeRemoved); + } + + private static void testEmptyAttributeRemoved(KeycloakSession session) { + Map attributes = new HashMap<>(); + + attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId()); + attributes.put(UserModel.FIRST_NAME, "John"); + attributes.put(UserModel.LAST_NAME, "Doe"); + attributes.put(UserModel.EMAIL, org.keycloak.models.utils.KeycloakModelUtils.generateId() + "@keycloak.org"); + attributes.put("address", "foo"); + + UserProfileProvider provider = getUserProfileProvider(session); + UPConfig config = UPConfigUtils.parseSystemDefaultConfig(); + config.addOrReplaceAttribute(new UPAttribute("address", new UPAttributePermissions(Set.of(), Set.of(ROLE_USER)))); + provider.setConfiguration(config); + + UserProfile profile = provider.create(UserProfileContext.USER_API, attributes); + UserModel user = profile.create(); + + // attribute explicitly set with an empty value so we assume it should be removed regardless the `removeAttributes` parameter being set to false + attributes.put("address", ""); + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes, user); + profile.update(false); + + assertNull(user.getFirstAttribute("address")); + + attributes.put("address", "bar"); + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes, user); + profile.update(); + assertEquals("bar", user.getFirstAttribute("address")); + + // attribute not provided so we assume there is no intention to remove the attribute + attributes.remove("address"); + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes, user); + profile.update(false); + + assertNotNull(user.getFirstAttribute("address")); + } + @Test public void testResolveProfile() { getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testResolveProfile);