[java-plugin-shibd-saml] branch dev/StateMgmtWIP updated: Adjust state being tracked and implement handler to check issuer/dest.

Codeberg noreply at shibboleth.net
Thu Apr 30 17:31:46 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch dev/StateMgmtWIP
in repository java-plugin-shibd-saml.

View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd-saml/commit/39eb6f330727cc09ea9224586364581a65941941

The following commit(s) were added to refs/heads/dev/StateMgmtWIP by this push:
     new 39eb6f3  Adjust state being tracked and implement handler to check issuer/dest.
39eb6f3 is described below

commit 39eb6f330727cc09ea9224586364581a65941941
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Thu Apr 30 13:31:31 2026 -0400

    Adjust state being tracked and implement handler to check issuer/dest.
---
 .../idp/flows/sp/consumer/saml2/saml2-beans.xml    | 18 +++++
 .../idp/flows/sp/consumer/saml2/saml2-flow.xml     |  2 +
 .../flows/saml2/SAML2TokenConsumerFlowTest.java    | 47 ++++++++++-
 .../impl/CheckDestinationAndIssuerHandler.java     | 93 ++++++++++++++++++++++
 .../sp/saml/saml2/messaging/impl/package-info.java | 18 +++++
 .../saml/saml2/profile/impl/AddAuthnRequest.java   |  3 +-
 .../saml/saml2/profile/impl/AddLogoutRequest.java  |  2 -
 7 files changed, 177 insertions(+), 6 deletions(-)

diff --git a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
index 21dc7df..ee4e6bf 100644
--- a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
+++ b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-beans.xml
@@ -42,6 +42,24 @@
         </property>
     </bean>
 
+    <bean id="CheckDestinationAndIssuerHandler" class="net.shibboleth.sp.profile.impl.WebFlowMessageHandlerAdaptor" scope="prototype"
+            c:executionDirection="INBOUND">
+        <constructor-arg>
+            <bean class="org.opensaml.messaging.handler.impl.BasicMessageHandlerChain">
+                <property name="handlers">
+                    <list>
+                        <bean class="net.shibboleth.sp.saml.saml2.messaging.impl.CheckDestinationAndIssuerHandler" scope="prototype"
+                            p:checkDuringInit="false"
+                            p:httpServletRequestSupplier-ref="shibboleth.RemotedHttpServletRequestSupplier" />
+                    </list>
+                </property>
+             </bean>
+        </constructor-arg>
+        <property name="errorEvent">
+            <util:constant static-field="org.opensaml.profile.action.EventIds.INVALID_MESSAGE" />
+        </property>
+    </bean>
+
     <bean id="InboundEntityIDLookup" parent="shibboleth.Functions.Compose">
         <constructor-arg name="g">
             <bean class="org.opensaml.saml.common.messaging.context.navigate.SAMLEntityIDFunction" />
diff --git a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
index b298db8..1a05992 100644
--- a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
+++ b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/saml2/saml2-flow.xml
@@ -12,7 +12,9 @@
         
         <evaluate expression="CheckMessageVersion" />
         <evaluate expression="HandleNoPassive" />
+        
         <evaluate expression="SAMLProtocolAndRole" />
+        <evaluate expression="CheckDestinationAndIssuerHandler" />
         <evaluate expression="SAMLMetadataLookup" />
 
         <evaluate expression="InitializeRelyingPartyContextFromSAMLPeer" />
diff --git a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
index 8005296..5574839 100644
--- a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
+++ b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2TokenConsumerFlowTest.java
@@ -589,6 +589,50 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
                         response.getAssertions().get(0).getID()));
     }
     
