bharos commented on code in PR #12757:
URL: https://github.com/apache/gravitino/pull/12757#discussion_r3938954158


##########
design-docs/tag-based-access-control.md:
##########
@@ -0,0 +1,428 @@
+<!--
+  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 Tag-Based Access Control in Gravitino
+
+**Status:** draft for discussion. The [open questions](#open-questions) are 
deliberately left
+undecided in this revision, each presented with its options; decisions will be 
folded in after
+review.
+
+Discussion: [#12619](https://github.com/apache/gravitino/discussions/12619)
+
+---
+
+## Summary
+
+An access rule is a `Policy` of type `system_access_control` whose `content` 
carries an action and
+a role condition. The policy is bound to a tag. Any object carrying that tag 
becomes subject to
+the rule.
+
+```json
+POST /api/metalakes/prod/policies
+{
+  "name": "analyst_read_certified",
+  "policyType": "system_access_control",
+  "enabled": true,
+  "content": {
+    "action": "SELECT_TABLE",
+    "role":   { "name": "analyst" }
+  }
+}
+```
+
+```
+PUT  /api/metalakes/prod/tags/certified/policies/analyst_read_certified
+     { "selector": { "type": "ALL_VALUES" } }
+
+POST /api/metalakes/prod/objects/TABLE/lakehouse.finance.orders/tags
+     { "tagsToAdd": [{ "name": "certified" }] }
+```
+
+Read together: *members of the role `analyst` may `SELECT` any table that 
carries the tag
+`certified`.*
+
+There is exactly one attachment — the policy-to-tag bind. The role condition 
is a value inside
+`content`, not an association. No new user-facing entity, REST resource or 
client API is
+introduced.
+
+---
+
+## Background
+
+Gravitino authorizes metadata operations through RBAC. A grant names a 
securable object and a
+privilege and binds them to a role; the authorization expression on each REST 
endpoint evaluates
+those grants over the object's ancestor chain.
+
+Tags are a separate subsystem. They apply to catalogs, schemas, tables, views, 
topics, filesets,
+models, columns and functions, carry assignment values (see
+[tag-assignment-values.md](tag-assignment-values.md)), and inherit down the 
object hierarchy.
+Policy-on-tag ([policy-on-tag.md](policy-on-tag.md)) lets governance policies 
be selected by those
+tags. Authorization does not read tags at all.
+
+The consequence is that intent expressed as classification cannot drive 
access. An organization
+that already labels tables `certified`, `pii` or `data_domain=finance` must 
still enumerate grants
+object by object to act on those labels. New objects need new grants, dropped 
objects leave stale
+ones, and the rule itself is written down nowhere — it exists only as the 
accumulated set of grants
+someone remembered to issue.
+
+---
+
+## Scope
+
+### In this version
+
+- A rule of the form *(action, role condition)* bound to a tag.
+- `ALLOW` only.
+- Roles as the matched condition.
+- Evaluation inside the existing authorization-expression path, composing with 
RBAC.
+- Reuse of the `Policy` entity, the policy-to-tag relation and 
`PolicySelector`, so tag conditions
+  are written identically for governance and for authorization.
+- No new REST resource or client API. The only storage addition is an internal 
derived index, not
+  written or read by any endpoint — see [Lifecycle](#lifecycle).
+
+### Not in this version
+
+| Excluded | Reason |
+|---|---|
+| `DENY` | Deny that cannot be suppressed by re-tagging a descendant is a 
separate problem. Allow-only keeps v1 evaluation total and order-independent. |
+| Users and groups as the matched condition | The condition schema can gain 
them later without changing the model. |
+| Row filtering and column masking | Distinct policy types; this design 
governs whole-object decisions. |
+| Cross-tag conditions | A rule matches one tag. Conditions spanning several 
tags await the `EXPRESSION` selector type in 
[policy-on-tag.md](policy-on-tag.md). |
+| Column-level decisions | A tag on a column does not affect decisions about 
its table. |
+| A `scope` field in `content`, restricting a rule to a subtree | Not a 
security boundary: creating a policy needs metalake-wide `CREATE_POLICY`, so 
whoever writes the rule sets its reach. It is also not consulted when a tag is 
applied, so it filters effect rather than preventing a wrong tag. Tag 
assignment values with a value-sensitive selector cover the 
one-tag-different-subtrees case. Additive later — absent has always meant 
metalake-wide. |
+| Replacing RBAC | Baseline privileges, ownership and traversal are unchanged. 
See [Composition with RBAC](#composition-with-rbac). |
+
+---
+
+## Alternatives considered
+
+| Option | Pros | Cons | Status |
+|---|---|---|---|
+| **A `system_access_control` policy type bound to a tag** | Reuses the 
entity, relation, selector and resolver; no new REST or client surface; one 
governance model to learn | The role condition lives in `content` JSON, so 
lookup by role needs a derived index rather than a foreign key | **Proposed** |
+| A dedicated `tag_access_policy` entity with action and role as columns | 
Foreign key on role; indexed lookup; cascade on role deletion falls out of the 
schema | New table across three dialects, new REST resource, new client and CLI 
surface, a second governance model alongside policies | Rejected |
+| Extend RBAC grants with a tag predicate | No new concepts | The grant table 
is object-identified; a predicate has no object, and every grant read path 
would change | Rejected |
+| Evaluate tags in an external engine (OPA and similar) | Arbitrary policy 
language | Moves the decision out of Gravitino, duplicates the tag hierarchy, 
and cannot use the existing expression path | Rejected |
+
+The consequence of that one con — referential integrity maintained by the 
server rather than by the
+schema — is addressed in [Lifecycle](#lifecycle).
+
+---
+
+## Model
+
+### Content
+
+`PolicyContent` is an interface, and each built-in policy type has a concrete 
implementation with
+typed fields and a `validate()` that runs at write time. 
`IcebergDataCompactionContent` is the
+existing example. `system_access_control` follows the same pattern with a new
+`AccessControlContent`:
+
+| Field | Type | Meaning |
+|---|---|---|
+| `action` | `Privilege.Name` | The privilege the rule confers. Must be one of 
the permitted names — see [Composition with RBAC](#composition-with-rbac) for 
the exclusions. |
+| `role` | object with `name` | The **condition**. Matched against the 
caller's expanded roles. |
+
+`validate()` rejects at creation rather than at evaluation:
+
+- `action` parses to a permitted `Privilege.Name`.
+- `role` name is non-blank.
+
+Rejecting at write time matters because the alternative failure is silent: a 
policy naming an
+action that does not parse simply grants nothing, and nothing surfaces until 
someone notices the
+access they expected is missing.
+
+Whether `validate()` also requires the named role to *exist* is part of
+[OQ-3](#oq-3--deleting-a-referenced-role), not a separate decision.
+
+### `role` is a condition, not a principal
+
+The rule does not grant anything to `analyst`. It states that *if* the caller 
holds `analyst`
+among their expanded roles *and* the object carries `certified`, then 
`SELECT_TABLE` is satisfied
+for this request.
+
+The distinction matters for two reasons. The rule is not a grant, so it does 
not appear in the
+role's securable objects and does not participate in grant listing. And a role 
that is never
+assigned to anyone confers nothing, exactly as an unassigned role does today.
+
+### The tag bind
+
+The policy is attached to the tag through the existing policy-to-tag relation 
and its selector,
+exactly as governance policies are. `ALL_VALUES` in the example above matches 
the tag regardless of
+assignment value; value-sensitive selectors work as they do for governance 
policies, and nothing in
+this design is specific to `ALL_VALUES`.
+
+---
+
+## Evaluation
+
+An authorization decision needs to know, for the object being accessed and the 
caller's roles,
+whether any access rule is satisfied. That requires three things:
+
+1. the tags effective at the object after nearest-wins resolution
+   ([tag-assignment-values.md](tag-assignment-values.md)), including those 
inherited from ancestors;
+2. the `system_access_control` policies bound to those tags;
+3. for each, whether `role` is among the caller's expanded roles.
+
+The question is *when* steps 1 and 2 happen. Two options, presented without 
preference.
+
+### Option 1 — expand when roles load
+
+When a role's policies are loaded into the authorizer, walk the tags reachable 
for that role and
+materialise a policy row per (role, object), alongside the rows already 
produced from RBAC grants.
+
+- Tag-derived permissions become indistinguishable from RBAC ones at decision 
time. The existing
+  composition — including the deny path, which narrows across all of a 
caller's roles rather than
+  only the active ones — applies unchanged, with no new expression.
+- The decision itself costs nothing extra; the work moves to load time.
+- Expansion runs over *inherited* tags, so the row set for one role depends on 
the tag state of
+  every ancestor of every object it can reach. That set changes when a tag is 
applied anywhere in
+  the hierarchy, not only when the role changes.
+- A missed invalidation leaves rows in place that should have been withdrawn. 
The failure direction
+  is permissive.
+
+### Option 2 — evaluate as a second stage at request time
+
+Leave the loaded rows as they are. When the RBAC decision does not already 
allow the request,

Review Comment:
   Tags can only add access - content has no deny or condition field, and DENY 
is out of scope for phase 1. So no, tags can't restrict a full or partial RBAC 
allow.
   
   That's why the check runs second: if RBAC already allows, nothing a tag says 
could change the answer, so there's no point evaluating it. It's a fast path, 
not an ordering rule. And RBAC deny is untouched — the !ANY(DENY_…) conjunct 
comes from deny, which the tag path never touches, so a tag allow can't get 
around an explicit deny.
   
   One thing: I rewrote this section in 15dfa5f, so the line you quoted is 
gone. I am actually inclined towards Option 2 here, but please provide your 
thoughts based on the latest version of the doc



-- 
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