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

Scott Cantor cantor.2 at osu.edu
Mon Apr 14 16:20:53 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=284e26fd926d5a8f93322201d6ce3e8013c7a343

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

commit 284e26fd926d5a8f93322201d6ce3e8013c7a343
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Apr 14 12:20:50 2025 -0400

    JSHIBDSAML-1 - Request/response correlation and passive tracking
    
    https://shibboleth.atlassian.net/browse/JSHIBDSAML-1
    
    Wire in action to produce cookie.
    Add some missing properties.
    Clean up Set-Cookie header generation.
    Adjust tests.
---
 .../shibboleth/idp/module/conf/sp/sp.properties    |  6 ++-
 .../net/shibboleth/sp/conf/agents-system.xml       |  3 +-
 .../sp/messaging/RemotedHttpServletResponse.java   | 50 ++++++++++------------
 .../sp/impl/CookieStateTokenManager.java           |  2 +-
 .../sp/profile/impl/IssueCorrelationCookie.java    | 28 +++++++++++-
 .../impl/RemotedHttpServletResponseTest.java       |  2 +-
 6 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties b/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
index 2e817bc..5d8f41d 100644
--- a/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
+++ b/sp-conf-impl/src/main/resources/net/shibboleth/idp/module/conf/sp/sp.properties
@@ -30,10 +30,14 @@ sp.service.agents.checkInterval = PT5M
 # Controls how many cookies for a given use case are allowed before purging
 #sp.cookie.limit = 10
 
-
 # Default state token management (SAML RelayState, etc.)
 #sp.stateToken.errorsFatal = false
 # Set to shibboleth.CookieStateTokenManager to switch to cookie-based mechanism
 #sp.stateToken.Manager = shibboleth.StorageStateTokenManager
 # Controls storage back-end for storage-based state tokens
 #sp.stateToken.StorageService = shibboleth.StorageService
+# Cookie prefix when using cookie-backed state
+#sp.stateToken.cookiePrefix = _Host-shibsp_state_
+
+# Request/response correlation control
+#sp.correlation.cookiePrefix = _Host-_shibsp_req_
diff --git a/sp-conf-impl/src/main/resources/net/shibboleth/sp/conf/agents-system.xml b/sp-conf-impl/src/main/resources/net/shibboleth/sp/conf/agents-system.xml
index b8e9b30..c1ca5f6 100644
--- a/sp-conf-impl/src/main/resources/net/shibboleth/sp/conf/agents-system.xml
+++ b/sp-conf-impl/src/main/resources/net/shibboleth/sp/conf/agents-system.xml
@@ -52,7 +52,8 @@
         p:storageService-ref="#{'%{sp.stateToken.StorageService:shibboleth.StorageService}'.trim()}" />
 
     <bean id="shibboleth.CookieStateTokenManager" class="net.shibboleth.sp.impl.CookieStateTokenManager" lazy-init="true"
-        p:cookieManager-ref="shibboleth.RemotedCookieManager" />
+        p:cookieManager-ref="shibboleth.RemotedCookieManager"
+        p:cookiePrefix="%{sp.stateToken.cookiePrefix:_Host-shibsp_state_}" />
 
     <!-- Wildcard import hook for plugins. -->
     <import resource="classpath*:/META-INF/net/shibboleth/sp/service/agent/postconfig.xml" />
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletResponse.java b/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletResponse.java
index 2377db0..21d3b66 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletResponse.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/messaging/RemotedHttpServletResponse.java
@@ -53,6 +53,12 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
     /** Field holding redirect location. */
     @Nonnull @NotEmpty public static final String REDIRECT = "redirect";
 
+    /** Field holding response data. */
+    @Nonnull @NotEmpty public static final String RESPONSE = "response";
+
+    /** Field holding header collection. */
+    @Nonnull @NotEmpty public static final String HEADERS = "headers";
+
     /** Underlying object for remoted data. */
     @Nonnull private final DDF obj;
     
@@ -87,7 +93,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
     
     /** {@inheritDoc} */
     public String getCharacterEncoding() {
-        return "UTF-8";
+        return StandardCharsets.UTF_8.name();
     }
 
     /** {@inheritDoc} */
