keycloak-aplcache

KEYCLOAK-4264

1/19/2017 12:59:28 PM

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