WW-5626 per-property authorization for Jackson REST handlers (Approach C)#1674
Draft
lukaszlenart wants to merge 13 commits intoWW-5626-cleanupfrom
Draft
WW-5626 per-property authorization for Jackson REST handlers (Approach C)#1674lukaszlenart wants to merge 13 commits intoWW-5626-cleanupfrom
lukaszlenart wants to merge 13 commits intoWW-5626-cleanupfrom
Conversation
Validates that the Approach C design is feasible before committing to a detailed implementation plan. Wraps each SettableBeanProperty via BeanDeserializerModifier; intercepts deserializeAndSet to authorize against a path built from a ThreadLocal Deque; uses skipChildren() to discard unauthorized values; uses [0] suffix for collection/map/array elements to match ParametersInterceptor depth semantics. Findings: - Delegating base class via 'protected delegate' field is the right pattern - addOrReplaceProperty(prop, true) is the correct builder API - Reject-at-parent skips all nested deserialization (better security than two-phase copy: setter side effects on unauthorized properties never fire) - JavaType#isCollectionLikeType/isMapLikeType/isArrayType detects the indexed-path case Spike is kept under .../spike/ as a learning artifact; it will be replaced by production code + tests in subsequent commits.
…orization Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r on Jackson mappers
Implements AuthorizationAwareContentTypeHandler. When ParameterAuthorizationContext is active (set by ContentTypeInterceptor when requireAnnotations=true), the handler walks the parsed result tree and copies only authorized properties to the target, descending into nested beans/collections/maps/arrays with indexed-path semantics ([0] suffix) for parity with ParametersInterceptor. Note: Juneau parses the entire result tree before our walk runs, so setter side effects on transient nested objects can fire even for unauthorized properties — those transient objects are then discarded. This is functionally equivalent to the legacy two-phase copy in ContentTypeInterceptor; only the Jackson handlers achieve the stronger guarantee where unauthorized subtrees are never instantiated at all (they use Jackson's BeanDeserializerModifier + skipChildren). When no context is bound (default config), behavior is unchanged: parser.parse + BeanUtils.copyProperties.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes WW-5626.
Part 2 of 2. Builds on the cleanup PR #1673 (do not merge until that one lands; base will be retargeted to
mainafterwards).What this changes
Replaces the post-hoc reflection-based property filtering in
ContentTypeInterceptorwith per-property authorization owned by the handlers themselves, for all Apache-shipped REST handlers.Architecture
ParameterAuthorizationContext(core) —ThreadLocalholder for per-request(authorizer, target, action, pathStack). The interceptor binds it before invoking authorization-aware handlers and unbinds in afinallyblock.AuthorizationAwareContentTypeHandler(rest) — marker interface. Handlers implementing it signal that theirtoObjecthonorsParameterAuthorizationContextfor per-property@StrutsParameterenforcement.ParameterAuthorizingModule+AuthorizingSettableBeanProperty(rest) — Jackson plumbing:BeanDeserializerModifierwraps everySettableBeanProperty, checksParameterAuthorizationContext.isAuthorized(path)before delegating, callsJsonParser.skipChildren()for unauthorized values, manages the path stack with[0]suffix for collection/map/array-typed properties.ContentTypeInterceptor— whenrequireAnnotations=trueANDhandler instanceof AuthorizationAwareContentTypeHandler: binds context, callshandler.toObject(...)directly. Otherwise: legacy two-phase copy (preserved as-is).Handler coverage
JacksonJsonHandler(defaultjson)BeanDeserializerModifier+skipChildren()— strongest guarantee: unauthorized nested subtrees are never instantiatedJacksonXmlHandler(defaultxml)XmlMapperextendsObjectMapper)JuneauXmlHandler(opt-in)ParameterAuthorizationContext. Setter side effects on transient unauthorized nested objects can fire — they're then discarded (a known limitation due to Juneau's inside-out parse order, see commitfc2f54155)XStreamHandler(opt-in)@Deprecated(since="7.2.0", forRemoval=true)ContentTypeInterceptor. Deprecation cites XStream's CVE history and the per-class allowlist requirement; users pointed toJacksonXmlHandlerWhy this is better than the cleanup PR's two-phase copy
ParameterAuthorizationContextdirectlyisNestedBeanTypepackage-name heuristic for Jackson — Jackson'sJavaTypeintrospection handles type detection correctlyOut of scope (preserved as legacy fallback)
The legacy two-phase copy in
ContentTypeInterceptoris not removed —XStreamHandlerand any custom third-party handlers without the marker interface continue to use it. A future PR migrating those (or removing XStream entirely after the deprecation period) can finally delete the dead code.Spike
A spike that validated the Jackson
BeanDeserializerModifiermechanism is in commit6110b8f6band removed in commit0166c6a0b(replaced by the production tests inParameterAuthorizingModuleTest). A second spike for Juneau'sBeanInterceptorconfirmed why parser-level interception isn't viable for that library (inside-out call order); the post-parse walk approach was chosen instead.Test plan
mvn test -DskipAssembly -pl core—ParameterAuthorizationContextTest12 new tests pass; full core suite zero regressionsmvn test -DskipAssembly -pl plugins/rest—ParameterAuthorizingModuleTest8 new tests pass;ContentTypeInterceptorIntegrationTest7 tests pass (5 from cleanup PR now silently exercise the new path + 2 new);JuneauXmlHandlerIntegrationTest5 new tests passmvn test -DskipAssembly -pl '!plugins/bean-validation,!plugins/tiles'— full multi-module BUILD SUCCESS (the two excluded modules fail identically onmainfor unrelated reasons:AnnotationFormatErrorfrom Hibernate Validator on a test model, and missing Velocity dependencies in the tiles plugin)