From a6609bd969fc1544496457216d651bfed2daa8ac Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Tue, 10 Oct 2023 21:54:37 +0200 Subject: [PATCH] Remove "You are already logged in" during authentication. Make other browser tabs to authenticate automatically when some browser tab successfully authenticate (#23517) Closes #12406 Co-authored-by: Jon Koops --- .../java/org/keycloak/models/Constants.java | 2 + .../AuthenticationProcessor.java | 7 + .../freemarker/AuthenticationStateCookie.java | 109 ++++++++++ .../FreeMarkerLoginFormsProvider.java | 10 + .../model/AuthenticationSessionBean.java | 43 ++++ .../forms/login/freemarker/model/UrlBean.java | 6 +- .../protocol/oidc/OIDCLoginProtocol.java | 11 +- .../keycloak/protocol/oidc/TokenManager.java | 4 +- .../keycloak/protocol/saml/SamlProtocol.java | 3 +- .../main/java/org/keycloak/services/Urls.java | 3 +- .../AuthenticationSessionManager.java | 43 +++- .../resources/LoginActionsService.java | 17 +- .../org/keycloak/testsuite/pages/AppPage.java | 4 + .../forms/MultipleTabsLoginTest.java | 134 +++++++----- .../testsuite/forms/ResetPasswordTest.java | 199 ++++++------------ .../base/login/resources/js/authChecker.js | 75 +++++++ .../resources/theme/base/login/template.ftl | 11 + 17 files changed, 465 insertions(+), 216 deletions(-) create mode 100644 services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java create mode 100644 services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationSessionBean.java create mode 100644 themes/src/main/resources/theme/base/login/resources/js/authChecker.js diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java index 7dd15c0e67..d0714f1435 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/Constants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/Constants.java @@ -79,6 +79,8 @@ public final class Constants { public static final String EXECUTION = "execution"; public static final String CLIENT_ID = "client_id"; public static final String TAB_ID = "tab_id"; + + public static final String SKIP_LOGOUT = "skip_logout"; public static final String KEY = "key"; public static final String KC_ACTION = "kc_action"; diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 0653c9edbe..dc37029e53 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -47,6 +47,7 @@ import org.keycloak.services.ErrorPage; import org.keycloak.services.ErrorPageException; import org.keycloak.services.ServicesLogger; import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.services.managers.AuthenticationSessionManager; import org.keycloak.services.managers.BruteForceProtector; import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.managers.UserSessionManager; @@ -68,6 +69,7 @@ import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification; @@ -931,6 +933,11 @@ public class AuthenticationProcessor { authSession.clearUserSessionNotes(); authSession.clearAuthNotes(); + Set requiredActions = authSession.getRequiredActions(); + for (String reqAction : requiredActions) { + authSession.removeRequiredAction(reqAction); + } + authSession.setAction(CommonClientSessionModel.Action.AUTHENTICATE.name()); authSession.setAuthNote(CURRENT_FLOW_PATH, flowPath); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java new file mode 100644 index 0000000000..9b4d52ceeb --- /dev/null +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/AuthenticationStateCookie.java @@ -0,0 +1,109 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.forms.login.freemarker; + +import java.io.IOException; +import java.util.Set; + +import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.ws.rs.core.UriInfo; +import org.jboss.logging.Logger; +import org.keycloak.common.ClientConnection; +import org.keycloak.models.KeycloakContext; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.services.managers.AuthenticationManager; +import org.keycloak.services.util.CookieHelper; +import org.keycloak.sessions.RootAuthenticationSessionModel; +import org.keycloak.util.JsonSerialization; + +/** + * Non http-only cookie with tracking remaining authSessions in current root authentication session + * + * @author Marek Posolda + */ +public class AuthenticationStateCookie { + + private static final Logger logger = Logger.getLogger(AuthenticationStateCookie.class); + + public static final String KC_AUTH_STATE = "KC_AUTH_STATE"; + + @JsonProperty("authSessionId") + private String authSessionId; + + @JsonProperty("remainingTabs") + private Set remainingTabs; + + public String getAuthSessionId() { + return authSessionId; + } + + public void setAuthSessionId(String authSessionId) { + this.authSessionId = authSessionId; + } + + public Set getRemainingTabs() { + return remainingTabs; + } + + public void setRemainingTabs(Set remainingTabs) { + this.remainingTabs = remainingTabs; + } + + public static void generateAndSetCookie(KeycloakSession session, RealmModel realm, RootAuthenticationSessionModel rootAuthSession) { + UriInfo uriInfo = session.getContext().getHttpRequest().getUri(); + String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); + boolean secureOnly = realm.getSslRequired().isRequired(session.getContext().getConnection()); + + // 1 minute by default. Same timeout, which is used for client to complete "authorization code" flow + // Very short timeout should be OK as when this cookie is set, other existing browser tabs are supposed to be refreshed immediatelly by JS script + // and login user automatically. No need to have cookie living any further + int cookieMaxAge = realm.getAccessCodeLifespan(); + + AuthenticationStateCookie cookie = new AuthenticationStateCookie(); + cookie.setAuthSessionId(rootAuthSession.getId()); + cookie.setRemainingTabs(rootAuthSession.getAuthenticationSessions().keySet()); + + try { + String encoded = JsonSerialization.writeValueAsString(cookie); + logger.tracef("Generating new %s cookie. Cookie: %s, Cookie lifespan: %d", KC_AUTH_STATE, encoded, cookieMaxAge); + + CookieHelper.addCookie(KC_AUTH_STATE, encoded, path, null, null, cookieMaxAge, secureOnly, false, session); + } catch (IOException ioe) { + throw new IllegalStateException("Exception thrown when encoding cookie", ioe); + } + } + + public static void expireCookie(RealmModel realm, KeycloakSession session) { + UriInfo uriInfo = session.getContext().getHttpRequest().getUri(); + String path = AuthenticationManager.getRealmCookiePath(realm, uriInfo); + boolean secureOnly = realm.getSslRequired().isRequired(session.getContext().getConnection()); + CookieHelper.addCookie(KC_AUTH_STATE, "", path, null, null, 0, secureOnly, false, session); + } + + @Override + public String toString() { + return new StringBuilder("AuthenticationStateCookie [ ") + .append("authSessionId=" + authSessionId) + .append(", remainingTabs=" + remainingTabs) + .append(" ]") + .toString(); + } +} diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index ed4d414d59..abfca5cc9e 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -46,6 +46,7 @@ import org.keycloak.common.util.ObjectUtil; import org.keycloak.forms.login.LoginFormsPages; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.forms.login.freemarker.model.AuthenticationContextBean; +import org.keycloak.forms.login.freemarker.model.AuthenticationSessionBean; import org.keycloak.forms.login.freemarker.model.RecoveryAuthnCodeInputLoginBean; import org.keycloak.forms.login.freemarker.model.RecoveryAuthnCodesBean; import org.keycloak.forms.login.freemarker.model.ClientBean; @@ -226,6 +227,15 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { attributes.put("statusCode", status.getStatusCode()); } + + if (!isDetachedAuthenticationSession()) { + if ((AuthenticationSessionModel.Action.AUTHENTICATE.name().equals(authenticationSession.getAction())) || + (AuthenticationSessionModel.Action.REQUIRED_ACTIONS.name().equals(authenticationSession.getAction())) || + (AuthenticationSessionModel.Action.OAUTH_GRANT.name().equals(authenticationSession.getAction()))) { + setAttribute("authenticationSession", new AuthenticationSessionBean(authenticationSession.getParentSession().getId(), authenticationSession.getTabId())); + } + } + switch (page) { case LOGIN_CONFIG_TOTP: attributes.put("totp", new TotpBean(session, realm, user, getTotpUriBuilder())); diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationSessionBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationSessionBean.java new file mode 100644 index 0000000000..e8128532a5 --- /dev/null +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/AuthenticationSessionBean.java @@ -0,0 +1,43 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.forms.login.freemarker.model; + +/** + * @author Marek Posolda + */ +public class AuthenticationSessionBean { + + private final String authSessionId; + private final String tabId; + + public AuthenticationSessionBean(String authSessionId, String tabId) { + this.authSessionId = authSessionId; + this.tabId = tabId; + } + + public String getAuthSessionId() { + return authSessionId; + } + + public String getTabId() { + return tabId; + } + +} diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/model/UrlBean.java b/services/src/main/java/org/keycloak/forms/login/freemarker/model/UrlBean.java index 266ffc54e2..e615ae29e9 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/model/UrlBean.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/model/UrlBean.java @@ -57,7 +57,11 @@ public class UrlBean { } public String getLoginRestartFlowUrl() { - return Urls.realmLoginRestartPage(baseURI, realm).toString(); + return Urls.realmLoginRestartPage(baseURI, realm, false).toString(); + } + + public String getSsoLoginInOtherTabsUrl() { + return Urls.realmLoginRestartPage(baseURI, realm, true).toString(); } public boolean hasAction() { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index 5d845a47c9..457ed7a19a 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -279,7 +279,7 @@ public class OIDCLoginProtocol implements LoginProtocol { session.clientPolicy().triggerOnEvent(new ImplicitHybridTokenResponse(authSession, clientSessionCtx, responseBuilder)); } catch (ClientPolicyException cpe) { event.error(cpe.getError()); - new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, true); + new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession); redirectUri.addParam(OAuth2Constants.ERROR_DESCRIPTION, cpe.getError()); if (!clientConfig.isExcludeIssuerFromAuthResponse()) { redirectUri.addParam(OAuth2Constants.ISSUER, clientSession.getNote(OIDCLoginProtocol.ISSUER)); @@ -333,12 +333,9 @@ public class OIDCLoginProtocol implements LoginProtocol { redirectUri.addParam(OAuth2Constants.STATE, state); } - if (error == Error.PASSIVE_LOGIN_REQUIRED || error == Error.PASSIVE_INTERACTION_REQUIRED) { - // passive check error, just delete the tabId maintaining session and don't reset the restart cookie - new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession); - } else { - new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, true); - } + // Remove authenticationSession from current tab + new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession); + return redirectUri.build(); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index 7d0351132f..35602a222c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -584,8 +584,8 @@ public class TokenManager { clientSession.setNote(Constants.LEVEL_OF_AUTHENTICATION, String.valueOf(new AcrStore(authSession).getLevelOfAuthenticationFromCurrentAuthentication())); clientSession.setTimestamp(userSession.getLastSessionRefresh()); - // Remove authentication session now - new AuthenticationSessionManager(session).removeAuthenticationSession(userSession.getRealm(), authSession, true); + // Remove authentication session now (just current tab, not whole "rootAuthenticationSession" in case we have more browser tabs with "authentications in progress") + new AuthenticationSessionManager(session).updateAuthenticationSessionAfterSuccessfulAuthentication(userSession.getRealm(), authSession); ClientSessionContext clientSessionCtx = DefaultClientSessionContext.fromClientSessionAndClientScopeIds(clientSession, clientScopeIds, session); return clientSessionCtx; diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index 4e3f6367a1..74c71d864a 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -236,7 +236,8 @@ public class SamlProtocol implements LoginProtocol { ); } } finally { - new AuthenticationSessionManager(session).removeAuthenticationSession(realm, authSession, true); + // Remove authenticationSession of current browser tab + new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession); } } diff --git a/services/src/main/java/org/keycloak/services/Urls.java b/services/src/main/java/org/keycloak/services/Urls.java index c9d5a34add..2a2832fcac 100755 --- a/services/src/main/java/org/keycloak/services/Urls.java +++ b/services/src/main/java/org/keycloak/services/Urls.java @@ -160,8 +160,9 @@ public class Urls { return loginActionsBase(baseUri).path(LoginActionsService.class, "authenticate").build(realmName); } - public static URI realmLoginRestartPage(URI baseUri, String realmId) { + public static URI realmLoginRestartPage(URI baseUri, String realmId, boolean skipLogout) { return loginActionsBase(baseUri).path(LoginActionsService.class, "restartSession") + .queryParam(Constants.SKIP_LOGOUT, String.valueOf(skipLogout)) .build(realmId); } diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java index e2f4180ac6..87877294bd 100644 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationSessionManager.java @@ -19,24 +19,30 @@ package org.keycloak.services.managers; import org.jboss.logging.Logger; import org.keycloak.common.util.ServerCookie.SameSiteAttributeValue; +import org.keycloak.common.util.Time; import org.keycloak.forms.login.LoginFormsProvider; +import org.keycloak.forms.login.freemarker.AuthenticationStateCookie; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.RestartLoginCookie; +import org.keycloak.services.resources.LoginActionsService; import org.keycloak.services.util.CookieHelper; import org.keycloak.sessions.AuthenticationSessionModel; +import org.keycloak.sessions.CommonClientSessionModel; import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.sessions.StickySessionEncoderProvider; import jakarta.ws.rs.core.UriInfo; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; +import static org.keycloak.authentication.AuthenticationProcessor.CURRENT_FLOW_PATH; import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification; /** @@ -215,25 +221,56 @@ public class AuthenticationSessionManager { public void removeAuthenticationSession(RealmModel realm, AuthenticationSessionModel authSession, boolean expireRestartCookie) { RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession(); - log.debugf("Removing authSession '%s'. Expire restart cookie: %b", rootAuthSession.getId(), expireRestartCookie); + log.debugf("Removing root authSession '%s'. Expire restart cookie: %b", rootAuthSession.getId(), expireRestartCookie); session.authenticationSessions().removeRootAuthenticationSession(realm, rootAuthSession); // expire restart cookie if (expireRestartCookie) { UriInfo uriInfo = session.getContext().getUri(); RestartLoginCookie.expireRestartCookie(realm, uriInfo, session); + AuthenticationStateCookie.expireCookie(realm, session); // With browser session, this makes sure that info/error pages will be rendered correctly when locale is changed on them session.getProvider(LoginFormsProvider.class).setDetachedAuthSession(); } } - public void removeTabIdInAuthenticationSession(RealmModel realm, AuthenticationSessionModel authSession) { + /** + * Remove authentication session from root session. Possibly remove whole root authentication session if there are no other browser tabs + * @param realm + * @param authSession + * @return true if whole root authentication session was removed. False just if single tab was removed + */ + public boolean removeTabIdInAuthenticationSession(RealmModel realm, AuthenticationSessionModel authSession) { RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession(); rootAuthSession.removeAuthenticationSessionByTabId(authSession.getTabId()); if (rootAuthSession.getAuthenticationSessions().isEmpty()) { // no more tabs, remove the session completely - removeAuthenticationSession(realm, authSession, false); + removeAuthenticationSession(realm, authSession, true); + return true; + } else { + return false; + } + } + + /** + * This happens when one browser tab successfully finished authentication (including required actions and consent screen if applicable) + * Just authenticationSession of the current browser tab is removed from "root authentication session" and other tabs are kept, so + * authentication can be automatically finished in other browser tabs (typically with authChecker.js javascript) + * + * @param realm + * @param authSession + */ + public void updateAuthenticationSessionAfterSuccessfulAuthentication(RealmModel realm, AuthenticationSessionModel authSession) { + // TODO: The authentication session might need to be expired in short interval (realm accessCodeLifespan, which is 1 minute by default). That should be sufficient for other browser tabs + // to finish authentication and at the same time we won't need to keep authentication sessions in storage longer than needed + boolean removedRootAuthSession = removeTabIdInAuthenticationSession(realm, authSession); + if (!removedRootAuthSession) { + RootAuthenticationSessionModel rootAuthSession = authSession.getParentSession(); + + log.tracef("Removed authentication session of root session '%s' with tabId '%s'. But there are remaining tabs in the root session", rootAuthSession.getId(), authSession.getTabId()); + + AuthenticationStateCookie.generateAndSetCookie(session, realm, rootAuthSession); } } diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index ec8e0d85cc..27b9a23f59 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -220,7 +220,8 @@ public class LoginActionsService { @GET public Response restartSession(@QueryParam(AUTH_SESSION_ID) String authSessionId, // optional, can get from cookie instead @QueryParam(Constants.CLIENT_ID) String clientId, - @QueryParam(Constants.TAB_ID) String tabId) { + @QueryParam(Constants.TAB_ID) String tabId, + @QueryParam(Constants.SKIP_LOGOUT) String skipLogout) { event.event(EventType.RESTART_AUTHENTICATION); SessionCodeChecks checks = new SessionCodeChecks(realm, session.getContext().getUri(), request, clientConnection, session, event, authSessionId, null, null, clientId, tabId, null); @@ -234,12 +235,14 @@ public class LoginActionsService { flowPath = AUTHENTICATE_PATH; } - // See if we already have userSession attached to authentication session. This means restart of authentication session during re-authentication - // We logout userSession in this case - UserSessionModel userSession = new AuthenticationSessionManager(session).getUserSession(authSession); - if (userSession != null) { - logger.debugf("Logout of user session %s when restarting flow during re-authentication", userSession.getId()); - AuthenticationManager.backchannelLogout(session, userSession, false); + if (!Boolean.parseBoolean(skipLogout)) { + // See if we already have userSession attached to authentication session. This means restart of authentication session during re-authentication + // We logout userSession in this case + UserSessionModel userSession = new AuthenticationSessionManager(session).getUserSession(authSession); + if (userSession != null) { + logger.debugf("Logout of user session %s when restarting flow during re-authentication", userSession.getId()); + AuthenticationManager.backchannelLogout(session, userSession, false); + } } AuthenticationProcessor.resetFlow(authSession, flowPath); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AppPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AppPage.java index 49e0bda724..5b6e91e1e4 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AppPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/AppPage.java @@ -53,6 +53,10 @@ public class AppPage extends AbstractPage { clickLink(accountLink); } + public WebElement getAccountLink() { + return accountLink; + } + public enum RequestType { AUTH_RESPONSE, LOGOUT_REQUEST, APP_REQUEST } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultipleTabsLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultipleTabsLoginTest.java index 84d288b46a..8c706cb00e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultipleTabsLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/MultipleTabsLoginTest.java @@ -17,12 +17,13 @@ package org.keycloak.testsuite.forms; -import static org.junit.Assert.fail; +import static org.hamcrest.MatcherAssert.assertThat; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWith; import java.net.MalformedURLException; import java.net.URL; + import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.jboss.arquillian.graphene.page.Page; @@ -58,7 +59,8 @@ import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.UserBuilder; -import org.openqa.selenium.NoSuchElementException; +import org.keycloak.testsuite.util.WaitUtils; +import org.openqa.selenium.htmlunit.HtmlUnitDriver; /** * Tries to simulate testing with multiple browser tabs @@ -136,34 +138,43 @@ public class MultipleTabsLoginTest extends AbstractTestRealmKeycloakTest { @Test public void multipleTabsParallelLoginTest() { - oauth.openLoginForm(); - loginPage.assertCurrent(); + try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { + assertThat(tabUtil.getCountOfTabs(), Matchers.is(1)); + oauth.openLoginForm(); + loginPage.assertCurrent(); - loginPage.login("login-test", "password"); - updatePasswordPage.assertCurrent(); + loginPage.login("login-test", "password"); + updatePasswordPage.assertCurrent(); - String tab1Url = driver.getCurrentUrl(); + // Simulate login in different browser tab tab2. I will be on loginPage again. + tabUtil.newTab(oauth.getLoginFormUrl()); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(2)); - // Simulate login in different browser tab tab2. I will be on loginPage again. - oauth.openLoginForm(); - loginPage.assertCurrent(); + oauth.openLoginForm(); + loginPage.assertCurrent(); - // Login in tab2 - loginPage.login("login-test", "password"); - updatePasswordPage.assertCurrent(); + // Login in tab2 + loginPage.login("login-test", "password"); + updatePasswordPage.assertCurrent(); - updatePasswordPage.changePassword("password", "password"); - updateProfilePage.prepareUpdate().firstName("John").lastName("Doe3") - .email("john@doe3.com").submit(); - appPage.assertCurrent(); + updatePasswordPage.changePassword("password", "password"); + updateProfilePage.prepareUpdate().firstName("John").lastName("Doe3") + .email("john@doe3.com").submit(); + appPage.assertCurrent(); - // Try to go back to tab 1. We should have ALREADY_LOGGED_IN info page - driver.navigate().to(tab1Url); - infoPage.assertCurrent(); - Assert.assertEquals("You are already logged in.", infoPage.getInfo()); + // Try to go back to tab 1. We should be logged-in automatically + tabUtil.closeTab(1); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(1)); - infoPage.clickBackToApplicationLink(); - appPage.assertCurrent(); + // Should be back on tab1 + if (driver instanceof HtmlUnitDriver) { + driver.navigate().refresh(); // Need to explicitly refresh with HtmlUnitDriver due the authChecker.js javascript does not work + } + + // Should be back on tab1 and logged-in automatically here + WaitUtils.waitUntilElement(appPage.getAccountLink()).is().clickable(); + appPage.assertCurrent(); + } } @Test @@ -448,49 +459,56 @@ public class MultipleTabsLoginTest extends AbstractTestRealmKeycloakTest { // KEYCLOAK-12161 @Test public void testEmptyBaseUrl() throws Exception { - String clientUuid = KeycloakModelUtils.generateId(); - ClientRepresentation emptyBaseclient = ClientBuilder.create() - .clientId("empty-baseurl-client") - .id(clientUuid) - .enabled(true) - .baseUrl("") - .addRedirectUri("*") - .secret("password") - .build(); - testRealm().clients().create(emptyBaseclient); - getCleanup().addClientUuid(clientUuid); + try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { + assertThat(tabUtil.getCountOfTabs(), Matchers.is(1)); - oauth.clientId("empty-baseurl-client"); - oauth.openLoginForm(); - loginPage.assertCurrent(); + String clientUuid = KeycloakModelUtils.generateId(); + ClientRepresentation emptyBaseclient = ClientBuilder.create() + .clientId("empty-baseurl-client") + .id(clientUuid) + .enabled(true) + .baseUrl("") + .addRedirectUri("*") + .secret("password") + .build(); + testRealm().clients().create(emptyBaseclient); + getCleanup().addClientUuid(clientUuid); - loginPage.login("login-test", "password"); - updatePasswordPage.assertCurrent(); + oauth.clientId("empty-baseurl-client"); + oauth.openLoginForm(); + loginPage.assertCurrent(); - String tab1Url = driver.getCurrentUrl(); + loginPage.login("login-test", "password"); + updatePasswordPage.assertCurrent(); - // Simulate login in different browser tab tab2. I will be on loginPage again. - oauth.openLoginForm(); - loginPage.assertCurrent(); + String tab1Url = driver.getCurrentUrl(); - // Login in tab2 - loginPage.login("login-test", "password"); - updatePasswordPage.assertCurrent(); + // Simulate login in different browser tab tab2. I will be on loginPage again. + tabUtil.newTab(oauth.getLoginFormUrl()); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(2)); - updatePasswordPage.changePassword("password", "password"); - updateProfilePage.prepareUpdate().firstName("John").lastName("Doe3") - .email("john@doe3.com").submit(); - appPage.assertCurrent(); + loginPage.assertCurrent(); - // Try to go back to tab 1. We should have ALREADY_LOGGED_IN info page - driver.navigate().to(tab1Url); - infoPage.assertCurrent(); - Assert.assertEquals("You are already logged in.", infoPage.getInfo()); + // Login in tab2 + loginPage.login("login-test", "password"); + updatePasswordPage.assertCurrent(); - try { - infoPage.clickBackToApplicationLink(); - fail(); + updatePasswordPage.changePassword("password", "password"); + updateProfilePage.prepareUpdate().firstName("John").lastName("Doe3") + .email("john@doe3.com").submit(); + appPage.assertCurrent(); + + // Try to go back to tab 1. We should be logged-in automatically + tabUtil.closeTab(1); + assertThat(tabUtil.getCountOfTabs(), Matchers.equalTo(1)); + + if (driver instanceof HtmlUnitDriver) { + driver.navigate().refresh(); // Need to explicitly refresh with HtmlUnitDriver due the authChecker.js javascript does not work + } + + // Should be back on tab1 and logged-in automatically here + WaitUtils.waitUntilElement(appPage.getAccountLink()).is().clickable(); + appPage.assertCurrent(); } - catch (NoSuchElementException ex) {} } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index e87d5dfe84..217ddcbc3e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -21,8 +21,8 @@ import org.jboss.arquillian.drone.api.annotation.Drone; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.authentication.actiontoken.resetcred.ResetCredentialsActionToken; import org.jboss.arquillian.graphene.page.Page; -import org.keycloak.common.Profile; import org.keycloak.common.constants.ServiceAccountConstants; +import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; @@ -30,14 +30,13 @@ import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.Constants; import org.keycloak.models.utils.SystemClientUtil; import org.keycloak.protocol.oidc.utils.RedirectUtils; -import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.services.resources.LoginActionsService; import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.admin.ApiUtil; -import org.keycloak.testsuite.arquillian.annotation.DisableFeature; import org.keycloak.testsuite.arquillian.annotation.IgnoreBrowserDriver; import org.keycloak.testsuite.federation.kerberos.AbstractKerberosTest; import org.keycloak.testsuite.pages.AppPage; @@ -82,10 +81,8 @@ import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.firefox.FirefoxDriver; -import org.openqa.selenium.support.ui.ExpectedConditions; -import org.openqa.selenium.support.ui.WebDriverWait; +import org.openqa.selenium.htmlunit.HtmlUnitDriver; -import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.*; @@ -1143,34 +1140,6 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { assertEquals("Invalid username or password.", errorPage.getError()); } - @Test - public void resetPasswordLinkNewTabAndProperRedirectAccount() throws IOException { - final String REQUIRED_URI = getAuthServerRoot() + "realms/test/account/login-redirect?path=applications"; - final String REDIRECT_URI = getAuthServerRoot() + "realms/test/account/login-redirect?path=applications"; - final String CLIENT_ID = "account"; - final String ACCOUNT_MANAGEMENT_TITLE = "Keycloak Account Management"; - - try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { - assertThat(tabUtil.getCountOfTabs(), Matchers.is(1)); - - oauth.redirectUri(REDIRECT_URI); - oauth.clientId(CLIENT_ID); - - loginPage.open(); - resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, false, REDIRECT_URI, REQUIRED_URI); - assertThat(driver.getTitle(), Matchers.equalTo(ACCOUNT_MANAGEMENT_TITLE)); - - String logoutUrl = oauth.getLogoutUrl().build(); - driver.navigate().to(logoutUrl); - logoutConfirmPage.assertCurrent(); - logoutConfirmPage.confirmLogout(); - - loginPage.open(); - resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, true, REDIRECT_URI, REQUIRED_URI); - assertThat(driver.getTitle(), Matchers.equalTo(ACCOUNT_MANAGEMENT_TITLE)); - } - } - @Test public void resetPasswordLinkNewTabAndProperRedirectClient() throws IOException { final String REDIRECT_URI = getAuthServerRoot() + "realms/master/app/auth"; @@ -1184,7 +1153,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { assertThat(tabUtil.getCountOfTabs(), Matchers.is(1)); loginPage.open(); - resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, false, REDIRECT_URI); + resetPasswordInNewTab(defaultUser, CLIENT_ID, REDIRECT_URI); assertThat(driver.getCurrentUrl(), Matchers.containsString(REDIRECT_URI)); String logoutUrl = oauth.getLogoutUrl().build(); @@ -1193,7 +1162,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { logoutConfirmPage.confirmLogout(); loginPage.open(); - resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, true, REDIRECT_URI); + resetPasswordInNewTab(defaultUser, CLIENT_ID, REDIRECT_URI); assertThat(driver.getCurrentUrl(), Matchers.containsString(REDIRECT_URI)); } } @@ -1274,114 +1243,72 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { submit.click(); } - private void resetPasswordTwiceInNewTab(UserRepresentation user, String clientId, boolean shouldLogOut, String redirectUri) throws IOException { - resetPasswordTwiceInNewTab(user, clientId, shouldLogOut, redirectUri, redirectUri); - } + private void resetPasswordInNewTab(UserRepresentation user, String clientId, String redirectUri) throws IOException { + try (BrowserTabUtil browserTabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { + events.clear(); - private void resetPasswordTwiceInNewTab(UserRepresentation user, String clientId, boolean shouldLogOut, String redirectUri, String requiredUri) throws IOException { - events.clear(); - updateForgottenPassword(user, clientId, redirectUri, requiredUri); + final int emailCount = greenMail.getReceivedMessages().length; - if (shouldLogOut) { - String sessionId = events.expectLogin().user(user.getId()).detail(Details.USERNAME, user.getUsername()) + // In tab1 start "Forget password" flow and make sure the email is sent + loginPage.assertCurrent(); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + resetPasswordPage.changePassword(user.getUsername()); + WaitUtils.waitForPageToLoad(); + + assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage()); + + String tab1Url = driver.getCurrentUrl(); + + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD) + .user(user.getId()) + .client(clientId) + .detail(Details.REDIRECT_URI, redirectUri) + .detail(Details.USERNAME, user.getUsername()) + .detail(Details.EMAIL, user.getEmail()) + .session((String) null) + .assertEvent(); + + assertEquals(emailCount + 1, greenMail.getReceivedMessages().length); + + final MimeMessage message = greenMail.getReceivedMessages()[emailCount]; + final String changePasswordUrl = MailUtils.getPasswordResetEmailLink(message); + + // Open link from email in the new tab + browserTabUtil.newTab(changePasswordUrl.trim()); + + // Change password in tab2 + changePasswordOnUpdatePage(driver); + + events.expectRequiredAction(EventType.UPDATE_PASSWORD) .detail(Details.REDIRECT_URI, redirectUri) .client(clientId) - .assertEvent().getSessionId(); + .user(user.getId()).detail(Details.USERNAME, user.getUsername()).assertEvent(); - String logoutUrl = oauth.getLogoutUrl().build(); - driver.navigate().to(logoutUrl); - logoutConfirmPage.assertCurrent(); - logoutConfirmPage.confirmLogout(); + // User should be authenticated in current tab (tab2) + WaitUtils.waitUntilElement(appPage.getAccountLink()).is().clickable(); + appPage.assertCurrent(); + assertThat(driver.getCurrentUrl(), Matchers.containsString(redirectUri)); - events.expectLogout(sessionId) - .client("account") - .user(user.getId()) - .removeDetail(Details.REDIRECT_URI) - .assertEvent(); - } + // Close tab2 + assertThat(browserTabUtil.getCountOfTabs(), Matchers.equalTo(2)); + browserTabUtil.closeTab(1); + assertThat(browserTabUtil.getCountOfTabs(), Matchers.equalTo(1)); - BrowserTabUtil util = BrowserTabUtil.getInstanceAndSetEnv(driver); - assertThat(util.getCountOfTabs(), Matchers.equalTo(2)); - util.closeTab(1); - assertThat(util.getCountOfTabs(), Matchers.equalTo(1)); + if (driver instanceof HtmlUnitDriver) { + // With HtmlUnit, authChecker javascript doesn't work. Hence need to manually trigger "reset flow" endpoint + KeycloakUriBuilder builder = KeycloakUriBuilder.fromUri(tab1Url); + String resetFlowPath = builder + .replacePath(builder.getPath().substring(0, builder.getPath().lastIndexOf('/') + 1) + LoginActionsService.RESTART_PATH) + .queryParam(Constants.SKIP_LOGOUT, "true") + .build().toString(); + driver.navigate().to(resetFlowPath); + } - if (shouldLogOut) { - final ClientRepresentation client = testRealm().clients() - .findByClientId(clientId) - .stream() - .findFirst() - .orElse(null); - - assertThat(client, Matchers.notNullValue()); - updateForgottenPassword(user, clientId, getValidRedirectUriWithRootUrl(client.getRootUrl(), client.getRedirectUris())); - } else { - doForgotPassword(user.getUsername()); - } - } - - private void updateForgottenPassword(UserRepresentation user, String clientId, String redirectUri) throws IOException { - updateForgottenPassword(user, clientId, redirectUri, redirectUri); - } - - private void updateForgottenPassword(UserRepresentation user, String clientId, String redirectUri, String requiredUri) throws IOException { - final int emailCount = greenMail.getReceivedMessages().length; - - doForgotPassword(user.getUsername()); - assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage()); - - events.expectRequiredAction(EventType.SEND_RESET_PASSWORD) - .user(user.getId()) - .client(clientId) - .detail(Details.REDIRECT_URI, redirectUri) - .detail(Details.USERNAME, user.getUsername()) - .detail(Details.EMAIL, user.getEmail()) - .session((String) null) - .assertEvent(); - - assertEquals(emailCount + 1, greenMail.getReceivedMessages().length); - - final MimeMessage message = greenMail.getReceivedMessages()[emailCount]; - final String changePasswordUrl = MailUtils.getPasswordResetEmailLink(message); - - BrowserTabUtil util = BrowserTabUtil.getInstanceAndSetEnv(driver); - util.newTab(changePasswordUrl.trim()); - - changePasswordOnUpdatePage(driver); - - events.expectRequiredAction(EventType.UPDATE_PASSWORD) - .detail(Details.REDIRECT_URI, redirectUri) - .client(clientId) - .user(user.getId()).detail(Details.USERNAME, user.getUsername()).assertEvent(); - - assertThat(driver.getCurrentUrl(), Matchers.containsString(requiredUri)); - } - - private void doForgotPassword(String username) { - loginPage.assertCurrent(); - loginPage.resetPassword(); - - resetPasswordPage.assertCurrent(); - resetPasswordPage.changePassword(username); - WaitUtils.waitForPageToLoad(); - } - - private String getValidRedirectUriWithRootUrl(String rootUrl, Collection redirectUris) { - final boolean isRootUrlValid = isValidUrl(rootUrl); - - return redirectUris.stream() - .map(uri -> isRootUrlValid && uri.startsWith("/") ? rootUrl + uri : uri) - .map(uri -> uri.startsWith("/") ? OAuthClient.AUTH_SERVER_ROOT + uri : uri) - .map(RedirectUtils::validateRedirectUriWildcard) - .findFirst() - .orElse(null); - } - - private boolean isValidUrl(String url) { - try { - new URL(url); - return true; - } catch (MalformedURLException e) { - return false; + // User should be automatically authenticated in tab1 as well (due authChecker.js on real browsers like FF or Chrome) + WaitUtils.waitUntilElement(appPage.getAccountLink()).is().clickable(); + appPage.assertCurrent(); } } } diff --git a/themes/src/main/resources/theme/base/login/resources/js/authChecker.js b/themes/src/main/resources/theme/base/login/resources/js/authChecker.js new file mode 100644 index 0000000000..e7273249bb --- /dev/null +++ b/themes/src/main/resources/theme/base/login/resources/js/authChecker.js @@ -0,0 +1,75 @@ +const CHECK_INTERVAL_MILLISECS = 2000; +const initialSession = getSession(); + +export function checkCookiesAndSetTimer(authSessionId, tabId, loginRestartUrl) { + if (initialSession) { + // We started with a session, so there is nothing to do, exit. + return; + } + + const session = getSession(); + + if (!session) { + // The session is not present, check again later. + setTimeout( + () => checkCookiesAndSetTimer(authSessionId, tabId, loginRestartUrl), + CHECK_INTERVAL_MILLISECS + ); + } else { + // The session is present, check the auth state. + checkAuthState(authSessionId, tabId, loginRestartUrl); + } +} + +function checkAuthState(authSessionId, tabId, loginRestartUrl) { + const authStateRaw = getAuthState(); + + if (!authStateRaw) { + // The auth state is not present, exit. + return; + } + + // Attempt to parse the auth state as JSON. + let authState; + try { + authState = JSON.parse(authStateRaw); + } catch (error) { + // The auth state is not valid JSON, exit. + return; + } + + if (authState.authSessionId !== authSessionId) { + // The session ID does not match, exit. + return; + } + + if ( + !Array.isArray(authState.remainingTabs) || + !authState.remainingTabs.includes(tabId) + ) { + // The remaining tabs don't include the provided tab ID, exit. + return; + } + + // We made it this far, redirect to the login restart URL. + location.href = loginRestartUrl; +} + +function getSession() { + return getCookieByName("KEYCLOAK_SESSION"); +} + +function getAuthState() { + return getCookieByName("KC_AUTH_STATE"); +} + +function getCookieByName(name) { + const cookies = new Map(); + + for (const cookie of document.cookie.split(";")) { + const [key, value] = cookie.split("=").map((value) => value.trim()); + cookies.set(key, value); + } + + return cookies.get(name) ?? null; +} diff --git a/themes/src/main/resources/theme/base/login/template.ftl b/themes/src/main/resources/theme/base/login/template.ftl index f4382cfcce..188cea5960 100644 --- a/themes/src/main/resources/theme/base/login/template.ftl +++ b/themes/src/main/resources/theme/base/login/template.ftl @@ -34,6 +34,17 @@ + <#if authenticationSession??> + +