[java-shib-shared] branch main updated: IDP-2374 - Implement support for SameSite directly when possible
Scott Cantor
cantor.2 at osu.edu
Wed Apr 9 20:04:14 UTC 2025
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-shib-shared.
View the commit online:
http://git.shibboleth.net/view/?p=java-shib-shared.git;a=commit;h=adc76b8dbff440d3019cb6a9396546fd33920c5d
The following commit(s) were added to refs/heads/main by this push:
new adc76b8d IDP-2374 - Implement support for SameSite directly when possible
adc76b8d is described below
commit adc76b8dbff440d3019cb6a9396546fd33920c5d
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Apr 9 16:04:11 2025 -0400
IDP-2374 - Implement support for SameSite directly when possible
https://shibboleth.atlassian.net/browse/IDP-2374
Port over SameSite features from filter to CookieManager.
Convert guard logic in CookieManager to servlet major version check.
Add auto-disable flag to filter, defaulting based on servlet version.
---
.../servlet/impl/SameSiteCookieHeaderFilter.java | 37 +++-
.../net/shibboleth/shared/net/CookieManager.java | 231 +++++++++++++++++----
.../shared/servlet/AbstractConditionalFilter.java | 1 -
.../shibboleth/shared/net/CookieManagerTest.java | 12 +-
4 files changed, 234 insertions(+), 47 deletions(-)
diff --git a/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/servlet/impl/SameSiteCookieHeaderFilter.java b/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/servlet/impl/SameSiteCookieHeaderFilter.java
index d6659f10..64c09ad7 100644
--- a/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/servlet/impl/SameSiteCookieHeaderFilter.java
+++ b/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/servlet/impl/SameSiteCookieHeaderFilter.java
@@ -70,7 +70,10 @@ public class SameSiteCookieHeaderFilter extends AbstractConditionalFilter implem
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(SameSiteCookieHeaderFilter.class);
-
+
+ /** Whether to disable automatically on newer API. */
+ private boolean disableBasedOnServletAPI;
+
/** The allowed same-site cookie attribute values.*/
public enum SameSiteValue{
@@ -123,9 +126,23 @@ public class SameSiteCookieHeaderFilter extends AbstractConditionalFilter implem
/** Constructor. */
public SameSiteCookieHeaderFilter() {
+ disableBasedOnServletAPI = true;
sameSiteCookies = CollectionSupport.emptyMap();
}
+ /**
+ * Sets whether to automatically disable filter if servlet API has native SameSite support.
+ *
+ * <p>Defaults to true.</p>
+ *
+ * @param flag flag to set
+ *
+ * @since 5.2.0
+ */
+ public void setDisableBasedOnServletAPI(final boolean flag) {
+ disableBasedOnServletAPI = flag;
+ }
+
/**
* Set an optional default value to apply to all unmapped cookies.
*
@@ -143,7 +160,7 @@ public class SameSiteCookieHeaderFilter extends AbstractConditionalFilter implem
* argument injection e.g. trying to set a session identifier cookie as both SameSite=Strict and SameSite=None.
* Instead, duplicates are detected here, throwing a terminating {@link IllegalArgumentException} if found.</p>
*
- * @param map the map of same-site attribute values to cookie names.
+ * @param map the map of same-site attribute values to cookie names
*/
public void setSameSiteCookies(@Nullable final Map<SameSiteValue,List<String>> map) {
if (map != null) {
@@ -180,6 +197,22 @@ public class SameSiteCookieHeaderFilter extends AbstractConditionalFilter implem
public int getOrder() {
return FilterOrder.EARLIEST.getValue();
}
+
+
+
+ /** {@inheritDoc} */
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+
+ // Implement an internal condition of sorts to optionally disable on newer API.
+ if (disableBasedOnServletAPI && request.getServletContext().getMajorVersion() >= 6) {
+ chain.doFilter(request, response);
+ return;
+ }
+
+ super.doFilter(request, response, chain);
+ }
/** {@inheritDoc} */
@Override
diff --git a/shib-networking/src/main/java/net/shibboleth/shared/net/CookieManager.java b/shib-networking/src/main/java/net/shibboleth/shared/net/CookieManager.java
index 136c905b..6c5b7214 100644
--- a/shib-networking/src/main/java/net/shibboleth/shared/net/CookieManager.java
+++ b/shib-networking/src/main/java/net/shibboleth/shared/net/CookieManager.java
@@ -15,6 +15,8 @@
package net.shibboleth.shared.net;
import java.time.Duration;
+import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.TreeSet;
import java.util.function.Predicate;
@@ -39,7 +41,6 @@ import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.logic.PredicateSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.shared.primitive.NonnullSupplier;
-import net.shibboleth.shared.primitive.ReflectionSupport;
import net.shibboleth.shared.primitive.StringSupport;
/**
@@ -56,9 +57,6 @@ public class CookieManager extends AbstractInitializableComponent {
/** Class logger. */
@Nonnull private Logger log = LoggerFactory.getLogger(CookieManager.class);
-
- /** Whether we're on a platform with {@link Cookie#setAttribute(String, String)}. */
- private final boolean hasSetAttribute;
/** Path of cookie. */
@Nullable private String cookiePath;
@@ -81,8 +79,54 @@ public class CookieManager extends AbstractInitializableComponent {
/** Maximum age in seconds, or -1 for session. */
private int maxAge;
+ /** The allowed same-site cookie attribute values.*/
+ public enum SameSiteValue{
+
+ /**
+ * Send the cookie for 'same-site' requests only.
+ */
+ Strict("Strict"),
+ /**
+ * Send the cookie for 'same-site' requests along with 'cross-site' top
+ * level navigations using safe HTTP methods (GET, HEAD, OPTIONS, and TRACE).
+ */
+ Lax("Lax"),
+ /**
+ * Send the cookie for 'same-site' and 'cross-site' requests.
+ */
+ None("None"),
+ /**
+ * Specify nothing.
+ */
+ Null("Null");
+
+ /** The same-site attribute value.*/
+ @Nonnull @NotEmpty private String value;
+
+ /**
+ * Constructor.
+ *
+ * @param attrValue the same-site attribute value.
+ */
+ private SameSiteValue(@Nonnull @NotEmpty final String attrValue) {
+ value = Constraint.isNotEmpty(attrValue, "the same-site attribute value can not be empty");
+ }
+
+ /**
+ * Get the same-site attribute value.
+ *
+ * @return Returns the value.
+ */
+ @Nonnull public String getValue() {
+ return value;
+ }
+ }
+
/** SameSite attribute. */
- @Nullable private String sameSite;
+ @Nonnull private SameSiteValue defaultSameSite;
+
+ /** Map of cookie name to same-site attribute value.*/
+ @Nonnull private Map<String,SameSiteValue> sameSiteCookies;
/** Condition controlling application of SameSite. */
@Nonnull private Predicate<HttpServletRequest> sameSiteCondition;
@@ -98,10 +142,11 @@ public class CookieManager extends AbstractInitializableComponent {
httpOnly = true;
secure = true;
maxAge = -1;
+ defaultSameSite = SameSiteValue.Null;
+ sameSiteCookies = CollectionSupport.emptyMap();
sameSiteCondition = PredicateSupport.alwaysTrue();
cookieAttributes = CollectionSupport.emptyMap();
cookieLimit = 0;
- hasSetAttribute = ReflectionSupport.getMethod(Cookie.class, "setAttribute", String.class, String.class) != null;
}
/**
@@ -194,9 +239,19 @@ public class CookieManager extends AbstractInitializableComponent {
return httpResponseSupplier.get();
}
+ /**
+ * Get the TLS-only flag.
+ *
+ * @return TLS-only flag
+ *
+ * @since 9.2.0
+ */
+ public boolean isSecure() {
+ return secure;
+ }
/**
- * Set the SSL-only flag.
+ * Set the TLS-only flag.
*
* @param flag flag to set
*/
@@ -205,7 +260,17 @@ public class CookieManager extends AbstractInitializableComponent {
secure = flag;
}
-
+
+ /**
+ * Get the HttpOnly flag.
+ *
+ * @return HttpOnly flag
+ *
+ * @since 9.2.0
+ */
+ public boolean isHttpOnly() {
+ return httpOnly;
+ }
/**
* Set the HttpOnly flag.
@@ -263,8 +328,8 @@ public class CookieManager extends AbstractInitializableComponent {
*
* @since 9.2.0
*/
- @Nullable @NotEmpty public String getSameSite() {
- return sameSite;
+ @Nonnull public SameSiteValue getSameSite() {
+ return defaultSameSite;
}
/**
@@ -276,10 +341,45 @@ public class CookieManager extends AbstractInitializableComponent {
*
* @since 9.2.0
*/
- public void setSameSite(@Nullable final String value) {
+ public void setSameSite(@Nonnull final SameSiteValue value) {
checkSetterPreconditions();
- sameSite = StringSupport.trimOrNull(value);
+ defaultSameSite = Constraint.isNotNull(value, "SameSite Value cannot be null");
+ }
+
+ /**
+ * Set the names of cookies to add the same-site attribute to.
+ *
+ * <p>The argument map is flattened to remove the nested collection. The argument map allows duplicate
+ * cookie names to appear in order to detect configuration errors which would otherwise not be found during
+ * argument injection e.g. trying to set a session identifier cookie as both SameSite=Strict and SameSite=None.
+ * Instead, duplicates are detected here, throwing a terminating {@link IllegalArgumentException} if found.</p>
+ *
+ * @param map the map of same-site attribute values to cookie names
+ *
+ * @since 9.2.0
+ */
+ public void setSameSiteCookies(@Nullable final Map<SameSiteValue,List<String>> map) {
+ if (map != null) {
+ sameSiteCookies = new HashMap<>(4);
+ for (final Map.Entry<SameSiteValue,List<String>> entry : map.entrySet()) {
+
+ for (final String cookieName : entry.getValue()) {
+ if (sameSiteCookies.get(cookieName) != null) {
+ log.error("Duplicate cookie name '{}' found in SameSite cookie map, "
+ + "please check configuration.",cookieName);
+ throw new IllegalArgumentException("Duplicate cookie name found in SameSite cookie map");
+ }
+ final String trimmedName = StringSupport.trimOrNull(cookieName);
+ if (trimmedName != null) {
+ sameSiteCookies.put(cookieName, entry.getKey());
+ }
+ }
+ }
+ } else {
+ sameSiteCookies = CollectionSupport.emptyMap();
+ }
+
}
/**
@@ -369,24 +469,20 @@ public class CookieManager extends AbstractInitializableComponent {
if (httpRequestSupplier == null || httpResponseSupplier == null) {
throw new ComponentInitializationException("Servlet request and response must be set");
}
-
- if (!hasSetAttribute && (sameSite != null || !cookieAttributes.isEmpty())) {
- log.info("Running on Servlet API < 6.0.0, some features are degraded");
- }
}
/**
- * Add a cookie with the specified name and value.
+ * Add a cookie with the specified attributes.
*
* @param name name of cookie
* @param value value of cookie
*/
public void addCookie(@Nonnull @NotEmpty final String name, @Nonnull @NotEmpty final String value) {
- addCookie(name, value, maxAge);
+ addCookie(name, value, getCookiePath(), getMaxAge());
}
/**
- * Add a cookie with the specified name and value.
+ * Add a cookie with the specified attributes.
*
* @param name name of cookie
* @param value value of cookie
@@ -396,20 +492,33 @@ public class CookieManager extends AbstractInitializableComponent {
*/
public void addCookie(@Nonnull @NotEmpty final String name, @Nonnull @NotEmpty final String value,
final int overrideMaxAge) {
+ addCookie(name, value, getCookiePath(), overrideMaxAge);
+ }
+
+ /**
+ * Add a cookie with the specified attributes.
+ *
+ * @param name name of cookie
+ * @param value value of cookie
+ * @param overridePath path value tp use
+ * @param overrideMaxAge max-age value to use
+ *
+ * @since 9.2.0
+ */
+ public void addCookie(@Nonnull @NotEmpty final String name, @Nonnull @NotEmpty final String value,
+ @Nullable @NotEmpty final String overridePath, final int overrideMaxAge) {
checkComponentActive();
final Cookie cookie = new Cookie(name, value);
- cookie.setPath(cookiePath != null ? cookiePath : contextPathToCookiePath());
- if (cookieDomain != null) {
- cookie.setDomain(cookieDomain);
+ cookie.setPath(overridePath != null ? overridePath : contextPathToCookiePath());
+ if (getCookieDomain() != null) {
+ cookie.setDomain(getCookieDomain());
}
- cookie.setSecure(secure);
- cookie.setHttpOnly(httpOnly);
+ cookie.setSecure(isSecure());
+ cookie.setHttpOnly(isHttpOnly());
cookie.setMaxAge(overrideMaxAge);
- if (hasSetAttribute) {
- if (sameSiteCondition.test(getHttpServletRequest())) {
- cookie.setAttribute("SameSite", sameSite);
- }
+ if (getHttpServletRequest().getServletContext().getMajorVersion() >= 6) {
+ attachSameSite(cookie);
cookieAttributes.forEach((n,v) -> {
cookie.setAttribute(n, v);
});
@@ -419,25 +528,33 @@ public class CookieManager extends AbstractInitializableComponent {
}
/**
- * Unsets a cookie with the specified name.
+ * Unsets a cookie with the specified name and the default path.
*
* @param name name of cookie
*/
public void unsetCookie(@Nonnull @NotEmpty final String name) {
+ unsetCookie(name, getCookiePath());
+ }
+
+ /**
+ * Unsets a cookie with the specified name and path.
+ *
+ * @param name name of cookie
+ * @param overridePath cookie path
+ */
+ public void unsetCookie(@Nonnull @NotEmpty final String name, @Nullable @NotEmpty final String overridePath) {
checkComponentActive();
final Cookie cookie = new Cookie(name, null);
- cookie.setPath(cookiePath != null ? cookiePath : contextPathToCookiePath());
+ cookie.setPath(overridePath != null ? overridePath : contextPathToCookiePath());
if (cookieDomain != null) {
- cookie.setDomain(cookieDomain);
+ cookie.setDomain(getCookieDomain());
}
- cookie.setSecure(secure);
- cookie.setHttpOnly(httpOnly);
+ cookie.setSecure(isSecure());
+ cookie.setHttpOnly(isHttpOnly());
cookie.setMaxAge(0);
- if (hasSetAttribute) {
- if (sameSiteCondition.test(getHttpServletRequest())) {
- cookie.setAttribute("SameSite", sameSite);
- }
+ if (getHttpServletRequest().getServletContext().getMajorVersion() >= 6) {
+ attachSameSite(cookie);
cookieAttributes.forEach((n,v) -> {
cookie.setAttribute(n, v);
});
@@ -445,7 +562,7 @@ public class CookieManager extends AbstractInitializableComponent {
getHttpServletResponse().addCookie(cookie);
}
-
+
/**
* Check whether a cookie has a certain value.
*
@@ -486,7 +603,7 @@ public class CookieManager extends AbstractInitializableComponent {
return defValue;
}
-
+
/**
* Unset cookies matching a given prefix in excess of the configured amount.
*
@@ -495,6 +612,19 @@ public class CookieManager extends AbstractInitializableComponent {
* @since 9.2.0
*/
public void purgeStaleCookies(@Nonnull @NotEmpty final String prefix) {
+ purgeStaleCookies(prefix, getCookiePath());
+ }
+
+ /**
+ * Unset cookies matching a given prefix in excess of the configured amount.
+ *
+ * @param prefix cookie name prefix to match on
+ * @param overridePath cookie path
+ *
+ * @since 9.2.0
+ */
+ public void purgeStaleCookies(@Nonnull @NotEmpty final String prefix,
+ @Nullable @NotEmpty final String overridePath) {
if (getCookieLimit() == 0) {
return;
@@ -522,7 +652,7 @@ public class CookieManager extends AbstractInitializableComponent {
--maxCookies;
} else {
// We're over the limit, so everything here and older gets cleaned up.
- unsetCookie(nameToPurge);
+ unsetCookie(nameToPurge, overridePath);
++purgedCookies;
}
}
@@ -532,6 +662,27 @@ public class CookieManager extends AbstractInitializableComponent {
}
}
+ /**
+ * Implementation of SameSite attachment logic when available.
+ *
+ * @param cookie cookie to attach attribute to
+ */
+ private void attachSameSite(@Nonnull final Cookie cookie) {
+
+ if (!sameSiteCondition.test(getHttpServletRequest())) {
+ return;
+ }
+
+ final SameSiteValue sameSiteValue = sameSiteCookies.get(cookie.getName());
+ if (sameSiteValue != null) {
+ if (sameSiteValue != SameSiteValue.Null) {
+ cookie.setAttribute("SameSite", sameSiteValue.getValue());
+ }
+ } else if (defaultSameSite != SameSiteValue.Null) {
+ cookie.setAttribute("SameSite", defaultSameSite.getValue());
+ }
+ }
+
/**
* Turn the servlet context path into an appropriate cookie path.
*
diff --git a/shib-networking/src/main/java/net/shibboleth/shared/servlet/AbstractConditionalFilter.java b/shib-networking/src/main/java/net/shibboleth/shared/servlet/AbstractConditionalFilter.java
index 33a785d7..bd195be9 100644
--- a/shib-networking/src/main/java/net/shibboleth/shared/servlet/AbstractConditionalFilter.java
+++ b/shib-networking/src/main/java/net/shibboleth/shared/servlet/AbstractConditionalFilter.java
@@ -85,7 +85,6 @@ public abstract class AbstractConditionalFilter implements Filter {
}
chain.doFilter(request, response);
- return;
}
/**
diff --git a/shib-networking/src/test/java/net/shibboleth/shared/net/CookieManagerTest.java b/shib-networking/src/test/java/net/shibboleth/shared/net/CookieManagerTest.java
index 06ac393c..78b09b35 100644
--- a/shib-networking/src/test/java/net/shibboleth/shared/net/CookieManagerTest.java
+++ b/shib-networking/src/test/java/net/shibboleth/shared/net/CookieManagerTest.java
@@ -35,7 +35,9 @@ import org.testng.annotations.Test;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.net.CookieManager.SameSiteValue;
import net.shibboleth.shared.primitive.NonnullSupplier;
/** {@link CookieManager} unit test. */
@@ -79,7 +81,7 @@ public class CookieManagerTest {
cm.setHttpServletRequestSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletRequest get() {return request;}});
cm.setHttpServletResponseSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletResponse get() {return response;}});
cm.setCookiePath("/idp");
- cm.setSameSite("None");
+ cm.setSameSite(SameSiteValue.None);
cm.initialize();
cm.addCookie("foo", "bar");
@@ -91,7 +93,7 @@ public class CookieManagerTest {
Assert.assertNull(cookie.getDomain());
Assert.assertTrue(cookie.getSecure());
Assert.assertEquals(cookie.getMaxAge(), -1);
- Assert.assertEquals(cookie.getAttribute("SameSite"), "None");
+ Assert.assertEquals(cookie.getAttribute("SameSite"), SameSiteValue.None.getValue());
}
@Test public void testCookieNoPath() throws ComponentInitializationException {
@@ -102,7 +104,7 @@ public class CookieManagerTest {
CookieManager cm = new CookieManager();
cm.setHttpServletRequestSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletRequest get() {return request;}});
cm.setHttpServletResponseSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletResponse get() {return response;}});
- cm.setSameSite("Strict");
+ cm.setSameSite(SameSiteValue.Strict);
cm.initialize();
cm.addCookie("foo", "bar");
@@ -114,7 +116,7 @@ public class CookieManagerTest {
Assert.assertNull(cookie.getDomain());
Assert.assertTrue(cookie.getSecure());
Assert.assertEquals(cookie.getMaxAge(), -1);
- Assert.assertEquals(cookie.getAttribute("SameSite"), "Strict");
+ Assert.assertEquals(cookie.getAttribute("SameSite"), SameSiteValue.Strict.getValue());
}
@Test public void testCookieUnset() throws ComponentInitializationException {
@@ -126,6 +128,7 @@ public class CookieManagerTest {
CookieManager cm = new CookieManager();
cm.setHttpServletRequestSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletRequest get() {return request;}});
cm.setHttpServletResponseSupplier(new NonnullSupplier<>() { @Nonnull public HttpServletResponse get() {return response;}});
+ cm.setSameSiteCookies(CollectionSupport.singletonMap(SameSiteValue.Lax, CollectionSupport.singletonList("foo")));
cm.initialize();
cm.unsetCookie("foo");
@@ -137,6 +140,7 @@ public class CookieManagerTest {
Assert.assertNull(cookie.getDomain());
Assert.assertTrue(cookie.getSecure());
Assert.assertEquals(cookie.getMaxAge(), 0);
+ Assert.assertEquals(cookie.getAttribute("SameSite"), SameSiteValue.Lax.getValue());
}
@Test public void testPurge() throws ComponentInitializationException, InterruptedException, EncoderException {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list