Add new PolaisAuthorizer.authorizeOrThrow for Fine Grained AuthorizableOperations #2767

HonahX · 2025-10-07T17:35:08Z

In #2697, we introduce fine grained authorizable operations which creates the need to authorize multiple authorizableOperations for one request.

This PR introduce new authorizeOrThrow api to help batch authorize multiple operations, instead of doing so in a for loop in CatalogHandler
Previous

for (PolarisAuthorizableOperation op : ops) {
      authorizer.authorizeOrThrow(
          polarisPrincipal,
          resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
          op,
          target,
          null /* secondary */);
    }

Now:

authorizer.authorizeOrThrow(
        polarisPrincipal,
        resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
        ops,
        target,
        null /* secondary */);

This will be helpful for authorizer backed by external backend, like OpaAuthorizer: #2680, which will send a rest request for every authorizeOrThrow call. Making authorizeOrThrow takes multiple operations allow the implementation to send a batch authorization request to the backend.

cc: @travis-bowen @collado-mike @dimas-b

…tions

singhpk234 · 2025-10-07T17:43:36Z

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

+  void authorizeOrThrow(
+      @Nonnull PolarisPrincipal polarisPrincipal,
+      @Nonnull Set<PolarisBaseEntity> activatedEntities,
+      @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+      @Nullable PolarisResolvedPathWrapper target,
+      @Nullable PolarisResolvedPathWrapper secondary);
+
+  void authorizeOrThrow(
+      @Nonnull PolarisPrincipal polarisPrincipal,
+      @Nonnull Set<PolarisBaseEntity> activatedEntities,
+      @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+      @Nullable List<PolarisResolvedPathWrapper> targets,
+      @Nullable List<PolarisResolvedPathWrapper> secondaries);


since this is a public interface, do you wanna may be add some function docs for this ?

also do we wanna mark the existing API's deprecated ? since these APIs can superseed them

do you wanna may be add some function docs for this ?

Sounds great!

also do we wanna mark the existing API's deprecated ? since these APIs can superseed them

I think we should keep the existing ones since they’re widely used across the codebase — they serve as polymorphic helpers.
However, we could provide a default implementation that delegates to the only version requiring an actual implementation. WDYT?

void authorizeOrThrow( @Nonnull PolarisPrincipal polarisPrincipal, @Nonnull Set<PolarisBaseEntity> activatedEntities, @Nonnull Set<PolarisAuthorizableOperation> authzOps, @Nullable List<PolarisResolvedPathWrapper> targets, @Nullable List<PolarisResolvedPathWrapper> secondaries);

I think this probably deserves a mailing list discussion, but I wonder if this API change is really enough to support batching everything together into one call. This API makes the assumption that all operations have the same primary and secondary entities. Is that always going to be the case? or are we fine to kick that can down the road?

Sounds good, will start a discussion!

IMHO, batch AuthorizableOperations should mainly be used for fine-grained cases like updateTable, where a single endpoint handles multiple types of updates that each require distinct authorization controls. The original intention behind AuthorizableOperation was that each operation corresponds to one API call, and updateTable is a special exception that needs finer-grained handling.

In the updateTable case, there’s a sort of internal inheritance — for example, a fine-grained operation like TABLE_ADD_SNAPSHOT conceptually extends a broader one such as TABLE_WRITE_PROPERTIES. All these fine-grained operations should share the same set of primary and secondary securable entities; it’s just that this “inheritance” relationship hasn’t been reflected in the code yet.

If two operations involve different sets of securables, they should ideally be authorized through separate API calls, each likely corresponding to its own endpoint.

If in the future the above assumption does not hold, I assume we will need large change anyway and thus a new set of apis will be introduced.

But the above are just my thoughts, let's discuss in dev mailing list to collect thoughts on the new api and see what we want to support : )

dimas-b

LGTM 👍 Just a small code maintenance suggestion.

dimas-b · 2025-10-07T22:41:25Z

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

      @Nullable List<PolarisResolvedPathWrapper> targets,
      @Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+  void authorizeOrThrow(


Would it be possible to add default (possibly inefficient) implementations to the new methods to simplify transition in downstream projects? This is just a matter of convenience, so optional.

dimas-b · 2025-10-07T22:43:46Z

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

-    } else if (authzOp == PolarisAuthorizableOperation.RESET_CREDENTIALS) {
-      if (!isRoot) {
-        throw new ForbiddenException("Only Root principal(service-admin) can perform %s", authzOp);
+    for (PolarisAuthorizableOperation authzOp : authzOps) {


This loop could be done in a default impl. of the new method (redirecting to old methods). Performance-wise it would be the same, as far as I can tell, but it would have an easier migration path downstream.

travis-bowen

Given the mailing list discussion agrees on the approach, this implementation looks good.

The functional java docs would be useful though.

travis-bowen · 2025-10-08T19:50:59Z

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

+      @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+      @Nullable PolarisResolvedPathWrapper target,
+      @Nullable PolarisResolvedPathWrapper secondary) {
+    authorizeOrThrow(


Honestly this and the other authorizeOrThrow (with single op and single target / secondary) would probably be nice to have a default as well to default to calling the option with a list of targets and secondaries.

The Polaris impls end up just delegating these other impls often do as well.

dimas-b · 2025-10-08T21:06:53Z

@travis-bowen :

Given the mailing list discussion agrees on the approach, [...]

Could you link the ML discussion? I might have missed it 😅

HonahX · 2025-10-12T02:10:54Z

Sorry seems there is some confusion around the dev list discussion. Just to clarify, the discussion for this PR is this thread:
https://lists.apache.org/thread/qgbrd959dyjsncr7wkfd8yz5xq6pl64q (which is still been actively discussed) : ). Thanks everyone who have already chimed in here!

sungwy · 2025-10-17T14:09:10Z

Hi folks, if we are fairly certain on moving forward with this PR, I'll make a change to the OpaPolarisAuthorizer PR to pass a list of actions instead of a single one, in the request payload: #2680

dimas-b · 2025-10-17T14:33:19Z

I'm not sure about making OPA depend on this PR 🤔 E.g. recent discussions in https://lists.apache.org/thread/dqk65wjyqp72po2mcbsvskj8pzygc3dm might require changes to the Authorizer API too... so this might turn into a bigger/longer effort 🤔

If we can make progress on OPA under the current API, let's not wait. If these PRs clash at merge time, it will be unfortunate, but I hope we can deal with that as we collect approvals.

Add new authorizeOrThrow variation for fine grained authorizableOpera…

9f9425d

…tions

github-project-automation bot added this to Basic Kanban Board Oct 7, 2025

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

singhpk234 reviewed Oct 7, 2025

View reviewed changes

dimas-b approved these changes Oct 7, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Oct 7, 2025

travis-bowen approved these changes Oct 8, 2025

View reviewed changes

Add new PolaisAuthorizer.authorizeOrThrow for Fine Grained AuthorizableOperations #2767

Are you sure you want to change the base?

Add new PolaisAuthorizer.authorizeOrThrow for Fine Grained AuthorizableOperations #2767

Conversation

HonahX commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

singhpk234 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

singhpk234 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

HonahX Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

collado-mike Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

HonahX Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

dimas-b Oct 7, 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!

travis-bowen left a comment

Choose a reason for hiding this comment

Uh oh!

travis-bowen Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

HonahX commented Oct 12, 2025

Uh oh!

sungwy commented Oct 17, 2025

Uh oh!

dimas-b commented Oct 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HonahX commented Oct 7, 2025 •

edited

Loading

dimas-b Oct 7, 2025 •

edited

Loading

dimas-b commented Oct 8, 2025 •

edited

Loading