keycloak-aplcache
Changes
adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java 2(+1 -1)
saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java 10(+8 -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 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);
+ }
}