You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "gracechen09 (via GitHub)" <gi...@apache.org> on 2026/06/19 03:59:21 UTC
[PR] Entity-level filtering: API and feature flag [polaris]
gracechen09 opened a new pull request, #4831:
URL: https://github.com/apache/polaris/pull/4831
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
This PR introduces the API for entity-level visibility filtering on LIST operations, which is the first part of the implementation of [proposal](https://docs.google.com/document/d/1PyIISgK2FVK0m8t4104KZpKAWb4d1Wy0UhZQzut4pIc/edit?usp=sharing), and has been discussed in dev mailing list [thread1](https://lists.apache.org/thread/w58zrjt6jpq33mv9lvqzndqc6no5l7xb) and [thread2](https://lists.apache.org/thread/k3g6vjkx53rf6s89v9yyrtj43blvzklj).
Changes:
- added a new request object and a new entity-filtering method with a backward-compatible default implementation
- added feature flag
Follow-up PRs will be:
1. implementation of `filterByVisibility` for RBAC authorizer
2. implementation of `filterByVisibility` for OPA authorizer
3. caller integration with `filterByVisibility`
## 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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3496634155
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
Any hope the API evolves to
1. be reactive (CompletionStage)
2. not filter in memory but enables pushdown upfront of the filtering in a backend (like zanzibar or precomputed permission database)
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5331077570
Reopening as I think this feature is valuable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3501657485
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
but in general, I'd +1 moving towards more concurrent and more scalable code :wink:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5334456213
thanks @adutra for helping re-opening the PR!
Have been working on discussed implementation and plan to push the changes by end of this week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4253000307
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
OPA gets the full remote namespace/table name from the candidate's securable name, but Ranger builds its resource from the resolved Polaris path. Without a local entity for that child, Ranger gets a namespace/catalog resource and cannot distinguish remote-only sibling policies. Requiring ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS for every provider would disable useful OPA filtering. That flag enables creation of synthetic local entities for grants, but does not guarantee that every remote child is registered locally. Could we define the supported federated behavior per provider and demonstrate it with a real-provider case for a remote-only table? Before claiming per-child support, that case should show the full candidate resource, or we should explicitly restrict the unsupported path while preserving OPA's name-based filtering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3510206082
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
this makes sense to me, I agree on using a batch method rather than a separate filterbyVisibility.
Laying out proposed approaches from the discussions:
Approach 1: reusing existing authorize() method
`List<AuthorizeDecision> authorize(state, batchRequest)`
pros:
- solves the duplication problem
- avoids the potential implantation drift between two authorize methods
cons:
- all single-entity authorization calls will have extra overheads for list creation and unwrapping
- need to migrate on the caller-sites
Approach 2: creating a new authorize() method and retrofit the old methods into it
`List<AuthorizeDecision> authorizeAll(state, batchRequest)`
retrofit old method:
```
default AuthorizeDecision authorize(state, request) {
return authorizeBatch(state, toBatch(request).get(0);
}
```
pros: same as approach 1 + no need to migrate the caller-sites immediately
cons: same as approach 1
Approach 3: creating a new authorize() method intended for batch
keep old method
`AuthorizationDecision authorize(state, request);`
create new batch method
`List<AuthorizationDecision> authorize(state, batchRequest);`
pros:
- no impact on existing callers
- each method has clean semantics
cons:
- doesn't solve the duplication problem
- potential implementation drift between the two
I'm a bit leaning toward approach 3 since the existing single-entity callers don't benefit from batch method, and the consistency and deduplication are also very appealing point for approach 1&2.
What do you think about the single-entity callers concern?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3503019404
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
Thanks for the review! I'll address comments one by one, starting with this suggestion.
Yes, filterByEntity needs to always be aligned with passing the data operation and securable through authorize(), and this creates some redundancy.
Just confirming my understanding, instead of a separate filterByVisibility method on the authorizer, are you proposing something like? Extending authorize() to accept a list of requests and return per-entity auth decisions, and the filtering can be triggered by caller. The caller-site (LIST operations) interprets the auth decisions for each entity, then returns the authorized entities.
I think the batch authorize() interface makes sense! My concern is: the filtering algorithm includes a short-circuit logic (if the caller holds the visibility privilege on the parent, skip per-entity checks via inheritance). This optimization is authorizer-specific, for example, only RBAC authorizer uses privileges inheritance, but an OPA authorizer may use a different model. So the authorizer would need to own some part of batch authorization logic.
Would like you hear your thoughts!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4396715130
##########
extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/RangerPolarisAuthorizer.java:
##########
@@ -130,6 +146,24 @@ private AuthorizationDecision authorizeIntent(
}
}
+ private static boolean isEntityVisibilityCheck(PolarisAuthorizableOperation operation) {
Review Comment:
nit: rename it to `isEntityListOperation` to be more descriptive?
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java:
##########
@@ -21,12 +21,27 @@
/** Intent-level Polaris operations that can be authorized. */
public enum PolarisAuthorizableOperation {
LIST_NAMESPACES,
+ /**
+ * Checked for each namespace when listNamespaces filters its results for a caller who lacks
+ * {@link #LIST_NAMESPACES} on the parent namespace or catalog: a namespace is returned only if
+ * this check passes. Separate from LIST_NAMESPACES so that authorizers can decide access to each
+ * namespace independently of the parent-level permission. The built-in RBAC authorizer maps it to
+ * {@code NAMESPACE_READ_PROPERTIES}.
+ */
+ LIST_NAMESPACES_ENTITY,
Review Comment:
Could we call it LIST_NAMESPACE instead of LIST_NAMESPACES_ENTITY? A singular form indicates the operation only applies to a single ns.
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java:
##########
@@ -21,12 +21,27 @@
/** Intent-level Polaris operations that can be authorized. */
public enum PolarisAuthorizableOperation {
LIST_NAMESPACES,
+ /**
+ * Checked for each namespace when listNamespaces filters its results for a caller who lacks
+ * {@link #LIST_NAMESPACES} on the parent namespace or catalog: a namespace is returned only if
+ * this check passes. Separate from LIST_NAMESPACES so that authorizers can decide access to each
+ * namespace independently of the parent-level permission. The built-in RBAC authorizer maps it to
+ * {@code NAMESPACE_READ_PROPERTIES}.
+ */
+ LIST_NAMESPACES_ENTITY,
CREATE_NAMESPACE,
LOAD_NAMESPACE_METADATA,
NAMESPACE_EXISTS,
DROP_NAMESPACE,
UPDATE_NAMESPACE_PROPERTIES,
LIST_TABLES,
+ /**
+ * Checked for each table when listTables filters its results for a caller who lacks {@link
+ * #LIST_TABLES} on the parent namespace: a table is returned only if this check passes. Separate
+ * from LIST_TABLES so that authorizers can decide access to each table independently of the
+ * parent-level permission. The built-in RBAC authorizer maps it to {@code TABLE_READ_PROPERTIES}.
+ */
+ LIST_TABLES_ENTITY,
Review Comment:
We could reuse `LIST_TABLES` and distinguish the checks by target type, a namespace for listing all tables, and an individual table for deciding whether it appears in the results. However, I think a new op is probably fine, and consistent with the namespace op (we cannot reuse `LIST_NAMESPACES` for `LIST_NAMESPACES_ENTITY` since the target could be both ns). With that, similar to https://github.com/apache/polaris/pull/4831/changes#r4396851364, could you call it `LIST_TABLE` given that this op is for a single table?
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java:
##########
@@ -48,6 +63,13 @@ public enum PolarisAuthorizableOperation {
RENAME_TABLE,
COMMIT_TRANSACTION,
LIST_VIEWS,
+ /**
+ * Checked for each view when listViews filters its results for a caller who lacks {@link
+ * #LIST_VIEWS} on the parent namespace: a view is returned only if this check passes. Separate
+ * from LIST_VIEWS so that authorizers can decide access to each view independently of the
+ * parent-level permission. The built-in RBAC authorizer maps it to {@code VIEW_READ_PROPERTIES}.
+ */
+ LIST_VIEWS_ENTITY,
Review Comment:
Same here, --> `LIST_VIEW`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3562403611
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
Re: `List<AuthorizationDecision> authorize(state, batchRequest)` - that LGTM :+1:
Re: `AuthorizationDecision authorize(state, request)` - this method exists, but it is NOT currently called.
Perhaps it might be best to migrate all authZ callers to use `authorize(state, request)` first, remove old `authorizeOrThrow` methods, then add batches, then revisit this PR. WDYT?
CC: @sungwy
Cf. #5034
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3510206082
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
this makes sense to me, I agree on using a batch method rather than a separate filterbyVisibility.
Laying out proposed approaches from the discussions:
Approach 1: reusing existing authorize() method
`List<AuthorizeDecision> authorize(state, batchRequest)`
pros:
- solves the duplication problem
- avoids the potential implantation drift between two authorize methods
cons:
- all single-entity authorization calls will have extra overheads for list creation and unwrapping
- need to migrate on the caller-sites
Approach 2: creating a new authorize() method and retrofit the old methods into it
`List<AuthorizeDecision> authorizeAll(state, batchRequest)`
retrofit old method:
```
default AuthorizeDecision authorize(state, request) {
return authorizeBatch(state, toBatch(request).get(0);
}
```
pros: same as approach 1 + no need to migrate the caller-sites immediately
cons: same as approach 1
Approach 3: creating a new authorize() method intended for batch
keep old method
`AuthorizationDecision authorize(state, request);`
create new batch method
`List<AuthorizationDecision> authorize(state, batchRequest);`
pros:
- no impact on existing callers
- each method has clean semantics
cons:
- doesn't solve the duplication problem
- potential implementation drift between the two
I'm a bit leaning toward approach 3 since the existing single-entity callers don't benefit from batch method, but the consistency and deduplication are also very appealing point for approach 1&2.
What do you think about the single-entity callers concern?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3920478285
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1778,4 +1837,110 @@ private ImmutableLoadCredentialsResponse fallbackToFullLoadTable(
.credentials(loadTableResponse.credentials())
.build();
}
+
+ private boolean isEntityLevelListFilteringEnabled() {
+ return realmConfig().getConfig(ENABLE_ENTITY_LEVEL_LIST_FILTERING, getResolvedCatalogEntity());
+ }
+
+ private <T> List<T> filterEntities(
+ List<T> entities,
+ Function<T, ResolverPath> toResolverPath,
+ Function<T, PolarisSecurable> toSecurable,
+ PolarisAuthorizableOperation op) {
+ if (entities.isEmpty()) {
+ return entities;
+ }
+ PolarisResolutionManifest filterManifest = newResolutionManifest();
+ List<ResolverPath> paths = new ArrayList<>(entities.size());
+ for (T entity : entities) {
+ ResolverPath path = toResolverPath.apply(entity);
+ paths.add(path);
+ filterManifest.addPath(path);
+ }
+ filterManifest.resolveAll();
+
+ AuthorizationState authzState = new AuthorizationState(filterManifest);
+
+ var principal = polarisPrincipal();
+ List<T> resolvable = new ArrayList<>();
+ List<AuthorizationRequest> requests = new ArrayList<>();
+ int pathIndex = 0;
+ for (T entity : entities) {
+ // Look the path up under the same key addPath registered it with, so the lookup key
+ // cannot drift from the registered one.
+ ResolverPath path = paths.get(pathIndex++);
+ if (filterManifest.getResolvedPath(ResolvedPathKey.of(path), true) != null) {
+ resolvable.add(entity);
+ requests.add(
+ new AuthorizationRequest(
+ principal,
+ List.of(new SingleTargetAuthorizationIntent(op, toSecurable.apply(entity)))));
+ }
+ }
+
+ List<AuthorizationDecision> decisions = authorizer().authorize(authzState, requests);
Review Comment:
This is a good question! We have some options:
Option 1: the entity filter can use the existing resolveAuthorizationInputs method.
```
authorizer().resolveAuthorizationInputs(authzState, allRequests.get(0));
```
Option 2: add a batch overload. For example:
```
@Override
public void resolveAuthorizationInputs(
AuthorizationState authzState, List<AuthorizationRequest> requests) {
authzState.getResolutionManifest().resolveAll();
}
```
This is more explicit but the batch's implementation would be identical to that of the single-request.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3495847650
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
This conceptually overlaps with the new `authorize()` method (line 53).
Once can call this method and deduce the authorization decision for the entity from the fact that is it excluding (or included) in the output of this method. At the same time, this should always be aligned with passing the data "operation" and "securable" through the `authorize()` method, right?
Would it be possible to have one main authorization method that would return an `AuthorizationDecision` for each element given a list of inputs. It would be called directly for filtering use cases. Simpler operations would call via via convenience utility methods to simplify default with singular parameters.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4831: Entity-level filtering: API and feature flag
URL: https://github.com/apache/polaris/pull/4831
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5944104613
Hi @gracechen09, thanks for the update. The PR is still in draft state. Is it ready for a review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-4824691529
> Thanks for driving this direction!
>
> I read the linked doc and I'm not sure I fully follow the problem statement. I think it merges two authorization axes that Polaris keeps separate today: discoverability and metadata access.
>
> Discoverability under a parent is the LIST privilege. TABLE_LIST is granted on the namespace, and listTables checks exactly that before returning the children. Metadata access is a separate per-entity grant (*_READ_PROPERTIES). The two don't gate each other: not holding TABLE_LIST on a namespace doesn't stop you from loading a table you were granted directly, and holding TABLE_LIST is what lets you see the child names under that namespace.
>
> On the two problems in the doc:
>
> * Discoverability: "I can access a table but can't list it without namespace LIST" looks like the model working as intended, not a gap. The grant gives you access to the entity, and enumerating names under the parent is what LIST grants, separately.
> * Visibility of unauthorized entities: I'd keep "unauthorized" (no metadata or data access) apart from "shows up in a list". A name appearing under a parent you hold LIST on is the LIST privilege doing its job, even if you can't read that entity's contents. Suppressing names from a LIST-privileged caller is a different requirement (anti-enumeration), not the current model misbehaving.
>
> Can we re-pin the problem statements?
Thank you for reviewing the proposal, this is a good point! The original problem statement conflates two distinct authorization concerns, therefore I updated the proposal to remove the discoverability from the problem statement. The motivation for this proposal is now aligned with the proposed solution, which is for user who holds LIST_* privilege on a parent should only receive child entities they have access to. The other case where a user holds per-entity grants but no parent-level LIST privileges is moved to Future Work section and requires more discussion in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3562403611
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
Re: `List<AuthorizationDecision> authorize(state, batchRequest)` - that LGTM :+1:
Re: `AuthorizationDecision authorize(state, request)` - this method exists, but it is NOT currently called.
Perhaps it might be best to migrate all authZ callers to use `authorize(state, request)` first, remove old `authorizeOrThrow` methods, then add batches then revisit this PR. WDYT?
CC: @sungwy
Cf. #5034
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3500520680
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
slowly moving to reactive code to use a remote backend, this has some serious impact on scalability (hundreds vs thousands of concurrent requests)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4365094837
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
I donโt think the catalog handler should depend on which authorizer is configured. It should construct authorization intents and filter results using the decisions returned through the common interface. Differences in how authorizers resolve resources and evaluate permissions should be encapsulated within their implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3501653586
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
Are virtual threads a possible alternative?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3503019404
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
Thanks for the review! I'll address comments one by one, starting with this suggestion.
Yes, filterByEntity needs to always be aligned with passing the data operation and securable through authorize(), and this creates some redundancy.
Just confirming my understanding, instead of a separate filterByVisibility method on the authorizer, are you proposing something like? Extending authorize() to accept a list of requests and return per-entity auth decisions, and the filtering can be triggered by caller. The caller-site (LIST operations) interprets the auth decisions for each entity, then returns the authorized entities.
I think the batch authorize() interface makes sense! My concern is: the filtering algorithm includes a short-circuit logic (if the caller holds the visibility privilege on the parent, skip per-entity checks via inheritance). This optimization is authorizer-specific, for example, only RBAC authorizer uses privileges inheritance, but an OPA authorizer may use a different model. So the authorizer would need to own some part of batch authorization logic.
Would like hear about your thoughts!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5974423200
> Hi @gracechen09, thanks for the update. The PR is still in draft state. Is it ready for a review?
Hi @flyrain thanks for checking in, this is ready for review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4242308270
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -400,20 +419,31 @@ public ListTablesResponse listTables(Namespace namespace, String pageToken, Inte
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES;
authorizeBasicNamespaceOperationOrThrow(op, namespace);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
Review Comment:
good idea, renamed the method name to `shouldFilterListResults()` and called it inline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5398260801
@gracechen09, thanks for keeping working on it. The PR is still in draft state. Is it ready for a review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3867122965
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1778,4 +1837,110 @@ private ImmutableLoadCredentialsResponse fallbackToFullLoadTable(
.credentials(loadTableResponse.credentials())
.build();
}
+
+ private boolean isEntityLevelListFilteringEnabled() {
+ return realmConfig().getConfig(ENABLE_ENTITY_LEVEL_LIST_FILTERING, getResolvedCatalogEntity());
+ }
+
+ private <T> List<T> filterEntities(
+ List<T> entities,
+ Function<T, ResolverPath> toResolverPath,
+ Function<T, PolarisSecurable> toSecurable,
+ PolarisAuthorizableOperation op) {
+ if (entities.isEmpty()) {
+ return entities;
+ }
+ PolarisResolutionManifest filterManifest = newResolutionManifest();
+ List<ResolverPath> paths = new ArrayList<>(entities.size());
+ for (T entity : entities) {
+ ResolverPath path = toResolverPath.apply(entity);
+ paths.add(path);
+ filterManifest.addPath(path);
+ }
+ filterManifest.resolveAll();
+
+ AuthorizationState authzState = new AuthorizationState(filterManifest);
+
+ var principal = polarisPrincipal();
+ List<T> resolvable = new ArrayList<>();
+ List<AuthorizationRequest> requests = new ArrayList<>();
+ int pathIndex = 0;
+ for (T entity : entities) {
+ // Look the path up under the same key addPath registered it with, so the lookup key
+ // cannot drift from the registered one.
+ ResolverPath path = paths.get(pathIndex++);
+ if (filterManifest.getResolvedPath(ResolvedPathKey.of(path), true) != null) {
+ resolvable.add(entity);
+ requests.add(
+ new AuthorizationRequest(
+ principal,
+ List.of(new SingleTargetAuthorizationIntent(op, toSecurable.apply(entity)))));
+ }
+ }
+
+ List<AuthorizationDecision> decisions = authorizer().authorize(authzState, requests);
Review Comment:
I think the batch API is missing the input-resolution half of the existing authorization contract. For a single request, PolarisAuthorizer.resolveAuthorizationInputs() lets each authorizer prepare the state it needs before authorize(). Here the caller instead pre-resolves a PolarisResolutionManifest for the whole batch and then calls authorize() directly, which assumes the built-in RBAC resolution shape and gives other authorizers no equivalent batch preparation hook.
Could we define how resolveAuthorizationInputs() is supposed to work for the batch path before wiring the caller? I don't think this caller should have to substitute authorizer-specific resolution with direct PolarisResolutionManifest resolution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4331658577
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -819,4 +819,15 @@ public static void enforceFeatureEnabledOrThrow(
+ "configured, but existing values cannot be changed to different ones.")
.defaultValue(false)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean> ENABLE_ENTITY_LEVEL_LIST_FILTERING =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ENABLE_ENTITY_LEVEL_LIST_FILTERING")
+ .catalogConfig("polaris.config.enable-entity-level-list-filtering")
+ .description(
Review Comment:
both proposal and PR are updated with the suggested option
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3859460339
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
sorry I missed this thread, this makes sense! I have some draft codes for what we discussed above and I'm keeping an eye on the ongoing authz callersite migration PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3503974007
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
virtual threads can be an option even if there are still cases they lead to antipatterns, I always prefer to explicit state by the signature the method is not compute only, `CompletionStage` enables to make it generic and quarkus independent/more interop (nicer for extensions IMHO) otherwise mutiny works in a quarkus only world
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3502472661
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/VisibilityFilterRequest.java:
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core.auth;
+
+import java.util.List;
+import org.jspecify.annotations.NonNull;
+
+/** Carries the context used to filter LIST-operation candidates by visibility. */
+public record VisibilityFilterRequest(
+ @NonNull PolarisAuthorizableOperation listOperation,
Review Comment:
`container` is `@NonNull PolarisSecurable`, but `PolarisSecurable.validate()` forbids ROOT segments (PolarisSecurable.java:81) and requires a top-level start (:77) , so there's no way to represent the root container that `LIST_CATALOGS` needs, even though the flag description names `listCatalogs`. I think this is another reason to reuse the existing method `authorize()` and expand on it.
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
Agreed with @dimas-b here. Reusing the existing `authorize()` avoids concept-level duplication. To achieve that, we also need some changes in `AuthorizationDecision` so that it can return the decision for a list of securables. Here is the general process:
1. Authorize the list operation on the container.
2. Get a list of securables under the container.
3. Bulk authorize a list of securables under that container, returning the decision on each securable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4213699323
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -819,4 +819,15 @@ public static void enforceFeatureEnabledOrThrow(
+ "configured, but existing values cannot be changed to different ones.")
.defaultValue(false)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean> ENABLE_ENTITY_LEVEL_LIST_FILTERING =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ENABLE_ENTITY_LEVEL_LIST_FILTERING")
+ .catalogConfig("polaris.config.enable-entity-level-list-filtering")
+ .description(
Review Comment:
this makes sense, after walking through the behavioral change for these two options, I agree the current option may have wider impact with the breaking change. And the suggestion solves breaking change issue and fixes #4573. Here's a quick comparison.
Option A (current PR):` *_LIST` on the container only allows the list request to proceed; each returned entity also needs `*_READ_PROPERTIES`.
Option B (your suggestion): `*_LIST` on the container returns everything. Callers without it get a filtered list instead of a 403.
Given that most table privileges already imply `TABLE_LIST (TABLE_CREATE, *_READ_*, *_WRITE_*, TABLE_FULL_METADATA, CATALOG_MANAGE_*)`, so anyone with a read or write grant at the namespace or catalog level sees everything under both options. The options only differ for two cases:
| case | today's behavior | A | B |
|---|---|---|---|
| the caller only has TABLE_LIST / TABLE_CREATE at namespace level | all names | only tables they can read (often none) | all names (unchanged) |
| the caller only has per-table grants | 403 | 403 | filtered list of their tables |
Let me update the proposal's goal and I'll update the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4179088986
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1919,4 +1960,148 @@ private ImmutableLoadCredentialsResponse fallbackToFullLoadTable(
.credentials(loadTableResponse.credentials())
.build();
}
+
+ private boolean isEntityLevelListFilteringEnabled() {
+ return realmConfig().getConfig(ENABLE_ENTITY_LEVEL_LIST_FILTERING, getResolvedCatalogEntity());
+ }
+
+ /**
+ * Maps a container-scoped LIST operation to the entity-scoped operation used to decide whether an
+ * individual candidate is visible.
+ *
+ * <p>The two are deliberately distinct. The container operation gates the list call itself (may I
+ * list here at all?), while the entity operation asks whether this one entity may be seen.
+ * Keeping them separate lets authorizers require a stronger permission per entity -- the built-in
+ * RBAC authorizer maps the entity operations to the {@code *_READ_PROPERTIES} privileges.
+ */
+ private static PolarisAuthorizableOperation entityVisibilityOperation(
+ PolarisAuthorizableOperation listOperation) {
+ return switch (listOperation) {
+ case LIST_NAMESPACES -> PolarisAuthorizableOperation.LIST_NAMESPACES_ENTITY;
+ case LIST_TABLES -> PolarisAuthorizableOperation.LIST_TABLES_ENTITY;
+ case LIST_VIEWS -> PolarisAuthorizableOperation.LIST_VIEWS_ENTITY;
+ default ->
+ throw new IllegalArgumentException("Not a filterable list operation: " + listOperation);
+ };
+ }
+
+ /**
+ * One filtering candidate: the entity, the path it resolves under, and its visibility request.
+ */
+ private record Candidate<T>(T entity, ResolverPath path, AuthorizationRequest request) {}
+
+ private <T> List<T> filterEntities(
+ List<T> entities,
+ Function<T, ResolverPath> toResolverPath,
+ Function<T, PolarisSecurable> toSecurable,
+ PolarisAuthorizableOperation op) {
+ if (entities.isEmpty()) {
+ return entities;
+ }
+ var principal = polarisPrincipal();
+ PolarisAuthorizableOperation entityOp = entityVisibilityOperation(op);
+ List<Candidate<T>> candidates = new ArrayList<>(entities.size());
+ for (T entity : entities) {
+ candidates.add(
+ new Candidate<>(
+ entity,
+ toResolverPath.apply(entity),
+ new AuthorizationRequest(
+ principal,
+ List.of(
+ new SingleTargetAuthorizationIntent(entityOp, toSecurable.apply(entity))))));
+ }
+
+ PolarisResolutionManifest filterManifest = newResolutionManifest();
+ candidates.forEach(candidate -> filterManifest.addPath(candidate.path()));
+ AuthorizationState authzState = new AuthorizationState(filterManifest);
+
+ // Requests are built before resolution so the authorizer decides what to resolve, same as
+ // in the single-request path.
+ authorizer()
+ .resolveAuthorizationInputs(
+ authzState, candidates.stream().map(Candidate::request).toList());
+
+ // Look each path up under the same key addPath registered it with, so the lookup key cannot
+ // drift from the registered one. Unresolvable candidates (e.g. deleted between list and
+ // filter) are dropped silently.
+ List<Candidate<T>> resolvable =
+ candidates.stream()
+ .filter(
+ candidate ->
+ filterManifest.getResolvedPath(ResolvedPathKey.of(candidate.path()), true)
Review Comment:
For federated catalogs, a remote-only table resolves partially, so it isn't dropped here. My read is that RBAC then authorizes it against the deepest resolved ancestor (the namespace). The federated tests use the leaf-name double, though, so I couldn't confirm that under real RBAC. Is ancestor-level visibility the intended rule until step 7 lands? If so, can we add a comment here?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4303302136
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
thank you for the details!
Let's assume that for federated catalogs, entity-level filtering is not gated on `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS`. Here are the filter behaviors for each authorizer:
**OPA**: same behavior as non-federated catalogs. Each candidate is evaluated by its full name, whether or not Polaris has a local entity for it.
**Built-in RBAC**:
If `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS = false`:
nothing below the catalog has a local entity, so every candidate is evaluated against the catalog
If `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS = true`:
- Case A: for Namespaces/tables that have been granted on have a local entity, authorizer evaluates the local entities
- Case B: for Namespaces/tables without a grant, authorizer evaluates against their deepest local ancestor
**Ranger**:
If namespaces/tables have a local entity:
Ranger evaluates the local entity, same behavior as non-federated catalog
If namespaces/tables are remote-only:
Ranger denies the visibility check, so they are hidden from the list.
Reason: without a local entity, Ranger can only see the parent, so it cannot evaluate it.
@flyrain @flyingImer do you think this is the reasonable behavior?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4408068730
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java:
##########
@@ -21,12 +21,27 @@
/** Intent-level Polaris operations that can be authorized. */
public enum PolarisAuthorizableOperation {
LIST_NAMESPACES,
+ /**
+ * Checked for each namespace when listNamespaces filters its results for a caller who lacks
+ * {@link #LIST_NAMESPACES} on the parent namespace or catalog: a namespace is returned only if
+ * this check passes. Separate from LIST_NAMESPACES so that authorizers can decide access to each
+ * namespace independently of the parent-level permission. The built-in RBAC authorizer maps it to
+ * {@code NAMESPACE_READ_PROPERTIES}.
+ */
+ LIST_NAMESPACES_ENTITY,
CREATE_NAMESPACE,
LOAD_NAMESPACE_METADATA,
NAMESPACE_EXISTS,
DROP_NAMESPACE,
UPDATE_NAMESPACE_PROPERTIES,
LIST_TABLES,
+ /**
+ * Checked for each table when listTables filters its results for a caller who lacks {@link
+ * #LIST_TABLES} on the parent namespace: a table is returned only if this check passes. Separate
+ * from LIST_TABLES so that authorizers can decide access to each table independently of the
+ * parent-level permission. The built-in RBAC authorizer maps it to {@code TABLE_READ_PROPERTIES}.
+ */
+ LIST_TABLES_ENTITY,
Review Comment:
this makes sense, renamed the LIST_*_ENTITY to its corresponding singular form.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5420313996
> @gracechen09, thanks for keeping working on it. The PR is still in draft state. Is it ready for a review?
thanks @flyrain for checking in, current change implements the [default batch authorize method](https://github.com/apache/polaris/pull/4831/changes#diff-dc1f8c7eeec7416df53fab73b1a4d4af9c60a82220233d6b57f4ccaa766dd64dR71) we agreed on, and the entity filtering logic on the caller sites. However, there are some RBAC specific issues I need to think through. I'll update the PR status once it's ready!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3503304006
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
+ @NonNull AuthorizationState authzState, @NonNull VisibilityFilterRequest request) {
Review Comment:
We can update the `authorize()` method or create a new one and retrofit the old methods into it. Something like `authorize(request(list of entities, etc..)) -> list of decisions per entity`.
The request will have some information that is shared across all entities in the list, and implementation are free to use it to short-circuit decisions, of course. Mapping the same decision to each entity in the returned value is probably not a lot of overhead after the decision is made.
We may need to change the return type of `authorize()` to allow more than one output decision.
Does this sound reasonable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4201555468
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -34,6 +36,36 @@ public interface PolarisAuthorizer {
void resolveAuthorizationInputs(
@NonNull AuthorizationState authzState, @NonNull AuthorizationRequest request);
+ /**
+ * Batch counterpart to {@link #resolveAuthorizationInputs(AuthorizationState,
+ * AuthorizationRequest)}, invoked once before {@link #authorize(AuthorizationState, List)} for
+ * requests that share one {@link AuthorizationState}.
+ *
+ * <p>All requests must carry the same principal. Implementations may narrow what they resolve,
+ * exactly as for the single-request method, and must resolve the shared state at most once.
+ * {@link #authorize(AuthorizationState, List)} may subsequently be called with a subset of {@code
+ * requests}.
+ *
+ * <p>The default implementation merges the intents of every request, in order, into one {@link
+ * AuthorizationRequest} and delegates to the single-request method, so any narrowing done there
+ * applies to the batch as well. Overriding is optional and only needed for batch-specific
+ * preparation.
+ */
+ default void resolveAuthorizationInputs(
+ @NonNull AuthorizationState authzState, @NonNull List<AuthorizationRequest> requests) {
+ if (requests.isEmpty()) {
+ return;
+ }
+ PolarisPrincipal principal = requests.get(0).principal();
+ Preconditions.checkArgument(
+ requests.stream().allMatch(request -> request.principal().equals(principal)),
+ "All requests in a batch must share one principal");
Review Comment:
Can we change the batch authorize method parameters to take a principal and a list of `AuthorizationIntent` instead of a list of `AuthorizationRequest`? So that we don't run into this weird issue. Unless, we want to support different principals for batch authZ. Do we?
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -819,4 +819,15 @@ public static void enforceFeatureEnabledOrThrow(
+ "configured, but existing values cannot be changed to different ones.")
.defaultValue(false)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean> ENABLE_ENTITY_LEVEL_LIST_FILTERING =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ENABLE_ENTITY_LEVEL_LIST_FILTERING")
+ .catalogConfig("polaris.config.enable-entity-level-list-filtering")
+ .description(
Review Comment:
"Authorized to see" hides the important part: each entity needs *_READ_PROPERTIES (or something that includes it). Principals that only hold TABLE_LIST will suddenly get empty lists, which is a break change. This is probably discussed somewhere, can we allow TABLE_LIST on the container still means "see everything". Filtering only helps callers who don't hold it: they get a filtered list instead of aย 403. There's no cliff, and it fixes the discoverability case (#4573, listCatalogs). But it no longer hides names from LIST holders, which seems missing part of the proposal's goal. Any thoughts on it?
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1919,4 +1960,148 @@ private ImmutableLoadCredentialsResponse fallbackToFullLoadTable(
.credentials(loadTableResponse.credentials())
.build();
}
+
+ private boolean isEntityLevelListFilteringEnabled() {
+ return realmConfig().getConfig(ENABLE_ENTITY_LEVEL_LIST_FILTERING, getResolvedCatalogEntity());
+ }
+
+ /**
+ * Maps a container-scoped LIST operation to the entity-scoped operation used to decide whether an
+ * individual candidate is visible.
+ *
+ * <p>The two are deliberately distinct. The container operation gates the list call itself (may I
+ * list here at all?), while the entity operation asks whether this one entity may be seen.
+ * Keeping them separate lets authorizers require a stronger permission per entity -- the built-in
+ * RBAC authorizer maps the entity operations to the {@code *_READ_PROPERTIES} privileges.
+ */
+ private static PolarisAuthorizableOperation entityVisibilityOperation(
+ PolarisAuthorizableOperation listOperation) {
+ return switch (listOperation) {
+ case LIST_NAMESPACES -> PolarisAuthorizableOperation.LIST_NAMESPACES_ENTITY;
+ case LIST_TABLES -> PolarisAuthorizableOperation.LIST_TABLES_ENTITY;
+ case LIST_VIEWS -> PolarisAuthorizableOperation.LIST_VIEWS_ENTITY;
+ default ->
+ throw new IllegalArgumentException("Not a filterable list operation: " + listOperation);
+ };
+ }
+
+ /**
+ * One filtering candidate: the entity, the path it resolves under, and its visibility request.
+ */
+ private record Candidate<T>(T entity, ResolverPath path, AuthorizationRequest request) {}
+
+ private <T> List<T> filterEntities(
+ List<T> entities,
+ Function<T, ResolverPath> toResolverPath,
+ Function<T, PolarisSecurable> toSecurable,
+ PolarisAuthorizableOperation op) {
+ if (entities.isEmpty()) {
+ return entities;
+ }
+ var principal = polarisPrincipal();
+ PolarisAuthorizableOperation entityOp = entityVisibilityOperation(op);
+ List<Candidate<T>> candidates = new ArrayList<>(entities.size());
+ for (T entity : entities) {
+ candidates.add(
+ new Candidate<>(
+ entity,
+ toResolverPath.apply(entity),
+ new AuthorizationRequest(
+ principal,
+ List.of(
+ new SingleTargetAuthorizationIntent(entityOp, toSecurable.apply(entity))))));
+ }
+
+ PolarisResolutionManifest filterManifest = newResolutionManifest();
+ candidates.forEach(candidate -> filterManifest.addPath(candidate.path()));
+ AuthorizationState authzState = new AuthorizationState(filterManifest);
+
+ // Requests are built before resolution so the authorizer decides what to resolve, same as
+ // in the single-request path.
+ authorizer()
+ .resolveAuthorizationInputs(
+ authzState, candidates.stream().map(Candidate::request).toList());
+
+ // Look each path up under the same key addPath registered it with, so the lookup key cannot
+ // drift from the registered one. Unresolvable candidates (e.g. deleted between list and
+ // filter) are dropped silently.
+ List<Candidate<T>> resolvable =
+ candidates.stream()
+ .filter(
+ candidate ->
+ filterManifest.getResolvedPath(ResolvedPathKey.of(candidate.path()), true)
Review Comment:
For federated catalogs, a remote-only table resolves partially, so it isn't dropped here. My read is that RBAC then authorizes it against the deepest resolved ancestor (the namespace). The federated tests use the leaf-name double, though, so I couldn't confirm that under real RBAC. Is ancestor-level visibility the intended rule until step 7 lands? If so, can we add a comment here?
<sub>๐ค AI-generated review comment โ agents make mistakes, so please verify before acting on it.</sub>
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
this is only needed when `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` is on, right? By default, the namespaces or tables don't have Polaris entity in a federated catalog. Should we gate this logic with `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS`?
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -400,20 +419,31 @@ public ListTablesResponse listTables(Namespace namespace, String pageToken, Inte
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES;
authorizeBasicNamespaceOperationOrThrow(op, namespace);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
Review Comment:
Minor: Recommend storing this in a class field or calling `isEntityLevelListFilteringEnabled()` directly inline below. We could simplify the method name if needed.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1640,14 +1670,23 @@ public ListTablesResponse listViews(Namespace namespace, String pageToken, Integ
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_VIEWS;
authorizeBasicNamespaceOperationOrThrow(op, namespace);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
if (baseCatalog instanceof ViewCatalog viewCatalog) {
ListTablesResponse response =
catalogHandlerUtils()
.listViews(
viewCatalog, namespace, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<TableIdentifier> visible = filterTableIdentifiers(response.identifiers(), op);
Review Comment:
Same here.
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -46,4 +78,24 @@ void resolveAuthorizationInputs(
*/
@NonNull AuthorizationDecision authorize(
@NonNull AuthorizationState authzState, @NonNull AuthorizationRequest request);
+
+ /**
+ * Batch authorization entry point.
+ *
+ * <p>The default implementation evaluates each request independently by delegating to {@link
+ * #authorize(AuthorizationState, AuthorizationRequest)}. Implementations may override this method
+ * for improved performance.
+ *
+ * <p>The returned list preserves the same size and order as {@code requests}. Each request is
+ * evaluated independently. Implementations must return a decision for every request and must not
+ * short-circuit across requests, even when an earlier request is denied.
+ *
+ * <p>Note that this differs from the contract <em>within</em> a single {@link
+ * AuthorizationRequest}, where the intents are AND-combined and evaluation may short-circuit on
+ * the first deny.
+ */
+ default @NonNull List<AuthorizationDecision> authorize(
+ @NonNull AuthorizationState authzState, @NonNull List<AuthorizationRequest> requests) {
Review Comment:
Do we want to change from list to stream to have lazy execution without materializing everything to the memory? cc @singhpk234 @HonahX @collado-mike
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -400,20 +419,31 @@ public ListTablesResponse listTables(Namespace namespace, String pageToken, Inte
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES;
authorizeBasicNamespaceOperationOrThrow(op, namespace);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListTablesResponse response =
catalogHandlerUtils()
.listTables(
baseCatalog, namespace, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<TableIdentifier> visible = filterTableIdentifiers(response.identifiers(), op);
Review Comment:
Same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4831:
URL: https://github.com/apache/polaris/pull/4831#issuecomment-5235150252
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4242385908
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -34,6 +36,36 @@ public interface PolarisAuthorizer {
void resolveAuthorizationInputs(
@NonNull AuthorizationState authzState, @NonNull AuthorizationRequest request);
+ /**
+ * Batch counterpart to {@link #resolveAuthorizationInputs(AuthorizationState,
+ * AuthorizationRequest)}, invoked once before {@link #authorize(AuthorizationState, List)} for
+ * requests that share one {@link AuthorizationState}.
+ *
+ * <p>All requests must carry the same principal. Implementations may narrow what they resolve,
+ * exactly as for the single-request method, and must resolve the shared state at most once.
+ * {@link #authorize(AuthorizationState, List)} may subsequently be called with a subset of {@code
+ * requests}.
+ *
+ * <p>The default implementation merges the intents of every request, in order, into one {@link
+ * AuthorizationRequest} and delegates to the single-request method, so any narrowing done there
+ * applies to the batch as well. Overriding is optional and only needed for batch-specific
+ * preparation.
+ */
+ default void resolveAuthorizationInputs(
+ @NonNull AuthorizationState authzState, @NonNull List<AuthorizationRequest> requests) {
+ if (requests.isEmpty()) {
+ return;
+ }
+ PolarisPrincipal principal = requests.get(0).principal();
+ Preconditions.checkArgument(
+ requests.stream().allMatch(request -> request.principal().equals(principal)),
+ "All requests in a batch must share one principal");
Review Comment:
good point! I don't think we plan to support different principals for foreseen use cases, so changing the method's signature makes sense here.
Updated the batch method to pass a record that takes one principal plus independent intents.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4369519785
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
A separate PR makes sense to me. It should be fine to let Ranger authorizer deny LIST_*_ENTITY checks for remote-only candidates. Thanks @gracechen09 !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4392502939
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
The agreed interim deny behavior is still missing: remote-only candidates can resolve to an ancestor path and reach Ranger without an explicit rejection. Could we add a guard in the Ranger authorizer to deny `LIST_*_ENTITY` when the requested child is not fully resolved locally, plus a focused regression test confirming `DENY` even when the ancestor policy would allow access? This keeps the catalog handler authorizer-agnostic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering: API and feature flag [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r3500426121
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -80,4 +80,19 @@ void authorizeOrThrow(
@NonNull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ /**
+ * Filters a candidate list of securables to only those the principal is authorized to see.
+ *
+ * <p>The default implementation returns all candidates unchanged, preserving backward
+ * compatibility for authorizers that do not implement visibility filtering.
+ *
+ * <p>If filtering encounters an error, implementations should throw rather than fall back to
+ * returning unfiltered results.
+ */
+ @NonNull
+ default List<PolarisSecurable> filterByVisibility(
Review Comment:
The callers in Polaris code are all synchronous now... what would be the benefit of making just the Authrorizer reactive?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4243577736
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
yeah, when `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` is off, there are no per-table grants, so every table gets the same answer from catalog-level grants. I think gating it makes sense.
However, my knowledge on federated catalogs is scattered, I was wondering if OPA or Ranger are used, will these authorizers still get the full namespaces and tables for federated catalogs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4213699323
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -819,4 +819,15 @@ public static void enforceFeatureEnabledOrThrow(
+ "configured, but existing values cannot be changed to different ones.")
.defaultValue(false)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<Boolean> ENABLE_ENTITY_LEVEL_LIST_FILTERING =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ENABLE_ENTITY_LEVEL_LIST_FILTERING")
+ .catalogConfig("polaris.config.enable-entity-level-list-filtering")
+ .description(
Review Comment:
this makes sense, after walking through the behavioral change for these two options, I agree the current option may have wider impact with the breaking change. And the suggestion solves breaking change and fixes #4573. Here's a quick comparison.
Option A (current PR):` *_LIST` on the container only allows the list request to proceed; each returned entity also needs `*_READ_PROPERTIES`.
Option B (your suggestion): `*_LIST` on the container returns everything. Callers without it get a filtered list instead of a 403.
Given that most table privileges already imply `TABLE_LIST (TABLE_CREATE, *_READ_*, *_WRITE_*, TABLE_FULL_METADATA, CATALOG_MANAGE_*)`, so anyone with a read or write grant at the namespace or catalog level sees everything under both options. The options only differ for two cases:
| case | today's behavior | A | B |
|---|---|---|---|
| the caller only has TABLE_LIST / TABLE_CREATE at namespace level | all names | only tables they can read (often none) | all names (unchanged) |
| the caller only has per-table grants | 403 | 403 | filtered list of their tables |
Let me update the proposal's goal and I'll update the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4365108026
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
Could we keep the catalog handlerโs flow unified and update Ranger to derive the full resource name from the authorization intent, using resolved local entities only for available attributes? That would let Ranger evaluate remote resources without requiring local namespace/table entities. We should apply this to both container-level LIST checks and per-entity filtering, and add a focused Ranger test matching the passing OPA case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4369341638
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
Agree that catalog handler should be authorizer-agnostic! The per-authorizer behavior I described above is what each authorizer implementation does behind the interface.
For updating Ranger to derive the full resource name from authorization intent, this makes sense to me. Shall we create a separate PR or adding the changes to the current PR?
I'm leaning towards a separate PR since it changes how Ranger evaluates resources in general. In the meanwhile, this PR could keep the handler flow unified and let Ranger authorizer deny `LIST_*_ENTITY` checks for remote-only candidates, so nothing is over exposed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4396117025
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -296,20 +304,31 @@ public ListNamespacesResponse listNamespaces(
PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_NAMESPACES;
authorizeBasicNamespaceOperationOrThrow(op, parent);
+ boolean filterEnabled = isEntityLevelListFilteringEnabled();
+
if (isFederated) {
ListNamespacesResponse response =
catalogHandlerUtils()
.listNamespaces(
namespaceCatalog, parent, boundedPageToken(pageToken), boundedPageSize(pageSize));
rejectIncompleteListing(pageToken, pageSize, response.nextPageToken());
- return response;
+ if (!filterEnabled) {
+ return response;
+ }
+ List<Namespace> visible = filterNamespaces(response.namespaces(), op);
Review Comment:
thanks @flyrain for clarifying. Added the guard for ranger to deny LIST_*_ENTITY checks for remote-only candidates, and also added the tests (in OPA, Ranger and default rbac) that align with the federated catalog behaviors we defined above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Entity-level filtering Part 1 [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on code in PR #4831:
URL: https://github.com/apache/polaris/pull/4831#discussion_r4408071764
##########
extensions/auth/ranger/src/main/java/org/apache/polaris/extension/auth/ranger/RangerPolarisAuthorizer.java:
##########
@@ -130,6 +146,24 @@ private AuthorizationDecision authorizeIntent(
}
}
+ private static boolean isEntityVisibilityCheck(PolarisAuthorizableOperation operation) {
Review Comment:
makes sense to me!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org