Hi Alex, Thank you, I understand now that the decoupling in PR #2307 was intentional and that `PolarisPrincipal` should not assume a backing `PrincipalEntity` exists.
I'll **close PR #4405** and work on the `SecurityIdentity` attribute approach instead. My understanding of the approach: 1. In `AuthenticatingAugmentor`, when a `PrincipalEntity` is available, add it as an optional attribute to `QuarkusSecurityIdentity` (e.g., under a key like `"org.apache.polaris.principal_entity"`) 2. Update `OpaPolarisAuthorizer` and `RangerUtils.getUserAttributes` to check for this attribute in the `SecurityIdentity` and extract user-defined properties from the `PrincipalEntity` when present 3. `PolarisPrincipal` remains unchanged, no property merging Also can you help with below: 1. Is the attribute key naming convention above acceptable, or is there an existing pattern I should follow for `SecurityIdentity` attribute keys in Polaris? 2. For `DefaultAuthenticator` when it constructs `PolarisPrincipal` from `PrincipalEntity`, should it also add the `PrincipalEntity` to `SecurityIdentity` attributes at that point, or should this only happen in `AuthenticatingAugmentor`? 3. For the OPA test suite that currently builds principals with user-defined properties should I update those tests to instead set the `PrincipalEntity` attribute on `SecurityIdentity` and verify that `OpaPolarisAuthorizer` picks up the properties from there? 4. Should I add a helper method somewhere (e.g., `PolarisPrincipal.getPrincipalEntityFromIdentity(SecurityIdentity)`) to encapsulate the attribute lookup, or is it better to keep the lookup inline in each consumer? Let me know if I'm on the right track and I'll proceed with the new PR. Thanks, Prithvi On Mon, Jun 15, 2026 at 8:52 PM Alexandre Dutra <[email protected]> wrote: > Hi Prithvi, > > Thanks for starting this thread! > > My stance on this matter remains unchanged: > > - PolarisPrincipal and PrincipalEntity should be decoupled. We must > avoid the assumption that a PrincipalEntity exists for every > principal, especially in cases involving federated or detached > principals. > > - In my view, the most effective method for propagating the > PrincipalEntity is to treat it as an *optional* attribute within the > SecurityIdentity. > > > how should we ensure user-defined properties are reliably forwarded > across all authenticator implementations, not just `DefaultAuthenticator`? > > Such a guarantee is likely unattainable. Because authenticators are > designed to be pluggable, custom implementations may be completely > unaware of PrincipalEntity. Consequently, any logic that requires the > presence of a PrincipalEntity within the SecurityIdentity is > fundamentally flawed and should be refactored to remove that > requirement. > > What is your use case for retrieving the PrincipalEntity's attributes > at runtime? Is it for auditing purposes or are you building some > business logic on top of it? If the former, it should be fine to just > log "null" if the entity is not present (and you can control the > authenticator in use to make sure it will be present); if the latter, > that would mean that your logic is now dependent on a specific > authenticator's ability to produce a PrincipalEntity, which isn't > great. In that case it may be safer to fetch the PrincipalEntity from > the metastore explicitly. > > Thanks, > Alex > > On Sun, Jun 14, 2026 at 8:56 PM Prithvi S <[email protected]> > wrote: > > > > Hi all, > > > > I'm opening this thread as suggested by @dimas-b in the review of PR > #4405 ( > > https://github.com/apache/polaris/pull/4405), which touches > authentication > > and authorization behavior. @flyrain also flagged a prior dev-list thread > > from April 2026 on the same topic, so I want to make sure that discussion > > is continued here. > > > > Background > > `PolarisPrincipal.of(PrincipalEntity, …)` currently forwards only the > > entity's *internal* properties (e.g. `client_id`) and silently drops any > > *user-defined* properties set at principal creation time (e.g. > > `region=northamerica`, `department=finance`). > > > > As a result, downstream consumers of `PolarisPrincipal.getProperties()` > > never see user attributes, and ABAC policies written against them never > > match. This affects: > > > > - `DefaultAuthenticator` : constructs `PolarisPrincipal` during > > authentication > > - `AuthenticatingAugmentor` : copies principal properties into > > `QuarkusSecurityIdentity` attributes > > - External authorizers : `OpaPolarisAuthorizer` and > > `RangerUtils.getUserAttributes` consume these as user attributes for > policy > > evaluation > > > > The existing OPA test suite already builds principals with user-defined > > properties (e.g. `department=finance`), demonstrating the intended > > contract, which the production code path cannot currently honor. > > > > What PR #4405 does > > The fix adds a `mergeEntityProperties()` helper that combines the > > `PrincipalEntity`'s user-defined and internal properties, and uses the > > merged map when constructing `PolarisPrincipal` via the > > `of(PrincipalEntity, …)` overload. > > > > The design question raised: > > @flyrain pointed out that in April 2026, @adutra raised a concern that > > `PolarisPrincipal` was *intentionally* decoupled from `PrincipalEntity` > > (see PR #2307), and suggested an alternative approach: expose the > persisted > > `PrincipalEntity` as a `SecurityIdentity` attribute rather than widening > > `getProperties()`. > > > > PR #4405 takes the opposite direction by merging properties directly into > > `PolarisPrincipal`. I want to understand whether this is acceptable or > > whether the community prefers the `SecurityIdentity` attribute approach > > instead. > > > > questions I have, > > 1. Was the decoupling of `PolarisPrincipal` from `PrincipalEntity` (PR > > #2307) intended to prevent exactly this kind of property merge? If so, > what > > is the preferred mechanism for exposing user-defined principal attributes > > to downstream auth consumers? > > 2. Is the `SecurityIdentity` attribute approach (exposing > `PrincipalEntity` > > directly) the right path, or are there other concerns with that? > > 3. Since authenticators are pluggable, how should we ensure user-defined > > properties are reliably forwarded across all authenticator > implementations, > > not just `DefaultAuthenticator`? > > > > Happy to revise the approach based on community feedback. > > > > Regards, > > Prithvi S >
