Hi Dmitri, Thanks for checking this. That makes sense, I will change it to make the auth layer forward to PolarisPrincipal properties. Can you help reopen the PR? I don't seem to have access to reopen.
Thanks, Prithvi S On Fri, Jul 10, 2026 at 6:25 PM Dmitri Bourlatchkov <[email protected]> wrote: > Hi Prithvi, Alex, > > Re: SecurityIdentity, it is a Quarkus class and it is certainly fine to use > it in infrastructure code that deals with technical authentication aspects > (e.g. in SecurityIdentityAugmentor). > > However, I'd like to avoid depending on SecurityIdentity in proper Polaris > code like the OPA Authorizer. > > I tend to think that the authentication layer should forward whatever user > information is necessary to PolarisPrincipal as optional attributes. OPA > and other authorizers will then have the opportunity to consider those > attributes. > > This will decouple Polaris authorizers both from Quarkus-specific code and > from PrincipalEntity. So the authorizers should be usable with any IdP > (internal or external) and not be affected by Quarkus upgrades. > > WDYT? > > Thanks, > Dmitri. > > > On Tue, Jul 7, 2026 at 5:15 AM Prithvi S <[email protected]> > wrote: > > > 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 > > > > > >
