From e91a0afca2f859cc56b294ac86915de70f43af3a Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Mon, 16 Oct 2023 08:34:07 -0300 Subject: [PATCH] The username in account is required and don't change when email as username is enabled Closes #23976 --- .../userprofile/DefaultAttributes.java | 3 +- .../AbstractUserProfileProvider.java | 11 +--- .../userprofile/LegacyAttributes.java | 11 ++-- .../account/AccountRestServiceTest.java | 9 ++-- ...AccountRestServiceWithUserProfileTest.java | 2 +- .../keycloak/testsuite/admin/UserTest.java | 54 ++++++++++++++++--- 6 files changed, 64 insertions(+), 26 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java index 07c1df8625..699f6ca993 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultAttributes.java @@ -183,9 +183,8 @@ public class DefaultAttributes extends HashMap> implements RealmModel realm = session.getContext().getRealm(); if (UserModel.USERNAME.equals(name) - && UserProfileContext.USER_API.equals(context) && realm.isRegistrationEmailAsUsername()) { - continue; + continue; } if (metadata == null || !metadata.canEdit(createAttributeContext(metadata))) { diff --git a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java index 065b7a09b9..c58c3234e9 100644 --- a/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java +++ b/services/src/main/java/org/keycloak/userprofile/AbstractUserProfileProvider.java @@ -82,15 +82,8 @@ public abstract class AbstractUserProfileProvider return !realm.isRegistrationEmailAsUsername(); } - if (USER_API.equals(c.getContext())) { - if (realm.isRegistrationEmailAsUsername()) { - return false; - } - - if (isNewUser(c)) { - // when creating a user the username is always editable - return true; - } + if (realm.isRegistrationEmailAsUsername()) { + return false; } return realm.isEditUsernameAllowed(); diff --git a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java index 192ae41254..c4556e6e3b 100644 --- a/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java +++ b/services/src/main/java/org/keycloak/userprofile/LegacyAttributes.java @@ -58,10 +58,8 @@ public class LegacyAttributes extends DefaultAttributes { if (UserProfileContext.IDP_REVIEW.equals(context)) { return false; } - if (UserProfileContext.USER_API.equals(context)) { - if (realm.isRegistrationEmailAsUsername()) { - return false; - } + if (realm.isRegistrationEmailAsUsername()) { + return true; } return !realm.isEditUsernameAllowed(); } @@ -94,9 +92,14 @@ public class LegacyAttributes extends DefaultAttributes { @Override public Map> getWritable() { Map> attributes = new HashMap<>(this); + RealmModel realm = session.getContext().getRealm(); for (String name : nameSet()) { if (isReadOnly(name)) { + if (UserModel.USERNAME.equals(name) + && realm.isRegistrationEmailAsUsername()) { + continue; + } attributes.remove(name); } } 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 20c80b43d6..01d4767cae 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 @@ -147,8 +147,9 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { user = getUser(); if (isDeclarativeUserProfile()) { assertNotNull(user.getUserProfileMetadata()); - // username is read-only and is the same as email, but email is writable - assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); + // username is read-only, not required, and is the same as email + // but email is writable + assertUserProfileAttributeMetadata(user, "username", "${username}", false, true); assertUserProfileAttributeMetadata(user, "email", "${email}", true, false); } user.setUsername("should-be-the-email"); @@ -164,7 +165,7 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { if (isDeclarativeUserProfile()) { assertNotNull(user.getUserProfileMetadata()); // username is read-only and is the same as email, but email is read-only - assertUserProfileAttributeMetadata(user, "username", "${username}", true, true); + assertUserProfileAttributeMetadata(user, "username", "${username}", false, true); assertUserProfileAttributeMetadata(user, "email", "${email}", true, true); } user.setUsername("should-be-the-email"); @@ -189,8 +190,8 @@ public class AccountRestServiceTest extends AbstractRestServiceTest { user = getUser(); user.setEmail("should-not-change@keycloak.org"); user = updateAndGet(user); - assertEquals("different-than-email", user.getUsername()); assertEquals("user@keycloak.org", user.getEmail()); + assertEquals(user.getEmail(), user.getUsername()); } finally { realmRep.setRegistrationEmailAsUsername(registrationEmailAsUsername); realmRep.setEditUsernameAllowed(editUsernameAllowed); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java index f5c0a063f7..ba7d5bc8cf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountRestServiceWithUserProfileTest.java @@ -102,7 +102,7 @@ public class AccountRestServiceWithUserProfileTest extends AccountRestServiceTes @Test @Override public void testEditUsernameAllowed() throws IOException { - + super.testEditUsernameAllowed(); setUserProfileConfiguration(UP_CONFIG_FOR_METADATA); 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 64f457387d..eab2d73334 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 @@ -596,14 +596,59 @@ public class UserTest extends AbstractAdminTest { @Test public void createUserWithEmailAsUsername() { - switchRegistrationEmailAsUsername(true); + RealmRepresentation realmRep = realm.toRepresentation(); + Boolean registrationEmailAsUsername = realmRep.isRegistrationEmailAsUsername(); + Boolean editUsernameAllowed = realmRep.isEditUsernameAllowed(); + getCleanup().addCleanup(() -> { + realmRep.setRegistrationEmailAsUsername(registrationEmailAsUsername); + realm.update(realmRep); + }); + getCleanup().addCleanup(() -> { + realmRep.setEditUsernameAllowed(editUsernameAllowed); + realm.update(realmRep); + }); + switchRegistrationEmailAsUsername(true); + switchEditUsernameAllowedOn(false); String id = createUser(); UserResource user = realm.users().get(id); UserRepresentation userRep = user.toRepresentation(); - assertEquals("user1@localhost", userRep.getUsername()); + assertEquals("user1@localhost", userRep.getEmail()); + assertEquals(userRep.getEmail(), userRep.getUsername()); + deleteUser(id); + + switchRegistrationEmailAsUsername(true); + switchEditUsernameAllowedOn(true); + id = createUser(); + user = realm.users().get(id); + userRep = user.toRepresentation(); + assertEquals("user1@localhost", userRep.getEmail()); + assertEquals(userRep.getEmail(), userRep.getUsername()); + deleteUser(id); switchRegistrationEmailAsUsername(false); + switchEditUsernameAllowedOn(true); + id = createUser(); + user = realm.users().get(id); + userRep = user.toRepresentation(); + assertEquals("user1", userRep.getUsername()); + assertEquals("user1@localhost", userRep.getEmail()); + deleteUser(id); + + switchRegistrationEmailAsUsername(false); + switchEditUsernameAllowedOn(false); + id = createUser(); + user = realm.users().get(id); + userRep = user.toRepresentation(); + assertEquals("user1", userRep.getUsername()); + assertEquals("user1@localhost", userRep.getEmail()); + } + + private void deleteUser(String id) { + try (Response response = realm.users().delete(id)) { + assertEquals(204, response.getStatus()); + } + assertAdminEvents.assertEvent(realmId, OperationType.DELETE, AdminEventPaths.userResourcePath(id), ResourceType.USER); } @Test @@ -1343,10 +1388,7 @@ public class UserTest extends AbstractAdminTest { @Test public void delete() { String userId = createUser(); - try (Response response = realm.users().delete(userId)) { - assertEquals(204, response.getStatus()); - } - assertAdminEvents.assertEvent(realmId, OperationType.DELETE, AdminEventPaths.userResourcePath(userId), ResourceType.USER); + deleteUser(userId); } @Test