Copilot commented on code in PR #13790: URL: https://github.com/apache/gravitino/pull/13790#discussion_r4247753732
########## design-docs/gravitino-authorization-without-jcasbin.md: ########## @@ -0,0 +1,461 @@ +<!-- + 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 a JCasbin-free Policy Evaluator for the Built-in Authorizer in Gravitino + +| Item | Value | +|---|---| +| Implementation PR | [#12286](https://github.com/apache/gravitino/pull/12286) | +| Related issues | [#12173](https://github.com/apache/gravitino/issues/12173) (per-role policy index), [#12282](https://github.com/apache/gravitino/issues/12282) (remove the enforcer), [#13748](https://github.com/apache/gravitino/issues/13748) (shared role bindings) | +| Scope | `JcasbinAuthorizer` in `server-common`, `AuthorizationRequestContext` in `core`, the JCasbin dependency in `server-common` and Iceberg REST | + +--- + Review Comment: The tables are written with a leading double pipe (`|| ... | ... |`), which renders as an extra empty first column in GitHub-flavored Markdown. If the intent is a 2-column table, switch to a single leading pipe (`| ... | ... |`) across these tables to avoid misrendering. ########## design-docs/gravitino-authorization-without-jcasbin.md: ########## @@ -0,0 +1,461 @@ +<!-- + 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 a JCasbin-free Policy Evaluator for the Built-in Authorizer in Gravitino + +| Item | Value | +|---|---| +| Implementation PR | [#12286](https://github.com/apache/gravitino/pull/12286) | +| Related issues | [#12173](https://github.com/apache/gravitino/issues/12173) (per-role policy index), [#12282](https://github.com/apache/gravitino/issues/12282) (remove the enforcer), [#13748](https://github.com/apache/gravitino/issues/13748) (shared role bindings) | +| Scope | `JcasbinAuthorizer` in `server-common`, `AuthorizationRequestContext` in `core`, the JCasbin dependency in `server-common` and Iceberg REST | + +--- + +## Background + +### What the authorizer decides + +Gravitino's built-in authorizer answers questions of the form: + +> Can the current caller use privilege `P` on metadata object `O` (type + id) in metalake `M`? + +Its inputs are: + +| Input | Source | +|---|---| +| The caller | A user stored in Gravitino, plus the groups carried by the request. With OAuth/JWT, groups come from the token's groups claim and live only in the request's `UserPrincipal`; Gravitino does not store user-to-group membership. | +| Roles held by the caller | Roles granted to the user directly, plus roles granted to each of the request's groups (`user_role_rel`, `group_role_rel`). | +| What each role allows or denies | The role's securable objects; each privilege has condition `ALLOW` or `DENY`. | +| Active roles | With role assumption, a request can narrow its ALLOW privileges to a subset of its roles. DENY always applies over all held roles. | +| Ownership | `OWNER` is answered from the owner relation (`owner_meta`), not from role privileges. | + +The decision rules are: + +1. A `DENY` from **any** held role on `(O, P)` wins. +2. Otherwise, an `ALLOW` from any **active** role on `(O, P)` grants access. +3. Otherwise the check is denied. Callers walk parent scopes (metalake, catalog, schema chain, object) and stop at the first scope that grants. + +List endpoints filter every returned object through these rules. `hasDenyPolicy` is a list shortcut: if the caller holds no DENY for the relevant privileges at any scope, a parent-level ALLOW is enough and per-object checks can be skipped. + +### How JCasbin is used today + +The authorizer embeds two JCasbin `SyncedEnforcer` instances, one for ALLOW and one for DENY, with this model (`jcasbin_model.conf`): + +```ini +[request_definition] +r = sub, metadataType, metadataId, act + +[policy_definition] +p = sub, metadataType, metadataId, act, eft + +[role_definition] +g = _, _ + +[policy_effect] +e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) + +[matchers] +m = g(r.sub, p.sub) && r.metadataId == p.metadataId && r.metadataType == p.metadataType && ((p.act == "OWNER" && r.act == "OWNER") || r.act == p.act) +``` + +Gravitino state is translated into JCasbin rows: + +| Gravitino state | JCasbin row | Written when | +|---|---|---| +| Role `R` grants `SELECT_TABLE` on table 42 | `p, R, TABLE, 42, SELECT_TABLE, allow` | Role policies are loaded or reloaded | +| User `U` holds role `R`, directly or through a request group | `g, U, R` | Each request binds its roles and prunes stale ones | + +A check calls `enforcer.enforce(userId, type, id, privilege)`. Around the enforcers, the authorizer already keeps version-validated caches: + +- `userRoleCache` / `groupRoleCache`: user or group to role ids, validated by `user_meta.updated_at` / `group_meta.updated_at`. +- `loadedRoles`: role id to its loaded version (`role_meta.updated_at`). Its removal listener deletes the role's `p` rows from both enforcers. +- `metadataIdCache` / `ownerRelCache`: name to id and id to owner, invalidated by local hooks and the entity change-log poller. +- A global `ReentrantReadWriteLock` guards every enforcer read and write. + +In practice JCasbin is used as an in-memory table of `(role, type, id, privilege, effect)` rows plus a user-to-role edge list. None of its richer features are used: no role hierarchy, no domains, no ABAC expressions, and the adapter is a no-op. + +### Problem 1: every probe scans every policy on the node + +JCasbin 1.99.0 has no index by subject or object. `enforce` evaluates the Aviator matcher against **every** `p` row loaded in the enforcer, including rows of roles the caller does not hold. The enforcer accumulates rows for up to `GRAVITINO_AUTHORIZATION_ROLE_CACHE_SIZE` roles (default 10,000) across all metalakes. + +Standalone benchmark from #12173 (real model, one user with one role, one target object): + +| Roles loaded | `p` rows | `enforce` avg | Hash lookup | +|---|---|---|---| +| 100 | 6,000 | ~1.0 ms | 0.06 µs | +| 1,000 | 60,000 | ~12.8 ms | 0.015 µs | +| 5,000 | 300,000 | ~51 ms | 0.015 µs | +| 10,000 | 600,000 | ~106 ms | 0.015 µs | + +The cost is multiplied because one request makes many probes: + +- each scope in the parent walk, including every level of a hierarchical schema name; +- the allow enforcer and the deny enforcer; +- with active-role narrowing, one probe per active role (`enforceNarrowed`); +- in list endpoints, all of the above per listed object. + +The latency of one user's list call therefore depends on how many policies *other* users' roles have loaded on the same node. + +### Problem 2: a shared, mutable user-to-role graph + +Role membership is computed per request (user roles plus roles of the request's groups), but then **published** to shared `g(userId, roleId)` rows that every request of the same user reads. + +**Correctness (#13748).** The same user can send concurrent requests with different group claims, for example an old token in a long-running Spark job and a new token after an IdP group change. Each request binds its roles and prunes the ones it does not hold, so one request can overwrite the other's view: + +| Step | Event | Shared `g(alice, ...)` | +|---|---|---| +| 1 | Request A (`groups=[analysts, contractors]`) binds `read_sales` and `no_pii` (DENY) | read_sales, no_pii | +| 2 | Request B (`groups=[analysts]`) prunes `no_pii`, which it does not hold | read_sales | +| 3 | A checks `SELECT_TABLE` on `sales.orders` | **allowed**: the DENY was lost | + +The reverse interleaving injects an ALLOW into a request that should hold no role. + +**Duplicated state.** Role membership already lives in version-validated caches. The `g` rows are a second copy that must be bound, pruned, and kept consistent with the first. Binding and pruning take the global write lock on the request path, and policy reload cannot use `deleteRole`, because that would also drop other requests' fresh `g` rows. + +### Problem 3: policy reload mutates shared state in place + +Reloading a role means removing its `p` rows and then adding the new ones on a shared enforcer. Without a lock, a concurrent `enforce` could observe the empty intermediate state and deny a valid request. All enforcer access therefore goes through one global read-write lock, and the cache removal listener also mutates the enforcer. Any eviction, including ordinary TTL or size eviction, removes rows that in-flight requests still depend on, and those requests must detect the change and reload. + +### Problem 4: the DENY existence check works on string rows + +`hasDenyPolicy` iterates `getRolesForUser` and `getFilteredNamedPolicy("p", 0, roleId)` and compares positional `List<String>` fields. Every policy is stored as a list of strings, with ids converted to `String`. There is no per-role summary of denied privileges. + +### Problem 5: dependency weight without matching benefit + +JCasbin brings the enforcer, Aviator, model files, a no-op adapter, engine-specific tests, and license entries in `LICENSE.bin` and `LICENSE.iceberg`. The work it actually does is an equality match that a hash map does in O(1). + +--- + +## Goals + +1. **Policy-count-independent probes**: The cost of one authorization probe does not depend on the number of policies loaded on the node. It is at most O(roles held by the caller) hash lookups, and O(1) after a request builds its merged index (see [Adaptive merge](#adaptive-merge-for-long-lists)). +2. **Request-isolated role membership**: A request's effective roles are computed from its own principal and stored only in its own context. No request can add or remove roles seen by another request. +3. **Immutable published policies**: Once a role's policies are published, they are never edited in place. Reload replaces a reference. +4. **Unchanged decisions**: Global DENY precedence, active-role narrowing for ALLOW only, OWNER via the owner relation, `role_meta.updated_at` version validation, and fail-closed handling of unreadable or unresolvable roles behave as today. This is verified by a differential comparison against the current JCasbin model. +5. **Bounded staleness**: After a role is changed or invalidated on a node, no new decision on that node uses the old version of the role. +6. **No JCasbin dependency**: `server-common` and the Iceberg REST server no longer depend on JCasbin or Aviator. + +--- + +## Non-Goals + +1. **Privilege model changes**: No new privileges, conditions, or decision rules. This design changes how decisions are computed, not what they are. +2. **Public API or configuration changes**: No REST, client, or configuration-key change. The class name `JcasbinAuthorizer` is kept so that existing `gravitino.authorization.impl` values keep working; renaming it is a separate decision. +3. **Cross-node invalidation changes**: The existing channels (local hooks and the entity change-log poller) are reused as they are. +4. **External policy engines**: OPA, Cedar, or Ranger are not introduced for the built-in authorizer; see [Solution Investigations](#solution-investigations). +5. **Persisting compiled policies**: Compiled indexes stay in memory and are rebuilt from role entities on demand, as JCasbin rows are today. No schema change is needed. + +--- + +## Solution Investigations + +| Approach | Pros | Cons | Decision | +|---|---|---|---| +| A. Keep JCasbin, replace `g(r.sub, p.sub)` with a request-aware `hasRole` function (#13747) | Small diff; fixes #13748 | Keeps the O(N) scan, in-place reload, the global lock around every probe, and the dependency | Rejected as the long-term design; acceptable as a short-term fix | +| B. Keep JCasbin, one enforcer per role or per user | Removes the global scan | Per role: still evaluates Aviator per row and pays model overhead per enforcer. Per user: duplicates policies across users and must be invalidated on every role change | Rejected | +| C. Precompute a per-user effective policy map | O(1) lookups | Group membership is per request, so the key is (user, group set, active roles). Any role change invalidates every combination that includes it, and the number of combinations is unbounded | Rejected | +| D. External policy engine (OPA, Cedar, Ranger) | Rich policy languages and central management | Adds a network hop or a new runtime, and still needs Gravitino's id resolution, versioning and owner logic. Ranger already exists as a separate authorization plugin | Rejected for the built-in authorizer | +| E. Immutable per-role hash indexes pinned by a request-local view | O(k) or O(1) probes; isolation by construction; no in-place mutation; no dependency | New concurrency code for publication and invalidation that must be proven correct | **Chosen** | + +Option A and option E both make role membership request-local, so they agree on #13748. Option E additionally removes the costs listed in Problems 1, 3, 4 and 5. + +--- + +## Proposal + +### Overview + +The key idea is: **resolve, don't scan.** A request already knows its role ids before it evaluates anything. Each role is compiled once into an immutable hash index, and the request pins references to the indexes of the roles it holds. + +``` +Shared, per role, immutable Per request, immutable +--------------------------------- ------------------------------------- +loadedRoles AuthorizationRequestContext + roleId -> CachedRolePolicies boundRoleIds = {7, 9} + updatedAt (role_meta version) rolePolicyView = RequestRolePolicies + index: PolicyKey -> Effect <--pin-- allowRoles, denyRoles + complete flag deniedPrivileges + deniedPrivileges summary generation +``` + +- The shared cache holds **per-role** data only. It contains no user or group membership. Review Comment: This diagram labels the per-request state as \"immutable\", but later `RequestRolePolicies` includes mutable fields (`remainingProbes`, lazily built `mergedIndex`, generation tracking). To avoid confusion, consider changing this wording to \"request-local\" (or explicitly clarify which parts are immutable vs. lazily/mutably updated within a single request). ########## design-docs/gravitino-authorization-without-jcasbin.md: ########## @@ -0,0 +1,461 @@ +<!-- + 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 a JCasbin-free Policy Evaluator for the Built-in Authorizer in Gravitino + +| Item | Value | +|---|---| +| Implementation PR | [#12286](https://github.com/apache/gravitino/pull/12286) | +| Related issues | [#12173](https://github.com/apache/gravitino/issues/12173) (per-role policy index), [#12282](https://github.com/apache/gravitino/issues/12282) (remove the enforcer), [#13748](https://github.com/apache/gravitino/issues/13748) (shared role bindings) | +| Scope | `JcasbinAuthorizer` in `server-common`, `AuthorizationRequestContext` in `core`, the JCasbin dependency in `server-common` and Iceberg REST | + +--- + +## Background + +### What the authorizer decides + +Gravitino's built-in authorizer answers questions of the form: + +> Can the current caller use privilege `P` on metadata object `O` (type + id) in metalake `M`? + +Its inputs are: + +| Input | Source | +|---|---| +| The caller | A user stored in Gravitino, plus the groups carried by the request. With OAuth/JWT, groups come from the token's groups claim and live only in the request's `UserPrincipal`; Gravitino does not store user-to-group membership. | +| Roles held by the caller | Roles granted to the user directly, plus roles granted to each of the request's groups (`user_role_rel`, `group_role_rel`). | +| What each role allows or denies | The role's securable objects; each privilege has condition `ALLOW` or `DENY`. | +| Active roles | With role assumption, a request can narrow its ALLOW privileges to a subset of its roles. DENY always applies over all held roles. | +| Ownership | `OWNER` is answered from the owner relation (`owner_meta`), not from role privileges. | + +The decision rules are: + +1. A `DENY` from **any** held role on `(O, P)` wins. +2. Otherwise, an `ALLOW` from any **active** role on `(O, P)` grants access. +3. Otherwise the check is denied. Callers walk parent scopes (metalake, catalog, schema chain, object) and stop at the first scope that grants. + +List endpoints filter every returned object through these rules. `hasDenyPolicy` is a list shortcut: if the caller holds no DENY for the relevant privileges at any scope, a parent-level ALLOW is enough and per-object checks can be skipped. + +### How JCasbin is used today + +The authorizer embeds two JCasbin `SyncedEnforcer` instances, one for ALLOW and one for DENY, with this model (`jcasbin_model.conf`): + +```ini +[request_definition] +r = sub, metadataType, metadataId, act + +[policy_definition] +p = sub, metadataType, metadataId, act, eft + +[role_definition] +g = _, _ + +[policy_effect] +e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) + +[matchers] +m = g(r.sub, p.sub) && r.metadataId == p.metadataId && r.metadataType == p.metadataType && ((p.act == "OWNER" && r.act == "OWNER") || r.act == p.act) +``` + +Gravitino state is translated into JCasbin rows: + +| Gravitino state | JCasbin row | Written when | +|---|---|---| +| Role `R` grants `SELECT_TABLE` on table 42 | `p, R, TABLE, 42, SELECT_TABLE, allow` | Role policies are loaded or reloaded | +| User `U` holds role `R`, directly or through a request group | `g, U, R` | Each request binds its roles and prunes stale ones | + +A check calls `enforcer.enforce(userId, type, id, privilege)`. Around the enforcers, the authorizer already keeps version-validated caches: + +- `userRoleCache` / `groupRoleCache`: user or group to role ids, validated by `user_meta.updated_at` / `group_meta.updated_at`. +- `loadedRoles`: role id to its loaded version (`role_meta.updated_at`). Its removal listener deletes the role's `p` rows from both enforcers. +- `metadataIdCache` / `ownerRelCache`: name to id and id to owner, invalidated by local hooks and the entity change-log poller. +- A global `ReentrantReadWriteLock` guards every enforcer read and write. + +In practice JCasbin is used as an in-memory table of `(role, type, id, privilege, effect)` rows plus a user-to-role edge list. None of its richer features are used: no role hierarchy, no domains, no ABAC expressions, and the adapter is a no-op. + +### Problem 1: every probe scans every policy on the node + +JCasbin 1.99.0 has no index by subject or object. `enforce` evaluates the Aviator matcher against **every** `p` row loaded in the enforcer, including rows of roles the caller does not hold. The enforcer accumulates rows for up to `GRAVITINO_AUTHORIZATION_ROLE_CACHE_SIZE` roles (default 10,000) across all metalakes. + +Standalone benchmark from #12173 (real model, one user with one role, one target object): + +| Roles loaded | `p` rows | `enforce` avg | Hash lookup | +|---|---|---|---| +| 100 | 6,000 | ~1.0 ms | 0.06 µs | +| 1,000 | 60,000 | ~12.8 ms | 0.015 µs | +| 5,000 | 300,000 | ~51 ms | 0.015 µs | +| 10,000 | 600,000 | ~106 ms | 0.015 µs | + +The cost is multiplied because one request makes many probes: + +- each scope in the parent walk, including every level of a hierarchical schema name; +- the allow enforcer and the deny enforcer; +- with active-role narrowing, one probe per active role (`enforceNarrowed`); +- in list endpoints, all of the above per listed object. + +The latency of one user's list call therefore depends on how many policies *other* users' roles have loaded on the same node. + +### Problem 2: a shared, mutable user-to-role graph + +Role membership is computed per request (user roles plus roles of the request's groups), but then **published** to shared `g(userId, roleId)` rows that every request of the same user reads. + +**Correctness (#13748).** The same user can send concurrent requests with different group claims, for example an old token in a long-running Spark job and a new token after an IdP group change. Each request binds its roles and prunes the ones it does not hold, so one request can overwrite the other's view: + +| Step | Event | Shared `g(alice, ...)` | +|---|---|---| +| 1 | Request A (`groups=[analysts, contractors]`) binds `read_sales` and `no_pii` (DENY) | read_sales, no_pii | +| 2 | Request B (`groups=[analysts]`) prunes `no_pii`, which it does not hold | read_sales | +| 3 | A checks `SELECT_TABLE` on `sales.orders` | **allowed**: the DENY was lost | + +The reverse interleaving injects an ALLOW into a request that should hold no role. + +**Duplicated state.** Role membership already lives in version-validated caches. The `g` rows are a second copy that must be bound, pruned, and kept consistent with the first. Binding and pruning take the global write lock on the request path, and policy reload cannot use `deleteRole`, because that would also drop other requests' fresh `g` rows. + +### Problem 3: policy reload mutates shared state in place + +Reloading a role means removing its `p` rows and then adding the new ones on a shared enforcer. Without a lock, a concurrent `enforce` could observe the empty intermediate state and deny a valid request. All enforcer access therefore goes through one global read-write lock, and the cache removal listener also mutates the enforcer. Any eviction, including ordinary TTL or size eviction, removes rows that in-flight requests still depend on, and those requests must detect the change and reload. + +### Problem 4: the DENY existence check works on string rows + +`hasDenyPolicy` iterates `getRolesForUser` and `getFilteredNamedPolicy("p", 0, roleId)` and compares positional `List<String>` fields. Every policy is stored as a list of strings, with ids converted to `String`. There is no per-role summary of denied privileges. + +### Problem 5: dependency weight without matching benefit + +JCasbin brings the enforcer, Aviator, model files, a no-op adapter, engine-specific tests, and license entries in `LICENSE.bin` and `LICENSE.iceberg`. The work it actually does is an equality match that a hash map does in O(1). + +--- + +## Goals + +1. **Policy-count-independent probes**: The cost of one authorization probe does not depend on the number of policies loaded on the node. It is at most O(roles held by the caller) hash lookups, and O(1) after a request builds its merged index (see [Adaptive merge](#adaptive-merge-for-long-lists)). +2. **Request-isolated role membership**: A request's effective roles are computed from its own principal and stored only in its own context. No request can add or remove roles seen by another request. +3. **Immutable published policies**: Once a role's policies are published, they are never edited in place. Reload replaces a reference. +4. **Unchanged decisions**: Global DENY precedence, active-role narrowing for ALLOW only, OWNER via the owner relation, `role_meta.updated_at` version validation, and fail-closed handling of unreadable or unresolvable roles behave as today. This is verified by a differential comparison against the current JCasbin model. +5. **Bounded staleness**: After a role is changed or invalidated on a node, no new decision on that node uses the old version of the role. +6. **No JCasbin dependency**: `server-common` and the Iceberg REST server no longer depend on JCasbin or Aviator. + +--- + +## Non-Goals + +1. **Privilege model changes**: No new privileges, conditions, or decision rules. This design changes how decisions are computed, not what they are. +2. **Public API or configuration changes**: No REST, client, or configuration-key change. The class name `JcasbinAuthorizer` is kept so that existing `gravitino.authorization.impl` values keep working; renaming it is a separate decision. +3. **Cross-node invalidation changes**: The existing channels (local hooks and the entity change-log poller) are reused as they are. +4. **External policy engines**: OPA, Cedar, or Ranger are not introduced for the built-in authorizer; see [Solution Investigations](#solution-investigations). +5. **Persisting compiled policies**: Compiled indexes stay in memory and are rebuilt from role entities on demand, as JCasbin rows are today. No schema change is needed. + +--- + +## Solution Investigations + +| Approach | Pros | Cons | Decision | +|---|---|---|---| +| A. Keep JCasbin, replace `g(r.sub, p.sub)` with a request-aware `hasRole` function (#13747) | Small diff; fixes #13748 | Keeps the O(N) scan, in-place reload, the global lock around every probe, and the dependency | Rejected as the long-term design; acceptable as a short-term fix | +| B. Keep JCasbin, one enforcer per role or per user | Removes the global scan | Per role: still evaluates Aviator per row and pays model overhead per enforcer. Per user: duplicates policies across users and must be invalidated on every role change | Rejected | +| C. Precompute a per-user effective policy map | O(1) lookups | Group membership is per request, so the key is (user, group set, active roles). Any role change invalidates every combination that includes it, and the number of combinations is unbounded | Rejected | +| D. External policy engine (OPA, Cedar, Ranger) | Rich policy languages and central management | Adds a network hop or a new runtime, and still needs Gravitino's id resolution, versioning and owner logic. Ranger already exists as a separate authorization plugin | Rejected for the built-in authorizer | +| E. Immutable per-role hash indexes pinned by a request-local view | O(k) or O(1) probes; isolation by construction; no in-place mutation; no dependency | New concurrency code for publication and invalidation that must be proven correct | **Chosen** | + +Option A and option E both make role membership request-local, so they agree on #13748. Option E additionally removes the costs listed in Problems 1, 3, 4 and 5. + +--- + +## Proposal + +### Overview + +The key idea is: **resolve, don't scan.** A request already knows its role ids before it evaluates anything. Each role is compiled once into an immutable hash index, and the request pins references to the indexes of the roles it holds. + +``` +Shared, per role, immutable Per request, immutable +--------------------------------- ------------------------------------- +loadedRoles AuthorizationRequestContext + roleId -> CachedRolePolicies boundRoleIds = {7, 9} + updatedAt (role_meta version) rolePolicyView = RequestRolePolicies + index: PolicyKey -> Effect <--pin-- allowRoles, denyRoles + complete flag deniedPrivileges + deniedPrivileges summary generation +``` + +- The shared cache holds **per-role** data only. It contains no user or group membership. +- A request computes its effective role ids (direct roles plus roles of its own groups), pins the matching `CachedRolePolicies`, and evaluates against them. +- Publishing a new role version or invalidating a role advances a **generation**. Before each new decision, a request whose pinned roles changed refreshes its view. + +### API changes + +There is no public API, REST, or client change. One internal interface is added to `core` so that the request context can hold an authorizer-defined view without depending on `server-common`: + +```java +// org.apache.gravitino.authorization.AuthorizationRequestContext +public interface RolePolicyView { + /** Returns the policy generation most recently validated for this view. */ + long generation(); +} + +@Nullable public RolePolicyView getRolePolicyView(); +public void setRolePolicyView(RolePolicyView view); +``` + +### Data model Review Comment: The snippet mixes a complete `RolePolicyView` interface with standalone method signatures that appear to belong to `AuthorizationRequestContext`, but that type isn’t shown. Consider wrapping `getRolePolicyView` / `setRolePolicyView` inside an `AuthorizationRequestContext` interface/class excerpt (or labeling them more explicitly) to avoid ambiguity about where these methods live. ########## design-docs/gravitino-authorization-without-jcasbin.md: ########## @@ -0,0 +1,461 @@ +<!-- + 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 a JCasbin-free Policy Evaluator for the Built-in Authorizer in Gravitino + +| Item | Value | +|---|---| +| Implementation PR | [#12286](https://github.com/apache/gravitino/pull/12286) | +| Related issues | [#12173](https://github.com/apache/gravitino/issues/12173) (per-role policy index), [#12282](https://github.com/apache/gravitino/issues/12282) (remove the enforcer), [#13748](https://github.com/apache/gravitino/issues/13748) (shared role bindings) | +| Scope | `JcasbinAuthorizer` in `server-common`, `AuthorizationRequestContext` in `core`, the JCasbin dependency in `server-common` and Iceberg REST | + +--- + +## Background + +### What the authorizer decides + +Gravitino's built-in authorizer answers questions of the form: + +> Can the current caller use privilege `P` on metadata object `O` (type + id) in metalake `M`? + +Its inputs are: + +| Input | Source | +|---|---| +| The caller | A user stored in Gravitino, plus the groups carried by the request. With OAuth/JWT, groups come from the token's groups claim and live only in the request's `UserPrincipal`; Gravitino does not store user-to-group membership. | +| Roles held by the caller | Roles granted to the user directly, plus roles granted to each of the request's groups (`user_role_rel`, `group_role_rel`). | +| What each role allows or denies | The role's securable objects; each privilege has condition `ALLOW` or `DENY`. | +| Active roles | With role assumption, a request can narrow its ALLOW privileges to a subset of its roles. DENY always applies over all held roles. | +| Ownership | `OWNER` is answered from the owner relation (`owner_meta`), not from role privileges. | + +The decision rules are: + +1. A `DENY` from **any** held role on `(O, P)` wins. +2. Otherwise, an `ALLOW` from any **active** role on `(O, P)` grants access. +3. Otherwise the check is denied. Callers walk parent scopes (metalake, catalog, schema chain, object) and stop at the first scope that grants. + +List endpoints filter every returned object through these rules. `hasDenyPolicy` is a list shortcut: if the caller holds no DENY for the relevant privileges at any scope, a parent-level ALLOW is enough and per-object checks can be skipped. + +### How JCasbin is used today + +The authorizer embeds two JCasbin `SyncedEnforcer` instances, one for ALLOW and one for DENY, with this model (`jcasbin_model.conf`): + +```ini +[request_definition] +r = sub, metadataType, metadataId, act + +[policy_definition] +p = sub, metadataType, metadataId, act, eft + +[role_definition] +g = _, _ + +[policy_effect] +e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) + +[matchers] +m = g(r.sub, p.sub) && r.metadataId == p.metadataId && r.metadataType == p.metadataType && ((p.act == "OWNER" && r.act == "OWNER") || r.act == p.act) +``` + +Gravitino state is translated into JCasbin rows: + +| Gravitino state | JCasbin row | Written when | +|---|---|---| +| Role `R` grants `SELECT_TABLE` on table 42 | `p, R, TABLE, 42, SELECT_TABLE, allow` | Role policies are loaded or reloaded | +| User `U` holds role `R`, directly or through a request group | `g, U, R` | Each request binds its roles and prunes stale ones | + +A check calls `enforcer.enforce(userId, type, id, privilege)`. Around the enforcers, the authorizer already keeps version-validated caches: + +- `userRoleCache` / `groupRoleCache`: user or group to role ids, validated by `user_meta.updated_at` / `group_meta.updated_at`. +- `loadedRoles`: role id to its loaded version (`role_meta.updated_at`). Its removal listener deletes the role's `p` rows from both enforcers. +- `metadataIdCache` / `ownerRelCache`: name to id and id to owner, invalidated by local hooks and the entity change-log poller. +- A global `ReentrantReadWriteLock` guards every enforcer read and write. + +In practice JCasbin is used as an in-memory table of `(role, type, id, privilege, effect)` rows plus a user-to-role edge list. None of its richer features are used: no role hierarchy, no domains, no ABAC expressions, and the adapter is a no-op. + +### Problem 1: every probe scans every policy on the node + +JCasbin 1.99.0 has no index by subject or object. `enforce` evaluates the Aviator matcher against **every** `p` row loaded in the enforcer, including rows of roles the caller does not hold. The enforcer accumulates rows for up to `GRAVITINO_AUTHORIZATION_ROLE_CACHE_SIZE` roles (default 10,000) across all metalakes. + +Standalone benchmark from #12173 (real model, one user with one role, one target object): + +| Roles loaded | `p` rows | `enforce` avg | Hash lookup | +|---|---|---|---| +| 100 | 6,000 | ~1.0 ms | 0.06 µs | +| 1,000 | 60,000 | ~12.8 ms | 0.015 µs | +| 5,000 | 300,000 | ~51 ms | 0.015 µs | +| 10,000 | 600,000 | ~106 ms | 0.015 µs | + +The cost is multiplied because one request makes many probes: + +- each scope in the parent walk, including every level of a hierarchical schema name; +- the allow enforcer and the deny enforcer; +- with active-role narrowing, one probe per active role (`enforceNarrowed`); +- in list endpoints, all of the above per listed object. + +The latency of one user's list call therefore depends on how many policies *other* users' roles have loaded on the same node. + +### Problem 2: a shared, mutable user-to-role graph + +Role membership is computed per request (user roles plus roles of the request's groups), but then **published** to shared `g(userId, roleId)` rows that every request of the same user reads. + +**Correctness (#13748).** The same user can send concurrent requests with different group claims, for example an old token in a long-running Spark job and a new token after an IdP group change. Each request binds its roles and prunes the ones it does not hold, so one request can overwrite the other's view: + +| Step | Event | Shared `g(alice, ...)` | +|---|---|---| +| 1 | Request A (`groups=[analysts, contractors]`) binds `read_sales` and `no_pii` (DENY) | read_sales, no_pii | +| 2 | Request B (`groups=[analysts]`) prunes `no_pii`, which it does not hold | read_sales | +| 3 | A checks `SELECT_TABLE` on `sales.orders` | **allowed**: the DENY was lost | + +The reverse interleaving injects an ALLOW into a request that should hold no role. + +**Duplicated state.** Role membership already lives in version-validated caches. The `g` rows are a second copy that must be bound, pruned, and kept consistent with the first. Binding and pruning take the global write lock on the request path, and policy reload cannot use `deleteRole`, because that would also drop other requests' fresh `g` rows. + +### Problem 3: policy reload mutates shared state in place + +Reloading a role means removing its `p` rows and then adding the new ones on a shared enforcer. Without a lock, a concurrent `enforce` could observe the empty intermediate state and deny a valid request. All enforcer access therefore goes through one global read-write lock, and the cache removal listener also mutates the enforcer. Any eviction, including ordinary TTL or size eviction, removes rows that in-flight requests still depend on, and those requests must detect the change and reload. + +### Problem 4: the DENY existence check works on string rows + +`hasDenyPolicy` iterates `getRolesForUser` and `getFilteredNamedPolicy("p", 0, roleId)` and compares positional `List<String>` fields. Every policy is stored as a list of strings, with ids converted to `String`. There is no per-role summary of denied privileges. + +### Problem 5: dependency weight without matching benefit + +JCasbin brings the enforcer, Aviator, model files, a no-op adapter, engine-specific tests, and license entries in `LICENSE.bin` and `LICENSE.iceberg`. The work it actually does is an equality match that a hash map does in O(1). + +--- + +## Goals + +1. **Policy-count-independent probes**: The cost of one authorization probe does not depend on the number of policies loaded on the node. It is at most O(roles held by the caller) hash lookups, and O(1) after a request builds its merged index (see [Adaptive merge](#adaptive-merge-for-long-lists)). +2. **Request-isolated role membership**: A request's effective roles are computed from its own principal and stored only in its own context. No request can add or remove roles seen by another request. +3. **Immutable published policies**: Once a role's policies are published, they are never edited in place. Reload replaces a reference. +4. **Unchanged decisions**: Global DENY precedence, active-role narrowing for ALLOW only, OWNER via the owner relation, `role_meta.updated_at` version validation, and fail-closed handling of unreadable or unresolvable roles behave as today. This is verified by a differential comparison against the current JCasbin model. +5. **Bounded staleness**: After a role is changed or invalidated on a node, no new decision on that node uses the old version of the role. +6. **No JCasbin dependency**: `server-common` and the Iceberg REST server no longer depend on JCasbin or Aviator. + +--- + +## Non-Goals + +1. **Privilege model changes**: No new privileges, conditions, or decision rules. This design changes how decisions are computed, not what they are. +2. **Public API or configuration changes**: No REST, client, or configuration-key change. The class name `JcasbinAuthorizer` is kept so that existing `gravitino.authorization.impl` values keep working; renaming it is a separate decision. +3. **Cross-node invalidation changes**: The existing channels (local hooks and the entity change-log poller) are reused as they are. +4. **External policy engines**: OPA, Cedar, or Ranger are not introduced for the built-in authorizer; see [Solution Investigations](#solution-investigations). +5. **Persisting compiled policies**: Compiled indexes stay in memory and are rebuilt from role entities on demand, as JCasbin rows are today. No schema change is needed. + +--- + +## Solution Investigations + +| Approach | Pros | Cons | Decision | +|---|---|---|---| +| A. Keep JCasbin, replace `g(r.sub, p.sub)` with a request-aware `hasRole` function (#13747) | Small diff; fixes #13748 | Keeps the O(N) scan, in-place reload, the global lock around every probe, and the dependency | Rejected as the long-term design; acceptable as a short-term fix | +| B. Keep JCasbin, one enforcer per role or per user | Removes the global scan | Per role: still evaluates Aviator per row and pays model overhead per enforcer. Per user: duplicates policies across users and must be invalidated on every role change | Rejected | +| C. Precompute a per-user effective policy map | O(1) lookups | Group membership is per request, so the key is (user, group set, active roles). Any role change invalidates every combination that includes it, and the number of combinations is unbounded | Rejected | +| D. External policy engine (OPA, Cedar, Ranger) | Rich policy languages and central management | Adds a network hop or a new runtime, and still needs Gravitino's id resolution, versioning and owner logic. Ranger already exists as a separate authorization plugin | Rejected for the built-in authorizer | Review Comment: This references `#13747` as plain text while most other issues/PRs in the doc are linked. Converting `(#13747)` into a full GitHub link (consistent with the other rows) would make navigation easier and keep references uniform. -- 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]
