[java-plugin-shibd-oidc] branch main updated: Add more state flow tests
Codeberg
noreply at shibboleth.net
Mon Feb 16 17:22:41 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository java-plugin-shibd-oidc.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd-oidc/commit/364c4072b8f2fa079fd0873ce7ccf3ecc46ce31f
The following commit(s) were added to refs/heads/main by this push:
new 364c407 Add more state flow tests
364c407 is described below
commit 364c4072b8f2fa079fd0873ce7ccf3ecc46ce31f
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Feb 16 17:22:28 2026 +0000
Add more state flow tests
- Also add a WIP for mapping state through the state manager
---
.../idp/flows/sp/consumer/oidc/oidc-beans.xml | 1 +
.../idp/flows/sp/initiator/oidc/oidc-beans.xml | 5 +-
.../idp/flows/sp/initiator/oidc/oidc-flow.xml | 8 --
.../oidc/flows/OIDCSessionInitiatorFlowTest.java | 21 +++
.../sp/oidc/flows/OIDCTokenConsumerFlowTest.java | 33 ++++-
.../shibboleth/sp/oidc/flows/TestConstants.java | 12 ++
.../sp/oidc/profile/impl/MapStateToStateToken.java | 159 +++++++++++++++++++++
.../sp/oidc/profile/impl/StateLookupStrategy.java | 4 +-
8 files changed, 229 insertions(+), 14 deletions(-)
diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
index 82db963..f3a1f5f 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/consumer/oidc/oidc-beans.xml
@@ -40,6 +40,7 @@
<constructor-arg name="f" ref="shibboleth.MessageContextLookup.Inbound"/>
</bean>
+ <!-- Correlation cookie is used to -->
<bean id="ProcessNonceCorrelationCookie" class="net.shibboleth.sp.oidc.profile.impl.ProcessStateCookie" scope="prototype"
p:cookieManager-ref="shibboleth.sp.RemotedCookieManager"
p:cookiePrefix="%{sp.correlation.cookiePrefix:__Host-shibsp_req_}"
diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-beans.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-beans.xml
index f4c0477..9c78a5d 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-beans.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-beans.xml
@@ -335,9 +335,10 @@
p:stateValueLookupStrategy-ref="AuthenticationRequestStateForStorageStrategy" />
<!-- TODO TRIAL -->
- <!-- <bean id="MapAuthenticationStateToStateToken" class="net.shibboleth.sp.oidc.profile.impl.MapStateToStateToken" scope="prototype"
+ <bean id="MapAuthenticationStateToStateToken" class="net.shibboleth.sp.oidc.profile.impl.MapStateToStateToken" scope="prototype"
p:createOutputObjects="true"
- p:errorFatal="%{sp.stateToken.errorsFatal:false}" /> -->
+ p:errorFatal="%{sp.stateToken.errorsFatal:true}"
+ p:stateValueLookupStrategy-ref="AuthenticationRequestStateForStorageStrategy"/>
<!-- A strategy for encoding and sealing any state from the authentication request needed for validating the response -->
<bean id="AuthenticationRequestStateForStorageStrategy" class="net.shibboleth.sp.oidc.profile.impl.AuthenticationRequestStateForStorageStrategy"
diff --git a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-flow.xml b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-flow.xml
index 6848e2f..6629579 100644
--- a/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-flow.xml
+++ b/sp-oidc-conf-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/sp/initiator/oidc/oidc-flow.xml
@@ -6,7 +6,6 @@
<action-state id="OIDCSessionInitiator">
<evaluate expression="ValidateSessionInitiatorRequest" />
<evaluate expression="PrepareInboundMessageContext" /> <!-- needs session and logout support -->
- <!-- <evaluate expression="SAMLProtocolAndRole" /> -->
<evaluate expression="ProviderMetadataLookup" />
<evaluate expression="InitializeRelyingPartyContextFromOIDCPeer" />
@@ -14,16 +13,9 @@
<evaluate expression="SelectProfileConfiguration" />
<evaluate expression="InitializeOutboundMessageContext" />
- <!-- <evaluate expression="InitializeMessageChannelSecurityContext" /> -->
- <!-- <evaluate expression="PopulateBindingAndEndpointContexts" /> -->
<evaluate expression="InitializeOAuth2ClientContext" />
<evaluate expression="InitializeAuthorizationRequest" />
- <!-- SAML<evaluate expression="PopulateRequestSignatureSigningParameters" />
- SAML<evaluate expression="PopulateEncryptionParameters" />
-
- <evaluate expression="EncryptNameIDs" /> -->
-
<evaluate expression="BuildAuthenticationRequest"/>
<evaluate expression="'proceed'" />
diff --git a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCSessionInitiatorFlowTest.java b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCSessionInitiatorFlowTest.java
index 9e8405c..f37bee7 100644
--- a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCSessionInitiatorFlowTest.java
+++ b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCSessionInitiatorFlowTest.java
@@ -34,6 +34,7 @@ import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.mockito.Mockito;
import org.opensaml.messaging.decoder.MessageDecodingException;
+import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.saml.common.binding.SAMLBindingSupport;
import org.springframework.context.ApplicationContext;
@@ -170,6 +171,26 @@ public class OIDCSessionInitiatorFlowTest extends AbstractSPFlowTest {
validateOutputMessage(result);
}
+ /**
+ * Basic flow test but where the resource URL is not supplied.
+ *
+ * @throws IOException on error
+ * @throws MessageDecodingException
+ */
+ @Test
+ public void testFailure_NoTarget() throws IOException, MessageDecodingException {
+ setDefaultAuth();
+
+ final DDF input = new DDF(null).structure();
+ input.addmember(RemotedHttpServletRequest.STRUCTURE_NAME).structure();
+ input.addmember(InitiatorConstants.RESPONSE_URL).string(RESPONSE_URL);
+ setApplicationRequest("test-oidc-application-with-ro", input);
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, FLOW_ID);
+ assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+ }
+
/**
* Basic flow test which uses the default profile configuration.
*
diff --git a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
index cf2062c..ba696b1 100644
--- a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
+++ b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/OIDCTokenConsumerFlowTest.java
@@ -539,12 +539,12 @@ public class OIDCTokenConsumerFlowTest extends AbstractSPFlowTest {
}
/**
- * Test failure, a bad nonce in the response state.
+ * Test failure, a bad nonce in the correlation cookie.
*
* @throws IOException on error
*/
@Test
- public void testFail_BadCorrelationNonceState() throws Exception {
+ public void testFail_BadCorrelationNonceState_InCookie() throws Exception {
final AuthenticationSuccessResponse response =
buildOIDCAuthorizationCodeResponse(TestConstants.RESPONSE_URL, ResponseMode.QUERY, TestConstants.STATE_STRING);
final DDF input = buildRemotedQueryStringResponse(response);
@@ -564,6 +564,35 @@ public class OIDCTokenConsumerFlowTest extends AbstractSPFlowTest {
assertFlowExecutionOutcome(result.getOutcome());
assertOutputMessageEvent(result, EventIds.MESSAGE_PROC_ERROR);
}
+
+ /**
+ * Test failure, a bad nonce in the authentication response. That is, what if somehow the return URL was manipulated
+ * accidently or on purpose.
+ *
+ * @throws IOException on error
+ */
+ @Test
+ public void testFail_BadCorrelationNonceState_InAuthnResponse() throws Exception {
+ final AuthenticationSuccessResponse response =
+ buildOIDCAuthorizationCodeResponse(TestConstants.RESPONSE_URL, ResponseMode.QUERY,
+ TestConstants.STATE_STRING_DIFFERENT_STATE);
+ final DDF input = buildRemotedQueryStringResponse(response);
+
+ // Add cookies
+ input.addmember("http.headers.Cookie").unsafe_string(TestConstants.buildCookieHeader(
+ TestConstants.AUTHENTICATION_STATE,
+ TestConstants.APPLICATION_ID,
+ TestConstants.AUTHENTICATION_NONCE,
+ TestConstants.TARGET_URL,
+ TestConstants.buildAuthenticationState(null, false, null)));
+
+ setApplicationRequest(TestConstants.APPLICATION_ID, input);
+
+ final FlowExecutionResult result = flowExecutor.launchExecution(TestConstants.FLOW_ID, null, externalContext);
+ assertFlowExecutionResult(result, TestConstants.FLOW_ID);
+ assertFlowExecutionOutcome(result.getOutcome());
+ assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+ }
/**
diff --git a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/TestConstants.java b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/TestConstants.java
index c8ff091..5617dd2 100644
--- a/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/TestConstants.java
+++ b/sp-oidc-conf-impl/src/test/java/net/shibboleth/sp/oidc/flows/TestConstants.java
@@ -91,6 +91,18 @@ public final class TestConstants {
eyJzdGF0ZSI6IjE3NjEzMTY5Njc3MTBfMTYyMmE1YzcyNmRhOGY3YjM2ZTI0ZjE5ZWVkODJhZWEiLCJub25jZSI6ImYyNmQ5MjIyMjEyMjZjZDE4MzcyOWJmMTMyNDdkYmUyIn0
""";
+ /**
+ * A returned state String consisting of a state and a nonce. The state has been changed, or is not expected:
+ *
+ * <pre>
+ * {"state":"1761316967710_wrong",
+ * "nonce":"f26d922221226cd183729bf13247dbe2"}
+ * </pre>
+ * */
+ public static final String STATE_STRING_DIFFERENT_STATE = """
+ eyJzdGF0ZSI6IjE3NjEzMTY5Njc3MTBfd3JvbmciLCJub25jZSI6ImYyNmQ5MjIyMjEyMjZjZDE4MzcyOWJmMTMyNDdkYmUyIn0=
+ """;
+
/**
* Build cookie header bytes for the given parameters. These take the form of:
* <pre>
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/MapStateToStateToken.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/MapStateToStateToken.java
new file mode 100644
index 0000000..5ca3ba5
--- /dev/null
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/MapStateToStateToken.java
@@ -0,0 +1,159 @@
+/*
+ * 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.oidc.profile.impl;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.function.Function;
+
+import javax.annotation.Nonnull;
+
+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.NonnullAfterInit;
+import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
+import net.shibboleth.shared.logic.Constraint;
+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.profile.AbstractApplicationAction;
+import net.shibboleth.sp.profile.SPConstants;
+
+/**
+ *
+ */
+public class MapStateToStateToken extends AbstractApplicationAction {
+
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(MapStateToStateToken.class);
+
+ /** Whether an error constructing a state token is fatal. */
+ private boolean errorFatal;
+
+ /** Agent input. */
+ @NonnullBeforeExec private DDF input;
+
+ /** Target resource to operate on. */
+ @NonnullBeforeExec private byte[] target;
+
+ /** Lookup strategy for the contents of the state cookie. */
+ @NonnullAfterInit private Function<ProfileRequestContext,String> stateValueLookupStrategy;
+
+ /** The value of the state token. */
+ @NonnullBeforeExec private String stateValue;
+
+
+ /**
+ * Sets whether an error computing a state token should result in a fatal event.
+ *
+ * <p>Defaults to false.</p>
+ *
+ * @param flag flag to set
+ */
+ public void setErrorFatal(final boolean flag) {
+ checkSetterPreconditions();
+
+ errorFatal = flag;
+ }
+
+ /**
+ * Sets the lookup strategy for obtaining the contents of the state cookie.
+ *
+ * @param strategy lookup strategy
+ */
+ public void setStateValueLookupStrategy(@Nonnull final Function<ProfileRequestContext,String> strategy) {
+ checkSetterPreconditions();
+
+ stateValueLookupStrategy = Constraint.isNotNull(strategy, "State value lookup strategy cannot be null");
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected boolean doPreExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+ if (!super.doPreExecute(profileRequestContext)) {
+ return false;
+ }
+
+ input = ensureAgentRequestContext().getInput();
+ if (input == null) {
+ log.debug("{} Input message was absent", getLogPrefix());
+ return false;
+ }
+
+// if (input.getmember(SPConstants.STATE).isstring()) {
+// log.debug("{} Input message already contains {} parameter", getLogPrefix(), SPConstants.STATE);
+// return false;
+// }
+//
+// target = input.getmember(SPConstants.TARGET).unsafe_string();
+// if (target == null) {
+// log.warn("{} Input message did not contain {} member", getLogPrefix(), SPConstants.TARGET);
+// ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+// return false;
+// }
+//
+// ensureAgentRequestContext().setTargetURL(target);
+
+ return true;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+
+ ensureOutputObjects();
+
+ final AgentRequestContext agentRequestContext = ensureAgentRequestContext();
+
+ String state = null;
+ if (input.getmember(SPConstants.STATE).isstring()) {
+ log.debug("{} Input message already contains {} parameter", getLogPrefix(), SPConstants.STATE);
+ state = input.getmember(SPConstants.STATE).string();
+ }
+
+ // We do the crazy stuff to accomodate cookie-backed state management.
+ try {
+ RemotedHttpServletRequestResponseContext.loadCurrent(agentRequestContext.getRemotedHttpServletRequest(),
+ agentRequestContext.getRemotedHttpServletResponse());
+
+ try {
+ final String stateValue = stateValueLookupStrategy.apply(profileRequestContext);
+
+ //TODO UTF-8?
+ final byte[] stateValueBytes = stateValue.getBytes(StandardCharsets.UTF_8);
+
+ final String token = ensureApplication().getStateTokenManager().preserveToStateToken(
+ ensureAgent(), ensureApplication(), stateValueBytes);
+ if (state == null) {
+ input.addmember(SPConstants.STATE).string(token);
+ }
+
+ log.debug("{} State preserved to state token: {}", getLogPrefix(), token);
+ } catch (final IOException e) {
+ log.warn("{} Exception preserving state to token", getLogPrefix(), e);
+ if (errorFatal) {
+ ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
+ }
+ }
+ } finally {
+ RemotedHttpServletRequestResponseContext.clearCurrent();
+ }
+ }
+
+}
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/StateLookupStrategy.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/StateLookupStrategy.java
index aa4d767..f4b42e2 100644
--- a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/StateLookupStrategy.java
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/StateLookupStrategy.java
@@ -32,8 +32,8 @@ import net.shibboleth.sp.oidc.profile.OIDCConstants;
import net.shibboleth.sp.profile.SPConstants;
/**
- * A strategy function that constructs a Base64URL encoded JSON object containing a state value from the input {@link DDF} and a
- * cryptographically secure nonce for CSRF protection.
+ * A strategy function that constructs a Base64URL encoded JSON object containing a state value from the input
+ * {@link DDF} and a cryptographically secure nonce for CSRF protection.
*
* <p>The JSON Object includes the following:
* <ul>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list