[java-plugin-shibd-oidc] branch main updated: Fix process state cookie action and add tests
Codeberg
noreply at shibboleth.net
Fri Jan 30 17:39:31 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/035efeaf199e6fe2fa09a282870c41954ab23c42
The following commit(s) were added to refs/heads/main by this push:
new 035efea Fix process state cookie action and add tests
035efea is described below
commit 035efeaf199e6fe2fa09a282870c41954ab23c42
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Jan 30 17:39:23 2026 +0000
Fix process state cookie action and add tests
---
.../idp/flows/sp/consumer/oidc/oidc-beans.xml | 4 +-
...relationCookie.java => ProcessStateCookie.java} | 40 ++---
.../oidc/profile/impl/ProcessStateCookieTest.java | 170 +++++++++++++++++++++
3 files changed, 192 insertions(+), 22 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 2855bdb..e487de8 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,7 +40,7 @@
<constructor-arg name="f" ref="shibboleth.MessageContextLookup.Inbound"/>
</bean>
- <bean id="ProcessNonceCorrelationCookie" class="net.shibboleth.sp.oidc.profile.impl.ProcessCorrelationCookie" scope="prototype"
+ <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_}"
p:createOutputObjects="true"
@@ -52,7 +52,7 @@
</bean>
<!-- TODO should this be a cookie or go through the state manager-->
- <bean id="ProcessAuthnStateCookie" class="net.shibboleth.sp.oidc.profile.impl.ProcessCorrelationCookie" scope="prototype"
+ <bean id="ProcessAuthnStateCookie" class="net.shibboleth.sp.oidc.profile.impl.ProcessStateCookie" scope="prototype"
p:cookieManager-ref="shibboleth.sp.RemotedCookieManager"
p:cookiePrefix="%{sp.correlation.cookiePrefix:__Host-_shibsp_authnstate_}"
p:createOutputObjects="true"
diff --git a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessCorrelationCookie.java b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookie.java
similarity index 87%
rename from sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessCorrelationCookie.java
rename to sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookie.java
index 2a62ef4..12a14bd 100644
--- a/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessCorrelationCookie.java
+++ b/sp-oidc-impl/src/main/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookie.java
@@ -37,35 +37,28 @@ import net.shibboleth.shared.net.CookieManager;
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.messaging.RemotedHttpServletRequestResponseContext;
import net.shibboleth.sp.profile.AbstractApplicationAction;
/**
- * Action that processes a previously issued correlation cookie submitted with the request and
+ * Action that processes a previously issued state cookie submitted with the request and
* extracts the value for use by subsequent validation steps.
*
* <p>The value is set onto the context tree using a custom consumer strategy.</p>
*
- * TODO this is almost the same as that in sp-server
- * TODO if used for more than correlation cookies, rename class to just processing cookies?
- *
- * <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>
+ * <p>The absence of either state value or cookie value can either be treated as fatal or not fatal depending on the
+ * errorFatal flag.</p>
*
* @event {@link EventIds#PROCEED_EVENT_ID}
* @event {@link EventIds#INVALID_MESSAGE}
*/
-public class ProcessCorrelationCookie extends AbstractApplicationAction {
+public class ProcessStateCookie extends AbstractApplicationAction {
/** Default cookie prefix. */
@Nonnull @NotEmpty public static final String DEFAULT_COOKIE_PREFIX = "_shibsp_req_";
/** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(ProcessCorrelationCookie.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(ProcessStateCookie.class);
/** Cookie manager. */
@NonnullAfterInit private CookieManager cookieManager;
@@ -77,11 +70,11 @@ public class ProcessCorrelationCookie extends AbstractApplicationAction {
@NonnullAfterInit private Function<ProfileRequestContext,String> stateTokenLookupStrategy;
/**
- * A consumer to set the cookie value into the appropriate context in the tree. The return value indicates
+ * A consumer to set the cookie value into the appropriate context in the tree. The return boolean indicates
* if the consumer function was successful or not. An unsuccessful outcome may trigger an error depending on the
- * errorFatal flag.
+ * {@link #errorFatal} flag.
*/
- @Nullable BiFunction<ProfileRequestContext, String, Boolean> cookieValueConsumerStrategy;
+ @Nullable private BiFunction<ProfileRequestContext, String, Boolean> cookieValueConsumerStrategy;
/** Whether an error recovering a correlation cookie is fatal. */
private boolean errorFatal;
@@ -90,7 +83,7 @@ public class ProcessCorrelationCookie extends AbstractApplicationAction {
@NonnullBeforeExec private String stateToken;
/** Constructor. */
- public ProcessCorrelationCookie() {
+ public ProcessStateCookie() {
cookiePrefix = DEFAULT_COOKIE_PREFIX;
}
@@ -121,7 +114,7 @@ public class ProcessCorrelationCookie extends AbstractApplicationAction {
/**
* Sets the cookie prefix.
*
- * <p>Defaults to {@link IssueCorrelationCookie#DEFAULT_COOKIE_PREFIX}.</p>
+ * <p>Defaults to {@link ProcessStateCookie#DEFAULT_COOKIE_PREFIX}.</p>
*
* @param prefix cookie prefix
*/
@@ -175,9 +168,16 @@ public class ProcessCorrelationCookie extends AbstractApplicationAction {
}
stateToken = stateTokenLookupStrategy.apply(profileRequestContext);
- if (stateToken == null && !errorFatal) {
- log.debug("{} No state token found in request, skipping correlation cookie processing", getLogPrefix());
- return false;
+ if (stateToken == null) {
+ if (!errorFatal) {
+ log.debug("{} No state token found in request, non-fatal skipping correlation cookie processing",
+ getLogPrefix());
+ return false;
+ } else {
+ log.warn("{} No state token found in request, fatal error", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+ return false;
+ }
}
// If error is fatal, we do not skip processing here.
return true;
diff --git a/sp-oidc-impl/src/test/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookieTest.java b/sp-oidc-impl/src/test/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookieTest.java
new file mode 100644
index 0000000..8bf87b6
--- /dev/null
+++ b/sp-oidc-impl/src/test/java/net/shibboleth/sp/oidc/profile/impl/ProcessStateCookieTest.java
@@ -0,0 +1,170 @@
+/*
+ * 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 static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+import org.mockito.Mockito;
+import org.opensaml.profile.action.EventIds;
+import org.opensaml.profile.context.EventContext;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.net.CookieManager;
+import net.shibboleth.sp.profile.impl.BaseAgplicationActionTest;
+
+/**
+ * Tests for {@link ProcessStateCookie}.
+ */
+public class ProcessStateCookieTest extends BaseAgplicationActionTest {
+
+ private ProcessStateCookie action;
+ private CookieManager cookieManager;
+
+
+ @BeforeMethod
+ public void BeforeMethod() throws ComponentInitializationException {
+ super.beforeMethod();
+ action = new ProcessStateCookie();
+ cookieManager = Mockito.mock(CookieManager.class);
+ action.setCookieManager(cookieManager);
+ }
+
+ @Test
+ public void testMissingStateTokenNonFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> null);
+ action.setErrorFatal(false);
+ action.initialize();
+
+ final boolean result = action.doPreExecute(prc);
+ assertFalse(result);
+ }
+
+ /* Missing state token is fatal.*/
+ @Test
+ public void testMissingStateTokenFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> null);
+ action.setErrorFatal(true);
+ action.initialize();
+
+ final boolean result = action.doPreExecute(prc);
+ assertFalse(result);
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNotNull(ec);
+ assert ec != null;
+ assertEquals(ec.getEvent(), EventIds.INVALID_MESSAGE);
+ }
+
+ /* The cookie is missing, but this is not fatal.*/
+ @Test
+ public void testMissingCookieNonFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> "state");
+ action.setErrorFatal(false);
+ action.initialize();
+
+ Mockito.when(cookieManager.getCookieValue(Mockito.anyString(), Mockito.any())).thenReturn(null);
+
+ assertTrue(action.doPreExecute(prc));
+ action.doExecute(prc);
+
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNull(ec);
+ }
+
+ /* The cookie is missing, and this is fatal.*/
+ @Test
+ public void testMissingCookieFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> "state");
+ action.setErrorFatal(true);
+ action.initialize();
+
+ Mockito.when(cookieManager.getCookieValue(Mockito.anyString(), Mockito.any())).thenReturn(null);
+
+ assertTrue(action.doPreExecute(prc));
+ action.doExecute(prc);
+
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNotNull(ec);
+ assert ec != null;
+ assertEquals(ec.getEvent(), EventIds.INVALID_MESSAGE);
+ }
+
+ /* Test cookie value found and set.*/
+ @Test
+ public void testConsumerSuccess() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> "state");
+ action.setCookieValueConsumerStrategy((prc, value) -> Boolean.TRUE);
+ action.setErrorFatal(true);
+ action.initialize();
+
+ Mockito.when(cookieManager.getCookieValue(Mockito.anyString(), Mockito.any())).thenReturn("cookie-value");
+
+ assertTrue(action.doPreExecute(prc));
+ action.doExecute(prc);
+
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNull(ec);
+
+ }
+
+ /* Could not set cookie back to context, but not fatal.*/
+ @Test
+ public void testConsumerFailureNonFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> "state");
+ action.setCookieValueConsumerStrategy((prc, value) -> Boolean.FALSE);
+ action.setErrorFatal(false);
+ action.initialize();
+
+ Mockito.when(cookieManager.getCookieValue(Mockito.anyString(), Mockito.any())).thenReturn("cookie-value");
+
+ assertTrue(action.doPreExecute(prc));
+ action.doExecute(prc);
+
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNull(ec);
+ }
+
+ /* Could not set cookie back to context, and fatal.*/
+ @Test
+ public void testConsumerFailureFatal() throws ComponentInitializationException {
+
+ action.setStateTokenLookupStrategy(prc -> "state");
+ action.setCookieValueConsumerStrategy((prc, value) -> Boolean.FALSE);
+ action.setErrorFatal(true);
+ action.initialize();
+
+ Mockito.when(cookieManager.getCookieValue(Mockito.anyString(), Mockito.any())).thenReturn("cookie-value");
+
+ assertTrue(action.doPreExecute(prc));
+ action.doExecute(prc);
+
+ final var ec = prc.getSubcontext(EventContext.class);
+ assertNotNull(ec);
+ assert ec != null;
+ assertEquals(ec.getEvent(), EventIds.INVALID_MESSAGE);
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list