Hi all, I went ahead and created a PR implementing the proposed changes:
https://github.com/apache/polaris/pull/2390 Please take a look and let me know what you think. Thanks, Alex On Wed, Aug 13, 2025 at 9:58 AM Robert Stupp <sn...@snazy.de> wrote: > > Merging those two things SGTM. > It's what Quarkus/Vert.X > 'HttpAuthenticationMechanism'/'SecurityIdentity' do (right). > > On Wed, Aug 13, 2025 at 1:55 AM Dmitri Bourlatchkov <di...@apache.org> wrote: > > > > Thanks for starting this thread, Alex! > > > > I fully support merging Authenticator and ActiveRolesProvider. > > > > Aside from the issues you mentioned, from my perspective, it also makes > > sense conceptually. Authenticating a request implies establishing the > > principal's > > identity and consequently its roles. It is a single integration point. > > Mixing one > > Authenticator implementation with another Role Resolver looks really > > awkward. > > > > If some Authenticator implementations need to break this process down into > > two > > stages, it is ok, but Polaris core still receives one (immutable) Principal > > per request > > with relevant roles in it. > > > > Cheers, > > Dmitri. > > > > On Mon, Aug 4, 2025 at 6:45 AM Alexandre Dutra <adu...@apache.org> wrote: > > > > > Hi all, > > > > > > ActiveRolesProvider was introduced back in January, in order to enrich > > > SecurityContext with valid roles for a given principal. > > > > > > But that was before the introduction of Quarkus, and the introduction > > > of external authentication with Quarkus Security and OIDC. > > > > > > TLDR: ActiveRolesProvider became problematic since (see details > > > below), and I propose to merge ActiveRolesProvider into Authenticator. > > > This would make Authenticator the central component for resolving > > > principals and principal roles, significantly simplifying both code > > > and configuration. > > > > > > That's it :-) I'm eager to know what the community thinks. > > > > > > Thanks, > > > Alex > > > > > > --------- > > > > > > Now, for the interested readers, some low-level details. > > > ActiveRolesProvider is problematic because of its signature, and the > > > reliance on conventions: > > > > > > - Its signature expects an AuthenticatedPolarisPrincipal, thus forcing > > > the Authenticator to create a "temporary" instance of > > > AuthenticatedPolarisPrincipal, *potentially containing invalid roles*. > > > This makes the code error-prone. For example, let's assume a principal > > > has been granted "catalog_admin", but presents a JWT containing also > > > "service_admin": > > > > > > 1) JWT roles = [PRINCIPAL:ROLE:catalog_admin, > > > PRINCIPAL:ROLE:service_admin] > > > 2) Authenticator: creates a temporary > > > AuthenticatedPolarisPrincipal{roles:[PRINCIPAL:ROLE:catalog_admin, > > > PRINCIPAL:ROLE:service_admin]} > > > 3) This temporary instance contains HIGHER privileges than the user was > > > granted! > > > 4) ActiveRolesProvider: removes the wrong role > > > 5) Final SecurityContext: > > > AuthenticatedPolarisPrincipal{roles:[PRINCIPAL_ROLE:catalog_admin]} > > > > > > While the final security context is correct and does not contain > > > "service_admin", the *temporary one does contain "service_admin" and > > > is thus dangerous*. It must not be leaked (injected or used). > > > > > > - Even worse, if this temporary instance contains invalid roles, by > > > convention the default Authenticator would filter out those roles, > > > while the default ActiveRolesProvider would (again, by convention) > > > assume that, since no roles were presented, then all roles should be > > > activated. This could allow a user to obtain more roles than the JWT > > > allows them to. Let's suppose a user has "service_admin", but presents > > > a token with wrong roles: > > > > > > 1) JWT roles = [foo, bar] > > > 2) Authenticator: was expecting "PRINCIPAL_ROLE:XYZ" => removes the roles > > > 3) Authenticator: creates a temporary > > > AuthenticatedPolarisPrincipal{roles:[]} > > > 4) ActiveRolesProvider: roles is empty => assumes PRINCIPAL_ROLE:ALL > > > => activates all roles > > > 5) Final SecurityContext: > > > AuthenticatedPolarisPrincipal{roles:[PRINCIPAL_ROLE:service_admin]} > > > > > > In the above scenario, the reliance on conventions results in the > > > principal getting higher activated roles than the JWT allowed (the JWT > > > above should have resulted in no roles activated). It's not a serious > > > security issue because only roles previously granted can be activated, > > > but it is concerning, because the original JWT did not contain such > > > roles. > > > > > > Merging ActiveRolesProvider into Authenticator fixes both of the issues. > > >
