Disable email verification when email manually changed by idp review

Closes #40446

Signed-off-by: rmartinc <rmartinc@redhat.com>
(cherry picked from commit 86f0a7864f)

docs/documentation/upgrading/topics/changes/changes-26_2_6.adoc

@@ -25,3 +25,15 @@ The docs also stated to update the `cache-ispn.xml` configuration file for volat
 The current version will always use safe settings for the number of owners and maximum cache size for the affected user and client session caches, and will log only an INFO message.
 With this behavior, there is no need any more to update the `cache-ispn.xml` configuration file.
 If you previously used a custom `cache-ispn.xml` in order to use volatile user sessions, we recommend reverting those changes and use the standard configuration file.
+
+=== Verify existing account by Email is only executed for the email and username sent by the identity provider
+
+The execution *Verify Existing Account By Email* is one of the alternatives that the *First Login Flow* has to allow a brokering account to be linked to an existing {project_name} user. This step is executed when the user logs in into {project_name} through the broker for the first time, and the identity provider account is already present in {project_name}. The execution sends an email to the current {project_name} address in order to confirm the user controls that account.
+
+Since this release, the *Verify Existing Account By Email* execution is only attempted in the *First Login Flow* if the linking attributes (email and username) sent by the external identity provider are not modified by the user during the review process. This new behavior avoids sending verification emails to an existing {project_name} account that can inadvertently accept the linking.
+
+In case the provider needs to modify the information sent by the identity provider (because emails or usernames are different in the broker), only the other alternative *Verify Existing Account By Re-authentication* is available to link the new account to the existing {project_name} user.
+
+If the data received from the identity provider is mandatory and cannot be modified, then the *Review Profile* step in the *First Login Flow* can be disabled to avoid any user intervention.
+
+For more information, see link:{adminguide_link}#_identity_broker_first_login[the First login flow section of the {adminguide_name}].

server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProvider.java

@@ -42,6 +42,10 @@ import java.util.UUID;
  */
 public abstract class AbstractIdentityProvider<C extends IdentityProviderModel> implements IdentityProvider<C> {
 
+    // The clientSession note flag to indicate that email or username provided by identityProvider was changed on updateProfile page
+    public static final String UPDATE_PROFILE_EMAIL_CHANGED = "UPDATE_PROFILE_EMAIL_CHANGED";
+    public static final String UPDATE_PROFILE_USERNAME_CHANGED = "UPDATE_PROFILE_USERNAME_CHANGED";
+
     public static final String ACCOUNT_LINK_URL = "account-link-url";
     protected final KeycloakSession session;
     private final C config;

server-spi-private/src/main/java/org/keycloak/forms/login/LoginFormsProvider.java

@@ -56,7 +56,7 @@ public interface LoginFormsProvider extends Provider {
 
     Response createForm(String form);
 
-    String getMessage(String message);
+    String getMessage(String message, Object... parameters);
 
     Response createLoginUsernamePassword();
 

services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java

@@ -103,9 +103,10 @@ public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenH
                     authSession.getClient().getClientId(), authSession.getTabId(), AuthenticationProcessor.getClientData(session, authSession));
             String confirmUri = builder.build(realm.getName()).toString();
 
-            return session.getProvider(LoginFormsProvider.class)
-                    .setAuthenticationSession(authSession)
-                    .setSuccess(Messages.CONFIRM_ACCOUNT_LINKING, token.getIdentityProviderUsername(), token.getIdentityProviderAlias())
+            LoginFormsProvider forms = session.getProvider(LoginFormsProvider.class);
+            return forms.setAuthenticationSession(authSession)
+                    .setAttribute("messageHeader", forms.getMessage(Messages.CONFIRM_ACCOUNT_LINKING, token.getIdentityProviderUsername(), token.getIdentityProviderAlias()))
+                    .setSuccess(Messages.CONFIRM_ACCOUNT_LINKING_BODY, token.getIdentityProviderUsername(), token.getIdentityProviderAlias())
                     .setAttribute(Constants.TEMPLATE_ATTR_ACTION_URI, confirmUri)
                     .createInfoPage();
         }

services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpConfirmLinkAuthenticator.java

@@ -30,6 +30,7 @@ import org.keycloak.models.UserModel;
 import org.keycloak.services.ServicesLogger;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.sessions.AuthenticationSessionModel;
+import org.keycloak.sessions.CommonClientSessionModel;
 
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
@@ -50,10 +51,19 @@ public class IdpConfirmLinkAuthenticator extends AbstractIdpAuthenticator {
             return;
         }
 
+        // hide the review button if the idp review execution was not successfully executed before
+        boolean hideReviewButton = authSession.getExecutionStatus().entrySet().stream()
+                .filter(entry -> CommonClientSessionModel.ExecutionStatus.SUCCESS.equals(entry.getValue()))
+                .map(entry -> context.getRealm().getAuthenticationExecutionById(entry.getKey()))
+                .filter(exec -> IdpReviewProfileAuthenticatorFactory.PROVIDER_ID.equals(exec.getAuthenticator()))
+                .findAny()
+                .isEmpty();
+
         ExistingUserInfo duplicationInfo = ExistingUserInfo.deserialize(existingUserInfo);
         Response challenge = context.form()
                 .setStatus(Response.Status.OK)
                 .setAttribute(LoginFormsProvider.IDENTITY_PROVIDER_BROKER_CONTEXT, brokerContext)
+                .setAttribute("hideReviewButton", hideReviewButton ? Boolean.TRUE : null)
                 .setError(Messages.FEDERATED_IDENTITY_CONFIRM_LINK_MESSAGE, duplicationInfo.getDuplicateAttributeName(), duplicationInfo.getDuplicateAttributeValue())
                 .createIdpLinkConfirmLinkPage();
         context.challenge(challenge);

services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpEmailVerificationAuthenticator.java

@@ -23,6 +23,7 @@ import org.keycloak.authentication.AuthenticationFlowError;
 import org.keycloak.authentication.AuthenticationProcessor;
 import org.keycloak.authentication.actiontoken.idpverifyemail.IdpVerifyAccountLinkActionToken;
 import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext;
+import org.keycloak.broker.provider.AbstractIdentityProvider;
 import org.keycloak.broker.provider.BrokeredIdentityContext;
 import org.keycloak.common.util.Time;
 import org.keycloak.email.EmailException;
@@ -70,6 +71,13 @@ public class IdpEmailVerificationAuthenticator extends AbstractIdpAuthenticator
             return;
         }
 
