making the external authorizers consume the user-defined properties from the entity attribute#5032
Conversation
github-project-automation
Bot
moved this from PRs In Progress
to Done
in Basic Kanban Board
iprithv
changed the title
github-project-automation
Bot
moved this from Done
to PRs In Progress
in Basic Kanban Board
ee0c065 to
d729092
Compare
github-project-automation
Bot
moved this from PRs In Progress
to Done
in Basic Kanban Board
github-project-automation
Bot
moved this from Done
to PRs In Progress
in Basic Kanban Board
github-project-automation
Bot
moved this from PRs In Progress
to Done
in Basic Kanban Board
github-project-automation
Bot
moved this from Done
to PRs In Progress
in Basic Kanban Board
d729092 to
ff0aba7
Compare
iprithv
changed the title
|
@iprithv : Please rebase to get compilation fixes from |
|
|
||
| ### Fixes | ||
| - Fixed native catalog credential vending paths (`loadCredentials` and `loadTable` with delegation) to re-validate locations against the *current* catalog `allowedLocations`. Previously these paths trusted persisted table entity locations, allowing stale credentials after an admin tightened allowed locations on a native catalog. (The federated path had a partial check.) | ||
| - Fixed `PolarisPrincipal` construction from a `PrincipalEntity` to expose the principal's user-defined properties (alongside internal properties) so external authorizers such as OPA and Ranger can use them for policy evaluation. Internal properties win on key collision so that system-managed values such as `client_id` cannot be shadowed by user input. |
There was a problem hiding this comment.
nit: I'd put this under "changes". Old behaviour was not a "bug" 😉
github-project-automation
Bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
ff0aba7 to
a245fbb
Compare
|
#5085 proposes a more fundamental refactoring, which should support this use case too. |
removing approval based on feedback from @adutra
| * collision the internal value wins, so that system-managed properties (for example {@code | ||
| * client_id}) cannot be shadowed by user-supplied properties. | ||
| */ | ||
| private static Map<String, String> mergeEntityProperties(PrincipalEntity principalEntity) { |
There was a problem hiding this comment.
This is the PrincipalEntity -> PolarisPrincipal property merge that the dev list moved away from (PR #4405 thread, Jul 7-11). The agreed direction is for the auth layer to forward user info to PolarisPrincipal as optional attributes, keeping PolarisPrincipal decoupled from PrincipalEntity (federated/detached principals may have no entity). It also only fires on the DefaultAuthenticator path -- a pluggable authenticator with no PrincipalEntity won't forward these. Is this still the intended mechanism, or superseded by the rework you described on the list?
There was a problem hiding this comment.
yeah right, I removed the original mergeEntityProperties approach and followed the direction from the dev-list thread and #5085: PolarisPrincipal stays decoupled from PrincipalEntity, and user info is forwarded via the optional PolarisPrincipal.PRINCIPAL_ENTITY_ATTRIBUTE_KEY.
OPA and Ranger authorizers read user-defined properties from that attribute when available, merging them with internal properties (internal wins on collision) locally in the authorizer. This keeps PolarisPrincipal unchanged and doesn't require a backing entity for all principals. For pluggable authenticators that don't attach the entity attribute, the authorizers just see no extra attributes, which matches the dev-list consensus. Thanks!
a245fbb to
2085d35
Compare
iprithv
changed the title
2085d35 to
590dbfc
Compare
|
Thanks @iprithv for working on it. Posted my concern in this thread, https://lists.apache.org/thread/blvxzb2x1ccn8jqvm9dcn2xw88nn2jmb. |
4 participants
follow-up Dmitri mentioned in #5085: after the
PolarisPrincipalgeneric-attributes refactor, forward user-defined principal properties to OPA and Ranger authorizers so the behavior from the earlier #5032 approach is preserved without merging properties intoPolarisPrincipalitself.PR #4405 originally proposed merging user-defined properties directly into
PolarisPrincipal. The dev-list discussion concluded thatPolarisPrincipalshould stay decoupled fromPrincipalEntity, and PR #5085 introduced generic attributes to expose the entity as an optional attribute. This PR completes that direction by making the external authorizers consume the user-defined properties from the entity attribute.