core: implement new authorize SPI in `PolarisAuthorizerImpl` and `OpaPolarisAuthorizer` by sungwy · Pull Request #3999 · apache/polaris

sungwy · 2026-03-15T17:07:42Z

This PR implements phases 2 and 3 of the authorization refactor tracking issue. (#3779)

It introduces request-based authorization with AuthorizationRequest, in both PolarisAuthorizerImpl and OpaPolarisAuthorizer. The new SPI method authorize is still not called by Polaris endpoint handlers. That will be done in phase 4 and 5 of the authorization refactoring effort. Hence, no behavior changes are expected in existing endpoint handling, except for ROOT handling in OpaPolarisAuthorizer noted below.

In order to decouple RBAC semantics from Polaris's core, we move RBAC operation semantics like Privileges and newly introduced Scope semantic into RbacOperationSemantics. The PR also adds FullyQualifiedPath as a lexical path with parent representation that OpaPolarisAuthorizer can use to construct the OPA request input payload.

Breaking change

OPA input payloads no longer include ROOT in resource parent chains. This is intentional, since ROOT is an internal Polaris RBAC semantic and should not be part of the external OPA request contract

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 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)

sungwy · 2026-03-15T17:09:56Z

polaris-core/src/main/java/org/apache/polaris/core/auth/FullyQualifiedPath.java

