diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java index 1fbafa3c7f..9e936239ba 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameter.java @@ -43,4 +43,15 @@ * In a practical sense, the depth dictates the number of periods or brackets that can appear in the parameter name. */ int depth() default 0; + + /** + * Allows a dynamic property sink, such as a REST JSON or XML any-setter, to accept parameter + * names that do not correspond to declared members on the action. + *

+ * This is an explicit opt-in for input channels that support dynamic keys. Ordinary parameter + * injection ignores this flag, although placing {@code @StrutsParameter} on a field still makes + * that field eligible for ordinary query and form parameter injection. The {@link #depth()} value + * limits nesting below each dynamic key. + */ + boolean allowDynamicKeys() default false; } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java index cb47b6a939..d2675ecee0 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestConstants.java @@ -23,6 +23,7 @@ public class RestConstants { public static final String REST_LOGGER = "struts.rest.logger"; public static final String REST_DEFAULT_ERROR_RESULT_NAME = "struts.rest.defaultErrorResultName"; public static final String REST_CONTENT_RESTRICT_TO_GET = "struts.rest.content.restrictToGET"; + public static final String REST_ANY_SETTER_REQUIRE_ANNOTATIONS = "struts.rest.anySetter.requireAnnotations"; public static final String REST_MAPPER_INDEX_METHOD_NAME = "struts.mapper.indexMethodName"; public static final String REST_MAPPER_GET_METHOD_NAME = "struts.mapper.getMethodName"; public static final String REST_MAPPER_POST_METHOD_NAME = "struts.mapper.postMethodName"; diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/config/entities/RestConstantConfig.java b/plugins/rest/src/main/java/org/apache/struts2/rest/config/entities/RestConstantConfig.java index d83f01b42b..2b814cfe09 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/config/entities/RestConstantConfig.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/config/entities/RestConstantConfig.java @@ -29,6 +29,7 @@ public class RestConstantConfig extends ConstantConfig { private Boolean restLogger; private String restDefaultErrorResultName; private Boolean restContentRestrictToGet; + private Boolean restAnySetterRequireAnnotations; private String mapperIndexMethodName; private String mapperGetMethodName; private String mapperPostMethodName; @@ -50,6 +51,8 @@ public Map getAllAsStringsMap() { map.put(RestConstants.REST_LOGGER, Objects.toString(restLogger, null)); map.put(RestConstants.REST_DEFAULT_ERROR_RESULT_NAME, restDefaultErrorResultName); map.put(RestConstants.REST_CONTENT_RESTRICT_TO_GET, Objects.toString(restContentRestrictToGet, null)); + map.put(RestConstants.REST_ANY_SETTER_REQUIRE_ANNOTATIONS, + Objects.toString(restAnySetterRequireAnnotations, null)); map.put(RestConstants.REST_MAPPER_INDEX_METHOD_NAME, mapperIndexMethodName); map.put(RestConstants.REST_MAPPER_GET_METHOD_NAME, mapperGetMethodName); map.put(RestConstants.REST_MAPPER_POST_METHOD_NAME, mapperPostMethodName); @@ -98,6 +101,14 @@ public void setRestContentRestrictToGet(Boolean restContentRestrictToGet) { this.restContentRestrictToGet = restContentRestrictToGet; } + public Boolean getRestAnySetterRequireAnnotations() { + return restAnySetterRequireAnnotations; + } + + public void setRestAnySetterRequireAnnotations(Boolean restAnySetterRequireAnnotations) { + this.restAnySetterRequireAnnotations = restAnySetterRequireAnnotations; + } + public String getMapperIndexMethodName() { return mapperIndexMethodName; } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonJsonHandler.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonJsonHandler.java index 834661cd5d..20f520d420 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonJsonHandler.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonJsonHandler.java @@ -21,9 +21,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.SerializationFeature; +import org.apache.commons.lang3.BooleanUtils; import org.apache.struts2.ActionInvocation; import org.apache.struts2.inject.Inject; import org.apache.struts2.StrutsConstants; +import org.apache.struts2.rest.RestConstants; +import org.apache.struts2.rest.handler.jackson.ParameterAuthorizingModule; import java.io.IOException; import java.io.Reader; @@ -36,14 +39,19 @@ public class JacksonJsonHandler implements AuthorizationAwareContentTypeHandler private static final String DEFAULT_CONTENT_TYPE = "application/json"; private String defaultEncoding = "ISO-8859-1"; + private final ParameterAuthorizingModule parameterAuthorizingModule = new ParameterAuthorizingModule(); private ObjectMapper mapper = new ObjectMapper() - .registerModule(new org.apache.struts2.rest.handler.jackson.ParameterAuthorizingModule()); + .registerModule(parameterAuthorizingModule); @Override public void toObject(ActionInvocation invocation, Reader in, Object target) throws IOException { mapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false); ObjectReader or = mapper.readerForUpdating(target); - or.readValue(in); + try { + or.readValue(in); + } finally { + parameterAuthorizingModule.clearAuthorizationContext(); + } } @Override @@ -67,4 +75,9 @@ public String getExtension() { public void setDefaultEncoding(String val) { this.defaultEncoding = val; } + + @Inject(value = RestConstants.REST_ANY_SETTER_REQUIRE_ANNOTATIONS, required = false) + public void setAnySetterRequireAnnotations(String value) { + parameterAuthorizingModule.setRequireAnySetterAnnotations(BooleanUtils.toBoolean(value)); + } } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java index ccc102023e..a25b151aa6 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java @@ -20,9 +20,13 @@ import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.dataformat.xml.XmlMapper; +import org.apache.commons.lang3.BooleanUtils; import org.apache.struts2.ActionInvocation; +import org.apache.struts2.inject.Inject; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.rest.RestConstants; +import org.apache.struts2.rest.handler.jackson.ParameterAuthorizingModule; import java.io.IOException; import java.io.Reader; @@ -37,17 +41,22 @@ public class JacksonXmlHandler implements AuthorizationAwareContentTypeHandler { private static final String DEFAULT_CONTENT_TYPE = "application/xml"; private final XmlMapper mapper; + private final ParameterAuthorizingModule parameterAuthorizingModule = new ParameterAuthorizingModule(); public JacksonXmlHandler() { mapper = new XmlMapper(); - mapper.registerModule(new org.apache.struts2.rest.handler.jackson.ParameterAuthorizingModule()); + mapper.registerModule(parameterAuthorizingModule); } @Override public void toObject(ActionInvocation invocation, Reader in, Object target) throws IOException { LOG.debug("Converting input into an object of: {}", target.getClass().getName()); ObjectReader or = mapper.readerForUpdating(target); - or.readValue(in); + try { + or.readValue(in); + } finally { + parameterAuthorizingModule.clearAuthorizationContext(); + } } @Override @@ -67,4 +76,9 @@ public String getExtension() { return "xml"; } + @Inject(value = RestConstants.REST_ANY_SETTER_REQUIRE_ANNOTATIONS, required = false) + public void setAnySetterRequireAnnotations(String value) { + parameterAuthorizingModule.setRequireAnySetterAnnotations(BooleanUtils.toBoolean(value)); + } + } diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java new file mode 100644 index 0000000000..6b11f13bf1 --- /dev/null +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableAnyProperty.java @@ -0,0 +1,266 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.struts2.rest.handler.jackson; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.BeanProperty; +import com.fasterxml.jackson.databind.DeserializationConfig; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.deser.SettableAnyProperty; +import com.fasterxml.jackson.databind.introspect.AnnotatedMember; +import com.fasterxml.jackson.databind.util.TokenBuffer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; +import org.apache.struts2.interceptor.parameter.StrutsParameter; + +import java.io.IOException; + +/** + * Requires an explicit {@link StrutsParameter#allowDynamicKeys()} opt-in before a Jackson + * any-setter can consume dynamic REST body properties. + */ +final class AuthorizingSettableAnyProperty extends SettableAnyProperty { + + private static final long serialVersionUID = 1L; + private static final Logger LOG = LogManager.getLogger(AuthorizingSettableAnyProperty.class); + private static final Object REJECTED_VALUE = new Object(); + + private final SettableAnyProperty delegate; + private final StrutsParameter permission; + private final boolean creatorParameter; + + AuthorizingSettableAnyProperty(SettableAnyProperty delegate) { + super(delegate.getProperty(), memberOf(delegate.getProperty()), delegate.getType(), + null, null, null); + this.delegate = delegate; + this.permission = permissionOf(delegate.getProperty()); + this.creatorParameter = delegate.getParameterIndex() >= 0; + } + + private static AnnotatedMember memberOf(BeanProperty property) { + return property == null ? null : property.getMember(); + } + + private static StrutsParameter permissionOf(BeanProperty property) { + AnnotatedMember member = memberOf(property); + return member == null ? null : member.getAnnotation(StrutsParameter.class); + } + + @Override + public SettableAnyProperty withValueDeserializer(JsonDeserializer deserializer) { + return new AuthorizingSettableAnyProperty(delegate.withValueDeserializer(deserializer)); + } + + @Override + public void fixAccess(DeserializationConfig config) { + delegate.fixAccess(config); + } + + @Override + public boolean hasValueDeserializer() { + return delegate.hasValueDeserializer(); + } + + @Override + public String getPropertyName() { + return delegate.getPropertyName(); + } + + @Override + public int getParameterIndex() { + return delegate.getParameterIndex(); + } + + @Override + public boolean isFieldType() { + return delegate.isFieldType(); + } + + @Override + public boolean isSetterType() { + return delegate.isSetterType(); + } + + @Override + public Object createParameterObject() { + return delegate.createParameterObject(); + } + + @Override + public void deserializeAndSet(JsonParser parser, DeserializationContext context, + Object instance, String propertyName) throws IOException { + if (!ParameterAuthorizationContext.isActive()) { + delegate.deserializeAndSet(parser, context, instance, propertyName); + return; + } + + String path = ParameterAuthorizationContext.pathFor(propertyName); + int allowedDepth = allowedDepth(path); + if (allowedDepth < 0) { + rejectPermission(parser, path); + return; + } + + try (TokenBuffer value = TokenBuffer.asCopyOfValue(parser)) { + int valueDepth = valueDepth(value); + if (valueDepth > allowedDepth) { + rejectDepth(parser, path, valueDepth, allowedDepth); + return; + } + try (JsonParser replay = value.asParserOnFirstToken()) { + boolean scopePushed = false; + boolean pathPushed = false; + try { + DynamicKeyAuthorizationContext.push(path, allowedDepth); + scopePushed = true; + ParameterAuthorizationContext.pushPath(prefixForNested(path)); + pathPushed = true; + delegate.deserializeAndSet(replay, context, instance, propertyName); + } finally { + if (pathPushed) { + ParameterAuthorizationContext.popPath(); + } + if (scopePushed) { + DynamicKeyAuthorizationContext.pop(); + } + } + } + } + } + + @Override + public Object deserialize(JsonParser parser, DeserializationContext context) throws IOException { + if (!ParameterAuthorizationContext.isActive()) { + return delegate.deserialize(parser, context); + } + + String propertyName = parser.currentName(); + if (propertyName == null) { + rejectMissingPropertyName(parser); + return REJECTED_VALUE; + } + + String path = ParameterAuthorizationContext.pathFor(propertyName); + int allowedDepth = allowedDepth(path); + if (allowedDepth < 0) { + rejectPermission(parser, path); + return REJECTED_VALUE; + } + + try (TokenBuffer value = TokenBuffer.asCopyOfValue(parser)) { + int valueDepth = valueDepth(value); + if (valueDepth > allowedDepth) { + rejectDepth(parser, path, valueDepth, allowedDepth); + return REJECTED_VALUE; + } + try (JsonParser replay = value.asParserOnFirstToken()) { + boolean scopePushed = false; + boolean pathPushed = false; + try { + DynamicKeyAuthorizationContext.push(path, allowedDepth); + scopePushed = true; + ParameterAuthorizationContext.pushPath(prefixForNested(path)); + pathPushed = true; + return delegate.deserialize(replay, context); + } finally { + if (pathPushed) { + ParameterAuthorizationContext.popPath(); + } + if (scopePushed) { + DynamicKeyAuthorizationContext.pop(); + } + } + } + } + } + + @Override + public void set(Object instance, Object propertyName, Object value) throws IOException { + if (value != REJECTED_VALUE) { + delegate.set(instance, propertyName, value); + } + } + + @Override + protected void _set(Object instance, Object propertyName, Object value) throws Exception { + if (value != REJECTED_VALUE) { + delegate.set(instance, propertyName, value); + } + } + + private int allowedDepth(String path) { + if (creatorParameter || permission == null || !permission.allowDynamicKeys()) { + return -1; + } + return DynamicKeyAuthorizationContext.limitForNestedScope(path, permission.depth()); + } + + private void rejectPermission(JsonParser parser, String path) throws IOException { + if (creatorParameter) { + LOG.warn("REST body creator-parameter any-setter [{}] rejected; dynamic-key consent " + + "can only be declared on an any-setter method or field", path); + } else { + LOG.warn("REST body any-setter parameter [{}] rejected; dynamic keys require " + + "@StrutsParameter(allowDynamicKeys = true) on a method or field", path); + } + redactAndSkip(parser); + } + + private void rejectDepth(JsonParser parser, String path, int valueDepth, int allowedDepth) throws IOException { + LOG.warn("REST body any-setter parameter [{}] rejected; value depth [{}] exceeds " + + "@StrutsParameter depth [{}]", path, valueDepth, allowedDepth); + redactAndSkip(parser); + } + + private void rejectMissingPropertyName(JsonParser parser) throws IOException { + LOG.warn("REST body any-setter parameter rejected; dynamic property name is unavailable"); + redactAndSkip(parser); + } + + private void redactAndSkip(JsonParser parser) throws IOException { + ParameterAuthorizationContext.markRedacted(); + parser.skipChildren(); + } + + private int valueDepth(TokenBuffer value) throws IOException { + int currentDepth = 0; + int maximumDepth = 0; + try (JsonParser parser = value.asParserOnFirstToken()) { + for (JsonToken token = parser.currentToken(); token != null; token = parser.nextToken()) { + if (token == JsonToken.START_OBJECT || token == JsonToken.START_ARRAY) { + currentDepth++; + maximumDepth = Math.max(maximumDepth, currentDepth); + } else if (token == JsonToken.END_OBJECT || token == JsonToken.END_ARRAY) { + currentDepth--; + } + } + } + return maximumDepth; + } + + private String prefixForNested(String path) { + JavaType type = delegate.getType(); + if (type != null && (type.isCollectionLikeType() || type.isMapLikeType() || type.isArrayType())) { + return path + "[0]"; + } + return path; + } +} 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 3da5c5f090..14c3bcec72 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 @@ -83,7 +83,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, Object return; } String path = ParameterAuthorizationContext.pathFor(getName()); - if (!ParameterAuthorizationContext.isAuthorized(path)) { + if (!DynamicKeyAuthorizationContext.isAuthorized(path)) { LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization on [{}]", path, instance.getClass().getName()); ParameterAuthorizationContext.markRedacted(); @@ -104,7 +104,7 @@ public Object deserializeSetAndReturn(JsonParser p, DeserializationContext ctxt, return delegate.deserializeSetAndReturn(p, ctxt, instance); } String path = ParameterAuthorizationContext.pathFor(getName()); - if (!ParameterAuthorizationContext.isAuthorized(path)) { + if (!DynamicKeyAuthorizationContext.isAuthorized(path)) { LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization on [{}]", path, instance.getClass().getName()); ParameterAuthorizationContext.markRedacted(); 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 0a1369e34c..fc9532960c 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 @@ -58,7 +58,7 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx return super.deserialize(p, ctxt); } String path = ParameterAuthorizationContext.pathFor(propertyName); - if (!ParameterAuthorizationContext.isAuthorized(path)) { + if (!DynamicKeyAuthorizationContext.isAuthorized(path)) { LOG.warn("REST body parameter [{}] rejected by @StrutsParameter authorization (creator-bound property)", path); ParameterAuthorizationContext.markRedacted(); p.skipChildren(); diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyAuthorizationContext.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyAuthorizationContext.java new file mode 100644 index 0000000000..fdafde4ebf --- /dev/null +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/DynamicKeyAuthorizationContext.java @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.struts2.rest.handler.jackson; + +import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; + +import java.util.ArrayDeque; +import java.util.Deque; + +/** + * Tracks the bounded subtree authorized by an explicitly opted-in Jackson any-setter. + */ +final class DynamicKeyAuthorizationContext { + + private static final ThreadLocal> SCOPES = new ThreadLocal<>(); + + private DynamicKeyAuthorizationContext() { + // utility + } + + static boolean isAuthorized(String path) { + Deque scopes = SCOPES.get(); + if (scopes == null || scopes.isEmpty()) { + return ParameterAuthorizationContext.isAuthorized(path); + } + return remainingDepth(scopes.peek(), path) >= 0; + } + + static int limitForNestedScope(String path, int requestedDepth) { + if (requestedDepth < 0) { + return -1; + } + Deque scopes = SCOPES.get(); + if (scopes == null || scopes.isEmpty()) { + return requestedDepth; + } + int remainingDepth = remainingDepth(scopes.peek(), path); + return remainingDepth < 0 ? -1 : Math.min(requestedDepth, remainingDepth); + } + + static void push(String basePath, int maxDepth) { + Deque scopes = SCOPES.get(); + if (scopes == null) { + scopes = new ArrayDeque<>(); + SCOPES.set(scopes); + } + scopes.push(new Scope(basePath, maxDepth)); + } + + static void pop() { + Deque scopes = SCOPES.get(); + if (scopes != null && !scopes.isEmpty()) { + scopes.pop(); + } + if (scopes == null || scopes.isEmpty()) { + SCOPES.remove(); + } + } + + static boolean isActive() { + Deque scopes = SCOPES.get(); + return scopes != null && !scopes.isEmpty(); + } + + static void clear() { + SCOPES.remove(); + } + + private static int remainingDepth(Scope scope, String path) { + if (path == null || scope.basePath == null || !path.startsWith(scope.basePath)) { + return -1; + } + if (path.length() == scope.basePath.length()) { + return scope.maxDepth; + } + + char boundary = path.charAt(scope.basePath.length()); + if (boundary != '.' && boundary != '[' && boundary != '(') { + return -1; + } + + int usedDepth = 0; + for (int i = scope.basePath.length(); i < path.length(); i++) { + char current = path.charAt(i); + if (current == '.' || current == '[' || current == '(') { + usedDepth++; + } + } + return scope.maxDepth - usedDepth; + } + + private static final class Scope { + private final String basePath; + private final int maxDepth; + + private Scope(String basePath, int maxDepth) { + this.basePath = basePath; + this.maxDepth = maxDepth; + } + } +} 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 c3a191ef0c..99450c185c 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 @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.JsonDeserializer; 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.module.SimpleModule; @@ -43,8 +44,14 @@ public class ParameterAuthorizingModule extends SimpleModule { private static final long serialVersionUID = 1L; + private volatile boolean requireAnySetterAnnotations; public ParameterAuthorizingModule() { + this(false); + } + + public ParameterAuthorizingModule(boolean requireAnySetterAnnotations) { + this.requireAnySetterAnnotations = requireAnySetterAnnotations; setDeserializerModifier(new BeanDeserializerModifier() { @Override public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, @@ -58,6 +65,13 @@ public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, } builder.addOrReplaceProperty(new AuthorizingSettableBeanProperty(original), true); } + if (ParameterAuthorizingModule.this.requireAnySetterAnnotations) { + SettableAnyProperty anySetter = builder.getAnySetter(); + if (anySetter != null && !(anySetter instanceof AuthorizingSettableAnyProperty)) { + builder.setAnySetter(null); + builder.setAnySetter(new AuthorizingSettableAnyProperty(anySetter)); + } + } return builder; } @@ -72,4 +86,21 @@ public JsonDeserializer modifyDeserializer(DeserializationConfig config, } }); } + + /** + * Configures any-setter enforcement. Set this before the mapper is first used so Jackson has + * not yet cached deserializers built by this module. + */ + public void setRequireAnySetterAnnotations(boolean requireAnySetterAnnotations) { + this.requireAnySetterAnnotations = requireAnySetterAnnotations; + } + + /** + * Clears request-scoped dynamic-key authorization state after a mapper read. + * + * @since 7.4.0 + */ + public void clearAuthorizationContext() { + DynamicKeyAuthorizationContext.clear(); + } } diff --git a/plugins/rest/src/main/resources/struts-plugin.xml b/plugins/rest/src/main/resources/struts-plugin.xml index cccc62d30d..73ad21146e 100644 --- a/plugins/rest/src/main/resources/struts-plugin.xml +++ b/plugins/rest/src/main/resources/struts-plugin.xml @@ -40,6 +40,7 @@ + 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 7e5407c17e..cfe9b2a610 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 @@ -18,6 +18,7 @@ */ package org.apache.struts2.rest; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.mockobjects.dynamic.AnyConstraintMatcher; import com.mockobjects.dynamic.Mock; import junit.framework.TestCase; @@ -37,6 +38,9 @@ import org.apache.struts2.util.StrutsProxyService; import org.springframework.mock.web.MockHttpServletRequest; +import java.util.LinkedHashMap; +import java.util.Map; + import static org.apache.struts2.ognl.OgnlCacheFactory.CacheType.LRU; /** @@ -59,6 +63,10 @@ protected void setUp() throws Exception { } private void setupInterceptorWithAction(Object actionInstance) { + setupInterceptorWithAction(actionInstance, false); + } + + private void setupInterceptorWithAction(Object actionInstance, boolean requireAnySetterAnnotations) { var ognlUtil = new OgnlUtil( new DefaultOgnlExpressionCacheFactory<>("1000", LRU.toString()), new DefaultOgnlBeanInfoCacheFactory<>("1000", LRU.toString()), @@ -80,10 +88,12 @@ private void setupInterceptorWithAction(Object actionInstance) { mockActionInvocation.expectAndReturn("getAction", actionInstance); mockActionInvocation.expectAndReturn("getAction", actionInstance); mockActionInvocation.expectAndReturn("invoke", Action.SUCCESS); + JacksonJsonHandler handler = new JacksonJsonHandler(); + handler.setAnySetterRequireAnnotations(Boolean.toString(requireAnySetterAnnotations)); mockSelector.expectAndReturn("getHandlerForRequest", new AnyConstraintMatcher() { @Override public boolean matches(Object[] args) { return true; } - }, new JacksonJsonHandler()); + }, handler); interceptor.setContentTypeHandlerSelector((ContentTypeHandlerManager) mockSelector.proxy()); } @@ -167,6 +177,27 @@ public void testRejectedAtParentNeverInstantiatesNestedObject() throws Exception restrictedAction.getUnauthorized()); } + public void testAnySetterEnforcementIsBackwardCompatibleByDefault() throws Exception { + UnannotatedAnySetterAction anySetterAction = new UnannotatedAnySetterAction(); + setupInterceptorWithAction(anySetterAction); + runWithBody("{\"role\":\"admin\"}"); + assertEquals("admin", anySetterAction.getValues().get("role")); + } + + public void testAnySetterEnforcementRejectsUnannotatedSinkWhenEnabled() throws Exception { + UnannotatedAnySetterAction anySetterAction = new UnannotatedAnySetterAction(); + setupInterceptorWithAction(anySetterAction, true); + runWithBody("{\"role\":\"admin\"}"); + assertTrue(anySetterAction.getValues().isEmpty()); + } + + public void testAnySetterEnforcementAcceptsExplicitDynamicKeySink() throws Exception { + AnnotatedAnySetterAction anySetterAction = new AnnotatedAnySetterAction(); + setupInterceptorWithAction(anySetterAction, true); + runWithBody("{\"role\":\"admin\"}"); + assertEquals("admin", anySetterAction.getValues().get("role")); + } + // --- Test fixtures for new path verification --- /** @@ -203,4 +234,31 @@ public void setUnauthorized(SecureRestAction.Address unauthorized) { this.unauthorized = unauthorized; } } + + public static class UnannotatedAnySetterAction extends ActionSupport { + private final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + public void put(String name, Object value) { + values.put(name, value); + } + + public Map getValues() { + return values; + } + } + + public static class AnnotatedAnySetterAction extends ActionSupport { + private final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true) + public void put(String name, Object value) { + values.put(name, value); + } + + public Map getValues() { + return values; + } + } } 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 34171e2524..8c04002e44 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 @@ -18,19 +18,35 @@ */ package org.apache.struts2.rest.handler.jackson; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonUnwrapped; +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.ObjectMapper; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; +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.module.SimpleModule; +import com.fasterxml.jackson.databind.util.TokenBuffer; +import com.fasterxml.jackson.dataformat.xml.XmlMapper; import junit.framework.TestCase; import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; import org.apache.struts2.interceptor.parameter.ParameterAuthorizer; +import org.apache.struts2.interceptor.parameter.StrutsParameter; +import org.apache.struts2.rest.handler.JacksonJsonHandler; import java.beans.ConstructorProperties; +import java.io.StringReader; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; public class ParameterAuthorizingModuleTest extends TestCase { @@ -44,6 +60,7 @@ protected void setUp() { @Override protected void tearDown() { ParameterAuthorizationContext.unbind(); + DynamicKeyAuthorizationContext.clear(); } private void bind(ParameterAuthorizer authorizer, Object instance) { @@ -115,6 +132,182 @@ public void testPathStackCleanAfterDeserialization() throws Exception { ParameterAuthorizationContext.currentPathPrefix()); } + public void testAnySetterEnforcementDisabledByDefault() throws Exception { + bind((path, t, a) -> false, new UnannotatedAnySetterBean()); + UnannotatedAnySetterBean result = mapper.readValue( + "{\"role\":\"admin\"}", UnannotatedAnySetterBean.class); + assertEquals("admin", result.values.get("role")); + } + + public void testUnannotatedAnySetterRejectedWhenEnforcementEnabled() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> true, new UnannotatedAnySetterBean()); + UnannotatedAnySetterBean result = enforcingMapper.readValue( + "{\"role\":\"admin\"}", UnannotatedAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testAnySetterWithoutDynamicKeyOptInRejected() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> true, new AnnotatedAnySetterBean()); + AnnotatedAnySetterBean result = enforcingMapper.readValue( + "{\"role\":\"admin\"}", AnnotatedAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testMethodAnySetterWithDynamicKeyOptInAcceptsScalar() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicScalarAnySetterBean()); + DynamicScalarAnySetterBean result = enforcingMapper.readValue( + "{\"role\":\"admin\"}", DynamicScalarAnySetterBean.class); + assertEquals("admin", result.values.get("role")); + } + + public void testFieldAnySetterWithDynamicKeyOptInAcceptsScalar() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicFieldAnySetterBean()); + DynamicFieldAnySetterBean result = enforcingMapper.readValue( + "{\"role\":\"admin\"}", DynamicFieldAnySetterBean.class); + assertEquals("admin", result.values.get("role")); + } + + public void testDynamicKeyDepthZeroRejectsNestedMembers() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthZeroAnySetterBean()); + DynamicDepthZeroAnySetterBean result = enforcingMapper.readValue( + "{\"home\":{\"city\":\"Warsaw\"}}", DynamicDepthZeroAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testDynamicKeyDepthOneAcceptsDirectMember() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthOneAnySetterBean()); + DynamicDepthOneAnySetterBean result = enforcingMapper.readValue( + "{\"home\":{\"city\":\"Warsaw\"}}", + DynamicDepthOneAnySetterBean.class); + assertEquals("Warsaw", result.values.get("home").city); + } + + public void testDynamicKeyDepthOneRejectsGrandchildBeforeConstruction() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthOneAnySetterBean()); + DynamicDepthOneAnySetterBean result = enforcingMapper.readValue( + "{\"home\":{\"city\":\"Warsaw\",\"geo\":{\"country\":\"PL\"}}}", + DynamicDepthOneAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testDynamicKeyDepthZeroRejectsUntypedObject() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicScalarAnySetterBean()); + DynamicScalarAnySetterBean result = enforcingMapper.readValue( + "{\"settings\":{\"role\":\"admin\"}}", DynamicScalarAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testDynamicKeyDepthTwoAcceptsGrandchild() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthTwoAnySetterBean()); + DynamicDepthTwoAnySetterBean result = enforcingMapper.readValue( + "{\"home\":{\"geo\":{\"country\":\"PL\"}}}", DynamicDepthTwoAnySetterBean.class); + assertEquals("PL", result.values.get("home").geo.country); + } + + public void testCreatorParameterAnySetterRejectedWhenEnforcementEnabled() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> true, new CreatorAnySetterBean(Map.of())); + CreatorAnySetterBean result = enforcingMapper.readValue( + "{\"role\":\"admin\"}", CreatorAnySetterBean.class); + assertTrue(result.values.isEmpty()); + } + + public void testDeserializeWithoutCurrentNameRejectsAndClearsScope() throws Exception { + AtomicReference captured = new AtomicReference<>(); + SimpleModule captureModule = new SimpleModule("capture-any-setter"); + captureModule.setDeserializerModifier(new BeanDeserializerModifier() { + @Override + public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, + BeanDescription beanDesc, + BeanDeserializerBuilder builder) { + if (beanDesc.getBeanClass() == PropertyCreatorWithAnySetterBean.class) { + captured.set(builder.getAnySetter()); + } + return builder; + } + }); + ObjectMapper captureMapper = new ObjectMapper().registerModule(captureModule); + captureMapper.readValue("{\"name\":\"alice\"}", PropertyCreatorWithAnySetterBean.class); + + assertNotNull(captured.get()); + AuthorizingSettableAnyProperty property = new AuthorizingSettableAnyProperty(captured.get()); + bind((path, t, a) -> true, new PropertyCreatorWithAnySetterBean("")); + try (TokenBuffer value = new TokenBuffer(captureMapper, false)) { + value.writeString("admin"); + try (JsonParser parser = value.asParserOnFirstToken()) { + assertNull(parser.currentName()); + assertNotNull(property.deserialize(parser, null)); + } + } + + assertFalse(DynamicKeyAuthorizationContext.isActive()); + assertEquals("", ParameterAuthorizationContext.currentPathPrefix()); + } + + public void testJacksonHandlerClearsDynamicScopeAfterReadFailure() throws Exception { + DynamicKeyAuthorizationContext.push("stale", 0); + assertTrue(DynamicKeyAuthorizationContext.isActive()); + + try { + new JacksonJsonHandler().toObject(null, new StringReader("{"), new Person()); + fail("expected malformed JSON to fail"); + } catch (Exception expected) { + // The handler's request-boundary cleanup must run even when Jackson aborts the read. + } + + assertFalse(DynamicKeyAuthorizationContext.isActive()); + } + + public void testUnauthorizedParentStillBlocksNestedAnySetter() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new AnySetterParent()); + AnySetterParent result = enforcingMapper.readValue( + "{\"child\":{\"role\":\"admin\"}}", AnySetterParent.class); + assertNull(result.child); + } + + public void testJsonUnwrappedRemainsUnaffected() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> true, new UnwrappedBean()); + UnwrappedBean result = enforcingMapper.readValue("{\"city\":\"Warsaw\"}", UnwrappedBean.class); + assertEquals("Warsaw", result.address.city); + } + + public void testDynamicKeyScopeCleanAfterDeserialization() throws Exception { + ObjectMapper enforcingMapper = enforcingMapper(); + bind((path, t, a) -> false, new DynamicDepthTwoAnySetterBean()); + enforcingMapper.readValue( + "{\"home\":{\"geo\":{\"country\":\"PL\"}}}", DynamicDepthTwoAnySetterBean.class); + assertFalse(DynamicKeyAuthorizationContext.isActive()); + assertEquals("", ParameterAuthorizationContext.currentPathPrefix()); + } + + public void testXmlAnySetterUsesSameOptIn() throws Exception { + XmlMapper xmlMapper = new XmlMapper(); + xmlMapper.registerModule(new ParameterAuthorizingModule(true)); + + bind((path, t, a) -> false, new DynamicScalarAnySetterBean()); + DynamicScalarAnySetterBean allowed = xmlMapper.readValue( + "admin", + DynamicScalarAnySetterBean.class); + assertEquals("admin", allowed.values.get("role")); + + bind((path, t, a) -> true, new UnannotatedAnySetterBean()); + UnannotatedAnySetterBean rejected = xmlMapper.readValue( + "admin", + UnannotatedAnySetterBean.class); + assertTrue(rejected.values.isEmpty()); + } + public void testBuilderDeserializationNoContextPassThrough() throws Exception { // No bind → AuthorizingSettableBeanProperty.deserializeSetAndReturn falls through // to the delegate without consulting the authorization context. @@ -280,6 +473,10 @@ public void testValidatingRecord_genuineClientErrorStillPropagates() throws Exce // --- Fixtures --- + private ObjectMapper enforcingMapper() { + return new ObjectMapper().registerModule(new ParameterAuthorizingModule(true)); + } + public static class Person { public String name; public String role; @@ -293,6 +490,110 @@ public static class Person { public static class Address { public String city; public String zip; + public Geo geo; + } + + public static class Geo { + public String country; + } + + public static class UnannotatedAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + public void put(String name, Object value) { + values.put(name, value); + } + } + + public static class AnnotatedAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter + public void put(String name, Object value) { + values.put(name, value); + } + } + + public static class DynamicScalarAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true) + public void put(String name, Object value) { + values.put(name, value); + } + } + + public static class DynamicFieldAnySetterBean { + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true) + public Map values = new LinkedHashMap<>(); + } + + public static class DynamicDepthZeroAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true) + public void put(String name, Address value) { + values.put(name, value); + } + } + + public static class DynamicDepthOneAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true, depth = 1) + public void put(String name, Address value) { + values.put(name, value); + } + } + + public static class DynamicDepthTwoAnySetterBean { + public final Map values = new LinkedHashMap<>(); + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true, depth = 2) + public void put(String name, Address value) { + values.put(name, value); + } + } + + public static class CreatorAnySetterBean { + public final Map values; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public CreatorAnySetterBean(@JsonAnySetter Map values) { + this.values = values; + } + } + + public static class PropertyCreatorWithAnySetterBean { + public final String name; + public final Map values = new LinkedHashMap<>(); + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public PropertyCreatorWithAnySetterBean(@JsonProperty("name") String name) { + this.name = name; + } + + @JsonAnySetter + @StrutsParameter(allowDynamicKeys = true) + public void put(String key, Object value) { + values.put(key, value); + } + } + + public static class AnySetterParent { + public DynamicScalarAnySetterBean child; + } + + public static class UnwrappedBean { + @JsonUnwrapped + public Address address = new Address(); } /**