libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Only remove attributes with empty values when updating user profile

Browse source 
Closes #27797

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2024-03-12 17:25:45 -03:00 committed by Marek Posolda
parent e22148043b
commit 9ad447390a
4 changed files with 65 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java
View file

@ -196,14 +196,22 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
@Override
public void removeAttribute(String name) {
List<UserAttributeEntity> toRemove = new ArrayList<>();
List<UserAttributeEntity> 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<UserEntity> {
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

9
server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java
View file

@ -119,8 +119,7 @@ public final class DefaultUserProfile implements UserProfile {
String name = attribute.getKey();
List<String> currentValue = user.getAttributeStream(name)
.filter(Objects::nonNull).collect(Collectors.toList());
List<String> updatedValue = attribute.getValue().stream()
.filter(StringUtil::isNotBlank).collect(Collectors.toList());
List<String> 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);

5
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java
View file

@ -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)));

42
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java
View file

@ -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<String, Object> 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);