Introduce `ExternalAuthenticator` and `isExternal` flag on `PolarisCredential` by sungwy · Pull Request #3250 · apache/polaris

sungwy · 2025-12-10T21:04:20Z

Draft PR to introduce an External mode in the PolarisPrincipal. The goal of this PR is to introduce an authentication flow that skips principal resolution in the metastore for enhanced compatibility with authentication -> authorization flows that rely on external IDPs and PDPs.

This PR is motivated by @adutra 's mailing list proposal

Checklist

🛡️ Don't disclose security issues! (contact [email protected])
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

dimas-b

Thanks for pushing this feature forward, @sungwy !

Some preliminary comments, while the proposal is still being discussed.

dimas-b · 2025-12-10T22:23:37Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

    // if the principal was not found, we can end right there
    if (this.resolvedCallerPrincipal == null
        || this.resolvedCallerPrincipal.getEntity().isDropped()) {
+      if (isExternalPrincipal()) {


can this be true without returning on line 759?

It cannot - this was before I moved the condition upto line 748 to avoid principal resolution altogether if it is external. I'll remove this check here to remove redundancy

dimas-b · 2025-12-10T22:25:10Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

      List<ResolvedPolarisEntity> toValidate) {

+    if (isExternalPrincipal()) {
+      PrincipalEntity externalPrincipal = createExternalPrincipalEntity();


This is an entity without a (real) ID... Some it's a kind of ephemeral entity... Would it be possible to avoid creating it at all?

That's a good question - I'm a bit confused about the role of PrincipalEntity. We do have a public method for getResolvedCallerPrincipal() that returns resolvedCallerPrincipal. It's not being used anywhere in the codebase today, but I thought it'd be safe to populate it as it requires it to be @Nonnull: https://github.com/apache/polaris/blob/23ba2a05adc9c75f3e72aaf2ca370b4886964328/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java#L272C19-L280

Conceptually, IMHO, the principal entity should only be required for AuthZ checks (which do not actually require it if it's external). So, any intermediate code that requires it may need to be refactored... but TBH, I do not remember that code very well 😅

I think the ideal solution would be to not materialize this entity at all – and very likely, pass an empty set of activatedEntities to the PolarisAuthorizer.

But I also understand that this may increase the blast radius of this PR, and also interfere with #3228.

So, I think having a temporary entity being created here can be a good compromise, as long as we don't forget to revisit this later.

In fact I think that #3427 and resolvePathsOnly could help here.

dimas-b · 2025-12-10T22:27:05Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

  }

+  private boolean isExternalPrincipal() {
+    return Boolean.parseBoolean(polarisPrincipal.getProperties().getOrDefault("external", "false"));


WDYT about making isExternal() a method of PolarisPrincipal?

I think that's worth considering. I put it into PolarisPrincipal as a part of the property for now because it was already available, but I think adding it as an attribute or adding a class method that infers the property value is up for discussion

Same remark: maybe we should expose a getType method in PolarisPrincipal that returns PolarisPrincipalType?

dimas-b · 2025-12-10T22:31:20Z

...ce/src/main/java/org/apache/polaris/service/auth/internal/broker/NoneTokenBrokerFactory.java

+            String grantType,
+            String scope,
+            TokenType requestedTokenType) {
+          return TokenResponse.of(OAuthError.invalid_request);


I'm not sure about invalid_request... it may actually be well-formed... How about unsupported_grant_type?

That's a great suggestion - thank you!

dimas-b · 2025-12-10T22:40:06Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

  private ResolverStatus resolveCallerPrincipalAndPrincipalRoles(
      List<ResolvedPolarisEntity> toValidate) {

+    if (isExternalPrincipal()) {


sungwy · 2025-12-11T14:12:08Z

runtime/service/src/main/java/org/apache/polaris/service/auth/ExternalAuthenticator.java

+
+  @Override
+  public PolarisPrincipal authenticate(PolarisCredential credentials) {
+    PolarisCredential externalCredentials = ensureExternal(credentials);


instead of overriding the Principal to be external here, maybe we could validate that it is external and raise an exception if it isn't instead. Then we have a clearer separation of responsibility between:

detecting whether a PolarisCredential's origin was internal or external

and determining if a PolarisPrincipal should be internal, federated or external.

@adutra : WDYT?

I think indeed it would be better if this authenticator rejected any non-external PolarisCredential.

However, the DefaultAuthenticator should not do the opposite, as it is valid to have persisted principals with an external IDP so it should accept any credential, internal or external.

In fact I'm wondering whether we need this second Authenticator.

We could instead introduce a polaris.authentication.principal.type setting (overridable per realm). Valid values would be the constants of the PolarisPrincipalType enum (e.g. persisted, federated, external).

Then the DefaultAuthenticator should be able to handle all kinds of principals. It would simply read this configuration setting to determine whether it should fetch the PolarisEntity or not. (It would also, of course, validate that if the principal type is external then the PolarisCredential must be external too.)

WDYT?

github-actions · 2026-01-17T02:12:44Z

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

adutra

Thanks a lot @sungwy for this PR, and my apologies for getting to it so late!

adutra · 2026-01-19T17:56:00Z

runtime/service/src/main/java/org/apache/polaris/service/auth/PolarisCredential.java

+
+  /** True if the credential represents an external principal. */
+  @Value.Default
+  default boolean isExternal() {


Should this property have a default value at all? I think it's safer if the code building the credential explicitly decides if it's external or not.

Also thinking forward, maybe we should have an enum PolarisPrincipalType + method getPrincipalType() ?

If federated principals are implemented eventually, it would be cumbersome to have two methods isExternal and isFederated as they are mutually exclusive.

Update: maybe not here... a credential can only be internal or external, there is no "federated" credential. Please disregard my comment above.

adutra · 2026-01-19T18:12:59Z

...ce/src/main/java/org/apache/polaris/service/auth/internal/broker/NoneTokenBrokerFactory.java

+/** A no-op token broker factory used when authentication is delegated to an external IdP. */
+@ApplicationScoped
+@Identifier("none")
+public class NoneTokenBrokerFactory implements TokenBrokerFactory {


Hmm this token broker existed and was deleted a while ago by #2634. Why are we adding it back?

adutra · 2026-01-19T18:15:26Z

.../src/main/java/org/apache/polaris/service/auth/internal/InternalAuthenticationMechanism.java

  @Override
  public Set<Class<? extends AuthenticationRequest>> getCredentialTypes() {
-    return Collections.singleton(TokenAuthenticationRequest.class);
+    return Collections.singleton(InternalAuthenticationRequest.class);


Whoops this is an interesting bug that you fixed 😅 But I wonder how it worked before 🤔

adutra · 2026-01-19T18:17:24Z

runtime/service/src/main/java/org/apache/polaris/service/auth/ExternalAuthenticator.java

+
+  @Override
+  public PolarisPrincipal authenticate(PolarisCredential credentials) {
+    PolarisCredential externalCredentials = ensureExternal(credentials);


I think indeed it would be better if this authenticator rejected any non-external PolarisCredential.

However, the DefaultAuthenticator should not do the opposite, as it is valid to have persisted principals with an external IDP so it should accept any credential, internal or external.

adutra · 2026-01-19T18:27:02Z

runtime/service/src/main/java/org/apache/polaris/service/auth/ExternalAuthenticator.java

+
+  @Override
+  public PolarisPrincipal authenticate(PolarisCredential credentials) {
+    PolarisCredential externalCredentials = ensureExternal(credentials);


In fact I'm wondering whether we need this second Authenticator.

We could instead introduce a polaris.authentication.principal.type setting (overridable per realm). Valid values would be the constants of the PolarisPrincipalType enum (e.g. persisted, federated, external).

Then the DefaultAuthenticator should be able to handle all kinds of principals. It would simply read this configuration setting to determine whether it should fetch the PolarisEntity or not. (It would also, of course, validate that if the principal type is external then the PolarisCredential must be external too.)

WDYT?

adutra · 2026-01-19T18:31:08Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

  }

+  private boolean isExternalPrincipal() {
+    return Boolean.parseBoolean(polarisPrincipal.getProperties().getOrDefault("external", "false"));


Same remark: maybe we should expose a getType method in PolarisPrincipal that returns PolarisPrincipalType?

adutra · 2026-01-19T18:34:51Z

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

      List<ResolvedPolarisEntity> toValidate) {

+    if (isExternalPrincipal()) {
+      PrincipalEntity externalPrincipal = createExternalPrincipalEntity();


I think the ideal solution would be to not materialize this entity at all – and very likely, pass an empty set of activatedEntities to the PolarisAuthorizer.

But I also understand that this may increase the blast radius of this PR, and also interfere with #3228.

So, I think having a temporary entity being created here can be a good compromise, as long as we don't forget to revisit this later.

SBylemans · 2026-02-04T09:01:04Z

Hello, I'm looking to use Polaris with a keycloak integration but would appreciate this feature. Any idea in which release this PR would be made available?

sungwy · 2026-02-04T12:57:31Z

Hi @SBylemans - we are working on a refactor in the PolarisAuthorizer that will introduce this functionality.

I'm doing a pass through an RFC doc that will be shared in the dev mailing list, and once the proposal is accepted, we'll progress on with the implementation. I can cross post the RFC here as well when it is ready.

introduce ExternalAuthenticator

23ba2a0

github-project-automation bot added this to Basic Kanban Board Dec 10, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Dec 10, 2025

dimas-b reviewed Dec 10, 2025

View reviewed changes

dimas-b mentioned this pull request Dec 10, 2025

Authorizer: expose the required level of detail #3228

Closed

sungwy commented Dec 11, 2025

View reviewed changes

adutra mentioned this pull request Jan 13, 2026

[FEATURE REQUEST] SSO / Federated User/Role support #441

Open

github-actions bot added the stale label Jan 17, 2026

adutra reviewed Jan 19, 2026

View reviewed changes

adutra removed the stale label Jan 19, 2026

adutra mentioned this pull request Jan 19, 2026

core: introduce resolvePathsOnly() in Resolver and PolarisResolutionManifest #3427

Open

6 tasks

Conversation

sungwy commented Dec 10, 2025

Checklist

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adutra Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sungwy Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 17, 2026

Uh oh!

adutra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adutra Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SBylemans commented Feb 4, 2026

Uh oh!

sungwy commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

dimas-b Dec 11, 2025 •

edited

Loading

adutra Jan 19, 2026 •

edited

Loading

sungwy Dec 11, 2025 •

edited

Loading

adutra Jan 19, 2026 •

edited

Loading

sungwy commented Feb 4, 2026 •

edited

Loading