From 433c4837fd5cae08049f8f88b1bbb15848be5f65 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 3 Mar 2025 21:50:15 +1100 Subject: [PATCH] WW-5534 Add coverage for proxy resolution --- .../StrutsParameterAnnotationTest.java | 139 ++++++++++++++++-- .../ognl/SecurityMemberAccessTest.java | 4 +- .../ognl/SecurityMemberAccessProxyTest.java | 41 +++++- 3 files changed, 170 insertions(+), 14 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java index 50de80c7b..500574d34 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java @@ -18,6 +18,8 @@ */ package org.apache.struts2.interceptor.parameter; +import org.aopalliance.intercept.Joinpoint; +import org.aopalliance.intercept.MethodInterceptor; import org.apache.commons.lang3.ClassUtils; import org.apache.struts2.ActionContext; import org.apache.struts2.ModelDriven; @@ -35,6 +37,7 @@ import org.apache.struts2.security.NotExcludedAcceptedPatternsChecker; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.framework.ProxyFactory; import java.util.HashMap; import java.util.HashSet; @@ -106,174 +109,267 @@ public class StrutsParameterAnnotationTest { return set; } + /** + * Private String field cannot be injected even when annotated. + */ @Test public void privateStrAnnotated() { testParameter(new FieldAction(), "privateStr", false); } + /** + * Public String field can be injected when annotated. + */ @Test public void publicStrAnnotated() { testParameter(new FieldAction(), "publicStr", true); assertThat(threadAllowlist.getAllowlist()).isEmpty(); } + /** + * Public String field cannot be injected when not annotated. + */ @Test public void publicStrNotAnnotated() { testParameter(new FieldAction(), "publicStrNotAnnotated", false); } + /** + * Private Pojo field cannot be injected even when annotated with the appropriate depth. + */ @Test public void privatePojoAnnotated() { testParameter(new FieldAction(), "privatePojo.key", false); } + /** + * Public Pojo field cannot be injected when annotated with depth zero. + */ @Test public void publicPojoDepthZero() { testParameter(new FieldAction(), "publicPojoDepthZero.key", false); } + /** + * Public Pojo field can be injected when annotated with depth one. + */ @Test public void publicPojoDepthOne() { testParameter(new FieldAction(), "publicPojoDepthOne.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo field can be injected when annotated with depth one, using the square bracket syntax. + */ @Test public void publicPojoDepthOne_sqrBracket() { testParameter(new FieldAction(), "publicPojoDepthOne['key']", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo field can be injected when annotated with depth one, using the bracket syntax. + */ @Test public void publicPojoDepthOne_bracket() { testParameter(new FieldAction(), "publicPojoDepthOne('key')", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } - @Test - public void publicNestedPojoDepthOne() { - testParameter(new FieldAction(), "publicPojoDepthOne.key.key", false); - } - + /** + * Public Pojo field can be injected when annotated with a depth greater than required. + */ @Test public void publicPojoDepthTwo() { testParameter(new FieldAction(), "publicPojoDepthTwo.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo field cannot be injected two levels when only annotated with depth one. + */ + @Test + public void publicNestedPojoDepthOne() { + testParameter(new FieldAction(), "publicPojoDepthOne.key.key", false); + } + + /** + * Public Pojo field can be injected two levels when annotated with depth two. + */ @Test public void publicNestedPojoDepthTwo() { testParameter(new FieldAction(), "publicPojoDepthTwo.key.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo field can be injected two levels when annotated with depth two, using the square bracket syntax. + */ @Test public void publicNestedPojoDepthTwo_sqrBracket() { testParameter(new FieldAction(), "publicPojoDepthTwo['key']['key']", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo field can be injected two levels when annotated with depth two, using the bracket syntax. + */ @Test public void publicNestedPojoDepthTwo_bracket() { testParameter(new FieldAction(), "publicPojoDepthTwo('key')('key')", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Private String setting method cannot be injected even when annotated. + */ @Test public void privateStrAnnotatedMethod() { testParameter(new MethodAction(), "privateStr", false); } + /** + * Public String setting method can be injected when annotated. + */ @Test public void publicStrAnnotatedMethod() { testParameter(new MethodAction(), "publicStr", true); assertThat(threadAllowlist.getAllowlist()).isEmpty(); } + /** + * Public String setting method cannot be injected when not annotated. + */ @Test public void publicStrNotAnnotatedMethod() { testParameter(new MethodAction(), "publicStrNotAnnotated", false); } + /** + * Private Pojo returning method cannot be injected even when annotated with the appropriate depth. + */ @Test public void privatePojoAnnotatedMethod() { testParameter(new MethodAction(), "privatePojo.key", false); } + /** + * Public Pojo returning method cannot be injected when annotated with depth zero. + */ @Test public void publicPojoDepthZeroMethod() { testParameter(new MethodAction(), "publicPojoDepthZero.key", false); } + /** + * Public Pojo returning method can be injected when annotated with depth one. + */ @Test public void publicPojoDepthOneMethod() { testParameter(new MethodAction(), "publicPojoDepthOne.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo returning method cannot be injected two levels when only annotated with depth one. + */ @Test public void publicNestedPojoDepthOneMethod() { testParameter(new MethodAction(), "publicPojoDepthOne.key.key", false); } + /** + * Public Pojo returning method can be injected when annotated with a depth greater than required. + */ @Test public void publicPojoDepthTwoMethod() { testParameter(new MethodAction(), "publicPojoDepthTwo.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public Pojo returning method can be injected two levels when annotated with depth two. + */ @Test public void publicNestedPojoDepthTwoMethod() { testParameter(new MethodAction(), "publicPojoDepthTwo.key.key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Pojo.class)); } + /** + * Public list of Pojo field cannot be injected when annotated with depth one. + */ @Test public void publicPojoListDepthOne() { testParameter(new FieldAction(), "publicPojoListDepthOne[0].key", false); } + /** + * Public list of Pojo field can be injected when annotated with depth two. + */ @Test public void publicPojoListDepthTwo() { testParameter(new FieldAction(), "publicPojoListDepthTwo[0].key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(List.class, Pojo.class)); } - @Test - public void publicPojoMapDepthTwo() { - testParameter(new FieldAction(), "publicPojoMapDepthTwo['a'].key", true); - assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class, String.class, Pojo.class)); - } - + /** + * Public list of Pojo returning method cannot be injected when annotated with depth one. + */ @Test public void publicPojoListDepthOneMethod() { testParameter(new MethodAction(), "publicPojoListDepthOne[0].key", false); } + /** + * Public list of Pojo returning method can be injected when annotated with depth two. + */ @Test public void publicPojoListDepthTwoMethod() { testParameter(new MethodAction(), "publicPojoListDepthTwo[0].key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(List.class, Pojo.class)); } + /** + * Public map of Pojo field can be injected when annotated with depth two. + */ + @Test + public void publicPojoMapDepthTwo() { + testParameter(new FieldAction(), "publicPojoMapDepthTwo['a'].key", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class, String.class, Pojo.class)); + } + + /** + * Public map of Pojo returning method can be injected when annotated with depth two. + */ @Test public void publicPojoMapDepthTwoMethod() { testParameter(new MethodAction(), "publicPojoMapDepthTwo['a'].key", true); assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class, String.class, Pojo.class)); } + /** + * Public String field can be injected even when not annotated, if transition mode is enabled. + */ @Test public void publicStrNotAnnotated_transitionMode() { parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString()); testParameter(new FieldAction(), "publicStrNotAnnotated", true); } + /** + * Public String setting method can be injected even when not annotated, if transition mode is enabled. + */ @Test public void publicStrNotAnnotatedMethod_transitionMode() { parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString()); testParameter(new MethodAction(), "publicStrNotAnnotated", true); } + /** + * Models of ModelDriven actions can be injected without any annotations on the Action. + */ @Test public void publicModelPojo() { var action = new ModelAction(); @@ -288,6 +384,27 @@ public class StrutsParameterAnnotationTest { assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Object.class, Pojo.class)); } + /** + * Models of ModelDriven actions can be injected without any annotations on the Action, even when the Action is + * proxied. + */ + @Test + public void publicModelPojo_proxied() { + var proxyFactory = new ProxyFactory(new ModelAction()); + proxyFactory.setProxyTargetClass(true); + proxyFactory.addAdvice((MethodInterceptor) Joinpoint::proceed); + var proxiedAction = (ModelAction) proxyFactory.getProxy(); + + // Emulate ModelDrivenInterceptor running previously + var valueStack = new StubValueStack(); + valueStack.push(proxiedAction.getModel()); + ActionContext.of().withValueStack(valueStack).bind(); + + testParameter(proxiedAction, "name", true); + testParameter(proxiedAction, "name.nested", true); + assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Object.class, Pojo.class)); + } + static class FieldAction { @StrutsParameter private String privateStr; diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java index e3d3fa589..ee775fc40 100644 --- a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java @@ -18,12 +18,12 @@ */ package org.apache.struts2.ognl; +import ognl.MemberAccess; +import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.struts2.TestBean; import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.test.TestBean2; import org.apache.struts2.util.Foo; -import ognl.MemberAccess; -import org.apache.commons.lang3.reflect.FieldUtils; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; import org.junit.Before; diff --git a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java index 43f82bfe5..160f0ef96 100644 --- a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java @@ -20,12 +20,15 @@ package org.apache.struts2.ognl; import org.apache.struts2.ActionProxy; import org.apache.struts2.XWorkJUnit4TestCase; -import org.apache.struts2.config.providers.XmlConfigurationProvider; import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.config.providers.XmlConfigurationProvider; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.MethodBeforeAdvice; +import org.springframework.aop.framework.ProxyFactory; import java.lang.reflect.Member; +import java.lang.reflect.Method; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -175,4 +178,40 @@ public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase { Object action = proxy.getAction(); assertTrue(sma.isAccessible(context, action, proxyObjectProxyMember, null)); } + + /** + * When the allowlist is enabled and proxy object access is allowed, Spring proxies should be allowlisted based + * on their underlying target object. Class allowlisting should work as expected. + */ + @Test + public void classInclusion_springProxy_allowProxyObjectAccess() throws Exception { + SpringService proxyObject = newSpringService(); + Method proxyMethod = proxyObject.getClass().getMethod("doSomething"); + + sma.useEnforceAllowlistEnabled(Boolean.TRUE.toString()); + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useAllowlistClasses(SpringServiceImpl.class.getName()); + + assertTrue(sma.checkAllowlist(proxyObject, proxyMethod)); + } + + private static SpringService newSpringService() { + SpringService target = new SpringServiceImpl(); + ProxyFactory proxyFactory = new ProxyFactory(target); + proxyFactory.addAdvice(((MethodBeforeAdvice) (method, args, target1) -> { + System.out.println("Intercepting: " + method.getName()); + })); + return (SpringService) proxyFactory.getProxy(); + } +} + +interface SpringService { + void doSomething(); +} + +class SpringServiceImpl implements SpringService { + @Override + public void doSomething() { + System.out.println("Doing something..."); + } }