Do not store empty attributes when updating user profile

Closes #22960
This commit is contained in:
Pedro Igor 2023-09-04 20:54:13 -03:00
parent 05afdfda6c
commit 04dd9afc5e
3 changed files with 73 additions and 13 deletions

View file

@ -32,6 +32,7 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelException; import org.keycloak.models.ModelException;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.storage.ReadOnlyException; import org.keycloak.storage.ReadOnlyException;
import org.keycloak.utils.StringUtil;
/** /**
* <p>The default implementation for {@link UserProfile}. Should be reused as much as possible by the different implementations * <p>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 { try {
for (Map.Entry<String, List<String>> attribute : attributes.getWritable().entrySet()) { for (Map.Entry<String, List<String>> attribute : attributes.getWritable().entrySet()) {
String name = attribute.getKey(); String name = attribute.getKey();
List<String> currentValue = user.getAttributeStream(name).filter(Objects::nonNull).collect(Collectors.toList()); List<String> currentValue = user.getAttributeStream(name)
List<String> updatedValue = attribute.getValue().stream().filter(Objects::nonNull).collect(Collectors.toList()); .filter(Objects::nonNull).collect(Collectors.toList());
List<String> updatedValue = attribute.getValue().stream()
.filter(StringUtil::isNotBlank).collect(Collectors.toList());
if (!CollectionUtil.collectionEquals(currentValue, updatedValue)) { if (CollectionUtil.collectionEquals(currentValue, updatedValue)) {
if (!removeAttributes && updatedValue.isEmpty()) { continue;
continue; }
}
user.setAttribute(name, updatedValue); boolean ignoreEmptyValue = !removeAttributes && updatedValue.isEmpty();
if (UserModel.EMAIL.equals(name) && metadata.getContext().isResetEmailVerified()) { if (isCustomAttribute(name) && ignoreEmptyValue) {
user.setEmailVerified(false); continue;
} }
for (AttributeChangeListener listener : changeListener) { user.setAttribute(name, updatedValue);
listener.onChange(name, user, currentValue);
} 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; return user;
} }
private boolean isCustomAttribute(String name) {
return !getAttributes().isRootAttribute(name);
}
@Override @Override
public Attributes getAttributes() { public Attributes getAttributes() {
return attributes; return attributes;

View file

@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.forms.VerifyProfileTest.PERMISSIONS_ALL; 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)); assertThat(StringUtils.isEmpty(user.getLastName()), is(true));
Assert.assertEquals("new@email.com", user.getEmail()); Assert.assertEquals("new@email.com", user.getEmail());
Assert.assertEquals(USERNAME1, user.getUsername()); Assert.assertEquals(USERNAME1, user.getUsername());
assertNull(user.getLastName());
} }
@Test @Test

View file

@ -1584,4 +1584,51 @@ public class UserProfileTest extends AbstractUserProfileTest {
assertEquals("new-email@test.com", userAttributes.getFirstValue(UserModel.EMAIL)); assertEquals("new-email@test.com", userAttributes.getFirstValue(UserModel.EMAIL));
assertNull(userAttributes.getFirstValue("test-attribute")); 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<String, List<String>> 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<String, List<String>> 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));
}
} }