diff --git a/CHANGELOG.md b/CHANGELOG.md index 60441a565554..5384fb6742bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ Full Keycloak upstream jira issue can be shown if filtered by Fix version. Our Keycloak version is working well with PostgreSQL database. For using other SQL databases, text field in database need to be evaluated. +## [Unreleased] +### Fixed +- Handling of changed profile information during sign-up when review profile is set to "missing" or T&C is requested + ## [22.0.11-1.11] - 2024-11-19 ### Added diff --git a/pom.xml b/pom.xml index dfffb3bf0817..8fab6f57d8ab 100644 --- a/pom.xml +++ b/pom.xml @@ -47,7 +47,7 @@ 3.2.12.Final 3.2.12.Final - ${project.version}-1.11 + ${project.version}-1.12rc1 ${timestamp} diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java index 1473e601b603..e344671ff43b 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java @@ -100,12 +100,7 @@ protected boolean requiresUpdateProfilePage( AuthenticatorConfigModel authentica return true; } - String updateProfileFirstLogin; - if (authenticatorConfig == null || !authenticatorConfig.getConfig().containsKey(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN)) { - updateProfileFirstLogin = IdentityProviderRepresentation.UPFLM_MISSING; - } else { - updateProfileFirstLogin = authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN); - } + String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(authenticatorConfig); if(IdentityProviderRepresentation.UPFLM_MISSING.equals(updateProfileFirstLogin)) { try { @@ -120,6 +115,14 @@ protected boolean requiresUpdateProfilePage( AuthenticatorConfigModel authentica } } + private String calculateUpdateProfileFirstLogin(AuthenticatorConfigModel authenticatorConfig) { + if (authenticatorConfig == null || !authenticatorConfig.getConfig().containsKey(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN)) { + return IdentityProviderRepresentation.UPFLM_MISSING; + } else { + return authenticatorConfig.getConfig().get(IdpReviewProfileAuthenticatorFactory.UPDATE_PROFILE_ON_FIRST_LOGIN); + } + } + @Override protected void actionImpl(AuthenticationFlowContext context, SerializedBrokeredIdentityContext userCtx, BrokeredIdentityContext brokerContext) { EventBuilder event = context.getEvent(); @@ -190,6 +193,39 @@ public String getServiceAccountClientLink() { attributes.remove(TERMS_FIELD); attributes.put(TermsAndConditions.USER_ATTRIBUTE, Arrays.asList(Integer.toString(Time.currentTime()))); } + //for security reason keep old values equal to broken data + String updateProfileFirstLogin = calculateUpdateProfileFirstLogin(context.getAuthenticatorConfig()); + if (IdentityProviderRepresentation.UPFLM_MISSING.equals(updateProfileFirstLogin)) { + if (userCtx.getUsername() != null) { + attributes.put("username", Stream.of(userCtx.getUsername()).toList()); + } + if (userCtx.getEmail() != null) { + attributes.put("email", Stream.of(userCtx.getEmail()).toList()); + } + if (userCtx.getFirstName() != null) { + attributes.put("firstName", Stream.of(userCtx.getFirstName()).toList()); + } + if (userCtx.getLastName() != null) { + attributes.put("lastName", Stream.of(userCtx.getLastName()).toList()); + } + } else if (IdentityProviderRepresentation.UPFLM_OFF.equals(updateProfileFirstLogin)) { + attributes.put("username", Stream.of(userCtx.getUsername()).toList()); + if (userCtx.getEmail() != null) { + attributes.put("email", Stream.of(userCtx.getEmail()).toList()); + } else { + attributes.remove("email"); + } + if (userCtx.getFirstName() != null) { + attributes.put("firstName", Stream.of(userCtx.getFirstName()).toList()); + } else { + attributes.remove("firstName"); + } + if (userCtx.getLastName() != null) { + attributes.put("lastName", Stream.of(userCtx.getLastName()).toList()); + } else { + attributes.remove("lastName"); + } + } UserProfile profile = profileProvider.create(UserProfileContext.IDP_REVIEW, attributes, updatedProfile, formData.containsKey(TERMS_FIELD)); try { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java index 057ee73f835f..5d0b1ee50e11 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java @@ -97,7 +97,7 @@ public String getDepartment() { } public boolean isDepartmentEnabled() { - return departmentInput.getAttribute("readOnly") == null; + return departmentInput.isEnabled(); } public boolean isCurrent() { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/VerifyProfilePage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/VerifyProfilePage.java index ac3cb0566e28..e1483ea8e6cb 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/VerifyProfilePage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/VerifyProfilePage.java @@ -121,7 +121,7 @@ public String getDepartment() { } public boolean isDepartmentEnabled() { - return departmentInput.getAttribute("readOnly") == null; + return departmentInput.isEnabled(); } public boolean isUsernamePresent() { @@ -142,7 +142,7 @@ public boolean isEmailPresent() { public boolean isUsernameEnabled() { try { - return driver.findElement(By.id("username")).getAttribute("readOnly") == null; + return driver.findElement(By.id("username")).isEnabled(); } catch (NoSuchElementException nse) { return false; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java index f4b51cae35c2..2ec25d953991 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java @@ -104,7 +104,7 @@ public void testErrorExistingUserWithUpdateProfile() { waitForPage(driver, "account already exists", false); assertTrue(idpConfirmLinkPage.isCurrent()); - assertEquals("User with username consumer already exists. How do you want to continue?", idpConfirmLinkPage.getMessage()); + assertEquals("User with email user@localhost.com already exists. How do you want to continue?", idpConfirmLinkPage.getMessage()); } @@ -870,6 +870,8 @@ public void testVerifyEmailRequiredActionWhenChangingEmailDuringFirstLogin() { realm.update(realmRep); + updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); + IdentityProviderRepresentation idpRep = identityProviderResource.toRepresentation(); idpRep.setTrustEmail(true); @@ -1088,7 +1090,7 @@ public void testEventsOnUpdateProfileNoEmailChange() { @Test public void testEventsOnUpdateProfileWithEmailChange() { - updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin); + updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); createUser(bc.providerRealmName(), "no-first-name", "password", null, "LastName", "no-first-name@localhost.com"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java index a50f683ef566..1d0cb06b5c5a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginTest.java @@ -269,7 +269,7 @@ public void testLoginWithDifferentBrokerWhenUpdatingProfile() { @Test public void testEditUsername() { - updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin); + updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); createUser(bc.providerRealmName(), "no-first-name", "password", null, "LastName", "no-first-name@localhost.com"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java index 4668f3336d47..a6e32e4a11cf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlIdPInitiatedSsoTest.java @@ -1,5 +1,6 @@ package org.keycloak.testsuite.broker; +import org.keycloak.admin.client.resource.AuthenticationManagementResource; import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.admin.client.resource.IdentityProviderResource; import org.keycloak.admin.client.resource.UsersResource; @@ -13,7 +14,9 @@ import org.keycloak.dom.saml.v2.assertion.NameIDType; import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; import org.keycloak.dom.saml.v2.protocol.ResponseType; +import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.protocol.saml.SamlPrincipalType; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.FederatedIdentityRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; @@ -39,6 +42,7 @@ import java.io.InputStream; import java.net.URI; import java.util.*; +import java.util.function.BiConsumer; import java.util.stream.Collectors; import jakarta.ws.rs.core.Response; import org.jboss.arquillian.graphene.page.Page; @@ -115,6 +119,8 @@ public void initRealmUrls() { urlRealmProvider = getAuthRoot() + "/auth/realms/" + REALM_PROV_NAME; urlRealmConsumer = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME; urlRealmConsumer2 = getAuthRoot() + "/auth/realms/" + REALM_CONS_NAME + "-2"; + + updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin); } @Before @@ -146,6 +152,14 @@ public void addTestRealms(List testRealms) { testRealms.add(loadFromClasspath("kc3731-broker-realm.json", p)); } + private void updateExecutions(BiConsumer< AuthenticationExecutionInfoRepresentation, AuthenticationManagementResource > action) { + AuthenticationManagementResource flows = adminClient.realm(REALM_PROV_NAME).flows(); + + for (AuthenticationExecutionInfoRepresentation execution : flows.getExecutions(DefaultAuthenticationFlows.FIRST_BROKER_LOGIN_FLOW)) { + action.accept(execution, flows); + } + } + @Test public void testProviderIdpInitiatedLogin() throws Exception { driver.navigate().to(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")); @@ -291,6 +305,7 @@ public void testConsumerIdpInitiatedLoginToApp() throws Exception { @Test public void testTwoConsequentIdpInitiatedLogins() throws Exception { + SAMLDocumentHolder samlResponse = new SamlClientBuilder() .navigateTo(getSamlIdpInitiatedUrl(REALM_PROV_NAME, "samlbroker")) // Login in provider realm diff --git a/themes/src/main/resources/theme/base/login/login-update-profile.ftl b/themes/src/main/resources/theme/base/login/login-update-profile.ftl index e5ceddf9c774..c97d34a84178 100755 --- a/themes/src/main/resources/theme/base/login/login-update-profile.ftl +++ b/themes/src/main/resources/theme/base/login/login-update-profile.ftl @@ -12,7 +12,7 @@
readonly + class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled aria-invalid="<#if messagesPerField.existsError('username')>true" /> @@ -31,7 +31,7 @@
readonly + class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled aria-invalid="<#if messagesPerField.existsError('email')>true" /> @@ -50,7 +50,7 @@
readonly + class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled aria-invalid="<#if messagesPerField.existsError('firstName')>true" /> @@ -68,7 +68,7 @@
readonly + class="${properties.kcInputClass!}" <#if isUpdateProfile?has_content && !isUpdateProfile>disabled aria-invalid="<#if messagesPerField.existsError('lastName')>true" /> diff --git a/themes/src/main/resources/theme/base/login/user-profile-commons.ftl b/themes/src/main/resources/theme/base/login/user-profile-commons.ftl index 34a0891365da..56db2368cddc 100644 --- a/themes/src/main/resources/theme/base/login/user-profile-commons.ftl +++ b/themes/src/main/resources/theme/base/login/user-profile-commons.ftl @@ -76,7 +76,7 @@ <#macro inputTag attribute> readOnly + <#if attribute.readOnly || (isUpdateProfile?has_content && !isUpdateProfile)>disabled <#if attribute.autocomplete??>autocomplete="${attribute.autocomplete}" <#if attribute.annotations.inputTypePlaceholder??>placeholder="${attribute.annotations.inputTypePlaceholder}" <#if attribute.annotations.inputTypePattern??>pattern="${attribute.annotations.inputTypePattern}" @@ -106,7 +106,7 @@ <#macro textareaTag attribute>