From dbd94292560d91eb57d29dd2f9fab0de4bd605df Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Fri, 6 Dec 2024 14:29:58 +0100 Subject: [PATCH] Incomplete registration form when edit email is disabled and email is set as username Closes #34876 Signed-off-by: Martin Kanis --- .../userprofile/DefaultAttributes.java | 19 ++- .../model/AbstractUserProfileBean.java | 1 - .../testsuite/pages/RegisterPage.java | 24 ++- .../user/profile/UIRealmResourceTest.java | 146 ++++++++++++++---- 4 files changed, 153 insertions(+), 37 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 9f6b409218d..6ff94bc5de9 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 @@ -94,6 +94,9 @@ public class DefaultAttributes extends HashMap> implements @Override public boolean isReadOnly(String name) { + if (isReadableOrWritableDuringRegistration(name)) { + return false; + } if (isReadOnlyFromMetadata(name) || isReadOnlyInternalAttribute(name)) { return true; } @@ -105,6 +108,14 @@ public class DefaultAttributes extends HashMap> implements return getMetadata(name) == null; } + private boolean isReadableOrWritableDuringRegistration(String name) { + if (context.equals(UserProfileContext.REGISTRATION) && isRequired(name)) { + // in context of registration, username or email (email as username) cannot be readonly otherwise registration is not possible + return UserModel.EMAIL.equals(name) || UserModel.USERNAME.equals(name); + } + return false; + } + private boolean isAllowEditUnmanagedAttribute() { UnmanagedAttributePolicy unmanagedAttributesPolicy = upConfig.getUnmanagedAttributePolicy(); @@ -284,10 +295,12 @@ public class DefaultAttributes extends HashMap> implements continue; } - AttributeContext attributeContext = createAttributeContext(metadata); + if (!isReadableOrWritableDuringRegistration(name)) { + AttributeContext attributeContext = createAttributeContext(metadata); - if (!metadata.canView(attributeContext) || !metadata.isSelected(attributeContext)) { - attributes.remove(name); + if (!metadata.canView(attributeContext) || !metadata.isSelected(attributeContext)) { + attributes.remove(name); + } } } diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/AbstractUserProfileBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AbstractUserProfileBean.java index 1fc4cb55f94..50d975c6083 100644 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/AbstractUserProfileBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AbstractUserProfileBean.java @@ -21,7 +21,6 @@ import org.keycloak.userprofile.AttributeValidatorMetadata; import org.keycloak.userprofile.Attributes; import org.keycloak.userprofile.UserProfile; import org.keycloak.userprofile.UserProfileProvider; - /** * Abstract base for Freemarker context bean providing information about user profile to render dynamic or crafted forms. * diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/RegisterPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/RegisterPage.java index 0437030a77d..ee97d4fb74e 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/RegisterPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/RegisterPage.java @@ -103,9 +103,11 @@ public class RegisterPage extends AbstractPage { lastNameInput.sendKeys(lastName); } - emailInput.clear(); - if (email != null) { - emailInput.sendKeys(email); + if (isEmailPresent()) { + emailInput.clear(); + if (email != null) { + emailInput.sendKeys(email); + } } usernameInput.clear(); @@ -244,6 +246,22 @@ public class RegisterPage extends AbstractPage { } } + public boolean isEmailPresent() { + try { + return driver.findElement(By.name("email")).isDisplayed(); + } catch (NoSuchElementException nse) { + return false; + } + } + + public boolean isUsernamePresent() { + try { + return driver.findElement(By.name("username")).isDisplayed(); + } catch (NoSuchElementException nse) { + return false; + } + } + public boolean isCurrent() { return isCurrent("Register"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UIRealmResourceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UIRealmResourceTest.java index f50e9229f9c..81970072e8e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UIRealmResourceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UIRealmResourceTest.java @@ -28,11 +28,13 @@ import java.io.IOException; import java.util.Map; import java.util.Set; import org.hamcrest.Matchers; +import org.jboss.arquillian.graphene.page.Page; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; +import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.KeycloakBuilder; import org.keycloak.admin.client.resource.BearerAuthFilter; @@ -53,6 +55,9 @@ import org.keycloak.representations.userprofile.config.UPAttributeRequired; import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.representations.userprofile.config.UPConfig.UnmanagedAttributePolicy; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; +import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.RegisterPage; import org.keycloak.testsuite.util.AssertAdminEvents; import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.userprofile.config.UPConfigUtils; @@ -77,6 +82,15 @@ public class UIRealmResourceTest extends AbstractTestRealmKeycloakTest { private static Keycloak keycloakAdminClientViewUsers; private static Keycloak keycloakAdminClientWithoutAdminRoles; + @Page + protected RegisterPage registerPage; + + @Page + protected AppPage appPage; + + @Rule + public AssertEvents events = new AssertEvents(this); + @Rule public AssertAdminEvents assertAdminEvents = new AssertAdminEvents(this); @@ -147,43 +161,115 @@ public class UIRealmResourceTest extends AbstractTestRealmKeycloakTest { @Test public void testUpdateUserProfileModification() throws IOException { RealmRepresentation rep = testRealm().toRepresentation(); + UPConfig upConfigOrig = testRealm().users().userProfile().getConfiguration(); + + try { + UPConfig upConfig = testRealm().users().userProfile().getConfiguration(); + upConfig.addOrReplaceAttribute(new UPAttribute("foo", + new UPAttributePermissions(Set.of(), Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN)))); + + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + AdminEventRepresentation adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + Assert.assertNotNull(adminEvent.getRepresentation()); + adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); + assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + + upConfig.getAttribute("foo").setDisplayName("Foo"); + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); + assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + + upConfig.getAttribute("foo").setPermissions(new UPAttributePermissions(Set.of(), Set.of(UPConfigUtils.ROLE_USER))); + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); + assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + + upConfig.getAttribute("foo").setRequired(new UPAttributeRequired(Set.of(UPConfigUtils.ROLE_ADMIN, UPConfigUtils.ROLE_USER), Set.of())); + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); + assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + + upConfig.getAttribute("foo").setValidations(Map.of("length", Map.of("min", "3", "max", "128"))); + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); + assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + + updateRealmExt(toUIRealmRepresentation(rep, upConfig)); + assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); + assertAdminEvents.assertEmpty(); + } finally { + updateRealmExt(toUIRealmRepresentation(rep, upConfigOrig)); + } + } + + @Test + public void testRegistrationFormWithNotReadableOrWritableRequiredEmail() throws IOException { + RealmRepresentation testRealm = testRealm().toRepresentation(); + testRealm.setRegistrationEmailAsUsername(true); + getCleanup().addCleanup(() -> { + testRealm.setRegistrationEmailAsUsername(false); + testRealm().update(testRealm); + }); + testRealm().update(testRealm); + + // set email as not readable or writable for a user UPConfig upConfig = testRealm().users().userProfile().getConfiguration(); - upConfig.addOrReplaceAttribute(new UPAttribute("foo", - new UPAttributePermissions(Set.of(), Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN)))); + upConfig.addOrReplaceAttribute(new UPAttribute("email", + new UPAttributePermissions(Set.of(UPConfigUtils.ROLE_ADMIN), Set.of(UPConfigUtils.ROLE_ADMIN)))); + updateRealmExt(toUIRealmRepresentation(testRealm, upConfig)); - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - AdminEventRepresentation adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - Assert.assertNotNull(adminEvent.getRepresentation()); - adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); - assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + // open the registration form + driver.navigate().to(oauth.getLoginFormUrl()); + loginPage.form().register(); + registerPage.assertCurrent(); - upConfig.getAttribute("foo").setDisplayName("Foo"); - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); - assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + Assert.assertTrue("Email is missing on the registration page.", registerPage.isEmailPresent()); - upConfig.getAttribute("foo").setPermissions(new UPAttributePermissions(Set.of(), Set.of(UPConfigUtils.ROLE_USER))); - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); - assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + registerPage.registerWithEmailAsUsername("Tom", "Brady", "tbrady@email.com", "password", "password"); - upConfig.getAttribute("foo").setRequired(new UPAttributeRequired(Set.of(UPConfigUtils.ROLE_ADMIN, UPConfigUtils.ROLE_USER), Set.of())); - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); - assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); - upConfig.getAttribute("foo").setValidations(Map.of("length", Map.of("min", "3", "max", "128"))); - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - adminEvent = assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, "ui-ext", ResourceType.USER_PROFILE); - assertEquals(upConfig, toUpConfig(adminEvent.getRepresentation())); + String userId = events.expectRegister("tbrady@email.com", "tbrady@email.com").assertEvent().getUserId(); + UserRepresentation user = testRealm().users().get(userId).toRepresentation(); + assertEquals("Tom", user.getFirstName()); + assertEquals("Brady", user.getLastName()); + } - updateRealmExt(toUIRealmRepresentation(rep, upConfig)); - assertAdminEvents.assertEvent(TEST_REALM_NAME, OperationType.UPDATE, Matchers.nullValue(String.class), ResourceType.REALM); - assertAdminEvents.assertEmpty(); + @Test + public void testRegistrationFormWithReadonlyUsernameAndEmail() throws IOException { + RealmRepresentation testRealm = testRealm().toRepresentation(); + + // set username and email as readonly for a user + UPConfig upConfig = testRealm().users().userProfile().getConfiguration(); + upConfig.addOrReplaceAttribute(new UPAttribute("username", + new UPAttributePermissions(Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN), Set.of(UPConfigUtils.ROLE_ADMIN)))); + upConfig.addOrReplaceAttribute(new UPAttribute("email", + new UPAttributePermissions(Set.of(UPConfigUtils.ROLE_USER, UPConfigUtils.ROLE_ADMIN), Set.of(UPConfigUtils.ROLE_ADMIN)))); + updateRealmExt(toUIRealmRepresentation(testRealm, upConfig)); + + // open the registration form + driver.navigate().to(oauth.getLoginFormUrl()); + loginPage.form().register(); + registerPage.assertCurrent(); + + Assert.assertTrue("Username is missing on the registration page.", registerPage.isUsernamePresent()); + Assert.assertFalse("Email should not be present on the registration page.", registerPage.isEmailPresent()); + + registerPage.register("Alice", "Wood", null, "awood", "password", "password"); + + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + + String userId = events.expectRegister("awood", null).removeDetail("email").assertEvent().getUserId(); + UserRepresentation user = testRealm().users().get(userId).toRepresentation(); + assertEquals("awood", user.getUsername()); + assertEquals("Alice", user.getFirstName()); + assertEquals("Wood", user.getLastName()); } @Test