[java-plugin-shibd] branch main updated: WIP on storage-backed state manager.
Codeberg
noreply at shibboleth.net
Mon Apr 20 18:50:48 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.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd/commit/dde20eb8243f8b023109e07400b7836e64d7f567
The following commit(s) were added to refs/heads/main by this push:
new dde20eb WIP on storage-backed state manager.
dde20eb is described below
commit dde20eb8243f8b023109e07400b7836e64d7f567
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Mon Apr 20 14:50:31 2026 -0400
WIP on storage-backed state manager.
---
.../sp/state/impl/StorageServiceStateManager.java | 121 ++++++++++++++--
.../state/impl/StorageServiceStateManagerTest.java | 159 +++++++++++++++++++++
2 files changed, 272 insertions(+), 8 deletions(-)
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/state/impl/StorageServiceStateManager.java b/sp-server-impl/src/main/java/net/shibboleth/sp/state/impl/StorageServiceStateManager.java
index e7a62dc..9136898 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/state/impl/StorageServiceStateManager.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/state/impl/StorageServiceStateManager.java
@@ -32,22 +32,39 @@ import net.shibboleth.shared.codec.DecodingException;
import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.net.CookieManager;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.StringSupport;
import net.shibboleth.sp.Agent;
import net.shibboleth.sp.Application;
import net.shibboleth.sp.state.AbstractStateManager;
import net.shibboleth.sp.state.StateManager;
/**
- * {@link StateManager} implemented with a {@link StorageService}.
+ * {@link StateManager} implemented with a {@link StorageService} and an optional CSRF-mitigation cookie.
*/
public class StorageServiceStateManager extends AbstractStateManager {
+
+ /** Default cookie prefix. */
+ @Nonnull @NotEmpty public static String DEFAULT_PREFIX = "_shibsp_state";
/** Class logger. */
@Nonnull private Logger log = LoggerFactory.getLogger(StorageServiceStateManager.class);
/** Storage back-end. */
@NonnullAfterInit private StorageService storageService;
+
+ /** Cookie manager. */
+ @Nullable private CookieManager cookieManager;
+
+ /** Fixed prefix for cookie names. */
+ @Nonnull @NotEmpty private String cookiePrefix;
+
+ /** Constructor. */
+ public StorageServiceStateManager() {
+ cookiePrefix = DEFAULT_PREFIX;
+ }
+
/**
* Set {@link StorageService} to use.
@@ -60,6 +77,35 @@ public class StorageServiceStateManager extends AbstractStateManager {
storageService = Constraint.isNotNull(storage, "StorageService cannot be null");
}
+ /**
+ * Set the fixed prefix to use for the cookies.
+ *
+ * <p>Defaults to "_shibsp_state".</p>
+ *
+ * @param prefix cookie prefix
+ */
+ public void setCookiePrefix(@Nonnull @NotEmpty final String prefix) {
+ checkSetterPreconditions();
+
+ cookiePrefix = Constraint.isNotNull(StringSupport.trimOrNull(prefix), "Cookie prefix cannot be null or empty");
+ }
+
+ /**
+ * Set the {@link CookieManager} to use.
+ *
+ * <p>If set, the implementation produces a second random key that points to a cookie that
+ * carries the actual storage key to retrieve. If unset, the original storage key is used by
+ * itself and no additional cookie is created.</p>
+ *
+ * @param manager instance to use
+ */
+ public void setCookieManager(@Nonnull final CookieManager manager) {
+ checkSetterPreconditions();
+
+ cookieManager = Constraint.isNotNull(manager, "CookieManager cannot be null");
+ }
+
+
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
@@ -90,11 +136,29 @@ public class StorageServiceStateManager extends AbstractStateManager {
}
if (storageService.create(context, key, encoded, Instant.now().plus(getExpiration()).toEpochMilli())) {
- log.trace("Created state token mapping ('{}', '{}') to value '{}'", context, key, encoded);
- return key;
+ final CookieManager localManager = cookieManager;
+ if (localManager != null) {
+ localManager.purgeStaleCookies(cookiePrefix);
+
+ final Instant ts = Instant.now();
+ assert ts != null;
+
+ final String key2 = Long.toString(ts.toEpochMilli()) + '_' + generateToken();
+ final String name = getCookieName(application, key2);
+ // We assume the original key is URL safe.
+ localManager.addCookie(name, key, (int) getExpiration().toSeconds());
+
+ log.trace("Created state record ('{}', '{}') for value '{}' using cookie name '{}'", context, key,
+ data, name);
+
+ return key2;
+ } else {
+ log.trace("Created state record ('{}', '{}') for value '{}'", context, key, encoded);
+ return key;
+ }
}
- throw new IOException("Unable to create storage record for state token");
+ throw new IOException("Unable to create storage record for state");
}
/** {@inheritDoc} */
@@ -102,18 +166,37 @@ public class StorageServiceStateManager extends AbstractStateManager {
@Nullable protected String doRecover(@Nonnull final Agent agent, @Nonnull final Application application,
@Nonnull @NotEmpty final String stateToken) throws IOException {
+ final String key;
final String context = getContext(agent, application);
- final StorageRecord<String> record = storageService.read(context, stateToken);
+
+ final CookieManager localManager = cookieManager;
+ if (localManager != null) {
+ // Indirect storage key from cookie matching state token.
+ final String cookieName = getCookieName(application, stateToken);
+ key = localManager.getCookieValue(cookieName, null);
+ if (key != null) {
+ localManager.unsetCookie(cookieName);
+ log.trace("Recovered cookie mapping from '{}' to storage key '{}'", stateToken, key);
+ } else {
+ log.warn("No cookie found matching state token: '{}'", stateToken);
+ return null;
+ }
+ } else {
+ // The original state token is the actual storage key.
+ key = stateToken;
+ }
+
+ final StorageRecord<String> record = storageService.read(context, key);
if (record != null) {
try {
storageService.delete(context, stateToken);
} catch (final IOException e) {
- log.warn("Unable to delete state token ('{}', '{}') from storage", context, stateToken, e);
+ log.warn("Unable to delete state record ('{}', '{}') from storage", context, key, e);
}
try {
final String decoded = getDataSealer() != null ? record.getValue() :
new String(Base64Support.decode(record.getValue()), StandardCharsets.UTF_8);
- log.trace("Recovered state token mapping ('{}', '{}') to value '{}'", context, stateToken, decoded);
+ log.trace("Recovered state record ('{}', '{}') with value '{}'", context, key, decoded);
return decoded;
} catch (final DecodingException e) {
throw new IOException(e);
@@ -137,5 +220,27 @@ public class StorageServiceStateManager extends AbstractStateManager {
builder.append('!').append(agent.getId()).append('!').append(application.getApplicationId());
return builder.toString();
}
-
+
+ /**
+ * Computes the name of a new state cookie.
+ *
+ * @param application the application
+ * @param uniquePortion unique portion of name
+ *
+ * @return cookie name
+ */
+ @Nonnull public String getCookieName(@Nonnull final Application application, @Nonnull final String uniquePortion) {
+
+ // Format is prefix_appId_timestamp_random
+ // The timestamp allows them to be sorted for staleness.
+
+ final StringBuilder builder = new StringBuilder(cookiePrefix);
+ builder.append('_')
+ .append(application.getId())
+ .append('_')
+ .append(uniquePortion);
+
+ return builder.toString();
+ }
+
}
\ No newline at end of file
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/StorageServiceStateManagerTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/StorageServiceStateManagerTest.java
new file mode 100644
index 0000000..85b811f
--- /dev/null
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/StorageServiceStateManagerTest.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.state.impl;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.storage.impl.MemoryStorageService;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.net.CookieManager;
+import net.shibboleth.shared.net.CookieManager.SameSiteValue;
+import net.shibboleth.shared.primitive.NonnullSupplier;
+import net.shibboleth.sp.profile.impl.BaseApplicationActionTest;
+import net.shibboleth.sp.state.StateData;
+
+/**
+ * Unit tests for {@link StorageServiceStateManager}.
+ */
+ at SuppressWarnings("javadoc")
+public class StorageServiceStateManagerTest extends BaseApplicationActionTest {
+
+ @Nonnull @NotEmpty private static final String TEST_ISSUER = "https://sp.example.org";
+
+ @Nonnull @NotEmpty private static final String TEST_AUTHORITY = "https://idp.example.org";
+
+ private MockHttpServletRequest request;
+ private MockHttpServletResponse response;
+
+ private CookieManager cookieManager;
+ private MemoryStorageService storageService;
+
+ @BeforeClass
+ public void setUp() throws ComponentInitializationException {
+ cookieManager = new CookieManager();
+ cookieManager.setCookiePath("/");
+ cookieManager.setSameSite(SameSiteValue.None);
+ cookieManager.setCookieLimit(10);
+ cookieManager.setHttpServletRequestSupplier(new NonnullSupplier<HttpServletRequest>() {
+ @Nonnull public HttpServletRequest get() {
+ assert request != null;
+ return request;
+ }
+ });
+ cookieManager.setHttpServletResponseSupplier(new NonnullSupplier<HttpServletResponse>() {
+ @Nonnull public HttpServletResponse get() {
+ assert response != null;
+ return response;
+ }
+ });
+ cookieManager.initialize();
+
+ storageService = new MemoryStorageService();
+ storageService.setId("test");
+ storageService.setCleanupInterval(Duration.ZERO);
+ storageService.initialize();
+ }
+
+ @AfterClass
+ public void tearDown() {
+ storageService.destroy();
+ }
+
+ @BeforeMethod
+ public void beforeMethod() throws ComponentInitializationException {
+ super.beforeMethod();
+ }
+
+ @Test
+ public void testMissing() throws IOException, ComponentInitializationException {
+ final var stateManager = getStateManager(false);
+ Assert.assertNull(stateManager.recoverFromStateToken(agent, application, "foo", StateData.class));
+ }
+
+ @Test
+ public void testRecoverNoCookie() throws IOException, ComponentInitializationException {
+ final var stateManager = getStateManager(false);
+
+ final StateData source = buildStateData();
+
+ final String token = stateManager.preserveToStateToken(agent, application, source);
+ assert token != null;
+
+ final StateData recovered = stateManager.recoverFromStateToken(agent, application, token, StateData.class);
+ Assert.assertEquals(source, recovered);
+
+ Assert.assertNull(stateManager.recoverFromStateToken(agent, application, token, StateData.class));
+ }
+
+ @Nonnull private StorageServiceStateManager getStateManager(final boolean useCookie)
+ throws ComponentInitializationException {
+
+ final var stateManager = new StorageServiceStateManager();
+ stateManager.setStorageService(storageService);
+ stateManager.setId("test");
+
+ if (useCookie) {
+ stateManager.setCookieManager(cookieManager);
+ }
+
+ final ObjectMapper mapper = new ObjectMapper();
+ mapper.registerModule(new JavaTimeModule());
+ stateManager.setObjectMapper(mapper);
+
+ stateManager.setHttpServletRequestSupplier(new NonnullSupplier<HttpServletRequest>() {
+ @Nonnull public HttpServletRequest get() {
+ assert request != null;
+ return request;
+ }
+ });
+
+ stateManager.initialize();
+
+ return stateManager;
+ }
+
+ @Nonnull private String getCookieName(@Nonnull final StorageServiceStateManager stateManager) {
+ final Instant now = Instant.now();
+ final String rand = stateManager.getIdentifierGenerationStrategy().generateIdentifier(false);
+ return stateManager.getCookieName(application, now.toEpochMilli() + '_' + rand);
+ }
+
+ @Nonnull private StateData buildStateData() {
+ final StateData data = new StateData();
+ data.setRequestTime(Instant.now());
+ data.setIssuer(TEST_ISSUER);
+ data.setAuthenticationAuthority(TEST_AUTHORITY);
+ return data;
+ }
+
+}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list