This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 60269a5b1 WW-5715 fix(rest): authorize REST body properties by Java
member name, not wire name (#1940)
60269a5b1 is described below
commit 60269a5b152005e9b8671aecb9cd1ffe1ebf4c26
Author: Lukasz Lenart <[email protected]>
AuthorDate: Mon Sep 14 18:14:02 2026 +0200
WW-5715 fix(rest): authorize REST body properties by Java member name, not
wire name (#1940)
AuthorizingSettableBeanProperty derived the authorization path from
getName(), Jackson's external property name, while
StrutsParameterAuthorizer resolves that path against the Java member.
Once an application renames a property (@JsonProperty, @JsonAlias, a
PropertyNamingStrategy) the two diverge: an annotated member renamed on
the wire is dropped as unannotated, and an unannotated member renamed
onto an annotated member's Java name is authorized as that member.
ParameterAuthorizingModule now keys each wrapper by the member Jackson
invokes for the property (SettableBeanProperty#getMember): the field
name, or the bean property a set/get/is accessor is named after, which
is what the authorizer resolves back to that member. The wrapper threads
the name through withDelegate and into AuthorizingValueDeserializer, so
nested path prefixes are built from member names as well.
BeanPropertyDefinition#getInternalName was tried first and rejected: it
names the merged property, not the mutator. Jackson merges an accessor
renamed with @JsonProperty into whatever property already owns that
external name and then invokes the explicitly named accessor, so an
unannotated @JsonProperty("name") setAdmin next to an annotated setName
kept the internal name "name" and still landed the value in setAdmin.
Creator parameters and accessors outside the bean convention keep the
external name; the former only occur nested, where the authorizer counts
depth alone, and the latter resolve to no member and fail closed.
The one-arg constructor stays as a deprecated shim for external callers;
its removal is WW-5744.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../jackson/AuthorizingSettableBeanProperty.java | 26 ++++++--
.../jackson/ParameterAuthorizingModule.java | 42 ++++++++++++-
.../ContentTypeInterceptorIntegrationTest.java | 71 ++++++++++++++++++++++
.../jackson/ParameterAuthorizingModuleTest.java | 65 ++++++++++++++++++++
4 files changed, 197 insertions(+), 7 deletions(-)
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableBeanProperty.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableBeanProperty.java
index a3365a8ff..0e5c8ab89 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableBeanProperty.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableBeanProperty.java
@@ -49,13 +49,29 @@ public class AuthorizingSettableBeanProperty extends
SettableBeanProperty.Delega
private static final Logger LOG =
LogManager.getLogger(AuthorizingSettableBeanProperty.class);
+ private final String memberName;
+
+ /**
+ * @deprecated keys authorization on the wire name; use
+ * {@link #AuthorizingSettableBeanProperty(SettableBeanProperty, String)}
with the Java member name
+ */
+ @Deprecated(since = "7.4.0", forRemoval = true)
public AuthorizingSettableBeanProperty(SettableBeanProperty delegate) {
+ this(delegate, delegate.getName());
+ }
+
+ /**
+ * @param memberName the Java member name the authorizer resolves, which
differs from
+ * {@link #getName()} once the property is renamed on
the wire
+ */
+ public AuthorizingSettableBeanProperty(SettableBeanProperty delegate,
String memberName) {
super(delegate);
+ this.memberName = memberName;
}
@Override
protected SettableBeanProperty withDelegate(SettableBeanProperty d) {
- return new AuthorizingSettableBeanProperty(d);
+ return new AuthorizingSettableBeanProperty(d, memberName);
}
/**
@@ -69,7 +85,7 @@ public class AuthorizingSettableBeanProperty extends
SettableBeanProperty.Delega
public SettableBeanProperty withValueDeserializer(JsonDeserializer<?>
deser) {
JsonDeserializer<?> effective = deser;
if (!(deser instanceof AuthorizingValueDeserializer)) {
- effective = new AuthorizingValueDeserializer(deser, getName(),
getType());
+ effective = new AuthorizingValueDeserializer(deser, memberName,
getType());
}
return _with(delegate.withValueDeserializer(effective));
}
@@ -80,7 +96,7 @@ public class AuthorizingSettableBeanProperty extends
SettableBeanProperty.Delega
delegate.deserializeAndSet(p, ctxt, instance);
return;
}
- String path = ParameterAuthorizationContext.pathFor(getName());
+ String path = ParameterAuthorizationContext.pathFor(memberName);
if (!DynamicKeyAuthorizationContext.isAuthorized(path)) {
LOG.warn("REST body parameter [{}] rejected by @StrutsParameter
authorization on [{}]",
path, instance.getClass().getName());
@@ -96,7 +112,7 @@ public class AuthorizingSettableBeanProperty extends
SettableBeanProperty.Delega
if (!ParameterAuthorizationContext.isActive()) {
return delegate.deserializeSetAndReturn(p, ctxt, instance);
}
- String path = ParameterAuthorizationContext.pathFor(getName());
+ String path = ParameterAuthorizationContext.pathFor(memberName);
if (!DynamicKeyAuthorizationContext.isAuthorized(path)) {
LOG.warn("REST body parameter [{}] rejected by @StrutsParameter
authorization on [{}]",
path, instance.getClass().getName());
@@ -132,7 +148,7 @@ public class AuthorizingSettableBeanProperty extends
SettableBeanProperty.Delega
if (!ParameterAuthorizationContext.isActive()) {
return true;
}
- String path = ParameterAuthorizationContext.pathFor(getName());
+ String path = ParameterAuthorizationContext.pathFor(memberName);
if (DynamicKeyAuthorizationContext.isAuthorized(path)) {
return true;
}
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
index 99450c185..d7aa5d262 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java
@@ -25,15 +25,22 @@ import
com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder;
import com.fasterxml.jackson.databind.deser.BeanDeserializerModifier;
import com.fasterxml.jackson.databind.deser.SettableAnyProperty;
import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
+import com.fasterxml.jackson.databind.introspect.AnnotatedField;
+import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
+import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.module.SimpleModule;
+import java.beans.Introspector;
import java.util.Iterator;
+import java.util.List;
/**
* Jackson {@link SimpleModule} that wraps every {@link SettableBeanProperty}
on every bean type
* with an {@link AuthorizingSettableBeanProperty}, enforcing {@code
@StrutsParameter} authorization
* during deserialization via the {@link
org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext}
- * ThreadLocal.
+ * ThreadLocal. Each wrapper is keyed by the Java member Jackson invokes for
the property, not the
+ * external name a {@code @JsonProperty} or naming strategy puts on the wire,
since the authorizer
+ * resolves the path against the member.
*
* <p>Register this module once on each handler's mapper (e.g. in the
constructor). All per-request
* authorization state is read from the ThreadLocal context, so the module +
mapper combination is
@@ -44,6 +51,8 @@ import java.util.Iterator;
public class ParameterAuthorizingModule extends SimpleModule {
private static final long serialVersionUID = 1L;
+ private static final List<String> MUTATOR_PREFIXES = List.of("set");
+ private static final List<String> GETTER_PREFIXES = List.of("get", "is");
private volatile boolean requireAnySetterAnnotations;
public ParameterAuthorizingModule() {
@@ -63,7 +72,8 @@ public class ParameterAuthorizingModule extends SimpleModule {
if (original instanceof AuthorizingSettableBeanProperty) {
continue; // idempotent; protect against
double-registration
}
- builder.addOrReplaceProperty(new
AuthorizingSettableBeanProperty(original), true);
+ builder.addOrReplaceProperty(
+ new AuthorizingSettableBeanProperty(original,
memberNameOf(original)), true);
}
if
(ParameterAuthorizingModule.this.requireAnySetterAnnotations) {
SettableAnyProperty anySetter = builder.getAnySetter();
@@ -87,6 +97,34 @@ public class ParameterAuthorizingModule extends SimpleModule
{
});
}
+ /**
+ * The bean property name {@code StrutsParameterAuthorizer} resolves to
the member Jackson will
+ * invoke for this property: the field itself, or the property a
one-argument {@code set} or
+ * no-argument {@code get}/{@code is} accessor is named after. A creator
parameter has no such
+ * member, and Jackson merges a renamed accessor into whatever property
already owns its external
+ * name, so neither the external name nor {@code
BeanPropertyDefinition#getInternalName()}
+ * identifies the member reliably. Properties with no member, or an
accessor outside the bean
+ * convention, keep the external name.
+ */
+ static String memberNameOf(SettableBeanProperty property) {
+ AnnotatedMember member = property.getMember();
+ if (member instanceof AnnotatedField) {
+ return member.getName();
+ }
+ if (member instanceof AnnotatedMethod method) {
+ List<String> prefixes = method.getParameterCount() == 1 ?
MUTATOR_PREFIXES
+ : method.getParameterCount() == 0 ? GETTER_PREFIXES :
List.of();
+ String methodName = method.getName();
+ for (String prefix : prefixes) {
+ if (methodName.length() > prefix.length() &&
methodName.startsWith(prefix)
+ &&
Character.isUpperCase(methodName.charAt(prefix.length()))) {
+ return
Introspector.decapitalize(methodName.substring(prefix.length()));
+ }
+ }
+ }
+ return property.getName();
+ }
+
/**
* Configures any-setter enforcement. Set this before the mapper is first
used so Jackson has
* not yet cached deserializers built by this module.
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
index cfe9b2a61..3cb03f0ba 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/ContentTypeInterceptorIntegrationTest.java
@@ -19,6 +19,7 @@
package org.apache.struts2.rest;
import com.fasterxml.jackson.annotation.JsonAnySetter;
+import com.fasterxml.jackson.annotation.JsonProperty;
import com.mockobjects.dynamic.AnyConstraintMatcher;
import com.mockobjects.dynamic.Mock;
import junit.framework.TestCase;
@@ -198,6 +199,29 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
assertEquals("admin", anySetterAction.getValues().get("role"));
}
+ public void testAnnotatedMemberRenamedOnTheWireIsApplied() throws
Exception {
+ RenamedPropertiesAction renamed = new RenamedPropertiesAction();
+ setupInterceptorWithAction(renamed);
+ runWithBody("{\"user_name\":\"alice\"}");
+ assertEquals("alice", renamed.getUserName());
+ }
+
+ public void
testUnannotatedMemberRenamedOntoAnnotatedMemberNameIsRejected() throws
Exception {
+ RenamedPropertiesAction renamed = new RenamedPropertiesAction();
+ setupInterceptorWithAction(renamed);
+ runWithBody("{\"name\":true}");
+ assertFalse("wire key [name] lands on the unannotated setAdmin, not
the annotated setName",
+ renamed.isAdmin());
+ }
+
+ public void
testUnannotatedMemberMergedIntoAnnotatedMemberPropertyIsRejected() throws
Exception {
+ MergedPropertyAction merged = new MergedPropertyAction();
+ setupInterceptorWithAction(merged);
+ runWithBody("{\"name\":\"x\"}");
+ assertNull("Jackson merges both setters into property [name] and
invokes the explicitly named,"
+ + " unannotated setAdmin", merged.admin());
+ }
+
// --- Test fixtures for new path verification ---
/**
@@ -261,4 +285,51 @@ public class ContentTypeInterceptorIntegrationTest extends
TestCase {
return values;
}
}
+
+ /**
+ * Jackson external names diverge from the Java members: the annotated
{@code userName} arrives
+ * as {@code user_name}, and the unannotated {@code admin} arrives under
the annotated member's
+ * Java name {@code name}.
+ */
+ public static class RenamedPropertiesAction extends ActionSupport {
+ private String userName;
+ private String name;
+ private boolean admin;
+
+ public String getUserName() { return userName; }
+
+ @StrutsParameter
+ @JsonProperty("user_name")
+ public void setUserName(String userName) { this.userName = userName; }
+
+ public String getName() { return name; }
+
+ @StrutsParameter
+ @JsonProperty("display_name")
+ public void setName(String name) { this.name = name; }
+
+ public boolean isAdmin() { return admin; }
+
+ @JsonProperty("name")
+ public void setAdmin(boolean admin) { this.admin = admin; }
+ }
+
+ /**
+ * The annotated {@code setName} and the unannotated {@code
@JsonProperty("name") setAdmin} collapse
+ * into a single Jackson property {@code name} whose mutator is {@code
setAdmin}.
+ */
+ public static class MergedPropertyAction extends ActionSupport {
+ private String name;
+ private String admin;
+
+ public String getName() { return name; }
+
+ @StrutsParameter
+ public void setName(String name) { this.name = name; }
+
+ public String admin() { return admin; }
+
+ @JsonProperty("name")
+ public void setAdmin(String admin) { this.admin = admin; }
+ }
}
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
index 0cae3eabe..bb9a6f99b 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
@@ -30,6 +30,7 @@ import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
import com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder;
@@ -46,8 +47,10 @@ import org.apache.struts2.rest.handler.JacksonJsonHandler;
import java.beans.ConstructorProperties;
import java.io.StringReader;
+import java.util.ArrayList;
import java.math.BigDecimal;
import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
@@ -136,6 +139,37 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
ParameterAuthorizationContext.currentPathPrefix());
}
+ public void testNamingStrategyAuthorizesJavaMemberNamesAtEveryLevel()
throws Exception {
+ ObjectMapper snakeCase = new ObjectMapper()
+ .setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
+ .registerModule(new ParameterAuthorizingModule());
+ List<String> seen = new ArrayList<>();
+ bind((path, t, a) -> seen.add(path), new CamelCasePerson());
+ CamelCasePerson result = snakeCase.readValue(
+
"{\"user_name\":\"alice\",\"home_address\":{\"street_name\":\"Main\"},"
+ + "\"other_addresses\":[{\"street_name\":\"Side\"}]}",
+ CamelCasePerson.class);
+ assertEquals("alice", result.userName);
+ assertEquals("Main", result.homeAddress.streetName);
+ assertEquals("Side", result.otherAddresses.get(0).streetName);
+ assertEquals(List.of("userName", "homeAddress",
"homeAddress.streetName",
+ "otherAddresses", "otherAddresses[0].streetName"),
List.copyOf(new LinkedHashSet<>(seen)));
+ }
+
+ public void
testMemberNameKeepsExternalNameForMutatorOutsideBeanConvention() throws
Exception {
+ List<String> seen = new ArrayList<>();
+ bind((path, t, a) -> seen.add(path), new FluentMutatorBean());
+ mapper.readValue("{\"settings\":\"dark\",\"issue\":\"open\"}",
FluentMutatorBean.class);
+ assertEquals(List.of("settings", "issue"), List.copyOf(new
LinkedHashSet<>(seen)));
+ }
+
+ public void testMemberNameKeepsExternalNameForOneArgGetterNamedMutator()
throws Exception {
+ List<String> seen = new ArrayList<>();
+ bind((path, t, a) -> seen.add(path), new GetterNamedMutatorBean());
+ mapper.readValue("{\"nick\":\"x\"}", GetterNamedMutatorBean.class);
+ assertEquals(List.of("nick"), List.copyOf(new LinkedHashSet<>(seen)));
+ }
+
public void testAnySetterEnforcementDisabledByDefault() throws Exception {
bind((path, t, a) -> false, new UnannotatedAnySetterBean());
UnannotatedAnySetterBean result = mapper.readValue(
@@ -1033,4 +1067,35 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
/** A record with a primitive component, to exercise
FAIL_ON_NULL_FOR_PRIMITIVES interaction. */
public record Money(int amount, String currency) {
}
+
+ public static class CamelCasePerson {
+ public String userName;
+ public CamelCaseAddress homeAddress;
+ public List<CamelCaseAddress> otherAddresses;
+ }
+
+ public static class CamelCaseAddress {
+ public String streetName;
+ }
+
+ public static class FluentMutatorBean {
+ String settings;
+ String issue;
+
+ @JsonProperty("settings")
+ public void settings(String settings) { this.settings = settings; }
+
+ @JsonProperty("issue")
+ public void issue(String issue) { this.issue = issue; }
+ }
+
+ public static class GetterNamedMutatorBean {
+ String name;
+ String nick;
+
+ public void setName(String name) { this.name = name; }
+
+ @JsonProperty("nick")
+ public void getName(String nick) { this.nick = nick; }
+ }
}