sgedward commented on code in PR #11144: URL: https://github.com/apache/gravitino/pull/11144#discussion_r3407355933
########## design-docs/gravitino-view-privilege.md: ########## @@ -0,0 +1,324 @@ +<!-- + 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**: Complete the privilege management and protect against the `DROP` operation, similar to function privilege management. + +2. **Enforce View Privileges on Generic REST Endpoints**: Enforce privilege authentication on REST endpoints. + +3. **Ownership Tracking for Views**: Track ownership in a manner similar to the function privilege management process, inheriting the existing ownership model. + +4. **View Security Mode**: Introduce Definer/Invoker security support for view privilege management, allowing owners flexibility in switching modes for new/existing views. + +6. **Backward Compatibility**: It should be backward compatible with existing privilege management and the underlying data engine if views have previously been defined within the sub-system. + +--- + +## 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. **Per-User Identity Propagation in Spark and Flink Connectors**: Today's Gravitino Spark and Flink connectors authenticate to Gravitino using a singleton service identity established at catalog initialization. Consequently, we only have session context-level identity and, unlike Trino, lack per-user caller identity. + +3. **Unifying Generic View Auth Paths**: The Data Lake/Warehouse and Database have different approaches to managing views, privileges, and security. Unifying them into a single model is out of scope. + +4. **Materialized Views and Temporary Views**: Out of scope per the parent design (only logical views are in scope). + +5. **Identity Mapping between Gravitino and Databases**: This implementation will not manage the identity alignment between Gravitino's user principal and the database's user principal. + +--- + +## Proposal + +### Privilege Types + +Three privilege types are defined for views, matching the existing `*_TABLE` and `*_FUNCTION` shape. `CREATE_VIEW` and `SELECT_VIEW` already exist; `ALTER_VIEW` is new. **Drop view operations are not gated by a dedicated privilege — they require ownership of the view itself, or ownership of any parent (schema, catalog, or metalake), consistent with `dropTable` / `dropFileset` / `dropFunction` in Gravitino.** + +| Privilege | Securable Object Levels | Description | Status | +|----------------|-----------------------------------------|------------------------------------------------------------------------------------------|----------| +| `CREATE_VIEW` | Metalake, Catalog, Schema | Permission to create new views in a schema. | Existing | +| `SELECT_VIEW` | Metalake, Catalog, Schema, View | Permission to read view metadata and resolve the view's SQL during query planning. | Existing | +| `ALTER_VIEW` | Metalake, Catalog, Schema, View | Permission to alter a view's metadata (comment, properties, representations, rename). | **New** | + +**Naming rationale:** + +- `CREATE_VIEW` — Consistent with `CREATE_TABLE`, `CREATE_FILESET`. Used for delegated objects whose primary store is the underlying catalog. No change. +- `SELECT_VIEW` — Consistent with `SELECT_TABLE`. Reading view metadata is the analog of selecting from a table at the metadata layer; the actual SELECT against underlying data remains the engine's responsibility. No change. +- `ALTER_VIEW` — Matches the parent View Management design doc and standard SQL `ALTER VIEW` syntax. Covers all alter operations supported by the parent design (rename, updateComment, setProperty, removeProperty, addRepresentation, updateRepresentation, removeRepresentation). Diverges from Gravitino's existing `MODIFY_TABLE` / `MODIFY_FUNCTION` convention but aligns with the parent doc and with the `alterView` REST/API/Java method names. + +**Privilege inheritance:** Privileges granted at metalake, catalog, or schema level cascade to all views within that scope. Same as `*_TABLE` and `*_FUNCTION`. + +**Deny privileges:** Each privilege has a corresponding deny form (`DENY_CREATE_VIEW`, `DENY_SELECT_VIEW`, `DENY_ALTER_VIEW`). Existing deny entries for `CREATE_VIEW` and `SELECT_VIEW` already exist at `Privileges.java:319-321`; `DENY_ALTER_VIEW` is added. + +**Bitmask allocation:** `CREATE_VIEW` and `SELECT_VIEW` keep their existing bits (`1L << 28`, `1L << 29`). `ALTER_VIEW` is allocated the next free bit after `MODIFY_FUNCTION (1L << 32)` — `1L << 33`. + +--- + +### Securable Object Hierarchy + +Views are schema-scoped, following Gravitino's standard four-level hierarchy: + +``` +metalake + └── catalog + └── schema + └── view +``` + +`MetadataObject.Type.VIEW` already exists, and `VIEW_SUPPORTED_TYPES` in `Privileges.java:60-65` already covers METALAKE, CATALOG, SCHEMA, VIEW. `VIEW` is also already accepted by `MANAGE_GRANTS_SUPPORTED_TYPES` (line 86). No structural changes are required. + +**Privilege and ownership applicability by level:** + +| Securable Object | CREATE_VIEW | SELECT_VIEW | ALTER_VIEW | Drop (ownership-gated) | +|------------------|-------------|-------------|------------|------------------------| +| Metalake | ✅ | ✅ | ✅ | ✅ (metalake owner) | +| Catalog | ✅ | ✅ | ✅ | ✅ (catalog owner) | +| Schema | ✅ | ✅ | ✅ | ✅ (schema owner) | +| View | — | ✅ | ✅ | ✅ (view owner) | + +> `CREATE_VIEW` is not applicable at the view level because creation happens at the schema level (a view must be created within a schema). +> +> Drop has no dedicated privilege — it is authorized by ownership at any level. Owning a parent (metalake / catalog / schema) implicitly authorizes dropping any view in that scope. The full expression is shown in Authorization Enforcement below. + +--- + +### Visibility Control + +View visibility and access follow the same patterns as tables: + +1. **`listViews`** + - Requires `USE_CATALOG` + `USE_SCHEMA` at the endpoint level to access the schema (consistent with `listTables`). + - Applies `FILTER_VIEW_AUTHORIZATION_EXPRESSION` (`AuthorizationExpressionConstants.java:119`) to the result set — only views the user has `SELECT_VIEW`, `ALTER_VIEW`, `CREATE_VIEW` (at schema, catalog, or metalake level), or ownership on are returned. + +2. **`loadView`** + - Requires `USE_CATALOG` + `USE_SCHEMA`, plus `SELECT_VIEW`, `ALTER_VIEW`, `CREATE_VIEW`, or view ownership (consistent with `loadTable`). + - If the user lacks privileges, the authorization framework denies access. + +3. **`createView`** + - Requires `USE_CATALOG` + `USE_SCHEMA` + (`CREATE_VIEW` at schema, catalog, or metalake, or schema ownership). + +4. **`alterView`** Review Comment: I think `ALTER VIEW` is still a broad term for supporting the main feature here. I have scoped out `RENAME` support for the current `ALTER_VIEW` feature. ########## design-docs/gravitino-view-privilege.md: ########## @@ -0,0 +1,324 @@ +<!-- + 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**: Complete the privilege management and protect against the `DROP` operation, similar to function privilege management. + +2. **Enforce View Privileges on Generic REST Endpoints**: Enforce privilege authentication on REST endpoints. + +3. **Ownership Tracking for Views**: Track ownership in a manner similar to the function privilege management process, inheriting the existing ownership model. + +4. **View Security Mode**: Introduce Definer/Invoker security support for view privilege management, allowing owners flexibility in switching modes for new/existing views. + +6. **Backward Compatibility**: It should be backward compatible with existing privilege management and the underlying data engine if views have previously been defined within the sub-system. + +--- + +## 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. **Per-User Identity Propagation in Spark and Flink Connectors**: Today's Gravitino Spark and Flink connectors authenticate to Gravitino using a singleton service identity established at catalog initialization. Consequently, we only have session context-level identity and, unlike Trino, lack per-user caller identity. + +3. **Unifying Generic View Auth Paths**: The Data Lake/Warehouse and Database have different approaches to managing views, privileges, and security. Unifying them into a single model is out of scope. + +4. **Materialized Views and Temporary Views**: Out of scope per the parent design (only logical views are in scope). + +5. **Identity Mapping between Gravitino and Databases**: This implementation will not manage the identity alignment between Gravitino's user principal and the database's user principal. + +--- + +## Proposal + +### Privilege Types + +Three privilege types are defined for views, matching the existing `*_TABLE` and `*_FUNCTION` shape. `CREATE_VIEW` and `SELECT_VIEW` already exist; `ALTER_VIEW` is new. **Drop view operations are not gated by a dedicated privilege — they require ownership of the view itself, or ownership of any parent (schema, catalog, or metalake), consistent with `dropTable` / `dropFileset` / `dropFunction` in Gravitino.** + +| Privilege | Securable Object Levels | Description | Status | +|----------------|-----------------------------------------|------------------------------------------------------------------------------------------|----------| +| `CREATE_VIEW` | Metalake, Catalog, Schema | Permission to create new views in a schema. | Existing | +| `SELECT_VIEW` | Metalake, Catalog, Schema, View | Permission to read view metadata and resolve the view's SQL during query planning. | Existing | +| `ALTER_VIEW` | Metalake, Catalog, Schema, View | Permission to alter a view's metadata (comment, properties, representations, rename). | **New** | + +**Naming rationale:** + +- `CREATE_VIEW` — Consistent with `CREATE_TABLE`, `CREATE_FILESET`. Used for delegated objects whose primary store is the underlying catalog. No change. +- `SELECT_VIEW` — Consistent with `SELECT_TABLE`. Reading view metadata is the analog of selecting from a table at the metadata layer; the actual SELECT against underlying data remains the engine's responsibility. No change. +- `ALTER_VIEW` — Matches the parent View Management design doc and standard SQL `ALTER VIEW` syntax. Covers all alter operations supported by the parent design (rename, updateComment, setProperty, removeProperty, addRepresentation, updateRepresentation, removeRepresentation). Diverges from Gravitino's existing `MODIFY_TABLE` / `MODIFY_FUNCTION` convention but aligns with the parent doc and with the `alterView` REST/API/Java method names. + +**Privilege inheritance:** Privileges granted at metalake, catalog, or schema level cascade to all views within that scope. Same as `*_TABLE` and `*_FUNCTION`. + +**Deny privileges:** Each privilege has a corresponding deny form (`DENY_CREATE_VIEW`, `DENY_SELECT_VIEW`, `DENY_ALTER_VIEW`). Existing deny entries for `CREATE_VIEW` and `SELECT_VIEW` already exist at `Privileges.java:319-321`; `DENY_ALTER_VIEW` is added. + +**Bitmask allocation:** `CREATE_VIEW` and `SELECT_VIEW` keep their existing bits (`1L << 28`, `1L << 29`). `ALTER_VIEW` is allocated the next free bit after `MODIFY_FUNCTION (1L << 32)` — `1L << 33`. + +--- + +### Securable Object Hierarchy + +Views are schema-scoped, following Gravitino's standard four-level hierarchy: + +``` +metalake + └── catalog + └── schema + └── view +``` + +`MetadataObject.Type.VIEW` already exists, and `VIEW_SUPPORTED_TYPES` in `Privileges.java:60-65` already covers METALAKE, CATALOG, SCHEMA, VIEW. `VIEW` is also already accepted by `MANAGE_GRANTS_SUPPORTED_TYPES` (line 86). No structural changes are required. + +**Privilege and ownership applicability by level:** + +| Securable Object | CREATE_VIEW | SELECT_VIEW | ALTER_VIEW | Drop (ownership-gated) | +|------------------|-------------|-------------|------------|------------------------| +| Metalake | ✅ | ✅ | ✅ | ✅ (metalake owner) | +| Catalog | ✅ | ✅ | ✅ | ✅ (catalog owner) | +| Schema | ✅ | ✅ | ✅ | ✅ (schema owner) | +| View | — | ✅ | ✅ | ✅ (view owner) | + +> `CREATE_VIEW` is not applicable at the view level because creation happens at the schema level (a view must be created within a schema). +> +> Drop has no dedicated privilege — it is authorized by ownership at any level. Owning a parent (metalake / catalog / schema) implicitly authorizes dropping any view in that scope. The full expression is shown in Authorization Enforcement below. + +--- + +### Visibility Control + +View visibility and access follow the same patterns as tables: + +1. **`listViews`** + - Requires `USE_CATALOG` + `USE_SCHEMA` at the endpoint level to access the schema (consistent with `listTables`). + - Applies `FILTER_VIEW_AUTHORIZATION_EXPRESSION` (`AuthorizationExpressionConstants.java:119`) to the result set — only views the user has `SELECT_VIEW`, `ALTER_VIEW`, `CREATE_VIEW` (at schema, catalog, or metalake level), or ownership on are returned. + +2. **`loadView`** + - Requires `USE_CATALOG` + `USE_SCHEMA`, plus `SELECT_VIEW`, `ALTER_VIEW`, `CREATE_VIEW`, or view ownership (consistent with `loadTable`). + - If the user lacks privileges, the authorization framework denies access. + +3. **`createView`** + - Requires `USE_CATALOG` + `USE_SCHEMA` + (`CREATE_VIEW` at schema, catalog, or metalake, or schema ownership). + +4. **`alterView`** Review Comment: @roryqi I think `ALTER VIEW` is still a broad term for supporting the main feature here. I have scoped out `RENAME` support for the current `ALTER_VIEW` feature. -- 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]
