From 00fb36437df1e6d25222133c50658f272b047a1a Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Fri, 8 Dec 2017 00:10:29 +0100 Subject: [PATCH] KEYCLOAK-5861 Remove AUTH_SESSION_ID when END_AFTER_REQUIRED_ACTIONS set --- .../managers/AuthenticationManager.java | 6 +- .../testsuite/auth/page/account/Account.java | 5 ++ .../keycloak/testsuite/page/AbstractPage.java | 6 ++ .../updaters/RealmAttributeUpdater.java | 22 ++------ .../updaters/UserAttributeUpdater.java | 55 +++++++++++++++++++ .../RequiredActionEmailVerificationTest.java | 38 ++++++++++++- 6 files changed, 111 insertions(+), 21 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index 649c77d7d6..d8483c8b3e 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -765,10 +765,10 @@ public class AuthenticationManager { } Response response = infoPage .createInfoPage(); + + new AuthenticationSessionManager(session).removeAuthenticationSession(authSession.getRealm(), authSession, true); + return response; - - // Don't remove authentication session for now, to ensure that browser buttons (back/refresh) will still work fine. - } RealmModel realm = authSession.getRealm(); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/account/Account.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/account/Account.java index 364b297377..1c3781a06d 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/account/Account.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/auth/page/account/Account.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.auth.page.account; +import org.keycloak.testsuite.util.URLUtils; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -77,4 +78,8 @@ public class Account extends AccountManagement { return this; } + public boolean isCurrent() { + return URLUtils.currentUrlStartWith(toString()); // Sometimes after login the URL ends with /# or similar + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPage.java index 34daa16138..c4a514ffed 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/page/AbstractPage.java @@ -26,6 +26,7 @@ import javax.ws.rs.core.UriBuilder; import java.net.URI; import java.util.HashMap; import java.util.Map; +import org.junit.Assert; /** * @@ -102,4 +103,9 @@ public abstract class AbstractPage { return URLUtils.currentUrlEqual(toString()); } + public void assertCurrent() { + String name = getClass().getSimpleName(); + Assert.assertTrue("Expected " + name + " but was " + driver.getTitle() + " (" + driver.getCurrentUrl() + ")", + isCurrent()); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java index 72c2c4b184..b69fec68b6 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java @@ -4,7 +4,6 @@ import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.representations.idm.RealmRepresentation; import java.io.Closeable; import java.util.HashMap; -import java.util.Map; /** * @@ -12,14 +11,14 @@ import java.util.Map; */ public class RealmAttributeUpdater { - private final Map originalAttributes = new HashMap<>(); - private final RealmResource realmResource; private final RealmRepresentation rep; + private final RealmRepresentation origRep; public RealmAttributeUpdater(RealmResource realmResource) { this.realmResource = realmResource; + this.origRep = realmResource.toRepresentation(); this.rep = realmResource.toRepresentation(); if (this.rep.getAttributes() == null) { this.rep.setAttributes(new HashMap<>()); @@ -27,29 +26,18 @@ public class RealmAttributeUpdater { } public RealmAttributeUpdater setAttribute(String name, String value) { - if (! originalAttributes.containsKey(name)) { - this.originalAttributes.put(name, this.rep.getAttributes().put(name, value)); - } else { - this.rep.getAttributes().put(name, value); - } + this.rep.getAttributes().put(name, value); return this; } public RealmAttributeUpdater removeAttribute(String name) { - if (! originalAttributes.containsKey(name)) { - this.originalAttributes.put(name, this.rep.getAttributes().put(name, null)); - } else { - this.rep.getAttributes().put(name, null); - } + this.rep.getAttributes().put(name, null); return this; } public Closeable update() { realmResource.update(rep); - return () -> { - rep.getAttributes().putAll(originalAttributes); - realmResource.update(rep); - }; + return () -> realmResource.update(origRep); } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java new file mode 100644 index 0000000000..b7e14d5471 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/UserAttributeUpdater.java @@ -0,0 +1,55 @@ +package org.keycloak.testsuite.updaters; + +import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.representations.idm.UserRepresentation; +import java.io.Closeable; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; + +/** + * + * @author hmlnarik + */ +public class UserAttributeUpdater { + + private final UserResource userResource; + + private final UserRepresentation rep; + private final UserRepresentation origRep; + + public UserAttributeUpdater(UserResource userResource) { + this.userResource = userResource; + this.origRep = userResource.toRepresentation(); + this.rep = userResource.toRepresentation(); + if (this.rep.getAttributes() == null) { + this.rep.setAttributes(new HashMap<>()); + } + } + + public UserAttributeUpdater setAttribute(String name, List value) { + this.rep.getAttributes().put(name, value); + return this; + } + + public UserAttributeUpdater setAttribute(String name, String... values) { + this.rep.getAttributes().put(name, Arrays.asList(values)); + return this; + } + + public UserAttributeUpdater removeAttribute(String name) { + this.rep.getAttributes().put(name, null); + return this; + } + + public UserAttributeUpdater setEmailVerified(Boolean emailVerified) { + rep.setEmailVerified(emailVerified); + return this; + } + + public Closeable update() { + userResource.update(rep); + + return () -> userResource.update(origRep); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java index e95d500432..662ea01014 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java @@ -27,6 +27,7 @@ import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; import org.keycloak.models.Constants; +import org.keycloak.models.UserModel.RequiredAction; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -41,15 +42,17 @@ import org.keycloak.testsuite.pages.InfoPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.RegisterPage; import org.keycloak.testsuite.pages.VerifyEmailPage; +import org.keycloak.testsuite.updaters.UserAttributeUpdater; import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.MailUtils; import org.keycloak.testsuite.util.UserActionTokenBuilder; import org.keycloak.testsuite.util.UserBuilder; +import java.io.Closeable; import javax.mail.MessagingException; -import javax.mail.Multipart; import javax.mail.internet.MimeMessage; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -581,4 +584,37 @@ public class RequiredActionEmailVerificationTest extends AbstractTestRealmKeyclo return MailUtils.getPasswordResetEmailLink(message); } + // https://issues.jboss.org/browse/KEYCLOAK-5861 + @Test + public void verifyEmailNewBrowserSessionWithClientRedirect() throws IOException, MessagingException { + try (Closeable u = new UserAttributeUpdater(testRealm().users().get(testUserId)) + .setEmailVerified(false) + .update()) { + testRealm().users().get(testUserId).executeActionsEmail(Arrays.asList(RequiredAction.VERIFY_EMAIL.name())); + + Assert.assertEquals(1, greenMail.getReceivedMessages().length); + MimeMessage message = greenMail.getLastReceivedMessage(); + + String verificationUrl = getPasswordResetEmailLink(message); + + driver.manage().deleteAllCookies(); + + driver.navigate().to(verificationUrl.trim()); + proceedPage.assertCurrent(); + proceedPage.clickProceedLink(); + + infoPage.assertCurrent(); + assertEquals("Your account has been updated.", infoPage.getInfo()); + + // Now log into account page + accountPage.setAuthRealm(testRealm().toRepresentation().getRealm()); + accountPage.navigateTo(); + + loginPage.assertCurrent(); + loginPage.login("test-user@localhost", "password"); + + accountPage.assertCurrent(); + } + } + }