keycloak-aplcache
Changes
adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java 38(+31 -7)
testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTest.java 8(+6 -2)
Details
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 1bbdf6d..cb9b4d9 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
@@ -60,6 +60,7 @@ import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
import org.keycloak.saml.processing.web.util.PostBindingUtil;
import org.w3c.dom.Document;
+import org.w3c.dom.Element;
import org.w3c.dom.Node;
import java.io.IOException;
@@ -70,14 +71,11 @@ import java.security.KeyManagementException;
import java.security.PublicKey;
import java.security.Signature;
import java.security.SignatureException;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
import org.keycloak.dom.saml.v2.SAML2Object;
import org.keycloak.dom.saml.v2.protocol.ExtensionsType;
import org.keycloak.rotation.KeyLocator;
import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
-import org.w3c.dom.Element;
/**
*
@@ -196,7 +194,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
challenge = new AuthChallenge() {
@Override
public boolean challenge(HttpFacade exchange) {
- SamlAuthenticationError error = new SamlAuthenticationError(SamlAuthenticationError.Reason.INVALID_SIGNATURE);
+ SamlAuthenticationError error = new SamlAuthenticationError(SamlAuthenticationError.Reason.INVALID_SIGNATURE, statusResponse);
exchange.getRequest().setError(error);
exchange.getResponse().sendError(403);
return true;
@@ -312,8 +310,25 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
return false;
}
- protected AuthOutcome handleLoginResponse(ResponseType responseType, boolean postBinding, OnSessionCreated onCreateSession) {
+ protected AuthOutcome handleLoginResponse(final ResponseType responseType, boolean postBinding, OnSessionCreated onCreateSession) {
AssertionType assertion = null;
+ if (! isSuccessfulSamlResponse(responseType) || responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {
+ challenge = new AuthChallenge() {
+ @Override
+ public boolean challenge(HttpFacade exchange) {
+ SamlAuthenticationError error = new SamlAuthenticationError(SamlAuthenticationError.Reason.ERROR_STATUS, responseType);
+ exchange.getRequest().setError(error);
+ exchange.getResponse().sendError(403);
+ return true;
+ }
+
+ @Override
+ public int getResponseCode() {
+ return 403;
+ }
+ };
+ return AuthOutcome.FAILED;
+ }
try {
assertion = AssertionUtil.getAssertion(responseType, deployment.getDecryptionKey());
if (AssertionUtil.hasExpired(assertion)) {
@@ -335,6 +350,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
return 403;
}
};
+ return AuthOutcome.FAILED;
}
if (deployment.getIDP().getSingleSignOnService().validateAssertionSignature()) {
@@ -346,7 +362,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
challenge = new AuthChallenge() {
@Override
public boolean challenge(HttpFacade exchange) {
- SamlAuthenticationError error = new SamlAuthenticationError(SamlAuthenticationError.Reason.INVALID_SIGNATURE);
+ SamlAuthenticationError error = new SamlAuthenticationError(SamlAuthenticationError.Reason.INVALID_SIGNATURE, responseType);
exchange.getRequest().setError(error);
exchange.getResponse().sendError(403);
return true;
@@ -449,6 +465,14 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
return AuthOutcome.AUTHENTICATED;
}
+ private boolean isSuccessfulSamlResponse(ResponseType responseType) {
+ return responseType != null
+ && responseType.getStatus() != null
+ && responseType.getStatus().getStatusCode() != null
+ && responseType.getStatus().getStatusCode().getValue() != null
+ && Objects.equals(responseType.getStatus().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_SUCCESS.get());
+ }
+
private String getAttributeValue(Object attrValue) {
String value = null;
if (attrValue instanceof String) {
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTest.java
index c4e2a36..d0c770e 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/keycloaksaml/SamlAdapterTest.java
@@ -117,8 +117,12 @@ public class SamlAdapterTest {
testStrategy.testSavedPostRequest();
}
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
public void testMetadataPostSignedLoginLogout() throws Exception {
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 9589fe9..91074e3 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
@@ -20,10 +20,12 @@ package org.keycloak.testsuite.keycloaksaml;
import org.apache.commons.io.IOUtils;
import org.junit.Assert;
import org.junit.rules.ExternalResource;
+
import org.keycloak.adapters.saml.SamlAuthenticationError;
import org.keycloak.adapters.saml.SamlPrincipal;
import org.keycloak.admin.client.Keycloak;
import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.common.util.*;
import org.keycloak.models.ClientModel;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
@@ -38,9 +40,8 @@ import org.keycloak.protocol.saml.mappers.RoleNameMapper;
import org.keycloak.protocol.saml.mappers.UserAttributeStatementMapper;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
-import org.keycloak.saml.BaseSAML2BindingBuilder;
-import org.keycloak.saml.SAML2ErrorResponseBuilder;
-import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
+import org.keycloak.saml.*;
+import org.keycloak.saml.common.constants.*;
import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants;
import org.keycloak.services.managers.RealmManager;
import org.keycloak.testsuite.KeycloakServer;
@@ -51,6 +52,7 @@ import org.keycloak.testsuite.rule.ErrorServlet;
import org.keycloak.testsuite.rule.KeycloakRule;
import org.keycloak.testsuite.rule.WebResource;
import org.keycloak.testsuite.rule.WebRule;
+
import org.openqa.selenium.WebDriver;
import org.w3c.dom.Document;
@@ -61,10 +63,11 @@ import javax.ws.rs.core.Form;
import javax.ws.rs.core.Response;
import java.io.IOException;
import java.net.URI;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Set;
+import java.security.*;
+import java.security.spec.*;
+import java.util.*;
+import java.util.Base64;
+import java.util.logging.*;
import static org.junit.Assert.assertEquals;
@@ -77,6 +80,24 @@ public class SamlAdapterTestStrategy extends ExternalResource {
protected String APP_SERVER_BASE_URL = "http://localhost:8081";
protected AbstractKeycloakRule keycloakRule;
+ private static final String REALM_PRIVATE_KEY_STR = "MIICXAIBAAKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQABAoGAfmO8gVhyBxdqlxmIuglbz8bcjQbhXJLR2EoS8ngTXmN1bo2L90M0mUKSdc7qF10LgETBzqL8jYlQIbt+e6TH8fcEpKCjUlyq0Mf/vVbfZSNaVycY13nTzo27iPyWQHK5NLuJzn1xvxxrUeXI6A2WFpGEBLbHjwpx5WQG9A+2scECQQDvdn9NE75HPTVPxBqsEd2z10TKkl9CZxu10Qby3iQQmWLEJ9LNmy3acvKrE3gMiYNWb6xHPKiIqOR1as7L24aTAkEAtyvQOlCvr5kAjVqrEKXalj0Tzewjweuxc0pskvArTI2Oo070h65GpoIKLc9jf+UA69cRtquwP93aZKtW06U8dQJAF2Y44ks/mK5+eyDqik3koCI08qaC8HYq2wVl7G2QkJ6sbAaILtcvD92ToOvyGyeE0flvmDZxMYlvaZnaQ0lcSQJBAKZU6umJi3/xeEbkJqMfeLclD27XGEFoPeNrmdx0q10Azp4NfJAY+Z8KRyQCR2BEG+oNitBOZ+YXF9KCpH3cdmECQHEigJhYg+ykOvr1aiZUMFT72HU0jnmQe2FVekuG+LJUt2Tm7GtMjTFoGpf0JwrVuZN39fOYAlo+nTixgeW7X8Y=";
+ private static PrivateKey REALM_PRIVATE_KEY;
+ private static final String REALM_PUBLIC_KEY_STR = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB";
+ private static PublicKey REALM_PUBLIC_KEY;
+
+ static {
+ try {
+ KeyFactory kf = KeyFactory.getInstance("RSA");
+ byte[] encoded = Base64.getDecoder().decode(REALM_PUBLIC_KEY_STR);
+ REALM_PUBLIC_KEY = (PublicKey) kf.generatePublic(new X509EncodedKeySpec(encoded));
+
+ encoded = Base64.getDecoder().decode(REALM_PRIVATE_KEY_STR);
+ REALM_PRIVATE_KEY = (PrivateKey) kf.generatePrivate(new PKCS8EncodedKeySpec(encoded));
+ } catch (NoSuchAlgorithmException | InvalidKeySpecException ex) {
+ Logger.getLogger(SamlAdapterTestStrategy.class.getName()).log(Level.SEVERE, null, ex);
+ }
+ }
+
public SamlAdapterTestStrategy(String AUTH_SERVER_URL, String APP_SERVER_BASE_URL, AbstractKeycloakRule keycloakRule) {
this.AUTH_SERVER_URL = AUTH_SERVER_URL;
this.APP_SERVER_BASE_URL = APP_SERVER_BASE_URL;
@@ -173,7 +194,7 @@ public class SamlAdapterTestStrategy extends ExternalResource {
- public void testErrorHandling() throws Exception {
+ public void testErrorHandlingUnsigned() throws Exception {
ErrorServlet.authError = null;
Client client = ClientBuilder.newClient();
// make sure
@@ -194,6 +215,36 @@ public class SamlAdapterTestStrategy extends ExternalResource {
client.close();
Assert.assertNotNull(ErrorServlet.authError);
SamlAuthenticationError error = (SamlAuthenticationError)ErrorServlet.authError;
+ Assert.assertEquals(SamlAuthenticationError.Reason.INVALID_SIGNATURE, error.getReason());
+ Assert.assertNotNull(error.getStatus());
+ ErrorServlet.authError = null;
+
+ }
+
+ public void testErrorHandlingSigned() throws Exception {
+ ErrorServlet.authError = null;
+ Client client = ClientBuilder.newClient();
+ // make sure
+ Response response = client.target(APP_SERVER_BASE_URL + "/employee-sig/").request().get();
+ response.close();
+ SAML2ErrorResponseBuilder builder = new SAML2ErrorResponseBuilder()
+ .destination(APP_SERVER_BASE_URL + "/employee-sig/saml")
+ .issuer(AUTH_SERVER_URL + "/realms/demo")
+ .status(JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get());
+ BaseSAML2BindingBuilder binding = new BaseSAML2BindingBuilder()
+ .relayState(null)
+ .signatureAlgorithm(SignatureAlgorithm.RSA_SHA256)
+ .signWith(KeyUtils.createKeyId(REALM_PRIVATE_KEY), REALM_PRIVATE_KEY, REALM_PUBLIC_KEY)
+ .signDocument();
+ Document document = builder.buildDocument();
+ URI uri = binding.generateRedirectUri(GeneralConstants.SAML_RESPONSE_KEY, APP_SERVER_BASE_URL + "/employee-sig/saml", document);
+ response = client.target(uri).request().get();
+ String errorPage = response.readEntity(String.class);
+ response.close();
+ Assert.assertTrue(errorPage.contains("Error Page"));
+ client.close();
+ Assert.assertNotNull(ErrorServlet.authError);
+ SamlAuthenticationError error = (SamlAuthenticationError)ErrorServlet.authError;
Assert.assertEquals(SamlAuthenticationError.Reason.ERROR_STATUS, error.getReason());
Assert.assertNotNull(error.getStatus());
ErrorServlet.authError = null;
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/samlfilter/SamlAdapterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/samlfilter/SamlAdapterTest.java
index acc27a6..b3cdec4 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/samlfilter/SamlAdapterTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/samlfilter/SamlAdapterTest.java
@@ -128,7 +128,7 @@ public class SamlAdapterTest {
@Test
public void testPostPassiveLoginLogout() {
- testStrategy.testPostPassiveLoginLogout(false);
+ testStrategy.testPostPassiveLoginLogout(true);
}
@Test
diff --git a/testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/JettySamlTest.java b/testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/JettySamlTest.java
index 58d4f71..064d46e 100755
--- a/testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/JettySamlTest.java
+++ b/testsuite/jetty/jetty81/src/test/java/org/keycloak/testsuite/JettySamlTest.java
@@ -111,8 +111,12 @@ public class JettySamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/JettySamlTest.java b/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/JettySamlTest.java
index 58d4f71..064d46e 100755
--- a/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/JettySamlTest.java
+++ b/testsuite/jetty/jetty91/src/test/java/org/keycloak/testsuite/JettySamlTest.java
@@ -111,8 +111,12 @@ public class JettySamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/JettySamlTest.java b/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/JettySamlTest.java
index 7f1ec98..f1e72a4 100755
--- a/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/JettySamlTest.java
+++ b/testsuite/jetty/jetty92/src/test/java/org/keycloak/testsuite/JettySamlTest.java
@@ -111,8 +111,12 @@ public class JettySamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/JettySamlTest.java b/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/JettySamlTest.java
index 7f1ec98..f1e72a4 100644
--- a/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/JettySamlTest.java
+++ b/testsuite/jetty/jetty93/src/test/java/org/keycloak/testsuite/JettySamlTest.java
@@ -111,8 +111,12 @@ public class JettySamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java b/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
index d306ede..dc3bd09 100755
--- a/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
+++ b/testsuite/tomcat6/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
@@ -120,8 +120,12 @@ public class TomcatSamlTest {
}
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java b/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
index 0eb7b94..b4a6d1c 100755
--- a/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
+++ b/testsuite/tomcat7/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
@@ -98,8 +98,12 @@ public class TomcatSamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test
diff --git a/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java b/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
index b347f48..1456496 100755
--- a/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
+++ b/testsuite/tomcat8/src/test/java/org/keycloak/testsuite/TomcatSamlTest.java
@@ -99,8 +99,12 @@ public class TomcatSamlTest {
@Test
- public void testErrorHandling() throws Exception {
- testStrategy.testErrorHandling();
+ public void testErrorHandlingSigned() throws Exception {
+ testStrategy.testErrorHandlingSigned();
+ }
+ @Test
+ public void testErrorHandlingUnsigned() throws Exception {
+ testStrategy.testErrorHandlingUnsigned();
}
@Test