diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AbstractInitiateLogin.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AbstractInitiateLogin.java index 248f4c30eb8..adaecdcba0b 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AbstractInitiateLogin.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AbstractInitiateLogin.java @@ -40,10 +40,16 @@ public abstract class AbstractInitiateLogin implements AuthChallenge { protected SamlDeployment deployment; protected SamlSessionStore sessionStore; + protected boolean saveRequestUri; public AbstractInitiateLogin(SamlDeployment deployment, SamlSessionStore sessionStore) { + this(deployment, sessionStore, true); + } + + public AbstractInitiateLogin(SamlDeployment deployment, SamlSessionStore sessionStore, boolean saveRequestUri) { this.deployment = deployment; this.sessionStore = sessionStore; + this.saveRequestUri = saveRequestUri; } @Override @@ -56,7 +62,9 @@ public abstract class AbstractInitiateLogin implements AuthChallenge { try { SAML2AuthnRequestBuilder authnRequestBuilder = buildSaml2AuthnRequestBuilder(deployment); BaseSAML2BindingBuilder binding = createSaml2Binding(deployment); - sessionStore.saveRequest(); + if (saveRequestUri) { + sessionStore.saveRequest(); + } sendAuthnRequest(httpFacade, authnRequestBuilder, binding); sessionStore.setCurrentAction(SamlSessionStore.CurrentAction.LOGGING_IN); diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AdapterConstants.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AdapterConstants.java index 3646ed45541..9901bf23bcf 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AdapterConstants.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AdapterConstants.java @@ -25,4 +25,5 @@ public class AdapterConstants { public static final String AUTH_DATA_PARAM_NAME="org.keycloak.saml.xml.adapterConfig"; public static final String REPLICATION_CONFIG_CONTAINER_PARAM_NAME = "org.keycloak.saml.replication.container"; public static final String REPLICATION_CONFIG_SSO_CACHE_PARAM_NAME = "org.keycloak.saml.replication.cache.sso"; + public static final String AUTHENTICATION_EXPIRED_MESSAGE = "authentication_expired"; } diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java index ab5463473cb..bfcb8997d80 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java @@ -32,6 +32,7 @@ import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; import org.jboss.logging.Logger; import org.keycloak.adapters.saml.AbstractInitiateLogin; +import org.keycloak.adapters.saml.AdapterConstants; import org.keycloak.adapters.saml.OnSessionCreated; import org.keycloak.adapters.saml.SamlAuthenticationError; import org.keycloak.adapters.saml.SamlDeployment; @@ -148,7 +149,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic log.debug("AUTHENTICATED: was cached"); return handleRequest(); } - return initiateLogin(); + return initiateLogin(true); } protected AuthOutcome handleRequest() { @@ -361,7 +362,10 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic final ResponseType responseType = (ResponseType) responseHolder.getSamlObject(); AssertionType assertion = null; - if (! isSuccessfulSamlResponse(responseType) || responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) { + if (isRetrayableSamlResponse(responseType)) { + // initiate the login but do not save the request cos it's /saml + return initiateLogin(false); + } else if (!isSuccessfulSamlResponse(responseType) || responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) { return failed(createAuthChallenge403(responseType)); } try { @@ -378,7 +382,8 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic // warning has been already emitted in DeploymentBuilder } if (! cvb.build().isValid()) { - return initiateLogin(); + // initiate the login but do not save the request cos it's /saml + return initiateLogin(false); } } catch (Exception e) { log.error("Error extracting SAML assertion: " + e.getMessage()); @@ -523,6 +528,21 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic && Objects.equals(responseType.getStatus().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_SUCCESS.get()); } + private boolean isRetrayableSamlResponse(ResponseType responseType) { + if (responseType == null || responseType.getStatus() == null) { + return false; + } + + StatusType status = responseType.getStatus(); + return status.getStatusCode() != null + && AdapterConstants.AUTHENTICATION_EXPIRED_MESSAGE.equals(status.getStatusMessage()) + && status.getStatusCode().getValue() != null + && Objects.equals(status.getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_RESPONDER.get()) + && status.getStatusCode().getStatusCode() != null + && status.getStatusCode().getStatusCode().getValue() != null + && Objects.equals(status.getStatusCode().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_AUTHNFAILED.get()); + } + private Element getAssertionFromResponse(final SAMLDocumentHolder responseHolder) throws ConfigurationException, ProcessingException { Element encryptedAssertion = DocumentUtil.getElement(responseHolder.getSamlDocument(), new QName(JBossSAMLConstants.ENCRYPTED_ASSERTION.get())); if (encryptedAssertion != null) { @@ -601,13 +621,13 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic } - protected AuthOutcome initiateLogin() { - challenge = createChallenge(); + protected AuthOutcome initiateLogin(boolean saveRequestUri) { + challenge = createChallenge(saveRequestUri); return AuthOutcome.NOT_ATTEMPTED; } - protected AbstractInitiateLogin createChallenge() { - return new AbstractInitiateLogin(deployment, sessionStore) { + protected AbstractInitiateLogin createChallenge(boolean saveRequestUri) { + return new AbstractInitiateLogin(deployment, sessionStore, saveRequestUri) { @Override protected void sendAuthnRequest(HttpFacade httpFacade, SAML2AuthnRequestBuilder authnRequestBuilder, BaseSAML2BindingBuilder binding) throws ProcessingException, ConfigurationException, IOException { if (isAutodetectedBearerOnly(httpFacade.getRequest())) { diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/ecp/EcpAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/ecp/EcpAuthenticationHandler.java index 7f40e5aed3c..375e2625cec 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/ecp/EcpAuthenticationHandler.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/ecp/EcpAuthenticationHandler.java @@ -105,8 +105,8 @@ public class EcpAuthenticationHandler extends AbstractSamlAuthenticationHandler } @Override - protected AbstractInitiateLogin createChallenge() { - return new AbstractInitiateLogin(deployment, sessionStore) { + protected AbstractInitiateLogin createChallenge(boolean saveChallenge) { + return new AbstractInitiateLogin(deployment, sessionStore, saveChallenge) { @Override protected void sendAuthnRequest(HttpFacade httpFacade, SAML2AuthnRequestBuilder authnRequestBuilder, BaseSAML2BindingBuilder binding) { try { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java index 827a929e8ed..fb7f92ac886 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java @@ -117,6 +117,7 @@ import org.keycloak.admin.client.resource.RoleScopeResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.util.Base64; import org.keycloak.common.util.KeyUtils; +import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.common.util.PemUtils; import org.keycloak.cookie.CookieType; @@ -129,6 +130,7 @@ import org.keycloak.keys.Attributes; import org.keycloak.keys.ImportedRsaKeyProviderFactory; import org.keycloak.keys.KeyProvider; import org.keycloak.models.Constants; +import org.keycloak.models.RealmModel; import org.keycloak.protocol.saml.SamlConfigAttributes; import org.keycloak.protocol.saml.SamlProtocol; import org.keycloak.representations.idm.ClientRepresentation; @@ -146,6 +148,7 @@ import org.keycloak.saml.processing.core.parsers.saml.SAMLParser; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil; import org.keycloak.services.resources.RealmsResource; +import org.keycloak.sessions.RootAuthenticationSessionModel; import org.keycloak.testsuite.adapter.page.*; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; @@ -164,15 +167,19 @@ import org.keycloak.testsuite.updaters.ClientAttributeUpdater; import org.keycloak.testsuite.updaters.Creator; import org.keycloak.testsuite.updaters.UserAttributeUpdater; import org.keycloak.testsuite.util.AdminClientUtil; +import org.keycloak.testsuite.util.BrowserTabUtil; +import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.SamlClient; import org.keycloak.testsuite.util.SamlClient.Binding; import org.keycloak.testsuite.util.SamlClientBuilder; +import org.keycloak.testsuite.util.UIUtils; import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.testsuite.utils.io.IOUtil; import org.openqa.selenium.By; import org.openqa.selenium.Cookie; +import org.openqa.selenium.htmlunit.HtmlUnitDriver; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -673,10 +680,14 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { } } - private static final KeyPair NEW_KEY_PAIR = KeyUtils.generateRsaKeyPair(org.keycloak.testsuite.util.KeyUtils.getLowestSupportedRsaKeySize()); - private static final String NEW_KEY_PRIVATE_KEY_PEM = PemUtils.encodeKey(NEW_KEY_PAIR.getPrivate()); + private static KeyPair NEW_KEY_PAIR; + private static String NEW_KEY_PRIVATE_KEY_PEM; private PublicKey createKeys(String priority) throws Exception { + if (NEW_KEY_PAIR == null) { + NEW_KEY_PAIR = KeyUtils.generateRsaKeyPair(org.keycloak.testsuite.util.KeyUtils.getLowestSupportedRsaKeySize()); + NEW_KEY_PRIVATE_KEY_PEM = PemUtils.encodeKey(NEW_KEY_PAIR.getPrivate()); + } PublicKey publicKey = NEW_KEY_PAIR.getPublic(); ComponentRepresentation rep = new ComponentRepresentation(); @@ -1827,6 +1838,93 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { checkLoggedOut(salesPostSigEmailServletPage, testRealmSAMLPostLoginPage); } + @Test + public void testMultipleTabsParallelLogin() throws Exception { + try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { + // open an application in tab1 and go to the login page + Assert.assertEquals(1, tabUtil.getCountOfTabs()); + salesPostServletPage.navigateTo(); + waitForPageToLoad(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + + // Prepare a login in tab2 + tabUtil.newTab(salesPostServletPage.buildUri().toASCIIString()); + waitForPageToLoad(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + Assert.assertEquals(2, tabUtil.getCountOfTabs()); + testRealmSAMLPostLoginPage.form().login(bburkeUser); + waitUntilElement(By.xpath("//body")).text().contains("principal=bburke"); + + // Go back to tab1 and it should automatically login + tabUtil.closeTab(1); + Assert.assertEquals(1, tabUtil.getCountOfTabs()); + if (driver instanceof HtmlUnitDriver) { + // go to restart URI manually as JS does not work + KeycloakUriBuilder current = KeycloakUriBuilder.fromUri(driver.getCurrentUrl(), false); + KeycloakUriBuilder restart = KeycloakUriBuilder.fromUri(OAuthClient.AUTH_SERVER_ROOT + "/realms/" + DEMO + "/login-actions/restart", false) + .replaceQuery(current.getQuery(), false) + .queryParam(Constants.SKIP_LOGOUT, Boolean.TRUE.toString()); + driver.navigate().to(restart.buildAsString()); + } + waitUntilElement(By.xpath("//body")).text().contains("principal=bburke"); + } finally { + salesPostServletPage.logout(); + } + } + + @Test + public void testMultipleTabsParallelLoginAfterAuthSessionExpiration() throws Exception { + try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) { + // open an application in tab1 and go to the login page + Assert.assertEquals(1, tabUtil.getCountOfTabs()); + salesPostServletPage.navigateTo(); + waitForPageToLoad(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + + // Prepare a login in tab2 + tabUtil.newTab(salesPostServletPage.buildUri().toASCIIString()); + waitForPageToLoad(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + Assert.assertEquals(2, tabUtil.getCountOfTabs()); + + // remove the authentication session in the server to simulate expiration + Cookie sessionCookie = driver.manage().getCookieNamed(CookieType.AUTH_SESSION_ID.getName()); + Assert.assertNotNull(sessionCookie); + final String authSessionId = sessionCookie.getValue(); + testingClient.server().run(session -> { + RealmModel realm = session.realms().getRealmByName(DEMO); + RootAuthenticationSessionModel root = session.authenticationSessions().getRootAuthenticationSession(realm, authSessionId); + session.authenticationSessions().removeRootAuthenticationSession(realm, root); + }); + + // finish the login that should fail + testRealmSAMLPostLoginPage.form().login(bburkeUser); + waitForPageToLoad(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); // we are still in login + Assert.assertEquals("Your login attempt timed out. Login will start from the beginning.", + UIUtils.getTextFromElement(driver.findElement(By.className("alert-error")))); + + // login successfully in tab2 after the error + loginPage.form().login(bburkeUser); + waitUntilElement(By.xpath("//body")).text().contains("principal=bburke"); + + // Go back to tab1 and it should automatically log into the app with retry + tabUtil.closeTab(1); + Assert.assertEquals(1, tabUtil.getCountOfTabs()); + if (driver instanceof HtmlUnitDriver) { + // go to restart URI manually as JS does not work + KeycloakUriBuilder current = KeycloakUriBuilder.fromUri(driver.getCurrentUrl(), false); + KeycloakUriBuilder restart = KeycloakUriBuilder.fromUri(OAuthClient.AUTH_SERVER_ROOT + "/realms/" + DEMO + "/login-actions/restart", false) + .replaceQuery(current.getQuery(), false) + .queryParam(Constants.SKIP_LOGOUT, Boolean.TRUE.toString()); + driver.navigate().to(restart.buildAsString()); + } + waitUntilElement(By.xpath("//body")).text().contains("principal=bburke"); + } finally { + salesPostServletPage.logout(); + } + } + private List impersonate(String admin, String adminPassword, String userId) throws IOException { ResteasyClientBuilder resteasyClientBuilder = (ResteasyClientBuilder) ResteasyClientBuilder.newBuilder(); resteasyClientBuilder.connectionPoolSize(10);