[java-plugin-shibd] 01/01: Redesigned actions and initiator flows, no testing done yet.

Codeberg noreply at shibboleth.net
Mon Apr 27 16:07:40 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.

View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd/commit/d428d181547e395d1aafc1ea4478c570d824117a

commit d428d181547e395d1aafc1ea4478c570d824117a
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Mon Apr 27 12:07:22 2026 -0400

    Redesigned actions and initiator flows, no testing done yet.
---
 .../idp/flows/sp/initiator/disco/disco-beans.xml   |   7 +
 .../idp/flows/sp/initiator/disco/disco-flow.xml    |   3 +
 .../session-initiator/session-initiator-beans.xml  |  11 +-
 .../session-initiator/session-initiator-flow.xml   |   2 +-
 .../sp/profile/impl/EncodeAgentResponse.java       |  50 +++++-
 .../sp/profile/impl/IssueCorrelationCookie.java    |   3 +
 .../sp/profile/impl/IssueDiscoveryRequest.java     |  44 +++--
 .../sp/profile/impl/PrepareAgentErrorResponse.java |  33 +++-
 .../impl/PrepareDiscoveryStateDataContext.java     | 114 +++++++++++++
 .../sp/profile/impl/PreservePostData.java          | 181 ++++++++++++++++-----
 .../sp/profile/impl/ProcessCorrelationCookie.java  |   3 +
 .../sp/profile/impl/RecoverStateData.java          |  46 ++++--
 12 files changed, 418 insertions(+), 79 deletions(-)

diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-beans.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-beans.xml
index 0cf4bb6..84c31d4 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-beans.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-beans.xml
@@ -16,6 +16,13 @@
         p:requireDiscoveryURL="true"
         p:requireRelyingPartyId="false" />
     
+    <bean id="PrepareDiscoveryStateDataContext"
+        class="net.shibboleth.sp.profile.impl.PrepareDiscoveryStateDataContext" scope="prototype" />
+    
+    <bean id="PreserveStateData"
+        class="net.shibboleth.sp.profile.impl.PreserveStateDataAction" scope="prototype"
+        p:errorFatal="%{sp.stateToken.errorsFatal:false}" />
+    
     <bean id="IssueDiscoveryRequest"
         class="net.shibboleth.sp.profile.impl.IssueDiscoveryRequest" scope="prototype"
         p:httpServletResponseSupplier-ref="shibboleth.RemotedHttpServletResponseSupplier"
diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-flow.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-flow.xml
index e3d0241..b857c4b 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-flow.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/disco/disco-flow.xml
@@ -5,6 +5,9 @@
 
     <action-state id="DiscoverySessionInitiator">
         <evaluate expression="ValidateSessionInitiatorRequest" />
+        <evaluate expression="PrepareDiscoveryStateDataContext" />
+        <evaluate expression="PreserveStateData" />
+        <evaluate expression="PreservePostData" />
         <evaluate expression="IssueDiscoveryRequest" />
         
         <evaluate expression="'proceed'" />
diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
index 6ce382b..9e01348 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-beans.xml
@@ -9,10 +9,9 @@
 
     <bean id="shibboleth.sp.profileId" class="java.lang.String" c:_0="http://shibboleth.net/ns/profiles/sp/session-initiator" />
     <bean id="shibboleth.sp.loggingId" class="java.lang.String" c:_0="%{sp.service.logging.initiator:SPAgent.SessionInitiator}" />
-
-    <bean id="MapResourceToStateToken" 
-        class="net.shibboleth.sp.profile.impl.MapResourceToStateToken" scope="prototype"
-        p:createOutputObjects="true"
-        p:errorFatal="%{sp.stateToken.errorsFatal:false}" />
-        
+    
+    <!-- Default configuration pulls state token from input message if present. -->
+    <bean id="RecoverStateData"
+        class="net.shibboleth.sp.profile.impl.RecoverStateData" scope="prototype" />
+    
 </beans>
diff --git a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-flow.xml b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-flow.xml
index 1cf23bb..5490cd9 100644
--- a/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-flow.xml
+++ b/sp-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/session-initiator/session-initiator-flow.xml
@@ -16,7 +16,7 @@
         <on-entry>
             <evaluate expression="new net.shibboleth.shared.collection.ReflectionSafeIterator(opensamlProfileRequestContext.ensureSubcontext(T(net.shibboleth.sp.context.AgentRequestContext)).getApplication().getSessionInitiators(opensamlProfileRequestContext).iterator())" result="flowScope.SessionInitiatorIterator" />
         </on-entry>
-        <evaluate expression="MapResourceToStateToken" />
+        <evaluate expression="RecoverStateData" />
         <evaluate expression="'proceed'" />
         
         <!-- Branch to child flow for actual work. -->
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/EncodeAgentResponse.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/EncodeAgentResponse.java
index f6ac184..1658d6b 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/EncodeAgentResponse.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/EncodeAgentResponse.java
@@ -16,9 +16,11 @@ package net.shibboleth.sp.profile.impl;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
 
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
@@ -27,18 +29,23 @@ import org.slf4j.Logger;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.sp.Agent;
 import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.StateDataContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.profile.AbstractAgentRequestAction;
+import net.shibboleth.sp.profile.SPConstants;
+import net.shibboleth.sp.state.StateData;
 
 /**
  * A profile action to encode an agent response from the output {@link DDF} in the
  * {@link AgentRequestContext}.
  * 
  * <p>This also ensures the final output is a structure, and contains an "event" member,
- * setting it to "success" if not already set.</p> 
+ * setting it to "success" if not already set, and also populates target from a {@link StateDataContext}
+ * if available.</p> 
  * 
  * <p>If enabled as a feature for the agent, the value of the session ID is also placed
  * in a structure member in the output before encoding to relay it to the agent.</p>
@@ -47,16 +54,37 @@ import net.shibboleth.sp.profile.AbstractAgentRequestAction;
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  * @event {@link EventIds#INVALID_MESSAGE}
  * @event {@link EventIds#IO_ERROR}
- * @pre <pre>AgentRequestContext.getOutput().getmember("event").isstring()</pre>
+ * 
+ * @post <pre>AgentRequestContext.getOutput().getmember("event").isstring()</pre>
  */
 public class EncodeAgentResponse extends AbstractAgentRequestAction {
 
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(EncodeAgentResponse.class);
 
+    /** Strategy used to create the {@link StateDataContext} to populate. */
+    @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextLookupStrategy;
+    
     /** Cached context containing the output message. */
     @NonnullBeforeExec private DDF outputMessage;
     
+    /** Constructor. */
+    public EncodeAgentResponse() {
+        stateDataContextLookupStrategy = new ChildContextLookup<>(StateDataContext.class);
+    }
+    
+    /**
+     * Sets the strategy used to lookup the {@link StateDataContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateDataContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,StateDataContext> strategy) {
+        checkSetterPreconditions();
+        stateDataContextLookupStrategy =
+                Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
+    }
+    
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -94,8 +122,22 @@ public class EncodeAgentResponse extends AbstractAgentRequestAction {
             return;
         }
         
-        if (outputMessage.getmember("event").isnull()) {
-            outputMessage.addmember("event").string("success");
+        if (outputMessage.getmember(SPConstants.EVENT).isnull()) {
+            outputMessage.addmember(SPConstants.EVENT).string("success");
+        }
+        
+        // Populate target if necessary.
+        if (outputMessage.getmember(SPConstants.TARGET).isnull()) {
+            final StateDataContext stateDataContext = stateDataContextLookupStrategy.apply(profileRequestContext);
+            if (stateDataContext != null) {
+                final StateData data = stateDataContext.getStateData();
+                if (data != null) {
+                    final byte[] target = data.getRawResource();
+                    if (target != null) {
+                        outputMessage.addmember(SPConstants.TARGET).unsafe_string(target);
+                    }
+                }
+            }
         }
         
         final Agent agent = ensureAgentRequestContext().getAgent();
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookie.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookie.java
index f635187..28bc140 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookie.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookie.java
@@ -46,7 +46,10 @@ import net.shibboleth.sp.profile.SPConstants;
  * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_MESSAGE}
+ * 
+ * @deprecated
  */
+ at Deprecated
 public class IssueCorrelationCookie extends AbstractApplicationAction {
     
     /** Default cookie prefix. */
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueDiscoveryRequest.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueDiscoveryRequest.java
index 52d9ddc..97b44ba 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueDiscoveryRequest.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/IssueDiscoveryRequest.java
@@ -21,6 +21,7 @@ import java.util.function.Predicate;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
@@ -35,6 +36,7 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.shared.resolver.CriteriaSet;
 import net.shibboleth.shared.resolver.ResolverException;
+import net.shibboleth.sp.context.StateDataContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
 import net.shibboleth.sp.profile.AbstractApplicationAction;
@@ -61,6 +63,9 @@ public class IssueDiscoveryRequest extends AbstractApplicationAction {
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(IssueDiscoveryRequest.class);
 
+    /** Strategy used to create the {@link StateDataContext} to populate. */
+    @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextLookupStrategy;
+    
     /** Strategy used to obtain the request issuer value. */
     @Nonnull private Function<ProfileRequestContext,String> issuerLookupStrategy;
     
@@ -79,15 +84,25 @@ public class IssueDiscoveryRequest extends AbstractApplicationAction {
     /** The agent handler URL to return control to. */
     @NonnullBeforeExec private String returnURL;
     
-    /** State token. */
-    @Nullable private String state;
-    
     /** Constructor. */
     public IssueDiscoveryRequest() {
+        stateDataContextLookupStrategy = new ChildContextLookup<>(StateDataContext.class);
         issuerLookupStrategy = new ApplicationDefaultIssuerStrategy();
         passiveCondition = new DefaultPassiveCondition();
     }
     
+    /**
+     * Sets the strategy used to lookup the {@link StateDataContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateDataContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,StateDataContext> strategy) {
+        checkSetterPreconditions();
+        stateDataContextLookupStrategy =
+                Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
+    }
+    
     /**
      * Set the strategy used to locate the issuer value to use.
      * 
@@ -97,7 +112,7 @@ public class IssueDiscoveryRequest extends AbstractApplicationAction {
         checkSetterPreconditions();
         issuerLookupStrategy = Constraint.isNotNull(strategy, "Issuer lookup strategy cannot be null");
     }
-
+    
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -126,9 +141,7 @@ public class IssueDiscoveryRequest extends AbstractApplicationAction {
             log.error("{} No input message from agent", getLogPrefix());
             return false;
         }
-        
-        state = input.getmember(SPConstants.STATE).string();
-        
+
         // This is checked earlier in the flow, so can be treated as an outright error here.
         returnURL = input.getmember(InitiatorConstants.DISCOVERY_RETURN_URL).string();
         if (returnURL == null) {
@@ -141,15 +154,18 @@ public class IssueDiscoveryRequest extends AbstractApplicationAction {
     }
     
     /** {@inheritDoc} */
-    @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-        
-        // We have to add the state parameter (usually placed there by our calling flow) to the query
-        // string of the already-URL-encoded return URL. We URL-encode the character sequence
-        // "&state=<value>" and append that to the existing URL. This ensures the agent will get our
-        // state token back from the DS to provide to us afterwards.  
-        
+    @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {        
+
         final Escaper escaper = UrlEscapers.urlFormParameterEscaper();
+                
+        // Check for state token in context.
+        final StateDataContext stateDataContext = stateDataContextLookupStrategy.apply(profileRequestContext);
+        final String state = stateDataContext != null ? stateDataContext.getStateToken() : null;
         if (state != null) {
+            // We have to add a state parameter to the query string of the already-URL-encoded return URL.
+            // We URL-encode the character sequence "&state=<value>" and append that to the existing URL.
+            // This ensures the agent will get our state token back from the DS to provide to the Hub afterwards.
+            
             returnURL += escaper.escape('&' + SPConstants.STATE + '=' + escaper.escape(state));
             
              // At this point, the return value is already encoded, and thus does not need to
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareAgentErrorResponse.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareAgentErrorResponse.java
index 61d4a7e..e0478bc 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareAgentErrorResponse.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareAgentErrorResponse.java
@@ -26,9 +26,11 @@ import org.opensaml.profile.context.navigate.CurrentOrPreviousEventLookup;
 
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.StateDataContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.profile.AbstractAgentRequestAction;
 import net.shibboleth.sp.profile.SPConstants;
+import net.shibboleth.sp.state.StateData;
 
 /**
  * A profile action to encode the current event (typically an error) into an agent response in a
@@ -45,10 +47,14 @@ public class PrepareAgentErrorResponse extends AbstractAgentRequestAction {
     /** Strategy function for access to {@link EventContext} to check. */
     @Nonnull private Function<ProfileRequestContext,EventContext> eventContextLookupStrategy;
     
+    /** Strategy used to create the {@link StateDataContext} to populate. */
+    @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextLookupStrategy;
+    
     /** Constructor. */
     public PrepareAgentErrorResponse() {
         setAgentRequestContextLookupStrategy(new ChildContextLookup<>(AgentRequestContext.class, true));
         eventContextLookupStrategy = new CurrentOrPreviousEventLookup();
+        stateDataContextLookupStrategy = new ChildContextLookup<>(StateDataContext.class);
     }
 
     /**
@@ -62,6 +68,18 @@ public class PrepareAgentErrorResponse extends AbstractAgentRequestAction {
         eventContextLookupStrategy = Constraint.isNotNull(strategy, "EventContext lookup strategy cannot be null");
     }
     
+    /**
+     * Sets the strategy used to lookup the {@link StateDataContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateDataContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,StateDataContext> strategy) {
+        checkSetterPreconditions();
+        stateDataContextLookupStrategy =
+                Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
+    }
+    
     /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -83,9 +101,18 @@ public class PrepareAgentErrorResponse extends AbstractAgentRequestAction {
             output.addmember(SPConstants.EVENT).string(EventIds.MESSAGE_PROC_ERROR);
         }
         
-        final byte[] target = agentContext.getTargetURL();
-        if (target != null) {
-            output.addmember(SPConstants.TARGET).unsafe_string(target);
+        // Populate target if necessary.
+        if (output.getmember(SPConstants.TARGET).isnull()) {
+            final StateDataContext stateDataContext = stateDataContextLookupStrategy.apply(profileRequestContext);
+            if (stateDataContext != null) {
+                final StateData data = stateDataContext.getStateData();
+                if (data != null) {
+                    final byte[] target = data.getRawResource();
+                    if (target != null) {
+                        output.addmember(SPConstants.TARGET).unsafe_string(target);
+                    }
+                }
+            }
         }
     }
 
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareDiscoveryStateDataContext.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareDiscoveryStateDataContext.java
new file mode 100644
index 0000000..4d1ab30
--- /dev/null
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PrepareDiscoveryStateDataContext.java
@@ -0,0 +1,114 @@
+/*
+ * 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.profile.impl;
+
+import java.util.function.Function;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
+import org.opensaml.profile.action.ActionSupport;
+import org.opensaml.profile.action.EventIds;
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.sp.context.StateDataContext;
+import net.shibboleth.sp.ddf.DDF;
+import net.shibboleth.sp.profile.AbstractApplicationAction;
+import net.shibboleth.sp.profile.SPConstants;
+import net.shibboleth.sp.state.StateData;
+
+/**
+ * Extracts target resource URL from input message and prepares a {@link StateDataContext} for
+ * subsequent preservation handling.
+ * 
+ * @event {@link EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_PROFILE_CTX}
+ * 
+ * @post StateDataContext exists and {@link StateDataContext#getStateData()} != null and
+ *      {@link StateData#getResource()} != null
+ */
+public class PrepareDiscoveryStateDataContext extends AbstractApplicationAction {
+
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(PrepareDiscoveryStateDataContext.class);
+    
+    /** Strategy used to create the {@link StateDataContext} to populate. */
+    @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextCreationStrategy;
+    
+    /** Input message. */
+    @NonnullBeforeExec private byte[] target;
+    
+    /** Constructor. */
+    public PrepareDiscoveryStateDataContext() {
+        stateDataContextCreationStrategy = new ChildContextLookup<>(StateDataContext.class, true);
+    }
+    
+    /**
+     * Sets the strategy used to create the {@link StateDataContext}.
+     * 
+     * @param strategy creation strategy
+     */
+    public void setStateDataContextCreationStrategy(
+            @Nonnull final Function<ProfileRequestContext,StateDataContext> strategy) {
+        checkSetterPreconditions();
+        stateDataContextCreationStrategy =
+                Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+        
+        if (!super.doPreExecute(profileRequestContext)) {
+            return false;
+        }
+        
+        final DDF input = ensureAgentRequestContext().getInput();
+        if (input == null) {
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+            log.error("{} No input message from agent", getLogPrefix());
+            return false;
+        }
+
+        target = input.getmember(SPConstants.TARGET).unsafe_string();
+        if (target == null) {
+            log.debug("{} No target parameter from agent, nothing to do", getLogPrefix());
+            return false;
+        }
+        
+        return true;
+    }
+    
+    /** {@inheritDoc} */
+    @Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {        
+
+        final StateDataContext context = stateDataContextCreationStrategy.apply(profileRequestContext);
+        if (context == null) {
+            log.error("{} Failed to create StateDataContext", getLogPrefix());
+            ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+            return;
+        }
+        
+        final StateData data = new StateData();
+        data.setRawResource(target);
+        
+        context.setStateData(data);
+    }
+
+}
\ No newline at end of file
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PreservePostData.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PreservePostData.java
index 1909e8d..ad37101 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PreservePostData.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/PreservePostData.java
@@ -18,12 +18,16 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.function.Function;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
+import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
 import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
+import org.opensaml.storage.StorageRecord;
 import org.opensaml.storage.StorageService;
 import org.slf4j.Logger;
 
@@ -42,6 +46,7 @@ import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.security.IdentifierGenerationStrategy;
 import net.shibboleth.shared.security.IdentifierGenerationStrategy.ProviderType;
 import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.StateDataContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequest;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
@@ -65,6 +70,9 @@ public class PreservePostData extends AbstractApplicationAction {
     /** Class logger. */
     @Nonnull private Logger log = LoggerFactory.getLogger(PreservePostData.class);
     
+    /** Strategy used to create the {@link StateDataContext} to populate. */
+    @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextLookupStrategy;
+    
     /** Identifier generation. */
     @NonnullAfterInit private IdentifierGenerationStrategy identifierStrategy;
     
@@ -86,14 +94,27 @@ public class PreservePostData extends AbstractApplicationAction {
     /** State token vakue used in cookie name. */
     @NonnullBeforeExec private String stateToken;
     
-    /** POST data to preserve. */
-    @NonnullBeforeExec private byte[] postData;
+    /** POST data to preserve in record. */
+    @NonnullBeforeExec private String postData;
     
     /** Constructor. */
     public PreservePostData() {
-        lifetime = Duration.ofMinutes(5);
+        stateDataContextLookupStrategy = new ChildContextLookup<>(StateDataContext.class);
+        lifetime = Duration.ofMinutes(30);
         cookiePrefix = DEFAULT_COOKIE_PREFIX;
     }    
+
+    /**
+     * Sets the strategy used to lookup the {@link StateDataContext}.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateDataContextLookupStrategy(
+            @Nonnull final Function<ProfileRequestContext,StateDataContext> strategy) {
+        checkSetterPreconditions();
+        stateDataContextLookupStrategy =
+                Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
+    }
     
     /**
      * Set {@link IdentifierGenerationStrategy} to use.
@@ -196,28 +217,15 @@ public class PreservePostData extends AbstractApplicationAction {
             return false;
         }
         
-        // First see if anything is even there.
-        final DDF input = ensureAgentRequestContext().getInput();
-        if (input != null) {
-            postData = input.getmember(RemotedHttpServletRequest.STRUCTURE_NAME)
-                    .getmember(RemotedHttpServletRequest.BODY)
-                    .unsafe_string();
-        }
-                
+        postData = getSubmittedData();
         if (postData == null) {
-            log.debug("{} No POST data to preserve", getLogPrefix());
-            return false;
-        }
-        
-        assert input != null;
-        final String contentType = input.getmember(RemotedHttpServletRequest.STRUCTURE_NAME)
-                .getmember(RemotedHttpServletRequest.CONTENT_TYPE)
-                .string();
-        if (!"application/x-www-form-urlencoded".equals(contentType)) {
-            log.warn("{} Unsupported content type: {}", getLogPrefix(), contentType);
-            return false;
+            postData = getPreviousData(profileRequestContext);
+            if (postData == null) {
+                log.debug("{} No POST data to preserve", getLogPrefix());
+                return false;
+            }
         }
-        
+                
         // Check permission.
         
         if (!ensureAgent().isSupportsPostPreservation()) {
@@ -226,26 +234,21 @@ public class PreservePostData extends AbstractApplicationAction {
         }
         
         long limit = ensureAgent().getPostLimit();
-        if (limit > 0 && postData.length > limit) {
+        if (limit > 0 && postData.length() > limit) {
             log.warn("{} POST data supplied for preservation, but size {} exceeds agent limit", getLogPrefix(),
-                    postData.length);
+                    postData.length());
             return false;
         }
         
         // Check for state token for cookie correlation.
-        
-        if (input != null) {
-            stateToken = input.getmember(SPConstants.STATE).string();
-        }
-        
+        final StateDataContext stateDataContext = stateDataContextLookupStrategy.apply(profileRequestContext);
+        stateToken = stateDataContext != null ? stateDataContext.getStateToken() : null;
         if (stateToken == null) {
             if (errorFatal) {
-                log.warn("{} Input was missing {} parameter, failing due to POST data preservationr",
-                        getLogPrefix(), SPConstants.STATE);
+                log.warn("{} Context missing state token to bind to, failing POST data preservationr", getLogPrefix());
                 ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
             } else {
-                log.warn("{} Input was missing {} parameter, skipping POST data preservation", getLogPrefix(),
-                        SPConstants.STATE);
+                log.warn("{} Context missing state token to bind to, skipping POST data preservation", getLogPrefix());
             }
             return false;
         }
@@ -266,17 +269,13 @@ public class PreservePostData extends AbstractApplicationAction {
 
             cookieManager.purgeStaleCookies(cookiePrefix);
 
-            log.debug("{} Preserving {} bytes of POST data against state token {}", getLogPrefix(), postData.length,
+            log.debug("{} Preserving {} bytes of POST data against state token {}", getLogPrefix(), postData.length(),
                     stateToken);
             
-            // Given the MIME type, we assume the POST data is URL-encoded, ergo safe to translate as UTF-8.
-            // At this stage the underlying data may not in fact be UTF-8 but the encoded characters are ASCII.
-            final String encoded = new String(postData, StandardCharsets.UTF_8);
-            
             // Generate a storage key.
             final String key = identifierStrategy.generateIdentifier(false);
             
-            if (!storageService.create(ensureAgent().getId() + ".PostData", key, encoded,
+            if (!storageService.create(ensureAgent().getId() + ".PostData", key, postData,
                     Instant.now().plus(lifetime).toEpochMilli())) {
                 log.warn("{} Collision attempting to create storage record for POST data under key {}", getLogPrefix(), key);
                 if (errorFatal) {
@@ -303,4 +302,106 @@ public class PreservePostData extends AbstractApplicationAction {
         }
     }
 
+    /**
+     * Gets data submitted with this request, if any.
+     * 
+     * @return submitted data or null
+     */
+    @Nullable private String getSubmittedData() {
+        
+        // First see if anything fresh is present, indicating a POST.
+        byte[] raw = null;
+        
+        final DDF input = ensureAgentRequestContext().getInput();
+        if (input != null) {
+            raw = input.getmember(RemotedHttpServletRequest.STRUCTURE_NAME)
+                    .getmember(RemotedHttpServletRequest.BODY)
+                    .unsafe_string();
+        }
+                
+        if (raw == null) {
+            return null;
+        }
+        
+        assert input != null;
+        final String contentType = input.getmember(RemotedHttpServletRequest.STRUCTURE_NAME)
+                .getmember(RemotedHttpServletRequest.CONTENT_TYPE)
+                .string();
+        if (!"application/x-www-form-urlencoded".equals(contentType)) {
+            log.warn("{} Unsupported content type: {}", getLogPrefix(), contentType);
+            return null;
+        }
+        
+        // Given the MIME type, we assume the POST data is URL-encoded, ergo safe to translate as UTF-8.
+        // At this stage the underlying data may not in fact be UTF-8 but the encoded characters are ASCII.
+        return new String(raw, StandardCharsets.UTF_8);
+    }
+    
+    /**
+     * Checks for previously preserved data tracked against a state token submitted by the agent.
+     * 
+     * <p>This case is triggered by use of discovery, which would have preserved the data against the
+     * state token issued at that time, which we have since recovered and disposed of.</p>
+     * 
+     * @param profileRequestContext profile request context
+     * 
+     * @return previous data or null
+     */
+    @Nullable private String getPreviousData(@Nonnull final ProfileRequestContext profileRequestContext) {
+        
+        final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
+        
+        final DDF input = agentRequestContext.getInput();
+        String previousStateToken = input != null ? input.getmember(SPConstants.STATE).string() : null;
+        if (previousStateToken == null) {
+            return null;
+        }
+
+        // Implement an "inline" recovery of the data to get it back and clear the original storage record.
+
+        if (previousStateToken.length() > 16) {
+            previousStateToken = previousStateToken.substring(0, 16);
+        }
+        
+        // Do the needful to allow cookies to be loaded/cleared from wrapped input.
+        try {
+            RemotedHttpServletRequestResponseContext.loadCurrent(agentRequestContext.getRemotedHttpServletRequest(),
+                    agentRequestContext.getRemotedHttpServletResponse());
+         
+            // Check for cookie to get storage key.
+            final String key = cookieManager.getCookieValue(cookiePrefix + stateToken, null);
+            if (key == null) {
+                log.debug("{} No recovery cookie for state token {}, skipping POST recovery", getLogPrefix(),
+                        stateToken);
+                return null;
+            }
+            
+            // Unset the cookie.
+            cookieManager.unsetCookie(cookiePrefix + stateToken);
+            
+            // Try and read/delete the storage record.
+            
+            final StorageRecord<String> record = storageService.read(ensureAgent().getId() + ".PostData", key);
+            if (record == null) {
+                log.warn("{} POST recovery record was missing for key: {}", getLogPrefix(), key);
+                return null;
+            }
+            
+            try {
+                storageService.delete(ensureAgent().getId() + ".PostData", key);
+            } catch (final IOException e) {
+                log.warn("{} Error deleting POST recovery record for key: {}", getLogPrefix(), key);
+            }
+            
+            return record.getValue();
+            
+        } catch (final IOException e) {
+            log.error("{} Error reading storage record for POST data", getLogPrefix(), e);
+        } finally {
+            RemotedHttpServletRequestResponseContext.clearCurrent();
+        }
+        
+        return null;
+    }
+    
 }
\ No newline at end of file
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookie.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookie.java
index 8dc0fa0..5e7275b 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookie.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookie.java
@@ -51,7 +51,10 @@ import net.shibboleth.sp.profile.AbstractApplicationAction;
  * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_MESSAGE}
+ * 
+ * @deprecated
  */
+ at Deprecated
 public class ProcessCorrelationCookie extends AbstractApplicationAction {
     
     /** Class logger. */
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/RecoverStateData.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/RecoverStateData.java
index 449e6ea..0a629df 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/RecoverStateData.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/RecoverStateData.java
@@ -18,6 +18,7 @@ import java.io.IOException;
 import java.util.function.Function;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.opensaml.messaging.context.navigate.ChildContextLookup;
 import org.opensaml.profile.action.ActionSupport;
@@ -32,8 +33,10 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.sp.context.AgentRequestContext;
 import net.shibboleth.sp.context.StateDataContext;
+import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
 import net.shibboleth.sp.profile.AbstractApplicationAction;
+import net.shibboleth.sp.profile.SPConstants;
 import net.shibboleth.sp.state.StateData;
 
 /**
@@ -74,16 +77,6 @@ public class RecoverStateData extends AbstractApplicationAction {
         stateDataClass = StateData.class;
     }
     
-    /**
-     * Sets the lookup strategy to obtain the protocol specific state token. 
-     *
-     * @param strategy lookup strategy
-     */
-    public void setStateTokenLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
-        checkSetterPreconditions();
-        stateTokenLookupStrategy = Constraint.isNotNull(strategy, "State token lookup strategy cannot be null");
-    }
-    
     /**
      * Sets the strategy used to create the {@link StateDataContext}.
      * 
@@ -96,6 +89,16 @@ public class RecoverStateData extends AbstractApplicationAction {
                 Constraint.isNotNull(strategy, "StateDataContext creation strategy cannot be null");
     }
     
+    /**
+     * Sets the lookup strategy to obtain the protocol specific state token. 
+     *
+     * @param strategy lookup strategy
+     */
+    public void setStateTokenLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
+        checkSetterPreconditions();
+        stateTokenLookupStrategy = Constraint.isNotNull(strategy, "State token lookup strategy cannot be null");
+    }
+    
     /**
      * Sets the type of {@link StateData} subclass to recover from the state token.
      * 
@@ -112,7 +115,7 @@ public class RecoverStateData extends AbstractApplicationAction {
         super.doInitialize();
         
         if (stateTokenLookupStrategy == null) {
-            throw new ComponentInitializationException("State token lookup strategy cannot be null");
+            stateTokenLookupStrategy = new DefaultStateTokenLookupStrategy();
         }
     }
 
@@ -169,5 +172,26 @@ public class RecoverStateData extends AbstractApplicationAction {
             RemotedHttpServletRequestResponseContext.clearCurrent();
         }
     }
+    
+    /**
+     * Default lookup strategy locates state token from Agent request.
+     */
+    public static final class DefaultStateTokenLookupStrategy implements Function<ProfileRequestContext,String> {
+
+        /** {@inheritDoc} */
+        @Nullable public String apply(@Nullable final ProfileRequestContext input) {
+            if (input != null) {
+                final AgentRequestContext agentRequestContext = input.getSubcontext(AgentRequestContext.class);
+                if (agentRequestContext != null) {
+                    final DDF inputMessage = agentRequestContext.getInput();
+                    if (inputMessage != null) {
+                        return inputMessage.getmember(SPConstants.STATE).string();
+                    }
+                }
+            }
+            return null;
+        }
+        
+    }
 
 }
\ No newline at end of file

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


More information about the commits mailing list