[java-plugin-shibd] 01/02: Establish output convention and add auth caching support.

Scott Cantor cantor.2 at osu.edu
Thu Jan 23 14:31:29 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=7e8cdc44f992c22ec04236216b1062c02433edf1

commit 7e8cdc44f992c22ec04236216b1062c02433edf1
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Jan 23 09:31:04 2025 -0500

    Establish output convention and add auth caching support.
---
 .../shibboleth/sp/flows/AbstractSPFlowTest.java    | 23 ++++++++-----
 .../java/net/shibboleth/sp/flows/PingFlowTest.java |  6 ++--
 .../net/shibboleth/sp/flows/SealerFlowTest.java    |  6 ++--
 .../net/shibboleth/sp/flows/StorageFlowTest.java   | 16 ++++-----
 .../src/main/java/net/shibboleth/sp/ddf/DDF.java   |  3 +-
 .../net/shibboleth/sp/profile/impl/DoPing.java     |  3 +-
 .../sp/profile/impl/EncodeAgentResponse.java       | 38 +++++++++++++++++++---
 7 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/AbstractSPFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/AbstractSPFlowTest.java
index 81e8085..63a3651 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/AbstractSPFlowTest.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/AbstractSPFlowTest.java
@@ -151,6 +151,17 @@ public abstract class AbstractSPFlowTest extends AbstractFlowTest {
         setRequest("POST", input);
     }
 
