From e3060e5e58dc4f469a564d2703fd8b45be68bcca Mon Sep 17 00:00:00 2001 From: Vlastimil Elias Date: Wed, 25 Nov 2015 13:46:29 +0100 Subject: [PATCH] rebased to latest master --- .../adapters/OIDCAuthenticationError.java | 7 +++++ .../saml/SamlAuthenticationError.java | 6 ++++ .../adapters/saml/SamlAuthenticator.java | 28 ++++++------------- .../saml/SAML2ErrorResponseBuilder.java | 10 ++----- .../keycloaksaml/SamlAdapterTestStrategy.java | 5 ++-- .../keycloak/testsuite/rule/ErrorServlet.java | 10 +++++-- 6 files changed, 36 insertions(+), 30 deletions(-) diff --git a/integration/adapter-core/src/main/java/org/keycloak/adapters/OIDCAuthenticationError.java b/integration/adapter-core/src/main/java/org/keycloak/adapters/OIDCAuthenticationError.java index 5b3f45d67e..089b689993 100755 --- a/integration/adapter-core/src/main/java/org/keycloak/adapters/OIDCAuthenticationError.java +++ b/integration/adapter-core/src/main/java/org/keycloak/adapters/OIDCAuthenticationError.java @@ -36,4 +36,11 @@ public class OIDCAuthenticationError implements AuthenticationError { public String getDescription() { return description; } + + @Override + public String toString() { + return "OIDCAuthenticationError [reason=" + reason + ", description=" + description + "]"; + } + + } diff --git a/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticationError.java b/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticationError.java index 8b631031cb..c85fd63ef3 100755 --- a/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticationError.java +++ b/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticationError.java @@ -40,4 +40,10 @@ public class SamlAuthenticationError implements AuthenticationError { public StatusResponseType getStatus() { return status; } + + @Override + public String toString() { + return "SamlAuthenticationError [reason=" + reason + ", status=" + status + "]"; + } + } diff --git a/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticator.java b/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticator.java index 919ec35b1a..13f52de169 100755 --- a/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticator.java +++ b/saml/client-adapter/core/src/main/java/org/keycloak/adapters/saml/SamlAuthenticator.java @@ -211,25 +211,7 @@ public abstract class SamlAuthenticator { return AuthOutcome.FAILED; } - if (statusResponse instanceof ResponseType) { - - //validate status - StatusType status = statusResponse.getStatus(); - if(status == null){ - log.error("Missing Status in SAML response"); - return AuthOutcome.FAILED; - } - if(!checkStatusCodeValue(status.getStatusCode(), JBossSAMLURIConstants.STATUS_SUCCESS.get())){ - if(checkStatusCodeValue(status.getStatusCode(), JBossSAMLURIConstants.STATUS_RESPONDER.get()) && checkStatusCodeValue(status.getStatusCode().getStatusCode(), JBossSAMLURIConstants.STATUS_NO_PASSIVE.get())){ - // KEYCLOAK-2107 - handle user not authenticated due passive mode - log.debug("Not authenticated due passive mode Status found in SAML response: " + status.toString()); - return AuthOutcome.NOT_AUTHENTICATED; - } - log.error("Error Status found in SAML response: " + status.toString()); - return AuthOutcome.FAILED; - - } - + if (statusResponse instanceof ResponseType) { try { if (deployment.getIDP().getSingleSignOnService().validateResponseSignature()) { try { @@ -276,7 +258,15 @@ public abstract class SamlAuthenticator { } } else if (sessionStore.isLoggingIn()) { + try { + // KEYCLOAK-2107 - handle user not authenticated due passive mode. Return special outcome so different authentication mechanisms can behave accordingly. + StatusType status = statusResponse.getStatus(); + if(checkStatusCodeValue(status.getStatusCode(), JBossSAMLURIConstants.STATUS_RESPONDER.get()) && checkStatusCodeValue(status.getStatusCode().getStatusCode(), JBossSAMLURIConstants.STATUS_NO_PASSIVE.get())){ + log.debug("Not authenticated due passive mode Status found in SAML response: " + status.toString()); + return AuthOutcome.NOT_AUTHENTICATED; + } + challenge = new AuthChallenge() { @Override public boolean challenge(HttpFacade exchange) { diff --git a/saml/saml-core/src/main/java/org/keycloak/saml/SAML2ErrorResponseBuilder.java b/saml/saml-core/src/main/java/org/keycloak/saml/SAML2ErrorResponseBuilder.java index a3d080052a..237365673b 100755 --- a/saml/saml-core/src/main/java/org/keycloak/saml/SAML2ErrorResponseBuilder.java +++ b/saml/saml-core/src/main/java/org/keycloak/saml/SAML2ErrorResponseBuilder.java @@ -1,28 +1,21 @@ package org.keycloak.saml; -<<<<<<< Upstream, based on keycloak/master import org.keycloak.dom.saml.v2.assertion.NameIDType; import org.keycloak.dom.saml.v2.protocol.StatusCodeType; import org.keycloak.dom.saml.v2.protocol.StatusResponseType; import org.keycloak.dom.saml.v2.protocol.StatusType; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; -======= -import org.keycloak.dom.saml.v2.protocol.ResponseType; ->>>>>>> 9408d08 KEYCLOAK-2107 - support IsPassive mode in SAML SP adapter library KEYCLOAK-2075 - added integration tests for both server and adapter side import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.common.exceptions.ParsingException; import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.processing.api.saml.v2.response.SAML2Response; import org.keycloak.saml.processing.core.saml.v2.common.IDGenerator; import org.keycloak.saml.processing.core.saml.v2.factories.JBossSAMLAuthnResponseFactory; -<<<<<<< Upstream, based on keycloak/master import org.keycloak.saml.processing.core.saml.v2.holders.IDPInfoHolder; import org.keycloak.saml.processing.core.saml.v2.holders.IssuerInfoHolder; import org.keycloak.saml.processing.core.saml.v2.holders.SPInfoHolder; import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; -======= ->>>>>>> 9408d08 KEYCLOAK-2107 - support IsPassive mode in SAML SP adapter library KEYCLOAK-2075 - added integration tests for both server and adapter side import org.w3c.dom.Document; import java.net.URI; @@ -52,6 +45,7 @@ public class SAML2ErrorResponseBuilder { return this; } + public Document buildDocument() throws ProcessingException { try { @@ -71,6 +65,8 @@ public class SAML2ErrorResponseBuilder { } catch (ParsingException e) { throw new ProcessingException(e); } + } + } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java index 383c3fc7a4..f2d8b68045 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTestStrategy.java @@ -140,12 +140,13 @@ public class SamlAdapterTestStrategy extends ExternalResource { } public void testPostPassiveLoginLogout(boolean forbiddenIfNotauthenticated) { - // first request on passive app - no login page shown, user not logged in as we are in passive mode + // first request on passive app - no login page shown, user not logged in as we are in passive mode. + // Shown page depends on used authentication mechanism, some may return forbidden error, some return requested page with anonymous user (not logged in) driver.navigate().to(APP_SERVER_BASE_URL + "/sales-post-passive/"); assertEquals(APP_SERVER_BASE_URL + "/sales-post-passive/", driver.getCurrentUrl()); System.out.println(driver.getPageSource()); if (forbiddenIfNotauthenticated) { - Assert.assertTrue(driver.getPageSource().contains("Forbidden")); + Assert.assertTrue(driver.getPageSource().contains("HTTP status code: 403")); } else { Assert.assertTrue(driver.getPageSource().contains("principal=null")); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/ErrorServlet.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/ErrorServlet.java index 68410d48bf..0cffa3c249 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/ErrorServlet.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/rule/ErrorServlet.java @@ -6,7 +6,6 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import java.io.IOException; import java.io.PrintWriter; @@ -20,10 +19,17 @@ public class ErrorServlet extends HttpServlet { protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { authError = (AuthenticationError)req.getAttribute(AuthenticationError.class.getName()); + Integer statusCode = (Integer) req.getAttribute("javax.servlet.error.status_code"); + resp.setContentType("text/html"); PrintWriter pw = resp.getWriter(); pw.printf("%s", "Error Page"); - pw.print("

There was an error

"); + pw.print("

There was an error

"); + if (statusCode != null) + pw.print("
HTTP status code: " + statusCode); + if (authError != null) + pw.print("
Error info: " + authError.toString()); + pw.print(""); pw.flush();