+import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
+
+/** Lexical fully qualified resource path split into leaf and ordered parent segments. */
+public record FullyQualifiedPath(PathSegment leaf, List<PathSegment> parents) {


Note to reviewer: this class was needed to absorb the logic required in converting a PolarisSecurable to a fully qualified path representation that included the correct parents and their respective PolarisEntityType that could be constructed and inserted into the OPA request payload.

I'm on the fence as to whether we'd want this to be the representation in AuthorizationRequest instead of PolarisSecurable. Leaning towards keeping them separate for now.

But this class is in polaris-core, which tends to indicate that it's not only for the OPA authorizer. Having both models in polaris-core seems a bit overkill.

This model is imho richer than PolarisSecurable. I wonder if you couldn't merge both models together, e.g.:

@PolarisImmutable public interface PolarisSecurable { /** Returns the path segments that identify the securable, in hierarchical order. */ @Nonnull List<PathSegment> getPathSegments(); default PathSegment getLeafPathSegment() { return getPathSegments().getLast(); } @Value.Check default void validate() { Preconditions.checkState(!getPathSegments().isEmpty(), "path segments must not be empty"); } }

Having both models in polaris-core seems a bit overkill.

I thought about this as well, I'm currently leaning towards keeping it in polaris-core because new PolarisAuthorizer implementations like the Ranger Authorizer may also need this.

This model is imho richer than PolarisSecurable. I wonder if you couldn't merge both models together, e.g.:

I'm open to this idea. In order to correctly construct the parent though, we'd need to pass the referenceCatalogName to an arg referenceCatalogName arg here like

default PathSegment getParents(@Nullable String referenceCatalogName) { ... }

Would that make sense?

I tend to think PolarisSecurable must know its catalog name anyway. Permission checks in Polaris are always performed within the context of a catalog 🤔 WDYT?

Yeah that's a good point.

I'm starting to think that the shape of FullyQualifiedPath is more in line with what we'd want to have in place of PolarisSecurable in the AuthorizationRequest, instead of having it represented as a PolarisEntityType and a List of String as its name.

Having clear PathSegment with ordered parents and reference catalog is a better representation of the resource... What do you folks think about just replacing PolarisSecurable with FullyQualifiedPath's shape instead?

What do you folks think about just replacing PolarisSecurable with FullyQualifiedPath's shape instead?

Works for me 👍

It sounds good yeah.

sungwy · 2026-03-15T17:11:00Z

polaris-core/src/main/java/org/apache/polaris/core/auth/FullyQualifiedPath.java

+        // Omit ROOT from fully qualified paths. It is an internal RBAC-only ancestor
+        if (resolvedParent.getEntity().getType() == PolarisEntityType.ROOT) {
+          continue;
+        }


Note to reviewer: important behavior change to remove ROOT semantics from OPA request payload, since this is an RBAC-only semantic

Ack, but we are introducing a specific behavior in polaris-core. Shouldn't we instead keep the ROOT entity here, and let specific authorizers filter it out? I feel like the ROOT case deserves some more thinking. It's also troubling that the current code sometimes prepends it, and sometimes not.

I fully agree.

public static FullyQualifiedPath of(PolarisResolvedPathWrapper wrapper)

This is only a temporary constructor introduced for compatibility, and will be removed from FullyQualifiedPath in the future. Which means ROOT semantics will no longer be in this module.

sungwy · 2026-03-15T17:12:24Z

polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java

Note to reviewer: large refactoring here to move PolarisPrivilege semantics out of PolarisAuthorizableOperation, and into a completely separate module specific to RBAC instead. See RbacOperationSemantics.java

+1 to this sub-section of the current PR. Does it depend on other changes a lot? I think it might be preferable to do this part in a dedicated PR. This way, OPA changes will hopefully be isolated from the internal RBAC changes in that commit (or at least less intertwined) WDYT?

Honestly, I had that thought as well. I'm happy to do it in a dedicated PR...

sweet, let's merge that first :)

jbonofre · 2026-03-15T17:53:29Z

polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java

+      TargetType targetType) {
+    boolean prependRootContainer =
+        switch (targetType) {
+          case TARGET -> semantics.targetScope() == PathEvaluationScope.ROOT;


I'm a bit confused here. Every operation uses PathEvaluationScope.ROOT except the policy attach/detach operations which use CATALOG.

Is this correct for catalog-scoped operations like LIST_NAMESPACES, LIST_TABLES, ...

These operations semantically happen within a catalog context. If the existing authorizeOrThrow callers already prepend the root container themselves, then ROOT make sense for backward compatibility. Maybe worth to add a comment here to clarify.

Hi @jbonofre - thanks for the early feedback on the PR :)

I was confused initially as well, so your confusion isn't unfounded. the ROOT scope here refers to whether ROOT container should be prepended to the resolved path, and hence whether the ROOT container resource should be included for privilege evaluation.

For example, PolarisAuthorizableOperation.LIST_NAMESPACES prepends root container:

polaris/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java

Line 135 in 00061ff

resolutionManifest.getResolvedPath(ResolvedPathKey.ofNamespace(namespace), true);

PolarisAuthorizableOperation.GET_APPLICABLE_POLICIES_ON_CATALOG sets prependRootContainer to false.

polaris/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java

Line 195 in 00061ff

resolutionManifest.getResolvedReferenceCatalogEntity();

I think the name PathEvaluationScope may be misleading. I'll think of a better name for this enum

adutra · 2026-03-16T12:01:33Z

polaris-core/src/main/java/org/apache/polaris/core/auth/FullyQualifiedPath.java

+import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
+
+/** Lexical fully qualified resource path split into leaf and ordered parent segments. */
+public record FullyQualifiedPath(PathSegment leaf, List<PathSegment> parents) {


But this class is in polaris-core, which tends to indicate that it's not only for the OPA authorizer. Having both models in polaris-core seems a bit overkill.

This model is imho richer than PolarisSecurable. I wonder if you couldn't merge both models together, e.g.:

@PolarisImmutable public interface PolarisSecurable { /** Returns the path segments that identify the securable, in hierarchical order. */ @Nonnull List<PathSegment> getPathSegments(); default PathSegment getLeafPathSegment() { return getPathSegments().getLast(); } @Value.Check default void validate() { Preconditions.checkState(!getPathSegments().isEmpty(), "path segments must not be empty"); } }

adutra · 2026-03-16T12:06:25Z

polaris-core/src/main/java/org/apache/polaris/core/auth/FullyQualifiedPath.java

+        // Omit ROOT from fully qualified paths. It is an internal RBAC-only ancestor
+        if (resolvedParent.getEntity().getType() == PolarisEntityType.ROOT) {
+          continue;
+        }


Ack, but we are introducing a specific behavior in polaris-core. Shouldn't we instead keep the ROOT entity here, and let specific authorizers filter it out? I feel like the ROOT case deserves some more thinking. It's also troubling that the current code sometimes prepends it, and sometimes not.

adutra · 2026-03-16T12:46:38Z

polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationRequest.java

+  @Nullable
+  @Value.Default
+  default String getReferenceCatalogName() {
+    return null;
+  }


This should be enough:

Suggested change

@Nullable

@Value.Default

default String getReferenceCatalogName() {

return null;

}

@Nullable

String getReferenceCatalogName();

But how about using an Optional here?

adutra · 2026-03-16T12:48:28Z

polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java

adutra · 2026-03-16T12:53:04Z

polaris-core/src/main/java/org/apache/polaris/core/auth/RbacOperationSemantics.java

+    ResolvedPathRooting targetScope,
+    ResolvedPathRooting secondaryScope) {
+
+  private RbacOperationSemantics(


Not sure we need this constructor, why don't you update RBAC_SEMANTICS_BY_OPERATION using EnumSet as below:

RBAC_SEMANTICS_BY_OPERATION.put( PolarisAuthorizableOperation.LIST_NAMESPACES, new RbacOperationSemantics(EnumSet.of(NAMESPACE_LIST), null, ResolvedPathRooting.ROOT, null));

It was more for convenience / beautification. Do you think using EnumSet directly is better? I'm okay either way

adutra · 2026-03-16T12:54:52Z

polaris-core/src/main/java/org/apache/polaris/core/auth/RbacOperationSemantics.java

+    EnumSet<PolarisPrivilege> targetPrivileges,
+    EnumSet<PolarisPrivilege> secondaryPrivileges,
+    ResolvedPathRooting targetScope,
+    ResolvedPathRooting secondaryScope) {


secondaryScope is either null (90% of cases), or identical to targetScope. Can we use one single ResolvedPathRooting maybe?

Yeah that's a great insight, but I also didn't want to make assumptions so I went with a safer approach of keeping them separate (feels more logical to do so)

On second thought, maybe we keep them as the same for now, since this is an internally used SPI, maybe it's okay to change it to decouple them in the future if we need to

dimas-b · 2026-03-16T17:43:49Z

polaris-core/src/main/java/org/apache/polaris/core/auth/FullyQualifiedPath.java

+import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
+
+/** Lexical fully qualified resource path split into leaf and ordered parent segments. */
+public record FullyQualifiedPath(PathSegment leaf, List<PathSegment> parents) {


nit: swap arguments maybe? In JRE's Path parents go first 🤔

Good point, but parents could be an empty list though. Feels counterintuitive to have an empty list as the first argument as well

How about we only have one List<PathSegment> in this record and add convenience methods for creating child paths based on an existing parent path?

That sounds great actually

Would a linked-list-like approach be more appropriate? E.g.

public record PathSegment(PolarisEntityType entityType, String name, @Nullable PathSegment parent) { public PathSegment(PolarisEntityType entityType, String name) { this(entityType, name, null); } public String fullyQualifiedName() { return parent == null ? name : parent.fullyQualifiedName() + "/" + name; } }

Then we might not need FullyQualifiedPath at all (but path traversals may be less efficient).

We can probably change recursion to iteration if it becomes critical.

I'm fine with either approach.

Minor comment: if we go with linked lists, I think it might be preferable to name the object something like EntityPath rather than PathSegment because it contains full path data down to the root.

dimas-b

LGTM overall - thank for keeping our AuthZ improvements going, @sungwy 😉

flyrain · 2026-03-25T23:28:37Z

Hi @sungwy , is this PR ready for review?

sungwy · 2026-03-25T23:31:37Z

Hi @flyrain - not yet, I will need to update the branch with some recent changes.

I've been at KubeCon this week, I'll get back to it soon :)

# Conflicts: # polaris-core/src/main/java/org/apache/polaris/core/auth/PathSegment.java # polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java # polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java # polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecurable.java # polaris-core/src/main/java/org/apache/polaris/core/auth/RbacOperationSemantics.java # polaris-core/src/test/java/org/apache/polaris/core/auth/AuthorizationRequestTest.java # polaris-core/src/test/java/org/apache/polaris/core/auth/RbacOperationSemanticsTest.java

implement new authorize SPI

519e5b4

github-project-automation bot added this to Basic Kanban Board Mar 15, 2026

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Mar 15, 2026

sungwy commented Mar 15, 2026

View reviewed changes

jbonofre reviewed Mar 15, 2026

View reviewed changes

clarify RBAC semantic on prepending root

c63c666

adutra reviewed Mar 16, 2026

View reviewed changes

dimas-b reviewed Mar 16, 2026

View reviewed changes

This was referenced Mar 17, 2026

core: decouple rbac privileges and semantics from PolarisAuthorizableOperations #4006

Merged

core: refactor PolarisSecurable with PathSegment #4016

Merged

sungwy added 3 commits March 28, 2026 17:15

merge new PolarisSecurable changes

4968938

preserve root handling behavior in legacy OPA API

2c782a5

Conversation

sungwy commented Mar 15, 2026

Breaking change

Checklist

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 Mar 16, 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!

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 Mar 15, 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!

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!

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 Mar 16, 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!

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 left a comment

Choose a reason for hiding this comment

Uh oh!

flyrain commented Mar 25, 2026

Uh oh!

dimas-b Mar 16, 2026 •

edited

Loading

sungwy Mar 15, 2026 •

edited

Loading

dimas-b Mar 16, 2026 •

edited

Loading