+    /**
+     * Checks that an output {@link DDF} was prepared, packaged in the response, and contains a success event.
+     * 
+     * @param result flow result
+     * 
+     * @return output object
+     */
+    @Nullable protected DDF assertOutputMessageSuccess(@Nonnull final FlowExecutionResult result) {
+        return assertOutputMessageEvent(result, "success");
+    }
+    
     /**
      * Checks that an output {@link DDF} was prepared, packaged in the response, and contains an event
      * that matches the specified value.
@@ -162,17 +173,13 @@ public abstract class AbstractSPFlowTest extends AbstractFlowTest {
      */
     @Nullable protected DDF assertOutputMessageEvent(@Nonnull final FlowExecutionResult result, @Nullable final String eventId) {
         
-        Assert.assertEquals(response.getContentType(), "text/plain");
-        
         final byte[] body = response.getContentAsByteArray();
         if (body == null || body.length == 0) {
-            if (eventId != null) {
-                Assert.fail("No response body");
-            } else {
-                return null;
-            }
+            Assert.fail("No response body");
         }
-        
+
+        Assert.assertEquals(response.getContentType(), "text/plain");
+
         try (final InputStream in = new ByteArrayInputStream(body)) {
             final DDF obj = DDF.deserialize(in);
             if (eventId != null) {
diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/PingFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/PingFlowTest.java
index 0a5627d..9553fa9 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/PingFlowTest.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/PingFlowTest.java
@@ -75,6 +75,7 @@ public class PingFlowTest extends AbstractSPFlowTest {
 
     /**
      * Test flow success.
+     * 
      * @throws IOException 
      */
     @Test
@@ -84,8 +85,9 @@ public class PingFlowTest extends AbstractSPFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
-        final DDF obj = assertOutputMessageEvent(result, null);
-        final Long time = obj != null ? obj.longinteger() : null;
+        final DDF obj = assertOutputMessageSuccess(result);
+        assert obj != null;
+        final Long time = obj.getmember("epoch").longinteger();
         Assert.assertTrue(time != null && time <= Instant.now().toEpochMilli() / 1000);
     }
     
diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SealerFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SealerFlowTest.java
index abd7078..fcc3289 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SealerFlowTest.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SealerFlowTest.java
@@ -88,7 +88,7 @@ public class SealerFlowTest extends AbstractSPFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
-        final DDF obj = assertOutputMessageEvent(result, null);
+        final DDF obj = assertOutputMessageSuccess(result);
         assert obj != null;
         
         final String wrapped = obj.getmember("value").string();
@@ -118,7 +118,7 @@ public class SealerFlowTest extends AbstractSPFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
-        final DDF obj = assertOutputMessageEvent(result, null);
+        final DDF obj = assertOutputMessageSuccess(result);
         assert obj != null;
         
         final String wrapped = obj.getmember("value").string();
@@ -180,7 +180,7 @@ public class SealerFlowTest extends AbstractSPFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
-        final DDF obj = assertOutputMessageEvent(result, null);
+        final DDF obj = assertOutputMessageSuccess(result);
         assert obj != null;
         
         final String unwrapped = obj.getmember("value").string();
diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
index 24938b1..a01fe72 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
@@ -137,7 +137,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
-        final DDF output = assertOutputMessageEvent(result, null);
+        final DDF output = assertOutputMessageSuccess(result);
         assert output != null;
         
         Assert.assertEquals(output.getmember(DoStorageOperation.VALUE).string(), TEST_VALUE);
@@ -190,8 +190,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
         
-        final DDF output = assertOutputMessageEvent(result, null);
-        Assert.assertNull(output);
+        assertOutputMessageSuccess(result);
         
         Assert.assertNull(getStorageService().read(AGENT_CONTEXT, TEST_KEY));
     }
@@ -215,8 +214,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
         
-        final DDF output = assertOutputMessageEvent(result, null);
-        Assert.assertNull(output);
+        assertOutputMessageSuccess(result);
         
         final StorageRecord<?> record = getStorageService().read(AGENT_CONTEXT, TEST_KEY);
         assert record != null;
@@ -268,8 +266,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
         
-        final DDF output = assertOutputMessageEvent(result, null);
-        Assert.assertNull(output);
+        assertOutputMessageSuccess(result);
         
         final StorageRecord<?> record = getStorageService().read(AGENT_CONTEXT, TEST_KEY);
         assert record != null;
@@ -296,8 +293,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
 
-        final DDF output = assertOutputMessageEvent(result, null);
-        Assert.assertNull(output);
+        assertOutputMessageSuccess(result);
         
         final StorageRecord<?> record = getStorageService().read(AGENT_CONTEXT, TEST_KEY);
         assert record != null;
@@ -327,7 +323,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
         assertFlowExecutionResult(result, FLOW_ID);
         assertFlowExecutionOutcome(result.getOutcome());
         
-        final DDF output = assertOutputMessageEvent(result, null);
+        final DDF output = assertOutputMessageSuccess(result);
         assert output != null;
         Assert.assertEquals(output.getmember(DoStorageOperation.VERSION).longinteger(), 2);
         
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/ddf/DDF.java b/sp-server-api/src/main/java/net/shibboleth/sp/ddf/DDF.java
index f9ed8ca..afaaec6 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/ddf/DDF.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/ddf/DDF.java
@@ -957,7 +957,8 @@ public class DDF implements Iterable<DDF> {
      * 
      * <p>The input path MUST contain at least one non-empty path segment.</p>
      * 
-     * <p>This node will be converted to a structure if not already one.</p>
+     * <p>This node will be converted to a structure if not already one unless it starts
+     * out as null.</p>
      * 
      * @param path dotted path to use
      * 
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoPing.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoPing.java
index ae3a234..c27649a 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoPing.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoPing.java
@@ -32,7 +32,8 @@ public class DoPing extends AbstractAgentRequestAction {
     @Override
     protected void doExecute(@Nonnull ProfileRequestContext profileRequestContext) {
 
-        final DDF out = new DDF().addmember("epoch").longinteger(Instant.now().getEpochSecond());
+        final DDF out = new DDF().structure();
+        out.addmember("epoch").longinteger(Instant.now().getEpochSecond());
         ensureAgentRequestContext().setOutput(out);
     }
     
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 ac4d234..f6ac184 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
@@ -24,9 +24,11 @@ import org.opensaml.profile.action.EventIds;
 import org.opensaml.profile.context.ProfileRequestContext;
 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.primitive.LoggerFactory;
+import net.shibboleth.sp.Agent;
 import net.shibboleth.sp.context.AgentRequestContext;
 import net.shibboleth.sp.ddf.DDF;
 import net.shibboleth.sp.profile.AbstractAgentRequestAction;
@@ -35,11 +37,17 @@ import net.shibboleth.sp.profile.AbstractAgentRequestAction;
  * 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> 
+ * 
+ * <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>
+ * 
  * @event {@link EventIds#PROCEED_EVENT_ID}
  * @event {@link EventIds#INVALID_PROFILE_CTX}
  * @event {@link EventIds#INVALID_MESSAGE}
  * @event {@link EventIds#IO_ERROR}
- * @pre <pre>AgentRequestContext.getOutput() != null</pre>
+ * @pre <pre>AgentRequestContext.getOutput().getmember("event").isstring()</pre>
  */
 public class EncodeAgentResponse extends AbstractAgentRequestAction {
 
@@ -48,7 +56,7 @@ public class EncodeAgentResponse extends AbstractAgentRequestAction {
 
     /** Cached context containing the output message. */
     @NonnullBeforeExec private DDF outputMessage;
-
+    
     /** {@inheritDoc} */
     @Override
     protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
@@ -59,10 +67,17 @@ public class EncodeAgentResponse extends AbstractAgentRequestAction {
         
         final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
         outputMessage = agentRequestContext.getOutput();
-        if (outputMessage == null) {
-            log.debug("{} No output message found, creating empty one", getLogPrefix());
+        if (outputMessage != null) {
+            if (!outputMessage.isstruct()) {
+                log.error("{} Output message was not a structure", getLogPrefix());
+                ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+                return false;
+            }
+        } else {
+            log.debug("{} No output message found, creating an empty structure signalling success", getLogPrefix());
             agentRequestContext.setOutput(new DDF());
             outputMessage = agentRequestContext.getOutput();
+            outputMessage.structure();
         }
         
         return true;
@@ -79,6 +94,21 @@ public class EncodeAgentResponse extends AbstractAgentRequestAction {
             return;
         }
         
+        if (outputMessage.getmember("event").isnull()) {
+            outputMessage.addmember("event").string("success");
+        }
+        
+        final Agent agent = ensureAgentRequestContext().getAgent();
+        if (agent != null && agent.isSupportsCachedAuthentication()) {
+            final HttpServletRequest request = getHttpServletRequest();
+            if (request != null) {
+                final String sessionID = request.getSession(true).getId();
+                if (sessionID != null) {
+                    outputMessage.addmember("cached_auth").string(sessionID);
+                }
+            }
+        }
+        
         response.setContentType("text/plain");
 
         try (final OutputStream out = response.getOutputStream()) {

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


More information about the commits mailing list