Fix/assertion element drift tests and fix

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

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,52 @@ 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 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
+        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++) {
+            if (children.item(i) instanceof Element childElement && isElement(childElement, type)) {
+                return childElement;
+            }
+        }
+        return null;
+    }
+
+    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 {
         List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
 

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<BrokeredIdentityContext> 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<BrokeredIdentityContext> 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<ConditionsValidator> 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,

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());

src/test/resources/saml/login-response-invalid-signature.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?><samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Destination="https://auth.testhost.lcl/realms/test-realm/broker/test-alias/endpoint" ID="_response-valid" IssueInstant="2026-10-06T10:00:00Z" Version="2.0">
+    <saml:Issuer>urn:etoegang:HM:00000003271247010000:entities:9113</saml:Issuer>
+    <samlp:Status>
+        <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
+    </samlp:Status>
+    <saml:Assertion ID="_assertion-valid" IssueInstant="2026-10-06T10:00:00Z" Version="2.0">
+        <saml:Issuer>urn:etoegang:HM:00000003271247010000:entities:9113</saml:Issuer><dsig:Signature xmlns:dsig="http://www.w3.org/2000/09/xmldsig#"><dsig:SignedInfo><dsig:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><dsig:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/><dsig:Reference URI="#_assertion-valid"><dsig:Transforms><dsig:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><dsig:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></dsig:Transforms><dsig:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><dsig:DigestValue>+aT5GzjRdw7Hli0lrIno6xe2DSGFfOCuAx+k96Bp1f0=</dsig:DigestValue></dsig:Reference></dsig:SignedInfo><dsig:SignatureValue>AIRE0J3kVWvZ/NjmLsk2xvd0SZssVgBZNQPZU/h1pru2cvn+izO3GVT8P4J4sPdPBeR6Xr+R+c1LBK2Zl3ostikvQvqMRL8Z216PCmlVWt/gW9XbLW7FTR/Mn9/ZDBrOctN148fwWxhGwm4X1oXJ8ddDXSbUo9VhFQsbOjmCT62goxPR98pNLpeEAR2c5oSuG0hs6b6ErM30+RIgtmShl7v1af9ybOncCapd82xgeGxWI26cn2h0O078Cb7TRa0Hp46dU1ppsHV1fjiPuHYlPOeVCHHCwv3XvW5mONkXOZFCTKg1erkDlFI1EOXJXhMyll1Nedrww0BdJMooNSIwiA==</dsig:SignatureValue><dsig:KeyInfo><dsig:KeyName>fixture-key</dsig:KeyName><dsig:KeyValue><dsig:RSAKeyValue><dsig:Modulus>sjK9SY04b8sKYzqKT0eZxhIyijTC0MRJraEtpAbAjWJLsnhj8kgkxxxr3ZgrfdwGDgIOgrYM/IaY9EP4CrB615yI976wEK+8k58Pq6CWhrwWsmsQ1Q5pWi9eObmRdopwJXdn4xOJrp2EaS2XYsFgXzXMAx0XnJr/5L/gRe02pR/zN7Nblyc9ijhHu1i01Um3WyjaMyATvsgLdqfSZp/OYB0/TI8OA9qKw/E757pG6FqNPObckbwrVXFloQJA0kSpl9NsJtbuLv665kbYilcqoLr1xdfGaGzUW1ZuhyVkxrNPizBw2iNLIosibFol5R0lU+4gpdaIPwS5gcoANaOGxQ==</dsig:Modulus><dsig:Exponent>AQAB</dsig:Exponent></dsig:RSAKeyValue></dsig:KeyValue></dsig:KeyInfo></dsig:Signature>
+        <saml:Subject>
+            <saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient">9170bd139d9b5e364290187482329e89bcb431eb</saml:NameID>
+        </saml:Subject>
+        <saml:Conditions NotBefore="2026-10-06T09:59:00Z" NotOnOrAfter="2026-10-06T10:05:00Z">
+            <saml:AudienceRestriction>
+                <saml:Audience>https://auth.testhost.lcl/realms/test-realm</saml:Audience>
+            </saml:AudienceRestriction>
+        </saml:Conditions>
+        <saml:AuthnStatement AuthnInstant="2026-10-06T10:00:00Z" SessionIndex="_session-_assertion-valid">
+            <saml:AuthnContext>
+                <saml:AuthnContextClassRef>urn:etoegang:core:assurance-class:loa3</saml:AuthnContextClassRef>
+            </saml:AuthnContext>
+        </saml:AuthnStatement>
+    </saml:Assertion>
+</samlp:Response>