core: decouple rbac privileges and semantics from `PolarisAuthorizableOperations` by sungwy · Pull Request #4006 · apache/polaris

sungwy · 2026-03-17T03:06:14Z

Refactoring to decouple RBAC specific privilege based authorization semantics from PolarisAuthorizableOperations. The PR includes a unit test suite that verifies that all PolarisAuthorizableOperations have their corresponding RbacOperationSemantics defined with a targetPrivilege.

No behavior change is introduced in this PR.

Note: This is an off-shoot from #3999.

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)

…ions

Copilot

Pull request overview

Refactors authorization logic to move RBAC privilege semantics out of PolarisAuthorizableOperation into a dedicated RbacOperationSemantics mapping, with tests ensuring every operation has explicit semantics defined.

Changes:

Introduced RbacOperationSemantics record to define RBAC privilege requirements per operation (plus rooting metadata).
Updated PolarisAuthorizerImpl to use RbacOperationSemantics for privilege checks instead of embedding privileges in the enum.
Simplified PolarisAuthorizableOperation to intent-level operations only and added unit tests validating the semantics mapping and constructor invariants.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
polaris-core/src/main/java/org/apache/polaris/core/auth/RbacOperationSemantics.java	Adds centralized RBAC semantics mapping and validation for all authorizable operations.
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java	Switches authorization checks to read required privileges from `RbacOperationSemantics`.
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java	Removes embedded RBAC privilege definitions, leaving a pure operation enum.
polaris-core/src/test/java/org/apache/polaris/core/auth/RbacOperationSemanticsTest.java	Adds tests ensuring complete semantics coverage and validates constructor behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

polaris-core/src/test/java/org/apache/polaris/core/auth/RbacOperationSemanticsTest.java

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

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

flyingImer

submit placeholder review to allow republish with improved draft

flyingImer

Summary

This review uses correctly posted body via submit API test.

dimas-b · 2026-03-18T21:56:24Z

@sungwy : it looks like we got conflicts from the PRs removing unused operation values 😅

dimas-b · 2026-03-18T22:05:00Z

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

+record RbacOperationSemantics(
+    Set<PolarisPrivilege> targetPrivileges,
+    Set<PolarisPrivilege> secondaryPrivileges,
+    ResolvedPathRooting rooting) {


How is rooting intended to be used? I do not see any readers of this value 🤔

Is it used in a follow-up PR?

Yes it is intended for use in #3999

Maybe it would make sense for me to remove this in the scope of this PR. And add it in #3999 instead

up to you... I was just wondering about the usage pattern... I'll check the other PR.

I'm still a bit fuzzy about the "prepend root container" logic, but it looks like it existed since day 1, so we're not changing it in these PRs, just making inputs nicer 👍

yes, exactly - currently the logic is embedded into the Handler call sites and I think it should be defined here as an RBAC semantic instead.

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

flyrain · 2026-03-20T16:48:58Z

Thanks a lot for the change, @sungwy ! The PR is in the right direction. Checked with @dennishuo a bit yesterday. He is interested in taking a look. Given the impact of this refactor, can we give a bit more time for reviewers?

dimas-b · 2026-03-20T16:52:43Z

@flyrain : this PR has been merged, but follow-up comments and changes are welcome, of course... or are you proposing to revert it?

sungwy · 2026-03-20T18:29:22Z

Hi @flyrain, thank you for the note.

As @dimas-b mentioned, I went ahead and merged this PR after the series of reviews here and on #3999. Since no use facing changes are introduced here, I felt it was reasonable to move things forward once it was approved.

That said, I’m very open to feedback. If there are concerns about the direction of the refactor, I’m happy to pause any follow-up changes while we take a closer look.

@dennishuo I would really appreciate your thoughts when you have a chance. Happy to adjust based on your feedback.

decouple rbac privileges and semantics from PolarisAuthorizableOperat…

cbb83ca

…ions

Copilot AI review requested due to automatic review settings March 17, 2026 03:06

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

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

refactor

86090bd

sungwy requested a review from dimas-b March 17, 2026 03:11

Copilot AI reviewed Mar 17, 2026

View reviewed changes

Copilot started reviewing on behalf of sungwy March 17, 2026 03:15 View session

adutra reviewed Mar 17, 2026

View reviewed changes

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

Merge remote-tracking branch 'origin/main' into decouple-rbac-from-ops

1c234bc

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

flyingImer reviewed Mar 17, 2026

View reviewed changes

make set immutable

61c6da8

dimas-b reviewed Mar 18, 2026

View reviewed changes

dimas-b previously approved these changes Mar 18, 2026

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Mar 18, 2026

Merge remote-tracking branch 'origin/main' into decouple-rbac-from-ops

a755f5c

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

sungwy dismissed dimas-b’s stale review via a755f5c March 19, 2026 01:08

sungwy requested a review from dimas-b March 19, 2026 01:38

dimas-b approved these changes Mar 19, 2026

View reviewed changes

sungwy mentioned this pull request Mar 19, 2026

Tracking Issue: Refactor PolarisAuthorizer SPI to skip native Principal and RBAC resolution requirement in Polaris #3779

Open

sungwy merged commit 4521081 into apache:main Mar 20, 2026
18 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Mar 20, 2026

sungwy deleted the decouple-rbac-from-ops branch March 20, 2026 02:33

dimas-b mentioned this pull request Mar 24, 2026

Implement Apache Ranger Authorization Plugin for Apache Polaris (#274) #3928

Open

6 tasks

Conversation

sungwy commented Mar 17, 2026

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flyingImer left a comment

Choose a reason for hiding this comment

Uh oh!

flyingImer left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

dimas-b commented Mar 18, 2026

Uh oh!

dimas-b Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

dimas-b Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

sungwy Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

dimas-b Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

dimas-b Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

sungwy Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flyrain commented Mar 20, 2026

Uh oh!

dimas-b commented Mar 20, 2026

Uh oh!

sungwy commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants