[java-plugin-shibd] branch main updated: JSHIBDSAML-1 - Request/response correlation and passive tracking

Scott Cantor cantor.2 at osu.edu
Mon Apr 14 20:05:15 UTC 2025


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

scantor pushed a commit to branch main
in repository java-plugin-shibd.

View the commit online:
http://git.shibboleth.net/view/?p=java-plugin-shibd.git;a=commit;h=842cbd0be54865ac8d3f5c3b8354fd37ee66c6b8

The following commit(s) were added to refs/heads/main by this push:
     new 842cbd0  JSHIBDSAML-1 - Request/response correlation and passive tracking
842cbd0 is described below

commit 842cbd0be54865ac8d3f5c3b8354fd37ee66c6b8
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Apr 14 16:05:12 2025 -0400

    JSHIBDSAML-1 - Request/response correlation and passive tracking
    
    https://shibboleth.atlassian.net/browse/JSHIBDSAML-1
    
    Factor output creation into base class.
    Flesh out handling of passive and target fields in output.
    Add action to process cookie and create new subcontext.
---
 .../sp/context/TokenConsumerContext.java           |  85 ++++++++++
 .../shibboleth/sp/profile/AbstractAgentAction.java |  37 +++++
 .../AbstractTokenConsumerResponseAction.java       |  29 ++--
 .../shibboleth/sp/profile/ConsumerConstants.java   |   5 +-
 .../sp/profile/impl/IssueCorrelationCookie.java    |  24 +--
 .../sp/profile/impl/MapResourceToStateToken.java   |  26 +--
 .../sp/profile/impl/PrepareAgentErrorResponse.java |  15 +-
 .../sp/profile/impl/ProcessCorrelationCookie.java  | 182 +++++++++++++++++++++
 .../profile/impl/IssueCorrelationCookieTest.java   |   2 -
 ...Test.java => ProcessCorrelationCookieTest.java} | 103 ++++--------
 10 files changed, 375 insertions(+), 133 deletions(-)

diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/context/TokenConsumerContext.java b/sp-server-api/src/main/java/net/shibboleth/sp/context/TokenConsumerContext.java
new file mode 100644
index 0000000..693c056
--- /dev/null
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/context/TokenConsumerContext.java
@@ -0,0 +1,85 @@
+/*
+ * 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.context;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.messaging.context.BaseContext;
+
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.primitive.StringSupport;
+
+/**
+ * Dedicated context for a token consumer operation's state.
+ */
+public class TokenConsumerContext extends BaseContext {
+    
+    /** Tracking of whether transaction at agent is operating in passive mode. */
+    private boolean passive;
+    
+    /** A message ID to use for correlating responses against. */
+    @Nullable @NotEmpty private String messageCorrelationID;
+    
+    
+    /**
+     * Gets whether the agent transaction is a "passive" one, i.e., instructed not to
+     * involve the UI.
+     * 
+     * @return passive state of agent request
+     */
+    public boolean isPassive() {
+        return passive;
+    }
+    
+    /**
+     * Gets whether the agent transaction is a "passive" one, i.e., instructed not to
+     * involve the UI.
+     * 
+     * <p>Defaults to false.</p>
+     * 
+     * @param flag flag to set
+     * 
+     * @return this context
+     */
+    @Nonnull public TokenConsumerContext setPassive(final boolean flag) {
+        passive = flag;
+        
+        return this;
+    }
+
+    /**
+     * Gets the message ID to correlate responses against.
+     * 
+     * @return original request message ID
+     */
+    @Nullable @NotEmpty public String getMessageCorrelationID() {
+        return messageCorrelationID;
+    }
+    
+    /**
+     * Sets the message ID to correlate responses against.
+     * 
+     * @param id original request message ID
+     * 
+     * @return this context
+     */
+    @Nonnull public TokenConsumerContext setMessageCorrelationID(@Nullable @NotEmpty String id) {
+        messageCorrelationID = StringSupport.trimOrNull(id);
+        
+        return this;
+    }
+    
+}
\ No newline at end of file
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractAgentAction.java b/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractAgentAction.java
index 3bc3170..79edcba 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractAgentAction.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractAgentAction.java
@@ -27,6 +27,8 @@ 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.ddf.DDF;
+import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
 
 /**
  * Base class for actions that need access to the {@link Agent} in an {@link AgentRequestContext}.
@@ -46,6 +48,26 @@ public abstract class AbstractAgentAction extends AbstractAgentRequestAction {
     /** Cached agent from context. */
     @NonnullBeforeExec private Agent agent;
 
