mchades commented on code in PR #11144:
URL: https://github.com/apache/gravitino/pull/11144#discussion_r3279179364


##########
design-docs/gravitino-view-privilege.md:
##########
@@ -0,0 +1,241 @@
+<!--
+  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.
+-->
+
+# Design of View Privilege Control in Gravitino
+
+## Background
+
+Apache Gravitino's [View Management 
design](https://docs.google.com/document/d/1qKZMcY5ifgZF-BjGF2FwYBNWyTwqrCDLaGW_D2jD_LY)
 provides unified view management across heterogeneous catalogs (Iceberg, 
Paimon, HMS, JDBC, and Gravitino-managed catalogs). The parent design assigns 
view privilege control to "Phase IV. Governance & Security" and lists four 
required privileges: `CREATE_VIEW`, `SELECT_VIEW`, `ALTER_VIEW`, `DROP_VIEW`.
+
+The existing Gravitino access control framework covers catalogs, schemas, 
tables, filesets, topics, models, tags, policies, jobs, and functions. Views 
are a partially-defined object type in this model — the privilege types exist 
in the API but are not enforced end-to-end, leaving view access control in an 
inconsistent state:
+
+- **Privilege type set is incomplete.** Only `CREATE_VIEW` and `SELECT_VIEW` 
exist in `Privilege.Name` (see 
`api/src/main/java/org/apache/gravitino/authorization/Privilege.java:146-148`). 
`ALTER_VIEW`-equivalent and `DROP_VIEW`-equivalent are missing, so view alter 
and drop operations have no privilege to check against.
+- **REST endpoints do not enforce privileges.** `ViewOperations.java` (the 
generic view REST endpoints) carries zero `@AuthorizationExpression` 
annotations on its five endpoints (`listViews`, `createView`, `loadView`, 
`alterView`, `dropView` at lines 67, 93, 132, 158, 191 respectively). Compare 
with `TableOperations.java` where every endpoint is wired (lines 83, 119, 167, 
204, 241).
+- **No visibility filtering on `listViews`.** `ViewOperations.listViews` 
(lines 71–94) returns the raw `dispatcher.listViews(viewNS)` result with no 
filter applied. Compare with `TableOperations.listTables` (lines 86–112) which 
calls `MetadataAuthzHelper.filterByExpression(..., 
FILTER_TABLE_AUTHORIZATION_EXPRESSION, ...)` to drop entries the caller has no 
privilege on. The `FILTER_VIEW_AUTHORIZATION_EXPRESSION` constant already 
exists in `AuthorizationExpressionConstants.java:119` but is consumed only by 
the Iceberg-REST path (`IcebergViewOperations.java:350`); the generic path 
never references it. Result: any user who can reach the list endpoint sees 
every view in the schema, regardless of per-view privileges.
+- **No ownership tracking for views.** 
`core/src/main/java/org/apache/gravitino/hook/` contains 12 `*HookDispatcher` 
classes (Catalog, Schema, Table, Fileset, Function, Model, …) but no 
`ViewHookDispatcher`. As a result, view owners are never set on creation, and 
the `VIEW::OWNER` clause already present in 
`LOAD_VIEW_AUTHORIZATION_EXPRESSION` 
(`AuthorizationExpressionConstants.java:97`) never resolves to true for the 
view creator.
+- **Engine-side ACL translation is silent.** The Ranger plugin 
(`authorizations/authorization-ranger/`) contains zero references to 
`MetadataObject.Type.VIEW` or any view privilege — view grants are not 
translated to Ranger ACLs.
+- **Iceberg-REST is the only path that enforces view auth today.** 
`IcebergViewOperations.java` (lines 90, 126, 164, 201, 243, 282) is fully wired 
with `@AuthorizationExpression` and 
`ICEBERG_LOAD_VIEW_AUTHORIZATION_EXPRESSION`. This means the same conceptual 
operation has two enforcement paths with different coverage depending on the 
catalog type, which is confusing for users and operators.
+
+The result is a privilege model where granting `SELECT_VIEW` to a user has 
visible effect on Iceberg-REST views but no effect on views served via the 
generic REST path, and where alter/drop operations on any view are unprotected.
+
+**Current state — two parallel paths, only one enforces view auth:**
+
+```
+                       ┌────────────────────────────────────────┐
+                       │       Authorization framework          │
+                       │  (roles, grants, owner, expressions)   │
+                       └──────────────┬─────────────────────────┘
+                                      │ checked at REST layer
+                                      │
+       ┌──────────────────────────────┴──────────────────────────────┐
+       │                                                             │
+       ▼                                                             ▼
+  IcebergViewOperations                                       ViewOperations
+  (Iceberg REST spec path)                                    (Gravitino REST 
path)
+       │ @AuthorizationExpression                                  │ (no 
@AuthorizationExpression)
+       │ on every endpoint  ✅                                      │ on any 
endpoint     ❌
+       ▼                                                            ▼
+  Iceberg REST handlers                                       
ViewNormalizeDispatcher
+                                                                    │
+                                                                    │ (no 
ViewHookDispatcher
+                                                                    │  → view 
owner never set ❌
+                                                                    │  → 
VIEW::OWNER clause
+                                                                    │    never 
resolves true)
+                                                                    ▼
+                                                              
ViewOperationDispatcher
+                                                                    │
+                                                                    ▼
+                                                          Catalog connector 
(Iceberg/
+                                                          
Paimon/HMS/JDBC/managed)
+
+  Ranger plugin: zero references to MetadataObject.Type.VIEW ❌
+  (view grants in Gravitino are not translated to Ranger ACLs)
+```
+
+---
+
+## Goals
+
+1. **Complete the View Privilege Type Set**: Define `ALTER_VIEW` so that 
create, read, and alter operations each have a corresponding grantable 
privilege (`CREATE_VIEW`, `SELECT_VIEW`, `ALTER_VIEW`). Drop operations are 
gated by ownership rather than by a privilege (see Goal 4), consistent with 
every other mutable Gravitino object. The `ALTER_VIEW` name follows the parent 
View Management design doc.
+
+2. **Enforce View Privileges on Generic REST Endpoints**: Wire 
`@AuthorizationExpression` on all five `ViewOperations.java` endpoints so that 
view privilege checks occur at the server-side authorization layer, matching 
the `TableOperations.java` pattern. Each endpoint receives an expression 
appropriate to its operation (create / read / alter / drop / list-filter).
+
+3. **Ownership Tracking for Views**: Add a `ViewHookDispatcher` that sets the 
creator as the view owner on `createView`, so the `VIEW::OWNER` clause in 
existing and new authorization expressions resolves correctly. View ownership 
remains transferable via Gravitino's existing owner-management API.
+
+4. **Hierarchical Drop Semantics**: View drops are gated by ownership at the 
view level **or any parent level** (schema, catalog, metalake), matching the 
existing `dropTable` / `dropFileset` / `dropFunction` expressions. This means 
platform admins (metalake/catalog owners) and domain owners (schema owners) can 
drop views in their scope without an explicit per-view grant, while reassigning 
ownership remains the path for delegating drop authority to a specific user 
without granting broader administrative power. No `DROP_VIEW` privilege is 
introduced, keeping views consistent with every other droppable object type.
+
+5. **Backward Compatibility**: When authorization is disabled, all view REST 
endpoints behave identically to today. Existing `CREATE_VIEW` and `SELECT_VIEW` 
privileges retain their current bit values and semantics. No client or DTO 
changes. The Iceberg-REST view authorization path 
(`IcebergViewOperations.java`) is unchanged.
+
+---
+
+## Non-Goals
+
+1. **Data-Level Access Control**: View privileges in Gravitino govern metadata 
access only. Whether the user can read the underlying tables referenced by the 
view SQL is enforced by the compute engine and the underlying catalog's 
permission system. The parent design doc commits to this scope split explicitly.
+
+2. **DEFINER / INVOKER Enforcement**: `securityConfig.securityMode` is stored 
by Gravitino as a metadata annotation and passed through to the engine. 
Gravitino does not enforce DEFINER/INVOKER semantics. This is the parent design 
doc's position and is unchanged here.

Review Comment:
   Although different engines have varying degrees of support for this 
attribute, it is a key design for view privilege, and we should not skip it in 
the design of view privilege. Since we also maintain connectors for 
Trino/Flink/Spark, we need to consider comprehensively in this design document 
how to support it.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to