[
https://issues.apache.org/jira/browse/WW-5746?focusedWorklogId=1041625&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1041625
]
ASF GitHub Bot logged work on WW-5746:
--------------------------------------
Author: ASF GitHub Bot
Created on: 15/Sep/26 11:15
Start Date: 15/Sep/26 11:15
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #1945:
URL: https://github.com/apache/struts/pull/1945#discussion_r4014943677
##########
plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java:
##########
@@ -103,18 +103,32 @@ public JsonDeserializer<?>
modifyDeserializer(DeserializationConfig config,
* Jackson builds the {@code ObjectIdReader} for a property-based {@code
@JsonIdentityInfo} before
* the deserializer modifiers run, capturing the id property as it was
then, and the
* {@code ObjectIdValueProperty} it adds at build time assigns the id
through that captured
- * property rather than through the builder's. Rebuild the reader around a
wrapped one.
+ * property rather than through the builder's. Rebuild the reader around a
wrapped one, and around
+ * a deserializer that puts a bean-typed id's members under the id
property's path.
*/
private static void authorizeObjectIdProperty(BeanDeserializerBuilder
builder) {
ObjectIdReader reader = builder.getObjectIdReader();
- if (reader == null || reader.idProperty == null
- || reader.idProperty instanceof
AuthorizingSettableBeanProperty) {
- return;
+ if (reader != null) {
+ builder.setObjectIdReader(authorizedObjectIdReader(reader));
}
- SettableBeanProperty idProperty = new AuthorizingSettableBeanProperty(
- reader.idProperty, memberNameOf(reader.idProperty));
- builder.setObjectIdReader(ObjectIdReader.construct(reader.getIdType(),
reader.propertyName,
- reader.generator, reader.getDeserializer(), idProperty,
reader.resolver));
+ }
+
+ /**
+ * The same rebuild for a reader Jackson constructs later, in {@code
createContextual}, for a
+ * {@code @JsonIdentityInfo} placed on the referring property; returns the
reader itself when it
+ * carries no id property or is already rebuilt.
+ */
+ static ObjectIdReader authorizedObjectIdReader(ObjectIdReader reader) {
+ if (reader.idProperty == null || reader.getDeserializer() instanceof
ObjectIdPathDeserializer) {
+ return reader;
+ }
+ String memberName = memberNameOf(reader.idProperty);
+ SettableBeanProperty idProperty = reader.idProperty instanceof
AuthorizingSettableBeanProperty
+ ? reader.idProperty
+ : new AuthorizingSettableBeanProperty(reader.idProperty,
memberName);
+ JsonDeserializer<?> idDeserializer = new
ObjectIdPathDeserializer(reader.getDeserializer(), memberName);
Review Comment:
This change touches security-sensitive framework code. Please confirm it is
not a fix for a suspected vulnerability before merging — see `SECURITY.md`.
Vulnerability fixes go through the private process at
`[email protected]`, not a public pull request.
Issue Time Tracking
-------------------
Worklog Id: (was: 1041625)
Time Spent: 0.5h (was: 20m)
> Members of a bean-typed @JsonIdentityInfo id are authorized at the enclosing
> path in the REST plugin
> ----------------------------------------------------------------------------------------------------
>
> Key: WW-5746
> URL: https://issues.apache.org/jira/browse/WW-5746
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Minor
> Fix For: 7.4.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> h2. Summary
> For a type using a property-based {{@JsonIdentityInfo}}, the id value is read
> by {{ObjectIdValueProperty}} through the {{ObjectIdReader}}'s own
> deserializer, which {{BeanDeserializerFactory.addObjectIdReader}} resolves
> with {{findRootValueDeserializer(idType)}}. That deserializer is not the
> property's value deserializer, so it is not wrapped by
> {{AuthorizingValueDeserializer}} and pushes no path prefix. When the id type
> is a bean rather than a scalar, its members are therefore authorized at the
> enclosing bean's level — {{k}} instead of {{id.k}} — and a grant on a
> same-named property of the enclosing bean authorizes the nested write.
> The id property itself is gated at {{id}} since WW-5727 (assignment goes
> through the wrapped property's {{setAndReturn}}), so an id value is never
> assigned without a grant on {{id}}; the gap is the path its members are
> checked under. Surfaced by the WW-5727 reviews; pre-existing and independent
> of that fix.
> h2. Current behaviour
> With an authorizer granting {{id}} and {{k}} on the enclosing bean, a body
> {{{"id": {"k": "x"}}}} on a type whose id is a bean with member {{k}} binds
> {{id.k}} on the strength of the grant for the top-level {{k}}. A scalar id is
> unaffected.
> h2. Proposed change
> Wrapping the reader's deserializer is not a drop-in fix: the same
> deserializer is used by {{ObjectIdReader.readObjectReference}} to read
> _references_ to already-seen objects, where no property is being assigned and
> no prefix applies. The push has to happen only on the
> {{ObjectIdValueProperty}} value path — e.g. rebuild the reader in
> {{ParameterAuthorizingModule.authorizeObjectIdProperty}} with a deserializer
> that pushes {{pathFor(memberName)}} around the delegate for the value read,
> while references keep the bare one; or wrap the {{ObjectIdValueProperty}}
> after build. Add a test with a bean-typed id where the enclosing bean has a
> same-named member, asserting the nested member is rejected under {{id.k}} and
> bound when {{id.k}} is granted.
> h2. Compatibility notes
> No configuration or API change. Only bean-typed ids under a property-based
> generator are affected, an unusual shape; an application relying on the
> enclosing grant for the id's members needs a grant at the nested path instead.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
