From baac060eb1036f0702309854de9896f11ea3cfeb Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 8 Aug 2023 16:16:34 -0300 Subject: [PATCH] Fixing how e-mail attribute permissions are set for both USER_API and ACCOUNT contexts Closes #21751 --- .../AbstractUserProfileProvider.java | 17 +++++++++--- .../userprofile/LegacyAttributes.java | 9 +++++-- .../account/AccountRestServiceTest.java | 22 +++++++++++++++ .../keycloak/testsuite/admin/UserTest.java | 27 +++++++++++++++++++ 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java index b015ce1c54..423f721762 100644 --- a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java @@ -39,6 +39,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import org.keycloak.Config; import org.keycloak.common.Profile; +import org.keycloak.common.Profile.Feature; import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; @@ -119,13 +120,21 @@ public abstract class AbstractUserProfileProvider } private static boolean readEmailCondition(AttributeContext c) { - RealmModel realm = c.getSession().getContext().getRealm(); + UserProfileContext context = c.getContext(); - if (realm.isRegistrationEmailAsUsername() && !realm.isEditUsernameAllowed()) { - return REGISTRATION_PROFILE.equals(c.getContext()); + if (UPDATE_PROFILE.equals(context)) { + if (Profile.isFeatureEnabled(Feature.UPDATE_EMAIL)) { + return false; + } + + RealmModel realm = c.getSession().getContext().getRealm(); + + if (realm.isRegistrationEmailAsUsername() && !realm.isEditUsernameAllowed()) { + return false; + } } - return !Profile.isFeatureEnabled(Profile.Feature.UPDATE_EMAIL) || c.getContext() != UPDATE_PROFILE; + return true; } public static Pattern getRegexPatternString(String[] builtinReadOnlyAttributes) { diff --git a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java index e6514e5389..cc48000a81 100644 --- a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java +++ b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java @@ -48,7 +48,12 @@ public class LegacyAttributes extends DefaultAttributes { @Override protected boolean isIncludeAttributeIfNotProvided(AttributeMetadata metadata) { - // user api expects that built-in attributes are not updated if not provided when in legacy mode - return UserProfileContext.USER_API.equals(context) && !isRootAttribute(metadata.getName()); + if (UserModel.LOCALE.equals(metadata.getName())) { + // locale is an internal attribute and should be updated as a regular attribute + return false; + } + + // user api expects that attributes are not updated if not provided when in legacy mode + return UserProfileContext.USER_API.equals(context); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java index 4734cf4608..1d565f1367 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceTest.java @@ -421,6 +421,28 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { } + @Test + public void testEmailReadableWhenEditUsernameDisabled() throws IOException { + RealmRepresentation realmRep = testRealm().toRepresentation(); + Boolean emailAsUsername = realmRep.isRegistrationEmailAsUsername(); + Boolean editUsernameAllowed = realmRep.isEditUsernameAllowed(); + realmRep.setRegistrationEmailAsUsername(true); + realmRep.setEditUsernameAllowed(false); + testRealm().update(realmRep); + + try { + UserRepresentation user = getUser(); + String email = user.getEmail(); + assertNotNull(email); + user = updateAndGet(user); + assertEquals(email, user.getEmail()); + } finally { + realmRep.setRegistrationEmailAsUsername(emailAsUsername); + realmRep.setEditUsernameAllowed(editUsernameAllowed); + testRealm().update(realmRep); + } + } + @Test public void testUpdateProfileCannotChangeThroughAttributes() throws IOException { UserRepresentation user = getUser(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index e117cac25b..00524d07cf 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -106,6 +106,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; @@ -2331,6 +2332,32 @@ public class UserTest extends AbstractAdminTest { } } + @Test + public void testKeepRootAttributeWhenOtherAttributesAreSet() { + String random = UUID.randomUUID().toString(); + String userName = String.format("username-%s", random); + String email = String.format("my@mail-%s.com", random); + UserRepresentation user = new UserRepresentation(); + user.setUsername(userName); + user.setEmail(email); + String userId = createUser(user); + + UserRepresentation created = realm.users().get(userId).toRepresentation(); + assertThat(created.getEmail(), equalTo(email)); + assertThat(created.getUsername(), equalTo(userName)); + assertThat(created.getAttributes(), Matchers.nullValue()); + + UserRepresentation update = new UserRepresentation(); + update.setId(userId); + update.setAttributes(Map.of("phoneNumber", List.of("123"))); + updateUser(realm.users().get(userId), update); + + UserRepresentation updated = realm.users().get(userId).toRepresentation(); + assertThat(updated.getUsername(), equalTo(userName)); + assertThat(updated.getAttributes(), equalTo(Map.of("phoneNumber", List.of("123")))); + assertThat(updated.getEmail(), equalTo(email)); + } + @Test public void updateUserWithNewUsernameNotPossible() { String id = createUser();