Hi everyone, Apologies for the delay in response, I've revised the proposal to align with the new authorization SPI changes. Here's the summary of the major changes:
1. Use the new authorize() SPI instead of authorizeOrThrow The original proposal used the authorizeOrThrow with exception-driven control flow to check per-entity visibility. The updated version uses the new authorize(AuthorizationState, AuthorizationRequest) SPI, which returns an AuthorizationDecision without throwing. This avoids using exceptions for expected control flow. 2. Refined the new entity filtering API on the PolarisAuthorizer interface After the authorization SPI refactoring, privilege hierarchy semantics moves into the RBAC authorizer implementation. The original proposal's filtering API leaks the RBAC assumptions into the interfaces. The updated proposal moves all filtering decisions (including short-circuit) inside each authorizer. This lets RBAC authorizer use privilege inheritance, OPA uses its own policy checks, decoupling RBAC semantics from authorizer implementation. 3. Defined error handling When the entity-filtering feature is on, if the entity-filtering logic throws an exception (e.g., OPA server unreachable, DB query timeout), the LIST request will fail with a 500 error. The response does not fall back to unfiltered results because it would silently turn off the feature. Sung and Dmitri, thanks for sharing the thoughts on the feature flag, I agree on checking the feature flag before performing any filtering. >From the caller site logic below, we could check the feature flag before step 3 to decide whether the filtering should be performed. What do you think? ``` // Step 1 existing code: checks user has TABLE_LIST on the namespace authorizeBasicNamespaceOperationOrThrow(LIST_TABLES, namespace); // Step 2 catalog backend queries the DB and returns table names List<TableIdentifier> candidates = baseCatalog.listTables(namespace, pageRequest); // Step 3 construct container securable from the namespace PolarisSecurable containerSecurable = PolarisSecurable.of( new PathSegment(PolarisEntityType.CATALOG, catalogName()), new PathSegment(PolarisEntityType.NAMESPACE, namespace.level(0))); // Step 4 per-entity filter: each authorizer decides visibility in its own way List<PolarisSecurable> visible = authorizer.filterByVisibility( authzState, new VisibilityFilterRequest(LIST_TABLES,containerSecurable, toSecurables(candidates))); ``` I'm working on the draft PR and will send it out once it's ready. In the meantime, feel free to share your thoughts on the proposal anytime! The link to the proposal is https://docs.google.com/document/d/1PyIISgK2FVK0m8t4104KZpKAWb4d1Wy0UhZQzut4pIc/edit?usp=sharing Thanks, Grace On Wed, Jun 17, 2026 at 5:32 PM Dmitri Bourlatchkov <[email protected]> wrote: > > Hi Sung, > > I'm not sure I understand what you mean by "switching off the boolean > authorization based on the existing operations", but we can probably > resolve that when we have a specific PR :) > > Cheers, > Dmitri. > > On Tue, Jun 16, 2026 at 10:30 PM Sung Yun <[email protected]> wrote: > > > Hi Dmitri, > > > > I actually meant completely switching off the boolean authorization based > > on the existing operations. But I think I could sway either way. > > > > I had a small bias towards just using the filter based approach without > > the first check if the feature flag is on, because that'll make it easier > > for system operators to reason about setting up their policies in their > > respective PAP. If we introduce two-layered checks, there will need to be > > two sets of policies for the same operation. > > > > Sung > > > > On 2026/06/16 02:52:19 Dmitri Bourlatchkov wrote: > > > Hi Sung, > > > > > > Good point about exceptions. I agree that it is preferable to avoid them > > > (as denial indicators) on the filtering call paths. > > > > > > Re: feature flags, I suppose we cloud model this as a two-layered check: > > 1) > > > per-batch permissions (same as now) and 2) per-element include/exclude > > > decisions. The first check is always "on", the second can be controlled > > by > > > a feature flag the the AuthZ layer. Callers (REST services) always get > > the > > > filtering flags, but if the feature is disabled, the flag are always > > > "include". > > > > > > This way the code in REST services can be uniform across all use cases, > > > which will simplify testing. > > > > > > I'm not sure you meant the same approach in your message, but I hope it > > > makes sense :) > > > > > > WDYT? > > > > > > Cheers, > > > Dmitri. > > > > > > > > > > > > On Mon, Jun 15, 2026 at 10:08 PM Sung Yun <[email protected]> wrote: > > > > > > > Hi Grace and Venkateshwaran, > > > > > > > > Thanks for the discussion. I agree with the general direction of > > > > introducing authorizer-layer filtering and returning callers a list > > scoped > > > > to what they can access. > > > > > > > > One thing I wanted to float: I think it would help to make the > > > > authorization input for list operations deterministic, rather than > > relying > > > > on a fallback path. By fallback I mean the handler running the normal > > > > LIST_* check and, on failure, dropping into a filtered path. That > > works, > > > > but it leans on exception-driven branching that I'd argue is harder to > > > > reason about and test. > > > > > > > > The alternative would be to treat this as a mode, enabled by a feature > > > > flag for filtering on list operations. When it's off, behavior is > > exactly > > > > what we have today. When it's on, every caller goes through the same > > > > listing/filtering path, and whether you see everything underneath just > > > > falls out of that check (admins included, via the container > > short-circuit) > > > > instead of being a separate branch keyed on who the caller is. > > > > > > > > That being said I think we are generally in agreement, and we could > > iron > > > > out these mechanisms on PR reviews as well. > > > > > > > > Cheers > > > > Sung > > > > > > > > On 2026/06/05 23:23:30 Yufei Gu wrote: > > > > > The proposed behavior makes sense. Returning a list of catalogs only > > a > > > > > caller can access feels more aligned with least privilege and with > > how > > > > most > > > > > users would expect catalog discovery to work. > > > > > > > > > > Implementation does not seem particularly difficult. The existing > > admin > > > > > behavior could remain unchanged, while non-admin callers receive a > > > > filtered > > > > > list based on catalog level authorization checks similar to those > > already > > > > > performed by getCatalog(). There are minor perf concerns, but I > > think it > > > > > should be acceptable in general. To me, the bigger question is > > agreeing > > > > on > > > > > the API semantics rather than the implementation itself. > > > > > > > > > > Overall, I'm supportive of the change. > > > > > > > > > > Yufei > > > > > > > > > > > > > > > On Thu, Jun 4, 2026 at 5:05 PM Dmitri Bourlatchkov <[email protected] > > > > > > > wrote: > > > > > > > > > > > It looks like this reply got disconnected from the original thread. > > > > > > > > > > > > Posting a link to it for reference: > > > > > > https://lists.apache.org/thread/w58zrjt6jpq33mv9lvqzndqc6no5l7xb > > > > > > > > > > > > Cheers, > > > > > > Dmitri. > > > > > > > > > > > > On Sun, May 31, 2026 at 12:15 PM venkateshwaran cholan < > > > > > > [email protected]> wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > Thanks Grace for putting together the proposal, and thanks > > everyone > > > > for > > > > > > the > > > > > > > detailed feedback and discussion so far. > > > > > > > > > > > > > > Dimas pointed me to this discussion from #4573, and I wanted to > > > > share an > > > > > > > observation after tracing the current implementation on main. > > > > > > > > > > > > > > The discoverability gap there (catalog-role access works for > > > > getCatalog > > > > > > and > > > > > > > Iceberg operations, but listCatalogs requires root CATALOG_LIST) > > > > looks > > > > > > like > > > > > > > one concrete case of the broader visibility model discussed here, > > > > > > > particularly if CATALOG_READ_PROPERTIES ends up being the > > selected > > > > > > > visibility privilege for catalogs. > > > > > > > > > > > > > > listCatalogs() still gates on > > > > > > > authorizeBasicRootOperationOrThrow(LIST_CATALOGS) against the > > root > > > > > > > container, while getCatalog() authorizes per catalog. That > > matches > > > > the > > > > > > > asymmetry described in the issue. > > > > > > > > > > > > > > I agree that authorizer-layer filtering is preferable to a > > > > > > > listCatalogs-only special case so default RBAC, OPA, and other > > > > > > authorizers > > > > > > > stay consistent. > > > > > > > > > > > > > > One thing I am still unclear on for LIST_CATALOGS: the proposal's > > > > scope > > > > > > > short-circuit does not apply because there is no scope container > > > > (unlike > > > > > > > LIST_NAMESPACES / LIST_TABLES). > > > > > > > > > > > > > > For #4573-style discoverability, it seems we would want callers > > > > without > > > > > > > root CATALOG_LIST to get a filtered list instead of 403. > > > > > > > > > > > > > > Separately, when entity filtering is enabled, should callers with > > > > root > > > > > > > CATALOG_LIST still get a filtered list, or is root CATALOG_LIST > > > > intended > > > > > > to > > > > > > > mean "see all catalogs" with filtering only handling the > > > > discoverability > > > > > > > case? > > > > > > > > > > > > > > Happy to help with regression coverage either way. For example, > > an > > > > authz > > > > > > > test where a principal has catalog-scoped CATALOG_READ_PROPERTIES > > > > via a > > > > > > > catalog role but no root CATALOG_LIST: getCatalog succeeds and > > > > > > listCatalogs > > > > > > > fails today, documenting the gap until filtering lands. > > > > > > > > > > > > > > Thanks, > > > > > > > Venkateshwaran > > > > > > > > > > > > > > > > > > > > > > > > > > >
