keycloak-aplcache

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 12cb924..be7de79 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
@@ -339,7 +339,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
             return AuthOutcome.FAILED;
         }
         try {
-            assertion = AssertionUtil.getAssertion(responseType, deployment.getDecryptionKey());
+            assertion = AssertionUtil.getAssertion(responseHolder, responseType, deployment.getDecryptionKey());
             if (AssertionUtil.hasExpired(assertion)) {
                 return initiateLogin();
             }
diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java
index 0de9c86..887df88 100755
--- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java
+++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java
@@ -67,6 +67,7 @@ import java.util.Set;
 
 import org.keycloak.rotation.HardcodedKeyLocator;
 import org.keycloak.saml.common.constants.GeneralConstants;
+import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
 
 /**
  * Utility to deal with assertions
@@ -552,7 +553,7 @@ public class AssertionUtil {
         return roles;
     }
 
-    public static AssertionType getAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
+    public static AssertionType getAssertion(SAMLDocumentHolder holder, ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
         List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
 
         if (assertions.isEmpty()) {
@@ -566,7 +567,7 @@ public class AssertionUtil {
             if (privateKey == null) {
                 throw new ProcessingException("Encryptd assertion and decrypt private key is null");
             }
-            decryptAssertion(responseType, privateKey);
+            decryptAssertion(holder, responseType, privateKey);
 
         }
         return responseType.getAssertions().get(0).getAssertion();
@@ -588,10 +589,8 @@ public class AssertionUtil {
      * @param responseType a response containg an encrypted assertion
      * @return the assertion element as it was decrypted. This can be used in signature verification.
      */
-    public static Element decryptAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
-        SAML2Response saml2Response = new SAML2Response();
-
-        Document doc = saml2Response.convert(responseType);
+    public static Element decryptAssertion(SAMLDocumentHolder holder, ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException {
+        Document doc = holder.getSamlDocument();
         Element enc = DocumentUtil.getElement(doc, new QName(JBossSAMLConstants.ENCRYPTED_ASSERTION.get()));
 
         if (enc == null) {
diff --git a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
index c54bbea..870ec51 100644
--- a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
+++ b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
@@ -67,7 +67,9 @@ import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
 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.request.SAML2Request;
 import org.keycloak.saml.processing.api.saml.v2.response.SAML2Response;
+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.core.saml.v2.util.XMLTimeUtil;
 import org.w3c.dom.Element;
@@ -137,6 +139,8 @@ public class SAMLParserTest {
             Object parsedObject;
             if (SAML2Object.class.isAssignableFrom(expectedType)) {
                 parsedObject = new SAML2Response().getSAML2ObjectFromStream(st);
+            } else if (SAMLDocumentHolder.class.isAssignableFrom(expectedType)) {
+                parsedObject = SAML2Request.getSAML2ObjectFromStream(st);
             } else {
                 parsedObject = parser.parse(st);
             }
@@ -185,7 +189,9 @@ public class SAMLParserTest {
 
     @Test
     public void testSaml20EncryptedAssertionWithNewlines() throws Exception {
-        ResponseType resp = assertParsed("KEYCLOAK-4489-encrypted-assertion-with-newlines.xml", ResponseType.class);
+        SAMLDocumentHolder holder = assertParsed("KEYCLOAK-4489-encrypted-assertion-with-newlines.xml", SAMLDocumentHolder.class);
+        assertThat(holder.getSamlObject(), instanceOf(ResponseType.class));
+        ResponseType resp = (ResponseType) holder.getSamlObject();
         assertThat(resp.getAssertions().size(), is(1));
 
         ResponseType.RTChoiceType rtChoiceType = resp.getAssertions().get(0);
@@ -193,7 +199,7 @@ public class SAMLParserTest {
         assertNotNull(rtChoiceType.getEncryptedAssertion());
 
         PrivateKey privateKey = DerUtils.decodePrivateKey(Base64.decode(PRIVATE_KEY));
-        AssertionUtil.decryptAssertion(resp, privateKey);
+        AssertionUtil.decryptAssertion(holder, resp, privateKey);
 
         rtChoiceType = resp.getAssertions().get(0);
         assertNotNull(rtChoiceType.getAssertion());
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
index e25d8ca..c8b8cd4 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@@ -366,7 +366,7 @@ public class SAMLEndpoint {
 
                 if (assertionIsEncrypted) {
                     // This methods writes the parsed and decrypted assertion back on the responseType parameter:
-                    assertionElement = AssertionUtil.decryptAssertion(responseType, keys.getPrivateKey());
+                    assertionElement = AssertionUtil.decryptAssertion(holder, responseType, keys.getPrivateKey());
                 } else {
                     /* We verify the assertion using original document to handle cases where the IdP
                     includes whitespace and/or newlines inside tags. */
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
index ab5ff26..f9f141d 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractBrokerTest.java
@@ -65,6 +65,9 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
         resetUserPassword(realmResource.users().get(userId), bc.getUserPassword(), false);
 
         if (testContext.isInitialized()) {
+            if (identityProviderResource == null) {
+                identityProviderResource = (IdentityProviderResource) testContext.getCustomValue("identityProviderResource");
+            }
             return;
         }
 
@@ -72,6 +75,7 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
         RealmResource realm = adminClient.realm(bc.consumerRealmName());
         realm.identityProviders().create(bc.setUpIdentityProvider(suiteContext)).close();
         identityProviderResource = realm.identityProviders().get(bc.getIDPAlias());
+        testContext.setCustomValue("identityProviderResource", identityProviderResource);
 
         // addClients
         List<ClientRepresentation> clients = bc.createProviderClients(suiteContext);
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
index f53e90f..e447998 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java
@@ -36,7 +36,6 @@ import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 import static org.keycloak.testsuite.broker.BrokerTestConstants.*;
-import static org.keycloak.testsuite.broker.BrokerTestTools.encodeUrl;
 import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
 
 public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
@@ -113,8 +112,7 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
         return new KcSamlSignedBrokerConfiguration();
     }
 
-    @Test
-    public void testSignedEncryptedAssertions() throws Exception {
+    public void withSignedEncryptedAssertions(Runnable testBody, boolean signedAssertion, boolean encryptedAssertion) throws Exception {
         ClientRepresentation client = adminClient.realm(bc.providerRealmName())
           .clients()
           .findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext))
@@ -130,31 +128,44 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
         Assert.assertThat(consumerCert, Matchers.notNullValue());
 
         try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
-            .setAttribute(SAMLIdentityProviderConfig.VALIDATE_SIGNATURE, "true")
-            .setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_SIGNED, "true")
-            .setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_ENCRYPTED, "true")
+            .setAttribute(SAMLIdentityProviderConfig.VALIDATE_SIGNATURE, Boolean.toString(signedAssertion))
+            .setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_SIGNED, Boolean.toString(signedAssertion))
+            .setAttribute(SAMLIdentityProviderConfig.WANT_ASSERTIONS_ENCRYPTED, Boolean.toString(encryptedAssertion))
             .setAttribute(SAMLIdentityProviderConfig.WANT_AUTHN_REQUESTS_SIGNED, "false")
             .setAttribute(SAMLIdentityProviderConfig.SIGNING_CERTIFICATE_KEY, providerCert)
             .update();
           Closeable clientUpdater = new ClientAttributeUpdater(clientResource)
-            .setAttribute(SamlConfigAttributes.SAML_ENCRYPT, "true")
+            .setAttribute(SamlConfigAttributes.SAML_ENCRYPT, Boolean.toString(encryptedAssertion))
             .setAttribute(SamlConfigAttributes.SAML_ENCRYPTION_CERTIFICATE_ATTRIBUTE, consumerCert)
             .setAttribute(SamlConfigAttributes.SAML_SERVER_SIGNATURE, "false")      // only sign assertions
-            .setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, "true")
+            .setAttribute(SamlConfigAttributes.SAML_ASSERTION_SIGNATURE, Boolean.toString(signedAssertion))
             .setAttribute(SamlConfigAttributes.SAML_CLIENT_SIGNATURE_ATTRIBUTE, "false")
             .update())
         {
-            // Login should pass because assertion is signed.
-            loginUser();
+            testBody.run();
+        }
+    }
 
-            // Logout should fail because logout response is not signed.
-            driver.navigate().to(BrokerTestTools.getAuthRoot(suiteContext)
-                    + "/auth/realms/" + bc.providerRealmName()
-                    + "/protocol/" + "openid-connect"
-                    + "/logout?redirect_uri=" + encodeUrl(getAccountUrl(bc.providerRealmName())));
+    @Test
+    public void testSignedEncryptedAssertions() throws Exception {
+        withSignedEncryptedAssertions(this::testAssertionSignatureRespected, true, true);
+    }
 
-            errorPage.assertCurrent();
-        }
+    @Test
+    public void testSignedAssertion() throws Exception {
+        withSignedEncryptedAssertions(this::testAssertionSignatureRespected, true, false);
+    }
+
+    private void testAssertionSignatureRespected() {
+        // Login should pass because assertion is signed.
+        loginUser();
+
+        // Logout should fail because logout response is not signed.
+        final String redirectUri = getAccountUrl(bc.providerRealmName());
+        final String logoutUri = oauth.realm(bc.providerRealmName()).getLogoutUrl().redirectUri(redirectUri).build();
+        driver.navigate().to(logoutUri);
+
+        errorPage.assertCurrent();
     }
 
     private Document extractNamespacesToTopLevelElement(Document original) {
@@ -202,10 +213,15 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
 
     // KEYCLOAK-5581
     @Test
-    public void loginUserAllNamespacesInTopElement() throws Exception {
+    public void loginUserAllNamespacesInTopElement() {
         AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST, null);
 
-        Document doc = extractNamespacesToTopLevelElement(SAML2Request.convert(loginRep));
+        Document doc;
+        try {
+            doc = extractNamespacesToTopLevelElement(SAML2Request.convert(loginRep));
+        } catch (Exception ex) {
+            throw new RuntimeException(ex);
+        }
 
         SAMLDocumentHolder samlResponse = new SamlClientBuilder()
           .authnRequest(getAuthServerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build()   // Request to consumer IdP
@@ -232,4 +248,18 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
         Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
     }
 
+    @Test
+    public void loginUserAllNamespacesInTopElementSignedEncryptedAssertion() throws Exception {
+        withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, true, true);
+    }
+
+    @Test
+    public void loginUserAllNamespacesInTopElementSignedAssertion() throws Exception {
+        withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, true, false);
+    }
+
+    @Test
+    public void loginUserAllNamespacesInTopElementEncryptedAssertion() throws Exception {
+        withSignedEncryptedAssertions(this::loginUserAllNamespacesInTopElement, false, true);
+    }
 }