From 001cea5d0e41cae8f9d6724c37c320d29551edf1 Mon Sep 17 00:00:00 2001 From: oadham Date: Tue, 6 Oct 2026 07:39:58 +0200 Subject: [PATCH 1/3] Add handleLoginResponse tests with static signed SAML Response fixtures. --- .../broker/saml/SAMLEndpointTest.java | 107 ++++++++++++++++++ .../java/nl/first8/saml/util/SAMLUtil.java | 37 +++++- .../saml/login-response-invalid-signature.xml | 22 ++++ ...-nameid-assertion-signed-in-extensions.xml | 40 +++++++ .../resources/saml/login-response-valid.xml | 22 ++++ 5 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/saml/login-response-invalid-signature.xml create mode 100644 src/test/resources/saml/login-response-unsigned-nameid-assertion-signed-in-extensions.xml create mode 100644 src/test/resources/saml/login-response-valid.xml diff --git a/src/test/java/nl/first8/keycloak/broker/saml/SAMLEndpointTest.java b/src/test/java/nl/first8/keycloak/broker/saml/SAMLEndpointTest.java index f7e9c26..d5d6d5e 100644 --- a/src/test/java/nl/first8/keycloak/broker/saml/SAMLEndpointTest.java +++ b/src/test/java/nl/first8/keycloak/broker/saml/SAMLEndpointTest.java @@ -2,11 +2,15 @@ import io.opentelemetry.api.trace.Span; import jakarta.ws.rs.core.Response; +import java.io.InputStream; import java.lang.reflect.Field; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.security.cert.CertificateException; import java.util.stream.Stream; +import nl.first8.keycloak.dom.saml.v2.protocol.ResponseType; +import nl.first8.keycloak.saml.SAMLRequestParser; import nl.first8.saml.util.SAMLUtil; import org.bouncycastle.operator.OperatorCreationException; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -20,6 +24,8 @@ import org.keycloak.common.ClientConnection; import org.keycloak.common.Profile; import org.keycloak.common.enums.SslRequired; +import org.keycloak.events.Errors; +import org.keycloak.events.EventBuilder; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeyManager; import org.keycloak.models.KeycloakContext; @@ -29,15 +35,20 @@ import org.keycloak.models.KeycloakUriInfo; import org.keycloak.models.RealmModel; import org.keycloak.models.RealmProvider; +import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; +import org.keycloak.saml.validators.ConditionsValidator; import org.keycloak.saml.validators.DestinationValidator; import org.keycloak.services.messages.Messages; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.tracing.TracingProvider; import org.mockito.ArgumentCaptor; +import org.mockito.MockedConstruction; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -148,6 +159,102 @@ public void testHandleSamlResponse_wrongSignature() throws URISyntaxException, M verify(loginFormsProvider).setError(eq(Messages.IDENTITY_PROVIDER_INVALID_SIGNATURE), any(Object[].class)); } + @Test + public void testHandleLoginResponse_validAssertionSignature() throws Exception { + KeycloakSession session = getMockKeycloakSession(); + UserAuthenticationIdentityProvider.AuthenticationCallback callback = getMockAuthenticationCallback(); + EventBuilder event = mock(EventBuilder.class); + + Response response = handleLoginResponse("saml/login-response-valid.xml", session, callback, event); + assertEquals(Response.Status.Family.REDIRECTION, response.getStatusInfo().getFamily()); + + ArgumentCaptor captor = ArgumentCaptor.forClass(BrokeredIdentityContext.class); + verify(callback).authenticated(captor.capture()); + BrokeredIdentityContext identity = captor.getValue(); + assertEquals("9170bd139d9b5e364290187482329e89bcb431ea", identity.getId()); + assertEquals("test-alias.9170bd139d9b5e364290187482329e89bcb431ea", identity.getBrokerUserId()); + verify(event, never()).error(anyString()); + } + + @Test + public void testHandleLoginResponse_invalidAssertionSignature() throws Exception { + KeycloakSession session = getMockKeycloakSession(); + LoginFormsProvider loginFormsProvider = session.getProvider(LoginFormsProvider.class); + UserAuthenticationIdentityProvider.AuthenticationCallback callback = getMockAuthenticationCallback(); + EventBuilder event = mock(EventBuilder.class); + + // The NameID was changed after signing, so the assertion's digest no longer matches. + Response response = handleLoginResponse("saml/login-response-invalid-signature.xml", session, callback, event); + assertEquals(Response.Status.Family.CLIENT_ERROR, response.getStatusInfo().getFamily()); + + verify(callback, never()).authenticated(any(BrokeredIdentityContext.class)); + verify(event).error(Errors.INVALID_SIGNATURE); + verify(loginFormsProvider).setError(eq(Messages.INVALID_REQUESTER), any(Object[].class)); + } + + @Test + public void testHandleLoginResponse_consumedAssertionUnsignedWhileSignedAssertionInExtensions() throws Exception { + KeycloakSession session = getMockKeycloakSession(); + LoginFormsProvider loginFormsProvider = session.getProvider(LoginFormsProvider.class); + UserAuthenticationIdentityProvider.AuthenticationCallback callback = getMockAuthenticationCallback(); + EventBuilder event = mock(EventBuilder.class); + + // The validly signed assertion (legit-user) sits in samlp:Extensions; the Response's actual + // assertion, which carries the NameID that would be consumed (evil-user), has no signature. + Response response = handleLoginResponse("saml/login-response-unsigned-nameid-assertion-signed-in-extensions.xml", session, callback, event); + + ArgumentCaptor captor = ArgumentCaptor.forClass(BrokeredIdentityContext.class); + verify(callback, atMost(1)).authenticated(captor.capture()); + assertTrue(captor.getAllValues().isEmpty(), + () -> "Endpoint was fooled: authenticated '" + captor.getValue().getId() + "' from an unsigned assertion"); + assertEquals(Response.Status.Family.CLIENT_ERROR, response.getStatusInfo().getFamily()); + verify(event).error(Errors.INVALID_SIGNATURE); + verify(loginFormsProvider).setError(eq(Messages.INVALID_REQUESTER), any(Object[].class)); + } + + /** + * Feeds a static SAML Response fixture straight into Binding.handleLoginResponse, skipping the + * message-level signature check in handleSamlResponse so the assertion-level checks are what's tested. + * The fixtures carry fixed (expired) timestamps, so ConditionsValidator is mocked to always pass. + */ + private Response handleLoginResponse(String fixture, KeycloakSession session, + UserAuthenticationIdentityProvider.AuthenticationCallback callback, + EventBuilder event) throws Exception { + byte[] xml; + try (InputStream in = getClass().getClassLoader().getResourceAsStream(fixture)) { + assertNotNull(in, "Missing fixture " + fixture); + xml = in.readAllBytes(); + } + SAMLDocumentHolder holder = SAMLRequestParser.parseResponseDocument(xml); + assertNotNull(holder, "Could not parse fixture " + fixture); + ResponseType responseType = (ResponseType) holder.getSamlObject(); + + SAMLEndpoint endpoint = new SAMLEndpoint(session, mock(SAMLIdentityProvider.class), + getMockConfigForLoginResponseFixtures(), callback, getMockDestinationValidator()); + // Normally set by Binding.execute(), which this direct call bypasses + endpoint.event = event; + + try (MockedConstruction ignored = mockConstruction(ConditionsValidator.class, + (validator, context) -> when(validator.isValid()).thenReturn(true))) { + return endpoint.new PostBinding().handleLoginResponse(new String(xml, StandardCharsets.UTF_8), holder, responseType, "relayState", null); + } + } + + /** + * Strictest assertion-signature setup, trusting the fixed key in src/test/resources/keys that signed the + * static fixtures (see LoginResponseFixtureGenerator). + */ + private SAMLIdentityProviderConfig getMockConfigForLoginResponseFixtures() throws OperatorCreationException, CertificateException { + SAMLIdentityProviderConfig config = mock(SAMLIdentityProviderConfig.class); + when(config.getAlias()).thenReturn("test-alias"); + when(config.isValidateSignature()).thenReturn(true); + when(config.isWantAssertionsSigned()).thenReturn(true); + when(config.getAllowedClockSkew()).thenReturn(15); + when(config.isEnabled()).thenReturn(true); + when(config.getSigningCertificates()).thenReturn(new String[]{SAMLUtil.createSelfSignedCertificateAsBase64(SAMLUtil.loadFixtureKeyPair())}); + return config; + } + /** * Config variant that enables signature validation and supplies the correct public key. * Used by the wrong-signature test, where the response is signed with a *different* key, diff --git a/src/test/java/nl/first8/saml/util/SAMLUtil.java b/src/test/java/nl/first8/saml/util/SAMLUtil.java index 275cf05..7e16e92 100644 --- a/src/test/java/nl/first8/saml/util/SAMLUtil.java +++ b/src/test/java/nl/first8/saml/util/SAMLUtil.java @@ -1,10 +1,18 @@ package nl.first8.saml.util; +import java.io.IOException; +import java.io.InputStream; import java.math.BigInteger; +import java.security.GeneralSecurityException; +import java.security.KeyFactory; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.PublicKey; import java.security.cert.CertificateException; +import java.security.spec.PKCS8EncodedKeySpec; +import java.security.spec.X509EncodedKeySpec; import java.security.cert.X509Certificate; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -419,18 +427,43 @@ public static String createArtifactResponseSignedWithWrongKeyAsString() throws M * so signature validation succeeds for TEST_KEY_PAIR-signed responses and fails for others. */ public static String getCorrectPublicKeyCertificateAsBase64() throws OperatorCreationException, CertificateException { + return createSelfSignedCertificateAsBase64(TEST_KEY_PAIR); + } + + /** + * Wraps the key pair's public key in a fresh self-signed X.509 cert (valid from yesterday for a year) + * and returns it Base64-encoded. Signature validation only looks at the public key, so the cert's + * own validity window never goes stale for fixtures signed long ago. + */ + public static String createSelfSignedCertificateAsBase64(KeyPair keyPair) throws OperatorCreationException, CertificateException { X500Name subject = new X500Name("CN=test"); Date notBefore = new Date(System.currentTimeMillis() - 86_400_000L); Date notAfter = new Date(System.currentTimeMillis() + 86_400_000L * 365); JcaX509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( - subject, BigInteger.ONE, notBefore, notAfter, subject, TEST_KEY_PAIR.getPublic()); - ContentSigner signer = new JcaContentSignerBuilder("SHA256withRSA").build(TEST_KEY_PAIR.getPrivate()); + subject, BigInteger.ONE, notBefore, notAfter, subject, keyPair.getPublic()); + ContentSigner signer = new JcaContentSignerBuilder("SHA256withRSA").build(keyPair.getPrivate()); X509Certificate cert = new JcaX509CertificateConverter().getCertificate(builder.build(signer)); return Base64.getEncoder().encodeToString(cert.getEncoded()); } + /** + * The fixed key pair in src/test/resources/keys (see the README there). Unlike TEST_KEY_PAIR it is + * stable across runs, so it signs the static SAML fixtures in src/test/resources/saml. + */ + public static KeyPair loadFixtureKeyPair() { + try (InputStream privateIn = SAMLUtil.class.getClassLoader().getResourceAsStream("keys/private.key"); + InputStream publicIn = SAMLUtil.class.getClassLoader().getResourceAsStream("keys/public.key")) { + KeyFactory keyFactory = KeyFactory.getInstance("RSA"); + PrivateKey privateKey = keyFactory.generatePrivate(new PKCS8EncodedKeySpec(privateIn.readAllBytes())); + PublicKey publicKey = keyFactory.generatePublic(new X509EncodedKeySpec(publicIn.readAllBytes())); + return new KeyPair(publicKey, privateKey); + } catch (IOException | GeneralSecurityException e) { + throw new IllegalStateException("Could not load keys/private.key and keys/public.key", e); + } + } + private static Status createStatus() { Status status = new StatusBuilder().buildObject(); status.setStatusCode(createStatusCode()); diff --git a/src/test/resources/saml/login-response-invalid-signature.xml b/src/test/resources/saml/login-response-invalid-signature.xml new file mode 100644 index 0000000..89317b3 --- /dev/null +++ b/src/test/resources/saml/login-response-invalid-signature.xml @@ -0,0 +1,22 @@ + + urn:etoegang:HM:00000003271247010000:entities:9113 + + + + + urn:etoegang:HM:00000003271247010000:entities:9113+aT5GzjRdw7Hli0lrIno6xe2DSGFfOCuAx+k96Bp1f0=AIRE0J3kVWvZ/NjmLsk2xvd0SZssVgBZNQPZU/h1pru2cvn+izO3GVT8P4J4sPdPBeR6Xr+R+c1LBK2Zl3ostikvQvqMRL8Z216PCmlVWt/gW9XbLW7FTR/Mn9/ZDBrOctN148fwWxhGwm4X1oXJ8ddDXSbUo9VhFQsbOjmCT62goxPR98pNLpeEAR2c5oSuG0hs6b6ErM30+RIgtmShl7v1af9ybOncCapd82xgeGxWI26cn2h0O078Cb7TRa0Hp46dU1ppsHV1fjiPuHYlPOeVCHHCwv3XvW5mONkXOZFCTKg1erkDlFI1EOXJXhMyll1Nedrww0BdJMooNSIwiA==fixture-keysjK9SY04b8sKYzqKT0eZxhIyijTC0MRJraEtpAbAjWJLsnhj8kgkxxxr3ZgrfdwGDgIOgrYM/IaY9EP4CrB615yI976wEK+8k58Pq6CWhrwWsmsQ1Q5pWi9eObmRdopwJXdn4xOJrp2EaS2XYsFgXzXMAx0XnJr/5L/gRe02pR/zN7Nblyc9ijhHu1i01Um3WyjaMyATvsgLdqfSZp/OYB0/TI8OA9qKw/E757pG6FqNPObckbwrVXFloQJA0kSpl9NsJtbuLv665kbYilcqoLr1xdfGaGzUW1ZuhyVkxrNPizBw2iNLIosibFol5R0lU+4gpdaIPwS5gcoANaOGxQ==AQAB + + 9170bd139d9b5e364290187482329e89bcb431eb + + + + https://auth.testhost.lcl/realms/test-realm + + + + + urn:etoegang:core:assurance-class:loa3 + + + + \ No newline at end of file diff --git a/src/test/resources/saml/login-response-unsigned-nameid-assertion-signed-in-extensions.xml b/src/test/resources/saml/login-response-unsigned-nameid-assertion-signed-in-extensions.xml new file mode 100644 index 0000000..0d4f136 --- /dev/null +++ b/src/test/resources/saml/login-response-unsigned-nameid-assertion-signed-in-extensions.xml @@ -0,0 +1,40 @@ + + urn:etoegang:HM:00000003271247010000:entities:9113 + + + urn:etoegang:HM:00000003271247010000:entities:9113IS38cxZzig1zFuiutBj+UQZk42cCmarKJI2S1S0wAVM=F61Z9/YqUUp2I6H8VdiUeaB5Fru5prdNjCSc64F41TViOe1Gr5vEar/wtHM2zZyKm3KuvOYCUq0bzFEFjtX/Iv50ISUWrNvnQh3xuJ2c5laD0R5Z74hfrRePZorKPxj11aVxKnmtw8XHJgPOAKM761YbbJLjlJ2sDPBc9QMwiiXYob9yCEgHBiydfnI9ywIZKt/Hieg+EaQMgHeShl2fWaeDY8wGBTvgJHoSc/eVXzkUIubhksUZHN6uZb3XZ/l5VHXfpsSUcGd7A32JK+EJZQS79HnUxvd+nSVsD4XoKJuNfHfWp0a4QaLbMMoDuqiG7tgas3s6rODDgi2ymwLz9g==fixture-keysjK9SY04b8sKYzqKT0eZxhIyijTC0MRJraEtpAbAjWJLsnhj8kgkxxxr3ZgrfdwGDgIOgrYM/IaY9EP4CrB615yI976wEK+8k58Pq6CWhrwWsmsQ1Q5pWi9eObmRdopwJXdn4xOJrp2EaS2XYsFgXzXMAx0XnJr/5L/gRe02pR/zN7Nblyc9ijhHu1i01Um3WyjaMyATvsgLdqfSZp/OYB0/TI8OA9qKw/E757pG6FqNPObckbwrVXFloQJA0kSpl9NsJtbuLv665kbYilcqoLr1xdfGaGzUW1ZuhyVkxrNPizBw2iNLIosibFol5R0lU+4gpdaIPwS5gcoANaOGxQ==AQAB + + legit-user + + + + https://auth.testhost.lcl/realms/test-realm + + + + + urn:etoegang:core:assurance-class:loa3 + + + + + + + + + urn:etoegang:HM:00000003271247010000:entities:9113 + + evil-user + + + + https://auth.testhost.lcl/realms/test-realm + + + + + urn:etoegang:core:assurance-class:loa3 + + + + \ No newline at end of file diff --git a/src/test/resources/saml/login-response-valid.xml b/src/test/resources/saml/login-response-valid.xml new file mode 100644 index 0000000..c73d98f --- /dev/null +++ b/src/test/resources/saml/login-response-valid.xml @@ -0,0 +1,22 @@ + + urn:etoegang:HM:00000003271247010000:entities:9113 + + + + + urn:etoegang:HM:00000003271247010000:entities:9113+aT5GzjRdw7Hli0lrIno6xe2DSGFfOCuAx+k96Bp1f0=AIRE0J3kVWvZ/NjmLsk2xvd0SZssVgBZNQPZU/h1pru2cvn+izO3GVT8P4J4sPdPBeR6Xr+R+c1LBK2Zl3ostikvQvqMRL8Z216PCmlVWt/gW9XbLW7FTR/Mn9/ZDBrOctN148fwWxhGwm4X1oXJ8ddDXSbUo9VhFQsbOjmCT62goxPR98pNLpeEAR2c5oSuG0hs6b6ErM30+RIgtmShl7v1af9ybOncCapd82xgeGxWI26cn2h0O078Cb7TRa0Hp46dU1ppsHV1fjiPuHYlPOeVCHHCwv3XvW5mONkXOZFCTKg1erkDlFI1EOXJXhMyll1Nedrww0BdJMooNSIwiA==fixture-keysjK9SY04b8sKYzqKT0eZxhIyijTC0MRJraEtpAbAjWJLsnhj8kgkxxxr3ZgrfdwGDgIOgrYM/IaY9EP4CrB615yI976wEK+8k58Pq6CWhrwWsmsQ1Q5pWi9eObmRdopwJXdn4xOJrp2EaS2XYsFgXzXMAx0XnJr/5L/gRe02pR/zN7Nblyc9ijhHu1i01Um3WyjaMyATvsgLdqfSZp/OYB0/TI8OA9qKw/E757pG6FqNPObckbwrVXFloQJA0kSpl9NsJtbuLv665kbYilcqoLr1xdfGaGzUW1ZuhyVkxrNPizBw2iNLIosibFol5R0lU+4gpdaIPwS5gcoANaOGxQ==AQAB + + 9170bd139d9b5e364290187482329e89bcb431ea + + + + https://auth.testhost.lcl/realms/test-realm + + + + + urn:etoegang:core:assurance-class:loa3 + + + + \ No newline at end of file From f25f6e5e6fa01723807dcb7750dc62f39996ae30 Mon Sep 17 00:00:00 2001 From: oadham Date: Tue, 6 Oct 2026 10:58:10 +0200 Subject: [PATCH 2/3] Removed SAMLEndpoint drift regarding assertionElement, as compared to Keycloak. Credit and appreciation to Ray Sabee (Whitehat Security) for noting the drift. --- .../keycloak/broker/saml/SAMLEndpoint.java | 2 +- .../core/saml/v2/util/AssertionUtil.java | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/nl/first8/keycloak/broker/saml/SAMLEndpoint.java b/src/main/java/nl/first8/keycloak/broker/saml/SAMLEndpoint.java index 03820f5..880e268 100644 --- a/src/main/java/nl/first8/keycloak/broker/saml/SAMLEndpoint.java +++ b/src/main/java/nl/first8/keycloak/broker/saml/SAMLEndpoint.java @@ -473,7 +473,7 @@ protected Response handleLoginResponse(String samlResponse, SAMLDocumentHolder h /* We verify the assertion using original document to handle cases where the IdP includes whitespace and/or newlines inside tags. */ logger.debug("Verify and get assertion!"); - assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(Objects.requireNonNull(JBossSAMLConstants.ASSERTION.get()))); + assertionElement = AssertionUtil.getAssertionElement(holder); } // Validate the response Issuer diff --git a/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java b/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java index e3d8801..b662e6d 100644 --- a/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java +++ b/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java @@ -63,6 +63,7 @@ import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; +import org.w3c.dom.NodeList; public class AssertionUtil { @@ -554,6 +555,30 @@ public static AssertionType getAssertion(SAMLDocumentHolder holder, ResponseType return responseType.getAssertions().get(0).getAssertion(); } + public static Element getAssertionElement(SAMLDocumentHolder holder) throws ProcessingException { + Document doc = holder.getSamlDocument(); + Element response = doc.getDocumentElement(); + if (!org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL.getNsUri().get().equals(response.getNamespaceURI()) + || !org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL.get().equals(response.getLocalName())) { + throw new ProcessingException("No response type."); + } + + // get the first assertion in the response + NodeList children = response.getChildNodes(); + for(int i = 0; i < children.getLength(); i++) { + Node childNode = children.item(i); + if (childNode instanceof Element) { + Element childElement = (Element) childNode; + if (org.keycloak.saml.common.constants.JBossSAMLConstants.ASSERTION.getNsUri().get().equals(childElement.getNamespaceURI()) + && org.keycloak.saml.common.constants.JBossSAMLConstants.ASSERTION.get().equals(childElement.getLocalName())) { + return childElement; + } + } + } + + throw new ProcessingException("No assertion from response."); + } + public static boolean isAssertionEncrypted(ResponseType responseType) throws ProcessingException { List assertions = responseType.getAssertions(); From 46511ee12284746abab9921f7947e4da5a362a85 Mon Sep 17 00:00:00 2001 From: oadham Date: Tue, 6 Oct 2026 16:19:40 +0200 Subject: [PATCH 3/3] Support ArtifactResponse documents in AssertionUtil.getAssertionElement. The strict assertion lookup introduced in f25f6e5 required the document root to be samlp:Response, which broke artifact binding logins where the root is samlp:ArtifactResponse wrapping the Response. --- .../core/saml/v2/util/AssertionUtil.java | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java b/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java index b662e6d..6d47002 100644 --- a/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java +++ b/src/main/java/nl/first8/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java @@ -557,26 +557,48 @@ public static AssertionType getAssertion(SAMLDocumentHolder holder, ResponseType public static Element getAssertionElement(SAMLDocumentHolder holder) throws ProcessingException { Document doc = holder.getSamlDocument(); - Element response = doc.getDocumentElement(); - if (!org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL.getNsUri().get().equals(response.getNamespaceURI()) - || !org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL.get().equals(response.getLocalName())) { + Element root = doc.getDocumentElement(); + Element response; + if (isElement(root, org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL)) { + response = root; + } else if (isElement(root, org.keycloak.saml.common.constants.JBossSAMLConstants.ARTIFACT_RESPONSE)) { + // Artifact binding: the Response is a direct child of the ArtifactResponse. + response = getFirstChildElement(root, org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL); + if (response == null) { + throw new ProcessingException("No response type."); + } + } else { + throw new ProcessingException("No response type."); + } + return getAssertionElement(response); + } + + public static Element getAssertionElement(Element response) throws ProcessingException { + if (!isElement(response, org.keycloak.saml.common.constants.JBossSAMLConstants.RESPONSE__PROTOCOL)) { throw new ProcessingException("No response type."); } // get the first assertion in the response - NodeList children = response.getChildNodes(); + Element assertion = getFirstChildElement(response, org.keycloak.saml.common.constants.JBossSAMLConstants.ASSERTION); + if (assertion == null) { + throw new ProcessingException("No assertion from response."); + } + return assertion; + } + + private static Element getFirstChildElement(Element parent, org.keycloak.saml.common.constants.JBossSAMLConstants type) { + NodeList children = parent.getChildNodes(); for(int i = 0; i < children.getLength(); i++) { - Node childNode = children.item(i); - if (childNode instanceof Element) { - Element childElement = (Element) childNode; - if (org.keycloak.saml.common.constants.JBossSAMLConstants.ASSERTION.getNsUri().get().equals(childElement.getNamespaceURI()) - && org.keycloak.saml.common.constants.JBossSAMLConstants.ASSERTION.get().equals(childElement.getLocalName())) { - return childElement; - } + if (children.item(i) instanceof Element childElement && isElement(childElement, type)) { + return childElement; } } + return null; + } - throw new ProcessingException("No assertion from response."); + private static boolean isElement(Element element, org.keycloak.saml.common.constants.JBossSAMLConstants type) { + return type.getNsUri().get().equals(element.getNamespaceURI()) + && type.get().equals(element.getLocalName()); } public static boolean isAssertionEncrypted(ResponseType responseType) throws ProcessingException {