+    /** Whether to create the output objects into which the message will be encoded. */
+    private boolean createOutputObjects;
+    
+    public boolean isCreateOutputObjects() {
+        return createOutputObjects;
+    }
+    
+    /**
+     * Sets whether to create an output {@link DDF} and {@link RemotedHttpServletResponse}.
+     * 
+     * <p>Defaults to false.</p>
+     * 
+     * @param flag flag to set
+     */
+    public void setCreateOutputObjects(final boolean flag) {
+        checkSetterPreconditions();
+        
+        createOutputObjects = flag;
+    }
+    
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -81,5 +103,20 @@ public abstract class AbstractAgentAction extends AbstractAgentRequestAction {
     @Nonnull public Agent ensureAgent() {
         return Constraint.isNotNull(agent, "Agent was null");
     }
+    
+    /**
+     * If {@link #isCreateOutputObjects()} is true, then this ensures an output
+     * {@link DDF} is in place and if creating one, adds the structure and installs
+     * the wrapper for a {@link RemotedHttpServletResponse}.
+     */
+    protected void ensureOutputObjects() {
+        final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
+        if (isCreateOutputObjects() && agentRequestContext.getOutput() == null) {
+            final DDF output = new DDF(null);
+            agentRequestContext.setOutput(output);
+            agentRequestContext.setRemotedHttpServletResponse(new RemotedHttpServletResponse(
+                    output.structure().addmember(RemotedHttpServletResponse.STRUCTURE_NAME)));
+        }
+    }
         
 }
\ No newline at end of file
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractTokenConsumerResponseAction.java b/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractTokenConsumerResponseAction.java
index a3c05e9..0c15e60 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractTokenConsumerResponseAction.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/profile/AbstractTokenConsumerResponseAction.java
@@ -35,6 +35,7 @@ import net.shibboleth.idp.attribute.context.AttributeContext;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.TokenConsumerContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
 import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
@@ -134,26 +135,32 @@ public abstract class AbstractTokenConsumerResponseAction extends AbstractApplic
             output.addmember(ConsumerConstants.SESSION_OPAQUE).string(sessionData);
         }
 
+        // Signal back passive status based on original request.
+        // This is more relevant for errors but for consistency...
+        if (agentRequestContext.ensureSubcontext(TokenConsumerContext.class).isPassive()) {
+            output.addmember(ConsumerConstants.PASSIVE).integer(1);
+        }
+        
         // Issue redirect to proper resource URL. Either recovered from protocol state,
         // or falling back to an input parameter from the agent. Final backtop is a relative
         // redirect to the site root.
         
         final RemotedHttpServletResponse remotedResponse = agentRequestContext.getRemotedHttpServletResponse();
         assert remotedResponse != null;
-
-        byte[] resource = recoverState(profileRequestContext, agentRequestContext);
-        if (resource != null && resource.length > 0) {
-            remotedResponse.sendRedirect(resource);
-            return;
-        }
         
-        final DDF in = agentRequestContext.getInput();
-        if (in != null) {
-            resource = in.getmember(ConsumerConstants.BASE_URL).unsafe_string();
-        }
+        byte[] resource = recoverState(profileRequestContext, agentRequestContext);
         if (resource == null || resource.length == 0) {
-            resource = "/".getBytes(StandardCharsets.UTF_8);
+            final DDF in = agentRequestContext.getInput();
+            if (in != null) {
+                resource = in.getmember(ConsumerConstants.BASE_URL).unsafe_string();
+            }
+            if (resource == null || resource.length == 0) {
+                resource = "/".getBytes(StandardCharsets.UTF_8);
+            }
         }
