Hi all,
I agree that the authentication layer may seem complex at first sight.
I also agree that ideally, augmentors should remain loosely coupled,
without dependencies on execution order relative to other augmentors.
In Polaris, augmentors are indeed a bit too tightly coupled.
My main reservation on the proposed simplification is that Injecting
the concrete OIDC component into AuthenticatingAugmentor makes the
otherwise source-neutral authentication stage know about one
particular external mechanism. That blurs the line between internal
and external auth paths.
Also, the proposal only mentions removing
OidcPolarisCredentialAugmentor, but OidcTenantResolvingAugmentor would
need to be removed as well.
Here is a slightly modified proposal: we could consolidate all
Polaris-owned OIDC preparation (tenant resolution and credential
mapping) into a single focused CDI bean (e.g. OidcIdentityPreparer).
Then, AuthenticatingAugmentor would become the single top-level
augmentor, which roughly executes the following workflow:
1. If source == OIDC, invoke OidcIdentityPreparer, which will:
a. Invoke OidcTenantResolver
b. Invoke PrincipalMapper
c. Invoke PrincipalRolesMapper
2. Invoke DefaultAuthenticator.authenticate() for all sources
The OIDC coupling is still there, but not too invasive. I could live with that.
Thanks,
Alex
On Thu, Sep 24, 2026 at 7:41 AM Jean-Baptiste Onofré <[email protected]> wrote:
>
> Hi Yufei
>
> +1 about the OidcPolarisCredentialAugmentor as CDI component and call it
> via AuthenticatingAugmentor.
>
> The current wiring (OidcPolarisCredentialAugmentor at priority 1100 handing
> off a PolarisCredential via SecurityIdentity to AuthenticatingAugmentor at
> priority 1000) only works because of the comment on
> OidcPolarisCredentialAugmentor saying it "must run before the
> authenticating augmentor": that's an invariant the compiler can't check,
> and it's easy to break by editing a priority constant without realizing the
> ordering matters. Making it an explicit call
> (credentialAugmentor.augment(identity) then
> authenticator.authenticate(identity)) turns that into something the type
> and a reader can verify.
>
> I checked and I don't see anything else registered between priority 1000
> and 1100 (OidcTenantResolvingAugmentor is 1200), so nothing depends on
> OidcPolarisCredentialAugmentor running as a distinct
> SecurityIdentityAugmentor.
> One thing I don't know is whether any downstream/vendor deployment has
> added its own augmentor that expects to observe or mutate the
> PolarisCredential between these two steps, turning it into a plain CDI bean
> removes that extension point. If nobody is aware of this use case, we are
> good to move forward.
>
> Regards
> JB
>
> On Thu, Sep 24, 2026 at 7:28 AM Yufei Gu <[email protected]> wrote:
>
> > Hi all,
> >
> > While reviewing #5119 <https://github.com/apache/polaris/pull/5119>, I
> > found the authentication flow a little confusing:
> >
> > -
> >
> > InternalIdentityProvider and OidcPolarisCredentialAugmentor both attach
> > a PolarisCredential to the identity, for their respective authentication
> > paths.
> > -
> >
> > AuthenticatingAugmentor then consumes that credential to produce a
> > PolarisPrincipal.
> >
> > Logically, the first two are source-specific preparation steps, followed by
> > a shared authentication step. But the OIDC preparation and shared
> > authentication steps both implement SecurityIdentityAugmentor. Their
> > dependency is hidden in priorities and the credential passed through
> > SecurityIdentity, rather than expressed as a direct call.
> >
> > Could we turn OidcPolarisCredentialAugmentor into a regular CDI component
> > and have AuthenticatingAugmentor call it explicitly first, then call
> > authenticator.authenticate(identity)? Internal authentication would skip
> > that mapping because its credential is already present.
> >
> > Do downstream extensions rely on running between these two augmentors?
> > Otherwise,
> > would this make the flow clearer?
> >
> > To be clear, this is not a blocker for PR 5119.
> > Yufei
> >