+    /**
+     * Test failure due to state recovery authority.
+     * 
+     * @throws IOException
+     * @throws ResolverException 
+     */
+    @Test
+    public void testFailedStateAuthority() throws IOException, ResolverException {
+        
+        final StateData data = buildStateData("foo");
+        data.setAuthenticationAuthority(ISSUER + "/bad");
+        final String stateToken = getStateToken(data);
+        
+        final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, "foo");
+        sign(response);
+        final DDF input = buildRemotedPOSTResponse(response, stateToken, response2.getCookies()[0]);
+
+        setApplicationRequest(APPLICATION_ID, input);
+        
+        validateError(EventIds.INVALID_MESSAGE, null);
+    }
+    
+    /**
+     * Test failure due to state recovery authority.
+     * 
+     * @throws IOException
+     * @throws ResolverException 
+     */
+    @Test
+    public void testFailedStateLocation() throws IOException, ResolverException {
+        
+        final StateData data = buildStateData("foo");
+        data.setResponseLocation(RESPONSE_URL + "/bad");
+        final String stateToken = getStateToken(data);
+        
+        final Response response = buildSAMLResponse(ISSUER, StatusCode.SUCCESS, "foo");
+        sign(response);
+        final DDF input = buildRemotedPOSTResponse(response, stateToken, response2.getCookies()[0]);
+
+        setApplicationRequest(APPLICATION_ID, input);
+        
+        validateError(EventIds.INVALID_MESSAGE, null);
+    }
+    
     /**
      * Test successful flow.
      * 
@@ -910,9 +954,8 @@ public class SAML2TokenConsumerFlowTest extends AbstractSPFlowTest {
     @Nonnull private StateData buildStateData(@Nonnull final String requestID) {
         final SAMLStateData data = new SAMLStateData();
         data.setRequestID(requestID);
-        data.setRequestTime(Instant.now());
-        data.setIssuer(AUDIENCE);
         data.setAuthenticationAuthority(ISSUER);
+        data.setResponseLocation(RESPONSE_URL);
         data.setRawResource(RESOURCE_URL.getBytes(StandardCharsets.UTF_8));
         return data;
     }
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/CheckDestinationAndIssuerHandler.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/CheckDestinationAndIssuerHandler.java
new file mode 100644
index 0000000..5594955
--- /dev/null
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/CheckDestinationAndIssuerHandler.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.sp.saml.saml2.messaging.impl;
+
+import java.util.Objects;
+import java.util.function.Function;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.messaging.context.InOutOperationContext;
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
+import org.opensaml.messaging.context.navigate.RecursiveTypedParentContextLookup;
+import org.opensaml.messaging.handler.AbstractHttpServletRequestMessageHandler;
+import org.opensaml.messaging.handler.MessageHandlerException;
+import org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext;
+import org.slf4j.Logger;
+
+import jakarta.servlet.http.HttpServletRequest;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.sp.context.StateDataContext;
+import net.shibboleth.sp.saml.saml2.SAMLStateData;
+import net.shibboleth.sp.state.StateData;
+
+/**
+ * Message handler that cross checks the message destination and issuer against previously
+ * tracked {@link StateData} if any.
+ */
+public class CheckDestinationAndIssuerHandler extends AbstractHttpServletRequestMessageHandler {
+
+    /** Class logger. */
+    @Nonnull private Logger log = LoggerFactory.getLogger(CheckDestinationAndIssuerHandler.class);
+    
+    /** Lookup strategy for {@link StateDataContext}. */
+    @Nonnull private Function<MessageContext,StateDataContext> stateDataContextLookupStrategy;
+    
+    /** Constructor. */
+    @SuppressWarnings("null")
+    public CheckDestinationAndIssuerHandler() {
+        stateDataContextLookupStrategy = new ChildContextLookup<>(StateDataContext.class).compose(
+                new RecursiveTypedParentContextLookup<>(InOutOperationContext.class));
+    }
+    
+    /**
+     * Sets lookup strategy for {@link StateDataContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateDataContextLookupStrategy(@Nonnull final Function<MessageContext,StateDataContext> strategy) {
+        stateDataContextLookupStrategy = Constraint.isNotNull(strategy,
+                "StateDataContext lookup strategy cannot be null");
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    public void doInvoke(@Nonnull final MessageContext messageContext) throws MessageHandlerException {
+        final StateDataContext context = stateDataContextLookupStrategy.apply(messageContext);
+        if (context != null && context.getStateData() instanceof SAMLStateData samlState) {
+            
+            // Check issuer.
+            final SAMLPeerEntityContext peer = messageContext.getSubcontext(SAMLPeerEntityContext.class);
+            if (peer != null && samlState.getAuthenticationAuthority() != null
+                    && !Objects.equals(peer.getEntityId(), samlState.getAuthenticationAuthority())) {
+                log.warn("{} Issuer mismatch, message issued by {}, expected issuer was {} ", getLogPrefix(),
+                        peer.getEntityId(), samlState.getAuthenticationAuthority());
+                throw new MessageHandlerException("Message issuer did not match expected request recipient.");
+            }
+            
+            // Check destination.
+            final HttpServletRequest request = getHttpServletRequest();
+            if (request != null && samlState.getResponseLocation() != null
+                    && !Objects.equals(samlState.getResponseLocation(), request.getRequestURL().toString())) {
+                log.warn("{} Destination mismatch, message expected at {}, delivered to {}", getLogPrefix(),
+                        samlState.getResponseLocation(), request.getRequestURL());
+                throw new MessageHandlerException("Message destination did not match expected location.");
+            }
+        }
+    }
+
+}
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/package-info.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/package-info.java
new file mode 100644
index 0000000..d2b5a09
--- /dev/null
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/messaging/impl/package-info.java
@@ -0,0 +1,18 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Message handlers for SAML 2 SP.
+ */
+package net.shibboleth.sp.saml.saml2.messaging.impl;
\ No newline at end of file
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
index 2da5ee0..7c44674 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
@@ -313,7 +313,6 @@ public class AddAuthnRequest extends AbstractApplicationAction {
         object.setVersion(SAMLVersion.VERSION_20);
         
         stateData.setRequestID(object.getID());
-        stateData.setRequestTime(object.getIssueInstant());
         
         if (!setResponseEndpoint(profileRequestContext, object)) {
             ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
@@ -331,7 +330,6 @@ public class AddAuthnRequest extends AbstractApplicationAction {
             final Issuer issuer = issuerBuilder.buildObject();
             issuer.setValue(issuerId);
             object.setIssuer(issuer);
-            stateData.setIssuer(issuerId);
         } else {
             log.debug("{} No issuer value available, leaving Issuer unset", getLogPrefix());
         }
@@ -580,6 +578,7 @@ public class AddAuthnRequest extends AbstractApplicationAction {
         if (operator != null) {
             log.debug("{} Setting RequestedAuthnContext operator to {}", getLogPrefix(), operator);
             rac.setComparison(operator);
+            stateData.setAuthnContextOperator(operator.toString());
         }
         
         return rac;
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
index 19e8c2a..171365f 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
@@ -253,7 +253,6 @@ public class AddLogoutRequest extends AbstractApplicationAction {
         object.setReason(LogoutRequest.USER_REASON);
 
         stateData.setRequestID(object.getID());
-        stateData.setRequestTime(object.getIssueInstant());
         
         if (issuerId != null) {
             log.debug("{} Setting Issuer to {}", getLogPrefix(), issuerId);
@@ -262,7 +261,6 @@ public class AddLogoutRequest extends AbstractApplicationAction {
             final Issuer issuer = issuerBuilder.buildObject();
             issuer.setValue(issuerId);
             object.setIssuer(issuer);
-            stateData.setIssuer(issuerId);
         } else {
             log.debug("{} No issuer value available, leaving Issuer unset", getLogPrefix());
         }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list