+        if (Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(AbstractIdentityProvider.UPDATE_PROFILE_USERNAME_CHANGED))
+                || Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(AbstractIdentityProvider.UPDATE_PROFILE_EMAIL_CHANGED))) {
+            logger.debug("Email or username changed on review. Ignoring email verification authenticator.");
+            context.attempted();
+            return;
+        }
+
         if (Objects.equals(authSession.getAuthNote(VERIFY_ACCOUNT_IDP_USERNAME), brokerContext.getUsername())) {
             UserModel existingUser = getExistingUser(session, realm, authSession);
 

services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java

@@ -20,6 +20,7 @@ package org.keycloak.authentication.authenticators.broker;
 import org.jboss.logging.Logger;
 import org.keycloak.authentication.AuthenticationFlowContext;
 import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext;
+import org.keycloak.broker.provider.AbstractIdentityProvider;
 import org.keycloak.broker.provider.BrokeredIdentityContext;
 import org.keycloak.events.Details;
 import org.keycloak.events.EventBuilder;
@@ -201,12 +202,21 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator {
         UserProfile profile = profileProvider.create(UserProfileContext.IDP_REVIEW, attributes, updatedProfile);
 
         try {
-            String oldEmail = userCtx.getEmail();
-
             profile.update((attributeName, userModel, oldValue) -> {
-                if (attributeName.equals(UserModel.EMAIL)) {
-                    context.getAuthenticationSession().setAuthNote(UPDATE_PROFILE_EMAIL_CHANGED, "true");
-                    event.clone().event(EventType.UPDATE_EMAIL).detail(Details.CONTEXT, UserProfileContext.IDP_REVIEW.name()).detail(Details.PREVIOUS_EMAIL, oldEmail).detail(Details.UPDATED_EMAIL, profile.getAttributes().getFirst(UserModel.EMAIL)).success();
+                if (attributeName.equals(UserModel.USERNAME)) {
+                    context.getAuthenticationSession().setAuthNote(AbstractIdentityProvider.UPDATE_PROFILE_USERNAME_CHANGED, "true");
+                    event.clone().event(EventType.UPDATE_PROFILE)
+                            .detail(Details.CONTEXT, UserProfileContext.IDP_REVIEW.name())
+                            .detail(Details.PREF_PREVIOUS + UserModel.USERNAME, oldValue)
+                            .detail(Details.PREF_UPDATED + UserModel.USERNAME, profile.getAttributes().getFirst(UserModel.USERNAME))
+                            .success();
+                } else if (attributeName.equals(UserModel.EMAIL)) {
+                    context.getAuthenticationSession().setAuthNote(AbstractIdentityProvider.UPDATE_PROFILE_EMAIL_CHANGED, "true");
+                    event.clone().event(EventType.UPDATE_EMAIL)
+                            .detail(Details.CONTEXT, UserProfileContext.IDP_REVIEW.name())
+                            .detail(Details.PREVIOUS_EMAIL, oldValue)
+                            .detail(Details.UPDATED_EMAIL, profile.getAttributes().getFirst(UserModel.EMAIL))
+                            .success();
                 }
             });
         } catch (ValidationException pve) {

services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java

@@ -468,8 +468,8 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
     }
 
     @Override
-    public String getMessage(String message) {
-        return formatMessage(new FormMessage(null, message));
+    public String getMessage(String message, Object... parameters) {
+        return formatMessage(new FormMessage(null, message, parameters));
     }
 
     /**

services/src/main/java/org/keycloak/services/messages/Messages.java

@@ -215,6 +215,8 @@ public class Messages {
 
     public static final String CONFIRM_ACCOUNT_LINKING = "confirmAccountLinking";
 
+    public static final String CONFIRM_ACCOUNT_LINKING_BODY = "confirmAccountLinkingBody";
+
     public static final String CONFIRM_EMAIL_ADDRESS_VERIFICATION = "confirmEmailAddressVerification";
 
     public static final String CONFIRM_EXECUTION_OF_ACTIONS = "confirmExecutionOfActions";

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/IdpConfirmLinkPage.java

@@ -17,6 +17,7 @@
 
 package org.keycloak.testsuite.pages;
 
+import org.keycloak.testsuite.util.UIUtils;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.support.FindBy;
 
@@ -47,6 +48,10 @@ public class IdpConfirmLinkPage extends LanguageComboboxAwarePage {
         updateProfileButton.click();
     }
 
+    public boolean isReviewProfileDisplayed() {
+        return UIUtils.isElementVisible(updateProfileButton);
+    }
+
     public void clickLinkAccount() {
         linkAccountButton.click();
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java

@@ -14,12 +14,15 @@ import org.junit.Test;
 import org.keycloak.admin.client.resource.IdentityProviderResource;
 import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.admin.client.resource.UserResource;
+import org.keycloak.authentication.authenticators.broker.IdpReviewProfileAuthenticatorFactory;
 import org.keycloak.broker.provider.HardcodedUserSessionAttributeMapper;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.events.Details;
 import org.keycloak.events.EventType;
+import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.models.IdentityProviderMapperModel;
 import org.keycloak.models.IdentityProviderSyncMode;
+import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.representations.idm.FederatedIdentityRepresentation;
@@ -823,6 +826,85 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractInitializedBa
         assertTrue(realm.users().get(linkedUserId).toRepresentation().isEmailVerified());
     }
 
+    @Test
+    public void testLinkAccountModifyingEmailLinkingByEmailNotAllowed() {
+        RealmResource providerRealm = adminClient.realm(bc.providerRealmName());
+
+        configureSMTPServer();
+
+        // change provider user email to changed@localhost.com
+        UserRepresentation userProvider = ApiUtil.findUserByUsername(providerRealm, bc.getUserLogin());
+        userProvider.setEmail("changed@localhost.com");
+        providerRealm.users().get(userProvider.getId()).update(userProvider);
+
+        //create user on consumer's site with the correct email
+        final String linkedUserId = createUser(bc.getUserLogin());
+
+        //test
+        oauth.config().clientId("broker-app");
+        loginPage.open(bc.consumerRealmName());
+
+        logInWithBroker(bc);
+
+        waitForPage(driver, "update account information", false);
+        Assert.assertTrue(updateAccountInformationPage.isCurrent());
+        Assert.assertTrue("We must be on correct realm right now",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/"));
+
+        // update the email to the correct one
+        log.debug("Updating info on updateAccount page");
+        updateAccountInformationPage.updateAccountInformation(USER_EMAIL, "Firstname", "Lastname");
+
+        //link account
+        waitForPage(driver, "account already exists", false);
+        idpConfirmLinkPage.clickLinkAccount();
+
+        //it should start the link using username and password as email has been changed
+        assertEquals("Authenticate to link your account with " + bc.getIDPAlias(), loginPage.getInfoMessage());
+
+        loginPage.login("password");
+        Assert.assertTrue(appPage.isCurrent());
+
+        assertNumFederatedIdentities(linkedUserId, 1);
+    }
+
+    @Test
+    public void testLinkAccountReviewDisabled() throws Exception {
+        RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName());
+
+        // disable the idp review step
+        AuthenticationExecutionInfoRepresentation idpReviewProfileExec = consumerRealm.flows().getExecutions("first broker login").stream()
+                .filter(execution -> IdpReviewProfileAuthenticatorFactory.PROVIDER_ID.equals(execution.getProviderId()))
+                .findAny()
+                .orElse(null);
+        Assert.assertNotNull("IdpReviewProfileAuthenticator execution not found", idpReviewProfileExec);
+        idpReviewProfileExec.setRequirement(AuthenticationExecutionModel.Requirement.DISABLED.name());
+        consumerRealm.flows().updateExecutions("first broker login", idpReviewProfileExec);
+
+        //create user on consumer's site with the correct email
+        final String linkedUserId = createUser(bc.getUserLogin());
+
+        //test
+        oauth.config().clientId("broker-app");
+        loginPage.open(bc.consumerRealmName());
+
+        logInWithBroker(bc);
+
+        // no review displayed and button in link not available
+        waitForPage(driver, "account already exists", false);
+        Assert.assertTrue("We must be on correct realm right now",
+                driver.getCurrentUrl().contains("/auth/realms/" + bc.consumerRealmName() + "/"));
+        Assert.assertFalse("Review Profile button is displayed", idpConfirmLinkPage.isReviewProfileDisplayed());
+        idpConfirmLinkPage.clickLinkAccount();
+
+        //linking the account using password as email not configured
+        assertEquals("Authenticate to link your account with " + bc.getIDPAlias(), loginPage.getInfoMessage());
+
+        loginPage.login("password");
+        Assert.assertTrue(appPage.isCurrent());
+
+        assertNumFederatedIdentities(linkedUserId, 1);
+    }
 
     /**
      * Refers to in old test suite: org.keycloak.testsuite.broker.AbstractKeycloakIdentityProviderTest#testSuccessfulAuthenticationWithoutUpdateProfile_emailProvided_emailVerifyEnabled
@@ -1051,7 +1133,8 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractInitializedBa
 
         // in the second browser confirm the mail
         driver2.navigate().to(url);
-        assertThat(driver2.findElement(By.className("instruction")).getText(), startsWith("Confirm linking the account"));
+        assertThat(driver2.findElement(By.id("kc-page-title")).getText(), startsWith("Confirm linking the account"));
+        assertThat(driver2.findElement(By.className("instruction")).getText(), startsWith("If you link the account, you will also be able to login using account"));
         driver2.findElement(By.linkText("» Click here to proceed")).click();
         assertThat(driver2.findElement(By.className("instruction")).getText(), startsWith("You successfully verified your email."));
 

themes/src/main/resources/theme/base/email/messages/messages_en.properties

@@ -13,8 +13,8 @@ emailTestSubject=[KEYCLOAK] - SMTP test message
 emailTestBody=This is a test message
 emailTestBodyHtml=<p>This is a test message</p>
 identityProviderLinkSubject=Link {0}
-identityProviderLinkBody=Someone wants to link your "{1}" account with "{0}" account of user {2} . If this was you, click the link below to link accounts\n\n{3}\n\nThis link will expire within {5}.\n\nIf you don''t want to link account, just ignore this message. If you link accounts, you will be able to login to {1} through {0}.
-identityProviderLinkBodyHtml=<p>Someone wants to link your <b>{1}</b> account with <b>{0}</b> account of user {2}. If this was you, click the link below to link accounts</p><p><a href="{3}">Link to confirm account linking</a></p><p>This link will expire within {5}.</p><p>If you don''t want to link account, just ignore this message. If you link accounts, you will be able to login to {1} through {0}.</p>
+identityProviderLinkBody=Someone wants to link your "{1}" account with "{0}" account of user {2} . If this was you, click the link below to link accounts\n\n{3}\n\nThis link will expire within {5}.\n\nIf you didn''t initiate this process or don''t want to link account, just ignore this message. If you link accounts, you will be able to login to {1} through {0}.
+identityProviderLinkBodyHtml=<p>Someone wants to link your <b>{1}</b> account with <b>{0}</b> account of user {2}. If this was you, click the link below to link accounts</p><p><a href="{3}">Link to confirm account linking</a></p><p>This link will expire within {5}.</p><p>If you didn''t initiate this process or don''t want to link account, just ignore this message. If you link accounts, you will be able to login to {1} through {0}.</p>
 passwordResetSubject=Reset password
 passwordResetBody=Someone just requested to change your {2} account''s credentials. If this was you, click on the link below to reset them.\n\n{0}\n\nThis link and code will expire within {3}.\n\nIf you don''t want to reset your credentials, just ignore this message and nothing will be changed.
 passwordResetBodyHtml=<p>Someone just requested to change your {2} account''s credentials. If this was you, click on the link below to reset them.</p><p><a href="{0}">Link to reset credentials</a></p><p>This link will expire within {3}.</p><p>If you don''t want to reset your credentials, just ignore this message and nothing will be changed.</p>

themes/src/main/resources/theme/base/login/login-idp-link-confirm.ftl

@@ -5,7 +5,9 @@
     <#elseif section = "form">
         <form id="kc-register-form" action="${url.loginAction}" method="post">
             <div class="${properties.kcFormGroupClass!}">
-                <button type="submit" class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" name="submitAction" id="updateProfile" value="updateProfile">${msg("confirmLinkIdpReviewProfile")}</button>
+                <#if !hideReviewButton?has_content>
+                    <button type="submit" class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" name="submitAction" id="updateProfile" value="updateProfile">${msg("confirmLinkIdpReviewProfile")}</button>
+                </#if>
                 <button type="submit" class="${properties.kcButtonClass!} ${properties.kcButtonDefaultClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" name="submitAction" id="linkAccount" value="linkAccount">${msg("confirmLinkIdpContinue", idpDisplayName)}</button>
             </div>
         </form>

themes/src/main/resources/theme/base/login/messages/messages_en.properties

@@ -376,6 +376,7 @@ emailVerifiedAlreadyMessage=Your email address has been verified already.
 staleEmailVerificationLink=The link you clicked is an old stale link and is no longer valid. Maybe you have already verified your email.
 identityProviderAlreadyLinkedMessage=Federated identity returned by {0} is already linked to another user.
 confirmAccountLinking=Confirm linking the account {0} of identity provider {1} with your account.
+confirmAccountLinkingBody=If you link the account, you will also be able to login using account {0} of the identity provider {1}. Do not proceed if you did not initiate this process or you do not want to link the account.
 confirmEmailAddressVerification=Confirm validity of e-mail address {0}.
 confirmExecutionOfActions=Perform the following action(s)
 
@@ -534,4 +535,4 @@ organization.confirm-membership=By clicking on the link below, you will become a
 organization.member.register.title=Create an account to join the ${kc.org.name} organization
 organization.select=Select an organization to proceed:
 notMemberOfOrganization=User is not a member of the organization {0}
-notMemberOfAnyOrganization=User is not a member of any organization
+notMemberOfAnyOrganization=User is not a member of any organization