From 7e4738b73db3188cb551c88ee2d07b73d3ebff1d Mon Sep 17 00:00:00 2001 From: roryqi Date: Mon, 30 Sep 2024 19:23:17 +0800 Subject: [PATCH] [#5054] improvement(api,server): Add the check of privileges (#5053) ### What changes were proposed in this pull request? Add the check of privileges ### Why are the changes needed? Fix: #5054 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Add some UTs --------- Co-authored-by: Jerry Shao --- .../gravitino/authorization/Privilege.java | 10 + .../gravitino/authorization/Privileges.java | 111 ++++++++++ .../authorization/TestSecurableObjects.java | 191 ++++++++++++++++++ .../dto/authorization/PrivilegeDTO.java | 10 + .../gravitino/dto/util/DTOConverters.java | 15 ++ .../authorization/AuthorizationUtils.java | 133 ++++++++++-- .../server/web/rest/RoleOperations.java | 98 +++------ .../server/web/rest/TestRoleOperations.java | 74 ++++++- 8 files changed, 547 insertions(+), 95 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/authorization/Privilege.java b/api/src/main/java/org/apache/gravitino/authorization/Privilege.java index fbfde267151..3ca4107a12d 100644 --- a/api/src/main/java/org/apache/gravitino/authorization/Privilege.java +++ b/api/src/main/java/org/apache/gravitino/authorization/Privilege.java @@ -18,6 +18,7 @@ */ package org.apache.gravitino.authorization; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.annotation.Unstable; /** @@ -39,6 +40,15 @@ public interface Privilege { */ Condition condition(); + /** + * If the privilege can bind to a securable object, then this method will return true, otherwise + * false. + * + * @param type The securable object type. + * @return It will return true if the privilege can bind to a securable object, otherwise false. + */ + boolean canBindTo(MetadataObject.Type type); + /** The name of this privilege. */ enum Name { /** The privilege to create a catalog. */ diff --git a/api/src/main/java/org/apache/gravitino/authorization/Privileges.java b/api/src/main/java/org/apache/gravitino/authorization/Privileges.java index ef9e441b322..5255bce1c5a 100644 --- a/api/src/main/java/org/apache/gravitino/authorization/Privileges.java +++ b/api/src/main/java/org/apache/gravitino/authorization/Privileges.java @@ -18,11 +18,37 @@ */ package org.apache.gravitino.authorization; +import com.google.common.collect.Sets; import java.util.Objects; +import java.util.Set; +import org.apache.gravitino.MetadataObject; /** The helper class for {@link Privilege}. */ public class Privileges { + private static final Set TABLE_SUPPORTED_TYPES = + Sets.immutableEnumSet( + MetadataObject.Type.METALAKE, + MetadataObject.Type.CATALOG, + MetadataObject.Type.SCHEMA, + MetadataObject.Type.TABLE); + private static final Set TOPIC_SUPPORTED_TYPES = + Sets.immutableEnumSet( + MetadataObject.Type.METALAKE, + MetadataObject.Type.CATALOG, + MetadataObject.Type.SCHEMA, + MetadataObject.Type.TOPIC); + private static final Set SCHEMA_SUPPORTED_TYPES = + Sets.immutableEnumSet( + MetadataObject.Type.METALAKE, MetadataObject.Type.CATALOG, MetadataObject.Type.SCHEMA); + + private static final Set FILESET_SUPPORTED_TYPES = + Sets.immutableEnumSet( + MetadataObject.Type.METALAKE, + MetadataObject.Type.CATALOG, + MetadataObject.Type.SCHEMA, + MetadataObject.Type.FILESET); + /** * Returns the Privilege with allow condition from the string representation. * @@ -241,6 +267,11 @@ public static CreateCatalog allow() { public static CreateCatalog deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } } /** The privilege to use a catalog. */ @@ -263,6 +294,11 @@ public static UseCatalog allow() { public static UseCatalog deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE || type == MetadataObject.Type.CATALOG; + } } /** The privilege to use a schema. */ @@ -283,6 +319,11 @@ public static UseSchema allow() { public static UseSchema deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return SCHEMA_SUPPORTED_TYPES.contains(type); + } } /** The privilege to create a schema. */ @@ -305,6 +346,11 @@ public static CreateSchema allow() { public static CreateSchema deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE || type == MetadataObject.Type.CATALOG; + } } /** The privilege to create a table. */ @@ -327,6 +373,11 @@ public static CreateTable allow() { public static CreateTable deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return SCHEMA_SUPPORTED_TYPES.contains(type); + } } /** The privilege to select data from a table. */ @@ -349,6 +400,11 @@ public static SelectTable allow() { public static SelectTable deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return TABLE_SUPPORTED_TYPES.contains(type); + } } /** The privilege to execute SQL `ALTER`, `INSERT`, `UPDATE`, or `DELETE` for a table. */ @@ -371,6 +427,11 @@ public static ModifyTable allow() { public static ModifyTable deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return TABLE_SUPPORTED_TYPES.contains(type); + } } /** The privilege to create a fileset. */ @@ -393,6 +454,11 @@ public static CreateFileset allow() { public static CreateFileset deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return SCHEMA_SUPPORTED_TYPES.contains(type); + } } /** The privilege to read a fileset. */ @@ -415,6 +481,11 @@ public static ReadFileset allow() { public static ReadFileset deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return FILESET_SUPPORTED_TYPES.contains(type); + } } /** The privilege to write a fileset. */ @@ -437,6 +508,11 @@ public static WriteFileset allow() { public static WriteFileset deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return FILESET_SUPPORTED_TYPES.contains(type); + } } /** The privilege to create a topic. */ @@ -459,6 +535,11 @@ public static CreateTopic allow() { public static CreateTopic deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return SCHEMA_SUPPORTED_TYPES.contains(type); + } } /** The privilege to consume from a topic. */ @@ -481,6 +562,11 @@ public static ConsumeTopic allow() { public static ConsumeTopic deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return TOPIC_SUPPORTED_TYPES.contains(type); + } } /** The privilege to produce to a topic. */ @@ -503,6 +589,11 @@ public static ProduceTopic allow() { public static ProduceTopic deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return TOPIC_SUPPORTED_TYPES.contains(type); + } } /** The privilege to manage users. */ @@ -525,6 +616,11 @@ public static ManageUsers allow() { public static ManageUsers deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } } /** The privilege to manage groups. */ @@ -547,6 +643,11 @@ public static ManageGroups allow() { public static ManageGroups deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } } /** The privilege to create a role. */ @@ -569,6 +670,11 @@ public static CreateRole allow() { public static CreateRole deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } } /** The privilege to grant or revoke a role for the user or the group. */ @@ -591,5 +697,10 @@ public static ManageGrants allow() { public static ManageGrants deny() { return DENY_INSTANCE; } + + @Override + public boolean canBindTo(MetadataObject.Type type) { + return type == MetadataObject.Type.METALAKE; + } } } diff --git a/api/src/test/java/org/apache/gravitino/authorization/TestSecurableObjects.java b/api/src/test/java/org/apache/gravitino/authorization/TestSecurableObjects.java index 82374f676e3..f3066666d9c 100644 --- a/api/src/test/java/org/apache/gravitino/authorization/TestSecurableObjects.java +++ b/api/src/test/java/org/apache/gravitino/authorization/TestSecurableObjects.java @@ -157,4 +157,195 @@ public void testSecurableObjects() { Lists.newArrayList(Privileges.UseSchema.allow()))); Assertions.assertTrue(e.getMessage().contains("the length of names is 3")); } + + @Test + public void testPrivileges() { + Privilege createCatalog = Privileges.CreateCatalog.allow(); + Privilege useCatalog = Privileges.UseCatalog.allow(); + Privilege createSchema = Privileges.CreateSchema.allow(); + Privilege useSchema = Privileges.UseSchema.allow(); + Privilege createTable = Privileges.CreateTable.allow(); + Privilege selectTable = Privileges.SelectTable.allow(); + Privilege modifyTable = Privileges.ModifyTable.allow(); + Privilege createFileset = Privileges.CreateFileset.allow(); + Privilege readFileset = Privileges.ReadFileset.allow(); + Privilege writeFileset = Privileges.WriteFileset.allow(); + Privilege createTopic = Privileges.CreateTopic.allow(); + Privilege consumeTopic = Privileges.ConsumeTopic.allow(); + Privilege produceTopic = Privileges.ProduceTopic.allow(); + Privilege createRole = Privileges.CreateRole.allow(); + Privilege manageUsers = Privileges.ManageUsers.allow(); + Privilege manageGroups = Privileges.ManageGroups.allow(); + Privilege manageGrants = Privileges.ManageGrants.allow(); + + // Test create catalog + Assertions.assertTrue(createCatalog.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createCatalog.canBindTo(MetadataObject.Type.COLUMN)); + + // Test use catalog + Assertions.assertTrue(useCatalog.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(useCatalog.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(useCatalog.canBindTo(MetadataObject.Type.COLUMN)); + + // Test create schema + Assertions.assertTrue(createSchema.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(createSchema.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createSchema.canBindTo(MetadataObject.Type.COLUMN)); + + // Test use schema + Assertions.assertTrue(useSchema.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(useSchema.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(useSchema.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(useSchema.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(useSchema.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(useSchema.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(useSchema.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(useSchema.canBindTo(MetadataObject.Type.COLUMN)); + + // Test create table + Assertions.assertTrue(createTable.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(createTable.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(createTable.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createTable.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createTable.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createTable.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createTable.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createTable.canBindTo(MetadataObject.Type.COLUMN)); + + // Test select table + Assertions.assertTrue(selectTable.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(selectTable.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(selectTable.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertTrue(selectTable.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(selectTable.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(selectTable.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(selectTable.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(selectTable.canBindTo(MetadataObject.Type.COLUMN)); + + // Test modify table + Assertions.assertTrue(modifyTable.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(modifyTable.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(modifyTable.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertTrue(modifyTable.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(modifyTable.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(modifyTable.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(modifyTable.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(modifyTable.canBindTo(MetadataObject.Type.COLUMN)); + + // Test create topic + Assertions.assertTrue(createTopic.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(createTopic.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(createTopic.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createTopic.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createTopic.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createTopic.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createTopic.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createTopic.canBindTo(MetadataObject.Type.COLUMN)); + + // Test consume topic + Assertions.assertTrue(consumeTopic.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(consumeTopic.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(consumeTopic.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(consumeTopic.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertTrue(consumeTopic.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(consumeTopic.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(consumeTopic.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(consumeTopic.canBindTo(MetadataObject.Type.COLUMN)); + + // Test produce topic + Assertions.assertTrue(produceTopic.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(produceTopic.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(produceTopic.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(produceTopic.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertTrue(produceTopic.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(produceTopic.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(produceTopic.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(produceTopic.canBindTo(MetadataObject.Type.COLUMN)); + + // Test create fileset + Assertions.assertTrue(createFileset.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(createFileset.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(createFileset.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createFileset.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createFileset.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createFileset.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createFileset.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createFileset.canBindTo(MetadataObject.Type.COLUMN)); + + // Test read fileset + Assertions.assertTrue(readFileset.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(readFileset.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(readFileset.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(readFileset.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(readFileset.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertTrue(readFileset.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(readFileset.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(readFileset.canBindTo(MetadataObject.Type.COLUMN)); + + // Test write fileset + Assertions.assertTrue(writeFileset.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertTrue(writeFileset.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertTrue(writeFileset.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(writeFileset.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(writeFileset.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertTrue(writeFileset.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(writeFileset.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(writeFileset.canBindTo(MetadataObject.Type.COLUMN)); + + // Test create role + Assertions.assertTrue(createRole.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(createRole.canBindTo(MetadataObject.Type.COLUMN)); + + // Test manager users + Assertions.assertTrue(manageUsers.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(manageUsers.canBindTo(MetadataObject.Type.COLUMN)); + + // Test manager groups + Assertions.assertTrue(manageGroups.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(manageGroups.canBindTo(MetadataObject.Type.COLUMN)); + + // Test manager grants + Assertions.assertTrue(manageGrants.canBindTo(MetadataObject.Type.METALAKE)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.CATALOG)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.SCHEMA)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.TABLE)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.TOPIC)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.FILESET)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.ROLE)); + Assertions.assertFalse(manageGrants.canBindTo(MetadataObject.Type.COLUMN)); + } } diff --git a/common/src/main/java/org/apache/gravitino/dto/authorization/PrivilegeDTO.java b/common/src/main/java/org/apache/gravitino/dto/authorization/PrivilegeDTO.java index d31b693838c..e8554c015ad 100644 --- a/common/src/main/java/org/apache/gravitino/dto/authorization/PrivilegeDTO.java +++ b/common/src/main/java/org/apache/gravitino/dto/authorization/PrivilegeDTO.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.gravitino.MetadataObject; import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Privileges; @@ -65,6 +66,15 @@ public Condition condition() { return condition; } + @Override + public boolean canBindTo(MetadataObject.Type type) { + if (Condition.ALLOW.equals(condition)) { + return Privileges.allow(name).canBindTo(type); + } else { + return Privileges.deny(name).canBindTo(type); + } + } + /** @return the builder for creating a new instance of PrivilegeDTO. */ public static Builder builder() { return new Builder(); diff --git a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java index 38224493b71..adc1f5f03e0 100644 --- a/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java +++ b/common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java @@ -32,6 +32,7 @@ import org.apache.gravitino.authorization.Group; import org.apache.gravitino.authorization.Owner; import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.authorization.Privileges; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.User; @@ -1003,4 +1004,18 @@ public static Transform fromDTO(Partitioning partitioning) { throw new IllegalArgumentException("Unsupported partitioning: " + partitioning.strategy()); } } + + /** + * Converts a Privilege DTO to a Privilege + * + * @param privilegeDTO The privilege DTO to be converted. + * @return The privilege. + */ + public static Privilege fromPrivilegeDTO(PrivilegeDTO privilegeDTO) { + if (privilegeDTO.condition().equals(Privilege.Condition.ALLOW)) { + return Privileges.allow(privilegeDTO.name()); + } else { + return Privileges.deny(privilegeDTO.name()); + } + } } diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java index c182980b8ec..5a4a62cd60e 100644 --- a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java +++ b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java @@ -18,11 +18,14 @@ */ package org.apache.gravitino.authorization; -import com.google.common.collect.Lists; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.common.collect.Sets; import java.io.IOException; import java.util.List; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Supplier; import org.apache.gravitino.Catalog; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityStore; @@ -33,6 +36,10 @@ import org.apache.gravitino.catalog.CatalogManager; import org.apache.gravitino.connector.BaseCatalog; import org.apache.gravitino.connector.authorization.AuthorizationPlugin; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; +import org.apache.gravitino.dto.util.DTOConverters; +import org.apache.gravitino.exceptions.NoSuchCatalogException; +import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.utils.MetadataObjectUtil; import org.apache.gravitino.utils.NameIdentifierUtil; @@ -47,15 +54,15 @@ public class AuthorizationUtils { static final String ROLE_DOES_NOT_EXIST_MSG = "Role %s does not exist in th metalake %s"; private static final Logger LOG = LoggerFactory.getLogger(AuthorizationUtils.class); private static final String METALAKE_DOES_NOT_EXIST_MSG = "Metalake %s does not exist"; - - private static final List pluginNotSupportsPrivileges = - Lists.newArrayList( - Privilege.Name.CREATE_CATALOG, - Privilege.Name.USE_CATALOG, - Privilege.Name.MANAGE_GRANTS, - Privilege.Name.MANAGE_USERS, - Privilege.Name.MANAGE_GROUPS, - Privilege.Name.CREATE_ROLE); + private static final Set FILESET_PRIVILEGES = + Sets.immutableEnumSet( + Privilege.Name.CREATE_FILESET, Privilege.Name.WRITE_FILESET, Privilege.Name.READ_FILESET); + private static final Set TABLE_PRIVILEGES = + Sets.immutableEnumSet( + Privilege.Name.CREATE_TABLE, Privilege.Name.MODIFY_TABLE, Privilege.Name.SELECT_TABLE); + private static final Set TOPIC_PRIVILEGES = + Sets.immutableEnumSet( + Privilege.Name.CREATE_TOPIC, Privilege.Name.PRODUCE_TOPIC, Privilege.Name.CONSUME_TOPIC); private AuthorizationUtils() {} @@ -195,11 +202,10 @@ private static void callAuthorizationPluginImpl( } public static boolean needApplyAuthorizationPluginAllCatalogs(SecurableObject securableObject) { - // TODO: Add `supportsSecurableObjects` method for every privilege to simplify this code if (securableObject.type() == MetadataObject.Type.METALAKE) { List privileges = securableObject.privileges(); for (Privilege privilege : privileges) { - if (!pluginNotSupportsPrivileges.contains(privilege.name())) { + if (privilege.canBindTo(MetadataObject.Type.CATALOG)) { return true; } } @@ -207,6 +213,109 @@ public static boolean needApplyAuthorizationPluginAllCatalogs(SecurableObject se return false; } + // Check every securable object whether exists and is imported. + public static void checkSecurableObject(String metalake, MetadataObject object) { + NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalake, object); + + Supplier exceptionToThrowSupplier = + () -> + new NoSuchMetadataObjectException( + "Securable object %s doesn't exist", object.fullName()); + + switch (object.type()) { + case METALAKE: + check( + GravitinoEnv.getInstance().metalakeDispatcher().metalakeExists(identifier), + exceptionToThrowSupplier); + break; + + case CATALOG: + check( + GravitinoEnv.getInstance().catalogDispatcher().catalogExists(identifier), + exceptionToThrowSupplier); + break; + + case SCHEMA: + check( + GravitinoEnv.getInstance().schemaDispatcher().schemaExists(identifier), + exceptionToThrowSupplier); + break; + + case FILESET: + check( + GravitinoEnv.getInstance().filesetDispatcher().filesetExists(identifier), + exceptionToThrowSupplier); + break; + + case TABLE: + check( + GravitinoEnv.getInstance().tableDispatcher().tableExists(identifier), + exceptionToThrowSupplier); + break; + + case TOPIC: + check( + GravitinoEnv.getInstance().topicDispatcher().topicExists(identifier), + exceptionToThrowSupplier); + break; + + default: + throw new IllegalArgumentException( + String.format("Doesn't support the type %s", object.type())); + } + } + + public static void checkPrivilege( + PrivilegeDTO privilegeDTO, MetadataObject object, String metalake) { + Privilege privilege = DTOConverters.fromPrivilegeDTO(privilegeDTO); + if (!privilege.canBindTo(object.type())) { + throw new IllegalArgumentException( + String.format( + "Securable object %s type %s don't support binding privilege %s", + object.fullName(), object.type(), privilege)); + } + + if (object.type() == MetadataObject.Type.CATALOG + || object.type() == MetadataObject.Type.SCHEMA) { + NameIdentifier identifier = MetadataObjectUtil.toEntityIdent(metalake, object); + NameIdentifier catalogIdent = NameIdentifierUtil.getCatalogIdentifier(identifier); + try { + if (FILESET_PRIVILEGES.contains(privilege.name())) { + checkCatalogType(catalogIdent, Catalog.Type.FILESET, privilege); + } + + if (TABLE_PRIVILEGES.contains(privilege.name())) { + checkCatalogType(catalogIdent, Catalog.Type.RELATIONAL, privilege); + } + + if (TOPIC_PRIVILEGES.contains(privilege.name())) { + checkCatalogType(catalogIdent, Catalog.Type.MESSAGING, privilege); + } + } catch (NoSuchCatalogException ne) { + throw new NoSuchMetadataObjectException( + "Securable object %s doesn't exist", object.fullName()); + } + } + } + + private static void check( + final boolean expression, Supplier exceptionToThrowSupplier) { + if (!expression) { + throw checkNotNull(exceptionToThrowSupplier).get(); + } + } + + private static void checkCatalogType( + NameIdentifier catalogIdent, Catalog.Type type, Privilege privilege) { + Catalog catalog = GravitinoEnv.getInstance().catalogDispatcher().loadCatalog(catalogIdent); + if (catalog.type() != type) { + throw new IllegalArgumentException( + String.format( + "Catalog %s type %s don't support privilege %s", + catalogIdent, catalog.type(), privilege)); + } + } + private static boolean needApplyAuthorizationPluginAllCatalogs(MetadataObject.Type type) { return type == MetadataObject.Type.METALAKE; } diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java index b006471e383..9810ad759e3 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java @@ -20,8 +20,10 @@ import com.codahale.metrics.annotation.ResponseMetered; import com.codahale.metrics.annotation.Timed; +import com.google.common.collect.Sets; import java.util.Arrays; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; @@ -34,20 +36,20 @@ import javax.ws.rs.core.Response; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.authorization.AccessControlDispatcher; import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Privilege; -import org.apache.gravitino.authorization.Privileges; import org.apache.gravitino.authorization.SecurableObject; import org.apache.gravitino.authorization.SecurableObjects; +import org.apache.gravitino.dto.authorization.PrivilegeDTO; import org.apache.gravitino.dto.authorization.SecurableObjectDTO; import org.apache.gravitino.dto.requests.RoleCreateRequest; import org.apache.gravitino.dto.responses.DeleteResponse; import org.apache.gravitino.dto.responses.NameListResponse; import org.apache.gravitino.dto.responses.RoleResponse; import org.apache.gravitino.dto.util.DTOConverters; -import org.apache.gravitino.exceptions.NoSuchMetadataObjectException; import org.apache.gravitino.lock.LockType; import org.apache.gravitino.lock.TreeLockUtils; import org.apache.gravitino.metrics.MetricNames; @@ -118,12 +120,29 @@ public Response getRole(@PathParam("metalake") String metalake, @PathParam("role @ResponseMetered(name = "create-role", absolute = true) public Response createRole(@PathParam("metalake") String metalake, RoleCreateRequest request) { try { + return Utils.doAs( httpRequest, () -> { + Set metadataObjects = Sets.newHashSet(); for (SecurableObjectDTO object : request.getSecurableObjects()) { - checkSecurableObject(metalake, object); + MetadataObject metadataObject = + MetadataObjects.parse(object.getFullName(), object.type()); + if (metadataObjects.contains(metadataObject)) { + throw new IllegalArgumentException( + String.format( + "Doesn't support specifying duplicated securable objects %s type %s", + object.fullName(), object.type())); + } else { + metadataObjects.add(metadataObject); + } + + for (Privilege privilege : object.privileges()) { + AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege, object, metalake); + } + AuthorizationUtils.checkSecurableObject(metalake, object); } + List securableObjects = Arrays.stream(request.getSecurableObjects()) .map( @@ -133,15 +152,9 @@ public Response createRole(@PathParam("metalake") String metalake, RoleCreateReq securableObjectDTO.type(), securableObjectDTO.privileges().stream() .map( - privilege -> { - if (privilege - .condition() - .equals(Privilege.Condition.ALLOW)) { - return Privileges.allow(privilege.name()); - } else { - return Privileges.deny(privilege.name()); - } - }) + privilege -> + DTOConverters.fromPrivilegeDTO( + (PrivilegeDTO) privilege)) .collect(Collectors.toList()))) .collect(Collectors.toList()); @@ -190,65 +203,4 @@ public Response deleteRole( return ExceptionHandlers.handleRoleException(OperationType.DELETE, role, metalake, e); } } - - // Check every securable object whether exists and is imported. - static void checkSecurableObject(String metalake, SecurableObjectDTO object) { - NameIdentifier identifier; - - // Securable object ignores the metalake namespace, so we should add it back. - if (object.type() == MetadataObject.Type.METALAKE) { - identifier = NameIdentifier.parse(object.fullName()); - } else { - identifier = NameIdentifier.parse(String.format("%s.%s", metalake, object.fullName())); - } - - String existErrMsg = "Securable object %s doesn't exist"; - - switch (object.type()) { - case METALAKE: - if (!GravitinoEnv.getInstance().metalakeDispatcher().metalakeExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - - case CATALOG: - if (!GravitinoEnv.getInstance().catalogDispatcher().catalogExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - - case SCHEMA: - if (!GravitinoEnv.getInstance().schemaDispatcher().schemaExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - - case FILESET: - if (!GravitinoEnv.getInstance().filesetDispatcher().filesetExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - case TABLE: - if (!GravitinoEnv.getInstance().tableDispatcher().tableExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - - case TOPIC: - if (!GravitinoEnv.getInstance().topicDispatcher().topicExists(identifier)) { - throw new NoSuchMetadataObjectException(existErrMsg, object.fullName()); - } - - break; - - default: - throw new IllegalArgumentException( - String.format("Doesn't support the type %s", object.type())); - } - } } diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java index a2f0c4847d6..5767464894a 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java @@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; import com.google.common.collect.Lists; @@ -39,6 +40,7 @@ import org.apache.gravitino.Config; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.authorization.AccessControlManager; +import org.apache.gravitino.authorization.AuthorizationUtils; import org.apache.gravitino.authorization.Privileges; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; @@ -257,6 +259,58 @@ public void testCreateRole() { ErrorResponse errorResponse2 = resp3.readEntity(ErrorResponse.class); Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE, errorResponse2.getCode()); Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResponse2.getType()); + + // Test with wrong binding privileges + SecurableObject wrongPrivilegeObject = + SecurableObjects.ofCatalog("wrong", Lists.newArrayList(Privileges.CreateCatalog.allow())); + RoleCreateRequest wrongPriRequest = + new RoleCreateRequest( + "role", + Collections.emptyMap(), + new SecurableObjectDTO[] {DTOConverters.toDTO(wrongPrivilegeObject)}); + + Response wrongPrivilegeResp = + target("/metalakes/metalake1/roles") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(wrongPriRequest, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals( + Response.Status.BAD_REQUEST.getStatusCode(), wrongPrivilegeResp.getStatus()); + + ErrorResponse wrongPriErrorResp = wrongPrivilegeResp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, wrongPriErrorResp.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), wrongPriErrorResp.getType()); + + // Test with empty securable objects request + RoleCreateRequest emptyObjectRequest = + new RoleCreateRequest("role", Collections.emptyMap(), new SecurableObjectDTO[] {}); + + Role emptyObjectRole = + RoleEntity.builder() + .withId(1L) + .withName("empty") + .withProperties(Collections.emptyMap()) + .withSecurableObjects(Collections.emptyList()) + .withAuditInfo( + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + reset(manager); + when(manager.createRole(any(), any(), any(), any())).thenReturn(emptyObjectRole); + + Response emptyObjectResp = + target("/metalakes/metalake1/roles") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(emptyObjectRequest, MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals(Response.Status.OK.getStatusCode(), emptyObjectResp.getStatus()); + Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE, emptyObjectResp.getMediaType()); + + RoleResponse emptyObjectResponse = emptyObjectResp.readEntity(RoleResponse.class); + Assertions.assertEquals(0, emptyObjectResponse.getCode()); + Role emptyRoleDTO = emptyObjectResponse.getRole(); + Assertions.assertEquals(emptyRoleDTO.name(), "empty"); } @Test @@ -384,11 +438,11 @@ public void testCheckSecurableObjects() { SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.UseCatalog.allow())); when(catalogDispatcher.catalogExists(any())).thenReturn(true); Assertions.assertDoesNotThrow( - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); when(catalogDispatcher.catalogExists(any())).thenReturn(false); Assertions.assertThrows( NoSuchMetadataObjectException.class, - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(catalog))); // check the schema SecurableObject schema = @@ -396,11 +450,11 @@ public void testCheckSecurableObjects() { catalog, "schema", Lists.newArrayList(Privileges.UseSchema.allow())); when(schemaDispatcher.schemaExists(any())).thenReturn(true); Assertions.assertDoesNotThrow( - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); when(schemaDispatcher.schemaExists(any())).thenReturn(false); Assertions.assertThrows( NoSuchMetadataObjectException.class, - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(schema))); // check the table SecurableObject table = @@ -408,11 +462,11 @@ public void testCheckSecurableObjects() { schema, "table", Lists.newArrayList(Privileges.SelectTable.allow())); when(tableDispatcher.tableExists(any())).thenReturn(true); Assertions.assertDoesNotThrow( - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(table))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(table))); when(tableDispatcher.tableExists(any())).thenReturn(false); Assertions.assertThrows( NoSuchMetadataObjectException.class, - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(table))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(table))); // check the topic SecurableObject topic = @@ -420,11 +474,11 @@ public void testCheckSecurableObjects() { schema, "topic", Lists.newArrayList(Privileges.ConsumeTopic.allow())); when(topicDispatcher.topicExists(any())).thenReturn(true); Assertions.assertDoesNotThrow( - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); when(topicDispatcher.topicExists(any())).thenReturn(false); Assertions.assertThrows( NoSuchMetadataObjectException.class, - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(topic))); // check the fileset SecurableObject fileset = @@ -432,11 +486,11 @@ public void testCheckSecurableObjects() { schema, "fileset", Lists.newArrayList(Privileges.ReadFileset.allow())); when(filesetDispatcher.filesetExists(any())).thenReturn(true); Assertions.assertDoesNotThrow( - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); when(filesetDispatcher.filesetExists(any())).thenReturn(false); Assertions.assertThrows( NoSuchMetadataObjectException.class, - () -> RoleOperations.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); + () -> AuthorizationUtils.checkSecurableObject("metalake", DTOConverters.toDTO(fileset))); } @Test