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.
> >
