You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@polaris.apache.org by Dmitri Bourlatchkov <dm...@dremio.com.INVALID> on 2024/12/18 01:15:25 UTC
Re: [DISCUSS] Polaris Federated Principals and Roles
Hi Mike,
I left some comments in the doc, but overall it looks good to me :)
I still think there are some hidden dependencies on Persistence. For
example, whether and how we can have composite keys for persisted federated
entities... but I guess we can work that out later.
Also, I think it is important for the Authorizer API to avoid assuming that
all principals are persisted. Specific authorizer implementations
(including the default one) can certainly expect persisted principals, but
the API should require that for the sake of flexibility of possible AuthN/Z
extensions. WDYT?
Cheers,
Dmitri.
On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <co...@gmail.com>
wrote:
> Hey folks
>
> As discussed during the community sync, I've put together some thoughts on
> how we'd add support for federated identities in Polaris. I copied over
> some of what I had in the issue at
> https://github.com/apache/polaris/issues/441 and put it into the doc here:
>
>
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> .
>
> Please take a look when you get some time and let me know what you think.
> Given that our next community sync is scheduled for the Thanksgiving
> holiday in the US, it might be useful to schedule a meeting specifically
> for this. I can schedule that sync if needed.
>
> Mike
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Yufei Gu <fl...@gmail.com>.
Thanks Micheal for working on this. The spec looks good to me!
Yufei
On Thu, Apr 17, 2025 at 10:44 AM Eric Maynard <er...@gmail.com>
wrote:
> +1 on the spec change
>
> On Wed, Apr 16, 2025 at 3:44 PM Michael Collado <co...@gmail.com>
> wrote:
>
> > Hey folks
> >
> > Some of you already know that I posted an initial PR to get federated
> > principals/roles added. One thing that came out of the feedback was a
> spec
> > change to make it clear when federated identities can be used in the
> APIs.
> > Notably, federated principals cannot be created or updated, but can be
> > returned in get/list calls, whereas federated roles *can* be created by
> the
> > API. The latter is useful/necessary in order to be able to assign
> > privileges to those roles without relying on the JIT creation on login.
> >
> > Please check out the spec change here and let me know what you think -
> >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> >
> > Mike
> >
> > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > <dm...@dremio.com.invalid> wrote:
> >
> > > Hi Mike,
> > >
> > > I left some comments in the doc, but overall it looks good to me :)
> > >
> > > I still think there are some hidden dependencies on Persistence. For
> > > example, whether and how we can have composite keys for persisted
> > federated
> > > entities... but I guess we can work that out later.
> > >
> > > Also, I think it is important for the Authorizer API to avoid assuming
> > that
> > > all principals are persisted. Specific authorizer implementations
> > > (including the default one) can certainly expect persisted principals,
> > but
> > > the API should require that for the sake of flexibility of possible
> > AuthN/Z
> > > extensions. WDYT?
> > >
> > > Cheers,
> > > Dmitri.
> > >
> > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> collado.mike@gmail.com>
> > > wrote:
> > >
> > > > Hey folks
> > > >
> > > > As discussed during the community sync, I've put together some
> thoughts
> > > on
> > > > how we'd add support for federated identities in Polaris. I copied
> over
> > > > some of what I had in the issue at
> > > > https://github.com/apache/polaris/issues/441 and put it into the doc
> > > here:
> > > >
> > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > .
> > > >
> > > > Please take a look when you get some time and let me know what you
> > think.
> > > > Given that our next community sync is scheduled for the Thanksgiving
> > > > holiday in the US, it might be useful to schedule a meeting
> > specifically
> > > > for this. I can schedule that sync if needed.
> > > >
> > > > Mike
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Eric Maynard <er...@gmail.com>.
+1 on the spec change
On Wed, Apr 16, 2025 at 3:44 PM Michael Collado <co...@gmail.com>
wrote:
> Hey folks
>
> Some of you already know that I posted an initial PR to get federated
> principals/roles added. One thing that came out of the feedback was a spec
> change to make it clear when federated identities can be used in the APIs.
> Notably, federated principals cannot be created or updated, but can be
> returned in get/list calls, whereas federated roles *can* be created by the
> API. The latter is useful/necessary in order to be able to assign
> privileges to those roles without relying on the JIT creation on login.
>
> Please check out the spec change here and let me know what you think -
>
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
>
> Mike
>
> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> <dm...@dremio.com.invalid> wrote:
>
> > Hi Mike,
> >
> > I left some comments in the doc, but overall it looks good to me :)
> >
> > I still think there are some hidden dependencies on Persistence. For
> > example, whether and how we can have composite keys for persisted
> federated
> > entities... but I guess we can work that out later.
> >
> > Also, I think it is important for the Authorizer API to avoid assuming
> that
> > all principals are persisted. Specific authorizer implementations
> > (including the default one) can certainly expect persisted principals,
> but
> > the API should require that for the sake of flexibility of possible
> AuthN/Z
> > extensions. WDYT?
> >
> > Cheers,
> > Dmitri.
> >
> > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <co...@gmail.com>
> > wrote:
> >
> > > Hey folks
> > >
> > > As discussed during the community sync, I've put together some thoughts
> > on
> > > how we'd add support for federated identities in Polaris. I copied over
> > > some of what I had in the issue at
> > > https://github.com/apache/polaris/issues/441 and put it into the doc
> > here:
> > >
> > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > .
> > >
> > > Please take a look when you get some time and let me know what you
> think.
> > > Given that our next community sync is scheduled for the Thanksgiving
> > > holiday in the US, it might be useful to schedule a meeting
> specifically
> > > for this. I can schedule that sync if needed.
> > >
> > > Mike
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
It doesn't really matter. Robert, if I remove the flag, will you remove the
-1 on the PR?
On Wed, May 7, 2025 at 10:43 AM Dmitri Bourlatchkov <di...@apache.org>
wrote:
> On Wed, May 7, 2025 at 11:29 AM Robert Stupp <sn...@snazy.de> wrote:
>
> > If federated principals cannot be created, it doesn't make sense to me
> > to even have that flag.
> >
>
> I think Robert has a point here. Still, from my POV (as I commented in GH
> [1])
> exposing the same property in PrincipalRole and Principal at the same time
> also
> has merit, that is to keep API "symmetric" because those entities are
> related
> as far as Identity Federation is concerned, and we do expect potentially
> importing
> Principals later.
>
> I'm fine with both approaches (property in both entities now or property
> just in role
> now and in Principal when absolutely necessary).
>
> [1]
> https://github.com/apache/polaris/pull/1353#pullrequestreview-2819283769
>
> Cheers,
> Dmitri.
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Dmitri Bourlatchkov <di...@apache.org>.
On Wed, May 7, 2025 at 11:29 AM Robert Stupp <sn...@snazy.de> wrote:
> If federated principals cannot be created, it doesn't make sense to me
> to even have that flag.
>
I think Robert has a point here. Still, from my POV (as I commented in GH
[1])
exposing the same property in PrincipalRole and Principal at the same time
also
has merit, that is to keep API "symmetric" because those entities are
related
as far as Identity Federation is concerned, and we do expect potentially
importing
Principals later.
I'm fine with both approaches (property in both entities now or property
just in role
now and in Principal when absolutely necessary).
[1] https://github.com/apache/polaris/pull/1353#pullrequestreview-2819283769
Cheers,
Dmitri.
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Robert Stupp <sn...@snazy.de>.
If federated principals cannot be created, it doesn't make sense to me
to even have that flag.
On 01.05.25 19:57, Michael Collado wrote:
>> Maybe our approaches are just different. As for me, making backend changes
> first, adding API later is more "natural" :)
>
> I definitely have the opposite approach. I’m very used to defining the
> contract first, before a working backend implementation is complete, as
> that usually allows dependent teams / UI developers / doc writers to begin
> work in parallel while the backend work is being done. There’s no UI work
> here, of course, but it’s a habit to work in this manner.
>
>> This is more of a blocker from my POV because the behaviour of the current
> builder code is hard to predict without reading the implementation, which
> IMHO developers do not have to do on a regular basis as builders have a
> pretty simple API, where a set call should result in the argument becoming
> the new actual value of the attribute.
>
> I’ve addressed this in the PR, but IMHO, the “flexibility” to set this
> field more than once is more likely to hide programming errors than to
> actually allow any real world use cases. Again,
> builder.setFederated(true).setFederated(false) does not feel like a
> realistic use case that we should have to support. More likely, that
> scenario arises when someone copies a long chain of builder.setFoo() calls
> and adds a .setFederated() call at the end without realizing it was already
> called earlier in the chain. Or when a builder is started in one method and
> returned or passed to another method for completion without the caller
> realizing setFederated was already called.
>
> And a builder that doesn’t allow for fields to be set twice isn’t without
> precedence. Even the Immutables library has “strict” mode, which throws
> IllegalStateExceptions if a field is set twice -
> https://immutables.github.io/immutable.html#strict-builder . The whole
> point of strict builders is to catch programming errors early so engineers
> don’t have to wonder “but I set it to federated here, why is the value
> missing from the map…?” during debugging sessions.
>
> All that said, as I stated in the PR comment, I feel this is a minor point.
> The latest revision of the PR includes Alex’s suggestion of removing the
> property from the map if it’s already present.
>
> Mike
>
> On Thu, May 1, 2025 at 8:35 AM Dmitri Bourlatchkov <di...@apache.org> wrote:
>
>> But even in the original code, there is a test asserting that federated
>> principals can't be created.
>>
>>
>> Yeah, I saw that, but this was actually the thing that felt awkward to me
>> in the PR.
>>
>> I'd prefer to avoid making API changes that do not have an execution option
>> in runtime (i.e. not possible to create Federated Principals in the API and
>> not having SCIM either). My rationale is that it is generally easy to add
>> API but hard to remove/change what has been exposed to the public.
>>
>> Maybe our approaches are just different. As for me, making backend changes
>> first, adding API later is more "natural" :)
>>
>> That said, from my POV this specific API change that adds the "federated"
>> property to Principals is rather basic and I'd be ok with making it now
>> even if we do not have SCIM (while I'd prefer not to make it as explained
>> above).
>>
>> https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
>>
>>
>> This is more of a blocker from my POV because the behaviour of the current
>> builder code is hard to predict without reading the implementation, which
>> IMHO developers do not have to do on a regular basis as builders have a
>> pretty simple API, where a set call should result in the argument becoming
>> the new actual value of the attribute.
>>
>> Cheers,
>> Dmitri.
>>
>> On Thu, May 1, 2025 at 12:48 AM Michael Collado <co...@gmail.com>
>> wrote:
>>
>>> Hi Dmitri
>>>
>>> I do have a revision ready to push. As of this morning (PDT), I was still
>>> waiting for a response to this issue (I see you've responded since) -
>>> https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
>>>
>>> But even in the original code, there is a test asserting that federated
>>> principals can't be created.
>>>
>>> I think to have a discussion that moves forward, it's important that
>> people
>>> feel heard. If someone has comments on the code or the proposed designs,
>>> I'm happy to have the discussion, but if neither the code nor the e-mail
>>> discussion are being read... I don't see how this conversation can move
>>> forward.
>>>
>>> Mike
>>>
>>> On Wed, Apr 30, 2025 at 5:58 PM Dmitri Bourlatchkov <di...@apache.org>
>>> wrote:
>>>
>>>> Hi Mike,
>>>>
>>>> I guess some of the confusion may come from PR 1353 [1] and this thread
>>>> being a bit out of sync.
>>>>
>>>> Would you mind removing federated Principals from the API in the PR for
>>> the
>>>> sake of clarity?
>>>>
>>>> From my POV, we can reconsider that if/when SCIM (or something similar)
>>>> comes.
>>>>
>>>> Thanks,
>>>> Dmitri.
>>>>
>>>> [1] https://github.com/apache/polaris/pull/1353
>>>>
>>>> On Wed, Apr 30, 2025 at 11:50 AM Michael Collado <
>> collado.mike@gmail.com
>>>> wrote:
>>>>
>>>>> Hi Robert
>>>>>
>>>>> Unsure if you read the previous emails in this thread. In my earlier
>>>>> response, I wrote
>>>>>
>>>>>> 1. TBH, I have no strong feelings about persisting the federated
>>>>> principals. I think the biggest advantage I saw was to support SCIM
>>> along
>>>>> with 3p identity providers. But for this implementation, if we want
>> to
>>>> skip
>>>>> persisting principal entities, I'm ok with it.
>>>>>
>>>>> I feel that should address the majority of the concern you just
>>>> expressed.
>>>>> Storing grants with references to some kind of FQN is certainly one
>>>>> approach, but IMO, is a massive undertaking with little benefit. For
>>>>> example, all of the grant APIs have to be modified to deal with
>> target
>>>>> entities that don’t exist. How do we do validation for these APIs? Do
>>>> these
>>>>> APIs now need to talk to external entities to ensure the caller
>> didn’t
>>>>> misspell a role name or use all caps instead of mixed case? UI
>>> developers
>>>>> can’t simply populate a list of roles for a dropdown list unless they
>>>> also
>>>>> connect directly to the idp. That makes it hard for someone to build
>> a
>>>>> generic Polaris admin UI without also building support for Okta,
>>> Google,
>>>>> Apple, etc.
>>>>>
>>>>> On the other hand, SCIM is an industry standard for a reason. Every
>>> major
>>>>> IdP supports it - and I doubt it’s because they just don’t know any
>>>> better.
>>>>> Supporting federated roles + SCIM means all the existing grant APIs
>>> Just
>>>>> Work and building generic privilege management tools/UI could be done
>>>>> today.
>>>>>
>>>>> Given the choice between a massive rewrite of the grant and
>>> authorization
>>>>> primitives with no user-facing benefit and supporting federated roles
>>>> with
>>>>> the option of SCIM support in the future, the answer seems clear to
>> me.
>>>>> Mike
>>>>>
>>>>> On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> thanks Mike for the effort.
>>>>>>
>>>>>> However, I think that persisting principals, which are not "owned"
>> by
>>>>>> Polaris, being persisted in Polaris is a good idea. How would
>> changes
>>>> to
>>>>>> a principal in the IdP be propagated to Polaris? What happens if a
>>>>>> principal that's deleted/disabled in the IdP - how is that
>> reflected
>>>> in
>>>>>> Polaris? What would happen if two Polaris instances concurrently
>>>>>> "transparently create" the same "federated principal entity"?
>>>>>>
>>>>>> Storing mirrored information that is only owned by an (external)
>> IdP
>>>>>> leads to a "what's the source-of-truth" problem.
>>>>>>
>>>>>> A principal is (mostly) an individual user or service that can
>> access
>>>>>> Polaris. The actual access-decision is up to the IdP (external IdP
>> or
>>>>>> Polaris w/ its own principals). Each principal can also be uniquely
>>>>>> identified (IDP + principal-ID).
>>>>>>
>>>>>> I really do not think that principals owned by "external IdPs"
>> should
>>>>>> ever be persisted in Polaris.
>>>>>>
>>>>>> Similar for roles manages by external IdPs.
>>>>>>
>>>>>> ACLs (grants) are (generally speaking) lists of
>> grantee+privilege(s)
>>>>>> tuples. It is IMHO better to use a qualified reference to "IDP +
>>>>>> principal/role-ID".
>>>>>>
>>>>>> AFAIR it's only possbible to grant privileges to roles, not
>>> individual
>>>>>> principals. And if that's the case, I do not see a requirement to
>>> have
>>>>>> "federated principals" being persisted in Polaris at all.
>>>>>>
>>>>>> I seems the underlying issue here is the very tight coupling of
>>>>>> principals + principal-roles + catalog-roles + grants that
>> requires a
>>>>>> unique integer-ID for each of those and using the same
>>>> database-oriented
>>>>>> approach for role-membership management and catalog-entity grants.
>>>>>>
>>>>>> What's IMHO needed is a proper separation of those things, which
>>> should
>>>>>> happen now and after that's done integrate external IdPs.
>>>>>>
>>>>>>
>>>>>> On 22.04.25 17:00, Alex Dutra wrote:
>>>>>>> Hi Mike,
>>>>>>>
>>>>>>> Thanks for the historical context, that is really helpful. And I
>>>>>> completely
>>>>>>> agree with your last paragraph, and especially this:
>>>>>>>
>>>>>>> My preference would be to make this an immutable class that has
>> the
>>>>>>>> principal entity and all active principal roles all in the
>>>>> constructor.
>>>>>>> I was arriving at the exact same conclusion – it's a pity that
>>>>>>> SecurityContext doesn't expose the principal roles. I guess, the
>>>>> problem
>>>>>>> now is how to move forward with this suggestion – I imagine a
>>> change
>>>>>>> in AuthenticatedPolarisPrincipal or in the Resolver would create
>>>>>>> repercussions in a lot of places in the code base as well as in
>>>>> existing
>>>>>>> PRs.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Alex
>>>>>>>
>>>>>>> On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <
>>>>> collado.mike@gmail.com
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hey
>>>>>>>>
>>>>>>>> 1. I'm unsure why the Resolver was written to assume no role
>> means
>>>> all
>>>>>>>> roles. But I think it has to do with
>>>>>>>> 2. The Resolver was intended to be able to resolve principal
>>> roles,
>>>>>> grants,
>>>>>>>> and target entities all at one "snapshot" based on the entity
>> and
>>>>> grant
>>>>>>>> versions returned by that loadEntitiesChangeTracking method. In
>>>> order
>>>>>> to do
>>>>>>>> this in one shot, the authenticator couldn't resolve the
>> principal
>>>>> roles
>>>>>>>> actually granted to the principal, so it didn't resolve the
>>>>>>>> PRINCIPAL_ROLE:ALL scope in the token. But that meant the
>> Resolver
>>>>>> needed
>>>>>>>> to verify the grants existed for the roles to be assumed by the
>>>>>> principal
>>>>>>>> (otherwise, a caller could just put whatever role they want in
>> the
>>>>>> scope).
>>>>>>>> To avoid the lookup during authn, ALL needed to be expanded by
>> the
>>>>>> Resolver
>>>>>>>> based on the grants. This was an impedance to the federated role
>>>>>> proposal,
>>>>>>>> which stated that the authn token is authoritative regarding
>> which
>>>>> roles
>>>>>>>> are granted to the principal (e.g., using a groups claim
>>>>>>>> <
>>>>>>>>
>> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
>>>>>>>>> ).
>>>>>>>> That's why
>>>>>>>> 3. I added the ActiveRolesProvider interface so that the default
>>>> impl
>>>>>> would
>>>>>>>> continue to validate the grants held by the principal for
>>> activating
>>>>>>>> principal roles but the federated alternative would delegate to
>>> the
>>>>>> token
>>>>>>>> claims (yet to be committed).
>>>>>>>>
>>>>>>>> Hopefully, that history helps. In short, I agree that
>>>>>>>> SecurityIdentity.getRoles() should be the right way to determine
>>>>>> activated
>>>>>>>> principal roles. The Resolver should rely on the authenticator
>> to
>>>>>> determine
>>>>>>>> the active set of principal roles and not rely on grant records.
>>> The
>>>>>>>> AuthenticatedPolarisPrincipal doesn't really need to hold role
>>> names
>>>>>>>> anymore, but the resolver does still need the list of active
>>>>>> PrincipalRoles
>>>>>>>> to load the appropriate catalog roles. My preference would be to
>>>> make
>>>>>> this
>>>>>>>> an immutable class that has the principal entity and all active
>>>>>> principal
>>>>>>>> roles all in the constructor.
>>>>>>>>
>>>>>>>> Mike
>>>>>>>>
>>>>>>>> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
>>>>>> <al...@dremio.com.invalid>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi Mike,
>>>>>>>>>
>>>>>>>>> I am generally fine with the spec changes.
>>>>>>>>>
>>>>>>>>> I have however some concerns around the way we handle principal
>>>> roles
>>>>>>>> today
>>>>>>>>> in Polaris, and as I was preparing the PR with support for
>>> external
>>>>>> IDPs
>>>>>>>>> [1], some oddities stood out:
>>>>>>>>>
>>>>>>>>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention,
>>> to
>>>> be
>>>>>>>>> converted to an empty set of roles by the Authenticator;
>> but
>>> I
>>>>>> wonder
>>>>>>>>> how we will be able to distinguish a token that contained
>> no
>>>> role
>>>>>>>> from a
>>>>>>>>> token that contained the pseudo-role. I added some changes
>> to
>>>> my
>>>>>> PR to
>>>>>>>>> be
>>>>>>>>> able to distinguish the two situations, but I think the
>>> general
>>>>>>>>> handling of
>>>>>>>>> this pseudo-role, as well as role mapping in general,
>>> deserves
>>>>> some
>>>>>>>> more
>>>>>>>>> thinking. The spec says this pseudo-role is
>> "non-functional"
>>> –
>>>>> but
>>>>>> I
>>>>>>>>> don't
>>>>>>>>> know what this implies exactly.
>>>>>>>>> 2. AuthenticatedPolarisPrincipal exposes principal roles
>> and
>>>> this
>>>>>> is
>>>>>>>>> imho concerning for two reasons: first, the Resolver seems
>> to
>>>>>> update
>>>>>>>> the
>>>>>>>>> principal roles during resolution, which makes the class
>>>> mutable,
>>>>>>>>> potentially changing the principal characteristics *after*
>>>>>>>>> authentication.
>>>>>>>>> And secondly, this creates an ambiguity as roles can be
>>>> accessed
>>>>> in
>>>>>>>> two
>>>>>>>>> ways: SecurityIdentity.getRoles() vs
>>>>>>>>>
>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
>>>>>> would be
>>>>>>>>> in favor of using the former consistently and completely
>>>> removing
>>>>>> the
>>>>>>>>> latter. (Note: SecurityContext.isUserInRole() relies on the
>>>>>> former.)
>>>>>>>>> 3. There seems to be some overlap between the primary roles
>>>>>>>> resolution,
>>>>>>>>> done by the ActiveRolesProvider, and the role resolution
>> done
>>>>>> later by
>>>>>>>>> the Resolver, cf. method
>>>>>> resolveCallerPrincipalAndPrincipalRoles(). I
>>>>>>>>> wonder if this second role resolution is really necessary?
>>>>>> Ultimately,
>>>>>>>>> the
>>>>>>>>> source of truth for role membership will always be
>>>>>>>>> SecurityIdentity.hasRole() or its JAX-RS variant,
>>>>>>>>> SecurityContext.isUserInRole(). We should not deviate from
>>>> that.
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Alex
>>>>>>>>>
>>>>>>>>> [1] https://github.com/apache/polaris/pull/1397
>>>>>>>>>
>>>>>>>>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <
>>>>> dimas@apache.org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Re 2: Thanks for the clarification, Mike! I guess my brain
>>> swapped
>>>>> out
>>>>>>>> a
>>>>>>>>>> large portion of that doc :)
>>>>>>>>>>
>>>>>>>>>> I'm still not sure how IdentityToPrincipalMapping can help
>> with
>>>>>>>> resolving
>>>>>>>>>> changes from API and from IdP integration.
>>>>>>>>>>
>>>>>>>>>> The doc talks about namespaces, but in PR# 1353, it looks like
>>> API
>>>>>>>> calls
>>>>>>>>>> are free to change anything in federated principal roles.
>>>>>>>>>>
>>>>>>>>>> Should IdentityToPrincipalMapping extend to validating changes
>>>>> coming
>>>>>>>>> from
>>>>>>>>>> the Admin API?
>>>>>>>>>>
>>>>>>>>>> Alternatively, we could treat that as a new permission and
>> allow
>>>> all
>>>>>>>>> admin
>>>>>>>>>> users to edit federated roles via API.
>>>>>>>>>>
>>>>>>>>>> Re 1: Yes, I guess it would be helpful with SCIM, but then
>> we'd
>>>> need
>>>>>> to
>>>>>>>>>> sync role membership, I guess.
>>>>>>>>>>
>>>>>>>>>> My understanding so far was that we'd get role membership at
>>>>>>>>> authentication
>>>>>>>>>> time and not persist it in Polaris.
>>>>>>>>>>
>>>>>>>>>> Is that still the plan?
>>>>>>>>>>
>>>>>>>>>> I think this relates to how we connect authentication and
>>>>>> authorization
>>>>>>>>> in
>>>>>>>>>> Polaris. If Principals are external, we get role membership
>> from
>>>> the
>>>>>>>>> access
>>>>>>>>>> token and all is clear. If Principals are local, and we still
>>> get
>>>>>> roles
>>>>>>>>>> from the access token, then the question arises about whether
>>> that
>>>>>> data
>>>>>>>>> is
>>>>>>>>>> in sync with local data (and the other way around too).
>>>>>>>>>>
>>>>>>>>>> That could be resolved by Polaris owning access tokens (via
>>> token
>>>>>>>>> exchange)
>>>>>>>>>> and assuming control over role membership based on local data
>>> and
>>>>>>>> access
>>>>>>>>>> time (I commented on this in the doc too).
>>>>>>>>>>
>>>>>>>>>> All in all, since SCIM is not part of phase 1 here, maybe we
>>> could
>>>>>>>> defer
>>>>>>>>>> dealing with federated Principal persistence, indeed.
>>>>>>>>>>
>>>>>>>>>> Re 3: I guess it also applies to client secret generation
>> code,
>>>> but
>>>>>>>> let's
>>>>>>>>>> see whether it actually causes too many conditions in code
>> when
>>> it
>>>>>>>> comes
>>>>>>>>> to
>>>>>>>>>> that.
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> Dmitri.
>>>>>>>>>>
>>>>>>>>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
>>>>>>>> collado.mike@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Thanks for the response.
>>>>>>>>>>>
>>>>>>>>>>> 1. TBH, I have no strong feelings about persisting the
>>> federated
>>>>>>>>>>> principals. I think the biggest advantage I saw was to
>> support
>>>> SCIM
>>>>>>>>> along
>>>>>>>>>>> with 3p identity providers. But for this implementation, if
>> we
>>>> want
>>>>>>>> to
>>>>>>>>>> skip
>>>>>>>>>>> persisting principal entities, I'm ok with it.
>>>>>>>>>>>
>>>>>>>>>>> 2. In the doc I wrote the following, which I think addresses
>>> the
>>>>>>>>> concern
>>>>>>>>>>> around conflicts.
>>>>>>>>>>>
>>>>>>>>>>> 1.
>>>>>>>>>>>
>>>>>>>>>>> The IdentityToPrincipalMapping should supply a naming
>>>>> convention
>>>>>>>> so
>>>>>>>>>> that
>>>>>>>>>>> PrincipalRoles and Principals created from that source do
>>> not
>>>>>>>>> conflict
>>>>>>>>>>> with
>>>>>>>>>>> entities created by a different IdentityProvider
>>>>>>>>>>>
>>>>>>>>>>> In the case that a role happens to exist even with the naming
>>>>>>>>> convention
>>>>>>>>>>> applied, the caller would be unable to authenticate with that
>>>> role.
>>>>>>>>>>> 3. I don't think we need special classes for federated roles.
>>> In
>>>>>>>> nearly
>>>>>>>>>> all
>>>>>>>>>>> of the Polaris code, they are treated as PolarisRoleEntity
>>>> without
>>>>>>>> any
>>>>>>>>>>> issue. The only code that needs to be aware of federated
>> roles
>>> is
>>>>>>>>>>> a) The Admin API, which prevents granting these roles to
>>>> principals
>>>>>>>> and
>>>>>>>>>>> b) The federated roles provider, which verifies that the
>>>> federated
>>>>>>>>>>> principal is assuming federated roles.
>>>>>>>>>>>
>>>>>>>>>>> The rest of the code (i.e., the resolver and authorizer)
>> treat
>>>>> these
>>>>>>>>>> roles
>>>>>>>>>>> exactly as native Polaris roles.
>>>>>>>>>>>
>>>>>>>>>>> Mike
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
>>>>>>>> dimas@apache.org>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Thanks for reviving this discussion, Mike!
>>>>>>>>>>>>
>>>>>>>>>>>> The API spec change by itself LGTM, but I have related
>>> concerns
>>>> in
>>>>>>>>> how
>>>>>>>>>>> this
>>>>>>>>>>>> feature is meant to work in general.
>>>>>>>>>>>>
>>>>>>>>>>>> 1) The need to expose Federated Principals in Polaris API.
>>>>>>>>>>>>
>>>>>>>>>>>> The design doc [1] discusses the possibility to expose
>>> Federated
>>>>>>>>>>> Principals
>>>>>>>>>>>> in Polaris API, but there are pros and cons. I do not think
>> it
>>>> is
>>>>> a
>>>>>>>>>>> trivial
>>>>>>>>>>>> decision. I'd like to discuss this in more detail before we
>>>> commit
>>>>>>>> to
>>>>>>>>>>> doing
>>>>>>>>>>>> that.
>>>>>>>>>>>>
>>>>>>>>>>>> From my POV the main benefit of exposing Federated
>> Principals
>>>> is
>>>>> to
>>>>>>>>> be
>>>>>>>>>>> able
>>>>>>>>>>>> to alter their properties in Polaris (as mentioned in the
>>> doc).
>>>>>>>>>> However,
>>>>>>>>>>>> your email indicates that this is actually forbidden.
>>>>>>>>>>>>
>>>>>>>>>>>> What use cases do you envision for exposing read-only
>>> Federated
>>>>>>>>>>> Principals?
>>>>>>>>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
>>>>>>>>>>>>
>>>>>>>>>>>> Let's focus on Principal Roles here (which are meant to be
>>>>> writable
>>>>>>>>> via
>>>>>>>>>>>> API).
>>>>>>>>>>>>
>>>>>>>>>>>> If users are able to make and change (I assume) Principal
>>> Roles
>>>>> via
>>>>>>>>>> API,
>>>>>>>>>>>> and federation code will also be creating and changing those
>>>>> roles,
>>>>>>>>>> what
>>>>>>>>>>> is
>>>>>>>>>>>> our approach to handling conflicts between those two streams
>>> of
>>>>>>>>>> changes?
>>>>>>>>>>>> Some examples for context (also mentioned in GH):
>>>>>>>>>>>>
>>>>>>>>>>>> A) A user creates normal Principal Role "A", and later role
>>> "A"
>>>> is
>>>>>>>>>>>> discovered through Federation.
>>>>>>>>>>>>
>>>>>>>>>>>> B) A user edits Federated Role "B" and later properties of
>> "B"
>>>> are
>>>>>>>>>>> updated
>>>>>>>>>>>> through Federation (or the other way around).
>>>>>>>>>>>>
>>>>>>>>>>>> Do we want to control property edits based on the origin of
>>> the
>>>>>>>>>> property
>>>>>>>>>>>> (spec out a "namespace" for Federated properties)?
>>>>>>>>>>>>
>>>>>>>>>>>> 3) OO types
>>>>>>>>>>>>
>>>>>>>>>>>> Currently Principals and Principal Roles are represented by
>>> one
>>>>>>>> java
>>>>>>>>>> type
>>>>>>>>>>>> each. When federation comes into play, would it make sense
>> to
>>>>>>>>> develop a
>>>>>>>>>>>> (java) type hierarchy for working with that data in Polaris
>>>> code?
>>>>>>>> My
>>>>>>>>>> main
>>>>>>>>>>>> concern here is avoiding a maze of if/else statements
>>> throughout
>>>>>>>> the
>>>>>>>>>>>> Polaris codebase for supporting Federation.
>>>>>>>>>>>>
>>>>>>>>>>>> I guess our approach to this depends largely on the outcome
>> of
>>>>>>>> topic
>>>>>>>>> 2
>>>>>>>>>>>> above.
>>>>>>>>>>>>
>>>>>>>>>>>> WDYT?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Dmitri.
>>>>>>>>>>>>
>>>>>>>>>>>> [1]
>>>>>>>>>>>>
>>>>>>>>>>>>
>> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
>>>>>>>>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
>>>>>>>>>> collado.mike@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hey folks
>>>>>>>>>>>>>
>>>>>>>>>>>>> Some of you already know that I posted an initial PR to get
>>>>>>>>> federated
>>>>>>>>>>>>> principals/roles added. One thing that came out of the
>>> feedback
>>>>>>>>> was a
>>>>>>>>>>>> spec
>>>>>>>>>>>>> change to make it clear when federated identities can be
>> used
>>>> in
>>>>>>>>> the
>>>>>>>>>>>> APIs.
>>>>>>>>>>>>> Notably, federated principals cannot be created or updated,
>>> but
>>>>>>>> can
>>>>>>>>>> be
>>>>>>>>>>>>> returned in get/list calls, whereas federated roles *can*
>> be
>>>>>>>>> created
>>>>>>>>>> by
>>>>>>>>>>>> the
>>>>>>>>>>>>> API. The latter is useful/necessary in order to be able to
>>>> assign
>>>>>>>>>>>>> privileges to those roles without relying on the JIT
>> creation
>>>> on
>>>>>>>>>> login.
>>>>>>>>>>>>> Please check out the spec change here and let me know what
>>> you
>>>>>>>>> think
>>>>>>>>>> -
>> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
>>>>>>>>>>>>> Mike
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
>>>>>>>>>>>>> <dm...@dremio.com.invalid> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Mike,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I left some comments in the doc, but overall it looks good
>>> to
>>>>>>>> me
>>>>>>>>> :)
>>>>>>>>>>>>>> I still think there are some hidden dependencies on
>>>>>>>> Persistence.
>>>>>>>>>> For
>>>>>>>>>>>>>> example, whether and how we can have composite keys for
>>>>>>>> persisted
>>>>>>>>>>>>> federated
>>>>>>>>>>>>>> entities... but I guess we can work that out later.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Also, I think it is important for the Authorizer API to
>>> avoid
>>>>>>>>>>> assuming
>>>>>>>>>>>>> that
>>>>>>>>>>>>>> all principals are persisted. Specific authorizer
>>>>>>>> implementations
>>>>>>>>>>>>>> (including the default one) can certainly expect persisted
>>>>>>>>>>> principals,
>>>>>>>>>>>>> but
>>>>>>>>>>>>>> the API should require that for the sake of flexibility of
>>>>>>>>> possible
>>>>>>>>>>>>> AuthN/Z
>>>>>>>>>>>>>> extensions. WDYT?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>> Dmitri.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
>>>>>>>>>>>> collado.mike@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hey folks
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> As discussed during the community sync, I've put together
>>>>>>>> some
>>>>>>>>>>>> thoughts
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>> how we'd add support for federated identities in
>> Polaris. I
>>>>>>>>>> copied
>>>>>>>>>>>> over
>>>>>>>>>>>>>>> some of what I had in the issue at
>>>>>>>>>>>>>>> https://github.com/apache/polaris/issues/441 and put it
>>> into
>>>>>>>>> the
>>>>>>>>>>> doc
>>>>>>>>>>>>>> here:
>>>>>>>>>>>>>>>
>> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
>>>>>>>>>>>>>>> .
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Please take a look when you get some time and let me know
>>>>>>>> what
>>>>>>>>>> you
>>>>>>>>>>>>> think.
>>>>>>>>>>>>>>> Given that our next community sync is scheduled for the
>>>>>>>>>>> Thanksgiving
>>>>>>>>>>>>>>> holiday in the US, it might be useful to schedule a
>> meeting
>>>>>>>>>>>>> specifically
>>>>>>>>>>>>>>> for this. I can schedule that sync if needed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Mike
>>>>>>>>>>>>>>>
>>>>>> --
>>>>>> Robert Stupp
>>>>>> @snazy
>>>>>>
>>>>>>
--
Robert Stupp
@snazy
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
> Maybe our approaches are just different. As for me, making backend changes
first, adding API later is more "natural" :)
I definitely have the opposite approach. I’m very used to defining the
contract first, before a working backend implementation is complete, as
that usually allows dependent teams / UI developers / doc writers to begin
work in parallel while the backend work is being done. There’s no UI work
here, of course, but it’s a habit to work in this manner.
> This is more of a blocker from my POV because the behaviour of the current
builder code is hard to predict without reading the implementation, which
IMHO developers do not have to do on a regular basis as builders have a
pretty simple API, where a set call should result in the argument becoming
the new actual value of the attribute.
I’ve addressed this in the PR, but IMHO, the “flexibility” to set this
field more than once is more likely to hide programming errors than to
actually allow any real world use cases. Again,
builder.setFederated(true).setFederated(false) does not feel like a
realistic use case that we should have to support. More likely, that
scenario arises when someone copies a long chain of builder.setFoo() calls
and adds a .setFederated() call at the end without realizing it was already
called earlier in the chain. Or when a builder is started in one method and
returned or passed to another method for completion without the caller
realizing setFederated was already called.
And a builder that doesn’t allow for fields to be set twice isn’t without
precedence. Even the Immutables library has “strict” mode, which throws
IllegalStateExceptions if a field is set twice -
https://immutables.github.io/immutable.html#strict-builder . The whole
point of strict builders is to catch programming errors early so engineers
don’t have to wonder “but I set it to federated here, why is the value
missing from the map…?” during debugging sessions.
All that said, as I stated in the PR comment, I feel this is a minor point.
The latest revision of the PR includes Alex’s suggestion of removing the
property from the map if it’s already present.
Mike
On Thu, May 1, 2025 at 8:35 AM Dmitri Bourlatchkov <di...@apache.org> wrote:
> But even in the original code, there is a test asserting that federated
> principals can't be created.
>
>
> Yeah, I saw that, but this was actually the thing that felt awkward to me
> in the PR.
>
> I'd prefer to avoid making API changes that do not have an execution option
> in runtime (i.e. not possible to create Federated Principals in the API and
> not having SCIM either). My rationale is that it is generally easy to add
> API but hard to remove/change what has been exposed to the public.
>
> Maybe our approaches are just different. As for me, making backend changes
> first, adding API later is more "natural" :)
>
> That said, from my POV this specific API change that adds the "federated"
> property to Principals is rather basic and I'd be ok with making it now
> even if we do not have SCIM (while I'd prefer not to make it as explained
> above).
>
> https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
>
>
> This is more of a blocker from my POV because the behaviour of the current
> builder code is hard to predict without reading the implementation, which
> IMHO developers do not have to do on a regular basis as builders have a
> pretty simple API, where a set call should result in the argument becoming
> the new actual value of the attribute.
>
> Cheers,
> Dmitri.
>
> On Thu, May 1, 2025 at 12:48 AM Michael Collado <co...@gmail.com>
> wrote:
>
> > Hi Dmitri
> >
> > I do have a revision ready to push. As of this morning (PDT), I was still
> > waiting for a response to this issue (I see you've responded since) -
> > https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
> >
> > But even in the original code, there is a test asserting that federated
> > principals can't be created.
> >
> > I think to have a discussion that moves forward, it's important that
> people
> > feel heard. If someone has comments on the code or the proposed designs,
> > I'm happy to have the discussion, but if neither the code nor the e-mail
> > discussion are being read... I don't see how this conversation can move
> > forward.
> >
> > Mike
> >
> > On Wed, Apr 30, 2025 at 5:58 PM Dmitri Bourlatchkov <di...@apache.org>
> > wrote:
> >
> > > Hi Mike,
> > >
> > > I guess some of the confusion may come from PR 1353 [1] and this thread
> > > being a bit out of sync.
> > >
> > > Would you mind removing federated Principals from the API in the PR for
> > the
> > > sake of clarity?
> > >
> > > From my POV, we can reconsider that if/when SCIM (or something similar)
> > > comes.
> > >
> > > Thanks,
> > > Dmitri.
> > >
> > > [1] https://github.com/apache/polaris/pull/1353
> > >
> > > On Wed, Apr 30, 2025 at 11:50 AM Michael Collado <
> collado.mike@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi Robert
> > > >
> > > > Unsure if you read the previous emails in this thread. In my earlier
> > > > response, I wrote
> > > >
> > > > > 1. TBH, I have no strong feelings about persisting the federated
> > > > principals. I think the biggest advantage I saw was to support SCIM
> > along
> > > > with 3p identity providers. But for this implementation, if we want
> to
> > > skip
> > > > persisting principal entities, I'm ok with it.
> > > >
> > > > I feel that should address the majority of the concern you just
> > > expressed.
> > > >
> > > > Storing grants with references to some kind of FQN is certainly one
> > > > approach, but IMO, is a massive undertaking with little benefit. For
> > > > example, all of the grant APIs have to be modified to deal with
> target
> > > > entities that don’t exist. How do we do validation for these APIs? Do
> > > these
> > > > APIs now need to talk to external entities to ensure the caller
> didn’t
> > > > misspell a role name or use all caps instead of mixed case? UI
> > developers
> > > > can’t simply populate a list of roles for a dropdown list unless they
> > > also
> > > > connect directly to the idp. That makes it hard for someone to build
> a
> > > > generic Polaris admin UI without also building support for Okta,
> > Google,
> > > > Apple, etc.
> > > >
> > > > On the other hand, SCIM is an industry standard for a reason. Every
> > major
> > > > IdP supports it - and I doubt it’s because they just don’t know any
> > > better.
> > > > Supporting federated roles + SCIM means all the existing grant APIs
> > Just
> > > > Work and building generic privilege management tools/UI could be done
> > > > today.
> > > >
> > > > Given the choice between a massive rewrite of the grant and
> > authorization
> > > > primitives with no user-facing benefit and supporting federated roles
> > > with
> > > > the option of SCIM support in the future, the answer seems clear to
> me.
> > > >
> > > > Mike
> > > >
> > > > On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > thanks Mike for the effort.
> > > > >
> > > > > However, I think that persisting principals, which are not "owned"
> by
> > > > > Polaris, being persisted in Polaris is a good idea. How would
> changes
> > > to
> > > > > a principal in the IdP be propagated to Polaris? What happens if a
> > > > > principal that's deleted/disabled in the IdP - how is that
> reflected
> > > in
> > > > > Polaris? What would happen if two Polaris instances concurrently
> > > > > "transparently create" the same "federated principal entity"?
> > > > >
> > > > > Storing mirrored information that is only owned by an (external)
> IdP
> > > > > leads to a "what's the source-of-truth" problem.
> > > > >
> > > > > A principal is (mostly) an individual user or service that can
> access
> > > > > Polaris. The actual access-decision is up to the IdP (external IdP
> or
> > > > > Polaris w/ its own principals). Each principal can also be uniquely
> > > > > identified (IDP + principal-ID).
> > > > >
> > > > > I really do not think that principals owned by "external IdPs"
> should
> > > > > ever be persisted in Polaris.
> > > > >
> > > > > Similar for roles manages by external IdPs.
> > > > >
> > > > > ACLs (grants) are (generally speaking) lists of
> grantee+privilege(s)
> > > > > tuples. It is IMHO better to use a qualified reference to "IDP +
> > > > > principal/role-ID".
> > > > >
> > > > > AFAIR it's only possbible to grant privileges to roles, not
> > individual
> > > > > principals. And if that's the case, I do not see a requirement to
> > have
> > > > > "federated principals" being persisted in Polaris at all.
> > > > >
> > > > > I seems the underlying issue here is the very tight coupling of
> > > > > principals + principal-roles + catalog-roles + grants that
> requires a
> > > > > unique integer-ID for each of those and using the same
> > > database-oriented
> > > > > approach for role-membership management and catalog-entity grants.
> > > > >
> > > > > What's IMHO needed is a proper separation of those things, which
> > should
> > > > > happen now and after that's done integrate external IdPs.
> > > > >
> > > > >
> > > > > On 22.04.25 17:00, Alex Dutra wrote:
> > > > > > Hi Mike,
> > > > > >
> > > > > > Thanks for the historical context, that is really helpful. And I
> > > > > completely
> > > > > > agree with your last paragraph, and especially this:
> > > > > >
> > > > > > My preference would be to make this an immutable class that has
> the
> > > > > >> principal entity and all active principal roles all in the
> > > > constructor.
> > > > > >
> > > > > > I was arriving at the exact same conclusion – it's a pity that
> > > > > > SecurityContext doesn't expose the principal roles. I guess, the
> > > > problem
> > > > > > now is how to move forward with this suggestion – I imagine a
> > change
> > > > > > in AuthenticatedPolarisPrincipal or in the Resolver would create
> > > > > > repercussions in a lot of places in the code base as well as in
> > > > existing
> > > > > > PRs.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Alex
> > > > > >
> > > > > > On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <
> > > > collado.mike@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > >> Hey
> > > > > >>
> > > > > >> 1. I'm unsure why the Resolver was written to assume no role
> means
> > > all
> > > > > >> roles. But I think it has to do with
> > > > > >> 2. The Resolver was intended to be able to resolve principal
> > roles,
> > > > > grants,
> > > > > >> and target entities all at one "snapshot" based on the entity
> and
> > > > grant
> > > > > >> versions returned by that loadEntitiesChangeTracking method. In
> > > order
> > > > > to do
> > > > > >> this in one shot, the authenticator couldn't resolve the
> principal
> > > > roles
> > > > > >> actually granted to the principal, so it didn't resolve the
> > > > > >> PRINCIPAL_ROLE:ALL scope in the token. But that meant the
> Resolver
> > > > > needed
> > > > > >> to verify the grants existed for the roles to be assumed by the
> > > > > principal
> > > > > >> (otherwise, a caller could just put whatever role they want in
> the
> > > > > scope).
> > > > > >> To avoid the lookup during authn, ALL needed to be expanded by
> the
> > > > > Resolver
> > > > > >> based on the grants. This was an impedance to the federated role
> > > > > proposal,
> > > > > >> which stated that the authn token is authoritative regarding
> which
> > > > roles
> > > > > >> are granted to the principal (e.g., using a groups claim
> > > > > >> <
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> > > > > >>> ).
> > > > > >> That's why
> > > > > >> 3. I added the ActiveRolesProvider interface so that the default
> > > impl
> > > > > would
> > > > > >> continue to validate the grants held by the principal for
> > activating
> > > > > >> principal roles but the federated alternative would delegate to
> > the
> > > > > token
> > > > > >> claims (yet to be committed).
> > > > > >>
> > > > > >> Hopefully, that history helps. In short, I agree that
> > > > > >> SecurityIdentity.getRoles() should be the right way to determine
> > > > > activated
> > > > > >> principal roles. The Resolver should rely on the authenticator
> to
> > > > > determine
> > > > > >> the active set of principal roles and not rely on grant records.
> > The
> > > > > >> AuthenticatedPolarisPrincipal doesn't really need to hold role
> > names
> > > > > >> anymore, but the resolver does still need the list of active
> > > > > PrincipalRoles
> > > > > >> to load the appropriate catalog roles. My preference would be to
> > > make
> > > > > this
> > > > > >> an immutable class that has the principal entity and all active
> > > > > principal
> > > > > >> roles all in the constructor.
> > > > > >>
> > > > > >> Mike
> > > > > >>
> > > > > >> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
> > > > > <al...@dremio.com.invalid>
> > > > > >> wrote:
> > > > > >>
> > > > > >>> Hi Mike,
> > > > > >>>
> > > > > >>> I am generally fine with the spec changes.
> > > > > >>>
> > > > > >>> I have however some concerns around the way we handle principal
> > > roles
> > > > > >> today
> > > > > >>> in Polaris, and as I was preparing the PR with support for
> > external
> > > > > IDPs
> > > > > >>> [1], some oddities stood out:
> > > > > >>>
> > > > > >>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention,
> > to
> > > be
> > > > > >>> converted to an empty set of roles by the Authenticator;
> but
> > I
> > > > > wonder
> > > > > >>> how we will be able to distinguish a token that contained
> no
> > > role
> > > > > >> from a
> > > > > >>> token that contained the pseudo-role. I added some changes
> to
> > > my
> > > > > PR to
> > > > > >>> be
> > > > > >>> able to distinguish the two situations, but I think the
> > general
> > > > > >>> handling of
> > > > > >>> this pseudo-role, as well as role mapping in general,
> > deserves
> > > > some
> > > > > >> more
> > > > > >>> thinking. The spec says this pseudo-role is
> "non-functional"
> > –
> > > > but
> > > > > I
> > > > > >>> don't
> > > > > >>> know what this implies exactly.
> > > > > >>> 2. AuthenticatedPolarisPrincipal exposes principal roles
> and
> > > this
> > > > > is
> > > > > >>> imho concerning for two reasons: first, the Resolver seems
> to
> > > > > update
> > > > > >> the
> > > > > >>> principal roles during resolution, which makes the class
> > > mutable,
> > > > > >>> potentially changing the principal characteristics *after*
> > > > > >>> authentication.
> > > > > >>> And secondly, this creates an ambiguity as roles can be
> > > accessed
> > > > in
> > > > > >> two
> > > > > >>> ways: SecurityIdentity.getRoles() vs
> > > > > >>>
> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
> > > > > would be
> > > > > >>> in favor of using the former consistently and completely
> > > removing
> > > > > the
> > > > > >>> latter. (Note: SecurityContext.isUserInRole() relies on the
> > > > > former.)
> > > > > >>> 3. There seems to be some overlap between the primary roles
> > > > > >> resolution,
> > > > > >>> done by the ActiveRolesProvider, and the role resolution
> done
> > > > > later by
> > > > > >>> the Resolver, cf. method
> > > > > resolveCallerPrincipalAndPrincipalRoles(). I
> > > > > >>> wonder if this second role resolution is really necessary?
> > > > > Ultimately,
> > > > > >>> the
> > > > > >>> source of truth for role membership will always be
> > > > > >>> SecurityIdentity.hasRole() or its JAX-RS variant,
> > > > > >>> SecurityContext.isUserInRole(). We should not deviate from
> > > that.
> > > > > >>>
> > > > > >>> Thanks,
> > > > > >>>
> > > > > >>> Alex
> > > > > >>>
> > > > > >>> [1] https://github.com/apache/polaris/pull/1397
> > > > > >>>
> > > > > >>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <
> > > > dimas@apache.org>
> > > > > >>> wrote:
> > > > > >>>
> > > > > >>>> Re 2: Thanks for the clarification, Mike! I guess my brain
> > swapped
> > > > out
> > > > > >> a
> > > > > >>>> large portion of that doc :)
> > > > > >>>>
> > > > > >>>> I'm still not sure how IdentityToPrincipalMapping can help
> with
> > > > > >> resolving
> > > > > >>>> changes from API and from IdP integration.
> > > > > >>>>
> > > > > >>>> The doc talks about namespaces, but in PR# 1353, it looks like
> > API
> > > > > >> calls
> > > > > >>>> are free to change anything in federated principal roles.
> > > > > >>>>
> > > > > >>>> Should IdentityToPrincipalMapping extend to validating changes
> > > > coming
> > > > > >>> from
> > > > > >>>> the Admin API?
> > > > > >>>>
> > > > > >>>> Alternatively, we could treat that as a new permission and
> allow
> > > all
> > > > > >>> admin
> > > > > >>>> users to edit federated roles via API.
> > > > > >>>>
> > > > > >>>> Re 1: Yes, I guess it would be helpful with SCIM, but then
> we'd
> > > need
> > > > > to
> > > > > >>>> sync role membership, I guess.
> > > > > >>>>
> > > > > >>>> My understanding so far was that we'd get role membership at
> > > > > >>> authentication
> > > > > >>>> time and not persist it in Polaris.
> > > > > >>>>
> > > > > >>>> Is that still the plan?
> > > > > >>>>
> > > > > >>>> I think this relates to how we connect authentication and
> > > > > authorization
> > > > > >>> in
> > > > > >>>> Polaris. If Principals are external, we get role membership
> from
> > > the
> > > > > >>> access
> > > > > >>>> token and all is clear. If Principals are local, and we still
> > get
> > > > > roles
> > > > > >>>> from the access token, then the question arises about whether
> > that
> > > > > data
> > > > > >>> is
> > > > > >>>> in sync with local data (and the other way around too).
> > > > > >>>>
> > > > > >>>> That could be resolved by Polaris owning access tokens (via
> > token
> > > > > >>> exchange)
> > > > > >>>> and assuming control over role membership based on local data
> > and
> > > > > >> access
> > > > > >>>> time (I commented on this in the doc too).
> > > > > >>>>
> > > > > >>>> All in all, since SCIM is not part of phase 1 here, maybe we
> > could
> > > > > >> defer
> > > > > >>>> dealing with federated Principal persistence, indeed.
> > > > > >>>>
> > > > > >>>> Re 3: I guess it also applies to client secret generation
> code,
> > > but
> > > > > >> let's
> > > > > >>>> see whether it actually causes too many conditions in code
> when
> > it
> > > > > >> comes
> > > > > >>> to
> > > > > >>>> that.
> > > > > >>>>
> > > > > >>>> Cheers,
> > > > > >>>> Dmitri.
> > > > > >>>>
> > > > > >>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> > > > > >> collado.mike@gmail.com>
> > > > > >>>> wrote:
> > > > > >>>>
> > > > > >>>>> Thanks for the response.
> > > > > >>>>>
> > > > > >>>>> 1. TBH, I have no strong feelings about persisting the
> > federated
> > > > > >>>>> principals. I think the biggest advantage I saw was to
> support
> > > SCIM
> > > > > >>> along
> > > > > >>>>> with 3p identity providers. But for this implementation, if
> we
> > > want
> > > > > >> to
> > > > > >>>> skip
> > > > > >>>>> persisting principal entities, I'm ok with it.
> > > > > >>>>>
> > > > > >>>>> 2. In the doc I wrote the following, which I think addresses
> > the
> > > > > >>> concern
> > > > > >>>>> around conflicts.
> > > > > >>>>>
> > > > > >>>>> 1.
> > > > > >>>>>
> > > > > >>>>> The IdentityToPrincipalMapping should supply a naming
> > > > convention
> > > > > >> so
> > > > > >>>> that
> > > > > >>>>> PrincipalRoles and Principals created from that source do
> > not
> > > > > >>> conflict
> > > > > >>>>> with
> > > > > >>>>> entities created by a different IdentityProvider
> > > > > >>>>>
> > > > > >>>>> In the case that a role happens to exist even with the naming
> > > > > >>> convention
> > > > > >>>>> applied, the caller would be unable to authenticate with that
> > > role.
> > > > > >>>>>
> > > > > >>>>> 3. I don't think we need special classes for federated roles.
> > In
> > > > > >> nearly
> > > > > >>>> all
> > > > > >>>>> of the Polaris code, they are treated as PolarisRoleEntity
> > > without
> > > > > >> any
> > > > > >>>>> issue. The only code that needs to be aware of federated
> roles
> > is
> > > > > >>>>>
> > > > > >>>>> a) The Admin API, which prevents granting these roles to
> > > principals
> > > > > >> and
> > > > > >>>>> b) The federated roles provider, which verifies that the
> > > federated
> > > > > >>>>> principal is assuming federated roles.
> > > > > >>>>>
> > > > > >>>>> The rest of the code (i.e., the resolver and authorizer)
> treat
> > > > these
> > > > > >>>> roles
> > > > > >>>>> exactly as native Polaris roles.
> > > > > >>>>>
> > > > > >>>>> Mike
> > > > > >>>>>
> > > > > >>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> > > > > >> dimas@apache.org>
> > > > > >>>>> wrote:
> > > > > >>>>>
> > > > > >>>>>> Thanks for reviving this discussion, Mike!
> > > > > >>>>>>
> > > > > >>>>>> The API spec change by itself LGTM, but I have related
> > concerns
> > > in
> > > > > >>> how
> > > > > >>>>> this
> > > > > >>>>>> feature is meant to work in general.
> > > > > >>>>>>
> > > > > >>>>>> 1) The need to expose Federated Principals in Polaris API.
> > > > > >>>>>>
> > > > > >>>>>> The design doc [1] discusses the possibility to expose
> > Federated
> > > > > >>>>> Principals
> > > > > >>>>>> in Polaris API, but there are pros and cons. I do not think
> it
> > > is
> > > > a
> > > > > >>>>> trivial
> > > > > >>>>>> decision. I'd like to discuss this in more detail before we
> > > commit
> > > > > >> to
> > > > > >>>>> doing
> > > > > >>>>>> that.
> > > > > >>>>>>
> > > > > >>>>>> From my POV the main benefit of exposing Federated
> Principals
> > > is
> > > > to
> > > > > >>> be
> > > > > >>>>> able
> > > > > >>>>>> to alter their properties in Polaris (as mentioned in the
> > doc).
> > > > > >>>> However,
> > > > > >>>>>> your email indicates that this is actually forbidden.
> > > > > >>>>>>
> > > > > >>>>>> What use cases do you envision for exposing read-only
> > Federated
> > > > > >>>>> Principals?
> > > > > >>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
> > > > > >>>>>>
> > > > > >>>>>> Let's focus on Principal Roles here (which are meant to be
> > > > writable
> > > > > >>> via
> > > > > >>>>>> API).
> > > > > >>>>>>
> > > > > >>>>>> If users are able to make and change (I assume) Principal
> > Roles
> > > > via
> > > > > >>>> API,
> > > > > >>>>>> and federation code will also be creating and changing those
> > > > roles,
> > > > > >>>> what
> > > > > >>>>> is
> > > > > >>>>>> our approach to handling conflicts between those two streams
> > of
> > > > > >>>> changes?
> > > > > >>>>>> Some examples for context (also mentioned in GH):
> > > > > >>>>>>
> > > > > >>>>>> A) A user creates normal Principal Role "A", and later role
> > "A"
> > > is
> > > > > >>>>>> discovered through Federation.
> > > > > >>>>>>
> > > > > >>>>>> B) A user edits Federated Role "B" and later properties of
> "B"
> > > are
> > > > > >>>>> updated
> > > > > >>>>>> through Federation (or the other way around).
> > > > > >>>>>>
> > > > > >>>>>> Do we want to control property edits based on the origin of
> > the
> > > > > >>>> property
> > > > > >>>>>> (spec out a "namespace" for Federated properties)?
> > > > > >>>>>>
> > > > > >>>>>> 3) OO types
> > > > > >>>>>>
> > > > > >>>>>> Currently Principals and Principal Roles are represented by
> > one
> > > > > >> java
> > > > > >>>> type
> > > > > >>>>>> each. When federation comes into play, would it make sense
> to
> > > > > >>> develop a
> > > > > >>>>>> (java) type hierarchy for working with that data in Polaris
> > > code?
> > > > > >> My
> > > > > >>>> main
> > > > > >>>>>> concern here is avoiding a maze of if/else statements
> > throughout
> > > > > >> the
> > > > > >>>>>> Polaris codebase for supporting Federation.
> > > > > >>>>>>
> > > > > >>>>>> I guess our approach to this depends largely on the outcome
> of
> > > > > >> topic
> > > > > >>> 2
> > > > > >>>>>> above.
> > > > > >>>>>>
> > > > > >>>>>> WDYT?
> > > > > >>>>>>
> > > > > >>>>>> Thanks,
> > > > > >>>>>> Dmitri.
> > > > > >>>>>>
> > > > > >>>>>> [1]
> > > > > >>>>>>
> > > > > >>>>>>
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > > > > >>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > > > > >>>> collado.mike@gmail.com>
> > > > > >>>>>> wrote:
> > > > > >>>>>>
> > > > > >>>>>>> Hey folks
> > > > > >>>>>>>
> > > > > >>>>>>> Some of you already know that I posted an initial PR to get
> > > > > >>> federated
> > > > > >>>>>>> principals/roles added. One thing that came out of the
> > feedback
> > > > > >>> was a
> > > > > >>>>>> spec
> > > > > >>>>>>> change to make it clear when federated identities can be
> used
> > > in
> > > > > >>> the
> > > > > >>>>>> APIs.
> > > > > >>>>>>> Notably, federated principals cannot be created or updated,
> > but
> > > > > >> can
> > > > > >>>> be
> > > > > >>>>>>> returned in get/list calls, whereas federated roles *can*
> be
> > > > > >>> created
> > > > > >>>> by
> > > > > >>>>>> the
> > > > > >>>>>>> API. The latter is useful/necessary in order to be able to
> > > assign
> > > > > >>>>>>> privileges to those roles without relying on the JIT
> creation
> > > on
> > > > > >>>> login.
> > > > > >>>>>>> Please check out the spec change here and let me know what
> > you
> > > > > >>> think
> > > > > >>>> -
> > > > > >>>>>>>
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > > > >>>>>>> Mike
> > > > > >>>>>>>
> > > > > >>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > > > >>>>>>> <dm...@dremio.com.invalid> wrote:
> > > > > >>>>>>>
> > > > > >>>>>>>> Hi Mike,
> > > > > >>>>>>>>
> > > > > >>>>>>>> I left some comments in the doc, but overall it looks good
> > to
> > > > > >> me
> > > > > >>> :)
> > > > > >>>>>>>> I still think there are some hidden dependencies on
> > > > > >> Persistence.
> > > > > >>>> For
> > > > > >>>>>>>> example, whether and how we can have composite keys for
> > > > > >> persisted
> > > > > >>>>>>> federated
> > > > > >>>>>>>> entities... but I guess we can work that out later.
> > > > > >>>>>>>>
> > > > > >>>>>>>> Also, I think it is important for the Authorizer API to
> > avoid
> > > > > >>>>> assuming
> > > > > >>>>>>> that
> > > > > >>>>>>>> all principals are persisted. Specific authorizer
> > > > > >> implementations
> > > > > >>>>>>>> (including the default one) can certainly expect persisted
> > > > > >>>>> principals,
> > > > > >>>>>>> but
> > > > > >>>>>>>> the API should require that for the sake of flexibility of
> > > > > >>> possible
> > > > > >>>>>>> AuthN/Z
> > > > > >>>>>>>> extensions. WDYT?
> > > > > >>>>>>>>
> > > > > >>>>>>>> Cheers,
> > > > > >>>>>>>> Dmitri.
> > > > > >>>>>>>>
> > > > > >>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > > > >>>>>> collado.mike@gmail.com>
> > > > > >>>>>>>> wrote:
> > > > > >>>>>>>>
> > > > > >>>>>>>>> Hey folks
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> As discussed during the community sync, I've put together
> > > > > >> some
> > > > > >>>>>> thoughts
> > > > > >>>>>>>> on
> > > > > >>>>>>>>> how we'd add support for federated identities in
> Polaris. I
> > > > > >>>> copied
> > > > > >>>>>> over
> > > > > >>>>>>>>> some of what I had in the issue at
> > > > > >>>>>>>>> https://github.com/apache/polaris/issues/441 and put it
> > into
> > > > > >>> the
> > > > > >>>>> doc
> > > > > >>>>>>>> here:
> > > > > >>>>>>>>>
> > > > > >>>>>>>>>
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > > >>>>>>>>> .
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> Please take a look when you get some time and let me know
> > > > > >> what
> > > > > >>>> you
> > > > > >>>>>>> think.
> > > > > >>>>>>>>> Given that our next community sync is scheduled for the
> > > > > >>>>> Thanksgiving
> > > > > >>>>>>>>> holiday in the US, it might be useful to schedule a
> meeting
> > > > > >>>>>>> specifically
> > > > > >>>>>>>>> for this. I can schedule that sync if needed.
> > > > > >>>>>>>>>
> > > > > >>>>>>>>> Mike
> > > > > >>>>>>>>>
> > > > > --
> > > > > Robert Stupp
> > > > > @snazy
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Dmitri Bourlatchkov <di...@apache.org>.
But even in the original code, there is a test asserting that federated
principals can't be created.
Yeah, I saw that, but this was actually the thing that felt awkward to me
in the PR.
I'd prefer to avoid making API changes that do not have an execution option
in runtime (i.e. not possible to create Federated Principals in the API and
not having SCIM either). My rationale is that it is generally easy to add
API but hard to remove/change what has been exposed to the public.
Maybe our approaches are just different. As for me, making backend changes
first, adding API later is more "natural" :)
That said, from my POV this specific API change that adds the "federated"
property to Principals is rather basic and I'd be ok with making it now
even if we do not have SCIM (while I'd prefer not to make it as explained
above).
https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
This is more of a blocker from my POV because the behaviour of the current
builder code is hard to predict without reading the implementation, which
IMHO developers do not have to do on a regular basis as builders have a
pretty simple API, where a set call should result in the argument becoming
the new actual value of the attribute.
Cheers,
Dmitri.
On Thu, May 1, 2025 at 12:48 AM Michael Collado <co...@gmail.com>
wrote:
> Hi Dmitri
>
> I do have a revision ready to push. As of this morning (PDT), I was still
> waiting for a response to this issue (I see you've responded since) -
> https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
>
> But even in the original code, there is a test asserting that federated
> principals can't be created.
>
> I think to have a discussion that moves forward, it's important that people
> feel heard. If someone has comments on the code or the proposed designs,
> I'm happy to have the discussion, but if neither the code nor the e-mail
> discussion are being read... I don't see how this conversation can move
> forward.
>
> Mike
>
> On Wed, Apr 30, 2025 at 5:58 PM Dmitri Bourlatchkov <di...@apache.org>
> wrote:
>
> > Hi Mike,
> >
> > I guess some of the confusion may come from PR 1353 [1] and this thread
> > being a bit out of sync.
> >
> > Would you mind removing federated Principals from the API in the PR for
> the
> > sake of clarity?
> >
> > From my POV, we can reconsider that if/when SCIM (or something similar)
> > comes.
> >
> > Thanks,
> > Dmitri.
> >
> > [1] https://github.com/apache/polaris/pull/1353
> >
> > On Wed, Apr 30, 2025 at 11:50 AM Michael Collado <collado.mike@gmail.com
> >
> > wrote:
> >
> > > Hi Robert
> > >
> > > Unsure if you read the previous emails in this thread. In my earlier
> > > response, I wrote
> > >
> > > > 1. TBH, I have no strong feelings about persisting the federated
> > > principals. I think the biggest advantage I saw was to support SCIM
> along
> > > with 3p identity providers. But for this implementation, if we want to
> > skip
> > > persisting principal entities, I'm ok with it.
> > >
> > > I feel that should address the majority of the concern you just
> > expressed.
> > >
> > > Storing grants with references to some kind of FQN is certainly one
> > > approach, but IMO, is a massive undertaking with little benefit. For
> > > example, all of the grant APIs have to be modified to deal with target
> > > entities that don’t exist. How do we do validation for these APIs? Do
> > these
> > > APIs now need to talk to external entities to ensure the caller didn’t
> > > misspell a role name or use all caps instead of mixed case? UI
> developers
> > > can’t simply populate a list of roles for a dropdown list unless they
> > also
> > > connect directly to the idp. That makes it hard for someone to build a
> > > generic Polaris admin UI without also building support for Okta,
> Google,
> > > Apple, etc.
> > >
> > > On the other hand, SCIM is an industry standard for a reason. Every
> major
> > > IdP supports it - and I doubt it’s because they just don’t know any
> > better.
> > > Supporting federated roles + SCIM means all the existing grant APIs
> Just
> > > Work and building generic privilege management tools/UI could be done
> > > today.
> > >
> > > Given the choice between a massive rewrite of the grant and
> authorization
> > > primitives with no user-facing benefit and supporting federated roles
> > with
> > > the option of SCIM support in the future, the answer seems clear to me.
> > >
> > > Mike
> > >
> > > On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
> > >
> > > > Hi,
> > > >
> > > > thanks Mike for the effort.
> > > >
> > > > However, I think that persisting principals, which are not "owned" by
> > > > Polaris, being persisted in Polaris is a good idea. How would changes
> > to
> > > > a principal in the IdP be propagated to Polaris? What happens if a
> > > > principal that's deleted/disabled in the IdP - how is that reflected
> > in
> > > > Polaris? What would happen if two Polaris instances concurrently
> > > > "transparently create" the same "federated principal entity"?
> > > >
> > > > Storing mirrored information that is only owned by an (external) IdP
> > > > leads to a "what's the source-of-truth" problem.
> > > >
> > > > A principal is (mostly) an individual user or service that can access
> > > > Polaris. The actual access-decision is up to the IdP (external IdP or
> > > > Polaris w/ its own principals). Each principal can also be uniquely
> > > > identified (IDP + principal-ID).
> > > >
> > > > I really do not think that principals owned by "external IdPs" should
> > > > ever be persisted in Polaris.
> > > >
> > > > Similar for roles manages by external IdPs.
> > > >
> > > > ACLs (grants) are (generally speaking) lists of grantee+privilege(s)
> > > > tuples. It is IMHO better to use a qualified reference to "IDP +
> > > > principal/role-ID".
> > > >
> > > > AFAIR it's only possbible to grant privileges to roles, not
> individual
> > > > principals. And if that's the case, I do not see a requirement to
> have
> > > > "federated principals" being persisted in Polaris at all.
> > > >
> > > > I seems the underlying issue here is the very tight coupling of
> > > > principals + principal-roles + catalog-roles + grants that requires a
> > > > unique integer-ID for each of those and using the same
> > database-oriented
> > > > approach for role-membership management and catalog-entity grants.
> > > >
> > > > What's IMHO needed is a proper separation of those things, which
> should
> > > > happen now and after that's done integrate external IdPs.
> > > >
> > > >
> > > > On 22.04.25 17:00, Alex Dutra wrote:
> > > > > Hi Mike,
> > > > >
> > > > > Thanks for the historical context, that is really helpful. And I
> > > > completely
> > > > > agree with your last paragraph, and especially this:
> > > > >
> > > > > My preference would be to make this an immutable class that has the
> > > > >> principal entity and all active principal roles all in the
> > > constructor.
> > > > >
> > > > > I was arriving at the exact same conclusion – it's a pity that
> > > > > SecurityContext doesn't expose the principal roles. I guess, the
> > > problem
> > > > > now is how to move forward with this suggestion – I imagine a
> change
> > > > > in AuthenticatedPolarisPrincipal or in the Resolver would create
> > > > > repercussions in a lot of places in the code base as well as in
> > > existing
> > > > > PRs.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Alex
> > > > >
> > > > > On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <
> > > collado.mike@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > >> Hey
> > > > >>
> > > > >> 1. I'm unsure why the Resolver was written to assume no role means
> > all
> > > > >> roles. But I think it has to do with
> > > > >> 2. The Resolver was intended to be able to resolve principal
> roles,
> > > > grants,
> > > > >> and target entities all at one "snapshot" based on the entity and
> > > grant
> > > > >> versions returned by that loadEntitiesChangeTracking method. In
> > order
> > > > to do
> > > > >> this in one shot, the authenticator couldn't resolve the principal
> > > roles
> > > > >> actually granted to the principal, so it didn't resolve the
> > > > >> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver
> > > > needed
> > > > >> to verify the grants existed for the roles to be assumed by the
> > > > principal
> > > > >> (otherwise, a caller could just put whatever role they want in the
> > > > scope).
> > > > >> To avoid the lookup during authn, ALL needed to be expanded by the
> > > > Resolver
> > > > >> based on the grants. This was an impedance to the federated role
> > > > proposal,
> > > > >> which stated that the authn token is authoritative regarding which
> > > roles
> > > > >> are granted to the principal (e.g., using a groups claim
> > > > >> <
> > > > >>
> > > >
> > >
> >
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> > > > >>> ).
> > > > >> That's why
> > > > >> 3. I added the ActiveRolesProvider interface so that the default
> > impl
> > > > would
> > > > >> continue to validate the grants held by the principal for
> activating
> > > > >> principal roles but the federated alternative would delegate to
> the
> > > > token
> > > > >> claims (yet to be committed).
> > > > >>
> > > > >> Hopefully, that history helps. In short, I agree that
> > > > >> SecurityIdentity.getRoles() should be the right way to determine
> > > > activated
> > > > >> principal roles. The Resolver should rely on the authenticator to
> > > > determine
> > > > >> the active set of principal roles and not rely on grant records.
> The
> > > > >> AuthenticatedPolarisPrincipal doesn't really need to hold role
> names
> > > > >> anymore, but the resolver does still need the list of active
> > > > PrincipalRoles
> > > > >> to load the appropriate catalog roles. My preference would be to
> > make
> > > > this
> > > > >> an immutable class that has the principal entity and all active
> > > > principal
> > > > >> roles all in the constructor.
> > > > >>
> > > > >> Mike
> > > > >>
> > > > >> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
> > > > <al...@dremio.com.invalid>
> > > > >> wrote:
> > > > >>
> > > > >>> Hi Mike,
> > > > >>>
> > > > >>> I am generally fine with the spec changes.
> > > > >>>
> > > > >>> I have however some concerns around the way we handle principal
> > roles
> > > > >> today
> > > > >>> in Polaris, and as I was preparing the PR with support for
> external
> > > > IDPs
> > > > >>> [1], some oddities stood out:
> > > > >>>
> > > > >>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention,
> to
> > be
> > > > >>> converted to an empty set of roles by the Authenticator; but
> I
> > > > wonder
> > > > >>> how we will be able to distinguish a token that contained no
> > role
> > > > >> from a
> > > > >>> token that contained the pseudo-role. I added some changes to
> > my
> > > > PR to
> > > > >>> be
> > > > >>> able to distinguish the two situations, but I think the
> general
> > > > >>> handling of
> > > > >>> this pseudo-role, as well as role mapping in general,
> deserves
> > > some
> > > > >> more
> > > > >>> thinking. The spec says this pseudo-role is "non-functional"
> –
> > > but
> > > > I
> > > > >>> don't
> > > > >>> know what this implies exactly.
> > > > >>> 2. AuthenticatedPolarisPrincipal exposes principal roles and
> > this
> > > > is
> > > > >>> imho concerning for two reasons: first, the Resolver seems to
> > > > update
> > > > >> the
> > > > >>> principal roles during resolution, which makes the class
> > mutable,
> > > > >>> potentially changing the principal characteristics *after*
> > > > >>> authentication.
> > > > >>> And secondly, this creates an ambiguity as roles can be
> > accessed
> > > in
> > > > >> two
> > > > >>> ways: SecurityIdentity.getRoles() vs
> > > > >>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
> > > > would be
> > > > >>> in favor of using the former consistently and completely
> > removing
> > > > the
> > > > >>> latter. (Note: SecurityContext.isUserInRole() relies on the
> > > > former.)
> > > > >>> 3. There seems to be some overlap between the primary roles
> > > > >> resolution,
> > > > >>> done by the ActiveRolesProvider, and the role resolution done
> > > > later by
> > > > >>> the Resolver, cf. method
> > > > resolveCallerPrincipalAndPrincipalRoles(). I
> > > > >>> wonder if this second role resolution is really necessary?
> > > > Ultimately,
> > > > >>> the
> > > > >>> source of truth for role membership will always be
> > > > >>> SecurityIdentity.hasRole() or its JAX-RS variant,
> > > > >>> SecurityContext.isUserInRole(). We should not deviate from
> > that.
> > > > >>>
> > > > >>> Thanks,
> > > > >>>
> > > > >>> Alex
> > > > >>>
> > > > >>> [1] https://github.com/apache/polaris/pull/1397
> > > > >>>
> > > > >>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <
> > > dimas@apache.org>
> > > > >>> wrote:
> > > > >>>
> > > > >>>> Re 2: Thanks for the clarification, Mike! I guess my brain
> swapped
> > > out
> > > > >> a
> > > > >>>> large portion of that doc :)
> > > > >>>>
> > > > >>>> I'm still not sure how IdentityToPrincipalMapping can help with
> > > > >> resolving
> > > > >>>> changes from API and from IdP integration.
> > > > >>>>
> > > > >>>> The doc talks about namespaces, but in PR# 1353, it looks like
> API
> > > > >> calls
> > > > >>>> are free to change anything in federated principal roles.
> > > > >>>>
> > > > >>>> Should IdentityToPrincipalMapping extend to validating changes
> > > coming
> > > > >>> from
> > > > >>>> the Admin API?
> > > > >>>>
> > > > >>>> Alternatively, we could treat that as a new permission and allow
> > all
> > > > >>> admin
> > > > >>>> users to edit federated roles via API.
> > > > >>>>
> > > > >>>> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd
> > need
> > > > to
> > > > >>>> sync role membership, I guess.
> > > > >>>>
> > > > >>>> My understanding so far was that we'd get role membership at
> > > > >>> authentication
> > > > >>>> time and not persist it in Polaris.
> > > > >>>>
> > > > >>>> Is that still the plan?
> > > > >>>>
> > > > >>>> I think this relates to how we connect authentication and
> > > > authorization
> > > > >>> in
> > > > >>>> Polaris. If Principals are external, we get role membership from
> > the
> > > > >>> access
> > > > >>>> token and all is clear. If Principals are local, and we still
> get
> > > > roles
> > > > >>>> from the access token, then the question arises about whether
> that
> > > > data
> > > > >>> is
> > > > >>>> in sync with local data (and the other way around too).
> > > > >>>>
> > > > >>>> That could be resolved by Polaris owning access tokens (via
> token
> > > > >>> exchange)
> > > > >>>> and assuming control over role membership based on local data
> and
> > > > >> access
> > > > >>>> time (I commented on this in the doc too).
> > > > >>>>
> > > > >>>> All in all, since SCIM is not part of phase 1 here, maybe we
> could
> > > > >> defer
> > > > >>>> dealing with federated Principal persistence, indeed.
> > > > >>>>
> > > > >>>> Re 3: I guess it also applies to client secret generation code,
> > but
> > > > >> let's
> > > > >>>> see whether it actually causes too many conditions in code when
> it
> > > > >> comes
> > > > >>> to
> > > > >>>> that.
> > > > >>>>
> > > > >>>> Cheers,
> > > > >>>> Dmitri.
> > > > >>>>
> > > > >>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> > > > >> collado.mike@gmail.com>
> > > > >>>> wrote:
> > > > >>>>
> > > > >>>>> Thanks for the response.
> > > > >>>>>
> > > > >>>>> 1. TBH, I have no strong feelings about persisting the
> federated
> > > > >>>>> principals. I think the biggest advantage I saw was to support
> > SCIM
> > > > >>> along
> > > > >>>>> with 3p identity providers. But for this implementation, if we
> > want
> > > > >> to
> > > > >>>> skip
> > > > >>>>> persisting principal entities, I'm ok with it.
> > > > >>>>>
> > > > >>>>> 2. In the doc I wrote the following, which I think addresses
> the
> > > > >>> concern
> > > > >>>>> around conflicts.
> > > > >>>>>
> > > > >>>>> 1.
> > > > >>>>>
> > > > >>>>> The IdentityToPrincipalMapping should supply a naming
> > > convention
> > > > >> so
> > > > >>>> that
> > > > >>>>> PrincipalRoles and Principals created from that source do
> not
> > > > >>> conflict
> > > > >>>>> with
> > > > >>>>> entities created by a different IdentityProvider
> > > > >>>>>
> > > > >>>>> In the case that a role happens to exist even with the naming
> > > > >>> convention
> > > > >>>>> applied, the caller would be unable to authenticate with that
> > role.
> > > > >>>>>
> > > > >>>>> 3. I don't think we need special classes for federated roles.
> In
> > > > >> nearly
> > > > >>>> all
> > > > >>>>> of the Polaris code, they are treated as PolarisRoleEntity
> > without
> > > > >> any
> > > > >>>>> issue. The only code that needs to be aware of federated roles
> is
> > > > >>>>>
> > > > >>>>> a) The Admin API, which prevents granting these roles to
> > principals
> > > > >> and
> > > > >>>>> b) The federated roles provider, which verifies that the
> > federated
> > > > >>>>> principal is assuming federated roles.
> > > > >>>>>
> > > > >>>>> The rest of the code (i.e., the resolver and authorizer) treat
> > > these
> > > > >>>> roles
> > > > >>>>> exactly as native Polaris roles.
> > > > >>>>>
> > > > >>>>> Mike
> > > > >>>>>
> > > > >>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> > > > >> dimas@apache.org>
> > > > >>>>> wrote:
> > > > >>>>>
> > > > >>>>>> Thanks for reviving this discussion, Mike!
> > > > >>>>>>
> > > > >>>>>> The API spec change by itself LGTM, but I have related
> concerns
> > in
> > > > >>> how
> > > > >>>>> this
> > > > >>>>>> feature is meant to work in general.
> > > > >>>>>>
> > > > >>>>>> 1) The need to expose Federated Principals in Polaris API.
> > > > >>>>>>
> > > > >>>>>> The design doc [1] discusses the possibility to expose
> Federated
> > > > >>>>> Principals
> > > > >>>>>> in Polaris API, but there are pros and cons. I do not think it
> > is
> > > a
> > > > >>>>> trivial
> > > > >>>>>> decision. I'd like to discuss this in more detail before we
> > commit
> > > > >> to
> > > > >>>>> doing
> > > > >>>>>> that.
> > > > >>>>>>
> > > > >>>>>> From my POV the main benefit of exposing Federated Principals
> > is
> > > to
> > > > >>> be
> > > > >>>>> able
> > > > >>>>>> to alter their properties in Polaris (as mentioned in the
> doc).
> > > > >>>> However,
> > > > >>>>>> your email indicates that this is actually forbidden.
> > > > >>>>>>
> > > > >>>>>> What use cases do you envision for exposing read-only
> Federated
> > > > >>>>> Principals?
> > > > >>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
> > > > >>>>>>
> > > > >>>>>> Let's focus on Principal Roles here (which are meant to be
> > > writable
> > > > >>> via
> > > > >>>>>> API).
> > > > >>>>>>
> > > > >>>>>> If users are able to make and change (I assume) Principal
> Roles
> > > via
> > > > >>>> API,
> > > > >>>>>> and federation code will also be creating and changing those
> > > roles,
> > > > >>>> what
> > > > >>>>> is
> > > > >>>>>> our approach to handling conflicts between those two streams
> of
> > > > >>>> changes?
> > > > >>>>>> Some examples for context (also mentioned in GH):
> > > > >>>>>>
> > > > >>>>>> A) A user creates normal Principal Role "A", and later role
> "A"
> > is
> > > > >>>>>> discovered through Federation.
> > > > >>>>>>
> > > > >>>>>> B) A user edits Federated Role "B" and later properties of "B"
> > are
> > > > >>>>> updated
> > > > >>>>>> through Federation (or the other way around).
> > > > >>>>>>
> > > > >>>>>> Do we want to control property edits based on the origin of
> the
> > > > >>>> property
> > > > >>>>>> (spec out a "namespace" for Federated properties)?
> > > > >>>>>>
> > > > >>>>>> 3) OO types
> > > > >>>>>>
> > > > >>>>>> Currently Principals and Principal Roles are represented by
> one
> > > > >> java
> > > > >>>> type
> > > > >>>>>> each. When federation comes into play, would it make sense to
> > > > >>> develop a
> > > > >>>>>> (java) type hierarchy for working with that data in Polaris
> > code?
> > > > >> My
> > > > >>>> main
> > > > >>>>>> concern here is avoiding a maze of if/else statements
> throughout
> > > > >> the
> > > > >>>>>> Polaris codebase for supporting Federation.
> > > > >>>>>>
> > > > >>>>>> I guess our approach to this depends largely on the outcome of
> > > > >> topic
> > > > >>> 2
> > > > >>>>>> above.
> > > > >>>>>>
> > > > >>>>>> WDYT?
> > > > >>>>>>
> > > > >>>>>> Thanks,
> > > > >>>>>> Dmitri.
> > > > >>>>>>
> > > > >>>>>> [1]
> > > > >>>>>>
> > > > >>>>>>
> > > > >>
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > > > >>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > > > >>>> collado.mike@gmail.com>
> > > > >>>>>> wrote:
> > > > >>>>>>
> > > > >>>>>>> Hey folks
> > > > >>>>>>>
> > > > >>>>>>> Some of you already know that I posted an initial PR to get
> > > > >>> federated
> > > > >>>>>>> principals/roles added. One thing that came out of the
> feedback
> > > > >>> was a
> > > > >>>>>> spec
> > > > >>>>>>> change to make it clear when federated identities can be used
> > in
> > > > >>> the
> > > > >>>>>> APIs.
> > > > >>>>>>> Notably, federated principals cannot be created or updated,
> but
> > > > >> can
> > > > >>>> be
> > > > >>>>>>> returned in get/list calls, whereas federated roles *can* be
> > > > >>> created
> > > > >>>> by
> > > > >>>>>> the
> > > > >>>>>>> API. The latter is useful/necessary in order to be able to
> > assign
> > > > >>>>>>> privileges to those roles without relying on the JIT creation
> > on
> > > > >>>> login.
> > > > >>>>>>> Please check out the spec change here and let me know what
> you
> > > > >>> think
> > > > >>>> -
> > > > >>>>>>>
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > > >>>>>>> Mike
> > > > >>>>>>>
> > > > >>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > > >>>>>>> <dm...@dremio.com.invalid> wrote:
> > > > >>>>>>>
> > > > >>>>>>>> Hi Mike,
> > > > >>>>>>>>
> > > > >>>>>>>> I left some comments in the doc, but overall it looks good
> to
> > > > >> me
> > > > >>> :)
> > > > >>>>>>>> I still think there are some hidden dependencies on
> > > > >> Persistence.
> > > > >>>> For
> > > > >>>>>>>> example, whether and how we can have composite keys for
> > > > >> persisted
> > > > >>>>>>> federated
> > > > >>>>>>>> entities... but I guess we can work that out later.
> > > > >>>>>>>>
> > > > >>>>>>>> Also, I think it is important for the Authorizer API to
> avoid
> > > > >>>>> assuming
> > > > >>>>>>> that
> > > > >>>>>>>> all principals are persisted. Specific authorizer
> > > > >> implementations
> > > > >>>>>>>> (including the default one) can certainly expect persisted
> > > > >>>>> principals,
> > > > >>>>>>> but
> > > > >>>>>>>> the API should require that for the sake of flexibility of
> > > > >>> possible
> > > > >>>>>>> AuthN/Z
> > > > >>>>>>>> extensions. WDYT?
> > > > >>>>>>>>
> > > > >>>>>>>> Cheers,
> > > > >>>>>>>> Dmitri.
> > > > >>>>>>>>
> > > > >>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > > >>>>>> collado.mike@gmail.com>
> > > > >>>>>>>> wrote:
> > > > >>>>>>>>
> > > > >>>>>>>>> Hey folks
> > > > >>>>>>>>>
> > > > >>>>>>>>> As discussed during the community sync, I've put together
> > > > >> some
> > > > >>>>>> thoughts
> > > > >>>>>>>> on
> > > > >>>>>>>>> how we'd add support for federated identities in Polaris. I
> > > > >>>> copied
> > > > >>>>>> over
> > > > >>>>>>>>> some of what I had in the issue at
> > > > >>>>>>>>> https://github.com/apache/polaris/issues/441 and put it
> into
> > > > >>> the
> > > > >>>>> doc
> > > > >>>>>>>> here:
> > > > >>>>>>>>>
> > > > >>>>>>>>>
> > > > >>
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > >>>>>>>>> .
> > > > >>>>>>>>>
> > > > >>>>>>>>> Please take a look when you get some time and let me know
> > > > >> what
> > > > >>>> you
> > > > >>>>>>> think.
> > > > >>>>>>>>> Given that our next community sync is scheduled for the
> > > > >>>>> Thanksgiving
> > > > >>>>>>>>> holiday in the US, it might be useful to schedule a meeting
> > > > >>>>>>> specifically
> > > > >>>>>>>>> for this. I can schedule that sync if needed.
> > > > >>>>>>>>>
> > > > >>>>>>>>> Mike
> > > > >>>>>>>>>
> > > > --
> > > > Robert Stupp
> > > > @snazy
> > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
Hi Dmitri
I do have a revision ready to push. As of this morning (PDT), I was still
waiting for a response to this issue (I see you've responded since) -
https://github.com/apache/polaris/pull/1353#discussion_r2067815515 .
But even in the original code, there is a test asserting that federated
principals can't be created.
I think to have a discussion that moves forward, it's important that people
feel heard. If someone has comments on the code or the proposed designs,
I'm happy to have the discussion, but if neither the code nor the e-mail
discussion are being read... I don't see how this conversation can move
forward.
Mike
On Wed, Apr 30, 2025 at 5:58 PM Dmitri Bourlatchkov <di...@apache.org>
wrote:
> Hi Mike,
>
> I guess some of the confusion may come from PR 1353 [1] and this thread
> being a bit out of sync.
>
> Would you mind removing federated Principals from the API in the PR for the
> sake of clarity?
>
> From my POV, we can reconsider that if/when SCIM (or something similar)
> comes.
>
> Thanks,
> Dmitri.
>
> [1] https://github.com/apache/polaris/pull/1353
>
> On Wed, Apr 30, 2025 at 11:50 AM Michael Collado <co...@gmail.com>
> wrote:
>
> > Hi Robert
> >
> > Unsure if you read the previous emails in this thread. In my earlier
> > response, I wrote
> >
> > > 1. TBH, I have no strong feelings about persisting the federated
> > principals. I think the biggest advantage I saw was to support SCIM along
> > with 3p identity providers. But for this implementation, if we want to
> skip
> > persisting principal entities, I'm ok with it.
> >
> > I feel that should address the majority of the concern you just
> expressed.
> >
> > Storing grants with references to some kind of FQN is certainly one
> > approach, but IMO, is a massive undertaking with little benefit. For
> > example, all of the grant APIs have to be modified to deal with target
> > entities that don’t exist. How do we do validation for these APIs? Do
> these
> > APIs now need to talk to external entities to ensure the caller didn’t
> > misspell a role name or use all caps instead of mixed case? UI developers
> > can’t simply populate a list of roles for a dropdown list unless they
> also
> > connect directly to the idp. That makes it hard for someone to build a
> > generic Polaris admin UI without also building support for Okta, Google,
> > Apple, etc.
> >
> > On the other hand, SCIM is an industry standard for a reason. Every major
> > IdP supports it - and I doubt it’s because they just don’t know any
> better.
> > Supporting federated roles + SCIM means all the existing grant APIs Just
> > Work and building generic privilege management tools/UI could be done
> > today.
> >
> > Given the choice between a massive rewrite of the grant and authorization
> > primitives with no user-facing benefit and supporting federated roles
> with
> > the option of SCIM support in the future, the answer seems clear to me.
> >
> > Mike
> >
> > On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
> >
> > > Hi,
> > >
> > > thanks Mike for the effort.
> > >
> > > However, I think that persisting principals, which are not "owned" by
> > > Polaris, being persisted in Polaris is a good idea. How would changes
> to
> > > a principal in the IdP be propagated to Polaris? What happens if a
> > > principal that's deleted/disabled in the IdP - how is that reflected
> in
> > > Polaris? What would happen if two Polaris instances concurrently
> > > "transparently create" the same "federated principal entity"?
> > >
> > > Storing mirrored information that is only owned by an (external) IdP
> > > leads to a "what's the source-of-truth" problem.
> > >
> > > A principal is (mostly) an individual user or service that can access
> > > Polaris. The actual access-decision is up to the IdP (external IdP or
> > > Polaris w/ its own principals). Each principal can also be uniquely
> > > identified (IDP + principal-ID).
> > >
> > > I really do not think that principals owned by "external IdPs" should
> > > ever be persisted in Polaris.
> > >
> > > Similar for roles manages by external IdPs.
> > >
> > > ACLs (grants) are (generally speaking) lists of grantee+privilege(s)
> > > tuples. It is IMHO better to use a qualified reference to "IDP +
> > > principal/role-ID".
> > >
> > > AFAIR it's only possbible to grant privileges to roles, not individual
> > > principals. And if that's the case, I do not see a requirement to have
> > > "federated principals" being persisted in Polaris at all.
> > >
> > > I seems the underlying issue here is the very tight coupling of
> > > principals + principal-roles + catalog-roles + grants that requires a
> > > unique integer-ID for each of those and using the same
> database-oriented
> > > approach for role-membership management and catalog-entity grants.
> > >
> > > What's IMHO needed is a proper separation of those things, which should
> > > happen now and after that's done integrate external IdPs.
> > >
> > >
> > > On 22.04.25 17:00, Alex Dutra wrote:
> > > > Hi Mike,
> > > >
> > > > Thanks for the historical context, that is really helpful. And I
> > > completely
> > > > agree with your last paragraph, and especially this:
> > > >
> > > > My preference would be to make this an immutable class that has the
> > > >> principal entity and all active principal roles all in the
> > constructor.
> > > >
> > > > I was arriving at the exact same conclusion – it's a pity that
> > > > SecurityContext doesn't expose the principal roles. I guess, the
> > problem
> > > > now is how to move forward with this suggestion – I imagine a change
> > > > in AuthenticatedPolarisPrincipal or in the Resolver would create
> > > > repercussions in a lot of places in the code base as well as in
> > existing
> > > > PRs.
> > > >
> > > > Thanks,
> > > >
> > > > Alex
> > > >
> > > > On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <
> > collado.mike@gmail.com
> > > >
> > > > wrote:
> > > >
> > > >> Hey
> > > >>
> > > >> 1. I'm unsure why the Resolver was written to assume no role means
> all
> > > >> roles. But I think it has to do with
> > > >> 2. The Resolver was intended to be able to resolve principal roles,
> > > grants,
> > > >> and target entities all at one "snapshot" based on the entity and
> > grant
> > > >> versions returned by that loadEntitiesChangeTracking method. In
> order
> > > to do
> > > >> this in one shot, the authenticator couldn't resolve the principal
> > roles
> > > >> actually granted to the principal, so it didn't resolve the
> > > >> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver
> > > needed
> > > >> to verify the grants existed for the roles to be assumed by the
> > > principal
> > > >> (otherwise, a caller could just put whatever role they want in the
> > > scope).
> > > >> To avoid the lookup during authn, ALL needed to be expanded by the
> > > Resolver
> > > >> based on the grants. This was an impedance to the federated role
> > > proposal,
> > > >> which stated that the authn token is authoritative regarding which
> > roles
> > > >> are granted to the principal (e.g., using a groups claim
> > > >> <
> > > >>
> > >
> >
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> > > >>> ).
> > > >> That's why
> > > >> 3. I added the ActiveRolesProvider interface so that the default
> impl
> > > would
> > > >> continue to validate the grants held by the principal for activating
> > > >> principal roles but the federated alternative would delegate to the
> > > token
> > > >> claims (yet to be committed).
> > > >>
> > > >> Hopefully, that history helps. In short, I agree that
> > > >> SecurityIdentity.getRoles() should be the right way to determine
> > > activated
> > > >> principal roles. The Resolver should rely on the authenticator to
> > > determine
> > > >> the active set of principal roles and not rely on grant records. The
> > > >> AuthenticatedPolarisPrincipal doesn't really need to hold role names
> > > >> anymore, but the resolver does still need the list of active
> > > PrincipalRoles
> > > >> to load the appropriate catalog roles. My preference would be to
> make
> > > this
> > > >> an immutable class that has the principal entity and all active
> > > principal
> > > >> roles all in the constructor.
> > > >>
> > > >> Mike
> > > >>
> > > >> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
> > > <al...@dremio.com.invalid>
> > > >> wrote:
> > > >>
> > > >>> Hi Mike,
> > > >>>
> > > >>> I am generally fine with the spec changes.
> > > >>>
> > > >>> I have however some concerns around the way we handle principal
> roles
> > > >> today
> > > >>> in Polaris, and as I was preparing the PR with support for external
> > > IDPs
> > > >>> [1], some oddities stood out:
> > > >>>
> > > >>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to
> be
> > > >>> converted to an empty set of roles by the Authenticator; but I
> > > wonder
> > > >>> how we will be able to distinguish a token that contained no
> role
> > > >> from a
> > > >>> token that contained the pseudo-role. I added some changes to
> my
> > > PR to
> > > >>> be
> > > >>> able to distinguish the two situations, but I think the general
> > > >>> handling of
> > > >>> this pseudo-role, as well as role mapping in general, deserves
> > some
> > > >> more
> > > >>> thinking. The spec says this pseudo-role is "non-functional" –
> > but
> > > I
> > > >>> don't
> > > >>> know what this implies exactly.
> > > >>> 2. AuthenticatedPolarisPrincipal exposes principal roles and
> this
> > > is
> > > >>> imho concerning for two reasons: first, the Resolver seems to
> > > update
> > > >> the
> > > >>> principal roles during resolution, which makes the class
> mutable,
> > > >>> potentially changing the principal characteristics *after*
> > > >>> authentication.
> > > >>> And secondly, this creates an ambiguity as roles can be
> accessed
> > in
> > > >> two
> > > >>> ways: SecurityIdentity.getRoles() vs
> > > >>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
> > > would be
> > > >>> in favor of using the former consistently and completely
> removing
> > > the
> > > >>> latter. (Note: SecurityContext.isUserInRole() relies on the
> > > former.)
> > > >>> 3. There seems to be some overlap between the primary roles
> > > >> resolution,
> > > >>> done by the ActiveRolesProvider, and the role resolution done
> > > later by
> > > >>> the Resolver, cf. method
> > > resolveCallerPrincipalAndPrincipalRoles(). I
> > > >>> wonder if this second role resolution is really necessary?
> > > Ultimately,
> > > >>> the
> > > >>> source of truth for role membership will always be
> > > >>> SecurityIdentity.hasRole() or its JAX-RS variant,
> > > >>> SecurityContext.isUserInRole(). We should not deviate from
> that.
> > > >>>
> > > >>> Thanks,
> > > >>>
> > > >>> Alex
> > > >>>
> > > >>> [1] https://github.com/apache/polaris/pull/1397
> > > >>>
> > > >>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <
> > dimas@apache.org>
> > > >>> wrote:
> > > >>>
> > > >>>> Re 2: Thanks for the clarification, Mike! I guess my brain swapped
> > out
> > > >> a
> > > >>>> large portion of that doc :)
> > > >>>>
> > > >>>> I'm still not sure how IdentityToPrincipalMapping can help with
> > > >> resolving
> > > >>>> changes from API and from IdP integration.
> > > >>>>
> > > >>>> The doc talks about namespaces, but in PR# 1353, it looks like API
> > > >> calls
> > > >>>> are free to change anything in federated principal roles.
> > > >>>>
> > > >>>> Should IdentityToPrincipalMapping extend to validating changes
> > coming
> > > >>> from
> > > >>>> the Admin API?
> > > >>>>
> > > >>>> Alternatively, we could treat that as a new permission and allow
> all
> > > >>> admin
> > > >>>> users to edit federated roles via API.
> > > >>>>
> > > >>>> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd
> need
> > > to
> > > >>>> sync role membership, I guess.
> > > >>>>
> > > >>>> My understanding so far was that we'd get role membership at
> > > >>> authentication
> > > >>>> time and not persist it in Polaris.
> > > >>>>
> > > >>>> Is that still the plan?
> > > >>>>
> > > >>>> I think this relates to how we connect authentication and
> > > authorization
> > > >>> in
> > > >>>> Polaris. If Principals are external, we get role membership from
> the
> > > >>> access
> > > >>>> token and all is clear. If Principals are local, and we still get
> > > roles
> > > >>>> from the access token, then the question arises about whether that
> > > data
> > > >>> is
> > > >>>> in sync with local data (and the other way around too).
> > > >>>>
> > > >>>> That could be resolved by Polaris owning access tokens (via token
> > > >>> exchange)
> > > >>>> and assuming control over role membership based on local data and
> > > >> access
> > > >>>> time (I commented on this in the doc too).
> > > >>>>
> > > >>>> All in all, since SCIM is not part of phase 1 here, maybe we could
> > > >> defer
> > > >>>> dealing with federated Principal persistence, indeed.
> > > >>>>
> > > >>>> Re 3: I guess it also applies to client secret generation code,
> but
> > > >> let's
> > > >>>> see whether it actually causes too many conditions in code when it
> > > >> comes
> > > >>> to
> > > >>>> that.
> > > >>>>
> > > >>>> Cheers,
> > > >>>> Dmitri.
> > > >>>>
> > > >>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> > > >> collado.mike@gmail.com>
> > > >>>> wrote:
> > > >>>>
> > > >>>>> Thanks for the response.
> > > >>>>>
> > > >>>>> 1. TBH, I have no strong feelings about persisting the federated
> > > >>>>> principals. I think the biggest advantage I saw was to support
> SCIM
> > > >>> along
> > > >>>>> with 3p identity providers. But for this implementation, if we
> want
> > > >> to
> > > >>>> skip
> > > >>>>> persisting principal entities, I'm ok with it.
> > > >>>>>
> > > >>>>> 2. In the doc I wrote the following, which I think addresses the
> > > >>> concern
> > > >>>>> around conflicts.
> > > >>>>>
> > > >>>>> 1.
> > > >>>>>
> > > >>>>> The IdentityToPrincipalMapping should supply a naming
> > convention
> > > >> so
> > > >>>> that
> > > >>>>> PrincipalRoles and Principals created from that source do not
> > > >>> conflict
> > > >>>>> with
> > > >>>>> entities created by a different IdentityProvider
> > > >>>>>
> > > >>>>> In the case that a role happens to exist even with the naming
> > > >>> convention
> > > >>>>> applied, the caller would be unable to authenticate with that
> role.
> > > >>>>>
> > > >>>>> 3. I don't think we need special classes for federated roles. In
> > > >> nearly
> > > >>>> all
> > > >>>>> of the Polaris code, they are treated as PolarisRoleEntity
> without
> > > >> any
> > > >>>>> issue. The only code that needs to be aware of federated roles is
> > > >>>>>
> > > >>>>> a) The Admin API, which prevents granting these roles to
> principals
> > > >> and
> > > >>>>> b) The federated roles provider, which verifies that the
> federated
> > > >>>>> principal is assuming federated roles.
> > > >>>>>
> > > >>>>> The rest of the code (i.e., the resolver and authorizer) treat
> > these
> > > >>>> roles
> > > >>>>> exactly as native Polaris roles.
> > > >>>>>
> > > >>>>> Mike
> > > >>>>>
> > > >>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> > > >> dimas@apache.org>
> > > >>>>> wrote:
> > > >>>>>
> > > >>>>>> Thanks for reviving this discussion, Mike!
> > > >>>>>>
> > > >>>>>> The API spec change by itself LGTM, but I have related concerns
> in
> > > >>> how
> > > >>>>> this
> > > >>>>>> feature is meant to work in general.
> > > >>>>>>
> > > >>>>>> 1) The need to expose Federated Principals in Polaris API.
> > > >>>>>>
> > > >>>>>> The design doc [1] discusses the possibility to expose Federated
> > > >>>>> Principals
> > > >>>>>> in Polaris API, but there are pros and cons. I do not think it
> is
> > a
> > > >>>>> trivial
> > > >>>>>> decision. I'd like to discuss this in more detail before we
> commit
> > > >> to
> > > >>>>> doing
> > > >>>>>> that.
> > > >>>>>>
> > > >>>>>> From my POV the main benefit of exposing Federated Principals
> is
> > to
> > > >>> be
> > > >>>>> able
> > > >>>>>> to alter their properties in Polaris (as mentioned in the doc).
> > > >>>> However,
> > > >>>>>> your email indicates that this is actually forbidden.
> > > >>>>>>
> > > >>>>>> What use cases do you envision for exposing read-only Federated
> > > >>>>> Principals?
> > > >>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
> > > >>>>>>
> > > >>>>>> Let's focus on Principal Roles here (which are meant to be
> > writable
> > > >>> via
> > > >>>>>> API).
> > > >>>>>>
> > > >>>>>> If users are able to make and change (I assume) Principal Roles
> > via
> > > >>>> API,
> > > >>>>>> and federation code will also be creating and changing those
> > roles,
> > > >>>> what
> > > >>>>> is
> > > >>>>>> our approach to handling conflicts between those two streams of
> > > >>>> changes?
> > > >>>>>> Some examples for context (also mentioned in GH):
> > > >>>>>>
> > > >>>>>> A) A user creates normal Principal Role "A", and later role "A"
> is
> > > >>>>>> discovered through Federation.
> > > >>>>>>
> > > >>>>>> B) A user edits Federated Role "B" and later properties of "B"
> are
> > > >>>>> updated
> > > >>>>>> through Federation (or the other way around).
> > > >>>>>>
> > > >>>>>> Do we want to control property edits based on the origin of the
> > > >>>> property
> > > >>>>>> (spec out a "namespace" for Federated properties)?
> > > >>>>>>
> > > >>>>>> 3) OO types
> > > >>>>>>
> > > >>>>>> Currently Principals and Principal Roles are represented by one
> > > >> java
> > > >>>> type
> > > >>>>>> each. When federation comes into play, would it make sense to
> > > >>> develop a
> > > >>>>>> (java) type hierarchy for working with that data in Polaris
> code?
> > > >> My
> > > >>>> main
> > > >>>>>> concern here is avoiding a maze of if/else statements throughout
> > > >> the
> > > >>>>>> Polaris codebase for supporting Federation.
> > > >>>>>>
> > > >>>>>> I guess our approach to this depends largely on the outcome of
> > > >> topic
> > > >>> 2
> > > >>>>>> above.
> > > >>>>>>
> > > >>>>>> WDYT?
> > > >>>>>>
> > > >>>>>> Thanks,
> > > >>>>>> Dmitri.
> > > >>>>>>
> > > >>>>>> [1]
> > > >>>>>>
> > > >>>>>>
> > > >>
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > > >>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > > >>>> collado.mike@gmail.com>
> > > >>>>>> wrote:
> > > >>>>>>
> > > >>>>>>> Hey folks
> > > >>>>>>>
> > > >>>>>>> Some of you already know that I posted an initial PR to get
> > > >>> federated
> > > >>>>>>> principals/roles added. One thing that came out of the feedback
> > > >>> was a
> > > >>>>>> spec
> > > >>>>>>> change to make it clear when federated identities can be used
> in
> > > >>> the
> > > >>>>>> APIs.
> > > >>>>>>> Notably, federated principals cannot be created or updated, but
> > > >> can
> > > >>>> be
> > > >>>>>>> returned in get/list calls, whereas federated roles *can* be
> > > >>> created
> > > >>>> by
> > > >>>>>> the
> > > >>>>>>> API. The latter is useful/necessary in order to be able to
> assign
> > > >>>>>>> privileges to those roles without relying on the JIT creation
> on
> > > >>>> login.
> > > >>>>>>> Please check out the spec change here and let me know what you
> > > >>> think
> > > >>>> -
> > > >>>>>>>
> > > >>
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > >>>>>>> Mike
> > > >>>>>>>
> > > >>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > >>>>>>> <dm...@dremio.com.invalid> wrote:
> > > >>>>>>>
> > > >>>>>>>> Hi Mike,
> > > >>>>>>>>
> > > >>>>>>>> I left some comments in the doc, but overall it looks good to
> > > >> me
> > > >>> :)
> > > >>>>>>>> I still think there are some hidden dependencies on
> > > >> Persistence.
> > > >>>> For
> > > >>>>>>>> example, whether and how we can have composite keys for
> > > >> persisted
> > > >>>>>>> federated
> > > >>>>>>>> entities... but I guess we can work that out later.
> > > >>>>>>>>
> > > >>>>>>>> Also, I think it is important for the Authorizer API to avoid
> > > >>>>> assuming
> > > >>>>>>> that
> > > >>>>>>>> all principals are persisted. Specific authorizer
> > > >> implementations
> > > >>>>>>>> (including the default one) can certainly expect persisted
> > > >>>>> principals,
> > > >>>>>>> but
> > > >>>>>>>> the API should require that for the sake of flexibility of
> > > >>> possible
> > > >>>>>>> AuthN/Z
> > > >>>>>>>> extensions. WDYT?
> > > >>>>>>>>
> > > >>>>>>>> Cheers,
> > > >>>>>>>> Dmitri.
> > > >>>>>>>>
> > > >>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > >>>>>> collado.mike@gmail.com>
> > > >>>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Hey folks
> > > >>>>>>>>>
> > > >>>>>>>>> As discussed during the community sync, I've put together
> > > >> some
> > > >>>>>> thoughts
> > > >>>>>>>> on
> > > >>>>>>>>> how we'd add support for federated identities in Polaris. I
> > > >>>> copied
> > > >>>>>> over
> > > >>>>>>>>> some of what I had in the issue at
> > > >>>>>>>>> https://github.com/apache/polaris/issues/441 and put it into
> > > >>> the
> > > >>>>> doc
> > > >>>>>>>> here:
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > >>>>>>>>> .
> > > >>>>>>>>>
> > > >>>>>>>>> Please take a look when you get some time and let me know
> > > >> what
> > > >>>> you
> > > >>>>>>> think.
> > > >>>>>>>>> Given that our next community sync is scheduled for the
> > > >>>>> Thanksgiving
> > > >>>>>>>>> holiday in the US, it might be useful to schedule a meeting
> > > >>>>>>> specifically
> > > >>>>>>>>> for this. I can schedule that sync if needed.
> > > >>>>>>>>>
> > > >>>>>>>>> Mike
> > > >>>>>>>>>
> > > --
> > > Robert Stupp
> > > @snazy
> > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Dmitri Bourlatchkov <di...@apache.org>.
Hi Mike,
I guess some of the confusion may come from PR 1353 [1] and this thread
being a bit out of sync.
Would you mind removing federated Principals from the API in the PR for the
sake of clarity?
From my POV, we can reconsider that if/when SCIM (or something similar)
comes.
Thanks,
Dmitri.
[1] https://github.com/apache/polaris/pull/1353
On Wed, Apr 30, 2025 at 11:50 AM Michael Collado <co...@gmail.com>
wrote:
> Hi Robert
>
> Unsure if you read the previous emails in this thread. In my earlier
> response, I wrote
>
> > 1. TBH, I have no strong feelings about persisting the federated
> principals. I think the biggest advantage I saw was to support SCIM along
> with 3p identity providers. But for this implementation, if we want to skip
> persisting principal entities, I'm ok with it.
>
> I feel that should address the majority of the concern you just expressed.
>
> Storing grants with references to some kind of FQN is certainly one
> approach, but IMO, is a massive undertaking with little benefit. For
> example, all of the grant APIs have to be modified to deal with target
> entities that don’t exist. How do we do validation for these APIs? Do these
> APIs now need to talk to external entities to ensure the caller didn’t
> misspell a role name or use all caps instead of mixed case? UI developers
> can’t simply populate a list of roles for a dropdown list unless they also
> connect directly to the idp. That makes it hard for someone to build a
> generic Polaris admin UI without also building support for Okta, Google,
> Apple, etc.
>
> On the other hand, SCIM is an industry standard for a reason. Every major
> IdP supports it - and I doubt it’s because they just don’t know any better.
> Supporting federated roles + SCIM means all the existing grant APIs Just
> Work and building generic privilege management tools/UI could be done
> today.
>
> Given the choice between a massive rewrite of the grant and authorization
> primitives with no user-facing benefit and supporting federated roles with
> the option of SCIM support in the future, the answer seems clear to me.
>
> Mike
>
> On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
>
> > Hi,
> >
> > thanks Mike for the effort.
> >
> > However, I think that persisting principals, which are not "owned" by
> > Polaris, being persisted in Polaris is a good idea. How would changes to
> > a principal in the IdP be propagated to Polaris? What happens if a
> > principal that's deleted/disabled in the IdP - how is that reflected in
> > Polaris? What would happen if two Polaris instances concurrently
> > "transparently create" the same "federated principal entity"?
> >
> > Storing mirrored information that is only owned by an (external) IdP
> > leads to a "what's the source-of-truth" problem.
> >
> > A principal is (mostly) an individual user or service that can access
> > Polaris. The actual access-decision is up to the IdP (external IdP or
> > Polaris w/ its own principals). Each principal can also be uniquely
> > identified (IDP + principal-ID).
> >
> > I really do not think that principals owned by "external IdPs" should
> > ever be persisted in Polaris.
> >
> > Similar for roles manages by external IdPs.
> >
> > ACLs (grants) are (generally speaking) lists of grantee+privilege(s)
> > tuples. It is IMHO better to use a qualified reference to "IDP +
> > principal/role-ID".
> >
> > AFAIR it's only possbible to grant privileges to roles, not individual
> > principals. And if that's the case, I do not see a requirement to have
> > "federated principals" being persisted in Polaris at all.
> >
> > I seems the underlying issue here is the very tight coupling of
> > principals + principal-roles + catalog-roles + grants that requires a
> > unique integer-ID for each of those and using the same database-oriented
> > approach for role-membership management and catalog-entity grants.
> >
> > What's IMHO needed is a proper separation of those things, which should
> > happen now and after that's done integrate external IdPs.
> >
> >
> > On 22.04.25 17:00, Alex Dutra wrote:
> > > Hi Mike,
> > >
> > > Thanks for the historical context, that is really helpful. And I
> > completely
> > > agree with your last paragraph, and especially this:
> > >
> > > My preference would be to make this an immutable class that has the
> > >> principal entity and all active principal roles all in the
> constructor.
> > >
> > > I was arriving at the exact same conclusion – it's a pity that
> > > SecurityContext doesn't expose the principal roles. I guess, the
> problem
> > > now is how to move forward with this suggestion – I imagine a change
> > > in AuthenticatedPolarisPrincipal or in the Resolver would create
> > > repercussions in a lot of places in the code base as well as in
> existing
> > > PRs.
> > >
> > > Thanks,
> > >
> > > Alex
> > >
> > > On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <
> collado.mike@gmail.com
> > >
> > > wrote:
> > >
> > >> Hey
> > >>
> > >> 1. I'm unsure why the Resolver was written to assume no role means all
> > >> roles. But I think it has to do with
> > >> 2. The Resolver was intended to be able to resolve principal roles,
> > grants,
> > >> and target entities all at one "snapshot" based on the entity and
> grant
> > >> versions returned by that loadEntitiesChangeTracking method. In order
> > to do
> > >> this in one shot, the authenticator couldn't resolve the principal
> roles
> > >> actually granted to the principal, so it didn't resolve the
> > >> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver
> > needed
> > >> to verify the grants existed for the roles to be assumed by the
> > principal
> > >> (otherwise, a caller could just put whatever role they want in the
> > scope).
> > >> To avoid the lookup during authn, ALL needed to be expanded by the
> > Resolver
> > >> based on the grants. This was an impedance to the federated role
> > proposal,
> > >> which stated that the authn token is authoritative regarding which
> roles
> > >> are granted to the principal (e.g., using a groups claim
> > >> <
> > >>
> >
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> > >>> ).
> > >> That's why
> > >> 3. I added the ActiveRolesProvider interface so that the default impl
> > would
> > >> continue to validate the grants held by the principal for activating
> > >> principal roles but the federated alternative would delegate to the
> > token
> > >> claims (yet to be committed).
> > >>
> > >> Hopefully, that history helps. In short, I agree that
> > >> SecurityIdentity.getRoles() should be the right way to determine
> > activated
> > >> principal roles. The Resolver should rely on the authenticator to
> > determine
> > >> the active set of principal roles and not rely on grant records. The
> > >> AuthenticatedPolarisPrincipal doesn't really need to hold role names
> > >> anymore, but the resolver does still need the list of active
> > PrincipalRoles
> > >> to load the appropriate catalog roles. My preference would be to make
> > this
> > >> an immutable class that has the principal entity and all active
> > principal
> > >> roles all in the constructor.
> > >>
> > >> Mike
> > >>
> > >> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
> > <al...@dremio.com.invalid>
> > >> wrote:
> > >>
> > >>> Hi Mike,
> > >>>
> > >>> I am generally fine with the spec changes.
> > >>>
> > >>> I have however some concerns around the way we handle principal roles
> > >> today
> > >>> in Polaris, and as I was preparing the PR with support for external
> > IDPs
> > >>> [1], some oddities stood out:
> > >>>
> > >>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
> > >>> converted to an empty set of roles by the Authenticator; but I
> > wonder
> > >>> how we will be able to distinguish a token that contained no role
> > >> from a
> > >>> token that contained the pseudo-role. I added some changes to my
> > PR to
> > >>> be
> > >>> able to distinguish the two situations, but I think the general
> > >>> handling of
> > >>> this pseudo-role, as well as role mapping in general, deserves
> some
> > >> more
> > >>> thinking. The spec says this pseudo-role is "non-functional" –
> but
> > I
> > >>> don't
> > >>> know what this implies exactly.
> > >>> 2. AuthenticatedPolarisPrincipal exposes principal roles and this
> > is
> > >>> imho concerning for two reasons: first, the Resolver seems to
> > update
> > >> the
> > >>> principal roles during resolution, which makes the class mutable,
> > >>> potentially changing the principal characteristics *after*
> > >>> authentication.
> > >>> And secondly, this creates an ambiguity as roles can be accessed
> in
> > >> two
> > >>> ways: SecurityIdentity.getRoles() vs
> > >>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
> > would be
> > >>> in favor of using the former consistently and completely removing
> > the
> > >>> latter. (Note: SecurityContext.isUserInRole() relies on the
> > former.)
> > >>> 3. There seems to be some overlap between the primary roles
> > >> resolution,
> > >>> done by the ActiveRolesProvider, and the role resolution done
> > later by
> > >>> the Resolver, cf. method
> > resolveCallerPrincipalAndPrincipalRoles(). I
> > >>> wonder if this second role resolution is really necessary?
> > Ultimately,
> > >>> the
> > >>> source of truth for role membership will always be
> > >>> SecurityIdentity.hasRole() or its JAX-RS variant,
> > >>> SecurityContext.isUserInRole(). We should not deviate from that.
> > >>>
> > >>> Thanks,
> > >>>
> > >>> Alex
> > >>>
> > >>> [1] https://github.com/apache/polaris/pull/1397
> > >>>
> > >>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <
> dimas@apache.org>
> > >>> wrote:
> > >>>
> > >>>> Re 2: Thanks for the clarification, Mike! I guess my brain swapped
> out
> > >> a
> > >>>> large portion of that doc :)
> > >>>>
> > >>>> I'm still not sure how IdentityToPrincipalMapping can help with
> > >> resolving
> > >>>> changes from API and from IdP integration.
> > >>>>
> > >>>> The doc talks about namespaces, but in PR# 1353, it looks like API
> > >> calls
> > >>>> are free to change anything in federated principal roles.
> > >>>>
> > >>>> Should IdentityToPrincipalMapping extend to validating changes
> coming
> > >>> from
> > >>>> the Admin API?
> > >>>>
> > >>>> Alternatively, we could treat that as a new permission and allow all
> > >>> admin
> > >>>> users to edit federated roles via API.
> > >>>>
> > >>>> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need
> > to
> > >>>> sync role membership, I guess.
> > >>>>
> > >>>> My understanding so far was that we'd get role membership at
> > >>> authentication
> > >>>> time and not persist it in Polaris.
> > >>>>
> > >>>> Is that still the plan?
> > >>>>
> > >>>> I think this relates to how we connect authentication and
> > authorization
> > >>> in
> > >>>> Polaris. If Principals are external, we get role membership from the
> > >>> access
> > >>>> token and all is clear. If Principals are local, and we still get
> > roles
> > >>>> from the access token, then the question arises about whether that
> > data
> > >>> is
> > >>>> in sync with local data (and the other way around too).
> > >>>>
> > >>>> That could be resolved by Polaris owning access tokens (via token
> > >>> exchange)
> > >>>> and assuming control over role membership based on local data and
> > >> access
> > >>>> time (I commented on this in the doc too).
> > >>>>
> > >>>> All in all, since SCIM is not part of phase 1 here, maybe we could
> > >> defer
> > >>>> dealing with federated Principal persistence, indeed.
> > >>>>
> > >>>> Re 3: I guess it also applies to client secret generation code, but
> > >> let's
> > >>>> see whether it actually causes too many conditions in code when it
> > >> comes
> > >>> to
> > >>>> that.
> > >>>>
> > >>>> Cheers,
> > >>>> Dmitri.
> > >>>>
> > >>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> > >> collado.mike@gmail.com>
> > >>>> wrote:
> > >>>>
> > >>>>> Thanks for the response.
> > >>>>>
> > >>>>> 1. TBH, I have no strong feelings about persisting the federated
> > >>>>> principals. I think the biggest advantage I saw was to support SCIM
> > >>> along
> > >>>>> with 3p identity providers. But for this implementation, if we want
> > >> to
> > >>>> skip
> > >>>>> persisting principal entities, I'm ok with it.
> > >>>>>
> > >>>>> 2. In the doc I wrote the following, which I think addresses the
> > >>> concern
> > >>>>> around conflicts.
> > >>>>>
> > >>>>> 1.
> > >>>>>
> > >>>>> The IdentityToPrincipalMapping should supply a naming
> convention
> > >> so
> > >>>> that
> > >>>>> PrincipalRoles and Principals created from that source do not
> > >>> conflict
> > >>>>> with
> > >>>>> entities created by a different IdentityProvider
> > >>>>>
> > >>>>> In the case that a role happens to exist even with the naming
> > >>> convention
> > >>>>> applied, the caller would be unable to authenticate with that role.
> > >>>>>
> > >>>>> 3. I don't think we need special classes for federated roles. In
> > >> nearly
> > >>>> all
> > >>>>> of the Polaris code, they are treated as PolarisRoleEntity without
> > >> any
> > >>>>> issue. The only code that needs to be aware of federated roles is
> > >>>>>
> > >>>>> a) The Admin API, which prevents granting these roles to principals
> > >> and
> > >>>>> b) The federated roles provider, which verifies that the federated
> > >>>>> principal is assuming federated roles.
> > >>>>>
> > >>>>> The rest of the code (i.e., the resolver and authorizer) treat
> these
> > >>>> roles
> > >>>>> exactly as native Polaris roles.
> > >>>>>
> > >>>>> Mike
> > >>>>>
> > >>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> > >> dimas@apache.org>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Thanks for reviving this discussion, Mike!
> > >>>>>>
> > >>>>>> The API spec change by itself LGTM, but I have related concerns in
> > >>> how
> > >>>>> this
> > >>>>>> feature is meant to work in general.
> > >>>>>>
> > >>>>>> 1) The need to expose Federated Principals in Polaris API.
> > >>>>>>
> > >>>>>> The design doc [1] discusses the possibility to expose Federated
> > >>>>> Principals
> > >>>>>> in Polaris API, but there are pros and cons. I do not think it is
> a
> > >>>>> trivial
> > >>>>>> decision. I'd like to discuss this in more detail before we commit
> > >> to
> > >>>>> doing
> > >>>>>> that.
> > >>>>>>
> > >>>>>> From my POV the main benefit of exposing Federated Principals is
> to
> > >>> be
> > >>>>> able
> > >>>>>> to alter their properties in Polaris (as mentioned in the doc).
> > >>>> However,
> > >>>>>> your email indicates that this is actually forbidden.
> > >>>>>>
> > >>>>>> What use cases do you envision for exposing read-only Federated
> > >>>>> Principals?
> > >>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
> > >>>>>>
> > >>>>>> Let's focus on Principal Roles here (which are meant to be
> writable
> > >>> via
> > >>>>>> API).
> > >>>>>>
> > >>>>>> If users are able to make and change (I assume) Principal Roles
> via
> > >>>> API,
> > >>>>>> and federation code will also be creating and changing those
> roles,
> > >>>> what
> > >>>>> is
> > >>>>>> our approach to handling conflicts between those two streams of
> > >>>> changes?
> > >>>>>> Some examples for context (also mentioned in GH):
> > >>>>>>
> > >>>>>> A) A user creates normal Principal Role "A", and later role "A" is
> > >>>>>> discovered through Federation.
> > >>>>>>
> > >>>>>> B) A user edits Federated Role "B" and later properties of "B" are
> > >>>>> updated
> > >>>>>> through Federation (or the other way around).
> > >>>>>>
> > >>>>>> Do we want to control property edits based on the origin of the
> > >>>> property
> > >>>>>> (spec out a "namespace" for Federated properties)?
> > >>>>>>
> > >>>>>> 3) OO types
> > >>>>>>
> > >>>>>> Currently Principals and Principal Roles are represented by one
> > >> java
> > >>>> type
> > >>>>>> each. When federation comes into play, would it make sense to
> > >>> develop a
> > >>>>>> (java) type hierarchy for working with that data in Polaris code?
> > >> My
> > >>>> main
> > >>>>>> concern here is avoiding a maze of if/else statements throughout
> > >> the
> > >>>>>> Polaris codebase for supporting Federation.
> > >>>>>>
> > >>>>>> I guess our approach to this depends largely on the outcome of
> > >> topic
> > >>> 2
> > >>>>>> above.
> > >>>>>>
> > >>>>>> WDYT?
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Dmitri.
> > >>>>>>
> > >>>>>> [1]
> > >>>>>>
> > >>>>>>
> > >>
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > >>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > >>>> collado.mike@gmail.com>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> Hey folks
> > >>>>>>>
> > >>>>>>> Some of you already know that I posted an initial PR to get
> > >>> federated
> > >>>>>>> principals/roles added. One thing that came out of the feedback
> > >>> was a
> > >>>>>> spec
> > >>>>>>> change to make it clear when federated identities can be used in
> > >>> the
> > >>>>>> APIs.
> > >>>>>>> Notably, federated principals cannot be created or updated, but
> > >> can
> > >>>> be
> > >>>>>>> returned in get/list calls, whereas federated roles *can* be
> > >>> created
> > >>>> by
> > >>>>>> the
> > >>>>>>> API. The latter is useful/necessary in order to be able to assign
> > >>>>>>> privileges to those roles without relying on the JIT creation on
> > >>>> login.
> > >>>>>>> Please check out the spec change here and let me know what you
> > >>> think
> > >>>> -
> > >>>>>>>
> > >>
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > >>>>>>> Mike
> > >>>>>>>
> > >>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > >>>>>>> <dm...@dremio.com.invalid> wrote:
> > >>>>>>>
> > >>>>>>>> Hi Mike,
> > >>>>>>>>
> > >>>>>>>> I left some comments in the doc, but overall it looks good to
> > >> me
> > >>> :)
> > >>>>>>>> I still think there are some hidden dependencies on
> > >> Persistence.
> > >>>> For
> > >>>>>>>> example, whether and how we can have composite keys for
> > >> persisted
> > >>>>>>> federated
> > >>>>>>>> entities... but I guess we can work that out later.
> > >>>>>>>>
> > >>>>>>>> Also, I think it is important for the Authorizer API to avoid
> > >>>>> assuming
> > >>>>>>> that
> > >>>>>>>> all principals are persisted. Specific authorizer
> > >> implementations
> > >>>>>>>> (including the default one) can certainly expect persisted
> > >>>>> principals,
> > >>>>>>> but
> > >>>>>>>> the API should require that for the sake of flexibility of
> > >>> possible
> > >>>>>>> AuthN/Z
> > >>>>>>>> extensions. WDYT?
> > >>>>>>>>
> > >>>>>>>> Cheers,
> > >>>>>>>> Dmitri.
> > >>>>>>>>
> > >>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > >>>>>> collado.mike@gmail.com>
> > >>>>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>>> Hey folks
> > >>>>>>>>>
> > >>>>>>>>> As discussed during the community sync, I've put together
> > >> some
> > >>>>>> thoughts
> > >>>>>>>> on
> > >>>>>>>>> how we'd add support for federated identities in Polaris. I
> > >>>> copied
> > >>>>>> over
> > >>>>>>>>> some of what I had in the issue at
> > >>>>>>>>> https://github.com/apache/polaris/issues/441 and put it into
> > >>> the
> > >>>>> doc
> > >>>>>>>> here:
> > >>>>>>>>>
> > >>>>>>>>>
> > >>
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > >>>>>>>>> .
> > >>>>>>>>>
> > >>>>>>>>> Please take a look when you get some time and let me know
> > >> what
> > >>>> you
> > >>>>>>> think.
> > >>>>>>>>> Given that our next community sync is scheduled for the
> > >>>>> Thanksgiving
> > >>>>>>>>> holiday in the US, it might be useful to schedule a meeting
> > >>>>>>> specifically
> > >>>>>>>>> for this. I can schedule that sync if needed.
> > >>>>>>>>>
> > >>>>>>>>> Mike
> > >>>>>>>>>
> > --
> > Robert Stupp
> > @snazy
> >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
Hi Robert
Unsure if you read the previous emails in this thread. In my earlier
response, I wrote
> 1. TBH, I have no strong feelings about persisting the federated
principals. I think the biggest advantage I saw was to support SCIM along
with 3p identity providers. But for this implementation, if we want to skip
persisting principal entities, I'm ok with it.
I feel that should address the majority of the concern you just expressed.
Storing grants with references to some kind of FQN is certainly one
approach, but IMO, is a massive undertaking with little benefit. For
example, all of the grant APIs have to be modified to deal with target
entities that don’t exist. How do we do validation for these APIs? Do these
APIs now need to talk to external entities to ensure the caller didn’t
misspell a role name or use all caps instead of mixed case? UI developers
can’t simply populate a list of roles for a dropdown list unless they also
connect directly to the idp. That makes it hard for someone to build a
generic Polaris admin UI without also building support for Okta, Google,
Apple, etc.
On the other hand, SCIM is an industry standard for a reason. Every major
IdP supports it - and I doubt it’s because they just don’t know any better.
Supporting federated roles + SCIM means all the existing grant APIs Just
Work and building generic privilege management tools/UI could be done
today.
Given the choice between a massive rewrite of the grant and authorization
primitives with no user-facing benefit and supporting federated roles with
the option of SCIM support in the future, the answer seems clear to me.
Mike
On Wed, Apr 30, 2025 at 7:04 AM Robert Stupp <sn...@snazy.de> wrote:
> Hi,
>
> thanks Mike for the effort.
>
> However, I think that persisting principals, which are not "owned" by
> Polaris, being persisted in Polaris is a good idea. How would changes to
> a principal in the IdP be propagated to Polaris? What happens if a
> principal that's deleted/disabled in the IdP - how is that reflected in
> Polaris? What would happen if two Polaris instances concurrently
> "transparently create" the same "federated principal entity"?
>
> Storing mirrored information that is only owned by an (external) IdP
> leads to a "what's the source-of-truth" problem.
>
> A principal is (mostly) an individual user or service that can access
> Polaris. The actual access-decision is up to the IdP (external IdP or
> Polaris w/ its own principals). Each principal can also be uniquely
> identified (IDP + principal-ID).
>
> I really do not think that principals owned by "external IdPs" should
> ever be persisted in Polaris.
>
> Similar for roles manages by external IdPs.
>
> ACLs (grants) are (generally speaking) lists of grantee+privilege(s)
> tuples. It is IMHO better to use a qualified reference to "IDP +
> principal/role-ID".
>
> AFAIR it's only possbible to grant privileges to roles, not individual
> principals. And if that's the case, I do not see a requirement to have
> "federated principals" being persisted in Polaris at all.
>
> I seems the underlying issue here is the very tight coupling of
> principals + principal-roles + catalog-roles + grants that requires a
> unique integer-ID for each of those and using the same database-oriented
> approach for role-membership management and catalog-entity grants.
>
> What's IMHO needed is a proper separation of those things, which should
> happen now and after that's done integrate external IdPs.
>
>
> On 22.04.25 17:00, Alex Dutra wrote:
> > Hi Mike,
> >
> > Thanks for the historical context, that is really helpful. And I
> completely
> > agree with your last paragraph, and especially this:
> >
> > My preference would be to make this an immutable class that has the
> >> principal entity and all active principal roles all in the constructor.
> >
> > I was arriving at the exact same conclusion – it's a pity that
> > SecurityContext doesn't expose the principal roles. I guess, the problem
> > now is how to move forward with this suggestion – I imagine a change
> > in AuthenticatedPolarisPrincipal or in the Resolver would create
> > repercussions in a lot of places in the code base as well as in existing
> > PRs.
> >
> > Thanks,
> >
> > Alex
> >
> > On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <collado.mike@gmail.com
> >
> > wrote:
> >
> >> Hey
> >>
> >> 1. I'm unsure why the Resolver was written to assume no role means all
> >> roles. But I think it has to do with
> >> 2. The Resolver was intended to be able to resolve principal roles,
> grants,
> >> and target entities all at one "snapshot" based on the entity and grant
> >> versions returned by that loadEntitiesChangeTracking method. In order
> to do
> >> this in one shot, the authenticator couldn't resolve the principal roles
> >> actually granted to the principal, so it didn't resolve the
> >> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver
> needed
> >> to verify the grants existed for the roles to be assumed by the
> principal
> >> (otherwise, a caller could just put whatever role they want in the
> scope).
> >> To avoid the lookup during authn, ALL needed to be expanded by the
> Resolver
> >> based on the grants. This was an impedance to the federated role
> proposal,
> >> which stated that the authn token is authoritative regarding which roles
> >> are granted to the principal (e.g., using a groups claim
> >> <
> >>
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> >>> ).
> >> That's why
> >> 3. I added the ActiveRolesProvider interface so that the default impl
> would
> >> continue to validate the grants held by the principal for activating
> >> principal roles but the federated alternative would delegate to the
> token
> >> claims (yet to be committed).
> >>
> >> Hopefully, that history helps. In short, I agree that
> >> SecurityIdentity.getRoles() should be the right way to determine
> activated
> >> principal roles. The Resolver should rely on the authenticator to
> determine
> >> the active set of principal roles and not rely on grant records. The
> >> AuthenticatedPolarisPrincipal doesn't really need to hold role names
> >> anymore, but the resolver does still need the list of active
> PrincipalRoles
> >> to load the appropriate catalog roles. My preference would be to make
> this
> >> an immutable class that has the principal entity and all active
> principal
> >> roles all in the constructor.
> >>
> >> Mike
> >>
> >> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra
> <al...@dremio.com.invalid>
> >> wrote:
> >>
> >>> Hi Mike,
> >>>
> >>> I am generally fine with the spec changes.
> >>>
> >>> I have however some concerns around the way we handle principal roles
> >> today
> >>> in Polaris, and as I was preparing the PR with support for external
> IDPs
> >>> [1], some oddities stood out:
> >>>
> >>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
> >>> converted to an empty set of roles by the Authenticator; but I
> wonder
> >>> how we will be able to distinguish a token that contained no role
> >> from a
> >>> token that contained the pseudo-role. I added some changes to my
> PR to
> >>> be
> >>> able to distinguish the two situations, but I think the general
> >>> handling of
> >>> this pseudo-role, as well as role mapping in general, deserves some
> >> more
> >>> thinking. The spec says this pseudo-role is "non-functional" – but
> I
> >>> don't
> >>> know what this implies exactly.
> >>> 2. AuthenticatedPolarisPrincipal exposes principal roles and this
> is
> >>> imho concerning for two reasons: first, the Resolver seems to
> update
> >> the
> >>> principal roles during resolution, which makes the class mutable,
> >>> potentially changing the principal characteristics *after*
> >>> authentication.
> >>> And secondly, this creates an ambiguity as roles can be accessed in
> >> two
> >>> ways: SecurityIdentity.getRoles() vs
> >>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I
> would be
> >>> in favor of using the former consistently and completely removing
> the
> >>> latter. (Note: SecurityContext.isUserInRole() relies on the
> former.)
> >>> 3. There seems to be some overlap between the primary roles
> >> resolution,
> >>> done by the ActiveRolesProvider, and the role resolution done
> later by
> >>> the Resolver, cf. method
> resolveCallerPrincipalAndPrincipalRoles(). I
> >>> wonder if this second role resolution is really necessary?
> Ultimately,
> >>> the
> >>> source of truth for role membership will always be
> >>> SecurityIdentity.hasRole() or its JAX-RS variant,
> >>> SecurityContext.isUserInRole(). We should not deviate from that.
> >>>
> >>> Thanks,
> >>>
> >>> Alex
> >>>
> >>> [1] https://github.com/apache/polaris/pull/1397
> >>>
> >>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <di...@apache.org>
> >>> wrote:
> >>>
> >>>> Re 2: Thanks for the clarification, Mike! I guess my brain swapped out
> >> a
> >>>> large portion of that doc :)
> >>>>
> >>>> I'm still not sure how IdentityToPrincipalMapping can help with
> >> resolving
> >>>> changes from API and from IdP integration.
> >>>>
> >>>> The doc talks about namespaces, but in PR# 1353, it looks like API
> >> calls
> >>>> are free to change anything in federated principal roles.
> >>>>
> >>>> Should IdentityToPrincipalMapping extend to validating changes coming
> >>> from
> >>>> the Admin API?
> >>>>
> >>>> Alternatively, we could treat that as a new permission and allow all
> >>> admin
> >>>> users to edit federated roles via API.
> >>>>
> >>>> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need
> to
> >>>> sync role membership, I guess.
> >>>>
> >>>> My understanding so far was that we'd get role membership at
> >>> authentication
> >>>> time and not persist it in Polaris.
> >>>>
> >>>> Is that still the plan?
> >>>>
> >>>> I think this relates to how we connect authentication and
> authorization
> >>> in
> >>>> Polaris. If Principals are external, we get role membership from the
> >>> access
> >>>> token and all is clear. If Principals are local, and we still get
> roles
> >>>> from the access token, then the question arises about whether that
> data
> >>> is
> >>>> in sync with local data (and the other way around too).
> >>>>
> >>>> That could be resolved by Polaris owning access tokens (via token
> >>> exchange)
> >>>> and assuming control over role membership based on local data and
> >> access
> >>>> time (I commented on this in the doc too).
> >>>>
> >>>> All in all, since SCIM is not part of phase 1 here, maybe we could
> >> defer
> >>>> dealing with federated Principal persistence, indeed.
> >>>>
> >>>> Re 3: I guess it also applies to client secret generation code, but
> >> let's
> >>>> see whether it actually causes too many conditions in code when it
> >> comes
> >>> to
> >>>> that.
> >>>>
> >>>> Cheers,
> >>>> Dmitri.
> >>>>
> >>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> >> collado.mike@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> Thanks for the response.
> >>>>>
> >>>>> 1. TBH, I have no strong feelings about persisting the federated
> >>>>> principals. I think the biggest advantage I saw was to support SCIM
> >>> along
> >>>>> with 3p identity providers. But for this implementation, if we want
> >> to
> >>>> skip
> >>>>> persisting principal entities, I'm ok with it.
> >>>>>
> >>>>> 2. In the doc I wrote the following, which I think addresses the
> >>> concern
> >>>>> around conflicts.
> >>>>>
> >>>>> 1.
> >>>>>
> >>>>> The IdentityToPrincipalMapping should supply a naming convention
> >> so
> >>>> that
> >>>>> PrincipalRoles and Principals created from that source do not
> >>> conflict
> >>>>> with
> >>>>> entities created by a different IdentityProvider
> >>>>>
> >>>>> In the case that a role happens to exist even with the naming
> >>> convention
> >>>>> applied, the caller would be unable to authenticate with that role.
> >>>>>
> >>>>> 3. I don't think we need special classes for federated roles. In
> >> nearly
> >>>> all
> >>>>> of the Polaris code, they are treated as PolarisRoleEntity without
> >> any
> >>>>> issue. The only code that needs to be aware of federated roles is
> >>>>>
> >>>>> a) The Admin API, which prevents granting these roles to principals
> >> and
> >>>>> b) The federated roles provider, which verifies that the federated
> >>>>> principal is assuming federated roles.
> >>>>>
> >>>>> The rest of the code (i.e., the resolver and authorizer) treat these
> >>>> roles
> >>>>> exactly as native Polaris roles.
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> >> dimas@apache.org>
> >>>>> wrote:
> >>>>>
> >>>>>> Thanks for reviving this discussion, Mike!
> >>>>>>
> >>>>>> The API spec change by itself LGTM, but I have related concerns in
> >>> how
> >>>>> this
> >>>>>> feature is meant to work in general.
> >>>>>>
> >>>>>> 1) The need to expose Federated Principals in Polaris API.
> >>>>>>
> >>>>>> The design doc [1] discusses the possibility to expose Federated
> >>>>> Principals
> >>>>>> in Polaris API, but there are pros and cons. I do not think it is a
> >>>>> trivial
> >>>>>> decision. I'd like to discuss this in more detail before we commit
> >> to
> >>>>> doing
> >>>>>> that.
> >>>>>>
> >>>>>> From my POV the main benefit of exposing Federated Principals is to
> >>> be
> >>>>> able
> >>>>>> to alter their properties in Polaris (as mentioned in the doc).
> >>>> However,
> >>>>>> your email indicates that this is actually forbidden.
> >>>>>>
> >>>>>> What use cases do you envision for exposing read-only Federated
> >>>>> Principals?
> >>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
> >>>>>>
> >>>>>> Let's focus on Principal Roles here (which are meant to be writable
> >>> via
> >>>>>> API).
> >>>>>>
> >>>>>> If users are able to make and change (I assume) Principal Roles via
> >>>> API,
> >>>>>> and federation code will also be creating and changing those roles,
> >>>> what
> >>>>> is
> >>>>>> our approach to handling conflicts between those two streams of
> >>>> changes?
> >>>>>> Some examples for context (also mentioned in GH):
> >>>>>>
> >>>>>> A) A user creates normal Principal Role "A", and later role "A" is
> >>>>>> discovered through Federation.
> >>>>>>
> >>>>>> B) A user edits Federated Role "B" and later properties of "B" are
> >>>>> updated
> >>>>>> through Federation (or the other way around).
> >>>>>>
> >>>>>> Do we want to control property edits based on the origin of the
> >>>> property
> >>>>>> (spec out a "namespace" for Federated properties)?
> >>>>>>
> >>>>>> 3) OO types
> >>>>>>
> >>>>>> Currently Principals and Principal Roles are represented by one
> >> java
> >>>> type
> >>>>>> each. When federation comes into play, would it make sense to
> >>> develop a
> >>>>>> (java) type hierarchy for working with that data in Polaris code?
> >> My
> >>>> main
> >>>>>> concern here is avoiding a maze of if/else statements throughout
> >> the
> >>>>>> Polaris codebase for supporting Federation.
> >>>>>>
> >>>>>> I guess our approach to this depends largely on the outcome of
> >> topic
> >>> 2
> >>>>>> above.
> >>>>>>
> >>>>>> WDYT?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Dmitri.
> >>>>>>
> >>>>>> [1]
> >>>>>>
> >>>>>>
> >>
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> >>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> >>>> collado.mike@gmail.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hey folks
> >>>>>>>
> >>>>>>> Some of you already know that I posted an initial PR to get
> >>> federated
> >>>>>>> principals/roles added. One thing that came out of the feedback
> >>> was a
> >>>>>> spec
> >>>>>>> change to make it clear when federated identities can be used in
> >>> the
> >>>>>> APIs.
> >>>>>>> Notably, federated principals cannot be created or updated, but
> >> can
> >>>> be
> >>>>>>> returned in get/list calls, whereas federated roles *can* be
> >>> created
> >>>> by
> >>>>>> the
> >>>>>>> API. The latter is useful/necessary in order to be able to assign
> >>>>>>> privileges to those roles without relying on the JIT creation on
> >>>> login.
> >>>>>>> Please check out the spec change here and let me know what you
> >>> think
> >>>> -
> >>>>>>>
> >>
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> >>>>>>> Mike
> >>>>>>>
> >>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> >>>>>>> <dm...@dremio.com.invalid> wrote:
> >>>>>>>
> >>>>>>>> Hi Mike,
> >>>>>>>>
> >>>>>>>> I left some comments in the doc, but overall it looks good to
> >> me
> >>> :)
> >>>>>>>> I still think there are some hidden dependencies on
> >> Persistence.
> >>>> For
> >>>>>>>> example, whether and how we can have composite keys for
> >> persisted
> >>>>>>> federated
> >>>>>>>> entities... but I guess we can work that out later.
> >>>>>>>>
> >>>>>>>> Also, I think it is important for the Authorizer API to avoid
> >>>>> assuming
> >>>>>>> that
> >>>>>>>> all principals are persisted. Specific authorizer
> >> implementations
> >>>>>>>> (including the default one) can certainly expect persisted
> >>>>> principals,
> >>>>>>> but
> >>>>>>>> the API should require that for the sake of flexibility of
> >>> possible
> >>>>>>> AuthN/Z
> >>>>>>>> extensions. WDYT?
> >>>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>> Dmitri.
> >>>>>>>>
> >>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> >>>>>> collado.mike@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Hey folks
> >>>>>>>>>
> >>>>>>>>> As discussed during the community sync, I've put together
> >> some
> >>>>>> thoughts
> >>>>>>>> on
> >>>>>>>>> how we'd add support for federated identities in Polaris. I
> >>>> copied
> >>>>>> over
> >>>>>>>>> some of what I had in the issue at
> >>>>>>>>> https://github.com/apache/polaris/issues/441 and put it into
> >>> the
> >>>>> doc
> >>>>>>>> here:
> >>>>>>>>>
> >>>>>>>>>
> >>
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> >>>>>>>>> .
> >>>>>>>>>
> >>>>>>>>> Please take a look when you get some time and let me know
> >> what
> >>>> you
> >>>>>>> think.
> >>>>>>>>> Given that our next community sync is scheduled for the
> >>>>> Thanksgiving
> >>>>>>>>> holiday in the US, it might be useful to schedule a meeting
> >>>>>>> specifically
> >>>>>>>>> for this. I can schedule that sync if needed.
> >>>>>>>>>
> >>>>>>>>> Mike
> >>>>>>>>>
> --
> Robert Stupp
> @snazy
>
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Robert Stupp <sn...@snazy.de>.
Hi,
thanks Mike for the effort.
However, I think that persisting principals, which are not "owned" by
Polaris, being persisted in Polaris is a good idea. How would changes to
a principal in the IdP be propagated to Polaris? What happens if a
principal that's deleted/disabled in the IdP - how is that reflected in
Polaris? What would happen if two Polaris instances concurrently
"transparently create" the same "federated principal entity"?
Storing mirrored information that is only owned by an (external) IdP
leads to a "what's the source-of-truth" problem.
A principal is (mostly) an individual user or service that can access
Polaris. The actual access-decision is up to the IdP (external IdP or
Polaris w/ its own principals). Each principal can also be uniquely
identified (IDP + principal-ID).
I really do not think that principals owned by "external IdPs" should
ever be persisted in Polaris.
Similar for roles manages by external IdPs.
ACLs (grants) are (generally speaking) lists of grantee+privilege(s)
tuples. It is IMHO better to use a qualified reference to "IDP +
principal/role-ID".
AFAIR it's only possbible to grant privileges to roles, not individual
principals. And if that's the case, I do not see a requirement to have
"federated principals" being persisted in Polaris at all.
I seems the underlying issue here is the very tight coupling of
principals + principal-roles + catalog-roles + grants that requires a
unique integer-ID for each of those and using the same database-oriented
approach for role-membership management and catalog-entity grants.
What's IMHO needed is a proper separation of those things, which should
happen now and after that's done integrate external IdPs.
On 22.04.25 17:00, Alex Dutra wrote:
> Hi Mike,
>
> Thanks for the historical context, that is really helpful. And I completely
> agree with your last paragraph, and especially this:
>
> My preference would be to make this an immutable class that has the
>> principal entity and all active principal roles all in the constructor.
>
> I was arriving at the exact same conclusion – it's a pity that
> SecurityContext doesn't expose the principal roles. I guess, the problem
> now is how to move forward with this suggestion – I imagine a change
> in AuthenticatedPolarisPrincipal or in the Resolver would create
> repercussions in a lot of places in the code base as well as in existing
> PRs.
>
> Thanks,
>
> Alex
>
> On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <co...@gmail.com>
> wrote:
>
>> Hey
>>
>> 1. I'm unsure why the Resolver was written to assume no role means all
>> roles. But I think it has to do with
>> 2. The Resolver was intended to be able to resolve principal roles, grants,
>> and target entities all at one "snapshot" based on the entity and grant
>> versions returned by that loadEntitiesChangeTracking method. In order to do
>> this in one shot, the authenticator couldn't resolve the principal roles
>> actually granted to the principal, so it didn't resolve the
>> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver needed
>> to verify the grants existed for the roles to be assumed by the principal
>> (otherwise, a caller could just put whatever role they want in the scope).
>> To avoid the lookup during authn, ALL needed to be expanded by the Resolver
>> based on the grants. This was an impedance to the federated role proposal,
>> which stated that the authn token is authoritative regarding which roles
>> are granted to the principal (e.g., using a groups claim
>> <
>> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
>>> ).
>> That's why
>> 3. I added the ActiveRolesProvider interface so that the default impl would
>> continue to validate the grants held by the principal for activating
>> principal roles but the federated alternative would delegate to the token
>> claims (yet to be committed).
>>
>> Hopefully, that history helps. In short, I agree that
>> SecurityIdentity.getRoles() should be the right way to determine activated
>> principal roles. The Resolver should rely on the authenticator to determine
>> the active set of principal roles and not rely on grant records. The
>> AuthenticatedPolarisPrincipal doesn't really need to hold role names
>> anymore, but the resolver does still need the list of active PrincipalRoles
>> to load the appropriate catalog roles. My preference would be to make this
>> an immutable class that has the principal entity and all active principal
>> roles all in the constructor.
>>
>> Mike
>>
>> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra <al...@dremio.com.invalid>
>> wrote:
>>
>>> Hi Mike,
>>>
>>> I am generally fine with the spec changes.
>>>
>>> I have however some concerns around the way we handle principal roles
>> today
>>> in Polaris, and as I was preparing the PR with support for external IDPs
>>> [1], some oddities stood out:
>>>
>>> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
>>> converted to an empty set of roles by the Authenticator; but I wonder
>>> how we will be able to distinguish a token that contained no role
>> from a
>>> token that contained the pseudo-role. I added some changes to my PR to
>>> be
>>> able to distinguish the two situations, but I think the general
>>> handling of
>>> this pseudo-role, as well as role mapping in general, deserves some
>> more
>>> thinking. The spec says this pseudo-role is "non-functional" – but I
>>> don't
>>> know what this implies exactly.
>>> 2. AuthenticatedPolarisPrincipal exposes principal roles and this is
>>> imho concerning for two reasons: first, the Resolver seems to update
>> the
>>> principal roles during resolution, which makes the class mutable,
>>> potentially changing the principal characteristics *after*
>>> authentication.
>>> And secondly, this creates an ambiguity as roles can be accessed in
>> two
>>> ways: SecurityIdentity.getRoles() vs
>>> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I would be
>>> in favor of using the former consistently and completely removing the
>>> latter. (Note: SecurityContext.isUserInRole() relies on the former.)
>>> 3. There seems to be some overlap between the primary roles
>> resolution,
>>> done by the ActiveRolesProvider, and the role resolution done later by
>>> the Resolver, cf. method resolveCallerPrincipalAndPrincipalRoles(). I
>>> wonder if this second role resolution is really necessary? Ultimately,
>>> the
>>> source of truth for role membership will always be
>>> SecurityIdentity.hasRole() or its JAX-RS variant,
>>> SecurityContext.isUserInRole(). We should not deviate from that.
>>>
>>> Thanks,
>>>
>>> Alex
>>>
>>> [1] https://github.com/apache/polaris/pull/1397
>>>
>>> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <di...@apache.org>
>>> wrote:
>>>
>>>> Re 2: Thanks for the clarification, Mike! I guess my brain swapped out
>> a
>>>> large portion of that doc :)
>>>>
>>>> I'm still not sure how IdentityToPrincipalMapping can help with
>> resolving
>>>> changes from API and from IdP integration.
>>>>
>>>> The doc talks about namespaces, but in PR# 1353, it looks like API
>> calls
>>>> are free to change anything in federated principal roles.
>>>>
>>>> Should IdentityToPrincipalMapping extend to validating changes coming
>>> from
>>>> the Admin API?
>>>>
>>>> Alternatively, we could treat that as a new permission and allow all
>>> admin
>>>> users to edit federated roles via API.
>>>>
>>>> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need to
>>>> sync role membership, I guess.
>>>>
>>>> My understanding so far was that we'd get role membership at
>>> authentication
>>>> time and not persist it in Polaris.
>>>>
>>>> Is that still the plan?
>>>>
>>>> I think this relates to how we connect authentication and authorization
>>> in
>>>> Polaris. If Principals are external, we get role membership from the
>>> access
>>>> token and all is clear. If Principals are local, and we still get roles
>>>> from the access token, then the question arises about whether that data
>>> is
>>>> in sync with local data (and the other way around too).
>>>>
>>>> That could be resolved by Polaris owning access tokens (via token
>>> exchange)
>>>> and assuming control over role membership based on local data and
>> access
>>>> time (I commented on this in the doc too).
>>>>
>>>> All in all, since SCIM is not part of phase 1 here, maybe we could
>> defer
>>>> dealing with federated Principal persistence, indeed.
>>>>
>>>> Re 3: I guess it also applies to client secret generation code, but
>> let's
>>>> see whether it actually causes too many conditions in code when it
>> comes
>>> to
>>>> that.
>>>>
>>>> Cheers,
>>>> Dmitri.
>>>>
>>>> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
>> collado.mike@gmail.com>
>>>> wrote:
>>>>
>>>>> Thanks for the response.
>>>>>
>>>>> 1. TBH, I have no strong feelings about persisting the federated
>>>>> principals. I think the biggest advantage I saw was to support SCIM
>>> along
>>>>> with 3p identity providers. But for this implementation, if we want
>> to
>>>> skip
>>>>> persisting principal entities, I'm ok with it.
>>>>>
>>>>> 2. In the doc I wrote the following, which I think addresses the
>>> concern
>>>>> around conflicts.
>>>>>
>>>>> 1.
>>>>>
>>>>> The IdentityToPrincipalMapping should supply a naming convention
>> so
>>>> that
>>>>> PrincipalRoles and Principals created from that source do not
>>> conflict
>>>>> with
>>>>> entities created by a different IdentityProvider
>>>>>
>>>>> In the case that a role happens to exist even with the naming
>>> convention
>>>>> applied, the caller would be unable to authenticate with that role.
>>>>>
>>>>> 3. I don't think we need special classes for federated roles. In
>> nearly
>>>> all
>>>>> of the Polaris code, they are treated as PolarisRoleEntity without
>> any
>>>>> issue. The only code that needs to be aware of federated roles is
>>>>>
>>>>> a) The Admin API, which prevents granting these roles to principals
>> and
>>>>> b) The federated roles provider, which verifies that the federated
>>>>> principal is assuming federated roles.
>>>>>
>>>>> The rest of the code (i.e., the resolver and authorizer) treat these
>>>> roles
>>>>> exactly as native Polaris roles.
>>>>>
>>>>> Mike
>>>>>
>>>>> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
>> dimas@apache.org>
>>>>> wrote:
>>>>>
>>>>>> Thanks for reviving this discussion, Mike!
>>>>>>
>>>>>> The API spec change by itself LGTM, but I have related concerns in
>>> how
>>>>> this
>>>>>> feature is meant to work in general.
>>>>>>
>>>>>> 1) The need to expose Federated Principals in Polaris API.
>>>>>>
>>>>>> The design doc [1] discusses the possibility to expose Federated
>>>>> Principals
>>>>>> in Polaris API, but there are pros and cons. I do not think it is a
>>>>> trivial
>>>>>> decision. I'd like to discuss this in more detail before we commit
>> to
>>>>> doing
>>>>>> that.
>>>>>>
>>>>>> From my POV the main benefit of exposing Federated Principals is to
>>> be
>>>>> able
>>>>>> to alter their properties in Polaris (as mentioned in the doc).
>>>> However,
>>>>>> your email indicates that this is actually forbidden.
>>>>>>
>>>>>> What use cases do you envision for exposing read-only Federated
>>>>> Principals?
>>>>>> 2) Reconciliation of Federated vs. Non-Federated changes.
>>>>>>
>>>>>> Let's focus on Principal Roles here (which are meant to be writable
>>> via
>>>>>> API).
>>>>>>
>>>>>> If users are able to make and change (I assume) Principal Roles via
>>>> API,
>>>>>> and federation code will also be creating and changing those roles,
>>>> what
>>>>> is
>>>>>> our approach to handling conflicts between those two streams of
>>>> changes?
>>>>>> Some examples for context (also mentioned in GH):
>>>>>>
>>>>>> A) A user creates normal Principal Role "A", and later role "A" is
>>>>>> discovered through Federation.
>>>>>>
>>>>>> B) A user edits Federated Role "B" and later properties of "B" are
>>>>> updated
>>>>>> through Federation (or the other way around).
>>>>>>
>>>>>> Do we want to control property edits based on the origin of the
>>>> property
>>>>>> (spec out a "namespace" for Federated properties)?
>>>>>>
>>>>>> 3) OO types
>>>>>>
>>>>>> Currently Principals and Principal Roles are represented by one
>> java
>>>> type
>>>>>> each. When federation comes into play, would it make sense to
>>> develop a
>>>>>> (java) type hierarchy for working with that data in Polaris code?
>> My
>>>> main
>>>>>> concern here is avoiding a maze of if/else statements throughout
>> the
>>>>>> Polaris codebase for supporting Federation.
>>>>>>
>>>>>> I guess our approach to this depends largely on the outcome of
>> topic
>>> 2
>>>>>> above.
>>>>>>
>>>>>> WDYT?
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitri.
>>>>>>
>>>>>> [1]
>>>>>>
>>>>>>
>> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
>>>>>> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
>>>> collado.mike@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hey folks
>>>>>>>
>>>>>>> Some of you already know that I posted an initial PR to get
>>> federated
>>>>>>> principals/roles added. One thing that came out of the feedback
>>> was a
>>>>>> spec
>>>>>>> change to make it clear when federated identities can be used in
>>> the
>>>>>> APIs.
>>>>>>> Notably, federated principals cannot be created or updated, but
>> can
>>>> be
>>>>>>> returned in get/list calls, whereas federated roles *can* be
>>> created
>>>> by
>>>>>> the
>>>>>>> API. The latter is useful/necessary in order to be able to assign
>>>>>>> privileges to those roles without relying on the JIT creation on
>>>> login.
>>>>>>> Please check out the spec change here and let me know what you
>>> think
>>>> -
>>>>>>>
>> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
>>>>>>> Mike
>>>>>>>
>>>>>>> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
>>>>>>> <dm...@dremio.com.invalid> wrote:
>>>>>>>
>>>>>>>> Hi Mike,
>>>>>>>>
>>>>>>>> I left some comments in the doc, but overall it looks good to
>> me
>>> :)
>>>>>>>> I still think there are some hidden dependencies on
>> Persistence.
>>>> For
>>>>>>>> example, whether and how we can have composite keys for
>> persisted
>>>>>>> federated
>>>>>>>> entities... but I guess we can work that out later.
>>>>>>>>
>>>>>>>> Also, I think it is important for the Authorizer API to avoid
>>>>> assuming
>>>>>>> that
>>>>>>>> all principals are persisted. Specific authorizer
>> implementations
>>>>>>>> (including the default one) can certainly expect persisted
>>>>> principals,
>>>>>>> but
>>>>>>>> the API should require that for the sake of flexibility of
>>> possible
>>>>>>> AuthN/Z
>>>>>>>> extensions. WDYT?
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Dmitri.
>>>>>>>>
>>>>>>>> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
>>>>>> collado.mike@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hey folks
>>>>>>>>>
>>>>>>>>> As discussed during the community sync, I've put together
>> some
>>>>>> thoughts
>>>>>>>> on
>>>>>>>>> how we'd add support for federated identities in Polaris. I
>>>> copied
>>>>>> over
>>>>>>>>> some of what I had in the issue at
>>>>>>>>> https://github.com/apache/polaris/issues/441 and put it into
>>> the
>>>>> doc
>>>>>>>> here:
>>>>>>>>>
>>>>>>>>>
>> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
>>>>>>>>> .
>>>>>>>>>
>>>>>>>>> Please take a look when you get some time and let me know
>> what
>>>> you
>>>>>>> think.
>>>>>>>>> Given that our next community sync is scheduled for the
>>>>> Thanksgiving
>>>>>>>>> holiday in the US, it might be useful to schedule a meeting
>>>>>>> specifically
>>>>>>>>> for this. I can schedule that sync if needed.
>>>>>>>>>
>>>>>>>>> Mike
>>>>>>>>>
--
Robert Stupp
@snazy
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Alex Dutra <al...@dremio.com.INVALID>.
Hi Mike,
Thanks for the historical context, that is really helpful. And I completely
agree with your last paragraph, and especially this:
My preference would be to make this an immutable class that has the
> principal entity and all active principal roles all in the constructor.
I was arriving at the exact same conclusion – it's a pity that
SecurityContext doesn't expose the principal roles. I guess, the problem
now is how to move forward with this suggestion – I imagine a change
in AuthenticatedPolarisPrincipal or in the Resolver would create
repercussions in a lot of places in the code base as well as in existing
PRs.
Thanks,
Alex
On Mon, Apr 21, 2025 at 10:30 PM Michael Collado <co...@gmail.com>
wrote:
> Hey
>
> 1. I'm unsure why the Resolver was written to assume no role means all
> roles. But I think it has to do with
> 2. The Resolver was intended to be able to resolve principal roles, grants,
> and target entities all at one "snapshot" based on the entity and grant
> versions returned by that loadEntitiesChangeTracking method. In order to do
> this in one shot, the authenticator couldn't resolve the principal roles
> actually granted to the principal, so it didn't resolve the
> PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver needed
> to verify the grants existed for the roles to be assumed by the principal
> (otherwise, a caller could just put whatever role they want in the scope).
> To avoid the lookup during authn, ALL needed to be expanded by the Resolver
> based on the grants. This was an impedance to the federated role proposal,
> which stated that the authn token is authoritative regarding which roles
> are granted to the principal (e.g., using a groups claim
> <
> https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/
> >).
> That's why
> 3. I added the ActiveRolesProvider interface so that the default impl would
> continue to validate the grants held by the principal for activating
> principal roles but the federated alternative would delegate to the token
> claims (yet to be committed).
>
> Hopefully, that history helps. In short, I agree that
> SecurityIdentity.getRoles() should be the right way to determine activated
> principal roles. The Resolver should rely on the authenticator to determine
> the active set of principal roles and not rely on grant records. The
> AuthenticatedPolarisPrincipal doesn't really need to hold role names
> anymore, but the resolver does still need the list of active PrincipalRoles
> to load the appropriate catalog roles. My preference would be to make this
> an immutable class that has the principal entity and all active principal
> roles all in the constructor.
>
> Mike
>
> On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra <al...@dremio.com.invalid>
> wrote:
>
> > Hi Mike,
> >
> > I am generally fine with the spec changes.
> >
> > I have however some concerns around the way we handle principal roles
> today
> > in Polaris, and as I was preparing the PR with support for external IDPs
> > [1], some oddities stood out:
> >
> > 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
> > converted to an empty set of roles by the Authenticator; but I wonder
> > how we will be able to distinguish a token that contained no role
> from a
> > token that contained the pseudo-role. I added some changes to my PR to
> > be
> > able to distinguish the two situations, but I think the general
> > handling of
> > this pseudo-role, as well as role mapping in general, deserves some
> more
> > thinking. The spec says this pseudo-role is "non-functional" – but I
> > don't
> > know what this implies exactly.
> > 2. AuthenticatedPolarisPrincipal exposes principal roles and this is
> > imho concerning for two reasons: first, the Resolver seems to update
> the
> > principal roles during resolution, which makes the class mutable,
> > potentially changing the principal characteristics *after*
> > authentication.
> > And secondly, this creates an ambiguity as roles can be accessed in
> two
> > ways: SecurityIdentity.getRoles() vs
> > AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I would be
> > in favor of using the former consistently and completely removing the
> > latter. (Note: SecurityContext.isUserInRole() relies on the former.)
> > 3. There seems to be some overlap between the primary roles
> resolution,
> > done by the ActiveRolesProvider, and the role resolution done later by
> > the Resolver, cf. method resolveCallerPrincipalAndPrincipalRoles(). I
> > wonder if this second role resolution is really necessary? Ultimately,
> > the
> > source of truth for role membership will always be
> > SecurityIdentity.hasRole() or its JAX-RS variant,
> > SecurityContext.isUserInRole(). We should not deviate from that.
> >
> > Thanks,
> >
> > Alex
> >
> > [1] https://github.com/apache/polaris/pull/1397
> >
> > On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <di...@apache.org>
> > wrote:
> >
> > > Re 2: Thanks for the clarification, Mike! I guess my brain swapped out
> a
> > > large portion of that doc :)
> > >
> > > I'm still not sure how IdentityToPrincipalMapping can help with
> resolving
> > > changes from API and from IdP integration.
> > >
> > > The doc talks about namespaces, but in PR# 1353, it looks like API
> calls
> > > are free to change anything in federated principal roles.
> > >
> > > Should IdentityToPrincipalMapping extend to validating changes coming
> > from
> > > the Admin API?
> > >
> > > Alternatively, we could treat that as a new permission and allow all
> > admin
> > > users to edit federated roles via API.
> > >
> > > Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need to
> > > sync role membership, I guess.
> > >
> > > My understanding so far was that we'd get role membership at
> > authentication
> > > time and not persist it in Polaris.
> > >
> > > Is that still the plan?
> > >
> > > I think this relates to how we connect authentication and authorization
> > in
> > > Polaris. If Principals are external, we get role membership from the
> > access
> > > token and all is clear. If Principals are local, and we still get roles
> > > from the access token, then the question arises about whether that data
> > is
> > > in sync with local data (and the other way around too).
> > >
> > > That could be resolved by Polaris owning access tokens (via token
> > exchange)
> > > and assuming control over role membership based on local data and
> access
> > > time (I commented on this in the doc too).
> > >
> > > All in all, since SCIM is not part of phase 1 here, maybe we could
> defer
> > > dealing with federated Principal persistence, indeed.
> > >
> > > Re 3: I guess it also applies to client secret generation code, but
> let's
> > > see whether it actually causes too many conditions in code when it
> comes
> > to
> > > that.
> > >
> > > Cheers,
> > > Dmitri.
> > >
> > > On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <
> collado.mike@gmail.com>
> > > wrote:
> > >
> > > > Thanks for the response.
> > > >
> > > > 1. TBH, I have no strong feelings about persisting the federated
> > > > principals. I think the biggest advantage I saw was to support SCIM
> > along
> > > > with 3p identity providers. But for this implementation, if we want
> to
> > > skip
> > > > persisting principal entities, I'm ok with it.
> > > >
> > > > 2. In the doc I wrote the following, which I think addresses the
> > concern
> > > > around conflicts.
> > > >
> > > > 1.
> > > >
> > > > The IdentityToPrincipalMapping should supply a naming convention
> so
> > > that
> > > > PrincipalRoles and Principals created from that source do not
> > conflict
> > > > with
> > > > entities created by a different IdentityProvider
> > > >
> > > > In the case that a role happens to exist even with the naming
> > convention
> > > > applied, the caller would be unable to authenticate with that role.
> > > >
> > > > 3. I don't think we need special classes for federated roles. In
> nearly
> > > all
> > > > of the Polaris code, they are treated as PolarisRoleEntity without
> any
> > > > issue. The only code that needs to be aware of federated roles is
> > > >
> > > > a) The Admin API, which prevents granting these roles to principals
> and
> > > >
> > > > b) The federated roles provider, which verifies that the federated
> > > > principal is assuming federated roles.
> > > >
> > > > The rest of the code (i.e., the resolver and authorizer) treat these
> > > roles
> > > > exactly as native Polaris roles.
> > > >
> > > > Mike
> > > >
> > > > On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <
> dimas@apache.org>
> > > > wrote:
> > > >
> > > > > Thanks for reviving this discussion, Mike!
> > > > >
> > > > > The API spec change by itself LGTM, but I have related concerns in
> > how
> > > > this
> > > > > feature is meant to work in general.
> > > > >
> > > > > 1) The need to expose Federated Principals in Polaris API.
> > > > >
> > > > > The design doc [1] discusses the possibility to expose Federated
> > > > Principals
> > > > > in Polaris API, but there are pros and cons. I do not think it is a
> > > > trivial
> > > > > decision. I'd like to discuss this in more detail before we commit
> to
> > > > doing
> > > > > that.
> > > > >
> > > > > From my POV the main benefit of exposing Federated Principals is to
> > be
> > > > able
> > > > > to alter their properties in Polaris (as mentioned in the doc).
> > > However,
> > > > > your email indicates that this is actually forbidden.
> > > > >
> > > > > What use cases do you envision for exposing read-only Federated
> > > > Principals?
> > > > >
> > > > > 2) Reconciliation of Federated vs. Non-Federated changes.
> > > > >
> > > > > Let's focus on Principal Roles here (which are meant to be writable
> > via
> > > > > API).
> > > > >
> > > > > If users are able to make and change (I assume) Principal Roles via
> > > API,
> > > > > and federation code will also be creating and changing those roles,
> > > what
> > > > is
> > > > > our approach to handling conflicts between those two streams of
> > > changes?
> > > > >
> > > > > Some examples for context (also mentioned in GH):
> > > > >
> > > > > A) A user creates normal Principal Role "A", and later role "A" is
> > > > > discovered through Federation.
> > > > >
> > > > > B) A user edits Federated Role "B" and later properties of "B" are
> > > > updated
> > > > > through Federation (or the other way around).
> > > > >
> > > > > Do we want to control property edits based on the origin of the
> > > property
> > > > > (spec out a "namespace" for Federated properties)?
> > > > >
> > > > > 3) OO types
> > > > >
> > > > > Currently Principals and Principal Roles are represented by one
> java
> > > type
> > > > > each. When federation comes into play, would it make sense to
> > develop a
> > > > > (java) type hierarchy for working with that data in Polaris code?
> My
> > > main
> > > > > concern here is avoiding a maze of if/else statements throughout
> the
> > > > > Polaris codebase for supporting Federation.
> > > > >
> > > > > I guess our approach to this depends largely on the outcome of
> topic
> > 2
> > > > > above.
> > > > >
> > > > > WDYT?
> > > > >
> > > > > Thanks,
> > > > > Dmitri.
> > > > >
> > > > > [1]
> > > > >
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > > > >
> > > > > On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > > collado.mike@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hey folks
> > > > > >
> > > > > > Some of you already know that I posted an initial PR to get
> > federated
> > > > > > principals/roles added. One thing that came out of the feedback
> > was a
> > > > > spec
> > > > > > change to make it clear when federated identities can be used in
> > the
> > > > > APIs.
> > > > > > Notably, federated principals cannot be created or updated, but
> can
> > > be
> > > > > > returned in get/list calls, whereas federated roles *can* be
> > created
> > > by
> > > > > the
> > > > > > API. The latter is useful/necessary in order to be able to assign
> > > > > > privileges to those roles without relying on the JIT creation on
> > > login.
> > > > > >
> > > > > > Please check out the spec change here and let me know what you
> > think
> > > -
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > > > >
> > > > > > Mike
> > > > > >
> > > > > > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > > > > <dm...@dremio.com.invalid> wrote:
> > > > > >
> > > > > > > Hi Mike,
> > > > > > >
> > > > > > > I left some comments in the doc, but overall it looks good to
> me
> > :)
> > > > > > >
> > > > > > > I still think there are some hidden dependencies on
> Persistence.
> > > For
> > > > > > > example, whether and how we can have composite keys for
> persisted
> > > > > > federated
> > > > > > > entities... but I guess we can work that out later.
> > > > > > >
> > > > > > > Also, I think it is important for the Authorizer API to avoid
> > > > assuming
> > > > > > that
> > > > > > > all principals are persisted. Specific authorizer
> implementations
> > > > > > > (including the default one) can certainly expect persisted
> > > > principals,
> > > > > > but
> > > > > > > the API should require that for the sake of flexibility of
> > possible
> > > > > > AuthN/Z
> > > > > > > extensions. WDYT?
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Dmitri.
> > > > > > >
> > > > > > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > > > collado.mike@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hey folks
> > > > > > > >
> > > > > > > > As discussed during the community sync, I've put together
> some
> > > > > thoughts
> > > > > > > on
> > > > > > > > how we'd add support for federated identities in Polaris. I
> > > copied
> > > > > over
> > > > > > > > some of what I had in the issue at
> > > > > > > > https://github.com/apache/polaris/issues/441 and put it into
> > the
> > > > doc
> > > > > > > here:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > > > > > .
> > > > > > > >
> > > > > > > > Please take a look when you get some time and let me know
> what
> > > you
> > > > > > think.
> > > > > > > > Given that our next community sync is scheduled for the
> > > > Thanksgiving
> > > > > > > > holiday in the US, it might be useful to schedule a meeting
> > > > > > specifically
> > > > > > > > for this. I can schedule that sync if needed.
> > > > > > > >
> > > > > > > > Mike
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
Hey
1. I'm unsure why the Resolver was written to assume no role means all
roles. But I think it has to do with
2. The Resolver was intended to be able to resolve principal roles, grants,
and target entities all at one "snapshot" based on the entity and grant
versions returned by that loadEntitiesChangeTracking method. In order to do
this in one shot, the authenticator couldn't resolve the principal roles
actually granted to the principal, so it didn't resolve the
PRINCIPAL_ROLE:ALL scope in the token. But that meant the Resolver needed
to verify the grants existed for the roles to be assumed by the principal
(otherwise, a caller could just put whatever role they want in the scope).
To avoid the lookup during authn, ALL needed to be expanded by the Resolver
based on the grants. This was an impedance to the federated role proposal,
which stated that the authn token is authoritative regarding which roles
are granted to the principal (e.g., using a groups claim
<https://developer.okta.com/docs/guides/customize-tokens-groups-claim/main/>).
That's why
3. I added the ActiveRolesProvider interface so that the default impl would
continue to validate the grants held by the principal for activating
principal roles but the federated alternative would delegate to the token
claims (yet to be committed).
Hopefully, that history helps. In short, I agree that
SecurityIdentity.getRoles() should be the right way to determine activated
principal roles. The Resolver should rely on the authenticator to determine
the active set of principal roles and not rely on grant records. The
AuthenticatedPolarisPrincipal doesn't really need to hold role names
anymore, but the resolver does still need the list of active PrincipalRoles
to load the appropriate catalog roles. My preference would be to make this
an immutable class that has the principal entity and all active principal
roles all in the constructor.
Mike
On Sun, Apr 20, 2025 at 8:34 AM Alex Dutra <al...@dremio.com.invalid>
wrote:
> Hi Mike,
>
> I am generally fine with the spec changes.
>
> I have however some concerns around the way we handle principal roles today
> in Polaris, and as I was preparing the PR with support for external IDPs
> [1], some oddities stood out:
>
> 1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
> converted to an empty set of roles by the Authenticator; but I wonder
> how we will be able to distinguish a token that contained no role from a
> token that contained the pseudo-role. I added some changes to my PR to
> be
> able to distinguish the two situations, but I think the general
> handling of
> this pseudo-role, as well as role mapping in general, deserves some more
> thinking. The spec says this pseudo-role is "non-functional" – but I
> don't
> know what this implies exactly.
> 2. AuthenticatedPolarisPrincipal exposes principal roles and this is
> imho concerning for two reasons: first, the Resolver seems to update the
> principal roles during resolution, which makes the class mutable,
> potentially changing the principal characteristics *after*
> authentication.
> And secondly, this creates an ambiguity as roles can be accessed in two
> ways: SecurityIdentity.getRoles() vs
> AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I would be
> in favor of using the former consistently and completely removing the
> latter. (Note: SecurityContext.isUserInRole() relies on the former.)
> 3. There seems to be some overlap between the primary roles resolution,
> done by the ActiveRolesProvider, and the role resolution done later by
> the Resolver, cf. method resolveCallerPrincipalAndPrincipalRoles(). I
> wonder if this second role resolution is really necessary? Ultimately,
> the
> source of truth for role membership will always be
> SecurityIdentity.hasRole() or its JAX-RS variant,
> SecurityContext.isUserInRole(). We should not deviate from that.
>
> Thanks,
>
> Alex
>
> [1] https://github.com/apache/polaris/pull/1397
>
> On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <di...@apache.org>
> wrote:
>
> > Re 2: Thanks for the clarification, Mike! I guess my brain swapped out a
> > large portion of that doc :)
> >
> > I'm still not sure how IdentityToPrincipalMapping can help with resolving
> > changes from API and from IdP integration.
> >
> > The doc talks about namespaces, but in PR# 1353, it looks like API calls
> > are free to change anything in federated principal roles.
> >
> > Should IdentityToPrincipalMapping extend to validating changes coming
> from
> > the Admin API?
> >
> > Alternatively, we could treat that as a new permission and allow all
> admin
> > users to edit federated roles via API.
> >
> > Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need to
> > sync role membership, I guess.
> >
> > My understanding so far was that we'd get role membership at
> authentication
> > time and not persist it in Polaris.
> >
> > Is that still the plan?
> >
> > I think this relates to how we connect authentication and authorization
> in
> > Polaris. If Principals are external, we get role membership from the
> access
> > token and all is clear. If Principals are local, and we still get roles
> > from the access token, then the question arises about whether that data
> is
> > in sync with local data (and the other way around too).
> >
> > That could be resolved by Polaris owning access tokens (via token
> exchange)
> > and assuming control over role membership based on local data and access
> > time (I commented on this in the doc too).
> >
> > All in all, since SCIM is not part of phase 1 here, maybe we could defer
> > dealing with federated Principal persistence, indeed.
> >
> > Re 3: I guess it also applies to client secret generation code, but let's
> > see whether it actually causes too many conditions in code when it comes
> to
> > that.
> >
> > Cheers,
> > Dmitri.
> >
> > On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <co...@gmail.com>
> > wrote:
> >
> > > Thanks for the response.
> > >
> > > 1. TBH, I have no strong feelings about persisting the federated
> > > principals. I think the biggest advantage I saw was to support SCIM
> along
> > > with 3p identity providers. But for this implementation, if we want to
> > skip
> > > persisting principal entities, I'm ok with it.
> > >
> > > 2. In the doc I wrote the following, which I think addresses the
> concern
> > > around conflicts.
> > >
> > > 1.
> > >
> > > The IdentityToPrincipalMapping should supply a naming convention so
> > that
> > > PrincipalRoles and Principals created from that source do not
> conflict
> > > with
> > > entities created by a different IdentityProvider
> > >
> > > In the case that a role happens to exist even with the naming
> convention
> > > applied, the caller would be unable to authenticate with that role.
> > >
> > > 3. I don't think we need special classes for federated roles. In nearly
> > all
> > > of the Polaris code, they are treated as PolarisRoleEntity without any
> > > issue. The only code that needs to be aware of federated roles is
> > >
> > > a) The Admin API, which prevents granting these roles to principals and
> > >
> > > b) The federated roles provider, which verifies that the federated
> > > principal is assuming federated roles.
> > >
> > > The rest of the code (i.e., the resolver and authorizer) treat these
> > roles
> > > exactly as native Polaris roles.
> > >
> > > Mike
> > >
> > > On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <di...@apache.org>
> > > wrote:
> > >
> > > > Thanks for reviving this discussion, Mike!
> > > >
> > > > The API spec change by itself LGTM, but I have related concerns in
> how
> > > this
> > > > feature is meant to work in general.
> > > >
> > > > 1) The need to expose Federated Principals in Polaris API.
> > > >
> > > > The design doc [1] discusses the possibility to expose Federated
> > > Principals
> > > > in Polaris API, but there are pros and cons. I do not think it is a
> > > trivial
> > > > decision. I'd like to discuss this in more detail before we commit to
> > > doing
> > > > that.
> > > >
> > > > From my POV the main benefit of exposing Federated Principals is to
> be
> > > able
> > > > to alter their properties in Polaris (as mentioned in the doc).
> > However,
> > > > your email indicates that this is actually forbidden.
> > > >
> > > > What use cases do you envision for exposing read-only Federated
> > > Principals?
> > > >
> > > > 2) Reconciliation of Federated vs. Non-Federated changes.
> > > >
> > > > Let's focus on Principal Roles here (which are meant to be writable
> via
> > > > API).
> > > >
> > > > If users are able to make and change (I assume) Principal Roles via
> > API,
> > > > and federation code will also be creating and changing those roles,
> > what
> > > is
> > > > our approach to handling conflicts between those two streams of
> > changes?
> > > >
> > > > Some examples for context (also mentioned in GH):
> > > >
> > > > A) A user creates normal Principal Role "A", and later role "A" is
> > > > discovered through Federation.
> > > >
> > > > B) A user edits Federated Role "B" and later properties of "B" are
> > > updated
> > > > through Federation (or the other way around).
> > > >
> > > > Do we want to control property edits based on the origin of the
> > property
> > > > (spec out a "namespace" for Federated properties)?
> > > >
> > > > 3) OO types
> > > >
> > > > Currently Principals and Principal Roles are represented by one java
> > type
> > > > each. When federation comes into play, would it make sense to
> develop a
> > > > (java) type hierarchy for working with that data in Polaris code? My
> > main
> > > > concern here is avoiding a maze of if/else statements throughout the
> > > > Polaris codebase for supporting Federation.
> > > >
> > > > I guess our approach to this depends largely on the outcome of topic
> 2
> > > > above.
> > > >
> > > > WDYT?
> > > >
> > > > Thanks,
> > > > Dmitri.
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > > >
> > > > On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> > collado.mike@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey folks
> > > > >
> > > > > Some of you already know that I posted an initial PR to get
> federated
> > > > > principals/roles added. One thing that came out of the feedback
> was a
> > > > spec
> > > > > change to make it clear when federated identities can be used in
> the
> > > > APIs.
> > > > > Notably, federated principals cannot be created or updated, but can
> > be
> > > > > returned in get/list calls, whereas federated roles *can* be
> created
> > by
> > > > the
> > > > > API. The latter is useful/necessary in order to be able to assign
> > > > > privileges to those roles without relying on the JIT creation on
> > login.
> > > > >
> > > > > Please check out the spec change here and let me know what you
> think
> > -
> > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > > >
> > > > > Mike
> > > > >
> > > > > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > > > <dm...@dremio.com.invalid> wrote:
> > > > >
> > > > > > Hi Mike,
> > > > > >
> > > > > > I left some comments in the doc, but overall it looks good to me
> :)
> > > > > >
> > > > > > I still think there are some hidden dependencies on Persistence.
> > For
> > > > > > example, whether and how we can have composite keys for persisted
> > > > > federated
> > > > > > entities... but I guess we can work that out later.
> > > > > >
> > > > > > Also, I think it is important for the Authorizer API to avoid
> > > assuming
> > > > > that
> > > > > > all principals are persisted. Specific authorizer implementations
> > > > > > (including the default one) can certainly expect persisted
> > > principals,
> > > > > but
> > > > > > the API should require that for the sake of flexibility of
> possible
> > > > > AuthN/Z
> > > > > > extensions. WDYT?
> > > > > >
> > > > > > Cheers,
> > > > > > Dmitri.
> > > > > >
> > > > > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > > collado.mike@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hey folks
> > > > > > >
> > > > > > > As discussed during the community sync, I've put together some
> > > > thoughts
> > > > > > on
> > > > > > > how we'd add support for federated identities in Polaris. I
> > copied
> > > > over
> > > > > > > some of what I had in the issue at
> > > > > > > https://github.com/apache/polaris/issues/441 and put it into
> the
> > > doc
> > > > > > here:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > > > > .
> > > > > > >
> > > > > > > Please take a look when you get some time and let me know what
> > you
> > > > > think.
> > > > > > > Given that our next community sync is scheduled for the
> > > Thanksgiving
> > > > > > > holiday in the US, it might be useful to schedule a meeting
> > > > > specifically
> > > > > > > for this. I can schedule that sync if needed.
> > > > > > >
> > > > > > > Mike
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Alex Dutra <al...@dremio.com.INVALID>.
Hi Mike,
I am generally fine with the spec changes.
I have however some concerns around the way we handle principal roles today
in Polaris, and as I was preparing the PR with support for external IDPs
[1], some oddities stood out:
1. The pseudo-role PRINCIPAL_ROLE:ALL seems, by convention, to be
converted to an empty set of roles by the Authenticator; but I wonder
how we will be able to distinguish a token that contained no role from a
token that contained the pseudo-role. I added some changes to my PR to be
able to distinguish the two situations, but I think the general handling of
this pseudo-role, as well as role mapping in general, deserves some more
thinking. The spec says this pseudo-role is "non-functional" – but I don't
know what this implies exactly.
2. AuthenticatedPolarisPrincipal exposes principal roles and this is
imho concerning for two reasons: first, the Resolver seems to update the
principal roles during resolution, which makes the class mutable,
potentially changing the principal characteristics *after* authentication.
And secondly, this creates an ambiguity as roles can be accessed in two
ways: SecurityIdentity.getRoles() vs
AuthenticatedPolarisPrincipal.getActivatedPrincipalRoles(). I would be
in favor of using the former consistently and completely removing the
latter. (Note: SecurityContext.isUserInRole() relies on the former.)
3. There seems to be some overlap between the primary roles resolution,
done by the ActiveRolesProvider, and the role resolution done later by
the Resolver, cf. method resolveCallerPrincipalAndPrincipalRoles(). I
wonder if this second role resolution is really necessary? Ultimately, the
source of truth for role membership will always be
SecurityIdentity.hasRole() or its JAX-RS variant,
SecurityContext.isUserInRole(). We should not deviate from that.
Thanks,
Alex
[1] https://github.com/apache/polaris/pull/1397
On Sat, Apr 19, 2025 at 2:47 AM Dmitri Bourlatchkov <di...@apache.org>
wrote:
> Re 2: Thanks for the clarification, Mike! I guess my brain swapped out a
> large portion of that doc :)
>
> I'm still not sure how IdentityToPrincipalMapping can help with resolving
> changes from API and from IdP integration.
>
> The doc talks about namespaces, but in PR# 1353, it looks like API calls
> are free to change anything in federated principal roles.
>
> Should IdentityToPrincipalMapping extend to validating changes coming from
> the Admin API?
>
> Alternatively, we could treat that as a new permission and allow all admin
> users to edit federated roles via API.
>
> Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need to
> sync role membership, I guess.
>
> My understanding so far was that we'd get role membership at authentication
> time and not persist it in Polaris.
>
> Is that still the plan?
>
> I think this relates to how we connect authentication and authorization in
> Polaris. If Principals are external, we get role membership from the access
> token and all is clear. If Principals are local, and we still get roles
> from the access token, then the question arises about whether that data is
> in sync with local data (and the other way around too).
>
> That could be resolved by Polaris owning access tokens (via token exchange)
> and assuming control over role membership based on local data and access
> time (I commented on this in the doc too).
>
> All in all, since SCIM is not part of phase 1 here, maybe we could defer
> dealing with federated Principal persistence, indeed.
>
> Re 3: I guess it also applies to client secret generation code, but let's
> see whether it actually causes too many conditions in code when it comes to
> that.
>
> Cheers,
> Dmitri.
>
> On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <co...@gmail.com>
> wrote:
>
> > Thanks for the response.
> >
> > 1. TBH, I have no strong feelings about persisting the federated
> > principals. I think the biggest advantage I saw was to support SCIM along
> > with 3p identity providers. But for this implementation, if we want to
> skip
> > persisting principal entities, I'm ok with it.
> >
> > 2. In the doc I wrote the following, which I think addresses the concern
> > around conflicts.
> >
> > 1.
> >
> > The IdentityToPrincipalMapping should supply a naming convention so
> that
> > PrincipalRoles and Principals created from that source do not conflict
> > with
> > entities created by a different IdentityProvider
> >
> > In the case that a role happens to exist even with the naming convention
> > applied, the caller would be unable to authenticate with that role.
> >
> > 3. I don't think we need special classes for federated roles. In nearly
> all
> > of the Polaris code, they are treated as PolarisRoleEntity without any
> > issue. The only code that needs to be aware of federated roles is
> >
> > a) The Admin API, which prevents granting these roles to principals and
> >
> > b) The federated roles provider, which verifies that the federated
> > principal is assuming federated roles.
> >
> > The rest of the code (i.e., the resolver and authorizer) treat these
> roles
> > exactly as native Polaris roles.
> >
> > Mike
> >
> > On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <di...@apache.org>
> > wrote:
> >
> > > Thanks for reviving this discussion, Mike!
> > >
> > > The API spec change by itself LGTM, but I have related concerns in how
> > this
> > > feature is meant to work in general.
> > >
> > > 1) The need to expose Federated Principals in Polaris API.
> > >
> > > The design doc [1] discusses the possibility to expose Federated
> > Principals
> > > in Polaris API, but there are pros and cons. I do not think it is a
> > trivial
> > > decision. I'd like to discuss this in more detail before we commit to
> > doing
> > > that.
> > >
> > > From my POV the main benefit of exposing Federated Principals is to be
> > able
> > > to alter their properties in Polaris (as mentioned in the doc).
> However,
> > > your email indicates that this is actually forbidden.
> > >
> > > What use cases do you envision for exposing read-only Federated
> > Principals?
> > >
> > > 2) Reconciliation of Federated vs. Non-Federated changes.
> > >
> > > Let's focus on Principal Roles here (which are meant to be writable via
> > > API).
> > >
> > > If users are able to make and change (I assume) Principal Roles via
> API,
> > > and federation code will also be creating and changing those roles,
> what
> > is
> > > our approach to handling conflicts between those two streams of
> changes?
> > >
> > > Some examples for context (also mentioned in GH):
> > >
> > > A) A user creates normal Principal Role "A", and later role "A" is
> > > discovered through Federation.
> > >
> > > B) A user edits Federated Role "B" and later properties of "B" are
> > updated
> > > through Federation (or the other way around).
> > >
> > > Do we want to control property edits based on the origin of the
> property
> > > (spec out a "namespace" for Federated properties)?
> > >
> > > 3) OO types
> > >
> > > Currently Principals and Principal Roles are represented by one java
> type
> > > each. When federation comes into play, would it make sense to develop a
> > > (java) type hierarchy for working with that data in Polaris code? My
> main
> > > concern here is avoiding a maze of if/else statements throughout the
> > > Polaris codebase for supporting Federation.
> > >
> > > I guess our approach to this depends largely on the outcome of topic 2
> > > above.
> > >
> > > WDYT?
> > >
> > > Thanks,
> > > Dmitri.
> > >
> > > [1]
> > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> > >
> > > On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <
> collado.mike@gmail.com>
> > > wrote:
> > >
> > > > Hey folks
> > > >
> > > > Some of you already know that I posted an initial PR to get federated
> > > > principals/roles added. One thing that came out of the feedback was a
> > > spec
> > > > change to make it clear when federated identities can be used in the
> > > APIs.
> > > > Notably, federated principals cannot be created or updated, but can
> be
> > > > returned in get/list calls, whereas federated roles *can* be created
> by
> > > the
> > > > API. The latter is useful/necessary in order to be able to assign
> > > > privileges to those roles without relying on the JIT creation on
> login.
> > > >
> > > > Please check out the spec change here and let me know what you think
> -
> > > >
> > > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > > >
> > > > Mike
> > > >
> > > > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > > <dm...@dremio.com.invalid> wrote:
> > > >
> > > > > Hi Mike,
> > > > >
> > > > > I left some comments in the doc, but overall it looks good to me :)
> > > > >
> > > > > I still think there are some hidden dependencies on Persistence.
> For
> > > > > example, whether and how we can have composite keys for persisted
> > > > federated
> > > > > entities... but I guess we can work that out later.
> > > > >
> > > > > Also, I think it is important for the Authorizer API to avoid
> > assuming
> > > > that
> > > > > all principals are persisted. Specific authorizer implementations
> > > > > (including the default one) can certainly expect persisted
> > principals,
> > > > but
> > > > > the API should require that for the sake of flexibility of possible
> > > > AuthN/Z
> > > > > extensions. WDYT?
> > > > >
> > > > > Cheers,
> > > > > Dmitri.
> > > > >
> > > > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > > collado.mike@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hey folks
> > > > > >
> > > > > > As discussed during the community sync, I've put together some
> > > thoughts
> > > > > on
> > > > > > how we'd add support for federated identities in Polaris. I
> copied
> > > over
> > > > > > some of what I had in the issue at
> > > > > > https://github.com/apache/polaris/issues/441 and put it into the
> > doc
> > > > > here:
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > > > .
> > > > > >
> > > > > > Please take a look when you get some time and let me know what
> you
> > > > think.
> > > > > > Given that our next community sync is scheduled for the
> > Thanksgiving
> > > > > > holiday in the US, it might be useful to schedule a meeting
> > > > specifically
> > > > > > for this. I can schedule that sync if needed.
> > > > > >
> > > > > > Mike
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Dmitri Bourlatchkov <di...@apache.org>.
Re 2: Thanks for the clarification, Mike! I guess my brain swapped out a
large portion of that doc :)
I'm still not sure how IdentityToPrincipalMapping can help with resolving
changes from API and from IdP integration.
The doc talks about namespaces, but in PR# 1353, it looks like API calls
are free to change anything in federated principal roles.
Should IdentityToPrincipalMapping extend to validating changes coming from
the Admin API?
Alternatively, we could treat that as a new permission and allow all admin
users to edit federated roles via API.
Re 1: Yes, I guess it would be helpful with SCIM, but then we'd need to
sync role membership, I guess.
My understanding so far was that we'd get role membership at authentication
time and not persist it in Polaris.
Is that still the plan?
I think this relates to how we connect authentication and authorization in
Polaris. If Principals are external, we get role membership from the access
token and all is clear. If Principals are local, and we still get roles
from the access token, then the question arises about whether that data is
in sync with local data (and the other way around too).
That could be resolved by Polaris owning access tokens (via token exchange)
and assuming control over role membership based on local data and access
time (I commented on this in the doc too).
All in all, since SCIM is not part of phase 1 here, maybe we could defer
dealing with federated Principal persistence, indeed.
Re 3: I guess it also applies to client secret generation code, but let's
see whether it actually causes too many conditions in code when it comes to
that.
Cheers,
Dmitri.
On Thu, Apr 17, 2025 at 7:42 PM Michael Collado <co...@gmail.com>
wrote:
> Thanks for the response.
>
> 1. TBH, I have no strong feelings about persisting the federated
> principals. I think the biggest advantage I saw was to support SCIM along
> with 3p identity providers. But for this implementation, if we want to skip
> persisting principal entities, I'm ok with it.
>
> 2. In the doc I wrote the following, which I think addresses the concern
> around conflicts.
>
> 1.
>
> The IdentityToPrincipalMapping should supply a naming convention so that
> PrincipalRoles and Principals created from that source do not conflict
> with
> entities created by a different IdentityProvider
>
> In the case that a role happens to exist even with the naming convention
> applied, the caller would be unable to authenticate with that role.
>
> 3. I don't think we need special classes for federated roles. In nearly all
> of the Polaris code, they are treated as PolarisRoleEntity without any
> issue. The only code that needs to be aware of federated roles is
>
> a) The Admin API, which prevents granting these roles to principals and
>
> b) The federated roles provider, which verifies that the federated
> principal is assuming federated roles.
>
> The rest of the code (i.e., the resolver and authorizer) treat these roles
> exactly as native Polaris roles.
>
> Mike
>
> On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <di...@apache.org>
> wrote:
>
> > Thanks for reviving this discussion, Mike!
> >
> > The API spec change by itself LGTM, but I have related concerns in how
> this
> > feature is meant to work in general.
> >
> > 1) The need to expose Federated Principals in Polaris API.
> >
> > The design doc [1] discusses the possibility to expose Federated
> Principals
> > in Polaris API, but there are pros and cons. I do not think it is a
> trivial
> > decision. I'd like to discuss this in more detail before we commit to
> doing
> > that.
> >
> > From my POV the main benefit of exposing Federated Principals is to be
> able
> > to alter their properties in Polaris (as mentioned in the doc). However,
> > your email indicates that this is actually forbidden.
> >
> > What use cases do you envision for exposing read-only Federated
> Principals?
> >
> > 2) Reconciliation of Federated vs. Non-Federated changes.
> >
> > Let's focus on Principal Roles here (which are meant to be writable via
> > API).
> >
> > If users are able to make and change (I assume) Principal Roles via API,
> > and federation code will also be creating and changing those roles, what
> is
> > our approach to handling conflicts between those two streams of changes?
> >
> > Some examples for context (also mentioned in GH):
> >
> > A) A user creates normal Principal Role "A", and later role "A" is
> > discovered through Federation.
> >
> > B) A user edits Federated Role "B" and later properties of "B" are
> updated
> > through Federation (or the other way around).
> >
> > Do we want to control property edits based on the origin of the property
> > (spec out a "namespace" for Federated properties)?
> >
> > 3) OO types
> >
> > Currently Principals and Principal Roles are represented by one java type
> > each. When federation comes into play, would it make sense to develop a
> > (java) type hierarchy for working with that data in Polaris code? My main
> > concern here is avoiding a maze of if/else statements throughout the
> > Polaris codebase for supporting Federation.
> >
> > I guess our approach to this depends largely on the outcome of topic 2
> > above.
> >
> > WDYT?
> >
> > Thanks,
> > Dmitri.
> >
> > [1]
> >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
> >
> > On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <co...@gmail.com>
> > wrote:
> >
> > > Hey folks
> > >
> > > Some of you already know that I posted an initial PR to get federated
> > > principals/roles added. One thing that came out of the feedback was a
> > spec
> > > change to make it clear when federated identities can be used in the
> > APIs.
> > > Notably, federated principals cannot be created or updated, but can be
> > > returned in get/list calls, whereas federated roles *can* be created by
> > the
> > > API. The latter is useful/necessary in order to be able to assign
> > > privileges to those roles without relying on the JIT creation on login.
> > >
> > > Please check out the spec change here and let me know what you think -
> > >
> > >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> > >
> > > Mike
> > >
> > > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > > <dm...@dremio.com.invalid> wrote:
> > >
> > > > Hi Mike,
> > > >
> > > > I left some comments in the doc, but overall it looks good to me :)
> > > >
> > > > I still think there are some hidden dependencies on Persistence. For
> > > > example, whether and how we can have composite keys for persisted
> > > federated
> > > > entities... but I guess we can work that out later.
> > > >
> > > > Also, I think it is important for the Authorizer API to avoid
> assuming
> > > that
> > > > all principals are persisted. Specific authorizer implementations
> > > > (including the default one) can certainly expect persisted
> principals,
> > > but
> > > > the API should require that for the sake of flexibility of possible
> > > AuthN/Z
> > > > extensions. WDYT?
> > > >
> > > > Cheers,
> > > > Dmitri.
> > > >
> > > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> > collado.mike@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey folks
> > > > >
> > > > > As discussed during the community sync, I've put together some
> > thoughts
> > > > on
> > > > > how we'd add support for federated identities in Polaris. I copied
> > over
> > > > > some of what I had in the issue at
> > > > > https://github.com/apache/polaris/issues/441 and put it into the
> doc
> > > > here:
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > > .
> > > > >
> > > > > Please take a look when you get some time and let me know what you
> > > think.
> > > > > Given that our next community sync is scheduled for the
> Thanksgiving
> > > > > holiday in the US, it might be useful to schedule a meeting
> > > specifically
> > > > > for this. I can schedule that sync if needed.
> > > > >
> > > > > Mike
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
Thanks for the response.
1. TBH, I have no strong feelings about persisting the federated
principals. I think the biggest advantage I saw was to support SCIM along
with 3p identity providers. But for this implementation, if we want to skip
persisting principal entities, I'm ok with it.
2. In the doc I wrote the following, which I think addresses the concern
around conflicts.
1.
The IdentityToPrincipalMapping should supply a naming convention so that
PrincipalRoles and Principals created from that source do not conflict with
entities created by a different IdentityProvider
In the case that a role happens to exist even with the naming convention
applied, the caller would be unable to authenticate with that role.
3. I don't think we need special classes for federated roles. In nearly all
of the Polaris code, they are treated as PolarisRoleEntity without any
issue. The only code that needs to be aware of federated roles is
a) The Admin API, which prevents granting these roles to principals and
b) The federated roles provider, which verifies that the federated
principal is assuming federated roles.
The rest of the code (i.e., the resolver and authorizer) treat these roles
exactly as native Polaris roles.
Mike
On Thu, Apr 17, 2025 at 2:39 PM Dmitri Bourlatchkov <di...@apache.org>
wrote:
> Thanks for reviving this discussion, Mike!
>
> The API spec change by itself LGTM, but I have related concerns in how this
> feature is meant to work in general.
>
> 1) The need to expose Federated Principals in Polaris API.
>
> The design doc [1] discusses the possibility to expose Federated Principals
> in Polaris API, but there are pros and cons. I do not think it is a trivial
> decision. I'd like to discuss this in more detail before we commit to doing
> that.
>
> From my POV the main benefit of exposing Federated Principals is to be able
> to alter their properties in Polaris (as mentioned in the doc). However,
> your email indicates that this is actually forbidden.
>
> What use cases do you envision for exposing read-only Federated Principals?
>
> 2) Reconciliation of Federated vs. Non-Federated changes.
>
> Let's focus on Principal Roles here (which are meant to be writable via
> API).
>
> If users are able to make and change (I assume) Principal Roles via API,
> and federation code will also be creating and changing those roles, what is
> our approach to handling conflicts between those two streams of changes?
>
> Some examples for context (also mentioned in GH):
>
> A) A user creates normal Principal Role "A", and later role "A" is
> discovered through Federation.
>
> B) A user edits Federated Role "B" and later properties of "B" are updated
> through Federation (or the other way around).
>
> Do we want to control property edits based on the origin of the property
> (spec out a "namespace" for Federated properties)?
>
> 3) OO types
>
> Currently Principals and Principal Roles are represented by one java type
> each. When federation comes into play, would it make sense to develop a
> (java) type hierarchy for working with that data in Polaris code? My main
> concern here is avoiding a maze of if/else statements throughout the
> Polaris codebase for supporting Federation.
>
> I guess our approach to this depends largely on the outcome of topic 2
> above.
>
> WDYT?
>
> Thanks,
> Dmitri.
>
> [1]
>
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
>
> On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <co...@gmail.com>
> wrote:
>
> > Hey folks
> >
> > Some of you already know that I posted an initial PR to get federated
> > principals/roles added. One thing that came out of the feedback was a
> spec
> > change to make it clear when federated identities can be used in the
> APIs.
> > Notably, federated principals cannot be created or updated, but can be
> > returned in get/list calls, whereas federated roles *can* be created by
> the
> > API. The latter is useful/necessary in order to be able to assign
> > privileges to those roles without relying on the JIT creation on login.
> >
> > Please check out the spec change here and let me know what you think -
> >
> >
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
> >
> > Mike
> >
> > On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> > <dm...@dremio.com.invalid> wrote:
> >
> > > Hi Mike,
> > >
> > > I left some comments in the doc, but overall it looks good to me :)
> > >
> > > I still think there are some hidden dependencies on Persistence. For
> > > example, whether and how we can have composite keys for persisted
> > federated
> > > entities... but I guess we can work that out later.
> > >
> > > Also, I think it is important for the Authorizer API to avoid assuming
> > that
> > > all principals are persisted. Specific authorizer implementations
> > > (including the default one) can certainly expect persisted principals,
> > but
> > > the API should require that for the sake of flexibility of possible
> > AuthN/Z
> > > extensions. WDYT?
> > >
> > > Cheers,
> > > Dmitri.
> > >
> > > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <
> collado.mike@gmail.com>
> > > wrote:
> > >
> > > > Hey folks
> > > >
> > > > As discussed during the community sync, I've put together some
> thoughts
> > > on
> > > > how we'd add support for federated identities in Polaris. I copied
> over
> > > > some of what I had in the issue at
> > > > https://github.com/apache/polaris/issues/441 and put it into the doc
> > > here:
> > > >
> > > >
> > > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > > .
> > > >
> > > > Please take a look when you get some time and let me know what you
> > think.
> > > > Given that our next community sync is scheduled for the Thanksgiving
> > > > holiday in the US, it might be useful to schedule a meeting
> > specifically
> > > > for this. I can schedule that sync if needed.
> > > >
> > > > Mike
> > > >
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Dmitri Bourlatchkov <di...@apache.org>.
Thanks for reviving this discussion, Mike!
The API spec change by itself LGTM, but I have related concerns in how this
feature is meant to work in general.
1) The need to expose Federated Principals in Polaris API.
The design doc [1] discusses the possibility to expose Federated Principals
in Polaris API, but there are pros and cons. I do not think it is a trivial
decision. I'd like to discuss this in more detail before we commit to doing
that.
From my POV the main benefit of exposing Federated Principals is to be able
to alter their properties in Polaris (as mentioned in the doc). However,
your email indicates that this is actually forbidden.
What use cases do you envision for exposing read-only Federated Principals?
2) Reconciliation of Federated vs. Non-Federated changes.
Let's focus on Principal Roles here (which are meant to be writable via
API).
If users are able to make and change (I assume) Principal Roles via API,
and federation code will also be creating and changing those roles, what is
our approach to handling conflicts between those two streams of changes?
Some examples for context (also mentioned in GH):
A) A user creates normal Principal Role "A", and later role "A" is
discovered through Federation.
B) A user edits Federated Role "B" and later properties of "B" are updated
through Federation (or the other way around).
Do we want to control property edits based on the origin of the property
(spec out a "namespace" for Federated properties)?
3) OO types
Currently Principals and Principal Roles are represented by one java type
each. When federation comes into play, would it make sense to develop a
(java) type hierarchy for working with that data in Polaris code? My main
concern here is avoiding a maze of if/else statements throughout the
Polaris codebase for supporting Federation.
I guess our approach to this depends largely on the outcome of topic 2
above.
WDYT?
Thanks,
Dmitri.
[1]
https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0#heading=h.cu1a1acu4lc5
On Wed, Apr 16, 2025 at 6:44 PM Michael Collado <co...@gmail.com>
wrote:
> Hey folks
>
> Some of you already know that I posted an initial PR to get federated
> principals/roles added. One thing that came out of the feedback was a spec
> change to make it clear when federated identities can be used in the APIs.
> Notably, federated principals cannot be created or updated, but can be
> returned in get/list calls, whereas federated roles *can* be created by the
> API. The latter is useful/necessary in order to be able to assign
> privileges to those roles without relying on the JIT creation on login.
>
> Please check out the spec change here and let me know what you think -
>
> https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
>
> Mike
>
> On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
> <dm...@dremio.com.invalid> wrote:
>
> > Hi Mike,
> >
> > I left some comments in the doc, but overall it looks good to me :)
> >
> > I still think there are some hidden dependencies on Persistence. For
> > example, whether and how we can have composite keys for persisted
> federated
> > entities... but I guess we can work that out later.
> >
> > Also, I think it is important for the Authorizer API to avoid assuming
> that
> > all principals are persisted. Specific authorizer implementations
> > (including the default one) can certainly expect persisted principals,
> but
> > the API should require that for the sake of flexibility of possible
> AuthN/Z
> > extensions. WDYT?
> >
> > Cheers,
> > Dmitri.
> >
> > On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <co...@gmail.com>
> > wrote:
> >
> > > Hey folks
> > >
> > > As discussed during the community sync, I've put together some thoughts
> > on
> > > how we'd add support for federated identities in Polaris. I copied over
> > > some of what I had in the issue at
> > > https://github.com/apache/polaris/issues/441 and put it into the doc
> > here:
> > >
> > >
> > >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > > .
> > >
> > > Please take a look when you get some time and let me know what you
> think.
> > > Given that our next community sync is scheduled for the Thanksgiving
> > > holiday in the US, it might be useful to schedule a meeting
> specifically
> > > for this. I can schedule that sync if needed.
> > >
> > > Mike
> > >
> >
>
Re: [DISCUSS] Polaris Federated Principals and Roles
Posted by Michael Collado <co...@gmail.com>.
Hey folks
Some of you already know that I posted an initial PR to get federated
principals/roles added. One thing that came out of the feedback was a spec
change to make it clear when federated identities can be used in the APIs.
Notably, federated principals cannot be created or updated, but can be
returned in get/list calls, whereas federated roles *can* be created by the
API. The latter is useful/necessary in order to be able to assign
privileges to those roles without relying on the JIT creation on login.
Please check out the spec change here and let me know what you think -
https://github.com/apache/polaris/pull/1353/files#diff-52444bc79608edfae86ed0b46d171f7ef63c20090860d877e4e135168311a986
Mike
On Tue, Dec 17, 2024 at 5:15 PM Dmitri Bourlatchkov
<dm...@dremio.com.invalid> wrote:
> Hi Mike,
>
> I left some comments in the doc, but overall it looks good to me :)
>
> I still think there are some hidden dependencies on Persistence. For
> example, whether and how we can have composite keys for persisted federated
> entities... but I guess we can work that out later.
>
> Also, I think it is important for the Authorizer API to avoid assuming that
> all principals are persisted. Specific authorizer implementations
> (including the default one) can certainly expect persisted principals, but
> the API should require that for the sake of flexibility of possible AuthN/Z
> extensions. WDYT?
>
> Cheers,
> Dmitri.
>
> On Thu, Nov 14, 2024 at 7:43 PM Michael Collado <co...@gmail.com>
> wrote:
>
> > Hey folks
> >
> > As discussed during the community sync, I've put together some thoughts
> on
> > how we'd add support for federated identities in Polaris. I copied over
> > some of what I had in the issue at
> > https://github.com/apache/polaris/issues/441 and put it into the doc
> here:
> >
> >
> >
> https://docs.google.com/document/d/15_3ZiRB6Lhzw0nxij341QUdxEIyFGTrI9_18bFIyJVo/edit?tab=t.0
> > .
> >
> > Please take a look when you get some time and let me know what you think.
> > Given that our next community sync is scheduled for the Thanksgiving
> > holiday in the US, it might be useful to schedule a meeting specifically
> > for this. I can schedule that sync if needed.
> >
> > Mike
> >
>