Hi Dennis and Mike, Thank you both for your flexibility regarding this change. Please feel free to reach out with any questions or comments.
> In general I think if we can make it clear in the code everywhere whether > we're dealing with "requestedScopes" or "validatedActiveRoles" and never > reuse the same Collection to mean both that'd help maintainability long-term You're absolutely right. Once all security augmentors have been invoked, we should have the final, definitive version of the authenticated principal and their validated roles. At this point, the principal and roles should be read-only, and the roles should be considered validated, no longer mere "scopes" or JWT claims. This has a significant implication: checking grant records between the principal and the requested principal roles effectively becomes a responsibility of the authentication process, handled within the Authenticator. However, checking grant records between the principal roles and the catalog roles is more of a business concern and remains a subsequent phase, handled in the Resolver. I believe this distinction is acceptable and establishes a clear division of responsibilities. Thanks, Alex On Thu, Aug 21, 2025 at 7:43 AM Michael Collado <collado.m...@gmail.com> wrote: > > Hi Alex > > Thanks for this. Admittedly, the ActiveRolesProvider was *intended* to > support the future OIDC/Quarkus support, but that was long before the > current HttpAuthenticationMechanism. > > I'm going to check out this branch and validate, but I think it's a good > change. It helps with some changes I had planned to make, but hadn't had a > chance to work on yet. > > Mike > > On Tue, Aug 19, 2025 at 6:49 AM Alexandre Dutra <adu...@apache.org> wrote: > > > 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. > > > > > > >
