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 0e5c8ab898..f017ec60ad 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 @@ -41,7 +41,9 @@ * {@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.

+ * straight pass-through to the delegate — no overhead for default-config requests. The one decision + * taken regardless is made when the deserializer is built, not per request: a polymorphic property + * is never merged in place, see {@link AuthorizingValueDeserializer#supportsUpdate}.

* * @since 7.2.0 */ @@ -79,13 +81,14 @@ protected SettableBeanProperty withDelegate(SettableBeanProperty d) { * 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. + * property; it owns the path push for nested members on both the direct and the buffered path, + * and refuses the in-place merge of a polymorphic value that would otherwise skip both. */ @Override public SettableBeanProperty withValueDeserializer(JsonDeserializer deser) { JsonDeserializer effective = deser; if (!(deser instanceof AuthorizingValueDeserializer)) { - effective = new AuthorizingValueDeserializer(deser, memberName, getType()); + effective = new AuthorizingValueDeserializer(deser, memberName, getType(), getValueTypeDeserializer()); } return _with(delegate.withValueDeserializer(effective)); } 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 a30cfe8ef0..122017f4ec 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 @@ -19,6 +19,7 @@ package org.apache.struts2.rest.handler.jackson; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationConfig; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.JsonDeserializer; @@ -44,16 +45,39 @@ final class AuthorizingValueDeserializer extends DelegatingDeserializer { private final String propertyName; private final JavaType propertyType; + private final transient TypeDeserializer valueTypeDeserializer; - AuthorizingValueDeserializer(JsonDeserializer delegate, String propertyName, JavaType propertyType) { + AuthorizingValueDeserializer(JsonDeserializer delegate, String propertyName, JavaType propertyType, + TypeDeserializer valueTypeDeserializer) { super(delegate); this.propertyName = propertyName; this.propertyType = propertyType; + this.valueTypeDeserializer = valueTypeDeserializer; } @Override protected JsonDeserializer newDelegatingInstance(JsonDeserializer newDelegatee) { - return new AuthorizingValueDeserializer(newDelegatee, propertyName, propertyType); + return new AuthorizingValueDeserializer(newDelegatee, propertyName, propertyType, valueTypeDeserializer); + } + + /** + * Merging into a non-null polymorphic value is Jackson's one path past both wrappers: the + * {@code final} {@code SettableBeanProperty#deserializeWith} deserializes in place through a + * deserializer resolved for the existing value's class. {@code BeanDeserializerBase.resolve()} + * asks the value deserializer first, whichever property wrapper it has built by then, so + * declining keeps the property on the ordinary authorized path: the value is replaced and the + * body must carry its type id. Jackson ignores the declined merge under + * {@code MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE} and reports a bad definition otherwise. + */ + @Override + public Boolean supportsUpdate(DeserializationConfig config) { + if (valueTypeDeserializer == null) { + return super.supportsUpdate(config); + } + LOG.warn("Merge disabled for polymorphic REST body property [{}]: an in-place polymorphic merge" + + " cannot be authorized, so the value is replaced and the body must carry its type id", + propertyName); + return Boolean.FALSE; } @Override 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 bb9a6f99bf..68c6e1b0d4 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 @@ -19,16 +19,21 @@ package org.apache.struts2.rest.handler.jackson; import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.annotation.JsonBackReference; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIdentityInfo; +import com.fasterxml.jackson.annotation.JsonManagedReference; 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.annotation.ObjectIdGenerators; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.BeanDescription; import com.fasterxml.jackson.databind.DeserializationConfig; import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; @@ -36,6 +41,7 @@ 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.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.util.TokenBuffer; import com.fasterxml.jackson.dataformat.xml.XmlMapper; @@ -584,6 +590,56 @@ public void testMergeIntoExistingValueIsAuthorized() throws Exception { assertNull("merged into an unauthorized property ?", result.address.city); } + public void testMergedPolymorphicPropertyWithInitialValueIsAuthorized() throws Exception { + // @JsonMerge into a non-null polymorphic value would otherwise deserialize in place through + // Jackson's typed merge, which reaches neither wrapper: the property is never authorized and + // the subtype's members are checked against the enclosing bean's grants. + bind((path, t, a) -> "owner".equals(path), new MergingKennel()); + MergingKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}", MergingKennel.class); + assertNull("subtype member assigned through an unauthorized merged property ?", + ((Dog) result.pet).owner); + } + + public void testMergedPolymorphicPropertyIsReplacedThroughTheAuthorizedPath() throws Exception { + MergingKennel bean = new MergingKennel(); + Animal initial = bean.pet; + bind((path, t, a) -> "pet".equals(path) || "pet.owner".equals(path), bean); + mapper.readerForUpdating(bean).readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}"); + assertEquals("alice", ((Dog) bean.pet).owner); + assertNotSame("merge must be disabled for a polymorphic property", initial, bean.pet); + } + + public void testMergedPolymorphicPropertyBehindObjectIdWrapperIsAuthorized() throws Exception { + // @JsonIdentityInfo makes resolve() copy the property into an ObjectIdReferenceProperty + // before it decides on merging; the merge must still be refused there. + bind((path, t, a) -> "owner".equals(path), new MergingIdentifiedKennel()); + MergingIdentifiedKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}", + MergingIdentifiedKennel.class); + assertNull("subtype member assigned through an unauthorized merged property ?", + ((IdentifiedDog) result.pet).owner); + } + + public void testMergedPolymorphicPropertyBehindManagedReferenceWrapperIsAuthorized() throws Exception { + bind((path, t, a) -> "owner".equals(path), new MergingManagedKennel()); + MergingManagedKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}", + MergingManagedKennel.class); + assertNull("subtype member assigned through an unauthorized merged property ?", + ((ManagedDog) result.pet).owner); + } + + public void testMergedPolymorphicPropertyIsRefusedWhenUnmergeableIsNotIgnored() throws Exception { + ObjectMapper strict = new ObjectMapper() + .disable(MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE) + .registerModule(new ParameterAuthorizingModule()); + bind((path, t, a) -> true, new MergingKennel()); + try { + strict.readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}", MergingKennel.class); + fail("a polymorphic merge the module cannot authorize must be reported as a bad definition"); + } catch (InvalidDefinitionException expected) { + assertTrue(expected.getMessage(), expected.getMessage().contains("cannot be merged")); + } + } + 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). @@ -816,6 +872,46 @@ public static class MergingBean { public Address address = new Address(); } + public static class MergingKennel { + @JsonMerge + public Animal pet = new Dog(); + public String owner; + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type") + @JsonSubTypes(@JsonSubTypes.Type(value = IdentifiedDog.class, name = "dog")) + @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") + public abstract static class IdentifiedAnimal { + } + + public static class IdentifiedDog extends IdentifiedAnimal { + public String owner; + } + + public static class MergingIdentifiedKennel { + @JsonMerge + public IdentifiedAnimal pet = new IdentifiedDog(); + public String owner; + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type") + @JsonSubTypes(@JsonSubTypes.Type(value = ManagedDog.class, name = "dog")) + public abstract static class ManagedAnimal { + @JsonBackReference + public MergingManagedKennel kennel; + } + + public static class ManagedDog extends ManagedAnimal { + public String owner; + } + + public static class MergingManagedKennel { + @JsonMerge + @JsonManagedReference + public ManagedAnimal pet = new ManagedDog(); + public String owner; + } + public static class CreatorHolder { public final CreatorWithSetter inner;