Hi JB, Dmitri, Got it, sure. Thanks for trying to reopen #4405. I updated it in https://github.com/apache/polaris/pull/5032.
Regards, Prithvi On Thu, Jul 16, 2026 at 4:42 PM Jean-Baptiste Onofré <[email protected]> wrote: > Yup, I suggest to re-open with a fresh PR. > > Regards > JB > > On Thu, Jul 16, 2026 at 5:10 AM Dmitri Bourlatchkov <[email protected]> > wrote: > > > > Hi Prithvi, > > > > Sorry, but GH refuses to reopen PR 4405 because its branch has been > > force-pushed or something like that. > > > > In the interest of making progress, let's not worry about it. Please open > > fresh PRs for all changes that got stuck in erroneously closed PRs and > > cross-reference them. > > > > Cheers, > > Dmitri. > > > > On Fri, Jul 10, 2026 at 7:25 PM Prithvi S <[email protected]> > > wrote: > > > > > Hi Dmitri, > > > > > > Ah, okay, got it. I closed https://github.com/apache/polaris/pull/5032 > . > > > Can > > > you try to reopen https://github.com/apache/polaris/pull/4405 now? > Thanks! > > > Also this access privilege is confusing. This must be a project > setting? > > > Can we have access to reopen it if it closes due to stale activity? > > > > > > Regards, > > > Prithvi S > > > > > > On Sat, Jul 11, 2026 at 4:30 AM Dmitri Bourlatchkov <[email protected]> > > > wrote: > > > > > > > Hi Prithvi, > > > > > > > > I cannot reopen [4405] either as you apparently have another PR > using the > > > > same branch, i.e. [5032] :) > > > > > > > > Since [5032] is fresh, I suggest closing it and restoring the branch > to > > > the > > > > PolarisPrincipal commits. Then the PR should be possible to re-open, > I > > > > hope. > > > > > > > > [4405] https://github.com/apache/polaris/pull/4405 > > > > > > > > [5032] https://github.com/apache/polaris/pull/5032 > > > > > > > > Cheers, > > > > Dmitri. > > > > > > > > On Fri, Jul 10, 2026 at 6:54 PM Prithvi S < > [email protected]> > > > > wrote: > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
