From 1d23c3c720a23e5e12883a108a3b52a0cb578a78 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Thu, 26 Sep 2024 13:49:46 +0200 Subject: [PATCH] Use note to detect the IDP verify email action is already done Closes #31563 Signed-off-by: rmartinc --- ...dpVerifyAccountLinkActionTokenHandler.java | 30 +++++++----- .../testsuite/pages/IdpLinkEmailPage.java | 9 +++- .../broker/AbstractFirstBrokerLoginTest.java | 48 +++++++++++++++++++ 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java b/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java index 324107a6d16..debd603ea10 100644 --- a/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java +++ b/services/src/main/java/org/keycloak/authentication/actiontoken/idpverifyemail/IdpVerifyAccountLinkActionTokenHandler.java @@ -30,7 +30,6 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.services.Urls; -import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.messages.Messages; import org.keycloak.sessions.AuthenticationSessionCompoundId; @@ -81,17 +80,21 @@ public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenH AuthenticationSessionModel authSession = tokenContext.getAuthenticationSession(); - if (user.isEmailVerified() && !isVerifyEmailActionSet(user, authSession)) { - event.user(user).error(Errors.EMAIL_ALREADY_VERIFIED); - return session.getProvider(LoginFormsProvider.class) - .setAuthenticationSession(session.getContext().getAuthenticationSession()) - .setInfo(Messages.EMAIL_VERIFIED_ALREADY, user.getEmail()) - .createInfoPage(); + if (authSession.getAuthNote(IdpEmailVerificationAuthenticator.VERIFY_ACCOUNT_IDP_USERNAME) != null) { + return sendEmailAlreadyVerified(session, event, user); } - event.success(); + AuthenticationSessionManager asm = new AuthenticationSessionManager(session); if (tokenContext.isAuthenticationSessionFresh()) { + AuthenticationSessionCompoundId compoundId = AuthenticationSessionCompoundId.encoded(token.getCompoundAuthenticationSessionId()); + ClientModel originalClient = realm.getClientById(compoundId.getClientUUID()); + AuthenticationSessionModel origAuthSession = asm.getAuthenticationSessionByIdAndClient(realm, + compoundId.getRootSessionId(), originalClient, compoundId.getTabId()); + if (origAuthSession == null || origAuthSession.getAuthNote(IdpEmailVerificationAuthenticator.VERIFY_ACCOUNT_IDP_USERNAME) != null) { + return sendEmailAlreadyVerified(session, event, user); + } + token.setOriginalCompoundAuthenticationSessionId(token.getCompoundAuthenticationSessionId()); String authSessionEncodedId = AuthenticationSessionCompoundId.fromAuthSession(authSession).getEncodedId(); @@ -109,9 +112,9 @@ public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenH // verify user email as we know it is valid as this entry point would never have gotten here. user.setEmailVerified(true); + event.success(); if (token.getOriginalCompoundAuthenticationSessionId() != null) { - AuthenticationSessionManager asm = new AuthenticationSessionManager(session); asm.removeAuthenticationSession(realm, authSession, true); AuthenticationSessionCompoundId compoundId = AuthenticationSessionCompoundId.encoded(token.getOriginalCompoundAuthenticationSessionId()); @@ -140,8 +143,11 @@ public class IdpVerifyAccountLinkActionTokenHandler extends AbstractActionTokenH return tokenContext.brokerFlow(null, null, authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH)); } - private boolean isVerifyEmailActionSet(UserModel user, AuthenticationSessionModel authSession) { - return Stream.concat(user.getRequiredActionsStream(), authSession.getRequiredActions().stream()) - .anyMatch(RequiredAction.VERIFY_EMAIL.name()::equals); + private Response sendEmailAlreadyVerified(KeycloakSession session, EventBuilder event, UserModel user) { + event.user(user).error(Errors.EMAIL_ALREADY_VERIFIED); + return session.getProvider(LoginFormsProvider.class) + .setAuthenticationSession(session.getContext().getAuthenticationSession()) + .setInfo(Messages.EMAIL_VERIFIED_ALREADY, user.getEmail()) + .createInfoPage(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/IdpLinkEmailPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/IdpLinkEmailPage.java index 86e7c9bfb77..330f9f5c7b6 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/IdpLinkEmailPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/IdpLinkEmailPage.java @@ -28,9 +28,12 @@ public class IdpLinkEmailPage extends AbstractPage { @FindBy(id = "instruction1") private WebElement message; - @FindBy(linkText = "Click here") + @FindBy(xpath = "//p[@id='instruction2']/a[text() = 'Click here']") private WebElement resendEmailLink; + @FindBy(xpath = "//p[@id='instruction3']/a[text() = 'Click here']") + private WebElement continueLink; + @Override public boolean isCurrent() { return PageUtils.getPageTitle(driver).startsWith("Link "); @@ -48,4 +51,8 @@ public class IdpLinkEmailPage extends AbstractPage { public void resendEmail() { resendEmailLink.click(); } + + public void continueLink() { + continueLink.click(); + } } 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 a6e9ec866b1..4032c8ab343 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 @@ -47,6 +47,7 @@ import org.openqa.selenium.support.PageFactory; import static org.junit.Assert.assertEquals; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertTrue; import static org.keycloak.storage.UserStorageProviderModel.IMPORT_ENABLED; import static org.keycloak.testsuite.admin.ApiUtil.removeUserByUsername; @@ -1018,6 +1019,53 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractInitializedBa waitForPage(driver, "your email address has been verified already.", false); } + @Test + public void testLinkAccountByEmailVerificationInAnotherBrowser() { + RealmResource realm = adminClient.realm(bc.consumerRealmName()); + + UserResource userResource = realm.users().get(createUser("consumer")); + UserRepresentation consumerUser = userResource.toRepresentation(); + + consumerUser.setEmail(bc.getUserEmail()); + consumerUser.setEmailVerified(true); + userResource.update(consumerUser); + configureSMTPServer(); + + oauth.clientId("broker-app"); + loginPage.open(bc.consumerRealmName()); + + logInWithBroker(bc); + + //link account by email + waitForPage(driver, "update account information", false); + Assert.assertTrue(updateAccountInformationPage.isCurrent()); + updateAccountInformationPage.updateAccountInformation("FirstName", "LastName"); + waitForPage(driver, "account already exists", false); + assertTrue(idpConfirmLinkPage.isCurrent()); + assertEquals("User with email user@localhost.com already exists. How do you want to continue?", idpConfirmLinkPage.getMessage()); + idpConfirmLinkPage.clickLinkAccount(); + idpLinkEmailPage.assertCurrent(); + + String url = assertEmailAndGetUrl(MailServerConfiguration.FROM, USER_EMAIL, + "Someone wants to link your ", false); + + // in the second browser confirm the mail + driver2.navigate().to(url); + assertThat(driver2.findElement(By.className("instruction")).getText(), startsWith("Confirm linking the account")); + driver2.findElement(By.linkText("ยป Click here to proceed")).click(); + assertThat(driver2.findElement(By.className("instruction")).getText(), startsWith("You successfully verified your email.")); + + idpLinkEmailPage.continueLink(); + + //test if user is logged in + assertTrue(driver.getCurrentUrl().startsWith(getConsumerRoot() + "/auth/realms/master/app/")); + // check user is linked + List identities = userResource.getFederatedIdentity(); + assertEquals(1, identities.size()); + assertEquals(bc.getIDPAlias(), identities.iterator().next().getIdentityProvider()); + assertEquals(bc.getUserLogin(), identities.iterator().next().getUserName()); + } + @Test public void testLinkAccountByEmailVerificationToEmailVerifiedUser() { // set up a user with verified email