diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties index 5337b35e858..970839313cb 100644 --- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties +++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties @@ -329,6 +329,7 @@ loginScreenCustomization=Login screen customization policiesConfigType=Configure via\: exportWarningTitle=Export with caution emailVerifiedHelp=Has the user's email been verified? +emailPendingVerification=Email pending verification duplicateFlow=Duplicate flow addExecution=Add execution noSearchResultsInstructions=Click on the search bar above to search again diff --git a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java index bb1a88af204..d7a83c53b32 100644 --- a/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java +++ b/server-spi-private/src/main/java/org/keycloak/userprofile/DefaultUserProfile.java @@ -30,18 +30,22 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.keycloak.common.util.CollectionUtil; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.idm.AbstractUserRepresentation; import org.keycloak.services.managers.BruteForceProtector; +import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.storage.ReadOnlyException; import org.keycloak.utils.StringUtil; @@ -128,9 +132,7 @@ public final class DefaultUserProfile implements UserProfile { continue; } - boolean ignoreEmptyValue = !removeAttributes && updatedValue.isEmpty(); - - if (isCustomAttribute(name) && ignoreEmptyValue) { + if (isIgnoreAttributeUpdate(removeAttributes, updatedValue, name)) { continue; } @@ -191,6 +193,23 @@ public final class DefaultUserProfile implements UserProfile { return user; } + private boolean isIgnoreAttributeUpdate(boolean removeAttributes, List updatedValue, String name) { + boolean ignoreEmptyValue = !removeAttributes && updatedValue.isEmpty(); + + if (isCustomAttribute(name) && ignoreEmptyValue) { + return true; + } + + if (UserModel.EMAIL.equals(name)) { + AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession(); + // do not set email when during an authentication flow if there is a pending update email action + Stream actions = Optional.ofNullable(user.getRequiredActionsStream()).orElse(Stream.empty()); + return authSession != null && actions.anyMatch(RequiredAction.UPDATE_EMAIL.name()::equals); + } + + return false; + } + private boolean isCustomAttribute(String name) { return !isRootAttribute(name); } diff --git a/server-spi/src/main/java/org/keycloak/models/UserModel.java b/server-spi/src/main/java/org/keycloak/models/UserModel.java index 5148838b370..0cac75263d9 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserModel.java @@ -35,6 +35,7 @@ public interface UserModel extends RoleMapperModel { String FIRST_NAME = "firstName"; String LAST_NAME = "lastName"; String EMAIL = "email"; + String EMAIL_PENDING = "kc.email.pending"; String EMAIL_VERIFIED = "emailVerified"; String LOCALE = "locale"; String ENABLED = "enabled"; diff --git a/services/src/main/java/org/keycloak/authentication/actiontoken/updateemail/UpdateEmailActionTokenHandler.java b/services/src/main/java/org/keycloak/authentication/actiontoken/updateemail/UpdateEmailActionTokenHandler.java index dc1147aa35d..572594cc406 100644 --- a/services/src/main/java/org/keycloak/authentication/actiontoken/updateemail/UpdateEmailActionTokenHandler.java +++ b/services/src/main/java/org/keycloak/authentication/actiontoken/updateemail/UpdateEmailActionTokenHandler.java @@ -86,7 +86,6 @@ public class UpdateEmailActionTokenHandler extends AbstractActionTokenHandler getRequiredActionsStream() { + return Stream.empty(); + } }; UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class); diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java index 0931ec345a3..48afbc1fbfd 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateEmail.java @@ -17,10 +17,13 @@ package org.keycloak.authentication.requiredactions; +import static org.keycloak.services.messages.Messages.EMAIL_VERIFICATION_PENDING; + import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; + import jakarta.ws.rs.core.MultivaluedHashMap; import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.UriInfo; @@ -49,7 +52,6 @@ import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; import org.keycloak.models.RequiredActionConfigModel; import org.keycloak.models.RequiredActionProviderModel; -import org.keycloak.models.SingleUseObjectProvider; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.models.utils.FormMessage; @@ -71,9 +73,12 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor public static final String CONFIG_VERIFY_EMAIL = "verifyEmail"; private static final String FORCE_EMAIL_VERIFICATION = "forceEmailVerification"; - private static final String PENDING_EMAIL_CACHE_KEY_PREFIX = "update-email-pending-"; public static boolean isEnabled(RealmModel realm) { + if (realm == null) { + return false; + } + if (!Profile.isFeatureEnabled(Profile.Feature.UPDATE_EMAIL)) { return false; } @@ -118,34 +123,51 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor @Override public void evaluateTriggers(RequiredActionContext context) { + UserModel user = context.getUser(); + if (user.getFirstAttribute(UserModel.EMAIL_PENDING) != null) { + user.addRequiredAction(RequiredAction.UPDATE_EMAIL); + return; + } + + Stream actions = user.getRequiredActionsStream(); + + if (actions.anyMatch(RequiredAction.UPDATE_EMAIL.name()::equals)) { + user.removeRequiredAction(RequiredAction.VERIFY_EMAIL); + } } @Override public void requiredActionChallenge(RequiredActionContext context) { if (isEnabled(context.getRealm())) { - KeycloakSession session = context.getSession(); + UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class); + UserModel user = context.getUser(); + UserProfile profile = profileProvider.create(UserProfileContext.UPDATE_EMAIL, user); // skip and clear UPDATE_EMAIL required action if email is readonly - UserProfileProvider profileProvider = context.getSession().getProvider(UserProfileProvider.class); - UserProfile profile = profileProvider.create(UserProfileContext.UPDATE_EMAIL, context.getUser()); if (profile.getAttributes().isReadOnly(UserModel.EMAIL)) { - context.getUser().removeRequiredAction(UserModel.RequiredAction.UPDATE_EMAIL); + user.removeRequiredAction(UserModel.RequiredAction.UPDATE_EMAIL); return; } - // Check if email verification is pending and show message for subsequent visits - String pendingEmail = getPendingEmailVerification(context); - if (pendingEmail != null) { - context.form().setInfo("emailVerificationPending", pendingEmail); - } + MultivaluedMap formData = new MultivaluedHashMap<>(context.getHttpRequest().getDecodedFormParameters()); + String newEmail = formData.getFirst(UserModel.EMAIL); - if (session.getAttributeOrDefault(FORCE_EMAIL_VERIFICATION, Boolean.FALSE)) { + if (newEmail != null) { + // Remove VERIFY_EMAIL to ensure UPDATE_EMAIL takes precedence when both realm verification and forced verification are enabled. + user.removeRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL); + setPendingEmailVerification(context, newEmail); sendEmailUpdateConfirmation(context, false); - return; - } + } else { + // Check if email verification is pending and show message for subsequent visits + String pendingEmail = getPendingEmailVerification(context); - context.challenge(context.form().createResponse(UserModel.RequiredAction.UPDATE_EMAIL)); + if (pendingEmail != null) { + context.form().setInfo(EMAIL_VERIFICATION_PENDING, pendingEmail); + } + + context.challenge(context.form().createResponse(UserModel.RequiredAction.UPDATE_EMAIL)); + } } } @@ -214,10 +236,7 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor } context.getEvent().success(); - // Set cache entry only if not already set - if (getPendingEmailVerification(context) == null) { - setPendingEmailVerification(context, newEmail); - } + setPendingEmailVerification(context, newEmail); LoginFormsProvider forms = context.form(); @@ -245,11 +264,12 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor } public static void updateEmailNow(EventBuilder event, UserModel user, UserProfile emailUpdateValidationResult) { - + user.removeRequiredAction(UserModel.RequiredAction.UPDATE_EMAIL); String oldEmail = user.getEmail(); String newEmail = emailUpdateValidationResult.getAttributes().getFirst(UserModel.EMAIL); event.event(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, newEmail); emailUpdateValidationResult.update(false, new EventAuditingAttributeChangeListener(emailUpdateValidationResult, event)); + user.removeAttribute(UserModel.EMAIL_PENDING); } @Override @@ -300,26 +320,17 @@ public class UpdateEmail implements RequiredActionProvider, RequiredActionFactor return Profile.isFeatureEnabled(Profile.Feature.UPDATE_EMAIL); } - private static String getPendingEmailCacheKey(RequiredActionContext context) { - return PENDING_EMAIL_CACHE_KEY_PREFIX + context.getUser().getId(); - } - - public static void setPendingEmailVerification(RequiredActionContext context, String email) { - SingleUseObjectProvider cache = context.getSession().singleUseObjects(); - // Use same expiration as verification link + small buffer - int linkValidityInSecs = context.getRealm().getActionTokenGeneratedByUserLifespan(UpdateEmailActionToken.TOKEN_TYPE); - long expirationSeconds = linkValidityInSecs + 300; - cache.put(getPendingEmailCacheKey(context), expirationSeconds, Map.of("email", email)); + private void setPendingEmailVerification(RequiredActionContext context, String email) { + UserModel user = context.getUser(); + user.setSingleAttribute(UserModel.EMAIL_PENDING, email); + user.setEmailVerified(false); } private String getPendingEmailVerification(RequiredActionContext context) { - SingleUseObjectProvider cache = context.getSession().singleUseObjects(); - Map pendingData = cache.get(getPendingEmailCacheKey(context)); - return pendingData != null ? pendingData.get("email") : null; + return context.getUser().getFirstAttribute(UserModel.EMAIL_PENDING); } private void clearPendingEmailVerification(RequiredActionContext context) { - SingleUseObjectProvider cache = context.getSession().singleUseObjects(); - cache.remove(getPendingEmailCacheKey(context)); + context.getUser().removeAttribute(UserModel.EMAIL_PENDING); } } diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java index ee459c56392..7232964217c 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateProfile.java @@ -80,20 +80,12 @@ public class UpdateProfile implements RequiredActionProvider, RequiredActionFact UserProfileProvider provider = context.getSession().getProvider(UserProfileProvider.class); UserProfile profile = provider.create(UserProfileContext.UPDATE_PROFILE, formData, user); - profile.update(false, new EventAuditingAttributeChangeListener(profile, event)); - if (isForceEmailVerification) { user.addRequiredAction(UserModel.RequiredAction.UPDATE_EMAIL); - - // Remove VERIFY_EMAIL to ensure UPDATE_EMAIL takes precedence when both realm verification and forced verification are enabled. - user.removeRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL); - - UpdateEmail.forceEmailVerification(context.getSession()); - - // Set cache entry so pending verification message shows on subsequent UPDATE_EMAIL visits - UpdateEmail.setPendingEmailVerification(context, newEmail); } + profile.update(false, new EventAuditingAttributeChangeListener(profile, event)); + context.success(); } catch (ValidationException pve) { List errors = Validation.getFormErrorsFromValidation(pve.getErrors()); diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java index 5e099b87f99..f43fd0ec707 100755 --- a/services/src/main/java/org/keycloak/services/messages/Messages.java +++ b/services/src/main/java/org/keycloak/services/messages/Messages.java @@ -94,6 +94,7 @@ public class Messages { public static final String RECAPTCHA_NOT_CONFIGURED = "recaptchaNotConfigured"; public static final String EMAIL_EXISTS = "emailExistsMessage"; + public static final String EMAIL_VERIFICATION_PENDING = "emailVerificationPending"; public static final String FEDERATED_IDENTITY_EXISTS = "federatedIdentityExistsMessage"; diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java index bfde60eaab6..777a1e5a617 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.regex.Pattern; @@ -92,7 +93,7 @@ public class DeclarativeUserProfileProviderFactory implements UserProfileProvide * There are the declarations for creating the built-in validations for read-only attributes. Regardless of the context where * user profiles are used. They are related to internal attributes with hard conditions on them in terms of management. */ - private static final String[] DEFAULT_READ_ONLY_ATTRIBUTES = { "KERBEROS_PRINCIPAL", "LDAP_ID", "LDAP_ENTRY_DN", "CREATED_TIMESTAMP", "createTimestamp", "modifyTimestamp", "userCertificate", "saml.persistent.name.id.for.*", "ENABLED", "EMAIL_VERIFIED", "disabledReason" }; + private static final String[] DEFAULT_READ_ONLY_ATTRIBUTES = { "KERBEROS_PRINCIPAL", "LDAP_ID", "LDAP_ENTRY_DN", "CREATED_TIMESTAMP", "createTimestamp", "modifyTimestamp", "userCertificate", "saml.persistent.name.id.for.*", "ENABLED", "EMAIL_VERIFIED", "disabledReason", UserModel.EMAIL_PENDING }; private static final String[] DEFAULT_ADMIN_READ_ONLY_ATTRIBUTES = { "KERBEROS_PRINCIPAL", "LDAP_ID", "LDAP_ENTRY_DN", "CREATED_TIMESTAMP", "createTimestamp", "modifyTimestamp" }; private static final Pattern readOnlyAttributesPattern = getRegexPatternString(DEFAULT_READ_ONLY_ATTRIBUTES); private static final Pattern adminReadOnlyAttributesPattern = getRegexPatternString(DEFAULT_ADMIN_READ_ONLY_ATTRIBUTES); @@ -479,6 +480,9 @@ public class DeclarativeUserProfileProviderFactory implements UserProfileProvide metadata.addAttribute(UserModel.LOCALE, -1, DeclarativeUserProfileProviderFactory::isInternationalizationEnabled, DeclarativeUserProfileProviderFactory::isInternationalizationEnabled) .setRequired(AttributeMetadata.ALWAYS_FALSE); + metadata.addAttribute(UserModel.EMAIL_PENDING, -1, this::isUpdateEmailFeatureEnabled, this::isUpdateEmailFeatureEnabled) + .setAttributeDisplayName("${emailPendingVerification}") + .setRequired(AttributeMetadata.ALWAYS_FALSE); metadata.addAttribute(TermsAndConditions.USER_ATTRIBUTE, -1, AttributeMetadata.ALWAYS_FALSE, DeclarativeUserProfileProviderFactory::isTermAndConditionsEnabled) @@ -559,4 +563,18 @@ public class DeclarativeUserProfileProviderFactory implements UserProfileProvide return rawAnnotations; } + + private boolean isUpdateEmailFeatureEnabled(AttributeContext context) { + Entry> attribute = context.getAttribute(); + + if (attribute.getValue().isEmpty()) { + return false; + } + + KeycloakSession session = context.getSession(); + KeycloakContext context1 = session.getContext(); + RealmModel realm = context1.getRealm(); + + return UpdateEmail.isEnabled(realm); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java index 11cc1e899c8..d91e1977248 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractRequiredActionUpdateEmailTest.java @@ -197,7 +197,7 @@ public abstract class AbstractRequiredActionUpdateEmailTest extends AbstractTest String lastName = user.getLastName(); assertNotNull(firstName); assertNotNull(lastName); - changeEmailUsingRequiredAction("new@localhost", true); + changeEmailUsingRequiredAction("new@localhost", true, true); user = ActionUtil.findUserWithAdminClient(adminClient, "new@localhost"); Assert.assertNotNull(user); firstName = user.getFirstName(); @@ -209,5 +209,5 @@ public abstract class AbstractRequiredActionUpdateEmailTest extends AbstractTest } } - protected abstract void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions) throws Exception; + protected abstract void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions, boolean newEmailAsUsername) throws Exception; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTest.java index b8710319021..a4e81fa5700 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTest.java @@ -45,7 +45,7 @@ import org.keycloak.testsuite.util.oauth.OAuthClient; public class RequiredActionUpdateEmailTest extends AbstractRequiredActionUpdateEmailTest { @Override - protected void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions) { + protected void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions, boolean newEmailAsUsername) { loginPage.open(); loginPage.login("test-user@localhost", "password"); @@ -69,7 +69,7 @@ public class RequiredActionUpdateEmailTest extends AbstractRequiredActionUpdateE // add the action and change it configureRequiredActionsToUser("test-user@localhost", UserModel.RequiredAction.UPDATE_EMAIL.name()); - changeEmailUsingRequiredAction("new@localhost", logoutOtherSessions); + changeEmailUsingRequiredAction("new@localhost", logoutOtherSessions, false); if (logoutOtherSessions) { events.expectLogout(event1.getSessionId()) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTestWithVerificationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTestWithVerificationTest.java index 241e6631762..63e457f30fa 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTestWithVerificationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateEmailTestWithVerificationTest.java @@ -21,7 +21,9 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; @@ -30,6 +32,12 @@ import jakarta.mail.Address; import jakarta.mail.Message; import jakarta.mail.MessagingException; import jakarta.mail.internet.MimeMessage; + +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.UUID; + import jakarta.ws.rs.core.Response.Status; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -63,10 +71,6 @@ import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.testsuite.util.oauth.OAuthClient; -import java.io.IOException; -import java.util.List; -import java.util.UUID; - public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractRequiredActionUpdateEmailTest { @Rule @@ -91,24 +95,31 @@ public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractR } @Override - protected void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions) throws Exception { + protected void changeEmailUsingRequiredAction(String newEmail, boolean logoutOtherSessions, boolean newEmailAsUsername) throws Exception { String redirectUri = OAuthClient.APP_ROOT + "/auth?nonce=" + UUID.randomUUID(); oauth.redirectUri(redirectUri); loginPage.open(); loginPage.login("test-user@localhost", "password"); + updateEmailPage.assertCurrent(); - updateEmailPage.assertCurrent(); - if (logoutOtherSessions) { - updateEmailPage.checkLogoutSessions(); - } - Assert.assertEquals(logoutOtherSessions, updateEmailPage.isLogoutSessionsChecked()); - updateEmailPage.changeEmail(newEmail); + if (logoutOtherSessions) { + updateEmailPage.checkLogoutSessions(); + } + + Assert.assertEquals(logoutOtherSessions, updateEmailPage.isLogoutSessionsChecked()); + updateEmailPage.changeEmail(newEmail); events.expect(EventType.SEND_VERIFY_EMAIL).detail(Details.EMAIL, newEmail).assertEvent(); UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); assertEquals("test-user@localhost", user.getEmail()); assertTrue(user.getRequiredActions().contains(UserModel.RequiredAction.UPDATE_EMAIL.name())); + assertNotEquals(newEmail, user.getEmail()); + assertFalse(user.isEmailVerified()); + Map> attributes = user.getAttributes(); + assertNotNull(attributes.get(UserModel.EMAIL_PENDING)); + assertEquals(1, attributes.get(UserModel.EMAIL_PENDING).size()); + assertEquals(newEmail, attributes.get(UserModel.EMAIL_PENDING).get(0)); driver.navigate().to(fetchEmailConfirmationLink(newEmail)); @@ -117,6 +128,16 @@ public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractR infoPage.clickBackToApplicationLink(); WaitUtils.waitForPageToLoad(); assertEquals(redirectUri, driver.getCurrentUrl()); + + if (newEmailAsUsername) { + user = ActionUtil.findUserWithAdminClient(adminClient, newEmail); + } else { + user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); + } + attributes = user.getAttributes(); + assertTrue(attributes == null || !attributes.containsKey(UserModel.EMAIL_PENDING)); + assertEquals(newEmail, user.getEmail()); + assertTrue(user.isEmailVerified()); } private void updateEmail(boolean logoutOtherSessions) throws Exception { @@ -130,7 +151,7 @@ public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractR // add action and change email configureRequiredActionsToUser("test-user@localhost", UserModel.RequiredAction.UPDATE_EMAIL.name()); - changeEmailUsingRequiredAction("new@localhost", logoutOtherSessions); + changeEmailUsingRequiredAction("new@localhost", logoutOtherSessions, false); if (logoutOtherSessions) { events.expectLogout(event1.getSessionId()) @@ -476,7 +497,7 @@ public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractR // Verification email should be sent and email should be set UserRepresentation updatedUser = testRealm().users().get(findUser("pendinguser").getId()).toRepresentation(); - assertEquals("Email should be set immediately", "pending@localhost", updatedUser.getEmail()); + assertNull("Email should be not set immediately", updatedUser.getEmail()); assertTrue("User should have UPDATE_EMAIL required action", updatedUser.getRequiredActions().contains(UserModel.RequiredAction.UPDATE_EMAIL.name())); @@ -564,5 +585,4 @@ public class RequiredActionUpdateEmailTestWithVerificationTest extends AbstractR ApiUtil.removeUserByUsername(testRealm(), "realmverifyuser"); } } - } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java index 1a9fd799746..4233fb5b7ac 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java @@ -116,7 +116,9 @@ public class RequiredActionUpdateProfileTest extends AbstractChangeImportedUserP @Test public void updateProfile() { loginPage.open(); - + UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); + user.setEmailVerified(true); + adminClient.realm("test").users().get(user.getId()).update(user); loginPage.login("test-user@localhost", getPassword("test-user@localhost")); updateProfilePage.assertCurrent(); @@ -133,7 +135,7 @@ public class RequiredActionUpdateProfileTest extends AbstractChangeImportedUserP events.expectLogin().assertEvent(); // assert user is really updated in persistent store - UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); + user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); Assert.assertEquals("New first", user.getFirstName()); Assert.assertEquals("New last", user.getLastName()); Assert.assertEquals("new@email.com", user.getEmail()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java index 0af9fc6d346..c3d77e738ab 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java @@ -290,7 +290,7 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest { johnResource.remove(); try { - john = johnResource.toRepresentation(true); + johnResource.toRepresentation(true); Assert.fail("should remove the user"); } catch (NotFoundException expected) { } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java index 86e597b5c24..8ec6f8f17a6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java @@ -50,6 +50,7 @@ import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.authentication.requiredactions.TermsAndConditions; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentValidationException; @@ -58,6 +59,7 @@ import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; +import org.keycloak.models.RequiredActionProviderModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.representations.idm.AbstractUserRepresentation; @@ -758,12 +760,12 @@ public class UserProfileTest extends AbstractUserProfileTest { } private static void testUpdateEmail(KeycloakSession session) { - Map attributes = new HashMap<>(); + RealmModel realm = session.getContext().getRealm(); + RequiredActionProviderModel actionConfig = realm.getRequiredActionProviderByAlias(RequiredAction.UPDATE_EMAIL.name()); - attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId()); - attributes.put(UserModel.FIRST_NAME, "John"); - attributes.put(UserModel.LAST_NAME, "Doe"); - attributes.put(UserModel.EMAIL, "canchange@foo.bar"); + actionConfig.setEnabled(true); + + realm.updateRequiredActionProvider(actionConfig); UserProfileProvider provider = getUserProfileProvider(session); UPConfig config = UPConfigUtils.parseSystemDefaultConfig(); @@ -773,25 +775,59 @@ public class UserProfileTest extends AbstractUserProfileTest { // configure email r/w for user provider.setConfiguration(config); - UserProfile profile = provider.create(UserProfileContext.ACCOUNT, attributes); + Map attributes = new HashMap<>(); + + attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId()); + attributes.put(UserModel.FIRST_NAME, "John"); + attributes.put(UserModel.LAST_NAME, "Doe"); + attributes.put(UserModel.EMAIL, "myemail@foo.bar"); + + UserProfile profile = provider.create(UserProfileContext.USER_API, attributes); UserModel user = profile.create(); + assertEquals(attributes.get(UserModel.EMAIL), user.getEmail()); + assertNull(user.getFirstAttribute(UserModel.EMAIL_PENDING)); assertThat(profile.getAttributes().nameSet(), - containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.FIRST_NAME, UserModel.LAST_NAME, UserModel.LOCALE)); - - profile = provider.create(UserProfileContext.USER_API, attributes, user); - - Set attributesUpdated = new HashSet<>(); - - profile.update((attributeName, userModel, oldValue) -> assertTrue(attributesUpdated.add(attributeName))); + containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.FIRST_NAME, UserModel.LAST_NAME, UserModel.LOCALE, TermsAndConditions.USER_ATTRIBUTE, UserModel.EMAIL_PENDING)); attributes.put("email", "changed@foo.bar"); profile = provider.create(UserProfileContext.ACCOUNT, attributes, user); + try { + profile.update(); + fail("Should fail"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(UserModel.EMAIL)); + } + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes, user); + try { + profile.update(); + fail("Should fail"); + } catch (ValidationException ve) { + assertTrue(ve.isAttributeOnError(UserModel.EMAIL)); + } + + attributes.remove(UserModel.EMAIL); + attributes.put(UserModel.EMAIL_PENDING, "pending@foo.bar"); + + profile = provider.create(UserProfileContext.ACCOUNT, attributes, user); profile.update(); + assertNull(user.getFirstAttribute(UserModel.EMAIL_PENDING)); - assertEquals("E-Mail address should have been changed!", "changed@foo.bar", user.getEmail()); + profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes, user); + profile.update(); + assertNull(user.getFirstAttribute(UserModel.EMAIL_PENDING)); + + profile = provider.create(UserProfileContext.USER_API, attributes, user); + profile.update(); + assertNotNull(user.getFirstAttribute(UserModel.EMAIL_PENDING)); + + attributes.put(UserModel.EMAIL_PENDING, ""); + + profile = provider.create(UserProfileContext.USER_API, attributes, user); + profile.update(); + assertNull(user.getFirstAttribute(UserModel.EMAIL_PENDING)); } @Test