[java-plugin-shibd-saml] branch dev/StateMgmtWIP updated: Redo state handling in logout initiator flow.
Codeberg
noreply at shibboleth.net
Tue Apr 28 16:18:32 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch dev/StateMgmtWIP
in repository java-plugin-shibd-saml.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd-saml/commit/97f533a1b7cecf8cee200ff74515d4d308dc4d95
The following commit(s) were added to refs/heads/dev/StateMgmtWIP by this push:
new 97f533a Redo state handling in logout initiator flow.
97f533a is described below
commit 97f533a1b7cecf8cee200ff74515d4d308dc4d95
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Tue Apr 28 12:18:09 2026 -0400
Redo state handling in logout initiator flow.
---
.../flows/sp/logout/initiator/saml2/saml2-flow.xml | 1 +
.../flows/saml2/SAML2LogoutInitiatorFlowTest.java | 83 ++++++----------------
.../flows/saml2/SAML2SessionInitiatorFlowTest.java | 30 ++++----
.../saml/saml2/profile/impl/AddAuthnRequest.java | 8 +--
.../saml/saml2/profile/impl/AddLogoutRequest.java | 60 +++++++++++++---
5 files changed, 91 insertions(+), 91 deletions(-)
diff --git a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/logout/initiator/saml2/saml2-flow.xml b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/logout/initiator/saml2/saml2-flow.xml
index ec8dfb3..783d3d8 100644
--- a/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/logout/initiator/saml2/saml2-flow.xml
+++ b/sp-saml-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/logout/initiator/saml2/saml2-flow.xml
@@ -24,6 +24,7 @@
<evaluate expression="EncryptNameIDs" />
<evaluate expression="HandleOutboundMessage" />
+ <evaluate expression="PreserveRelayState" />
<evaluate expression="EncodeMessage" />
<evaluate expression="'proceed'" />
diff --git a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2LogoutInitiatorFlowTest.java b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2LogoutInitiatorFlowTest.java
index d65f684..3451adc 100644
--- a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2LogoutInitiatorFlowTest.java
+++ b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2LogoutInitiatorFlowTest.java
@@ -209,30 +209,6 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
assertFlowExecutionOutcome(result.getOutcome());
assertOutputMessageEvent(result, AuthnEventIds.NO_POTENTIAL_FLOW);
}
-
- /**
- * Test simple success case with preset relay state.
- *
- * @throws IOException
- * @throws MessageDecodingException
- */
- @Test
- public void testSimpleWithState() throws IOException, MessageDecodingException {
- setDefaultAuth();
-
- final DDF input = new DDF(null).structure();
- input.addmember(ConsumerConstants.SESSION_OPAQUE).addmember(PrepareAgentResponse.NAMEID_PARAM).string(SESSION_DATA);
- input.addmember(RemotedHttpServletRequest.STRUCTURE_NAME).structure();
- input.addmember(SPConstants.STATE).string("foostate");
- setApplicationRequest(APPLICATION_ID, input);
-
- final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
- assertFlowExecutionResult(result, FLOW_ID);
- assertFlowExecutionOutcome(result.getOutcome());
-
- assertOutputMessageSuccess(result);
- validateOutputMessage(result, "12345", false);
- }
/**
* Test simple success case with encryption enabled.
@@ -247,7 +223,7 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
final DDF input = new DDF(null).structure();
input.addmember(ConsumerConstants.SESSION_OPAQUE).addmember(PrepareAgentResponse.NAMEID_PARAM).string(SESSION_DATA);
input.addmember(RemotedHttpServletRequest.STRUCTURE_NAME).structure();
- input.addmember(SPConstants.STATE).string("foostate");
+ input.addmember(SPConstants.TARGET).unsafe_string(RESOURCE_URL);
setApplicationRequest("logout-encryption", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -257,31 +233,6 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
assertOutputMessageSuccess(result);
validateOutputMessage(result, "12345", true);
}
-
- /**
- * Test simple success case with preset relay state.
- *
- * @throws IOException
- * @throws MessageDecodingException
- */
- @Test
- public void testSimpleWithStateNoIndex() throws IOException, MessageDecodingException {
- setDefaultAuth();
-
- final DDF input = new DDF(null).structure();
- input.addmember(ConsumerConstants.SESSION_OPAQUE).addmember(PrepareAgentResponse.NAMEID_PARAM).string(SESSION_DATA_NO_INDEX);
- input.addmember(RemotedHttpServletRequest.STRUCTURE_NAME).structure();
- input.addmember(SPConstants.STATE).string("foostate");
- setApplicationRequest(APPLICATION_ID, input);
-
- final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
- assertFlowExecutionResult(result, FLOW_ID);
- assertFlowExecutionOutcome(result.getOutcome());
-
- assertOutputMessageSuccess(result);
- validateOutputMessage(result, null, false);
- }
-
/**
* Test simple success case with computed relay state.
@@ -290,7 +241,7 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
* @throws MessageDecodingException
*/
@Test
- public void testSimpleWithoutState() throws IOException, MessageDecodingException {
+ public void testSimple() throws IOException, MessageDecodingException {
setDefaultAuth();
final DDF input = new DDF(null).structure();
@@ -324,9 +275,9 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
final ProfileRequestContext prc = retrieveProfileRequestContext(result);
assert prc != null;
final AgentRequestContext arc = prc.ensureSubcontext(AgentRequestContext.class);
- final DDF input = arc.getInput();
final DDF output = arc.getOutput();
-
+ final DDF input = arc.getInput();
+ assert input != null;
assert output != null;
Assert.assertTrue(output.isstruct());
final DDF http = output.getmember(RemotedHttpServletResponse.STRUCTURE_NAME);
@@ -336,8 +287,7 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
final byte[] redirect = http.getmember(RemotedHttpServletResponse.REDIRECT).unsafe_string();
if (redirect != null) {
final String redirectURL = new String(redirect, StandardCharsets.UTF_8);
- final SAMLObject saml = decodeRedirect(redirectURL,
- input != null ? input.getmember(SPConstants.STATE).string() : null);
+ final SAMLObject saml = decodeRedirect(redirectURL);
assert saml instanceof LogoutRequest;
logoutRequest = (LogoutRequest) saml;
Assert.assertTrue(redirectURL.startsWith(logoutRequest.getDestination()));
@@ -348,8 +298,6 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
final Object saml = prc.ensureOutboundMessageContext().ensureMessage();
assert saml instanceof LogoutRequest;
logoutRequest = (LogoutRequest) saml;
- Assert.assertEquals(SAMLBindingSupport.getRelayState(prc.ensureOutboundMessageContext()),
- input != null ? input.getmember(SPConstants.STATE).string() : null);
}
assert logoutRequest != null;
@@ -380,6 +328,23 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
assert exts != null;
Assert.assertEquals(exts.getUnknownXMLObjects(Asynchronous.DEFAULT_ELEMENT_NAME).size(), 1);
+ boolean foundCorrelationCookie = false;
+ for (final DDF header : http.getmember(RemotedHttpServletResponse.HEADERS)) {
+ if ("Set-Cookie".equals(header.name())) {
+ final String cookie = header.string();
+ assert cookie != null;
+ // TODO: Ideally we would extract the cookie value from the header and be able to
+ // test against a base64'd JSON string, but that's a fair bit of work.
+ if (cookie.startsWith("__Host-" + "shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_')) {
+ Assert.assertTrue(
+ cookie.startsWith("__Host-shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_'
+ + SAMLBindingSupport.getRelayState(prc.ensureOutboundMessageContext()) + '='));
+ foundCorrelationCookie = true;
+ }
+ }
+ }
+ Assert.assertTrue(foundCorrelationCookie);
+
return logoutRequest;
}
@@ -388,12 +353,11 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
* Decodes a SAML message encoded via HTTP-Redirect binding.
*
* @param url the encoded redirect
- * @param relayState RelayState to check for
*
* @return decoded message
* @throws MessageDecodingException
*/
- @Nonnull protected SAMLObject decodeRedirect(@Nullable final String url, @Nullable final String relayState)
+ @Nonnull protected SAMLObject decodeRedirect(@Nullable final String url)
throws MessageDecodingException {
final MockHttpServletRequest mock = new MockHttpServletRequest("GET", url);
final int index = url != null ? url.indexOf('?') : -1;
@@ -424,7 +388,6 @@ public class SAML2LogoutInitiatorFlowTest extends AbstractSPFlowTest {
decoder.destroy();
if (mc != null && mc.getMessage() instanceof SAMLObject saml) {
- Assert.assertEquals(SAMLBindingSupport.getRelayState(mc), relayState);
return saml;
}
throw new MessageDecodingException("No message, or incorrect type.");
diff --git a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2SessionInitiatorFlowTest.java b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2SessionInitiatorFlowTest.java
index e3c2ace..ad1de05 100644
--- a/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2SessionInitiatorFlowTest.java
+++ b/sp-saml-conf-impl/src/test/java/net/shibboleth/sp/saml/flows/saml2/SAML2SessionInitiatorFlowTest.java
@@ -554,7 +554,7 @@ public class SAML2SessionInitiatorFlowTest extends AbstractSPFlowTest {
final AgentRequestContext arc = prc.ensureSubcontext(AgentRequestContext.class);
final DDF input = arc.getInput();
final DDF output = arc.getOutput();
-
+ assert input != null;
assert output != null;
Assert.assertTrue(output.isstruct());
final DDF http = output.getmember(RemotedHttpServletResponse.STRUCTURE_NAME);
@@ -584,24 +584,22 @@ public class SAML2SessionInitiatorFlowTest extends AbstractSPFlowTest {
assert issuer != null;
Assert.assertEquals(issuer.getValue(), ISSUER);
- if (input != null) {
- boolean foundCorrelationCookie = false;
- for (final DDF header : http.getmember(RemotedHttpServletResponse.HEADERS)) {
- if ("Set-Cookie".equals(header.name())) {
- final String cookie = header.string();
- assert cookie != null;
- // TODO: Ideally we would extract the cookie value from the header and be able to
- // test against a base64'd JSON string, but that's a fair bit of work.
- if (cookie.startsWith("__Host-" + "shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_')) {
- Assert.assertTrue(
- cookie.startsWith("__Host-shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_'
- + SAMLBindingSupport.getRelayState(prc.ensureOutboundMessageContext()) + '='));
- foundCorrelationCookie = true;
- }
+ boolean foundCorrelationCookie = false;
+ for (final DDF header : http.getmember(RemotedHttpServletResponse.HEADERS)) {
+ if ("Set-Cookie".equals(header.name())) {
+ final String cookie = header.string();
+ assert cookie != null;
+ // TODO: Ideally we would extract the cookie value from the header and be able to
+ // test against a base64'd JSON string, but that's a fair bit of work.
+ if (cookie.startsWith("__Host-" + "shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_')) {
+ Assert.assertTrue(
+ cookie.startsWith("__Host-shibsp_state_" + input.getmember(SPConstants.APPLICATION).string() + '_'
+ + SAMLBindingSupport.getRelayState(prc.ensureOutboundMessageContext()) + '='));
+ foundCorrelationCookie = true;
}
}
- Assert.assertTrue(foundCorrelationCookie);
}
+ Assert.assertTrue(foundCorrelationCookie);
final NameIDPolicy pol = authnRequest.getNameIDPolicy();
assert pol != null;
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
index a8f943b..2da5ee0 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddAuthnRequest.java
@@ -133,9 +133,6 @@ public class AddAuthnRequest extends AbstractApplicationAction {
/** EntityID to populate into Issuer element. */
@Nullable private String issuerId;
- /** Stashed off target resource for state mgmt. */
- @Nullable private byte[] target;
-
/** Constructor. */
public AddAuthnRequest() {
// Default strategy is a 16-byte secure random source.
@@ -273,7 +270,7 @@ public class AddAuthnRequest extends AbstractApplicationAction {
}
// Get target URL from either existing StateData or input message.
- target = input.getmember(SPConstants.TARGET).unsafe_string();
+ byte[] target = input.getmember(SPConstants.TARGET).unsafe_string();
if (target == null) {
final StateData oldStateData = stateDataContext.getStateData();
if (oldStateData != null) {
@@ -286,6 +283,7 @@ public class AddAuthnRequest extends AbstractApplicationAction {
stateDataContext.setStateData(stateData);
assert rpCtx != null;
stateData.setAuthenticationAuthority(rpCtx.getRelyingPartyId());
+ stateData.setRawResource(target);
if (issuerLookupStrategy != null) {
issuerId = issuerLookupStrategy.apply(profileRequestContext);
@@ -363,8 +361,6 @@ public class AddAuthnRequest extends AbstractApplicationAction {
object.setScoping(buildScoping(profileRequestContext));
profileRequestContext.ensureOutboundMessageContext().setMessage(object);
-
- stateData.setRawResource(target);
if (profileConfiguration.isCheckAddress(profileRequestContext)) {
// We could create all the machinery to wrap this in a servlet interface, but...
diff --git a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
index 8fab808..19e8c2a 100644
--- a/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
+++ b/sp-saml-impl/src/main/java/net/shibboleth/sp/saml/saml2/profile/impl/AddLogoutRequest.java
@@ -27,11 +27,14 @@ import net.shibboleth.profile.context.navigate.IssuerLookupFunction;
import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.security.IdentifierGenerationStrategy;
+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.saml.saml2.SAMLStateData;
import net.shibboleth.sp.saml.saml2.context.SAMLLogoutContext;
import net.shibboleth.sp.saml.saml2.profile.config.SingleLogoutProfileConfiguration;
+import net.shibboleth.sp.state.StateData;
import org.opensaml.core.xml.XMLObjectBuilderFactory;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
@@ -39,12 +42,12 @@ import org.opensaml.core.xml.io.MarshallingException;
import org.opensaml.core.xml.io.UnmarshallingException;
import org.opensaml.core.xml.util.XMLObjectSupport;
import org.opensaml.messaging.context.MessageContext;
+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.saml.common.SAMLObjectBuilder;
import org.opensaml.saml.common.SAMLVersion;
-import org.opensaml.saml.common.binding.SAMLBindingSupport;
import org.opensaml.saml.ext.saml2aslo.Asynchronous;
import org.opensaml.saml.saml2.core.LogoutRequest;
import org.opensaml.saml.saml2.core.Extensions;
@@ -83,6 +86,9 @@ public class AddLogoutRequest extends AbstractApplicationAction {
/** Strategy used to locate the {@link IdentifierGenerationStrategy} to use. */
@Nonnull private Function<ProfileRequestContext,IdentifierGenerationStrategy> idGeneratorLookupStrategy;
+ /** Strategy used to create the {@link StateDataContext} to populate. */
+ @Nonnull private Function<ProfileRequestContext,StateDataContext> stateDataContextCreationStrategy;
+
/** Strategy used to obtain the request issuer value. */
@Nullable private Function<ProfileRequestContext,String> issuerLookupStrategy;
@@ -95,6 +101,9 @@ public class AddLogoutRequest extends AbstractApplicationAction {
/** Cached logout context. */
@NonnullBeforeExec private SAMLLogoutContext logoutContext;
+ /** Cached state data object to populate. */
+ @NonnullBeforeExec private SAMLStateData stateData;
+
/** EntityID to populate into Issuer element. */
@Nullable private String issuerId;
@@ -102,7 +111,7 @@ public class AddLogoutRequest extends AbstractApplicationAction {
public AddLogoutRequest() {
// Default strategy is a 16-byte secure random source.
idGeneratorLookupStrategy = new IdentifierGenerationStrategyLookupFunction();
-
+ stateDataContextCreationStrategy = new ChildContextLookup<>(StateDataContext.class, true);
issuerLookupStrategy = new IssuerLookupFunction();
}
@@ -128,6 +137,18 @@ public class AddLogoutRequest extends AbstractApplicationAction {
Constraint.isNotNull(strategy, "IdentifierGenerationStrategy lookup strategy cannot be null");
}
+ /**
+ * 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");
+ }
+
/**
* Set the strategy used to locate the issuer value to use.
*
@@ -181,6 +202,30 @@ public class AddLogoutRequest extends AbstractApplicationAction {
return false;
}
+ final StateDataContext stateDataContext = stateDataContextCreationStrategy.apply(profileRequestContext);
+ if (stateDataContext == null) {
+ log.error("{} Error creating StateDataContext", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+ return false;
+ }
+
+ // Get target URL from either existing StateData or input message.
+ final DDF input = ensureAgentRequestContext().getInput();
+ byte[] target = input != null ? input.getmember(SPConstants.TARGET).unsafe_string() : null;
+ if (target == null) {
+ final StateData oldStateData = stateDataContext.getStateData();
+ if (oldStateData != null) {
+ target = oldStateData.getRawResource();
+ }
+ }
+
+ // Establish fresh StateData of the correct type.
+ stateData = new SAMLStateData();
+ stateDataContext.setStateData(stateData);
+ assert rpCtx != null;
+ stateData.setAuthenticationAuthority(rpCtx.getRelyingPartyId());
+ stateData.setRawResource(target);
+
if (issuerLookupStrategy != null) {
issuerId = issuerLookupStrategy.apply(profileRequestContext);
}
@@ -207,6 +252,9 @@ public class AddLogoutRequest extends AbstractApplicationAction {
// This might need to be configurable if the action is reused for some non-user purpose.
object.setReason(LogoutRequest.USER_REASON);
+ stateData.setRequestID(object.getID());
+ stateData.setRequestTime(object.getIssueInstant());
+
if (issuerId != null) {
log.debug("{} Setting Issuer to {}", getLogPrefix(), issuerId);
final SAMLObjectBuilder<Issuer> issuerBuilder =
@@ -214,6 +262,7 @@ public class AddLogoutRequest extends AbstractApplicationAction {
final Issuer issuer = issuerBuilder.buildObject();
issuer.setValue(issuerId);
object.setIssuer(issuer);
+ stateData.setIssuer(issuerId);
} else {
log.debug("{} No issuer value available, leaving Issuer unset", getLogPrefix());
}
@@ -245,13 +294,6 @@ public class AddLogoutRequest extends AbstractApplicationAction {
profileRequestContext.ensureOutboundMessageContext().setMessage(object);
- // Check for RelayState.
- final DDF input = ensureAgentRequestContext().getInput();
- final String relayState = input != null ? input.getmember(SPConstants.STATE).string() : null;
- if (relayState != null) {
- SAMLBindingSupport.setRelayState(profileRequestContext.ensureOutboundMessageContext(), relayState);
- }
-
log.info("{} Generated LogoutRequest with ID {} from {}", getLogPrefix(), object.getID(), issuerId);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list