WW-5626 per-property authorization for Jackson REST handlers (Approach C)

core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContext.java

@@ -0,0 +1,144 @@
+/*
+ * 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.interceptor.parameter;
+
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.Objects;
+
+/**
+ * ThreadLocal holder for per-request parameter authorization state, used by deserializer-level
+ * authorization (e.g. the REST plugin's {@code ContentTypeInterceptor}). All state — the
+ * {@link ParameterAuthorizer}, the target, the action, and the current property-path stack — is
+ * bound by input-channel interceptors before invoking the deserializer, and unbound in a
+ * {@code finally} block afterwards.
+ *
+ * <p>Implementations that consult this context (e.g. {@code AuthorizingSettableBeanProperty}) call
+ * {@link #isActive()} to decide whether to enforce authorization at all — when no context is bound
+ * (default config, {@code requireAnnotations=false}), they short-circuit to the delegate behavior.</p>
+ *
+ * @since 7.2.0
+ */
+public final class ParameterAuthorizationContext {
+
+    private static final ThreadLocal<State> STATE = new ThreadLocal<>();
+    private static final ThreadLocal<Deque<String>> PATH_STACK = ThreadLocal.withInitial(ArrayDeque::new);
+
+    private ParameterAuthorizationContext() {
+        // utility
+    }
+
+    /**
+     * Binds an authorizer, target, and action to the current thread. {@code target} is the object
+     * being populated — typically the action itself, or the model object for {@code ModelDriven}
+     * actions (the same contract as {@link ParameterAuthorizer#isAuthorized}). {@code action} is
+     * always the action instance. A subsequent call without an intervening {@link #unbind()} replaces
+     * the prior state without resetting the path stack.
+     *
+     * @param authorizer the authorizer to use for this request; must not be {@code null}
+     * @param target     the object being populated (action or model)
+     * @param action     the action instance
+     */
+    public static void bind(ParameterAuthorizer authorizer, Object target, Object action) {
+        Objects.requireNonNull(authorizer, "authorizer");
+        STATE.set(new State(authorizer, target, action));
+    }
+
+    /**
+     * Removes the bound authorizer state and clears the path stack for the current thread.
+     * Safe to call even when no context has been bound.
+     */
+    public static void unbind() {
+        STATE.remove();
+        PATH_STACK.remove();
+    }
+
+    /**
+     * Returns {@code true} if an authorizer has been bound on the current thread via {@link #bind}.
+     */
+    public static boolean isActive() {
+        return STATE.get() != null;
+    }
+
+    /**
+     * Authorizes a parameter at the given path against the bound authorizer. Returns {@code true}
+     * when no context is bound — callers that don't want enforcement at all should not bind context
+     * in the first place; this default keeps wrapping deserializers safe for non-authorized requests.
+     */
+    public static boolean isAuthorized(String parameterPath) {
+        State state = STATE.get();
+        if (state == null) {
+            return true;
+        }
+        return state.authorizer.isAuthorized(parameterPath, state.target, state.action);
+    }
+
+    /**
+     * Pushes the full cumulative path prefix onto the stack. Subsequent {@link #pathFor(String)}
+     * calls will append {@code name} to this prefix. Callers building a collection-element prefix
+     * (e.g. {@code items[0]}) must pass the full string including the suffix.
+     *
+     * @param cumulativePath the full path prefix to push (e.g. {@code "address"} or {@code "items[0]"})
+     */
+    public static void pushPath(String cumulativePath) {
+        PATH_STACK.get().push(cumulativePath);
+    }
+
+    /**
+     * Pops the top path prefix from the stack. Has no effect if the stack is empty.
+     */
+    public static void popPath() {
+        Deque<String> stack = PATH_STACK.get();
+        if (!stack.isEmpty()) {
+            stack.pop();
+        }
+    }
+
+    /**
+     * @return the current top-of-stack path prefix, or empty string if none
+     */
+    public static String currentPathPrefix() {
+        Deque<String> stack = PATH_STACK.get();
+        if (stack.isEmpty()) {
+            return "";
+        }
+        return stack.peek();
+    }
+
+    /**
+     * Builds the full path for a property at the current nesting level: {@code prefix.propertyName}
+     * (or just {@code propertyName} when at the root).
+     */
+    public static String pathFor(String propertyName) {
+        String prefix = currentPathPrefix();
+        return prefix.isEmpty() ? propertyName : prefix + "." + propertyName;
+    }
+
+    private static final class State {
+        final ParameterAuthorizer authorizer;
+        final Object target;
+        final Object action;
+
+        State(ParameterAuthorizer authorizer, Object target, Object action) {
+            this.authorizer = authorizer;
+            this.target = target;
+            this.action = action;
+        }
+    }
+}

core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizationContextTest.java