+        
+        ensureAgentRequestContext().setTargetURL(resource);
+        output.addmember(SPConstants.TARGET).unsafe_string(resource);
         remotedResponse.sendRedirect(resource);
     }
     
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/profile/ConsumerConstants.java b/sp-server-api/src/main/java/net/shibboleth/sp/profile/ConsumerConstants.java
index d6fd70a..5ee9ff4 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/profile/ConsumerConstants.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/profile/ConsumerConstants.java
@@ -28,13 +28,16 @@ public final class ConsumerConstants {
 
     /** Token validation errors data member. */
     @Nonnull @NotEmpty public static final String VALIDATION_ERRORS = "validation_errors";
-    
+        
     /** Opaque session data member. */
     @Nonnull @NotEmpty public static final String SESSION_OPAQUE = "session.opaque";
 
     /** Session attributes data member. */
     @Nonnull @NotEmpty public static final String SESSION_ATTRIBUTES = "session.attributes";
 
+    /** Passive indicator output parameter name. */
+    @Nonnull @NotEmpty public static final String PASSIVE = "passive";
+
     /** Private constructor. */
     private ConsumerConstants() {
         
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 d6bd7d0..ebd5932 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
@@ -39,7 +39,6 @@ import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.sp.context.AgentRequestContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
-import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
 import net.shibboleth.sp.profile.AbstractApplicationAction;
 import net.shibboleth.sp.profile.SPConstants;
 
@@ -66,9 +65,6 @@ public class IssueCorrelationCookie extends AbstractApplicationAction {
     
     /** Cookie prefix. */
     @Nonnull private String cookiePrefix;
-    
-    /** Whether to create the output objects into which the message will be encoded. */
-    private boolean createOutputObjects;
 
     /** Whether an error constructing a correlation cookie is fatal. */
     private boolean errorFatal;
@@ -117,19 +113,6 @@ public class IssueCorrelationCookie extends AbstractApplicationAction {
         
         cookiePrefix = Constraint.isNotNull(StringSupport.trimOrNull(prefix), "Cookie prefix cannot be null or empty");
     }
-
-    /**
-     * Sets whether to create the output {@link DDF} and {@link RemotedHttpServletResponse}.
-     * 
-     * <p>Defaults to false.</p>
-     * 
-     * @param flag flag to set
-     */
-    public void setCreateOutputObjects(final boolean flag) {
-        checkSetterPreconditions();
-        
-        createOutputObjects = flag;
-    }
     
     /**
      * Sets whether an error computing a state token should result in a fatal event.
@@ -220,13 +203,8 @@ public class IssueCorrelationCookie extends AbstractApplicationAction {
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
 
+        ensureOutputObjects();
         final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
-        if (createOutputObjects && agentRequestContext.getOutput() == null) {
-            final DDF output = new DDF(null);
-            agentRequestContext.setOutput(output);
-            agentRequestContext.setRemotedHttpServletResponse(new RemotedHttpServletResponse(
-                    output.structure().addmember(RemotedHttpServletResponse.STRUCTURE_NAME)));
-        }
 
         // We do the crazy stuff to accomodate the cookies being set or unset.
         try {
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
index a8c8747..a55d5f7 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/MapResourceToStateToken.java
@@ -28,7 +28,6 @@ import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.sp.context.AgentRequestContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
-import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
 import net.shibboleth.sp.profile.AbstractApplicationAction;
 import net.shibboleth.sp.profile.SPConstants;
 
@@ -49,9 +48,6 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
     
     /** Class logger. */
     @Nonnull private Logger log = LoggerFactory.getLogger(MapResourceToStateToken.class);
-    
-    /** Whether to create the output objects into which the message will be encoded. */
-    private boolean createOutputObjects;
 
     /** Whether an error constructing a state token is fatal. */
     private boolean errorFatal;
@@ -61,19 +57,6 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
 
     /** Remoted request structure. */
     @NonnullBeforeExec private byte[] target;
-
-    /**
-     * Sets whether to create the output {@link DDF} and {@link RemotedHttpServletResponse}.
-     * 
-     * <p>Defaults to false.</p>
-     * 
-     * @param flag flag to set
-     */
-    public void setCreateOutputObjects(final boolean flag) {
-        checkSetterPreconditions();
-        
-        createOutputObjects = flag;
-    }
     
     /**
      * Sets whether an error computing a state token should result in a fatal event.
@@ -120,15 +103,10 @@ public class MapResourceToStateToken extends AbstractApplicationAction {
     /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+
+        ensureOutputObjects();
         
         final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
-        
-        if (createOutputObjects && agentRequestContext.getOutput() == null) {
-            final DDF output = new DDF(null);
-            agentRequestContext.setOutput(output);
-            agentRequestContext.setRemotedHttpServletResponse(new RemotedHttpServletResponse(
-                    output.structure().addmember(RemotedHttpServletResponse.STRUCTURE_NAME)));
-        }
 
         // We do the crazy stuff to accomodate cookie-backed state management.
         try {
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 e06499d..fa49312 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,8 +26,10 @@ import org.opensaml.profile.context.navigate.CurrentOrPreviousEventLookup;
 
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.TokenConsumerContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.profile.AbstractAgentRequestAction;
+import net.shibboleth.sp.profile.ConsumerConstants;
 import net.shibboleth.sp.profile.SPConstants;
 
 /**
@@ -66,10 +68,12 @@ public class PrepareAgentErrorResponse extends AbstractAgentRequestAction {
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
         
-        DDF output = ensureAgentRequestContext().getOutput();
+        final AgentRequestContext agentContext = ensureAgentRequestContext();
+        
+        DDF output = agentContext.getOutput();
         if (output == null) {
             output = new DDF().structure();
-            ensureAgentRequestContext().setOutput(output);
+            agentContext.setOutput(output);
         }
 
         final EventContext eventCtx = eventContextLookupStrategy.apply(profileRequestContext);
@@ -81,10 +85,15 @@ public class PrepareAgentErrorResponse extends AbstractAgentRequestAction {
             output.addmember(SPConstants.EVENT).string(EventIds.MESSAGE_PROC_ERROR);
         }
         
-        final byte[] target = ensureAgentRequestContext().getTargetURL();
+        final byte[] target = agentContext.getTargetURL();
         if (target != null) {
             output.addmember(SPConstants.TARGET).unsafe_string(target);
         }
+        
+        final TokenConsumerContext tokenContext = agentContext.getSubcontext(TokenConsumerContext.class);
+        if (tokenContext != null && tokenContext.isPassive()) {
+            output.addmember(ConsumerConstants.PASSIVE).integer(1);
+        }
     }
 
 }
\ 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
new file mode 100644
index 0000000..9e4231c
--- /dev/null
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookie.java
@@ -0,0 +1,182 @@
+/*
+ * 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.profile.action.EventIds;
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+
+import com.google.common.escape.Escaper;
+import com.google.common.net.UrlEscapers;
+
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.net.CookieManager;
+import net.shibboleth.shared.net.URISupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.StringSupport;
+import net.shibboleth.sp.context.AgentRequestContext;
+import net.shibboleth.sp.context.TokenConsumerContext;
+import net.shibboleth.sp.ddf.DDF;
+import net.shibboleth.sp.messaging.RemotedHttpServletRequestResponseContext;
+import net.shibboleth.sp.messaging.RemotedHttpServletResponse;
+import net.shibboleth.sp.profile.AbstractApplicationAction;
+
+/**
+ * Action that processes a previously issued correlation cookie submitted with the request and
+ * extracts the passive status and message ID for use by subsequent validation steps.
+ * 
+ * <p>The information captured is stored in a {@link TokenConsumerContext} child of the
+ * {@link AgentRequestContext}.</p>
+ * 
+ * <p>The absence of state or the cookie are not treated as an error or fatal at this stage
+ * due to the existence of unsolicited responses.</p>
+ * 
+ * @event {@link EventIds#PROCEED_EVENT_ID}
+ * @event {@link EventIds#INVALID_MESSAGE}
+ */
+public class ProcessCorrelationCookie extends AbstractApplicationAction {
+    
+    /** Class logger. */
+    @Nonnull private Logger log = LoggerFactory.getLogger(ProcessCorrelationCookie.class);
+    
+    /** Cookie manager. */
+    @NonnullAfterInit private CookieManager cookieManager;
+    
+    /** Cookie prefix. */
+    @Nonnull private String cookiePrefix;
+        
+    /** Lookup strategy for state token value. */
+    @NonnullAfterInit private Function<ProfileRequestContext,String> stateTokenLookupStrategy;
+    
+    /** State token accompanying request. */
+    @NonnullBeforeExec private String stateToken;
+    
+    /** Constructor. */
+    public ProcessCorrelationCookie() {
+        cookiePrefix = IssueCorrelationCookie.DEFAULT_COOKIE_PREFIX;
+    }
+    
+    /**
+     * Sets the {@link CookieManager} to use.
+     * 
+     * @param manager cookie manager instance
+     */
+    public void setCookieManager(@Nonnull final CookieManager manager) {
+        checkSetterPreconditions();
+        
+        cookieManager = Constraint.isNotNull(manager, "CookieManager cannot be null");
+    }
+    
+    /**
+     * Sets the cookie prefix.
+     * 
+     * <p>Defaults to {@link IssueCorrelationCookie#DEFAULT_COOKIE_PREFIX}.</p>
+     * 
+     * @param prefix cookie prefix
+     */
+    public void setCookiePrefix(@Nonnull @NotEmpty final String prefix) {
+        checkSetterPreconditions();
+        
+        cookiePrefix = Constraint.isNotNull(StringSupport.trimOrNull(prefix), "Cookie prefix cannot be null or empty");
+    }
+    
+    /**
+     * Sets the lookup strategy for obtaining the request message's ID.
+     * 
+     * @param strategy lookup strategy
+     */
+    public void setStateTokenLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
+        checkSetterPreconditions();
+        
+        stateTokenLookupStrategy = Constraint.isNotNull(strategy, "Request ID lookup strategy cannot be null");
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    protected void doInitialize() throws ComponentInitializationException {
+        super.doInitialize();
+        
+        if (cookieManager == null) {
+            throw new ComponentInitializationException("CookieManager cannot be null");
+        } else if (stateTokenLookupStrategy == null) {
+            throw new ComponentInitializationException("State token lookup strategy cannot be null");
+        }
+    }    
+
+    /** {@inheritDoc} */
+    @Override
+    protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+        if (!super.doPreExecute(profileRequestContext)) {
+            return false;
+        }
+
+        stateToken = stateTokenLookupStrategy.apply(profileRequestContext);
+        if (stateToken == null) {
+            log.debug("{} No state token found in request, skipping correlation cookie processing", getLogPrefix());
+            return false;
+        }
+        
+        return true;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+
+        ensureOutputObjects();
+        final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
+
+        // We do the crazy stuff to accomodate the cookies being read and unset.
+        try {
+            RemotedHttpServletRequestResponseContext.loadCurrent(agentRequestContext.getRemotedHttpServletRequest(),
+                    agentRequestContext.getRemotedHttpServletResponse());
+
+            final Escaper escaper = UrlEscapers.urlFormParameterEscaper();
+            final String cookieName = cookiePrefix + escaper.escape(stateToken);
+
+            final String value = cookieManager.getCookieValue(cookieName, null);
+            if (value == null) {
+                log.debug("{} No correlation cookie found for state token '{}'", getLogPrefix(), stateToken);
+                return;
+            }
+
+            cookieManager.unsetCookie(cookieName);
+
+            final String decoded = URISupport.doURLDecode(value);
+            if (decoded != null && decoded.startsWith("T:")) {
+                agentRequestContext.ensureSubcontext(TokenConsumerContext.class).setPassive(true);
+            } else if (decoded == null || !decoded.startsWith("F:")) {
+                log.warn("{} Correlation cookie for state token '{}' had invalid value: ", getLogPrefix(), stateToken,
+                        decoded);
+                return;
+            }
+            
+            log.debug("{} Extracted request ID '{}' for state token '{}'", getLogPrefix(), decoded.substring(2), stateToken);
+
+            agentRequestContext.ensureSubcontext(TokenConsumerContext.class).setMessageCorrelationID(decoded.substring(2));
+        } finally {
+            RemotedHttpServletRequestResponseContext.clearCurrent();
+        }
+    }
+
+}
\ No newline at end of file
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java
index 76f7d19..5c86d9b 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java
@@ -71,8 +71,6 @@ public class IssueCorrelationCookieTest extends BaseAgplicationActionTest {
     public void setUp() throws ComponentInitializationException {
         super.beforeMethod();
         
-        passive = false;
-        requestId = TEST_STATE;
         request = new MockHttpServletRequest();
         response = new MockHttpServletResponse();
         
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookieTest.java
similarity index 52%
copy from sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java
copy to sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookieTest.java
index 76f7d19..bb7b045 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/IssueCorrelationCookieTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/profile/impl/ProcessCorrelationCookieTest.java
@@ -14,14 +14,10 @@
 
 package net.shibboleth.sp.profile.impl;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.function.Function;
-import java.util.function.Predicate;
 
 import javax.annotation.Nonnull;
 
-import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
@@ -32,35 +28,36 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import com.google.common.escape.Escaper;
+import com.google.common.net.UrlEscapers;
+
 import jakarta.servlet.http.Cookie;
 import net.shibboleth.idp.profile.testing.ActionTestingSupport;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
-import net.shibboleth.shared.codec.DecodingException;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.net.CookieManager;
 import net.shibboleth.shared.net.CookieManager.SameSiteValue;
 import net.shibboleth.shared.primitive.NonnullSupplier;
+import net.shibboleth.sp.context.TokenConsumerContext;
 import net.shibboleth.sp.ddf.DDF;
-import net.shibboleth.sp.profile.SPConstants;
 
 /**
- * Unit test for {@link IssueCorrelationCookie} action.
+ * Unit test for {@link ProcessCorrelationCookie} action.
  */
 @SuppressWarnings("javadoc")
-public class IssueCorrelationCookieTest extends BaseAgplicationActionTest {
+public class ProcessCorrelationCookieTest extends BaseAgplicationActionTest {
 
     @Nonnull @NotEmpty private final static String TEST_STATE = "foo";
     @Nonnull @NotEmpty private final static String TEST_ID = "123456789";
 
-    private String requestId;
-    private boolean passive;
+    private String state;
     
     private DDF input;
     private MockHttpServletRequest request;
     private MockHttpServletResponse response;
     
     private CookieManager cookieManager;
-    private IssueCorrelationCookie action;
+    private ProcessCorrelationCookie action;
         
     /**
      * Set up test.
@@ -71,8 +68,6 @@ public class IssueCorrelationCookieTest extends BaseAgplicationActionTest {
     public void setUp() throws ComponentInitializationException {
         super.beforeMethod();
         
-        passive = false;
-        requestId = TEST_STATE;
         request = new MockHttpServletRequest();
         response = new MockHttpServletResponse();
         
@@ -84,23 +79,14 @@ public class IssueCorrelationCookieTest extends BaseAgplicationActionTest {
         cookieManager.setMaxAge(-1);
         cookieManager.initialize();
         
-        action = new IssueCorrelationCookie();
+        action = new ProcessCorrelationCookie();
         action.setCookieManager(cookieManager);
-        action.setPassiveRequestPredicate(new Predicate<>() {
-            public boolean test(ProfileRequestContext t) {
-                return passive;
-            }});
-        action.setRequestIDLookupStrategy(new Function<>() {
+        action.setStateTokenLookupStrategy(new Function<>() {
             public String apply(ProfileRequestContext t) {
-                return requestId;
+                return state;
             }
         });
-        
-        action.setErrorFatal(true);
         action.initialize();
-
-        input = new DDF(null).structure();
-        arc.setInput(input);
     }
     
     /**
@@ -115,59 +101,38 @@ public class IssueCorrelationCookieTest extends BaseAgplicationActionTest {
     @DataProvider
     Object[][] correlationData() {
         return new Object[][] {
-            new Object[] { null, TEST_ID, false},
-            new Object[] { TEST_STATE, null, false},
-            new Object[] { TEST_STATE, TEST_ID, false},
-            new Object[] { TEST_STATE, TEST_ID, true},
+            new Object[] {false, null, null, false},
+            new Object[] {false, TEST_STATE, null, false},
+            new Object[] {true, null, null, false},
+            new Object[] {true, TEST_STATE, TEST_ID, false},
+            new Object[] {true, TEST_STATE, TEST_ID, true},
         };
     }
         
     @Test(dataProvider="correlationData")
-    public void testAction(final String state, final String id, final Boolean passiveFlag) {
-        evaluateAction(state, id, passiveFlag);
-    }
-    
-    private void evaluateAction(final String state, final String id, final Boolean passiveFlag) {
-        passive = passiveFlag;
-        requestId = id;
-        if (state != null) {
-            input.addmember(SPConstants.STATE).string(state);
+    public void testAction(final Boolean createCookie, final String token, final String id, final Boolean passiveFlag) {
+        state = token;
+
+        if (createCookie && token != null) {
+            final Escaper escaper = UrlEscapers.urlFormParameterEscaper();
+            final Cookie cookie = new Cookie(IssueCorrelationCookie.DEFAULT_COOKIE_PREFIX + escaper.escape(token),
+                    (passiveFlag ? "T:" : "F:") + id);
+            request.setCookies(cookie);
         }
-        
+
         final Event event = action.execute(src);
+        ActionTestingSupport.assertProceedEvent(event);
         
-        if (state != null) {
-            ActionTestingSupport.assertProceedEvent(event);
+        Assert.assertEquals(response.getCookies().length, createCookie && token != null ? 1 : 0);
+
+        final TokenConsumerContext tcc = arc.getSubcontext(TokenConsumerContext.class);
+        if (id != null) {
+            assert tcc != null;
+            Assert.assertEquals(tcc.isPassive(), passiveFlag);
+            Assert.assertEquals(tcc.getMessageCorrelationID(), id);
         } else {
-            ActionTestingSupport.assertEvent(event, EventIds.INVALID_MESSAGE);
-            return;
+            Assert.assertNull(tcc);
         }
-        
-        if (id == null) {
-            Assert.assertEquals(response.getCookies().length, 0);
-            return;
-        }
-        
-        final Cookie cookie = response.getCookie(IssueCorrelationCookie.DEFAULT_COOKIE_PREFIX + state);
-        assert cookie != null;
-        Assert.assertEquals(cookie.getValue(), (passive ? "T:" : "F:") + requestId);
-        Assert.assertEquals(cookie.getMaxAge(), -1);
-        Assert.assertEquals(cookie.getAttribute("SameSite"), SameSiteValue.None.getValue());
     }
     
-    @Test
-    public void testPurge() throws ComponentInitializationException, DecodingException, InterruptedException {
-        
-        final List<Cookie> cookies = new ArrayList<>(12);
-        for (int i = 0; i < 12; ++i) {
-            cookies.add(new Cookie(IssueCorrelationCookie.DEFAULT_COOKIE_PREFIX + i, "foo" + i));
-            Thread.sleep(250);
-        }
-        request.setCookies(cookies.toArray(new Cookie[12]));
-        
-        evaluateAction(TEST_STATE, TEST_ID, false);
-        
-        Assert.assertEquals(response.getCookies().length, 3);
-    }
-
 }
\ 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