Path tracking: the wrapper pushes the full path of the current property onto the context's - * path stack before delegating, then pops in a {@code finally} block. For collection / map / array-typed - * properties, the path pushed is suffixed with {@code [0]} so nested element members produce paths like - * {@code items[0].field} — matching {@code ParametersInterceptor} depth semantics.
+ *Path tracking for nested members is done by the {@link AuthorizingValueDeserializer} wrapped + * around every property's value deserializer, so the direct path and the buffered creator path + * compute the same paths. Values Jackson assigns after construction go through {@link #set} and + * {@link #setAndReturn}, which apply the same authorization to the already-materialized value.
* *When {@link ParameterAuthorizationContext#isActive()} is {@code false}, this wrapper is a * straight pass-through to the delegate — no overhead for default-config requests.
@@ -61,17 +59,17 @@ protected SettableBeanProperty withDelegate(SettableBeanProperty d) { } /** - * Creator-bound properties (records, {@code @JsonCreator} constructors) never reach - * {@link #deserializeAndSet}/{@link #deserializeSetAndReturn}: Jackson calls the {@code final} - * {@code SettableBeanProperty#deserialize} directly, through this property's own value deserializer. - * Wrap that deserializer with {@link AuthorizingValueDeserializer}, scoped to {@link CreatorProperty} - * so ordinary setter/field/builder properties -- already authorized below -- aren't double-checked. + * Creator-bound properties, and non-creator properties Jackson buffers while collecting creator + * parameters, never reach {@link #deserializeAndSet}/{@link #deserializeSetAndReturn}: Jackson calls + * the {@code final} {@code SettableBeanProperty#deserialize} directly, through this property's own + * value deserializer. Wrap that deserializer with {@link AuthorizingValueDeserializer} for every + * property; it owns the path push for nested members on both the direct and the buffered path. */ @Override public SettableBeanProperty withValueDeserializer(JsonDeserializer deser) { JsonDeserializer effective = deser; - if (delegate instanceof CreatorProperty && !(deser instanceof AuthorizingValueDeserializer)) { - effective = new AuthorizingValueDeserializer(deser, getName()); + if (!(deser instanceof AuthorizingValueDeserializer)) { + effective = new AuthorizingValueDeserializer(deser, getName(), getType()); } return _with(delegate.withValueDeserializer(effective)); } @@ -90,12 +88,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, Object p.skipChildren(); return; } - ParameterAuthorizationContext.pushPath(prefixForNested(path)); - try { - delegate.deserializeAndSet(p, ctxt, instance); - } finally { - ParameterAuthorizationContext.popPath(); - } + delegate.deserializeAndSet(p, ctxt, instance); } @Override @@ -111,24 +104,41 @@ public Object deserializeSetAndReturn(JsonParser p, DeserializationContext ctxt, p.skipChildren(); return instance; } - ParameterAuthorizationContext.pushPath(prefixForNested(path)); - try { - return delegate.deserializeSetAndReturn(p, ctxt, instance); - } finally { - ParameterAuthorizationContext.popPath(); + return delegate.deserializeSetAndReturn(p, ctxt, instance); + } + + @Override + public void set(Object instance, Object value) throws IOException { + if (isAuthorizedForSet(instance)) { + delegate.set(instance, value); } } + @Override + public Object setAndReturn(Object instance, Object value) throws IOException { + if (isAuthorizedForSet(instance)) { + return delegate.setAndReturn(instance, value); + } + return instance; + } + /** - * For Collection / Map / Array properties, the path to push for nested element members is - * {@code path + "[0]"} — matching {@code ParametersInterceptor} bracket-depth semantics. Scalar / - * bean properties push the path unchanged. + * Guards the already-materialized assignment path: Jackson buffers non-creator properties seen + * before the last creator parameter and assigns them after construction via + * {@code PropertyValue.Regular.assign} -> {@code set()}, which does not go through + * {@link #deserializeAndSet}. */ - private String prefixForNested(String pathOfThisProperty) { - JavaType type = getType(); - if (type != null && (type.isCollectionLikeType() || type.isMapLikeType() || type.isArrayType())) { - return pathOfThisProperty + "[0]"; + private boolean isAuthorizedForSet(Object instance) { + if (!ParameterAuthorizationContext.isActive()) { + return true; + } + String path = ParameterAuthorizationContext.pathFor(getName()); + if (DynamicKeyAuthorizationContext.isAuthorized(path)) { + return true; } - return pathOfThisProperty; + LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization on [{}]", + path, instance.getClass().getName()); + ParameterAuthorizationContext.markRedacted(); + return false; } } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java index fc9532960c..a30cfe8ef0 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java @@ -20,36 +20,40 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.deser.std.DelegatingDeserializer; +import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; import java.io.IOException; -import java.util.Collection; -import java.util.Map; /** - * Enforces {@code @StrutsParameter} authorization for creator-bound properties (Java records, - * {@code @JsonCreator} constructors, {@code @ConstructorProperties}), which Jackson deserializes - * through the value deserializer directly rather than through a {@code SettableBeanProperty}. - * See {@link AuthorizingSettableBeanProperty#withValueDeserializer} for where this is installed. + * Enforces {@code @StrutsParameter} authorization on a property's value deserializer, and owns the + * path push for its nested members. It is installed on every property by + * {@link AuthorizingSettableBeanProperty#withValueDeserializer}, so the same path is computed whether + * Jackson reaches the value through {@code deserializeAndSet}, through the {@code final} + * {@code SettableBeanProperty#deserialize} used for creator parameters and buffered properties, in + * place for a setterless collection, or through a type deserializer for a polymorphic property. */ final class AuthorizingValueDeserializer extends DelegatingDeserializer { private static final Logger LOG = LogManager.getLogger(AuthorizingValueDeserializer.class); private final String propertyName; + private final JavaType propertyType; - AuthorizingValueDeserializer(JsonDeserializer delegate, String propertyName) { + AuthorizingValueDeserializer(JsonDeserializer delegate, String propertyName, JavaType propertyType) { super(delegate); this.propertyName = propertyName; + this.propertyType = propertyType; } @Override protected JsonDeserializer newDelegatingInstance(JsonDeserializer newDelegatee) { - return new AuthorizingValueDeserializer(newDelegatee, propertyName); + return new AuthorizingValueDeserializer(newDelegatee, propertyName, propertyType); } @Override @@ -58,10 +62,7 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx return super.deserialize(p, ctxt); } String path = ParameterAuthorizationContext.pathFor(propertyName); - if (!DynamicKeyAuthorizationContext.isAuthorized(path)) { - LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization (creator-bound property)", path); - ParameterAuthorizationContext.markRedacted(); - p.skipChildren(); + if (!authorize(path, p)) { // Returning null redacts the value. For a primitive creator component this becomes the // type default (0/false) unless FAIL_ON_NULL_FOR_PRIMITIVES is on (then construction // fails and RedactionAwareDeserializer drops the whole object) -- either way the @@ -76,15 +77,59 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx } } + @Override + public Object deserialize(JsonParser p, DeserializationContext ctxt, Object intoValue) throws IOException { + if (!ParameterAuthorizationContext.isActive()) { + return super.deserialize(p, ctxt, intoValue); + } + String path = ParameterAuthorizationContext.pathFor(propertyName); + if (!authorize(path, p)) { + return intoValue; + } + ParameterAuthorizationContext.pushPath(prefixForNested(path)); + try { + return super.deserialize(p, ctxt, intoValue); + } finally { + ParameterAuthorizationContext.popPath(); + } + } + + @Override + public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer) + throws IOException { + if (!ParameterAuthorizationContext.isActive()) { + return super.deserializeWithType(p, ctxt, typeDeserializer); + } + String path = ParameterAuthorizationContext.pathFor(propertyName); + if (!authorize(path, p)) { + return null; + } + ParameterAuthorizationContext.pushPath(prefixForNested(path)); + try { + return super.deserializeWithType(p, ctxt, typeDeserializer); + } finally { + ParameterAuthorizationContext.popPath(); + } + } + + private boolean authorize(String path, JsonParser p) throws IOException { + if (DynamicKeyAuthorizationContext.isAuthorized(path)) { + return true; + } + LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization", path); + ParameterAuthorizationContext.markRedacted(); + p.skipChildren(); + return false; + } + /** - * For Collection / Map / Array-valued creator parameters, the path to push for nested element - * members is {@code path + "[0]"} -- matching {@code ParametersInterceptor} bracket-depth - * semantics, and {@link AuthorizingSettableBeanProperty#prefixForNested}. Scalar / bean-valued - * parameters push the path unchanged. + * For Collection / Map / Array properties, the path to push for nested element members is + * {@code path + "[0]"} -- matching {@code ParametersInterceptor} bracket-depth semantics. Scalar / + * bean-valued properties push the path unchanged. */ private String prefixForNested(String pathOfThisProperty) { - Class type = handledType(); - if (type != null && (Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type) || type.isArray())) { + if (propertyType != null + && (propertyType.isCollectionLikeType() || propertyType.isMapLikeType() || propertyType.isArrayType())) { return pathOfThisProperty + "[0]"; } return pathOfThisProperty; 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 61ee26eafa..0cae3eabe0 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 @@ -20,7 +20,10 @@ import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonMerge; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonUnwrapped; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.BeanDescription; @@ -434,6 +437,130 @@ public void testConstructorPropertiesAuthorizedByPath() throws Exception { assertNull(result.secret); } + public void testSetterBufferedBeforeCreatorParamIsAuthorized() throws Exception { + // A non-creator property that appears before the last creator parameter is buffered by + // Jackson and assigned through SettableBeanProperty.set() after construction. + bind((path, t, a) -> "name".equals(path), new CreatorWithSetter("")); + CreatorWithSetter result = mapper.readValue("{\"role\":\"admin\",\"name\":\"alice\"}", CreatorWithSetter.class); + assertEquals("alice", result.name); + assertNull("buffered setter property assigned without authorization ?", result.getRole()); + } + + public void testSetterAfterCreatorParamIsAuthorized() throws Exception { + // Control: the same property after the last creator parameter takes the direct path. + bind((path, t, a) -> "name".equals(path), new CreatorWithSetter("")); + CreatorWithSetter result = mapper.readValue("{\"name\":\"alice\",\"role\":\"admin\"}", CreatorWithSetter.class); + assertEquals("alice", result.name); + assertNull(result.getRole()); + } + + public void testSetterOnlyTypeSameMemberOrderIsAuthorized() throws Exception { + // Control: without a creator nothing is buffered, so member order does not matter. + bind((path, t, a) -> "name".equals(path), new Person()); + Person result = mapper.readValue("{\"role\":\"admin\",\"name\":\"alice\"}", Person.class); + assertEquals("alice", result.name); + assertNull(result.role); + } + + public void testNestedSetterBufferedBeforeCreatorParamIsAuthorized() throws Exception { + bind((path, t, a) -> "inner".equals(path) || "inner.name".equals(path), new CreatorHolder(null)); + CreatorHolder result = mapper.readValue( + "{\"inner\":{\"role\":\"admin\",\"name\":\"alice\"}}", CreatorHolder.class); + assertNotNull(result.inner); + assertEquals("alice", result.inner.name); + assertNull("nested buffered setter property assigned without authorization ?", result.inner.getRole()); + } + + public void testBufferedBeanValuedSetterChildrenAuthorizedAtOwnDepth() throws Exception { + // The buffered read goes through the final SettableBeanProperty.deserialize(), so the + // nested bean's members must still be authorized under the property's own prefix. + bind((path, t, a) -> "inner".equals(path) || "inner.name".equals(path) + || "inner.address".equals(path) || "inner.address.city".equals(path), new CreatorAddressHolder(null)); + CreatorAddressHolder result = mapper.readValue( + "{\"inner\":{\"address\":{\"city\":\"Warsaw\",\"zip\":\"00-001\"},\"name\":\"alice\"}}", + CreatorAddressHolder.class); + assertEquals("alice", result.inner.name); + assertNotNull("buffered nested bean dropped ?", result.inner.getAddress()); + assertEquals("nested member checked at the wrong depth ?", "Warsaw", result.inner.getAddress().city); + assertNull(result.inner.getAddress().zip); + } + + public void testBeanValuedSetterAfterCreatorParamChildrenAuthorizedAtOwnDepth() throws Exception { + // Control: the direct path for the same property and authorizer. + bind((path, t, a) -> "inner".equals(path) || "inner.name".equals(path) + || "inner.address".equals(path) || "inner.address.city".equals(path), new CreatorAddressHolder(null)); + CreatorAddressHolder result = mapper.readValue( + "{\"inner\":{\"name\":\"alice\",\"address\":{\"city\":\"Warsaw\",\"zip\":\"00-001\"}}}", + CreatorAddressHolder.class); + assertEquals("alice", result.inner.name); + assertEquals("Warsaw", result.inner.getAddress().city); + assertNull(result.inner.getAddress().zip); + } + + public void testSetterlessCollectionElementsAuthorizedAtOwnDepth() throws Exception { + // A collection getter without a setter is deserialized in place through the three-argument + // deserialize(); element members must be checked under items[0], not under the parent. + bind((path, t, a) -> "order".equals(path) || "order.items".equals(path) || "order.name".equals(path), + new OrderHolder()); + OrderHolder result = mapper.readValue("{\"order\":{\"items\":[{\"name\":\"x\"}]}}", OrderHolder.class); + assertEquals(1, result.order.getItems().size()); + assertNull("element member authorized by the parent's sibling grant ?", result.order.getItems().get(0).name); + } + + public void testPolymorphicPropertyMembersAuthorizedAtOwnDepth() throws Exception { + // A @JsonTypeInfo property is deserialized through deserializeWithType(); the subtype's + // members must be checked under pet, not against the enclosing bean. + bind((path, t, a) -> "pet".equals(path) || "owner".equals(path), new Kennel()); + Kennel result = mapper.readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}", Kennel.class); + assertTrue(result.pet instanceof Dog); + assertNull("subtype member authorized by the enclosing bean's sibling grant ?", ((Dog) result.pet).owner); + } + + public void testNoContext_passThroughBufferedSetter() throws Exception { + CreatorWithSetter result = mapper.readValue("{\"role\":\"admin\",\"name\":\"alice\"}", CreatorWithSetter.class); + assertEquals("alice", result.name); + assertEquals("admin", result.getRole()); + } + + public void testNoContext_passThroughSetterlessCollection() throws Exception { + OrderHolder result = mapper.readValue("{\"order\":{\"items\":[{\"name\":\"x\"}]}}", OrderHolder.class); + assertEquals("x", result.order.getItems().get(0).name); + } + + public void testNoContext_passThroughPolymorphicProperty() throws Exception { + Kennel result = mapper.readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}", Kennel.class); + assertEquals("alice", ((Dog) result.pet).owner); + } + + public void testBufferedPolymorphicSetterIsAuthorized() throws Exception { + // A polymorphic setter buffered before the creator parameter goes through deserializeWithType(). + bind((path, t, a) -> "name".equals(path), new CreatorWithPet("")); + CreatorWithPet result = mapper.readValue( + "{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"},\"name\":\"alice\"}", CreatorWithPet.class); + assertEquals("alice", result.name); + assertNull("buffered polymorphic setter assigned without authorization ?", result.getPet()); + } + + public void testMergeIntoExistingValueIsAuthorized() throws Exception { + // @JsonMerge into a non-null value deserializes in place through the three-argument + // deserialize(); an unauthorized property must leave the existing value untouched. + bind((path, t, a) -> "name".equals(path), new MergingBean()); + MergingBean result = mapper.readValue("{\"name\":\"alice\",\"address\":{\"city\":\"Warsaw\"}}", MergingBean.class); + assertEquals("alice", result.name); + assertNull("merged into an unauthorized property ?", result.address.city); + } + + public void testBufferedSetterInsideDynamicKeyScopeIsAuthorizedByDepth() throws Exception { + // Inside a dynamic-key scope the buffered path must consult the same depth rule as the + // direct path, not the annotation authorizer (which rejects everything here). + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthTwoCreatorAnySetterBean()); + DynamicDepthTwoCreatorAnySetterBean result = enforcingMapper.readValue( + "{\"home\":{\"role\":\"admin\",\"name\":\"alice\"}}", DynamicDepthTwoCreatorAnySetterBean.class); + assertEquals("alice", result.values.get("home").name); + assertEquals("admin", result.values.get("home").getRole()); + } + public void testCreatorPropertyEntirelyRejected_dropsWholeSubtree() throws Exception { // "inner" itself is never authorized -- the whole nested creator-bound object must be // dropped, matching how a rejected non-creator nested bean property behaves (see @@ -555,6 +682,115 @@ public static class Person { public RecordAddress recordAddress; } + public static class CreatorWithSetter { + public final String name; + private String role; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public CreatorWithSetter(@JsonProperty("name") String name) { + this.name = name; + } + + public String getRole() { + return role; + } + + public void setRole(String role) { + this.role = role; + } + } + + public static class CreatorWithAddress { + public final String name; + private Address address; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public CreatorWithAddress(@JsonProperty("name") String name) { + this.name = name; + } + + public Address getAddress() { + return address; + } + + public void setAddress(Address address) { + this.address = address; + } + } + + public static class CreatorAddressHolder { + public final CreatorWithAddress inner; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public CreatorAddressHolder(@JsonProperty("inner") CreatorWithAddress inner) { + this.inner = inner; + } + } + + public static class OrderHolder { + public Order order; + } + + public static class Order { + public String name; + private final java.util.List