@@ -0,0 +1,129 @@
+/*
+ * 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.interceptor.parameter;
+
+import org.junit.After;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class ParameterAuthorizationContextTest {
+
+    @After
+    public void tearDown() {
+        ParameterAuthorizationContext.unbind();
+    }
+
+    @Test
+    public void notActive_byDefault() {
+        assertThat(ParameterAuthorizationContext.isActive()).isFalse();
+    }
+
+    @Test
+    public void bind_thenActive() {
+        ParameterAuthorizer authorizer = (n, t, a) -> true;
+        Object action = new Object();
+        ParameterAuthorizationContext.bind(authorizer, action, action);
+        assertThat(ParameterAuthorizationContext.isActive()).isTrue();
+    }
+
+    @Test
+    public void unbind_clearsState() {
+        ParameterAuthorizer authorizer = (n, t, a) -> true;
+        Object action = new Object();
+        ParameterAuthorizationContext.bind(authorizer, action, action);
+        ParameterAuthorizationContext.unbind();
+        assertThat(ParameterAuthorizationContext.isActive()).isFalse();
+    }
+
+    @Test
+    public void isAuthorized_delegatesToBoundAuthorizer() {
+        Object action = new Object();
+        ParameterAuthorizationContext.bind((n, t, a) -> "name".equals(n), action, action);
+        assertThat(ParameterAuthorizationContext.isAuthorized("name")).isTrue();
+        assertThat(ParameterAuthorizationContext.isAuthorized("role")).isFalse();
+    }
+
+    @Test
+    public void isAuthorized_returnsTrue_whenNotActive() {
+        // Defensive default: no context bound = no enforcement
+        assertThat(ParameterAuthorizationContext.isAuthorized("anything")).isTrue();
+    }
+
+    @Test
+    public void pathStack_emptyByDefault() {
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty();
+    }
+
+    @Test
+    public void pushPath_buildsPrefix() {
+        ParameterAuthorizationContext.pushPath("address");
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEqualTo("address");
+        ParameterAuthorizationContext.pushPath("address.city");
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEqualTo("address.city");
+    }
+
+    @Test
+    public void popPath_unwinds() {
+        ParameterAuthorizationContext.pushPath("address");
+        ParameterAuthorizationContext.pushPath("address.city");
+        ParameterAuthorizationContext.popPath();
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEqualTo("address");
+        ParameterAuthorizationContext.popPath();
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty();
+    }
+
+    @Test
+    public void pathFor_concatenatesPropertyName() {
+        assertThat(ParameterAuthorizationContext.pathFor("name")).isEqualTo("name");
+        ParameterAuthorizationContext.pushPath("address");
+        assertThat(ParameterAuthorizationContext.pathFor("city")).isEqualTo("address.city");
+    }
+
+    @Test
+    public void unbind_clearsPathStack() {
+        ParameterAuthorizationContext.bind((n, t, a) -> true, new Object(), new Object());
+        ParameterAuthorizationContext.pushPath("address");
+        ParameterAuthorizationContext.unbind();
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty();
+    }
+
+    @Test
+    public void bind_replacesPriorState_doesNotResetPathStack() {
+        Object firstAction = new Object();
+        Object secondAction = new Object();
+        ParameterAuthorizationContext.bind((n, t, a) -> "first".equals(n), firstAction, firstAction);
+        ParameterAuthorizationContext.pushPath("address");
+        // Rebind with a different authorizer
+        ParameterAuthorizationContext.bind((n, t, a) -> "second".equals(n), secondAction, secondAction);
+        // New authorizer in effect
+        assertThat(ParameterAuthorizationContext.isAuthorized("first")).isFalse();
+        assertThat(ParameterAuthorizationContext.isAuthorized("second")).isTrue();
+        // Path stack is preserved across rebind (it's a separate ThreadLocal)
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEqualTo("address");
+    }
+
+    @Test
+    public void unbind_whenNeverBound_isSafeNoOp() {
+        // Should not throw; isActive should remain false
+        ParameterAuthorizationContext.unbind();
+        assertThat(ParameterAuthorizationContext.isActive()).isFalse();
+        assertThat(ParameterAuthorizationContext.currentPathPrefix()).isEmpty();
+    }
+}

plugins/rest/src/main/java/org/apache/struts2/rest/ContentTypeInterceptor.java

@@ -81,7 +81,7 @@
         this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations);
     }
 
     public String intercept(ActionInvocation invocation) throws Exception {
         HttpServletRequest request = ServletActionContext.getRequest();
         ContentTypeHandler handler = selector.getHandlerForRequest(request);
 
@@ -96,18 +96,33 @@
             InputStreamReader reader = encoding == null ? new InputStreamReader(is) : new InputStreamReader(is, encoding);
 
             if (requireAnnotations) {
-                // Two-phase deserialization: deserialize into a fresh instance, then copy only authorized properties.
-                // Requires a public no-arg constructor on the target class.
-                // If absent, body processing is rejected entirely — a best-effort scrub cannot guarantee
-                // that every nested unauthorized property is nulled out, so the safer choice is to skip.
-                Object freshInstance = createFreshInstance(target.getClass());
-                if (freshInstance != null) {
-                    handler.toObject(invocation, reader, freshInstance);
-                    copyAuthorizedProperties(freshInstance, target, invocation.getAction(), target, "");
+                if (handler instanceof org.apache.struts2.rest.handler.AuthorizationAwareContentTypeHandler) {
+                    // Handler authorizes per-property during deserialization (no two-phase copy needed).
+                    // Bind context so the handler's deserializer can consult ParameterAuthorizationContext.
+                    Object action = invocation.getAction();
+                    Object resolvedTarget = parameterAuthorizer.resolveTarget(action);
+                    org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext.bind(
+                            parameterAuthorizer, resolvedTarget, action);
+                    try {
+                        handler.toObject(invocation, reader, target);
+                    } finally {
+                        org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext.unbind();
+                    }
                 } else {
-                    LOG.warn("REST body rejected: requireAnnotations=true but [{}] has no no-arg constructor; "
-                            + "body deserialization skipped to preserve @StrutsParameter authorization integrity",
-                            target.getClass().getName());
+                    // Legacy two-phase deserialization for handlers that don't authorize themselves.
+                    // Deserialize into a fresh instance, then copy only authorized properties.
+                    // Requires a public no-arg constructor on the target class.
+                    // If absent, body processing is rejected entirely — a best-effort scrub cannot guarantee
+                    // that every nested unauthorized property is nulled out, so the safer choice is to skip.
+                    Object freshInstance = createFreshInstance(target.getClass());
+                    if (freshInstance != null) {
+                        handler.toObject(invocation, reader, freshInstance);
+                        copyAuthorizedProperties(freshInstance, target, invocation.getAction(), target, "");
+                    } else {
+                        LOG.warn("REST body rejected: requireAnnotations=true but [{}] has no no-arg constructor; "
+                                + "body deserialization skipped to preserve @StrutsParameter authorization integrity",
+                                target.getClass().getName());
+                    }
                 }
             } else {
                 // Direct deserialization (backward compat when requireAnnotations is not enabled)

plugins/rest/src/main/java/org/apache/struts2/rest/handler/AuthorizationAwareContentTypeHandler.java

@@ -0,0 +1,46 @@
+/*
+ * 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;
+
+/**
+ * Marker interface for {@link ContentTypeHandler} implementations that respect the
+ * {@code ParameterAuthorizationContext} ThreadLocal during deserialization, enforcing
+ * {@code @StrutsParameter} authorization per-property.
+ *
+ * <p>When {@code struts.parameters.requireAnnotations=true}, the REST plugin's
+ * {@code ContentTypeInterceptor} binds the authorization context before invoking handlers that
+ * implement this interface, allowing them to filter unauthorized properties during deserialization
+ * (rather than after, via reflection-based copying).</p>
+ *
+ * <p>Handlers that do NOT implement this interface fall back to the legacy two-phase copy in
+ * {@code ContentTypeInterceptor} — correct but more expensive (and requires a no-arg constructor
+ * on the target).</p>
+ *
+ * <p><strong>Implementer responsibility:</strong> A handler that declares this interface MUST register
+ * the authorization-aware mechanism on its underlying parser (e.g. for Jackson, register
+ * {@code ParameterAuthorizingModule} on the {@code ObjectMapper}). If the handler implements the
+ * interface but its parser does not honor the context, authorization will silently do nothing —
+ * a serious security bug. The marker interface is the contract; implementations must uphold it.</p>
+ *
+ * @since 7.2.0
+ */
+public interface AuthorizationAwareContentTypeHandler extends ContentTypeHandler {
+    // Marker interface — no methods. Implementations signal that their toObject() method
+    // honors ParameterAuthorizationContext for per-property @StrutsParameter enforcement.
+}

plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonJsonHandler.java

@@ -32,11 +32,12 @@
 /**
  * Handles JSON content using jackson-lib
  */
-public class JacksonJsonHandler implements ContentTypeHandler {
+public class JacksonJsonHandler implements AuthorizationAwareContentTypeHandler {
 
     private static final String DEFAULT_CONTENT_TYPE = "application/json";
     private String defaultEncoding = "ISO-8859-1";
-    private ObjectMapper mapper = new ObjectMapper();
+    private ObjectMapper mapper = new ObjectMapper()
+            .registerModule(new org.apache.struts2.rest.handler.jackson.ParameterAuthorizingModule());
 
     @Override
     public void toObject(ActionInvocation invocation, Reader in, Object target) throws IOException {