@@ -114,7 +120,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
 
     /** {@inheritDoc} */
     public void setCharacterEncoding(final String charset) {
-        if (!"UTF-8".equals(charset)) {
+        if (!StandardCharsets.UTF_8.name().equals(charset)) {
             throw new IllegalArgumentException("Character set must be UTF-8.");
         }
     }
@@ -187,32 +193,20 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
 
     /** {@inheritDoc} */
     public void addCookie(final Cookie cookie) {
-        // The C++ side already manages SameSite independently, so we'll likely continue that.
         final StringBuffer buffer = new StringBuffer(cookie.getName()).append('=');
         if (cookie.getValue() != null) {
             buffer.append(cookie.getValue());
         }
-        if (cookie.getMaxAge() >= 0) {
-            buffer.append("; MaxAge=").append(cookie.getMaxAge());
-        }
-        if (cookie.getPath() != null) {
-            buffer.append("; ").append("Path=").append(cookie.getPath());
-        }
-        if (cookie.getDomain() != null) {
-            buffer.append("; ").append("Domain=").append(cookie.getDomain());
-        }
-        if (cookie.getSecure()) {
-            buffer.append("; Secure");
-        }
-        if (cookie.isHttpOnly()) {
-            buffer.append("; HttpOnly");
-        }
+        // All of the cookie properties are part of the "attributes" now.
+        cookie.getAttributes().forEach((n, v) -> {
+            buffer.append("; ").append(n).append('=').append(v);
+        });
         addHeader("Set-Cookie", buffer.toString());
     }
 
     /** {@inheritDoc} */
     public boolean containsHeader(final String name) {
-        for (final DDF header : obj.getmember("headers").asList()) {
+        for (final DDF header : obj.getmember(HEADERS).asList()) {
             if (name.equals(header.name())) {
                 return true;
             }
@@ -255,7 +249,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
             throw new IllegalStateException("Response already committed");
         }
 
-        obj.getmember("response").remove();
+        obj.getmember(RESPONSE).remove();
         obj.addmember(REDIRECT).unsafe_string(location);
         committed = true;
         outputStream = null;
@@ -310,19 +304,19 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
 
     /** {@inheritDoc} */
     public void setStatus(final int sc) {
-        obj.addmember("response.status").integer(sc);
+        obj.addmember(RESPONSE + ".status").integer(sc);
     }
 
     /** {@inheritDoc} */
     public int getStatus() {
-        final Integer i = obj.getmember("response.status").integer();
+        final Integer i = obj.getmember(RESPONSE + ".status").integer();
         return i != null ? i : -1;
     }
 
     /** {@inheritDoc} */
     public String getHeader(final String name) {
         final Optional<DDF> header =
-                obj.getmember("headers").asList()
+                obj.getmember(HEADERS).asList()
                     .stream()
                     .filter(ddf -> name.equalsIgnoreCase(ddf.name()))
                     .findFirst();
@@ -341,7 +335,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
 
     /** {@inheritDoc} */
     public Collection<String> getHeaders(final String name) {
-        return obj.getmember("headers").asList()
+        return obj.getmember(HEADERS).asList()
             .stream()
             .filter(ddf -> name.equalsIgnoreCase(ddf.name()))
             .map(DDF::string)
@@ -350,7 +344,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
 
     /** {@inheritDoc} */
     public Collection<String> getHeaderNames() {
-        return obj.getmember("headers").asList()
+        return obj.getmember(HEADERS).asList()
                 .stream()
                 .map(DDF::name)
                 .collect(Collectors.toUnmodifiableSet());
@@ -369,7 +363,7 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
         
         // This is safe because the asList copy is divorced from the original list
         // but the DDF child objects are the same.
-        obj.getmember("headers").asList()
+        obj.getmember(HEADERS).asList()
             .stream()
             .filter(ddf -> name.equalsIgnoreCase(ddf.name()))
             .forEach(DDF::remove);
@@ -386,11 +380,11 @@ public class RemotedHttpServletResponse implements HttpServletResponse {
             throw new IllegalStateException("Response already committed");
         }
         
-        final DDF headers = obj.getmember("headers");
+        final DDF headers = obj.getmember(HEADERS);
         if (headers.islist()) {
             return headers;
         }
-        return obj.addmember("headers").list();
+        return obj.addmember(HEADERS).list();
     }
 
     /** Wrapper allowing use of containers of arrays. */
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/impl/CookieStateTokenManager.java b/sp-server-impl/src/main/java/net/shibboleth/sp/impl/CookieStateTokenManager.java
index 26e936d..0adae11 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/impl/CookieStateTokenManager.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/impl/CookieStateTokenManager.java
@@ -48,7 +48,7 @@ import net.shibboleth.sp.StateTokenManager;
 public class CookieStateTokenManager extends AbstractStateTokenManager {
     
     /** Default cookie prefix. */
-    @Nonnull @NotEmpty public static String DEFAULT_PREFIX = "_shibsp_state";
+    @Nonnull @NotEmpty public static String DEFAULT_PREFIX = "_shibsp_state_";
     
     /** Class logger. */
     @Nonnull private Logger log = LoggerFactory.getLogger(CookieStateTokenManager.class);
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 4e58aa2..d6bd7d0 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,6 +39,7 @@ 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,6 +67,9 @@ 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;
     
@@ -113,6 +117,19 @@ 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.
@@ -202,10 +219,17 @@ public class IssueCorrelationCookie extends AbstractApplicationAction {
     /** {@inheritDoc} */
     @Override
     protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-        
+
+        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 {
-            final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
             RemotedHttpServletRequestResponseContext.loadCurrent(agentRequestContext.getRemotedHttpServletRequest(),
                     agentRequestContext.getRemotedHttpServletResponse());
 
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/messaging/impl/RemotedHttpServletResponseTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/messaging/impl/RemotedHttpServletResponseTest.java
index b7c93f0..f7bab6e 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/messaging/impl/RemotedHttpServletResponseTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/messaging/impl/RemotedHttpServletResponseTest.java
@@ -70,7 +70,7 @@ public class RemotedHttpServletResponseTest {
         final DDF headers = obj.getmember("headers");
         assertTrue(headers.islist());
         final DDF cheader = headers.asList().stream().filter(ddf -> "Set-Cookie".equalsIgnoreCase(ddf.name())).findFirst().orElseThrow();
-        assertEquals(cheader.string(), "cookie1=value1; Path=/idp; Secure; HttpOnly");
+        assertEquals(cheader.string(), "cookie1=value1; HttpOnly=true; Path=/idp; Secure=true");
     }
 
     @Test

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


More information about the commits mailing list