[
https://issues.apache.org/jira/browse/WW-5715?focusedWorklogId=1041442&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1041442
]
ASF GitHub Bot logged work on WW-5715:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Sep/26 15:51
Start Date: 14/Sep/26 15:51
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1940:
URL: https://github.com/apache/struts/pull/1940
Fixes [WW-5715](https://issues.apache.org/jira/browse/WW-5715)
## Problem
`AuthorizingSettableBeanProperty` derived the `@StrutsParameter`
authorization path from `getName()` — Jackson's *external* property name —
while `StrutsParameterAuthorizer` resolves that path against the *Java member*.
The two names are identical until the application renames a property
(`@JsonProperty`, `@JsonAlias`, a `PropertyNamingStrategy`); once they diverge,
the authorization decision is made about one member while the value is assigned
to another:
- **Wrongly denied** — `@StrutsParameter setUserName` arriving as
`user_name` is dropped, and the WARN names a property the developer never wrote.
- **Wrongly allowed** — an unannotated `@JsonProperty("name") setAdmin` next
to an annotated `setName` is authorized as `name` and receives the value.
## Change
`ParameterAuthorizingModule` now keys each wrapper by the member Jackson
invokes for the property (`SettableBeanProperty#getMember()`): the field name,
or the bean property a one-argument `set` / no-argument `get`/`is` accessor is
named after — the name `StrutsParameterAuthorizer` resolves back to that
member. The wrapper threads the name through `withDelegate` and into
`AuthorizingValueDeserializer`, so nested path prefixes are built from member
names as well.
Things that deliberately keep the external name:
- creator parameters — they only occur nested (the root is always populated
via `readerForUpdating`), where the authorizer counts depth alone;
- mutators outside the bean convention (`settings(...)`, a one-argument
`getName(...)`), which resolve to no member and fail closed as before.
`BeanPropertyDefinition#getInternalName()` was tried first and rejected: it
names the merged property, not the mutator. Jackson merges an accessor renamed
with `@JsonProperty` into whatever property already owns that external name and
then invokes the explicitly named accessor, so the ticket's literal
`@JsonProperty("name") setAdmin` + `@StrutsParameter setName` case kept the
internal name `name` and still landed in `setAdmin`. That case is now a
regression test.
The one-arg `AuthorizingSettableBeanProperty` constructor stays as a
deprecated shim; its removal is
[WW-5744](https://issues.apache.org/jira/browse/WW-5744) at 8.0.0.
## Tests
- `ContentTypeInterceptorIntegrationTest` (real `StrutsParameterAuthorizer`
+ `JacksonJsonHandler`): renamed annotated member binds; unannotated member
renamed onto an annotated member's name is rejected; unannotated member
*merged* into an annotated member's property is rejected.
- `ParameterAuthorizingModuleTest`: `SNAKE_CASE` naming strategy yields
Java-member paths at every nesting level; non-convention and one-argument
getter-named mutators keep the external name.
`mvn test -DskipAssembly -pl plugins/rest`: 179 tests, 0 failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1041442)
Remaining Estimate: 0h
Time Spent: 10m
> REST body authorization keys off the Jackson external property name instead
> of the Java member
> ----------------------------------------------------------------------------------------------
>
> Key: WW-5715
> URL: https://issues.apache.org/jira/browse/WW-5715
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Minor
> Fix For: 7.4.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Found while triaging WW-5712; independent of it and present on main today.
> {{AuthorizingSettableBeanProperty.deserializeAndSet}} (line 85) and
> {{deserializeSetAndReturn}} (line 105) derive the authorization path from
> {{getName()}}, which is Jackson's _external_ property name — the key as it
> appears in the request body. The authorizer on the other side resolves that
> string against the _Java member_:
> {{StrutsParameterAuthorizer.hasValidAnnotatedMember}} introspects the target
> for a bean property or public field of that name and reads its
> {{@StrutsParameter}}.
> The two names are identical until the application renames a property —
> {{@JsonProperty}}, {{@JsonAlias}}, or a configured
> {{PropertyNamingStrategy}}. Once they diverge, the authorization decision is
> made about one member while the value is assigned to another.
> Two directions, both wrong:
> _Wrongly denied._ With a snake-case naming strategy, an annotated member
> {{userName}} arrives as {{user_name}}. The authorizer looks for a member
> called {{user_name}}, finds none, and rejects — a correctly annotated
> property is silently dropped, and the WARN names a property the developer
> never wrote.
> _Wrongly allowed._ Given an unannotated setter renamed onto an annotated
> member's name:
> @JsonProperty("name")
> public void setAdmin(boolean admin) { ... } // not annotated
> @StrutsParameter
> public void setName(String name) { ... } // annotated
> a request body that sends the key {{name}} with the value {{true}} authorizes
> the path {{name}}, which _is_ annotated, and Jackson then routes the value
> through the external name to {{setAdmin}}. The member that receives the value
> was never the member that was authorized.
> The same derivation feeds {{AuthorizingValueDeserializer}} (line 75) for
> creator properties, so constructor-bound parameters inherit the behaviour.
> Scope: this needs the application to rename properties, and for the
> permissive direction the renaming has to land on an annotated member's name,
> so it is not a default-configuration issue. Filed as a correctness gap in the
> control rather than anything more.
> Fix shape: derive the path from the Java member — {{getMember()}}, the way
> {{AuthorizingSettableAnyProperty}} already does when it reads the annotation
> — rather than the wire name. Note that the path prefix for nested objects is
> currently built from wire names, so member-based and prefix-based naming have
> to be made consistent, not changed in one place only.
> A regression test should cover both directions: an annotated member renamed
> by a naming strategy must still bind, and an unannotated member renamed onto
> an annotated member's name must not.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
