This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5627-cookie-authorization in repository https://gitbox.apache.org/repos/asf/struts.git
commit 39b8fe92964325324602384fa0bee85dfa4884e5 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat May 9 19:13:51 2026 +0200 WW-5627 gate CookieInterceptor cookie injection through ParameterAuthorizer Adds a 5-arg `populateCookieValueIntoStack(name, value, map, stack, action)` hook that runs cookie writes through `ParameterAuthorizer.isAuthorized` and primes `ThreadAllowlist` via `ParameterAllowlister` for nested paths, then delegates to the legacy 4-arg form. The 4-arg form is `@Deprecated(since="7.2.0")` but its body is unchanged, so existing subclass overrides automatically receive only authorized cookies. Default-config behavior is preserved because the authorizer short-circuits when `requireAnnotations=false`. Existing `CookieInterceptorTest` instantiates `new CookieInterceptor()` rather than going through the container, leaving the new injected fields null. Wires explicit pass-through lambdas through a `disableAuthorizationGate(...)` helper so those tests continue to exercise default-config behavior. --- .../struts2/interceptor/CookieInterceptor.java | 50 ++++++++++++++++++++-- .../struts2/interceptor/CookieInterceptorTest.java | 18 ++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java index 6ca7ecb0a..7a3d7fa29 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java @@ -26,6 +26,8 @@ import org.apache.struts2.ActionInvocation; import org.apache.struts2.ServletActionContext; import org.apache.struts2.action.CookiesAware; import org.apache.struts2.inject.Inject; +import org.apache.struts2.interceptor.parameter.ParameterAllowlister; +import org.apache.struts2.interceptor.parameter.ParameterAuthorizer; import org.apache.struts2.security.AcceptedPatternsChecker; import org.apache.struts2.security.ExcludedPatternsChecker; import org.apache.struts2.util.TextParseUtil; @@ -187,6 +189,8 @@ public class CookieInterceptor extends AbstractInterceptor { private ExcludedPatternsChecker excludedPatternsChecker; private AcceptedPatternsChecker acceptedPatternsChecker; + private ParameterAuthorizer parameterAuthorizer; + private ParameterAllowlister parameterAllowlister; @Inject public void setExcludedPatternsChecker(ExcludedPatternsChecker excludedPatternsChecker) { @@ -199,6 +203,16 @@ public class CookieInterceptor extends AbstractInterceptor { this.acceptedPatternsChecker.setAcceptedPatterns(ACCEPTED_PATTERN); } + @Inject + public void setParameterAuthorizer(ParameterAuthorizer parameterAuthorizer) { + this.parameterAuthorizer = parameterAuthorizer; + } + + @Inject + public void setParameterAllowlister(ParameterAllowlister parameterAllowlister) { + this.parameterAllowlister = parameterAllowlister; + } + /** * @param cookiesName the <code>cookiesName</code> which if matched will allow the cookie * to be injected into action, could be comma-separated string. @@ -234,6 +248,8 @@ public class CookieInterceptor extends AbstractInterceptor { public String intercept(ActionInvocation invocation) throws Exception { LOG.debug("start interception"); + final Object action = invocation.getAction(); + // contains selected cookies final Map<String, String> cookiesMap = new LinkedHashMap<>(); @@ -248,9 +264,9 @@ public class CookieInterceptor extends AbstractInterceptor { if (isAcceptableName(name)) { if (cookiesNameSet.contains("*")) { LOG.debug("Contains cookie name [*] in configured cookies name set, cookie with name [{}] with value [{}] will be injected", name, value); - populateCookieValueIntoStack(name, value, cookiesMap, stack); + populateCookieValueIntoStack(name, value, cookiesMap, stack, action); } else if (cookiesNameSet.contains(cookie.getName())) { - populateCookieValueIntoStack(name, value, cookiesMap, stack); + populateCookieValueIntoStack(name, value, cookiesMap, stack, action); } } else { LOG.warn("Cookie name [{}] with value [{}] was rejected!", name, value); @@ -259,7 +275,7 @@ public class CookieInterceptor extends AbstractInterceptor { } // inject the cookiesMap, even if we don't have any cookies - injectIntoCookiesAwareAction(invocation.getAction(), cookiesMap); + injectIntoCookiesAwareAction(action, cookiesMap); return invocation.invoke(); } @@ -314,6 +330,29 @@ public class CookieInterceptor extends AbstractInterceptor { return false; } + /** + * Authorizes the cookie against {@link ParameterAuthorizer}, primes OGNL allowlist for any nested path via + * {@link ParameterAllowlister}, then delegates to the legacy {@link #populateCookieValueIntoStack(String, String, + * Map, ValueStack)} hook so existing subclass overrides continue to participate. Override this method to customize + * the authorization behavior itself. + * + * @param cookieName cookie name (potentially an OGNL path; {@code ACCEPTED_PATTERN} restricts the character set) + * @param cookieValue cookie value + * @param cookiesMap map of cookies populated for {@link org.apache.struts2.action.CookiesAware} + * @param stack current request value stack + * @param action the action instance from {@link ActionInvocation#getAction()}; used for {@code @StrutsParameter} target resolution + * @since 7.2.0 + */ + protected void populateCookieValueIntoStack(String cookieName, String cookieValue, Map<String, String> cookiesMap, ValueStack stack, Object action) { + Object target = parameterAuthorizer.resolveTarget(action); + if (!parameterAuthorizer.isAuthorized(cookieName, target, action)) { + LOG.debug("Cookie [{}] rejected by @StrutsParameter authorization on target [{}]", cookieName, target.getClass().getSimpleName()); + return; + } + parameterAllowlister.allowlistAuthorizedPath(cookieName, target); + populateCookieValueIntoStack(cookieName, cookieValue, cookiesMap, stack); + } + /** * Hook that populate cookie value into value stack (hence the action) * if the criteria is satisfied (if the cookie value matches with those configured). @@ -322,7 +361,12 @@ public class CookieInterceptor extends AbstractInterceptor { * @param cookieValue cookie value * @param cookiesMap map of cookies * @param stack value stack + * @deprecated since 7.2.0. Override + * {@link #populateCookieValueIntoStack(String, String, Map, ValueStack, Object)} instead so cookie writes are + * authorized by {@link ParameterAuthorizer}. The default 5-arg implementation calls this method after the + * authorization gate, so existing overrides continue to receive only authorized cookies. */ + @Deprecated(since = "7.2.0") protected void populateCookieValueIntoStack(String cookieName, String cookieValue, Map<String, String> cookiesMap, ValueStack stack) { if (cookiesValueSet.isEmpty() || cookiesValueSet.contains("*")) { // If the interceptor is configured to accept any cookie value diff --git a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java index 8189f5ce8..8bcfe704c 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java @@ -43,6 +43,15 @@ import static org.easymock.EasyMock.verify; public class CookieInterceptorTest extends StrutsInternalTestCase { + /** + * These tests construct {@link CookieInterceptor} via {@code new} rather than the DI container, so the + * {@code @StrutsParameter} authorization gate added in WW-5627 has no injected services. We supply explicit + * pass-through lambdas to mirror the default-config behavior these tests assume ({@code requireAnnotations=false}). + */ + private static void disableAuthorizationGate(CookieInterceptor interceptor) { + interceptor.setParameterAuthorizer((name, target, action) -> true); + interceptor.setParameterAllowlister((name, target) -> {}); + } public void testIntercepDefault() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -68,6 +77,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.intercept(invocation); @@ -105,6 +115,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("*"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -147,6 +158,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("cookie1, cookie2, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -188,6 +200,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -230,6 +243,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -271,6 +285,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue(""); interceptor.intercept(invocation); @@ -313,6 +328,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { CookieInterceptor interceptor = new CookieInterceptor(); interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value"); interceptor.intercept(invocation); @@ -395,6 +411,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { excludedPatternsChecker.setAdditionalExcludePatterns(".*(^|\\.|\\[|'|\")class(\\.|\\[|'|\").*"); interceptor.setExcludedPatternsChecker(excludedPatternsChecker); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation(); @@ -441,6 +458,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { }; interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setAcceptedPatternsChecker(new DefaultAcceptedPatternsChecker()); + disableAuthorizationGate(interceptor); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation();
