This is an automated email from the ASF dual-hosted git repository.
bharos pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new dc4b20e957 [#13510] feat(api): add AccessControlContent policy type
(#13511)
dc4b20e957 is described below
commit dc4b20e9579ab3fdd6e631e15afc6fdd1029a829
Author: Bharath Krishna <[email protected]>
AuthorDate: Thu Oct 15 21:24:18 2026 -0300
[#13510] feat(api): add AccessControlContent policy type (#13511)
### What changes were proposed in this pull request?
Adds the `ACCESS_CONTROL` built-in policy type (wire name
`system_access_control`) and its
content class `AccessControlContent`, constructed through
`PolicyContents.accessControl(...)`.
The content carries two fields:
- `privileges` — the privileges the rule confers on an object the tag is
applied to.
- `applicableRoles` — the roles that satisfy the rule's condition. These
are a *condition* on
the caller's roles, not the principals the privilege is granted to.
A rule is authored as an ordinary policy and bound to a tag:
```java
PolicyContent content =
PolicyContents.accessControl(
ImmutableList.of(Privilege.Name.SELECT_TABLE,
Privilege.Name.MODIFY_TABLE),
ImmutableList.of("analyst", "data_engineer"));
```
which carries the content
```json
{
"privileges": ["SELECT_TABLE", "MODIFY_TABLE"],
"applicableRoles": ["analyst", "data_engineer"]
}
```
and, once bound to the tag `certified`, reads: *a caller whose active
roles include either
`analyst` or `data_engineer` may select from and modify any object
carrying the tag `certified`.*
Any listed role satisfies the condition; every listed privilege is then
conferred.
Two invariants are enforced in the class:
- `PERMITTED_PRIVILEGES` is an explicit allowlist rather than a
denylist, so the boundary fails
closed: a privilege added to `Privilege.Name` later confers nothing
through a tag until it is
listed here deliberately. `USE_CATALOG` and `USE_SCHEMA` are excluded on
purpose — traversal
stays RBAC, so a tag can only widen access inside territory a role can
already reach.
- `supportedObjectTypes()` is the union of `canBindTo` over
`PERMITTED_PRIVILEGES`, minus
`METALAKE`.
This is the API surface only. Nothing reads it yet: no authorizer
change, no enforcement, no
REST path.
**This is part 1 of M1** in the milestone table of the design doc. The
rest of M1 — the content
DTO, the `DTOConverters` branches, and the derived policy-to-role record
written on create and
update — follows in separate PRs. Until the DTO lands,
`system_access_control` is accepted by
`fromPolicyType` but is not creatable over REST.
### Why are the changes needed?
Tag-based access control needs a policy content type before a rule can
be authored, stored or
evaluated. Every later milestone rests on this shape.
Fix: #13510
### Does this PR introduce _any_ user-facing change?
New public API in the `api` module:
- `Policy.BuiltInType.ACCESS_CONTROL`, wire name `system_access_control`
- `AccessControlContent`, with `privileges()` and `applicableRoles()`
- `PolicyContents.accessControl(List<Privilege.Name>, List<String>)`
No property keys are added and no existing behaviour changes. The type
is not reachable over
REST yet.
### How was this patch tested?
Unit tests in `TestPolicyContents` and `TestPolicyBuiltInType`,
covering:
- every privilege in `PERMITTED_PRIVILEGES` is accepted, and privileges
outside it — including
`USE_CATALOG` and `USE_SCHEMA` — are rejected
- `privileges` and `applicableRoles` reject null, empty, blank and
empty-string entries
- `supportedObjectTypes()` is recomputed from `PERMITTED_PRIVILEGES` in
the test rather than
asserted against a hardcoded set, so the two cannot drift apart silently
- equality and the built-in type name mapping in both directions
```
./gradlew :api:test --tests '*TestPolicyContents*' --tests
'*TestPolicyBuiltInType*'
```
---
.../gravitino/policy/AccessControlContent.java | 193 +++++++++++++++++++++
.../java/org/apache/gravitino/policy/Policy.java | 3 +
.../apache/gravitino/policy/PolicyContents.java | 15 ++
.../gravitino/policy/TestPolicyBuiltInType.java | 11 ++
.../gravitino/policy/TestPolicyContents.java | 161 +++++++++++++++++
5 files changed, 383 insertions(+)
diff --git
a/api/src/main/java/org/apache/gravitino/policy/AccessControlContent.java
b/api/src/main/java/org/apache/gravitino/policy/AccessControlContent.java
new file mode 100644
index 0000000000..63c254a571
--- /dev/null
+++ b/api/src/main/java/org/apache/gravitino/policy/AccessControlContent.java
@@ -0,0 +1,193 @@
+/*
+ * 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.
+ */
+package org.apache.gravitino.policy;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.authorization.Privilege;
+
+/**
+ * The content of a {@link Policy.BuiltInType#ACCESS_CONTROL} policy.
+ *
+ * <p>The policy is bound to a tag, and every object carrying that tag confers
{@link #privileges()}
+ * on any caller whose active roles include one of {@link #applicableRoles()}.
The roles are a
+ * condition on the rule, not the principals it is granted to.
+ */
+public final class AccessControlContent implements PolicyContent {
+
+ /** Rule key for the privileges the rule confers. */
+ public static final String PRIVILEGES_KEY = "privileges";
+
+ /** Rule key for the roles that satisfy the rule's condition. */
+ public static final String APPLICABLE_ROLES_KEY = "applicableRoles";
+
+ /**
+ * The privileges a tag is allowed to confer.
+ *
+ * <p>An allowlist rather than a denylist, so the boundary fails closed: a
privilege added to
+ * {@link Privilege.Name} later confers nothing through a tag until it is
added here deliberately.
+ * It holds only privileges that act on the tagged object itself. Privileges
that hand out access,
+ * create entities, change what other principals execute, or open a path to
untagged objects are
+ * excluded.
+ */
+ public static final Set<Privilege.Name> PERMITTED_PRIVILEGES =
+ ImmutableSet.of(
+ Privilege.Name.SELECT_TABLE,
+ Privilege.Name.MODIFY_TABLE,
+ Privilege.Name.PROBE_TABLE_LIKE,
+ Privilege.Name.SELECT_VIEW,
+ Privilege.Name.READ_FILESET,
+ Privilege.Name.WRITE_FILESET,
+ Privilege.Name.CONSUME_TOPIC,
+ Privilege.Name.PRODUCE_TOPIC,
+ Privilege.Name.USE_MODEL,
+ Privilege.Name.EXECUTE_FUNCTION);
+
+ // The object types a tag can be applied to, minus COLUMN, which no
permitted privilege can be
+ // scoped to. METALAKE is absent because a tag cannot be applied to a
metalake.
+ private static final Set<MetadataObject.Type> SUPPORTED_OBJECT_TYPES =
+ ImmutableSet.of(
+ MetadataObject.Type.CATALOG,
+ MetadataObject.Type.SCHEMA,
+ MetadataObject.Type.TABLE,
+ MetadataObject.Type.VIEW,
+ MetadataObject.Type.FILESET,
+ MetadataObject.Type.TOPIC,
+ MetadataObject.Type.MODEL,
+ MetadataObject.Type.FUNCTION);
+
+ private final List<Privilege.Name> privileges;
+ private final List<String> applicableRoles;
+
+ /** Default constructor for Jackson deserialization only. */
+ private AccessControlContent() {
+ this(null, null);
+ }
+
+ AccessControlContent(List<Privilege.Name> privileges, List<String>
applicableRoles) {
+ // Copied without rejecting nulls so that validate() is the single place
that reports a bad
+ // entry, with the offending value named.
+ this.privileges =
+ privileges == null
+ ? Collections.emptyList()
+ : Collections.unmodifiableList(new ArrayList<>(privileges));
+ this.applicableRoles =
+ applicableRoles == null
+ ? Collections.emptyList()
+ : Collections.unmodifiableList(new ArrayList<>(applicableRoles));
+ }
+
+ /**
+ * Returns the privileges this rule confers on the tagged object.
+ *
+ * @return the conferred privileges
+ */
+ public List<Privilege.Name> privileges() {
+ return privileges;
+ }
+
+ /**
+ * Returns the roles that satisfy this rule's condition.
+ *
+ * @return the applicable role names
+ */
+ public List<String> applicableRoles() {
+ return applicableRoles;
+ }
+
+ @Override
+ public Set<MetadataObject.Type> supportedObjectTypes() {
+ return SUPPORTED_OBJECT_TYPES;
+ }
+
+ @Override
+ public Map<String, String> properties() {
+ return ImmutableMap.of();
+ }
+
+ @Override
+ public Map<String, Object> rules() {
+ Map<String, Object> rules = new LinkedHashMap<>();
+ rules.put(
+ PRIVILEGES_KEY,
+ privileges.stream()
+ .map(privilege -> privilege == null ? null : privilege.name())
+ .collect(Collectors.toList()));
+ rules.put(APPLICABLE_ROLES_KEY, applicableRoles);
+ return Collections.unmodifiableMap(rules);
+ }
+
+ @Override
+ public void validate() throws IllegalArgumentException {
+ PolicyContent.super.validate();
+ Preconditions.checkArgument(!privileges.isEmpty(), "privileges cannot be
empty");
+ privileges.forEach(
+ privilege ->
+ Preconditions.checkArgument(
+ PERMITTED_PRIVILEGES.contains(privilege),
+ "privilege %s cannot be conferred by a tag, permitted
privileges are %s",
+ privilege,
+ PERMITTED_PRIVILEGES));
+
+ Preconditions.checkArgument(!applicableRoles.isEmpty(), "applicableRoles
cannot be empty");
+ applicableRoles.forEach(
+ role ->
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(role), "applicable role name cannot be
blank"));
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof AccessControlContent)) {
+ return false;
+ }
+ AccessControlContent that = (AccessControlContent) o;
+ return Objects.equals(privileges, that.privileges)
+ && Objects.equals(applicableRoles, that.applicableRoles);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(privileges, applicableRoles);
+ }
+
+ @Override
+ public String toString() {
+ return "AccessControlContent{"
+ + "privileges="
+ + privileges
+ + ", applicableRoles="
+ + applicableRoles
+ + '}';
+ }
+}
diff --git a/api/src/main/java/org/apache/gravitino/policy/Policy.java
b/api/src/main/java/org/apache/gravitino/policy/Policy.java
index c71a9c213b..337ca0f7ce 100644
--- a/api/src/main/java/org/apache/gravitino/policy/Policy.java
+++ b/api/src/main/java/org/apache/gravitino/policy/Policy.java
@@ -62,6 +62,9 @@ public interface Policy extends Auditable {
/** Built-in policy type for masking columns of tagged tables or tagged
columns. */
COLUMN_MASK(BUILT_IN_TYPE_PREFIX + "column_mask", ColumnMaskContent.class),
+ /** Built-in policy type for access conferred by a tag. */
+ ACCESS_CONTROL(BUILT_IN_TYPE_PREFIX + "access_control",
AccessControlContent.class),
+
/** Non-built-in policies use the fixed wire value {@code custom}. */
CUSTOM("custom", PolicyContents.CustomContent.class);
diff --git a/api/src/main/java/org/apache/gravitino/policy/PolicyContents.java
b/api/src/main/java/org/apache/gravitino/policy/PolicyContents.java
index b7e92193f6..9905811353 100644
--- a/api/src/main/java/org/apache/gravitino/policy/PolicyContents.java
+++ b/api/src/main/java/org/apache/gravitino/policy/PolicyContents.java
@@ -19,11 +19,13 @@
package org.apache.gravitino.policy;
import com.google.common.collect.ImmutableSet;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.annotation.Nullable;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.authorization.Privilege;
/** Utility class for creating instances of {@link PolicyContent}. */
public class PolicyContents {
@@ -218,6 +220,19 @@ public class PolicyContents {
return new ColumnMaskContent(expression);
}
+ /**
+ * Creates an access control policy content.
+ *
+ * @param privileges the privileges the rule confers on the tagged object,
each of which must be
+ * in {@link AccessControlContent#PERMITTED_PRIVILEGES}
+ * @param applicableRoles the roles that satisfy the rule's condition
+ * @return access control policy content
+ */
+ public static PolicyContent accessControl(
+ List<Privilege.Name> privileges, List<String> applicableRoles) {
+ return new AccessControlContent(privileges, applicableRoles);
+ }
+
private PolicyContents() {}
/**
diff --git
a/api/src/test/java/org/apache/gravitino/policy/TestPolicyBuiltInType.java
b/api/src/test/java/org/apache/gravitino/policy/TestPolicyBuiltInType.java
index 93df4a1d6a..ce2bde09a7 100644
--- a/api/src/test/java/org/apache/gravitino/policy/TestPolicyBuiltInType.java
+++ b/api/src/test/java/org/apache/gravitino/policy/TestPolicyBuiltInType.java
@@ -84,4 +84,15 @@ public class TestPolicyBuiltInType {
Assertions.assertEquals(RowFilterContent.class,
Policy.BuiltInType.ROW_FILTER.contentClass());
Assertions.assertEquals(ColumnMaskContent.class,
Policy.BuiltInType.COLUMN_MASK.contentClass());
}
+
+ @Test
+ void testAccessControlPolicyType() {
+ Assertions.assertEquals(
+ Policy.BuiltInType.ACCESS_CONTROL,
+ Policy.BuiltInType.fromPolicyType("system_access_control"));
+ Assertions.assertEquals(
+ "system_access_control",
Policy.BuiltInType.ACCESS_CONTROL.policyType());
+ Assertions.assertEquals(
+ AccessControlContent.class,
Policy.BuiltInType.ACCESS_CONTROL.contentClass());
+ }
}
diff --git
a/api/src/test/java/org/apache/gravitino/policy/TestPolicyContents.java
b/api/src/test/java/org/apache/gravitino/policy/TestPolicyContents.java
index 4bac0a9d0f..2e13103d2a 100644
--- a/api/src/test/java/org/apache/gravitino/policy/TestPolicyContents.java
+++ b/api/src/test/java/org/apache/gravitino/policy/TestPolicyContents.java
@@ -20,9 +20,16 @@
package org.apache.gravitino.policy;
import com.google.common.collect.ImmutableSet;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.authorization.Privileges;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -172,6 +179,160 @@ public class TestPolicyContents {
Assertions.assertTrue(exception.getMessage().contains("maxPartitionNum"));
}
+ @Test
+ void testAccessControlContent() {
+ AccessControlContent content =
+ (AccessControlContent)
+ PolicyContents.accessControl(
+ Arrays.asList(Privilege.Name.SELECT_TABLE,
Privilege.Name.MODIFY_TABLE),
+ Arrays.asList("analyst", "data_engineer"));
+
+ Assertions.assertDoesNotThrow(content::validate);
+ Assertions.assertEquals(
+ Arrays.asList(Privilege.Name.SELECT_TABLE,
Privilege.Name.MODIFY_TABLE),
+ content.privileges());
+ Assertions.assertEquals(Arrays.asList("analyst", "data_engineer"),
content.applicableRoles());
+ Assertions.assertEquals(
+ Arrays.asList("SELECT_TABLE", "MODIFY_TABLE"),
+ content.rules().get(AccessControlContent.PRIVILEGES_KEY));
+ Assertions.assertEquals(
+ Arrays.asList("analyst", "data_engineer"),
+ content.rules().get(AccessControlContent.APPLICABLE_ROLES_KEY));
+ Assertions.assertTrue(content.properties().isEmpty());
+ }
+
+ @Test
+ void testAccessControlContentSupportedObjectTypes() {
+ Assertions.assertEquals(
+ ImmutableSet.of(
+ MetadataObject.Type.CATALOG,
+ MetadataObject.Type.SCHEMA,
+ MetadataObject.Type.TABLE,
+ MetadataObject.Type.VIEW,
+ MetadataObject.Type.FILESET,
+ MetadataObject.Type.TOPIC,
+ MetadataObject.Type.MODEL,
+ MetadataObject.Type.FUNCTION),
+ PolicyContents.accessControl(
+ Collections.singletonList(Privilege.Name.SELECT_TABLE),
+ Collections.singletonList("analyst"))
+ .supportedObjectTypes());
+ }
+
+ @Test
+ void testAccessControlContentSupportedObjectTypesTrackPermittedPrivileges() {
+ Set<MetadataObject.Type> union =
+ Arrays.stream(MetadataObject.Type.values())
+ .filter(type -> type != MetadataObject.Type.METALAKE)
+ .filter(
+ type ->
+ AccessControlContent.PERMITTED_PRIVILEGES.stream()
+ .anyMatch(name ->
Privileges.allow(name).canBindTo(type)))
+ .collect(Collectors.toSet());
+
+ Assertions.assertEquals(
+ union,
+ PolicyContents.accessControl(
+ Collections.singletonList(Privilege.Name.SELECT_TABLE),
+ Collections.singletonList("analyst"))
+ .supportedObjectTypes(),
+ "supportedObjectTypes must stay the union of canBindTo over
PERMITTED_PRIVILEGES, "
+ + "minus METALAKE, which a tag cannot be applied to");
+ }
+
+ @Test
+ void testAccessControlContentAcceptsEveryPermittedPrivilege() {
+ for (Privilege.Name privilege : AccessControlContent.PERMITTED_PRIVILEGES)
{
+ PolicyContent content =
+ PolicyContents.accessControl(
+ Collections.singletonList(privilege),
Collections.singletonList("analyst"));
+ Assertions.assertDoesNotThrow(content::validate, privilege + " should be
permitted");
+ }
+ }
+
+ @Test
+ void testAccessControlContentRejectsPrivilegesOutsideAllowlist() {
+ for (Privilege.Name privilege : Privilege.Name.values()) {
+ if (AccessControlContent.PERMITTED_PRIVILEGES.contains(privilege)) {
+ continue;
+ }
+
+ PolicyContent content =
+ PolicyContents.accessControl(
+ Collections.singletonList(privilege),
Collections.singletonList("analyst"));
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ content::validate,
+ privilege + " should not be conferrable by a tag");
+ Assertions.assertTrue(exception.getMessage().contains(privilege.name()));
+ }
+ }
+
+ @Test
+ void testAccessControlContentRejectsEmptyPrivileges() {
+ List<String> roles = Collections.singletonList("analyst");
+ Assertions.assertTrue(
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PolicyContents.accessControl(null, roles).validate())
+ .getMessage()
+ .contains("privileges cannot be empty"));
+ Assertions.assertTrue(
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PolicyContents.accessControl(Collections.emptyList(),
roles).validate())
+ .getMessage()
+ .contains("privileges cannot be empty"));
+ }
+
+ @Test
+ void testAccessControlContentRejectsInvalidApplicableRoles() {
+ List<Privilege.Name> privileges =
Collections.singletonList(Privilege.Name.SELECT_TABLE);
+ Assertions.assertTrue(
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PolicyContents.accessControl(privileges,
Collections.emptyList()).validate())
+ .getMessage()
+ .contains("applicableRoles cannot be empty"));
+ Assertions.assertTrue(
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PolicyContents.accessControl(privileges,
null).validate())
+ .getMessage()
+ .contains("applicableRoles cannot be empty"));
+
+ for (List<String> roles :
+ Arrays.asList(
+ Arrays.asList("analyst", " "),
+ Arrays.asList("analyst", ""),
+ Arrays.asList("analyst", (String) null))) {
+ Assertions.assertTrue(
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> PolicyContents.accessControl(privileges,
roles).validate())
+ .getMessage()
+ .contains("applicable role name cannot be blank"),
+ roles + " should be rejected");
+ }
+ }
+
+ @Test
+ void testAccessControlContentEquality() {
+ List<Privilege.Name> privileges =
Collections.singletonList(Privilege.Name.SELECT_TABLE);
+ PolicyContent content =
+ PolicyContents.accessControl(privileges,
Collections.singletonList("analyst"));
+
+ Assertions.assertEquals(
+ content, PolicyContents.accessControl(privileges,
Collections.singletonList("analyst")));
+ Assertions.assertEquals(
+ content.hashCode(),
+ PolicyContents.accessControl(privileges,
Collections.singletonList("analyst")).hashCode());
+ Assertions.assertNotEquals(
+ content,
+ PolicyContents.accessControl(privileges,
Collections.singletonList("data_engineer")));
+ }
+
private static Map<String, String> mapOf(String key, String value) {
Map<String, String> map = new HashMap<>();
map.put(key, value);
