diff --git a/api/src/main/java/com/datastrato/gravitino/authorization/Privilege.java b/api/src/main/java/com/datastrato/gravitino/authorization/Privilege.java index e33bd19eb70..3c5c9484044 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/Privilege.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/Privilege.java @@ -19,6 +19,12 @@ public interface Privilege { /** @return A readable string representation for the privilege. */ String simpleString(); + /** + * @return The effect of the privilege. `ALLOW` means that you can use the privilege, `DENY` means + * that you can't use the privilege + */ + Effect effect(); + /** The name of this privilege. */ enum Name { /** The privilege to create a catalog. */ @@ -116,4 +122,9 @@ public long getHighBits() { return highBits; } } + + enum Effect { + ALLOW, + DENY + } } diff --git a/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java b/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java index f91e83e33a8..803ea481b57 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java @@ -48,9 +48,9 @@ public class Privileges { * @param privilege The string representation of the privilege. * @return The Privilege. */ - public static Privilege fromString(String privilege) { + public static Privilege allowPrivilegeFromString(String privilege) { Privilege.Name name = Privilege.Name.valueOf(privilege); - return fromName(name); + return allowPrivilegeFromName(name); } /** @@ -59,93 +59,191 @@ public static Privilege fromString(String privilege) { * @param name The `Privilege.Name` of the privilege. * @return The Privilege. */ - public static Privilege fromName(Privilege.Name name) { + public static Privilege allowPrivilegeFromName(Privilege.Name name) { switch (name) { // Catalog case CREATE_CATALOG: - return CreateCatalog.get(); + return CreateCatalog.allow(); case DROP_CATALOG: - return DropCatalog.get(); + return DropCatalog.allow(); case ALTER_CATALOG: - return AlterCatalog.get(); + return AlterCatalog.allow(); case USE_CATALOG: - return UseCatalog.get(); + return UseCatalog.allow(); // Schema case CREATE_SCHEMA: - return CreateSchema.get(); + return CreateSchema.allow(); case DROP_SCHEMA: - return DropSchema.get(); + return DropSchema.allow(); case ALTER_SCHEMA: - return AlterSchema.get(); + return AlterSchema.allow(); case USE_SCHEMA: - return UseSchema.get(); + return UseSchema.allow(); // Table case CREATE_TABLE: - return CreateTable.get(); + return CreateTable.allow(); case DROP_TABLE: - return DropTable.get(); + return DropTable.allow(); case WRITE_TABLE: - return WriteTable.get(); + return WriteTable.allow(); case READ_TABLE: - return ReadTable.get(); + return ReadTable.allow(); // Fileset case CREATE_FILESET: - return CreateFileset.get(); + return CreateFileset.allow(); case DROP_FILESET: - return DropFileset.get(); + return DropFileset.allow(); case WRITE_FILESET: - return WriteFileset.get(); + return WriteFileset.allow(); case READ_FILESET: - return ReadFileset.get(); + return ReadFileset.allow(); // Topic case CREATE_TOPIC: - return CreateTopic.get(); + return CreateTopic.allow(); case DROP_TOPIC: - return DropTopic.get(); + return DropTopic.allow(); case WRITE_TOPIC: - return WriteTopic.get(); + return WriteTopic.allow(); case READ_TOPIC: - return ReadTopic.get(); + return ReadTopic.allow(); // Metalake case CREATE_METALAKE: - return CreateMetalake.get(); + return CreateMetalake.allow(); case MANAGE_METALAKE: - return ManageMetalake.get(); + return ManageMetalake.allow(); case USE_METALAKE: - return UseMetalake.get(); + return UseMetalake.allow(); // User case ADD_USER: - return AddUser.get(); + return AddUser.allow(); case REMOVE_USER: - return RemoveUser.get(); + return RemoveUser.allow(); case GET_USER: - return GetUser.get(); + return GetUser.allow(); // Group case ADD_GROUP: - return AddGroup.get(); + return AddGroup.allow(); case REMOVE_GROUP: - return RemoveGroup.get(); + return RemoveGroup.allow(); case GET_GROUP: - return GetGroup.get(); + return GetGroup.allow(); // Role case CREATE_ROLE: - return CreateRole.get(); + return CreateRole.allow(); case DELETE_ROLE: - return DeleteRole.get(); + return DeleteRole.allow(); case GRANT_ROLE: - return GrantRole.get(); + return GrantRole.allow(); case REVOKE_ROLE: - return RevokeRole.get(); + return RevokeRole.allow(); case GET_ROLE: - return GetRole.get(); + return GetRole.allow(); + + default: + throw new IllegalArgumentException("Don't support the privilege: " + name); + } + } + + public static Privilege denyPrivilegeFromString(String privilege) { + Privilege.Name name = Privilege.Name.valueOf(privilege); + return denyPrivilegeFromName(name); + } + + public static Privilege denyPrivilegeFromName(Privilege.Name name) { + switch (name) { + // Catalog + case CREATE_CATALOG: + return CreateCatalog.deny(); + case DROP_CATALOG: + return DropCatalog.deny(); + case ALTER_CATALOG: + return AlterCatalog.deny(); + case USE_CATALOG: + return UseCatalog.deny(); + + // Schema + case CREATE_SCHEMA: + return CreateSchema.deny(); + case DROP_SCHEMA: + return DropSchema.deny(); + case ALTER_SCHEMA: + return AlterSchema.deny(); + case USE_SCHEMA: + return UseSchema.deny(); + + // Table + case CREATE_TABLE: + return CreateTable.deny(); + case DROP_TABLE: + return DropTable.deny(); + case WRITE_TABLE: + return WriteTable.deny(); + case READ_TABLE: + return ReadTable.deny(); + + // Fileset + case CREATE_FILESET: + return CreateFileset.deny(); + case DROP_FILESET: + return DropFileset.deny(); + case WRITE_FILESET: + return WriteFileset.deny(); + case READ_FILESET: + return ReadFileset.deny(); + + // Topic + case CREATE_TOPIC: + return CreateTopic.deny(); + case DROP_TOPIC: + return DropTopic.deny(); + case WRITE_TOPIC: + return WriteTopic.deny(); + case READ_TOPIC: + return ReadTopic.deny(); + + // Metalake + case CREATE_METALAKE: + return CreateMetalake.deny(); + case MANAGE_METALAKE: + return ManageMetalake.deny(); + case USE_METALAKE: + return UseMetalake.deny(); + + // User + case ADD_USER: + return AddUser.deny(); + case REMOVE_USER: + return RemoveUser.deny(); + case GET_USER: + return GetUser.deny(); + + // Group + case ADD_GROUP: + return AddGroup.deny(); + case REMOVE_GROUP: + return RemoveGroup.deny(); + case GET_GROUP: + return GetGroup.deny(); + + // Role + case CREATE_ROLE: + return CreateRole.deny(); + case DELETE_ROLE: + return DeleteRole.deny(); + case GRANT_ROLE: + return GrantRole.deny(); + case REVOKE_ROLE: + return RevokeRole.deny(); + case GET_ROLE: + return GetRole.deny(); default: throw new IllegalArgumentException("Don't support the privilege: " + name); @@ -153,15 +251,32 @@ public static Privilege fromName(Privilege.Name name) { } /** The privilege to create a catalog. */ - public static class CreateCatalog implements Privilege { + public abstract static class CreateCatalog implements Privilege { - private static final CreateCatalog INSTANCE = new CreateCatalog(); + private static final CreateCatalog ALLOW_INSTANCE = + new CreateCatalog() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; - private CreateCatalog() {} + private static final CreateCatalog DENY_INSTANCE = + new CreateCatalog() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; - /** @return The instance of the privilege. */ - public static CreateCatalog get() { - return INSTANCE; + /** @return The instance with allow effect of the privilege. */ + public static CreateCatalog allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny effect of the privilege. */ + public static CreateCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -173,20 +288,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create catalog"; + return effect().name() + " create catalog"; } } /** The privilege to alter a catalog. */ - public static class AlterCatalog implements Privilege { + public abstract static class AlterCatalog implements Privilege { - private static final AlterCatalog INSTANCE = new AlterCatalog(); + private static final AlterCatalog ALLOW_INSTANCE = + new AlterCatalog() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; - private AlterCatalog() {} + private static final AlterCatalog DENY_INSTANCE = + new AlterCatalog() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; - /** @return The instance of the privilege. */ - public static AlterCatalog get() { - return INSTANCE; + /** @return The instance with allow effect of the privilege. */ + public static AlterCatalog allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny effect of the privilege. */ + public static AlterCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -198,20 +330,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "alter catalog"; + return effect().name() + " alter catalog"; } } /** The privilege to drop a catalog. */ - public static class DropCatalog implements Privilege { + public abstract static class DropCatalog implements Privilege { - private static final DropCatalog INSTANCE = new DropCatalog(); + private static final DropCatalog ALLOW_INSTANCE = + new DropCatalog() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; - private DropCatalog() {} + private static final DropCatalog DENY_INSTANCE = + new DropCatalog() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; - /** @return The instance of the privilege. */ - public static DropCatalog get() { - return INSTANCE; + /** @return The instance with allow effect of the privilege. */ + public static DropCatalog allow() { + return ALLOW_INSTANCE; + } + + public static DropCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -223,20 +371,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop catalog"; + return effect().name() + " drop catalog"; } } /** The privilege to use a catalog. */ - public static class UseCatalog implements Privilege { - private static final UseCatalog INSTANCE = new UseCatalog(); + public abstract static class UseCatalog implements Privilege { + private static final UseCatalog ALLOW_INSTANCE = + new UseCatalog() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final UseCatalog DENY_INSTANCE = + new UseCatalog() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static UseCatalog get() { - return INSTANCE; + public static UseCatalog allow() { + return ALLOW_INSTANCE; } - private UseCatalog() {} + public static UseCatalog deny() { + return DENY_INSTANCE; + } /** @return The generic name of the privilege. */ @Override @@ -247,18 +411,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use catalog"; + return effect().name() + " use catalog"; } } /** The privilege to use a schema. */ - public static class UseSchema implements Privilege { - - private static final UseSchema INSTANCE = new UseSchema(); + public abstract static class UseSchema implements Privilege { + + private static final UseSchema ALLOW_INSTANCE = + new UseSchema() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final UseSchema DENY_INSTANCE = + new UseSchema() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static UseSchema get() { - return INSTANCE; + public static UseSchema allow() { + return ALLOW_INSTANCE; + } + + public static UseSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -270,18 +452,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use schema"; + return effect().name() + " use schema"; } } /** The privilege to create a schema. */ - public static class CreateSchema implements Privilege { - - private static final CreateSchema INSTANCE = new CreateSchema(); + public abstract static class CreateSchema implements Privilege { + + private static final CreateSchema ALLOW_INSTANCE = + new CreateSchema() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateSchema DENY_INSTANCE = + new CreateSchema() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static CreateSchema get() { - return INSTANCE; + public static CreateSchema allow() { + return ALLOW_INSTANCE; + } + + public static CreateSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -293,18 +493,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create schema"; + return effect().name() + " create schema"; } } /** The privilege to alter a schema. */ - public static class AlterSchema implements Privilege { - - private static final AlterSchema INSTANCE = new AlterSchema(); + public abstract static class AlterSchema implements Privilege { + + private static final AlterSchema ALLOW_INSTANCE = + new AlterSchema() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final AlterSchema DENY_INSTANCE = + new AlterSchema() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static AlterSchema get() { - return INSTANCE; + public static AlterSchema allow() { + return ALLOW_INSTANCE; + } + + public static AlterSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -316,18 +534,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "alter schema"; + return effect().name() + " alter schema"; } } /** The privilege to drop a schema. */ - public static class DropSchema implements Privilege { - - private static final DropSchema INSTANCE = new DropSchema(); + public abstract static class DropSchema implements Privilege { + + private static final DropSchema ALLOW_INSTANCE = + new DropSchema() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final DropSchema DENY_INSTANCE = + new DropSchema() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static DropSchema get() { - return INSTANCE; + public static DropSchema allow() { + return ALLOW_INSTANCE; + } + + public static DropSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -339,20 +575,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop schema"; + return effect().name() + " drop schema"; } } /** The privilege to create a table. */ - public static class CreateTable implements Privilege { - - private static final CreateTable INSTANCE = new CreateTable(); + public abstract static class CreateTable implements Privilege { + + private static final CreateTable ALLOW_INSTANCE = + new CreateTable() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateTable DENY_INSTANCE = + new CreateTable() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private CreateTable() {} /** @return The instance of the privilege. */ - public static CreateTable get() { - return INSTANCE; + public static CreateTable allow() { + return ALLOW_INSTANCE; + } + + public static CreateTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -364,20 +618,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create table"; + return effect().name() + " create table"; } } /** The privilege to drop a table. */ - public static class DropTable implements Privilege { - - private static final DropTable INSTANCE = new DropTable(); - - private DropTable() {} + public abstract static class DropTable implements Privilege { + + private static final DropTable ALLOW_INSTANCE = + new DropTable() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final DropTable DENY_INSTANCE = + new DropTable() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static DropTable get() { - return INSTANCE; + public static DropTable allow() { + return ALLOW_INSTANCE; + } + + public static DropTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -389,20 +659,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop table"; + return effect().name() + " drop table"; } } /** The privilege to read a table. */ - public static class ReadTable implements Privilege { - - private static final ReadTable INSTANCE = new ReadTable(); - - private ReadTable() {} + public abstract static class ReadTable implements Privilege { + + private static final ReadTable ALLOW_INSTANCE = + new ReadTable() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final ReadTable DENY_INSTANCE = + new ReadTable() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static ReadTable get() { - return INSTANCE; + public static ReadTable allow() { + return ALLOW_INSTANCE; + } + + public static ReadTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -414,20 +700,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read table"; + return effect().name() + " read table"; } } /** The privilege to write a table. */ - public static class WriteTable implements Privilege { - - private static final WriteTable INSTANCE = new WriteTable(); - - private WriteTable() {} + public abstract static class WriteTable implements Privilege { + + private static final WriteTable ALLOW_INSTANCE = + new WriteTable() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final WriteTable DENY_INSTANCE = + new WriteTable() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static WriteTable get() { - return INSTANCE; + public static WriteTable allow() { + return ALLOW_INSTANCE; + } + + public static WriteTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -439,20 +741,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write table"; + return effect().name() + " write table"; } } /** The privilege to create a fileset. */ - public static class CreateFileset implements Privilege { - - private static final CreateFileset INSTANCE = new CreateFileset(); - - private CreateFileset() {} + public abstract static class CreateFileset implements Privilege { + + private static final CreateFileset ALLOW_INSTANCE = + new CreateFileset() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateFileset DENY_INSTANCE = + new CreateFileset() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static CreateFileset get() { - return INSTANCE; + public static CreateFileset allow() { + return ALLOW_INSTANCE; + } + + public static CreateFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -464,20 +782,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create fileset"; + return effect().name() + " create fileset"; } } /** The privilege to drop a fileset. */ - public static class DropFileset implements Privilege { - - private static final DropFileset INSTANCE = new DropFileset(); - - private DropFileset() {} + public abstract static class DropFileset implements Privilege { + + private static final DropFileset ALLOW_INSTANCE = + new DropFileset() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final DropFileset DENY_INSTANCE = + new DropFileset() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static DropFileset get() { - return INSTANCE; + public static DropFileset allow() { + return ALLOW_INSTANCE; + } + + public static DropFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -489,20 +823,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop fileset"; + return effect().name() + " drop fileset"; } } /** The privilege to read a fileset. */ - public static class ReadFileset implements Privilege { - - private static final ReadFileset INSTANCE = new ReadFileset(); - - private ReadFileset() {} + public abstract static class ReadFileset implements Privilege { + + private static final ReadFileset ALLOW_INSTANCE = + new ReadFileset() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final ReadFileset DENY_INSTANCE = + new ReadFileset() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static ReadFileset get() { - return INSTANCE; + public static ReadFileset allow() { + return ALLOW_INSTANCE; + } + + public static ReadFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -514,20 +864,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read fileset"; + return effect().name() + " read fileset"; } } /** The privilege to write a fileset. */ - public static class WriteFileset implements Privilege { - - private static final WriteFileset INSTANCE = new WriteFileset(); - - private WriteFileset() {} + public abstract static class WriteFileset implements Privilege { + + private static final WriteFileset ALLOW_INSTANCE = + new WriteFileset() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final WriteFileset DENY_INSTANCE = + new WriteFileset() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static WriteFileset get() { - return INSTANCE; + public static WriteFileset allow() { + return ALLOW_INSTANCE; + } + + public static WriteFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -539,20 +905,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write fileset"; + return effect().name() + " write fileset"; } } /** The privilege to create a topic. */ - public static class CreateTopic implements Privilege { - - private static final CreateTopic INSTANCE = new CreateTopic(); + public abstract static class CreateTopic implements Privilege { + + private static final CreateTopic ALLOW_INSTANCE = + new CreateTopic() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateTopic DENY_INSTANCE = + new CreateTopic() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private CreateTopic() {} /** @return The instance of the privilege. */ - public static CreateTopic get() { - return INSTANCE; + public static CreateTopic allow() { + return ALLOW_INSTANCE; + } + + public static CreateTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -564,20 +948,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create topic"; + return effect().name() + " create topic"; } } /** The privilege to drop a topic. */ - public static class DropTopic implements Privilege { - - private static final DropTopic INSTANCE = new DropTopic(); - - private DropTopic() {} + public abstract static class DropTopic implements Privilege { + + private static final DropTopic ALLOW_INSTANCE = + new DropTopic() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final DropTopic DENY_INSTANCE = + new DropTopic() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static DropTopic get() { - return INSTANCE; + public static DropTopic allow() { + return ALLOW_INSTANCE; + } + + public static DropTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -589,20 +989,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop topic"; + return effect().name() + " drop topic"; } } /** The privilege to read a topic. */ - public static class ReadTopic implements Privilege { - - private static final ReadTopic INSTANCE = new ReadTopic(); - - private ReadTopic() {} + public abstract static class ReadTopic implements Privilege { + + private static final ReadTopic ALLOW_INSTANCE = + new ReadTopic() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final ReadTopic DENY_INSTANCE = + new ReadTopic() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static ReadTopic get() { - return INSTANCE; + public static ReadTopic allow() { + return ALLOW_INSTANCE; + } + + public static ReadTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -614,20 +1030,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read topic"; + return effect().name() + " read topic"; } } /** The privilege to write a topic. */ - public static class WriteTopic implements Privilege { - - private static final WriteTopic INSTANCE = new WriteTopic(); - - private WriteTopic() {} + public abstract static class WriteTopic implements Privilege { + + private static final WriteTopic ALLOW_INSTANCE = + new WriteTopic() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final WriteTopic DENY_INSTANCE = + new WriteTopic() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static WriteTopic get() { - return INSTANCE; + public static WriteTopic allow() { + return ALLOW_INSTANCE; + } + + public static WriteTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -639,20 +1071,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write topic"; + return effect().name() + " write topic"; } } /** The privilege to manage a metalake. */ - public static class ManageMetalake implements Privilege { - - private static final ManageMetalake INSTANCE = new ManageMetalake(); - - private ManageMetalake() {} + public abstract static class ManageMetalake implements Privilege { + + private static final ManageMetalake ALLOW_INSTANCE = + new ManageMetalake() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final ManageMetalake DENY_INSTANCE = + new ManageMetalake() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static ManageMetalake get() { - return INSTANCE; + public static ManageMetalake allow() { + return ALLOW_INSTANCE; + } + + public static ManageMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -664,20 +1112,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "manage metalake"; + return effect().name() + " manage metalake"; } } /** The privilege to manage a metalake. */ - public static class CreateMetalake implements Privilege { - - private static final CreateMetalake INSTANCE = new CreateMetalake(); - - private CreateMetalake() {} + public abstract static class CreateMetalake implements Privilege { + + private static final CreateMetalake ALLOW_INSTANCE = + new CreateMetalake() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateMetalake DENY_INSTANCE = + new CreateMetalake() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static CreateMetalake get() { - return INSTANCE; + public static CreateMetalake allow() { + return ALLOW_INSTANCE; + } + + public static CreateMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -689,20 +1153,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create metalake"; + return effect().name() + " create metalake"; } } /** The privilege to use a metalake. */ - public static class UseMetalake implements Privilege { - - private static final UseMetalake INSTANCE = new UseMetalake(); + public abstract static class UseMetalake implements Privilege { + + private static final UseMetalake ALLOW_INSTANCE = + new UseMetalake() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final UseMetalake DENY_INSTANCE = + new UseMetalake() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private UseMetalake() {} /** @return The instance of the privilege. */ - public static UseMetalake get() { - return INSTANCE; + public static UseMetalake allow() { + return ALLOW_INSTANCE; + } + + public static UseMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -714,20 +1196,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use metalake"; + return effect().name() + " use metalake"; } } /** The privilege to get a user. */ - public static class GetUser implements Privilege { - - private static final GetUser INSTANCE = new GetUser(); - - private GetUser() {} + public abstract static class GetUser implements Privilege { + + private static final GetUser ALLOW_INSTANCE = + new GetUser() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final GetUser DENY_INSTANCE = + new GetUser() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static GetUser get() { - return INSTANCE; + public static GetUser allow() { + return ALLOW_INSTANCE; + } + + public static GetUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -739,20 +1237,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get user"; + return effect().name() + " get user"; } } /** The privilege to add a user. */ - public static class AddUser implements Privilege { - - private static final AddUser INSTANCE = new AddUser(); + public abstract static class AddUser implements Privilege { + + private static final AddUser ALLOW_INSTANCE = + new AddUser() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final AddUser DENY_INSTANCE = + new AddUser() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private AddUser() {} /** @return The instance of the privilege. */ - public static AddUser get() { - return INSTANCE; + public static AddUser allow() { + return ALLOW_INSTANCE; + } + + public static AddUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -764,20 +1280,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "add user"; + return effect().name() + " add user"; } } /** The privilege to remove a user. */ - public static class RemoveUser implements Privilege { - - private static final RemoveUser INSTANCE = new RemoveUser(); - - private RemoveUser() {} + public abstract static class RemoveUser implements Privilege { + + private static final RemoveUser ALLOW_INSTANCE = + new RemoveUser() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final RemoveUser DENY_INSTANCE = + new RemoveUser() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static RemoveUser get() { - return INSTANCE; + public static RemoveUser allow() { + return ALLOW_INSTANCE; + } + + public static RemoveUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -789,20 +1321,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "remove user"; + return effect().name() + " remove user"; } } /** The privilege to add a group. */ - public static class AddGroup implements Privilege { - - private static final AddGroup INSTANCE = new AddGroup(); + public abstract static class AddGroup implements Privilege { + + private static final AddGroup ALLOW_INSTANCE = + new AddGroup() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final AddGroup DENY_INSTANCE = + new AddGroup() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private AddGroup() {} /** @return The instance of the privilege. */ - public static AddGroup get() { - return INSTANCE; + public static AddGroup allow() { + return ALLOW_INSTANCE; + } + + public static AddGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -814,20 +1364,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "add group"; + return effect().name() + " add group"; } } /** The privilege to remove a group. */ - public static class RemoveGroup implements Privilege { - - private static final RemoveGroup INSTANCE = new RemoveGroup(); + public abstract static class RemoveGroup implements Privilege { + + private static final RemoveGroup ALLOW_INSTANCE = + new RemoveGroup() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final RemoveGroup DENY_INSTANCE = + new RemoveGroup() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private RemoveGroup() {} /** @return The instance of the privilege. */ - public static RemoveGroup get() { - return INSTANCE; + public static RemoveGroup allow() { + return ALLOW_INSTANCE; + } + + public static RemoveGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -839,20 +1407,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "remove group"; + return effect().name() + " remove group"; } } /** The privilege to get a group. */ - public static class GetGroup implements Privilege { - - private static final GetGroup INSTANCE = new GetGroup(); + public abstract static class GetGroup implements Privilege { + + private static final GetGroup ALLOW_INSTANCE = + new GetGroup() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final GetGroup DENY_INSTANCE = + new GetGroup() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private GetGroup() {} /** @return The instance of the privilege. */ - public static GetGroup get() { - return INSTANCE; + public static GetGroup allow() { + return ALLOW_INSTANCE; + } + + public static GetGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -864,20 +1450,36 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get group"; + return effect().name() + " get group"; } } /** The privilege to create a role. */ - public static class CreateRole implements Privilege { - - private static final CreateRole INSTANCE = new CreateRole(); - - private CreateRole() {} + public abstract static class CreateRole implements Privilege { + + private static final CreateRole ALLOW_INSTANCE = + new CreateRole() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final CreateRole DENY_INSTANCE = + new CreateRole() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; /** @return The instance of the privilege. */ - public static CreateRole get() { - return INSTANCE; + public static CreateRole allow() { + return ALLOW_INSTANCE; + } + + public static CreateRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -889,20 +1491,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create role"; + return effect().name() + " create role"; } } /** The privilege to get a role. */ - public static class GetRole implements Privilege { - - private static final GetRole INSTANCE = new GetRole(); + public abstract static class GetRole implements Privilege { + + private static final GetRole ALLOW_INSTANCE = + new GetRole() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final GetRole DENY_INSTANCE = + new GetRole() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private GetRole() {} /** @return The instance of the privilege. */ - public static GetRole get() { - return INSTANCE; + public static GetRole allow() { + return ALLOW_INSTANCE; + } + + public static GetRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -914,20 +1534,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get role"; + return effect().name() + " get role"; } } /** The privilege to delete a role. */ - public static class DeleteRole implements Privilege { - - private static final DeleteRole INSTANCE = new DeleteRole(); + public abstract static class DeleteRole implements Privilege { + + private static final DeleteRole ALLOW_INSTANCE = + new DeleteRole() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final DeleteRole DENY_INSTANCE = + new DeleteRole() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private DeleteRole() {} /** @return The instance of the privilege. */ - public static DeleteRole get() { - return INSTANCE; + public static DeleteRole allow() { + return ALLOW_INSTANCE; + } + + public static DeleteRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -939,20 +1577,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "delete role"; + return effect().name() + " delete role"; } } /** The privilege to grant a role to the user or the group. */ - public static class GrantRole implements Privilege { - - private static final GrantRole INSTANCE = new GrantRole(); + public abstract static class GrantRole implements Privilege { + + private static final GrantRole ALLOW_INSTANCE = + new GrantRole() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final GrantRole DENY_INSTANCE = + new GrantRole() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private GrantRole() {} /** @return The instance of the privilege. */ - public static GrantRole get() { - return INSTANCE; + public static GrantRole allow() { + return ALLOW_INSTANCE; + } + + public static GrantRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -964,20 +1620,38 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "grant role"; + return effect().name() + " grant role"; } } /** The privilege to revoke a role from the user or the group. */ - public static class RevokeRole implements Privilege { - - private static final RevokeRole INSTANCE = new RevokeRole(); + public abstract static class RevokeRole implements Privilege { + + private static final RevokeRole ALLOW_INSTANCE = + new RevokeRole() { + @Override + public Effect effect() { + return Effect.ALLOW; + } + }; + + private static final RevokeRole DENY_INSTANCE = + new RevokeRole() { + @Override + public Effect effect() { + return Effect.DENY; + } + }; private RevokeRole() {} /** @return The instance of the privilege. */ - public static RevokeRole get() { - return INSTANCE; + public static RevokeRole allow() { + return ALLOW_INSTANCE; + } + + public static RevokeRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -989,7 +1663,7 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "revoke role"; + return effect().name() + " revoke role"; } } } diff --git a/api/src/main/java/com/datastrato/gravitino/authorization/Role.java b/api/src/main/java/com/datastrato/gravitino/authorization/Role.java index 914fb7159c6..02591035e4b 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/Role.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/Role.java @@ -31,21 +31,12 @@ public interface Role extends Auditable { */ Map properties(); - /** - * The privileges of the role. All privileges belong to one securable object. For example: If the - * securable object is a table, the privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a - * schema has the privilege of `LOAD TABLE`. It means the role can all tables of the schema. - * - * @return The privileges of the role. - */ - List privileges(); - /** * The securable object represents a special kind of entity with a unique identifier. All * securable objects are organized by tree structure. For example: If the securable object is a * table, the identifier may be `catalog1.schema1.table1`. * - * @return The securable object of the role. + * @return The securable objects of the role. */ - SecurableObject securableObject(); + List securableObjects(); } diff --git a/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObject.java b/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObject.java index e045efa3817..840e2c5d38c 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObject.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObject.java @@ -5,6 +5,7 @@ package com.datastrato.gravitino.authorization; import com.datastrato.gravitino.annotation.Unstable; +import java.util.List; import javax.annotation.Nullable; /** @@ -73,6 +74,17 @@ public interface SecurableObject { */ Type type(); + /** + * The privileges of the role. All privileges belong to one securable object. For example: If the + * securable object is a table, the privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a + * schema has the privilege of `LOAD TABLE`. It means the role can all tables of the schema. + * + * @return The privileges of the role. + */ + List privileges(); + + void bindPrivileges(List privileges); + /** * The type of securable object in the Gravitino system. Every type will map one kind of the * entity of the underlying system. diff --git a/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java b/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java index 2c9d99bde72..a654e0ddf69 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java @@ -4,6 +4,7 @@ */ package com.datastrato.gravitino.authorization; +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -146,6 +147,7 @@ private static class SecurableObjectImpl implements SecurableObject { private final SecurableObject parent; private final String name; private final Type type; + private List privileges; SecurableObjectImpl(SecurableObject parent, String name, Type type) { this.parent = parent; @@ -173,9 +175,21 @@ public Type type() { return type; } + @Override + public List privileges() { + return privileges; + } + + @Override + public void bindPrivileges(List privileges) { + Preconditions.checkArgument( + privileges != null && !privileges.isEmpty(), "Privileges can't be null or empty"); + this.privileges = privileges; + } + @Override public int hashCode() { - return Objects.hash(parent, name, type); + return Objects.hash(parent, name, type, privileges); } @Override @@ -196,7 +210,8 @@ public boolean equals(Object other) { SecurableObject otherSecurableObject = (SecurableObject) other; return Objects.equals(parent, otherSecurableObject.parent()) && Objects.equals(name, otherSecurableObject.name()) - && Objects.equals(type, otherSecurableObject.type()); + && Objects.equals(type, otherSecurableObject.type()) + && Objects.equals(privileges, otherSecurableObject.privileges()); } } diff --git a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoAdminClient.java b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoAdminClient.java index aaa15cf41a9..a772e3a913f 100644 --- a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoAdminClient.java +++ b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoAdminClient.java @@ -9,10 +9,10 @@ import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.SupportsMetalakes; import com.datastrato.gravitino.authorization.Group; -import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Role; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.User; +import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.GroupAddRequest; import com.datastrato.gravitino.dto.requests.MetalakeCreateRequest; import com.datastrato.gravitino.dto.requests.MetalakeUpdateRequest; @@ -42,7 +42,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -430,28 +429,19 @@ public boolean deleteRole(String metalake, String role) throws NoSuchMetalakeExc * @param role The name of the Role. * @param properties The properties of the Role. * @param securableObject The securable object of the Role. - * @param privileges The privileges of the Role. * @return The created Role instance. * @throws RoleAlreadyExistsException If a Role with the same name already exists. * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. * @throws RuntimeException If creating the Role encounters storage issues. */ public Role createRole( - String metalake, - String role, - Map properties, - SecurableObject securableObject, - List privileges) + String metalake, String role, Map properties, SecurableObject securableObject) throws RoleAlreadyExistsException, NoSuchMetalakeException { RoleCreateRequest req = new RoleCreateRequest( role, properties, - privileges.stream() - .map(Privilege::name) - .map(Objects::toString) - .collect(Collectors.toList()), - DTOConverters.toSecurableObject(securableObject)); + new SecurableObjectDTO[] {DTOConverters.toSecurableObject(securableObject)}); req.validate(); RoleResponse resp = diff --git a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRole.java b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRole.java index 97bbcd01833..6d61bb4ae00 100644 --- a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRole.java +++ b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestRole.java @@ -4,6 +4,7 @@ */ package com.datastrato.gravitino.client; +import static com.datastrato.gravitino.dto.util.DTOConverters.toDTO; import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_OK; @@ -14,6 +15,7 @@ import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.dto.AuditDTO; +import com.datastrato.gravitino.dto.authorization.PrivilegeDTO; import com.datastrato.gravitino.dto.authorization.RoleDTO; import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; @@ -49,25 +51,25 @@ public void testCreateRoles() throws Exception { new RoleCreateRequest( roleName, ImmutableMap.of("k1", "v1"), - Lists.newArrayList("USE_CATALOG"), - SecurableObjectDTO.builder() - .withFullName("catalog") - .withType(SecurableObject.Type.CATALOG) - .build()); + new SecurableObjectDTO[] { + SecurableObjectDTO.builder() + .withFullName("catalog") + .withType(SecurableObject.Type.CATALOG) + .withPrivileges(new PrivilegeDTO[] {toDTO(Privileges.UseCatalog.allow())}) + .build() + }); RoleDTO mockRole = mockRoleDTO(roleName); RoleResponse roleResponse = new RoleResponse(mockRole); buildMockResource(Method.POST, rolePath, request, roleResponse, SC_OK); + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + Role createdRole = - client.createRole( - metalakeName, - roleName, - ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get())); - Assertions.assertEquals(1L, Privileges.CreateCatalog.get().name().getLowBits()); - Assertions.assertEquals(0L, Privileges.CreateCatalog.get().name().getHighBits()); + client.createRole(metalakeName, roleName, ImmutableMap.of("k1", "v1"), securableObject); + Assertions.assertEquals(1L, Privileges.CreateCatalog.allow().name().getLowBits()); + Assertions.assertEquals(0L, Privileges.CreateCatalog.allow().name().getHighBits()); Assertions.assertNotNull(createdRole); assertRole(createdRole, mockRole); @@ -81,11 +83,7 @@ public void testCreateRoles() throws Exception { RoleAlreadyExistsException.class, () -> client.createRole( - metalakeName, - roleName, - ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()))); + metalakeName, roleName, ImmutableMap.of("k1", "v1"), securableObject)); Assertions.assertEquals("role already exists", ex.getMessage()); // test NoSuchMetalakeException @@ -97,11 +95,7 @@ public void testCreateRoles() throws Exception { NoSuchMetalakeException.class, () -> client.createRole( - metalakeName, - roleName, - ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()))); + metalakeName, roleName, ImmutableMap.of("k1", "v1"), securableObject)); Assertions.assertEquals("metalake not found", ex.getMessage()); // test RuntimeException @@ -110,12 +104,7 @@ public void testCreateRoles() throws Exception { Assertions.assertThrows( RuntimeException.class, () -> - client.createRole( - metalakeName, - roleName, - ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get())), + client.createRole(metalakeName, roleName, ImmutableMap.of("k1", "v1"), securableObject), "internal error"); } @@ -158,12 +147,14 @@ public void testGetRoles() throws Exception { // test SecurableDTO use parent method Role testParentRole = mockHasParentRoleDTO("test"); - Assertions.assertEquals("schema", testParentRole.securableObject().name()); - Assertions.assertEquals(SecurableObject.Type.SCHEMA, testParentRole.securableObject().type()); - Assertions.assertEquals("catalog", testParentRole.securableObject().parent().fullName()); - Assertions.assertEquals("catalog", testParentRole.securableObject().parent().name()); + Assertions.assertEquals("schema", testParentRole.securableObjects().get(0).name()); + Assertions.assertEquals( + SecurableObject.Type.SCHEMA, testParentRole.securableObjects().get(0).type()); Assertions.assertEquals( - SecurableObject.Type.CATALOG, testParentRole.securableObject().parent().type()); + "catalog", testParentRole.securableObjects().get(0).parent().fullName()); + Assertions.assertEquals("catalog", testParentRole.securableObjects().get(0).parent().name()); + Assertions.assertEquals( + SecurableObject.Type.CATALOG, testParentRole.securableObjects().get(0).parent().type()); } @Test @@ -188,31 +179,36 @@ public void testDeleteRoles() throws Exception { } private RoleDTO mockRoleDTO(String name) { + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + return RoleDTO.builder() .withName(name) .withProperties(ImmutableMap.of("k1", "v1")) - .withSecurableObject(DTOConverters.toSecurableObject(SecurableObjects.ofCatalog("catalog"))) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObjects( + new SecurableObjectDTO[] {DTOConverters.toSecurableObject(securableObject)}) .withAudit(AuditDTO.builder().withCreator("creator").withCreateTime(Instant.now()).build()) .build(); } private RoleDTO mockHasParentRoleDTO(String name) { SecurableObject catalog = SecurableObjects.ofCatalog("catalog"); + SecurableObject schema = SecurableObjects.ofSchema(catalog, "schema"); + schema.bindPrivileges(Lists.newArrayList(Privileges.UseSchema.allow())); return RoleDTO.builder() .withName(name) .withProperties(ImmutableMap.of("k1", "v1")) - .withSecurableObject( - DTOConverters.toSecurableObject(SecurableObjects.ofSchema(catalog, "schema"))) - .withPrivileges(Lists.newArrayList(Privileges.UseSchema.get())) + .withSecurableObjects(new SecurableObjectDTO[] {DTOConverters.toSecurableObject(schema)}) .withAudit(AuditDTO.builder().withCreator("creator").withCreateTime(Instant.now()).build()) .build(); } private void assertRole(Role expected, Role actual) { Assertions.assertEquals(expected.name(), actual.name()); - Assertions.assertEquals(expected.privileges(), actual.privileges()); Assertions.assertEquals( - expected.securableObject().fullName(), actual.securableObject().fullName()); + expected.securableObjects().get(0).privileges(), + actual.securableObjects().get(0).privileges()); + Assertions.assertEquals( + expected.securableObjects().get(0).fullName(), actual.securableObjects().get(0).fullName()); } } diff --git a/common/src/main/java/com/datastrato/gravitino/dto/authorization/PrivilegeDTO.java b/common/src/main/java/com/datastrato/gravitino/dto/authorization/PrivilegeDTO.java new file mode 100644 index 00000000000..4974864222d --- /dev/null +++ b/common/src/main/java/com/datastrato/gravitino/dto/authorization/PrivilegeDTO.java @@ -0,0 +1,68 @@ +/* + * Copyright 2024 Datastrato Pvt Ltd. + * This software is licensed under the Apache License version 2. + */ +package com.datastrato.gravitino.dto.authorization; + +import com.datastrato.gravitino.authorization.Privilege; +import com.datastrato.gravitino.authorization.Privileges; +import com.fasterxml.jackson.annotation.JsonProperty; + +public class PrivilegeDTO implements Privilege { + + @JsonProperty("name") + private Name name; + + @JsonProperty("effect") + private Effect effect; + + /** Default constructor for Jackson deserialization. */ + protected PrivilegeDTO() {} + + protected PrivilegeDTO(Name name, Effect effect) {} + + @Override + public Name name() { + return name; + } + + @Override + public String simpleString() { + if (Effect.ALLOW.equals(effect)) { + return Privileges.allowPrivilegeFromName(name).simpleString(); + } else { + return Privileges.denyPrivilegeFromName(name).simpleString(); + } + } + + @Override + public Effect effect() { + return effect; + } + + /** @return the builder for creating a new instance of SecurableObjectDTO. */ + public static Builder builder() { + return new Builder(); + } + + /** Builder for {@link PrivilegeDTO}. */ + public static class Builder { + + private Name name; + private Effect effect; + + public Builder withName(Name name) { + this.name = name; + return this; + } + + public Builder withEffect(Effect effect) { + this.effect = effect; + return this; + } + + public PrivilegeDTO build() { + return new PrivilegeDTO(name, effect); + } + } +} diff --git a/common/src/main/java/com/datastrato/gravitino/dto/authorization/RoleDTO.java b/common/src/main/java/com/datastrato/gravitino/dto/authorization/RoleDTO.java index 6a9c24fa3a1..ec7e793f21a 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/authorization/RoleDTO.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/authorization/RoleDTO.java @@ -5,20 +5,15 @@ package com.datastrato.gravitino.dto.authorization; import com.datastrato.gravitino.Audit; -import com.datastrato.gravitino.authorization.Privilege; -import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.Role; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.dto.AuditDTO; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import java.util.Collections; +import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; import javax.annotation.Nullable; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; /** Represents a Role Data Transfer Object (DTO). */ @@ -34,11 +29,8 @@ public class RoleDTO implements Role { @JsonProperty("properties") private Map properties; - @JsonProperty("privileges") - private List privileges; - - @JsonProperty("securableObject") - private SecurableObjectDTO securableObject; + @JsonProperty("securableObjects") + private SecurableObjectDTO[] securableObjects; /** Default constructor for Jackson deserialization. */ protected RoleDTO() {} @@ -48,21 +40,18 @@ protected RoleDTO() {} * * @param name The name of the Role DTO. * @param properties The properties of the Role DTO. - * @param privileges The privileges of the Role DTO. - * @param securableObject The securable object of the Role DTO. + * @param securableObjects The securable objects of the Role DTO. * @param audit The audit information of the Role DTO. */ protected RoleDTO( String name, Map properties, - List privileges, - SecurableObjectDTO securableObject, + SecurableObjectDTO[] securableObjects, AuditDTO audit) { this.name = name; this.audit = audit; this.properties = properties; - this.privileges = privileges; - this.securableObject = securableObject; + this.securableObjects = securableObjects; } /** @return The name of the Role DTO. */ @@ -81,18 +70,6 @@ public Map properties() { return properties; } - /** - * The privileges of the role. All privileges belong to one resource. For example: If the resource - * is a table, the privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a schema has the - * privilege of `LOAD TABLE`. It means the role can all tables of the schema. - * - * @return The privileges of the role. - */ - @Override - public List privileges() { - return privileges.stream().map(Privileges::fromString).collect(Collectors.toList()); - } - /** * The resource represents a special kind of entity with a unique identifier. All resources are * organized by tree structure. For example: If the resource is a table, the identifier may be @@ -101,8 +78,8 @@ public List privileges() { * @return The securable object of the role. */ @Override - public SecurableObject securableObject() { - return securableObject; + public List securableObjects() { + return Arrays.asList(securableObjects); } /** @return The audit information of the Role DTO. */ @@ -130,17 +107,14 @@ public static class Builder { /** The name of the role. */ protected String name; - /** The privileges of the role. */ - protected List privileges = Collections.emptyList(); - /** The audit information of the role. */ protected AuditDTO audit; /** The properties of the role. */ protected Map properties; - /** The securable object of the role. */ - protected SecurableObjectDTO securableObject; + /** The securable objects of the role. */ + protected SecurableObjectDTO[] securableObjects; /** * Sets the name of the role. @@ -153,20 +127,6 @@ public S withName(String name) { return (S) this; } - /** - * Sets the privileges of the role. - * - * @param privileges The privileges of the role. - * @return The builder instance. - */ - public S withPrivileges(List privileges) { - if (privileges != null) { - this.privileges = privileges; - } - - return (S) this; - } - /** * Sets the properties of the role. * @@ -182,13 +142,13 @@ public S withProperties(Map properties) { } /** - * Sets the securable object of the role. + * Sets the securable objects of the role. * - * @param securableObject The securableObject of the role. + * @param securableObjects The securableObjects of the role. * @return The builder instance. */ - public S withSecurableObject(SecurableObjectDTO securableObject) { - this.securableObject = securableObject; + public S withSecurableObjects(SecurableObjectDTO[] securableObjects) { + this.securableObjects = securableObjects; return (S) this; } @@ -213,18 +173,10 @@ public RoleDTO build() { Preconditions.checkArgument(StringUtils.isNotBlank(name), "name cannot be null or empty"); Preconditions.checkArgument(audit != null, "audit cannot be null"); Preconditions.checkArgument( - !CollectionUtils.isEmpty(privileges), "privileges can't be empty"); - Preconditions.checkArgument(securableObject != null, "securable object can't null"); - - return new RoleDTO( - name, - properties, - privileges.stream() - .map(Privilege::name) - .map(Objects::toString) - .collect(Collectors.toList()), - securableObject, - audit); + securableObjects != null && securableObjects.length != 0, + "securable objects can't null or empty"); + + return new RoleDTO(name, properties, securableObjects, audit); } } } diff --git a/common/src/main/java/com/datastrato/gravitino/dto/authorization/SecurableObjectDTO.java b/common/src/main/java/com/datastrato/gravitino/dto/authorization/SecurableObjectDTO.java index abde242dd77..fe634b3b747 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/authorization/SecurableObjectDTO.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/authorization/SecurableObjectDTO.java @@ -4,10 +4,13 @@ */ package com.datastrato.gravitino.dto.authorization; +import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; import org.apache.commons.lang3.StringUtils; @@ -20,6 +23,9 @@ public class SecurableObjectDTO implements SecurableObject { @JsonProperty("type") private Type type; + @JsonProperty("privileges") + private PrivilegeDTO[] privileges; + private SecurableObject parent; private String name; @@ -61,6 +67,14 @@ public Type type() { return type; } + @Override + public List privileges() { + return Arrays.asList(privileges); + } + + @Override + public void bindPrivileges(List privileges) {} + /** @return the builder for creating a new instance of SecurableObjectDTO. */ public static Builder builder() { return new Builder(); @@ -70,6 +84,7 @@ public static Builder builder() { public static class Builder { private String fullName; private Type type; + private PrivilegeDTO[] privileges; /** * Sets the full name of the securable object. @@ -93,6 +108,17 @@ public Builder withType(Type type) { return this; } + /** + * Sets the privileges of the securable object. + * + * @param privileges The privileges of the securable object. + * @return The builder instance. + */ + public Builder withPrivileges(PrivilegeDTO[] privileges) { + this.privileges = privileges; + return this; + } + /** * Builds an instance of SecurableObjectDTO using the builder's properties. * @@ -105,7 +131,10 @@ public SecurableObjectDTO build() { Preconditions.checkArgument(type != null, "type cannot be null"); - return new SecurableObjectDTO(fullName, type); + SecurableObjectDTO object = new SecurableObjectDTO(fullName, type); + + object.bindPrivileges(Arrays.asList(privileges)); + return object; } } } diff --git a/common/src/main/java/com/datastrato/gravitino/dto/requests/RoleCreateRequest.java b/common/src/main/java/com/datastrato/gravitino/dto/requests/RoleCreateRequest.java index 89b11de658f..f8ed4d1b1b1 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/requests/RoleCreateRequest.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/requests/RoleCreateRequest.java @@ -8,7 +8,6 @@ import com.datastrato.gravitino.rest.RESTRequest; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; import lombok.Builder; @@ -16,7 +15,6 @@ import lombok.Getter; import lombok.ToString; import lombok.extern.jackson.Jacksonized; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; /** Represents a request to create a role. */ @@ -34,15 +32,12 @@ public class RoleCreateRequest implements RESTRequest { @JsonProperty("properties") private Map properties; - @JsonProperty("privileges") - private List privileges; - - @JsonProperty("securableObject") - private SecurableObjectDTO securableObject; + @JsonProperty("securableObjects") + private SecurableObjectDTO[] securableObjects; /** Default constructor for RoleCreateRequest. (Used for Jackson deserialization.) */ public RoleCreateRequest() { - this(null, null, null, null); + this(null, null, null); } /** @@ -50,19 +45,14 @@ public RoleCreateRequest() { * * @param name The name of the role. * @param properties The properties of the role. - * @param securableObject The securable object of the role. - * @param privileges The privileges of the role. + * @param securableObjects The securable objects of the role. */ public RoleCreateRequest( - String name, - Map properties, - List privileges, - SecurableObjectDTO securableObject) { + String name, Map properties, SecurableObjectDTO[] securableObjects) { super(); this.name = name; this.properties = properties; - this.privileges = privileges; - this.securableObject = securableObject; + this.securableObjects = securableObjects; } /** @@ -76,8 +66,7 @@ public void validate() throws IllegalArgumentException { StringUtils.isNotBlank(name), "\"name\" field is required and cannot be empty"); Preconditions.checkArgument( - !CollectionUtils.isEmpty(privileges), "\"privileges\" can't be empty"); - - Preconditions.checkArgument(securableObject != null, "\"securableObject\" can't null"); + securableObjects != null && securableObjects.length != 0, + "\"securableObjects\" can't null or empty"); } } diff --git a/common/src/main/java/com/datastrato/gravitino/dto/responses/RoleResponse.java b/common/src/main/java/com/datastrato/gravitino/dto/responses/RoleResponse.java index 5c1086bb43c..a948207259f 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/responses/RoleResponse.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/responses/RoleResponse.java @@ -10,7 +10,6 @@ import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; /** Represents a response for a role. */ @@ -52,8 +51,7 @@ public void validate() throws IllegalArgumentException { StringUtils.isNotBlank(role.name()), "role 'name' must not be null and empty"); Preconditions.checkArgument(role.auditInfo() != null, "role 'auditInfo' must not be null"); Preconditions.checkArgument( - !CollectionUtils.isEmpty(role.privileges()), "role 'privileges' can't be empty"); - Preconditions.checkArgument( - role.securableObject() != null, "role 'securableObject' can't null"); + role.securableObjects() != null && !role.securableObjects().isEmpty(), + "role 'securableObjects' can't null or empty"); } } diff --git a/common/src/main/java/com/datastrato/gravitino/dto/util/DTOConverters.java b/common/src/main/java/com/datastrato/gravitino/dto/util/DTOConverters.java index 507d1f63cc0..cb673d9a9af 100644 --- a/common/src/main/java/com/datastrato/gravitino/dto/util/DTOConverters.java +++ b/common/src/main/java/com/datastrato/gravitino/dto/util/DTOConverters.java @@ -10,6 +10,7 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.Metalake; import com.datastrato.gravitino.authorization.Group; +import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Role; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.User; @@ -17,6 +18,7 @@ import com.datastrato.gravitino.dto.CatalogDTO; import com.datastrato.gravitino.dto.MetalakeDTO; import com.datastrato.gravitino.dto.authorization.GroupDTO; +import com.datastrato.gravitino.dto.authorization.PrivilegeDTO; import com.datastrato.gravitino.dto.authorization.RoleDTO; import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.authorization.UserDTO; @@ -396,8 +398,10 @@ public static RoleDTO toDTO(Role role) { return RoleDTO.builder() .withName(role.name()) - .withSecurableObject(toDTO(role.securableObject())) - .withPrivileges(role.privileges()) + .withSecurableObjects( + role.securableObjects().stream() + .map(DTOConverters::toDTO) + .toArray(SecurableObjectDTO[]::new)) .withProperties(role.properties()) .withAudit(toDTO(role.auditInfo())) .build(); @@ -417,9 +421,21 @@ public static SecurableObjectDTO toDTO(SecurableObject securableObject) { return SecurableObjectDTO.builder() .withFullName(securableObject.fullName()) .withType(securableObject.type()) + .withPrivileges( + securableObject.privileges().stream() + .map(DTOConverters::toDTO) + .toArray(PrivilegeDTO[]::new)) .build(); } + public static PrivilegeDTO toDTO(Privilege privilege) { + if (privilege instanceof PrivilegeDTO) { + return (PrivilegeDTO) privilege; + } + + return PrivilegeDTO.builder().withName(privilege.name()).withEffect(privilege.effect()).build(); + } + /** * Converts a Expression to an FunctionArg DTO. * diff --git a/common/src/test/java/com/datastrato/gravitino/dto/responses/TestResponses.java b/common/src/test/java/com/datastrato/gravitino/dto/responses/TestResponses.java index f4515c726b0..440c0adb40c 100644 --- a/common/src/test/java/com/datastrato/gravitino/dto/responses/TestResponses.java +++ b/common/src/test/java/com/datastrato/gravitino/dto/responses/TestResponses.java @@ -12,12 +12,14 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.authorization.Privileges; +import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.dto.AuditDTO; import com.datastrato.gravitino.dto.CatalogDTO; import com.datastrato.gravitino.dto.MetalakeDTO; import com.datastrato.gravitino.dto.authorization.GroupDTO; import com.datastrato.gravitino.dto.authorization.RoleDTO; +import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.authorization.UserDTO; import com.datastrato.gravitino.dto.rel.ColumnDTO; import com.datastrato.gravitino.dto.rel.SchemaDTO; @@ -265,11 +267,12 @@ void testGroupResponseException() throws IllegalArgumentException { void testRoleResponse() throws IllegalArgumentException { AuditDTO audit = AuditDTO.builder().withCreator("creator").withCreateTime(Instant.now()).build(); + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); RoleDTO role = RoleDTO.builder() .withName("role1") - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) - .withSecurableObject(DTOConverters.toDTO(SecurableObjects.ofCatalog("catalog"))) + .withSecurableObjects(new SecurableObjectDTO[] {DTOConverters.toDTO(securableObject)}) .withAudit(audit) .build(); RoleResponse response = new RoleResponse(role); diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java index 5f1843614f6..d8024b924ff 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/AccessControlManager.java @@ -250,21 +250,16 @@ public boolean isMetalakeAdmin(String user) { * @param role The name of the Role. * @param properties The properties of the Role. * @param securableObject The securable object of the Role. - * @param privileges The privileges of the Role. * @return The created Role instance. * @throws RoleAlreadyExistsException If a Role with the same name already exists. * @throws NoSuchMetalakeException If the Metalake with the given name does not exist. * @throws RuntimeException If creating the Role encounters storage issues. */ public Role createRole( - String metalake, - String role, - Map properties, - SecurableObject securableObject, - List privileges) + String metalake, String role, Map properties, SecurableObject securableObject) throws RoleAlreadyExistsException, NoSuchMetalakeException { return doWithNonAdminLock( - () -> roleManager.createRole(metalake, role, properties, securableObject, privileges)); + () -> roleManager.createRole(metalake, role, properties, securableObject)); } /** diff --git a/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java b/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java index 71d72c36e41..654aab9a023 100644 --- a/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java +++ b/core/src/main/java/com/datastrato/gravitino/authorization/RoleManager.java @@ -72,11 +72,7 @@ class RoleManager { } RoleEntity createRole( - String metalake, - String role, - Map properties, - SecurableObject securableObject, - List privileges) + String metalake, String role, Map properties, SecurableObject securableObject) throws RoleAlreadyExistsException { AuthorizationUtils.checkMetalakeExists(metalake); RoleEntity roleEntity = @@ -85,7 +81,6 @@ RoleEntity createRole( .withName(role) .withProperties(properties) .withSecurableObject(securableObject) - .withPrivileges(privileges) .withNamespace(AuthorizationUtils.ofRoleNamespace(metalake)) .withAuditInfo( AuditInfo.builder() diff --git a/core/src/main/java/com/datastrato/gravitino/meta/RoleEntity.java b/core/src/main/java/com/datastrato/gravitino/meta/RoleEntity.java index 8e9ec407f8d..aaa8c15b36b 100644 --- a/core/src/main/java/com/datastrato/gravitino/meta/RoleEntity.java +++ b/core/src/main/java/com/datastrato/gravitino/meta/RoleEntity.java @@ -9,9 +9,9 @@ import com.datastrato.gravitino.Field; import com.datastrato.gravitino.HasIdentifier; import com.datastrato.gravitino.Namespace; -import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Role; import com.datastrato.gravitino.authorization.SecurableObject; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import java.util.Collections; import java.util.List; @@ -36,14 +36,10 @@ public class RoleEntity implements Role, Entity, Auditable, HasIdentifier { Field.required( "securable_object", SecurableObject.class, "The securable object of the role entity."); - public static final Field PRIVILEGES = - Field.required("privileges", List.class, "The privileges of the role entity."); - private Long id; private String name; private Map properties; private AuditInfo auditInfo; - private List privileges; private Namespace namespace; private SecurableObject securableObject; @@ -78,10 +74,10 @@ public Map properties() { * securable objects are organized by tree structure. For example: If the securable object is a * table, the identifier may be `catalog1.schema1.table1`. * - * @return The securable object of the role. + * @return The securable objects of the role. */ @Override - public SecurableObject securableObject() { + public List securableObjects() { // The securable object is a special kind of entities. Some entity types aren't the securable // object, such as // User, Role, etc. @@ -91,19 +87,7 @@ public SecurableObject securableObject() { // So one type of them can't be the securable object at least if there are the two same // identifier // entities . - return securableObject; - } - - /** - * The privileges of the role. All privileges belong to one securable object. For example: If the - * securable object is a table, the privileges could be `READ TABLE`, `WRITE TABLE`, etc. If a - * schema has the privilege of `LOAD TABLE`. It means the role can all tables of the schema. - * - * @return The privileges of the role. - */ - @Override - public List privileges() { - return privileges; + return Lists.newArrayList(securableObject); } /** @@ -119,7 +103,6 @@ public Map fields() { fields.put(AUDIT_INFO, auditInfo); fields.put(PROPERTIES, properties); fields.put(SECURABLE_OBJECT, securableObject); - fields.put(PRIVILEGES, privileges); return Collections.unmodifiableMap(fields); } @@ -155,13 +138,12 @@ public boolean equals(Object o) { && Objects.equals(namespace, that.namespace) && Objects.equals(auditInfo, that.auditInfo) && Objects.equals(properties, that.properties) - && Objects.equals(securableObject, that.securableObject) - && Objects.equals(privileges, that.privileges); + && Objects.equals(securableObject, that.securableObject); } @Override public int hashCode() { - return Objects.hash(id, name, properties, auditInfo, securableObject, privileges); + return Objects.hash(id, name, properties, auditInfo, securableObject); } /** @@ -240,17 +222,6 @@ public Builder withSecurableObject(SecurableObject securableObject) { return this; } - /** - * Sets the privileges of the role entity. - * - * @param privileges The privileges of the role entity. - * @return The builder instance. - */ - public Builder withPrivileges(List privileges) { - roleEntity.privileges = privileges; - return this; - } - /** * Sets the namespace of the role entity. * diff --git a/core/src/main/java/com/datastrato/gravitino/proto/RoleEntitySerDe.java b/core/src/main/java/com/datastrato/gravitino/proto/RoleEntitySerDe.java index 92d7623b13b..cc32e48086b 100644 --- a/core/src/main/java/com/datastrato/gravitino/proto/RoleEntitySerDe.java +++ b/core/src/main/java/com/datastrato/gravitino/proto/RoleEntitySerDe.java @@ -5,10 +5,13 @@ package com.datastrato.gravitino.proto; import com.datastrato.gravitino.Namespace; +import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.meta.RoleEntity; +import com.google.common.collect.Lists; +import java.util.List; import java.util.stream.Collectors; public class RoleEntitySerDe implements ProtoSerDe { @@ -27,11 +30,15 @@ public Role serialize(RoleEntity roleEntity) { .setName(roleEntity.name()) .setAuditInfo(new AuditInfoSerDe().serialize(roleEntity.auditInfo())) .addAllPrivileges( - roleEntity.privileges().stream() + roleEntity.securableObjects().get(0).privileges().stream() .map(privilege -> privilege.name().toString()) .collect(Collectors.toList())) - .setSecurableObjectFullName(roleEntity.securableObject().fullName()) - .setSecurableObjectType(roleEntity.securableObject().type().name()); + .addAllPrivilegeEffects( + roleEntity.securableObjects().get(0).privileges().stream() + .map(privilege -> privilege.effect().toString()) + .collect(Collectors.toList())) + .setSecurableObjectFullName(roleEntity.securableObjects().get(0).fullName()) + .setSecurableObjectType(roleEntity.securableObjects().get(0).type().name()); if (roleEntity.properties() != null && !roleEntity.properties().isEmpty()) { builder.putAllProperties(roleEntity.properties()); @@ -48,19 +55,31 @@ public Role serialize(RoleEntity roleEntity) { */ @Override public RoleEntity deserialize(Role role, Namespace namespace) { + SecurableObject securableObject = + SecurableObjects.parse( + role.getSecurableObjectFullName(), + SecurableObject.Type.valueOf(role.getSecurableObjectType())); + + if (!role.getPrivilegesList().isEmpty()) { + List privileges = Lists.newArrayList(); + + for (int index = 0; index < role.getPrivilegeEffectsCount(); index++) { + if (Privilege.Effect.ALLOW.name().equals(role.getPrivilegeEffects(index))) { + privileges.add(Privileges.allowPrivilegeFromString(role.getPrivileges(index))); + } else { + privileges.add(Privileges.denyPrivilegeFromString(role.getPrivileges(index))); + } + } + + securableObject.bindPrivileges(privileges); + } + RoleEntity.Builder builder = RoleEntity.builder() .withId(role.getId()) .withName(role.getName()) .withNamespace(namespace) - .withPrivileges( - role.getPrivilegesList().stream() - .map(Privileges::fromString) - .collect(Collectors.toList())) - .withSecurableObject( - SecurableObjects.parse( - role.getSecurableObjectFullName(), - SecurableObject.Type.valueOf(role.getSecurableObjectType()))) + .withSecurableObject(securableObject) .withAuditInfo(new AuditInfoSerDe().deserialize(role.getAuditInfo(), namespace)); if (!role.getPropertiesMap().isEmpty()) { diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/RoleMetaMapper.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/RoleMetaMapper.java index b1aa10e375a..7149ab3c231 100644 --- a/core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/RoleMetaMapper.java +++ b/core/src/main/java/com/datastrato/gravitino/storage/relational/mapper/RoleMetaMapper.java @@ -31,6 +31,7 @@ public interface RoleMetaMapper { + " securable_object_full_name as securableObjectFullName," + " securable_object_type as securableObjectType," + " privileges as privileges," + + " privilege_effects as privilegeEffects," + " audit_info as auditInfo, current_version as currentVersion," + " last_version as lastVersion, deleted_at as deletedAt" + " FROM " @@ -54,6 +55,7 @@ Long selectRoleIdByMetalakeIdAndName( + " securable_object_full_name as securableObjectFullName," + " securable_object_type as securableObjectType," + " ro.privileges as privileges," + + " ro.privilege_effects as privilegeEffects," + " ro.audit_info as auditInfo, ro.current_version as currentVersion," + " ro.last_version as lastVersion, ro.deleted_at as deletedAt" + " FROM " @@ -71,6 +73,7 @@ Long selectRoleIdByMetalakeIdAndName( + " ro.securable_object_full_name as securableObjectFullName," + " ro.securable_object_type as securableObjectType," + " ro.privileges as privileges," + + " ro.privilege_effects as privilegeEffects," + " ro.audit_info as auditInfo, ro.current_version as currentVersion," + " ro.last_version as lastVersion, ro.deleted_at as deletedAt" + " FROM " @@ -90,6 +93,7 @@ Long selectRoleIdByMetalakeIdAndName( + " securable_object_full_name," + " securable_object_type," + " privileges," + + " privilege_effects," + " audit_info, current_version, last_version, deleted_at)" + " VALUES(" + " #{roleMeta.roleId}," @@ -99,6 +103,7 @@ Long selectRoleIdByMetalakeIdAndName( + " #{roleMeta.securableObjectFullName}," + " #{roleMeta.securableObjectType}," + " #{roleMeta.privileges}," + + " #{roleMeta.privilegeEffects}," + " #{roleMeta.auditInfo}," + " #{roleMeta.currentVersion}," + " #{roleMeta.lastVersion}," @@ -134,6 +139,7 @@ Long selectRoleIdByMetalakeIdAndName( + " securable_object_full_name = #{roleMeta.securableObjectFullName}," + " securable_object_type = #{roleMeta.securableObjectType}," + " privileges = #{roleMeta.privileges}," + + " privilege_effects = #{roleMeta.privilegeEffects}," + " audit_info = #{roleMeta.auditInfo}," + " current_version = #{roleMeta.currentVersion}," + " last_version = #{roleMeta.lastVersion}," diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/po/RolePO.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/po/RolePO.java index 0da18ff804c..ce052043e19 100644 --- a/core/src/main/java/com/datastrato/gravitino/storage/relational/po/RolePO.java +++ b/core/src/main/java/com/datastrato/gravitino/storage/relational/po/RolePO.java @@ -15,6 +15,7 @@ public class RolePO { private String securableObjectFullName; private String securableObjectType; private String privileges; + private String privilegeEffects; private String auditInfo; private Long currentVersion; private Long lastVersion; @@ -48,6 +49,10 @@ public String getPrivileges() { return privileges; } + public String getPrivilegeEffects() { + return privilegeEffects; + } + public String getAuditInfo() { return auditInfo; } @@ -83,7 +88,8 @@ public boolean equals(Object o) { && Objects.equal(getAuditInfo(), tablePO.getAuditInfo()) && Objects.equal(getCurrentVersion(), tablePO.getCurrentVersion()) && Objects.equal(getLastVersion(), tablePO.getLastVersion()) - && Objects.equal(getDeletedAt(), tablePO.getDeletedAt()); + && Objects.equal(getDeletedAt(), tablePO.getDeletedAt()) + && Objects.equal(getPrivilegeEffects(), tablePO.getPrivilegeEffects()); } @Override @@ -99,7 +105,8 @@ public int hashCode() { getAuditInfo(), getCurrentVersion(), getLastVersion(), - getDeletedAt()); + getDeletedAt(), + getPrivilegeEffects()); } public static class Builder { @@ -144,6 +151,11 @@ public Builder withPrivileges(String privileges) { return this; } + public Builder withPrivilegeEffects(String privilegeEffects) { + rolePO.privilegeEffects = privilegeEffects; + return this; + } + public Builder withAuditInfo(String auditInfo) { rolePO.auditInfo = auditInfo; return this; @@ -173,6 +185,7 @@ private void validate() { Preconditions.checkArgument( rolePO.securableObjectType != null, "Securable object type is required"); Preconditions.checkArgument(rolePO.privileges != null, "Privileges is required"); + Preconditions.checkArgument(rolePO.privilegeEffects != null, "PrivilegeEffects is required"); Preconditions.checkArgument(rolePO.auditInfo != null, "Audit info is required"); Preconditions.checkArgument(rolePO.currentVersion != null, "Current version is required"); Preconditions.checkArgument(rolePO.lastVersion != null, "Last version is required"); diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/POConverters.java index 5c1285075c3..6994dd375fb 100644 --- a/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/POConverters.java @@ -7,6 +7,7 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.Namespace; +import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; @@ -782,14 +783,20 @@ public static RolePO initializeRolePOWithVersion(RoleEntity roleEntity, RolePO.B .withRoleId(roleEntity.id()) .withRoleName(roleEntity.name()) .withProperties(JsonUtils.anyFieldMapper().writeValueAsString(roleEntity.properties())) - .withSecurableObjectFullName(roleEntity.securableObject().fullName()) - .withSecurableObjectType(roleEntity.securableObject().type().name()) + .withSecurableObjectFullName(roleEntity.securableObjects().get(0).fullName()) + .withSecurableObjectType(roleEntity.securableObjects().get(0).type().name()) .withPrivileges( JsonUtils.anyFieldMapper() .writeValueAsString( - roleEntity.privileges().stream() + roleEntity.securableObjects().get(0).privileges().stream() .map(privilege -> privilege.name().toString()) .collect(Collectors.toList()))) + .withPrivilegeEffects( + JsonUtils.anyFieldMapper() + .writeValueAsString( + roleEntity.securableObjects().get(0).privileges().stream() + .map(privilege -> privilege.effect().toString()) + .collect(Collectors.toList()))) .withAuditInfo(JsonUtils.anyFieldMapper().writeValueAsString(roleEntity.auditInfo())) .withCurrentVersion(INIT_VERSION) .withLastVersion(INIT_VERSION) @@ -881,20 +888,34 @@ public static List initializeGroupRoleRelsPOWithVersion( public static RoleEntity fromRolePO(RolePO rolePO, Namespace namespace) { try { + SecurableObject securableObject = + SecurableObjects.parse( + rolePO.getSecurableObjectFullName(), + SecurableObject.Type.valueOf(rolePO.getSecurableObjectType())); + + List privilegeNames = + JsonUtils.anyFieldMapper() + .readValue(rolePO.getPrivileges(), new TypeReference>() {}); + List privilegeEffects = + JsonUtils.anyFieldMapper() + .readValue(rolePO.getPrivilegeEffects(), new TypeReference>() {}); + + List privileges = Lists.newArrayList(); + for (int index = 0; index < privilegeNames.size(); index++) { + if (Privilege.Effect.ALLOW.name().equals(privilegeEffects.get(index))) { + privileges.add(Privileges.allowPrivilegeFromString(privilegeNames.get(index))); + } else { + privileges.add(Privileges.allowPrivilegeFromString(privilegeNames.get(index))); + } + } + securableObject.bindPrivileges(privileges); + return RoleEntity.builder() .withId(rolePO.getRoleId()) .withName(rolePO.getRoleName()) .withNamespace(namespace) .withProperties(JsonUtils.anyFieldMapper().readValue(rolePO.getProperties(), Map.class)) - .withPrivileges( - JsonUtils.anyFieldMapper() - .readValue(rolePO.getPrivileges(), new TypeReference>() {}).stream() - .map(Privileges::fromString) - .collect(Collectors.toList())) - .withSecurableObject( - SecurableObjects.parse( - rolePO.getSecurableObjectFullName(), - SecurableObject.Type.valueOf(rolePO.getSecurableObjectType()))) + .withSecurableObject(securableObject) .withAuditInfo( JsonUtils.anyFieldMapper().readValue(rolePO.getAuditInfo(), AuditInfo.class)) .build(); diff --git a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java index 8e4d88e4520..25a9cd419f8 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java @@ -222,11 +222,7 @@ public void testCreateRole() { Role role = accessControlManager.createRole( - "metalake", - "create", - props, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList()); + "metalake", "create", props, SecurableObjects.ofCatalog("catalog")); Assertions.assertEquals("create", role.name()); testProperties(props, role.properties()); @@ -235,11 +231,7 @@ public void testCreateRole() { RoleAlreadyExistsException.class, () -> accessControlManager.createRole( - "metalake", - "create", - props, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList())); + "metalake", "create", props, SecurableObjects.ofCatalog("catalog"))); } @Test @@ -247,7 +239,7 @@ public void testLoadRole() { Map props = ImmutableMap.of("k1", "v1"); accessControlManager.createRole( - "metalake", "loadRole", props, SecurableObjects.ofCatalog("catalog"), Lists.newArrayList()); + "metalake", "loadRole", props, SecurableObjects.ofCatalog("catalog")); Role cachedRole = accessControlManager.getRole("metalake", "loadRole"); accessControlManager.getRoleManager().getCache().invalidateAll(); @@ -271,7 +263,7 @@ public void testDropRole() { Map props = ImmutableMap.of("k1", "v1"); accessControlManager.createRole( - "metalake", "testDrop", props, SecurableObjects.ofCatalog("catalog"), Lists.newArrayList()); + "metalake", "testDrop", props, SecurableObjects.ofCatalog("catalog")); // Test drop role boolean dropped = accessControlManager.deleteRole("metalake", "testDrop"); diff --git a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java index f30f84ee944..89df8c73205 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java @@ -85,7 +85,6 @@ public class TestAccessControlManagerForPermissions { .withId(1L) .withName("role") .withProperties(Maps.newHashMap()) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .withSecurableObject(SecurableObjects.ofCatalog(CATALOG)) .withAuditInfo(auditInfo) .build(); @@ -270,7 +269,6 @@ public void testDropRole() throws IOException { .withId(1L) .withName(anotherRole) .withProperties(Maps.newHashMap()) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .withSecurableObject(SecurableObjects.ofCatalog(CATALOG)) .withAuditInfo(auditInfo) .build(); diff --git a/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java b/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java index 109254af7fc..7bb2c00c0d3 100644 --- a/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java +++ b/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java @@ -6,7 +6,6 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.Field; -import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.file.Fileset; import com.google.common.collect.ImmutableMap; @@ -267,7 +266,6 @@ public void testRole() { .withName(roleName) .withAuditInfo(auditInfo) .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .withProperties(map) .build(); @@ -276,8 +274,6 @@ public void testRole() { Assertions.assertEquals(roleName, fields.get(RoleEntity.NAME)); Assertions.assertEquals(auditInfo, fields.get(RoleEntity.AUDIT_INFO)); Assertions.assertEquals(map, fields.get(RoleEntity.PROPERTIES)); - Assertions.assertEquals( - Lists.newArrayList(Privileges.UseCatalog.get()), fields.get(RoleEntity.PRIVILEGES)); Assertions.assertEquals( SecurableObjects.ofCatalog(catalogName), fields.get(RoleEntity.SECURABLE_OBJECT)); @@ -287,7 +283,6 @@ public void testRole() { .withName(roleName) .withAuditInfo(auditInfo) .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .build(); Assertions.assertNull(roleWithoutFields.properties()); } diff --git a/core/src/test/java/com/datastrato/gravitino/proto/TestEntityProtoSerDe.java b/core/src/test/java/com/datastrato/gravitino/proto/TestEntityProtoSerDe.java index 3f88105e21c..2c4bc64037c 100644 --- a/core/src/test/java/com/datastrato/gravitino/proto/TestEntityProtoSerDe.java +++ b/core/src/test/java/com/datastrato/gravitino/proto/TestEntityProtoSerDe.java @@ -9,6 +9,7 @@ import com.datastrato.gravitino.EntitySerDeFactory; import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.authorization.Privileges; +import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.meta.GroupEntity; import com.datastrato.gravitino.meta.RoleEntity; @@ -380,14 +381,16 @@ public void testEntitiesSerDe() throws IOException { Namespace.of("metalake", Entity.SYSTEM_CATALOG_RESERVED_NAME, Entity.ROLE_SCHEMA_NAME); Long roleId = 1L; String roleName = "testRole"; + SecurableObject securableObject = SecurableObjects.ofCatalog(catalogName); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + RoleEntity roleEntity = RoleEntity.builder() .withId(roleId) .withName(roleName) .withNamespace(roleNamespace) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject(securableObject) .withProperties(props) .build(); byte[] roleBytes = protoEntitySerDe.serialize(roleEntity); @@ -401,8 +404,7 @@ public void testEntitiesSerDe() throws IOException { .withName(roleName) .withNamespace(roleNamespace) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject(securableObject) .build(); roleBytes = protoEntitySerDe.serialize(roleWithoutFields); roleFromBytes = protoEntitySerDe.deserialize(roleBytes, RoleEntity.class, roleNamespace); diff --git a/core/src/test/java/com/datastrato/gravitino/storage/TestEntityStorage.java b/core/src/test/java/com/datastrato/gravitino/storage/TestEntityStorage.java index 95e651a6982..4dbfd0d129d 100644 --- a/core/src/test/java/com/datastrato/gravitino/storage/TestEntityStorage.java +++ b/core/src/test/java/com/datastrato/gravitino/storage/TestEntityStorage.java @@ -31,6 +31,7 @@ import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.authorization.AuthorizationUtils; import com.datastrato.gravitino.authorization.Privileges; +import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.exceptions.AlreadyExistsException; import com.datastrato.gravitino.exceptions.NoSuchEntityException; @@ -1261,13 +1262,15 @@ private static GroupEntity createGroup( } private static RoleEntity createRole(Long id, String metalake, String name, AuditInfo auditInfo) { + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + return RoleEntity.builder() .withId(id) .withNamespace(AuthorizationUtils.ofRoleNamespace(metalake)) .withName(name) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog("catalog")) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject(securableObject) .withProperties(null) .build(); } diff --git a/core/src/test/java/com/datastrato/gravitino/storage/memory/TestMemoryEntityStore.java b/core/src/test/java/com/datastrato/gravitino/storage/memory/TestMemoryEntityStore.java index bf4dfc6f13a..bb42f923e8d 100644 --- a/core/src/test/java/com/datastrato/gravitino/storage/memory/TestMemoryEntityStore.java +++ b/core/src/test/java/com/datastrato/gravitino/storage/memory/TestMemoryEntityStore.java @@ -16,7 +16,6 @@ import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.TestCatalog; import com.datastrato.gravitino.authorization.AuthorizationUtils; -import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.exceptions.NoSuchEntityException; import com.datastrato.gravitino.file.Fileset; @@ -31,7 +30,6 @@ import com.datastrato.gravitino.meta.TableEntity; import com.datastrato.gravitino.meta.UserEntity; import com.datastrato.gravitino.utils.Executable; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; import java.io.IOException; import java.time.Instant; @@ -229,7 +227,6 @@ public void testEntityStoreAndRetrieve() throws Exception { .withNamespace(AuthorizationUtils.ofRoleNamespace("metalake")) .withAuditInfo(auditInfo) .withSecurableObject(SecurableObjects.ofCatalog("catalog")) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .build(); InMemoryEntityStore store = new InMemoryEntityStore(); diff --git a/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java b/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java index 1f1a0163c5a..a5e79f74849 100644 --- a/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java +++ b/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java @@ -765,14 +765,16 @@ public static UserEntity createUserEntity( public static RoleEntity createRoleEntity( Long id, Namespace namespace, String name, AuditInfo auditInfo) { + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + return RoleEntity.builder() .withId(id) .withName(name) .withNamespace(namespace) .withProperties(null) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog("catalog")) - .withPrivileges(Lists.newArrayList(Privileges.fromName(Privilege.Name.USE_CATALOG))) + .withSecurableObject(securableObject) .build(); } @@ -813,6 +815,8 @@ public static RoleEntity createRoleEntity( SecurableObject securableObject, List privileges, Map properties) { + securableObject.bindPrivileges(privileges); + return RoleEntity.builder() .withId(id) .withName(name) @@ -820,7 +824,6 @@ public static RoleEntity createRoleEntity( .withNamespace(namespace) .withAuditInfo(auditInfo) .withSecurableObject(securableObject) - .withPrivileges(privileges) .build(); } } diff --git a/core/src/test/java/com/datastrato/gravitino/storage/relational/service/TestRoleMetaService.java b/core/src/test/java/com/datastrato/gravitino/storage/relational/service/TestRoleMetaService.java index 8153dc67035..079b710d3c7 100644 --- a/core/src/test/java/com/datastrato/gravitino/storage/relational/service/TestRoleMetaService.java +++ b/core/src/test/java/com/datastrato/gravitino/storage/relational/service/TestRoleMetaService.java @@ -58,7 +58,7 @@ void getRoleByIdentifier() { "role1", auditInfo, SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k1", "v1")); roleMetaService.insertRole(role1, false); @@ -83,7 +83,7 @@ void insertRole() { "role1", auditInfo, SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( NoSuchEntityException.class, @@ -99,7 +99,7 @@ void insertRole() { "role1", auditInfo, SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( AlreadyExistsException.class, () -> roleMetaService.insertRole(role1Exist, false)); @@ -112,7 +112,7 @@ void insertRole() { "role1Overwrite", auditInfo, SecurableObjects.ofCatalog("catalogOverwrite"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k2", "v2")); Assertions.assertDoesNotThrow(() -> roleMetaService.insertRole(role1Overwrite, true)); Assertions.assertEquals( @@ -147,7 +147,7 @@ void deleteRole() { "role1", auditInfo, SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( NoSuchEntityException.class, @@ -167,7 +167,7 @@ void deleteRole() { "role2", auditInfo, SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + Lists.newArrayList(Privileges.UseCatalog.allow()), ImmutableMap.of("k1", "v1")); roleMetaService.insertRole(role2, false); GroupEntity group1 = diff --git a/core/src/test/resources/h2/schema-h2.sql b/core/src/test/resources/h2/schema-h2.sql index cba1590c448..0b5f951e241 100644 --- a/core/src/test/resources/h2/schema-h2.sql +++ b/core/src/test/resources/h2/schema-h2.sql @@ -146,6 +146,7 @@ CREATE TABLE IF NOT EXISTS `role_meta` ( `securable_object_full_name` VARCHAR(256) NOT NULL COMMENT 'securable object full name', `securable_object_type` VARCHAR(32) NOT NULL COMMENT 'securable object type', `privileges` VARCHAR(64) NOT NULL COMMENT 'securable privileges', + `privilege_effects` VARCHAR(64) NOT NULL COMMENT 'securable privilege effects', `audit_info` MEDIUMTEXT NOT NULL COMMENT 'role audit info', `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role current version', `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role last version', diff --git a/meta/src/main/proto/gravitino_meta.proto b/meta/src/main/proto/gravitino_meta.proto index 98162c3ba98..607745dc190 100644 --- a/meta/src/main/proto/gravitino_meta.proto +++ b/meta/src/main/proto/gravitino_meta.proto @@ -122,8 +122,9 @@ message Role { uint64 id = 1; string name = 2; repeated string privileges = 3; - string securable_object_full_name = 4; - string securable_object_type = 5; - map properties = 6; - AuditInfo audit_info = 7; + repeated string privilege_effects = 4; + string securable_object_full_name = 5; + string securable_object_type = 6; + map properties = 7; + AuditInfo audit_info = 8; } diff --git a/scripts/mysql/schema-0.5.0-mysql.sql b/scripts/mysql/schema-0.5.0-mysql.sql index 72c58a69637..b79d01e5286 100644 --- a/scripts/mysql/schema-0.5.0-mysql.sql +++ b/scripts/mysql/schema-0.5.0-mysql.sql @@ -138,6 +138,7 @@ CREATE TABLE IF NOT EXISTS `role_meta` ( `securable_object_full_name` VARCHAR(256) NOT NULL COMMENT 'securable object full name', `securable_object_type` VARCHAR(32) NOT NULL COMMENT 'securable object type', `privileges` VARCHAR(64) NOT NULL COMMENT 'securable privileges', + `privilege_effects` VARCHAR(64) NOT NULL COMMENT 'securable privilege effects', `audit_info` MEDIUMTEXT NOT NULL COMMENT 'role audit info', `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role current version', `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role last version', diff --git a/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java b/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java index 811553e1938..102e0765e9b 100644 --- a/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java +++ b/server/src/main/java/com/datastrato/gravitino/server/web/rest/RoleOperations.java @@ -8,7 +8,9 @@ import com.codahale.metrics.annotation.Timed; import com.datastrato.gravitino.GravitinoEnv; import com.datastrato.gravitino.authorization.AccessControlManager; +import com.datastrato.gravitino.authorization.Privilege; import com.datastrato.gravitino.authorization.Privileges; +import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; import com.datastrato.gravitino.dto.responses.DeleteResponse; @@ -68,20 +70,32 @@ public Response createRole(@PathParam("metalake") String metalake, RoleCreateReq return Utils.doAs( httpRequest, - () -> - Utils.ok( - new RoleResponse( - DTOConverters.toDTO( - accessControlManager.createRole( - metalake, - request.getName(), - request.getProperties(), - SecurableObjects.parse( - request.getSecurableObject().fullName(), - request.getSecurableObject().type()), - request.getPrivileges().stream() - .map(Privileges::fromString) - .collect(Collectors.toList())))))); + () -> { + SecurableObject securableObject = + SecurableObjects.parse( + request.getSecurableObjects()[0].fullName(), + request.getSecurableObjects()[0].type()); + securableObject.bindPrivileges( + request.getSecurableObjects()[0].privileges().stream() + .map( + privilege -> { + if (privilege.effect().equals(Privilege.Effect.ALLOW)) { + return Privileges.allowPrivilegeFromName(privilege.name()); + } else { + return Privileges.denyPrivilegeFromName(privilege.name()); + } + }) + .collect(Collectors.toList())); + return Utils.ok( + new RoleResponse( + DTOConverters.toDTO( + accessControlManager.createRole( + metalake, + request.getName(), + request.getProperties(), + securableObject)))); + }); + } catch (Exception e) { return ExceptionHandlers.handleRoleException( OperationType.CREATE, request.getName(), metalake, e); diff --git a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java index 2c6d88468c8..e3d23afc3fe 100644 --- a/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java +++ b/server/src/test/java/com/datastrato/gravitino/server/web/rest/TestRoleOperations.java @@ -17,8 +17,10 @@ import com.datastrato.gravitino.authorization.AccessControlManager; import com.datastrato.gravitino.authorization.Privileges; import com.datastrato.gravitino.authorization.Role; +import com.datastrato.gravitino.authorization.SecurableObject; import com.datastrato.gravitino.authorization.SecurableObjects; import com.datastrato.gravitino.dto.authorization.RoleDTO; +import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.RoleCreateRequest; import com.datastrato.gravitino.dto.responses.DeleteResponse; import com.datastrato.gravitino.dto.responses.ErrorConstants; @@ -97,15 +99,17 @@ protected void configure() { @Test public void testCreateRole() { + SecurableObject securableObject = SecurableObjects.ofCatalog("catalog"); + securableObject.bindPrivileges(Lists.newArrayList(Privileges.UseCatalog.allow())); + RoleCreateRequest req = new RoleCreateRequest( "role", Collections.emptyMap(), - Lists.newArrayList(Privileges.UseCatalog.get().name().toString()), - DTOConverters.toDTO(SecurableObjects.ofCatalog("catalog"))); + new SecurableObjectDTO[] {DTOConverters.toDTO(securableObject)}); Role role = buildRole("role1"); - when(manager.createRole(any(), any(), any(), any(), any())).thenReturn(role); + when(manager.createRole(any(), any(), any(), any())).thenReturn(role); Response resp = target("/metalakes/metalake1/roles") @@ -122,13 +126,16 @@ public void testCreateRole() { RoleDTO roleDTO = roleResponse.getRole(); Assertions.assertEquals("role1", roleDTO.name()); Assertions.assertEquals( - SecurableObjects.ofCatalog("catalog").fullName(), roleDTO.securableObject().fullName()); - Assertions.assertEquals(Lists.newArrayList(Privileges.UseCatalog.get()), roleDTO.privileges()); + SecurableObjects.ofCatalog("catalog").fullName(), + roleDTO.securableObjects().get(0).fullName()); + Assertions.assertEquals( + Lists.newArrayList(Privileges.UseCatalog.allow()), + roleDTO.securableObjects().get(0).privileges()); // Test to throw NoSuchMetalakeException doThrow(new NoSuchMetalakeException("mock error")) .when(manager) - .createRole(any(), any(), any(), any(), any()); + .createRole(any(), any(), any(), any()); Response resp1 = target("/metalakes/metalake1/roles") .request(MediaType.APPLICATION_JSON_TYPE) @@ -145,7 +152,7 @@ public void testCreateRole() { // Test to throw RoleAlreadyExistsException doThrow(new RoleAlreadyExistsException("mock error")) .when(manager) - .createRole(any(), any(), any(), any(), any()); + .createRole(any(), any(), any(), any()); Response resp2 = target("/metalakes/metalake1/roles") .request(MediaType.APPLICATION_JSON_TYPE) @@ -162,7 +169,7 @@ public void testCreateRole() { // Test to throw internal RuntimeException doThrow(new RuntimeException("mock error")) .when(manager) - .createRole(any(), any(), any(), any(), any()); + .createRole(any(), any(), any(), any()); Response resp3 = target("/metalakes/metalake1/roles") .request(MediaType.APPLICATION_JSON_TYPE) @@ -197,8 +204,11 @@ public void testGetRole() { Assertions.assertEquals("role1", roleDTO.name()); Assertions.assertTrue(role.properties().isEmpty()); Assertions.assertEquals( - SecurableObjects.ofCatalog("catalog").fullName(), roleDTO.securableObject().fullName()); - Assertions.assertEquals(Lists.newArrayList(Privileges.UseCatalog.get()), roleDTO.privileges()); + SecurableObjects.ofCatalog("catalog").fullName(), + roleDTO.securableObjects().get(0).fullName()); + Assertions.assertEquals( + Lists.newArrayList(Privileges.UseCatalog.allow()), + roleDTO.securableObjects().get(0).privileges()); // Test to throw NoSuchMetalakeException doThrow(new NoSuchMetalakeException("mock error")).when(manager).getRole(any(), any()); @@ -248,7 +258,6 @@ private Role buildRole(String role) { return RoleEntity.builder() .withId(1L) .withName(role) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .withProperties(Collections.emptyMap()) .withSecurableObject(SecurableObjects.ofCatalog("catalog")) .withAuditInfo(