mchades commented on code in PR #11144: URL: https://github.com/apache/gravitino/pull/11144#discussion_r3264276342
########## 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 Review Comment: The design should take this into consideration: https://github.com/apache/gravitino/pull/10724#discussion_r3056018270 <img width="802" height="768" alt="Image" src="https://github.com/user-attachments/assets/12b78a44-040c-4f1a-a7e7-74df1dc7cbbb"; /> ########## 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: Gravitino currently does not explicitly store the `securityConfig.securityMode`. Should we consider adding this to the view field or view properties? Additionally, are these values mutable or immutable? -- 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]
