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..bc7f56b88a2 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 condition of the privilege. `ALLOW` means that you are allowed to use the + * privilege, `DENY` means that you are denied to use the privilege + */ + Condition condition(); + /** The name of this privilege. */ enum Name { /** The privilege to create a catalog. */ @@ -116,4 +122,16 @@ public long getHighBits() { return highBits; } } + + /** + * The condition of this privilege. `ALLOW` means that you are allowed to use the privilege, + * `DENY` means that you are denied to use the privilege. If you have `ALLOW` and `DENY` for the + * same privilege name of the same securable object, the `DENY` will take effect. + */ + enum Condition { + /** Allow to use the privilege */ + ALLOW, + /** Deny to use the privilege */ + 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..1bbe1254b1c 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/Privileges.java @@ -43,125 +43,254 @@ public class Privileges { /** - * Returns the Privilege from the string representation. + * Returns the Privilege with allow condition from the string representation. * * @param privilege The string representation of the privilege. * @return The Privilege. */ - public static Privilege fromString(String privilege) { + public static Privilege allow(String privilege) { Privilege.Name name = Privilege.Name.valueOf(privilege); - return fromName(name); + return allow(name); } /** - * Returns the Privilege from the `Privilege.Name`. + * Returns the Privilege with allow condition from the `Privilege.Name`. * * @param name The `Privilege.Name` of the privilege. * @return The Privilege. */ - public static Privilege fromName(Privilege.Name name) { + public static Privilege allow(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); + throw new IllegalArgumentException("Doesn't support the privilege: " + name); } } - /** The privilege to create a catalog. */ - public static class CreateCatalog implements Privilege { + /** + * Returns the Privilege with deny condition from the string representation. + * + * @param privilege The string representation of the privilege. + * @return The Privilege. + */ + public static Privilege deny(String privilege) { + Privilege.Name name = Privilege.Name.valueOf(privilege); + return deny(name); + } - private static final CreateCatalog INSTANCE = new CreateCatalog(); + /** + * Returns the Privilege with deny condition from the `Privilege.Name`. + * + * @param name The `Privilege.Name` of the privilege. + * @return The Privilege. + */ + public static Privilege deny(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("Doesn't support the privilege: " + name); + } + } + + /** The privilege to create a catalog. */ + public abstract static class CreateCatalog implements Privilege { + + private static final CreateCatalog ALLOW_INSTANCE = + new CreateCatalog() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; + + private static final CreateCatalog DENY_INSTANCE = + new CreateCatalog() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private CreateCatalog() {} - /** @return The instance of the privilege. */ - public static CreateCatalog get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static CreateCatalog allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static CreateCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -173,20 +302,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create catalog"; + return condition().name() + " create catalog"; } } /** The privilege to alter a catalog. */ - public static class AlterCatalog implements Privilege { - - private static final AlterCatalog INSTANCE = new AlterCatalog(); + public abstract static class AlterCatalog implements Privilege { + + private static final AlterCatalog ALLOW_INSTANCE = + new AlterCatalog() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; + + private static final AlterCatalog DENY_INSTANCE = + new AlterCatalog() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private AlterCatalog() {} - /** @return The instance of the privilege. */ - public static AlterCatalog get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static AlterCatalog allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static AlterCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -198,20 +346,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "alter catalog"; + return condition().name() + " alter catalog"; } } /** The privilege to drop a catalog. */ - public static class DropCatalog implements Privilege { - - private static final DropCatalog INSTANCE = new DropCatalog(); + public abstract static class DropCatalog implements Privilege { + + private static final DropCatalog ALLOW_INSTANCE = + new DropCatalog() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; + + private static final DropCatalog DENY_INSTANCE = + new DropCatalog() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private DropCatalog() {} - /** @return The instance of the privilege. */ - public static DropCatalog get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static DropCatalog allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static DropCatalog deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -223,20 +390,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop catalog"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final UseCatalog DENY_INSTANCE = + new UseCatalog() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; + + private UseCatalog() {} - /** @return The instance of the privilege. */ - public static UseCatalog get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static UseCatalog allow() { + return ALLOW_INSTANCE; } - private UseCatalog() {} + /** @return The instance with deny condition of the privilege. */ + public static UseCatalog deny() { + return DENY_INSTANCE; + } /** @return The generic name of the privilege. */ @Override @@ -247,18 +433,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use catalog"; + return condition().name() + " use catalog"; } } /** The privilege to use a schema. */ - public static class UseSchema implements Privilege { + public abstract static class UseSchema implements Privilege { + + private static final UseSchema ALLOW_INSTANCE = + new UseSchema() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final UseSchema INSTANCE = new UseSchema(); + private static final UseSchema DENY_INSTANCE = + new UseSchema() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; + + private UseSchema() {} + + /** @return The instance with allow condition of the privilege. */ + public static UseSchema allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static UseSchema get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static UseSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -270,18 +477,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use schema"; + return condition().name() + " use schema"; } } /** The privilege to create a schema. */ - public static class CreateSchema implements Privilege { + public abstract static class CreateSchema implements Privilege { - private static final CreateSchema INSTANCE = new CreateSchema(); + private static final CreateSchema ALLOW_INSTANCE = + new CreateSchema() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - /** @return The instance of the privilege. */ - public static CreateSchema get() { - return INSTANCE; + private static final CreateSchema DENY_INSTANCE = + new CreateSchema() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; + + private CreateSchema() {} + + /** @return The instance with allow condition of the privilege. */ + public static CreateSchema allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static CreateSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -293,18 +521,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create schema"; + return condition().name() + " create schema"; } } /** The privilege to alter a schema. */ - public static class AlterSchema implements Privilege { + public abstract static class AlterSchema implements Privilege { + + private static final AlterSchema ALLOW_INSTANCE = + new AlterSchema() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final AlterSchema INSTANCE = new AlterSchema(); + private static final AlterSchema DENY_INSTANCE = + new AlterSchema() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static AlterSchema get() { - return INSTANCE; + private AlterSchema() {} + + /** @return The instance with allow condition of the privilege. */ + public static AlterSchema allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static AlterSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -316,18 +565,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "alter schema"; + return condition().name() + " alter schema"; } } /** The privilege to drop a schema. */ - public static class DropSchema implements Privilege { + public abstract static class DropSchema implements Privilege { + + private static final DropSchema ALLOW_INSTANCE = + new DropSchema() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; + + private static final DropSchema DENY_INSTANCE = + new DropSchema() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - private static final DropSchema INSTANCE = new DropSchema(); + private DropSchema() {} - /** @return The instance of the privilege. */ - public static DropSchema get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static DropSchema allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static DropSchema deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -339,20 +609,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop schema"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final CreateTable DENY_INSTANCE = + new CreateTable() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private CreateTable() {} - /** @return The instance of the privilege. */ - public static CreateTable get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static CreateTable allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static CreateTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -364,20 +653,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create table"; + return condition().name() + " create table"; } } /** The privilege to drop a table. */ - public static class DropTable implements Privilege { + public abstract static class DropTable implements Privilege { - private static final DropTable INSTANCE = new DropTable(); + private static final DropTable ALLOW_INSTANCE = + new DropTable() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private DropTable() {} + private static final DropTable DENY_INSTANCE = + new DropTable() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static DropTable get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static DropTable allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static DropTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -389,20 +695,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop table"; + return condition().name() + " drop table"; } } /** The privilege to read a table. */ - public static class ReadTable implements Privilege { + public abstract static class ReadTable implements Privilege { - private static final ReadTable INSTANCE = new ReadTable(); + private static final ReadTable ALLOW_INSTANCE = + new ReadTable() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private ReadTable() {} + private static final ReadTable DENY_INSTANCE = + new ReadTable() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static ReadTable get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static ReadTable allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static ReadTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -414,20 +737,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read table"; + return condition().name() + " read table"; } } /** The privilege to write a table. */ - public static class WriteTable implements Privilege { + public abstract static class WriteTable implements Privilege { + + private static final WriteTable ALLOW_INSTANCE = + new WriteTable() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final WriteTable INSTANCE = new WriteTable(); + private static final WriteTable DENY_INSTANCE = + new WriteTable() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - private WriteTable() {} + /** @return The instance with allow condition of the privilege. */ + public static WriteTable allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static WriteTable get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static WriteTable deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -439,20 +779,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write table"; + return condition().name() + " write table"; } } /** The privilege to create a fileset. */ - public static class CreateFileset implements Privilege { + public abstract static class CreateFileset implements Privilege { + + private static final CreateFileset ALLOW_INSTANCE = + new CreateFileset() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final CreateFileset INSTANCE = new CreateFileset(); + private static final CreateFileset DENY_INSTANCE = + new CreateFileset() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - private CreateFileset() {} + /** @return The instance with allow condition of the privilege. */ + public static CreateFileset allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static CreateFileset get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static CreateFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -464,20 +821,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create fileset"; + return condition().name() + " create fileset"; } } /** The privilege to drop a fileset. */ - public static class DropFileset implements Privilege { + public abstract static class DropFileset implements Privilege { - private static final DropFileset INSTANCE = new DropFileset(); + private static final DropFileset ALLOW_INSTANCE = + new DropFileset() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private DropFileset() {} + private static final DropFileset DENY_INSTANCE = + new DropFileset() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; + + /** @return The instance with allow condition of the privilege. */ + public static DropFileset allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static DropFileset get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static DropFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -489,20 +863,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop fileset"; + return condition().name() + " drop fileset"; } } /** The privilege to read a fileset. */ - public static class ReadFileset implements Privilege { + public abstract static class ReadFileset implements Privilege { - private static final ReadFileset INSTANCE = new ReadFileset(); + private static final ReadFileset ALLOW_INSTANCE = + new ReadFileset() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private ReadFileset() {} + private static final ReadFileset DENY_INSTANCE = + new ReadFileset() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static ReadFileset get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static ReadFileset allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static ReadFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -514,20 +905,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read fileset"; + return condition().name() + " read fileset"; } } /** The privilege to write a fileset. */ - public static class WriteFileset implements Privilege { + public abstract static class WriteFileset implements Privilege { - private static final WriteFileset INSTANCE = new WriteFileset(); + private static final WriteFileset ALLOW_INSTANCE = + new WriteFileset() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private WriteFileset() {} + private static final WriteFileset DENY_INSTANCE = + new WriteFileset() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static WriteFileset get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static WriteFileset allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static WriteFileset deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -539,20 +947,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write fileset"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final CreateTopic DENY_INSTANCE = + new CreateTopic() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private CreateTopic() {} - /** @return The instance of the privilege. */ - public static CreateTopic get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static CreateTopic allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static CreateTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -564,20 +991,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create topic"; + return condition().name() + " create topic"; } } /** The privilege to drop a topic. */ - public static class DropTopic implements Privilege { + public abstract static class DropTopic implements Privilege { - private static final DropTopic INSTANCE = new DropTopic(); + private static final DropTopic ALLOW_INSTANCE = + new DropTopic() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private DropTopic() {} + private static final DropTopic DENY_INSTANCE = + new DropTopic() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static DropTopic get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static DropTopic allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static DropTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -589,20 +1033,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "drop topic"; + return condition().name() + " drop topic"; } } /** The privilege to read a topic. */ - public static class ReadTopic implements Privilege { + public abstract static class ReadTopic implements Privilege { - private static final ReadTopic INSTANCE = new ReadTopic(); + private static final ReadTopic ALLOW_INSTANCE = + new ReadTopic() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private ReadTopic() {} + private static final ReadTopic DENY_INSTANCE = + new ReadTopic() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static ReadTopic get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static ReadTopic allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static ReadTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -614,20 +1075,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "read topic"; + return condition().name() + " read topic"; } } /** The privilege to write a topic. */ - public static class WriteTopic implements Privilege { + public abstract static class WriteTopic implements Privilege { + + private static final WriteTopic ALLOW_INSTANCE = + new WriteTopic() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final WriteTopic INSTANCE = new WriteTopic(); + private static final WriteTopic DENY_INSTANCE = + new WriteTopic() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - private WriteTopic() {} + /** @return The instance with allow condition of the privilege. */ + public static WriteTopic allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static WriteTopic get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static WriteTopic deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -639,20 +1117,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "write topic"; + return condition().name() + " write topic"; } } /** The privilege to manage a metalake. */ - public static class ManageMetalake implements Privilege { + public abstract static class ManageMetalake implements Privilege { - private static final ManageMetalake INSTANCE = new ManageMetalake(); + private static final ManageMetalake ALLOW_INSTANCE = + new ManageMetalake() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private ManageMetalake() {} + private static final ManageMetalake DENY_INSTANCE = + new ManageMetalake() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; + + /** @return The instance with allow condition of the privilege. */ + public static ManageMetalake allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static ManageMetalake get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static ManageMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -664,20 +1159,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "manage metalake"; + return condition().name() + " manage metalake"; } } /** The privilege to manage a metalake. */ - public static class CreateMetalake implements Privilege { + public abstract static class CreateMetalake implements Privilege { - private static final CreateMetalake INSTANCE = new CreateMetalake(); + private static final CreateMetalake ALLOW_INSTANCE = + new CreateMetalake() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private CreateMetalake() {} + private static final CreateMetalake DENY_INSTANCE = + new CreateMetalake() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static CreateMetalake get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static CreateMetalake allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static CreateMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -689,20 +1201,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create metalake"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final UseMetalake DENY_INSTANCE = + new UseMetalake() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private UseMetalake() {} - /** @return The instance of the privilege. */ - public static UseMetalake get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static UseMetalake allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static UseMetalake deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -714,20 +1245,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "use metalake"; + return condition().name() + " use metalake"; } } /** The privilege to get a user. */ - public static class GetUser implements Privilege { + public abstract static class GetUser implements Privilege { - private static final GetUser INSTANCE = new GetUser(); + private static final GetUser ALLOW_INSTANCE = + new GetUser() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private GetUser() {} + private static final GetUser DENY_INSTANCE = + new GetUser() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static GetUser get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static GetUser allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static GetUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -739,20 +1287,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get user"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final AddUser DENY_INSTANCE = + new AddUser() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private AddUser() {} - /** @return The instance of the privilege. */ - public static AddUser get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static AddUser allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static AddUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -764,20 +1331,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "add user"; + return condition().name() + " add user"; } } /** The privilege to remove a user. */ - public static class RemoveUser implements Privilege { + public abstract static class RemoveUser implements Privilege { - private static final RemoveUser INSTANCE = new RemoveUser(); + private static final RemoveUser ALLOW_INSTANCE = + new RemoveUser() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private RemoveUser() {} + private static final RemoveUser DENY_INSTANCE = + new RemoveUser() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - /** @return The instance of the privilege. */ - public static RemoveUser get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static RemoveUser allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static RemoveUser deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -789,20 +1373,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "remove user"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final AddGroup DENY_INSTANCE = + new AddGroup() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private AddGroup() {} - /** @return The instance of the privilege. */ - public static AddGroup get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static AddGroup allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static AddGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -814,20 +1417,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "add group"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final RemoveGroup DENY_INSTANCE = + new RemoveGroup() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private RemoveGroup() {} - /** @return The instance of the privilege. */ - public static RemoveGroup get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static RemoveGroup allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static RemoveGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -839,20 +1461,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "remove group"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final GetGroup DENY_INSTANCE = + new GetGroup() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private GetGroup() {} - /** @return The instance of the privilege. */ - public static GetGroup get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static GetGroup allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static GetGroup deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -864,20 +1505,37 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get group"; + return condition().name() + " get group"; } } /** The privilege to create a role. */ - public static class CreateRole implements Privilege { + public abstract static class CreateRole implements Privilege { + + private static final CreateRole ALLOW_INSTANCE = + new CreateRole() { + @Override + public Condition condition() { + return Condition.ALLOW; + } + }; - private static final CreateRole INSTANCE = new CreateRole(); + private static final CreateRole DENY_INSTANCE = + new CreateRole() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; - private CreateRole() {} + /** @return The instance with allow condition of the privilege. */ + public static CreateRole allow() { + return ALLOW_INSTANCE; + } - /** @return The instance of the privilege. */ - public static CreateRole get() { - return INSTANCE; + /** @return The instance with deny condition of the privilege. */ + public static CreateRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -889,20 +1547,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "create role"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final GetRole DENY_INSTANCE = + new GetRole() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private GetRole() {} - /** @return The instance of the privilege. */ - public static GetRole get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static GetRole allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static GetRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -914,20 +1591,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "get role"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final DeleteRole DENY_INSTANCE = + new DeleteRole() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private DeleteRole() {} - /** @return The instance of the privilege. */ - public static DeleteRole get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static DeleteRole allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static DeleteRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -939,20 +1635,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "delete role"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final GrantRole DENY_INSTANCE = + new GrantRole() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private GrantRole() {} - /** @return The instance of the privilege. */ - public static GrantRole get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static GrantRole allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static GrantRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -964,20 +1679,39 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "grant role"; + return condition().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 Condition condition() { + return Condition.ALLOW; + } + }; + + private static final RevokeRole DENY_INSTANCE = + new RevokeRole() { + @Override + public Condition condition() { + return Condition.DENY; + } + }; private RevokeRole() {} - /** @return The instance of the privilege. */ - public static RevokeRole get() { - return INSTANCE; + /** @return The instance with allow condition of the privilege. */ + public static RevokeRole allow() { + return ALLOW_INSTANCE; + } + + /** @return The instance with deny condition of the privilege. */ + public static RevokeRole deny() { + return DENY_INSTANCE; } /** @return The generic name of the privilege. */ @@ -989,7 +1723,7 @@ public Name name() { /** @return A readable string representation for the privilege. */ @Override public String simpleString() { - return "revoke role"; + return condition().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..8cc27bb7194 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; /** @@ -43,13 +44,13 @@ public interface SecurableObject { /** - * The parent securable object. If the securable object doesn't have parent, this method will - * return null. + * The parent full name of securable object. If the securable object doesn't have parent, this + * method will return null. * - * @return The parent securable object. + * @return The parent full name of securable object. */ @Nullable - SecurableObject parent(); + String parent(); /** * The name of th securable object. @@ -73,6 +74,15 @@ public interface SecurableObject { */ Type type(); + /** + * The privileges of the 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 load all tables of the schema. + * + * @return The privileges of the role. + */ + 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..46022c6a83a 100644 --- a/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java +++ b/api/src/main/java/com/datastrato/gravitino/authorization/SecurableObjects.java @@ -5,9 +5,8 @@ package com.datastrato.gravitino.authorization; import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import java.util.Collections; import java.util.List; import java.util.Objects; import org.apache.commons.lang3.StringUtils; @@ -21,82 +20,84 @@ public class SecurableObjects { * Create the metalake {@link SecurableObject} with the given metalake name. * * @param metalake The metalake name + * @param privileges The privileges of the metalake * @return The created metalake {@link SecurableObject} */ - public static SecurableObject ofMetalake(String metalake) { - checkName(metalake); - - return new SecurableObjectImpl(null, metalake, SecurableObject.Type.METALAKE); + public static SecurableObject ofMetalake(String metalake, List privileges) { + return of(SecurableObject.Type.METALAKE, Lists.newArrayList(metalake), privileges); } /** * Create the catalog {@link SecurableObject} with the given catalog name. * * @param catalog The catalog name + * @param privileges The privileges of the catalog * @return The created catalog {@link SecurableObject} */ - public static SecurableObject ofCatalog(String catalog) { - checkName(catalog); - - return new SecurableObjectImpl(null, catalog, SecurableObject.Type.CATALOG); + public static SecurableObject ofCatalog(String catalog, List privileges) { + return of(SecurableObject.Type.CATALOG, Lists.newArrayList(catalog), privileges); } /** * Create the schema {@link SecurableObject} with the given securable catalog object and schema * name. * - * @param catalog The securable catalog object + * @param catalog The catalog securable object. * @param schema The schema name + * @param privileges The privileges of the schema * @return The created schema {@link SecurableObject} */ - public static SecurableObject ofSchema(SecurableObject catalog, String schema) { - checkCatalog(catalog); - checkName(schema); + public static SecurableObject ofSchema( + SecurableObject catalog, String schema, List privileges) { - return new SecurableObjectImpl(catalog, schema, SecurableObject.Type.SCHEMA); + return of( + SecurableObject.Type.SCHEMA, Lists.newArrayList(catalog.fullName(), schema), privileges); } /** * Create the table {@link SecurableObject} with the given securable schema object and table name. * - * @param schema The securable schema object + * @param schema The schema securable object * @param table The table name + * @param privileges The privileges of the table * @return The created table {@link SecurableObject} */ - public static SecurableObject ofTable(SecurableObject schema, String table) { - checkSchema(schema); - checkName(table); - - return new SecurableObjectImpl(schema, table, SecurableObject.Type.TABLE); + public static SecurableObject ofTable( + SecurableObject schema, String table, List privileges) { + List names = Lists.newArrayList(DOT.splitToList(schema.fullName())); + names.add(table); + return of(SecurableObject.Type.TABLE, names, privileges); } /** * Create the topic {@link SecurableObject} with the given securable schema object and topic name. * - * @param schema The securable schema object + * @param schema The schema securable object * @param topic The topic name + * @param privileges The privileges of the topic * @return The created topic {@link SecurableObject} */ - public static SecurableObject ofTopic(SecurableObject schema, String topic) { - checkSchema(schema); - checkName(topic); - - return new SecurableObjectImpl(schema, topic, SecurableObject.Type.TOPIC); + public static SecurableObject ofTopic( + SecurableObject schema, String topic, List privileges) { + List names = Lists.newArrayList(DOT.splitToList(schema.fullName())); + names.add(topic); + return of(SecurableObject.Type.TOPIC, names, privileges); } /** * Create the table {@link SecurableObject} with the given securable schema object and fileset * name. * - * @param schema The securable schema object + * @param schema The schema securable object * @param fileset The fileset name + * @param privileges The privileges of the fileset * @return The created fileset {@link SecurableObject} */ - public static SecurableObject ofFileset(SecurableObject schema, String fileset) { - checkSchema(schema); - checkName(fileset); - - return new SecurableObjectImpl(schema, fileset, SecurableObject.Type.FILESET); + public static SecurableObject ofFileset( + SecurableObject schema, String fileset, List privileges) { + List names = Lists.newArrayList(DOT.splitToList(schema.fullName())); + names.add(fileset); + return of(SecurableObject.Type.FILESET, names, privileges); } /** @@ -105,56 +106,29 @@ public static SecurableObject ofFileset(SecurableObject schema, String fileset) * object is only used for metalake admin. You can't grant any privilege to this securable object. * You can't bind this securable object to any role, too. * + * @param privileges The privileges of the all metalakes * @return The created {@link SecurableObject} */ - public static SecurableObject ofAllMetalakes() { - return ALL_METALAKES; - } - - private static void checkSchema(SecurableObject schema) { - if (schema == null) { - throw new IllegalArgumentException("Securable schema object can't be null"); - } - - if (schema.type() != SecurableObject.Type.SCHEMA) { - throw new IllegalArgumentException("Securable schema object type must be SCHEMA"); - } - - checkCatalog(schema.parent()); - } - - private static void checkCatalog(SecurableObject catalog) { - if (catalog == null) { - throw new IllegalArgumentException("Securable catalog object can't be null"); - } - - if (catalog.type() != SecurableObject.Type.CATALOG) { - throw new IllegalArgumentException("Securable catalog type must be CATALOG"); - } - - if (catalog.parent() != null) { - throw new IllegalArgumentException( - String.format("The parent of securable catalog object %s must be null", catalog.name())); - } + public static SecurableObject ofAllMetalakes(List privileges) { + return new SecurableObjectImpl(null, "*", SecurableObject.Type.METALAKE, privileges); } - private static final SecurableObject ALL_METALAKES = - new SecurableObjectImpl(null, "*", SecurableObject.Type.METALAKE); - private static class SecurableObjectImpl implements SecurableObject { - private final SecurableObject parent; + private final String parent; private final String name; private final Type type; + private List privileges; - SecurableObjectImpl(SecurableObject parent, String name, Type type) { + SecurableObjectImpl(String parent, String name, Type type, List privileges) { this.parent = parent; this.name = name; this.type = type; + this.privileges = ImmutableList.copyOf(privileges); } @Override - public SecurableObject parent() { + public String parent() { return parent; } @@ -173,15 +147,20 @@ public Type type() { return type; } + @Override + public List privileges() { + return privileges; + } + @Override public int hashCode() { - return Objects.hash(parent, name, type); + return Objects.hash(parent, name, type, privileges); } @Override public String toString() { if (parent != null) { - return parent.toString() + "." + name; + return parent + "." + name; } else { return name; } @@ -196,7 +175,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()); } } @@ -205,22 +185,25 @@ public boolean equals(Object other) { * * @param fullName The full name of securable object. * @param type The securable object type. + * @param privileges The secureable object privileges. * @return The created {@link SecurableObject} */ - public static SecurableObject parse(String fullName, SecurableObject.Type type) { + public static SecurableObject parse( + String fullName, SecurableObject.Type type, List privileges) { if ("*".equals(fullName)) { if (type != SecurableObject.Type.METALAKE) { throw new IllegalArgumentException("If securable object isn't metalake, it can't be `*`"); } - return SecurableObjects.ofAllMetalakes(); + return SecurableObjects.ofAllMetalakes(privileges); } if (StringUtils.isBlank(fullName)) { - throw new IllegalArgumentException("securable object identifier can't be blank"); + throw new IllegalArgumentException("securable object full name can't be blank"); } - Iterable parts = DOT.split(fullName); - return SecurableObjects.of(type, Iterables.toArray(parts, String.class)); + List parts = DOT.splitToList(fullName); + + return SecurableObjects.of(type, parts, privileges); } /** @@ -228,15 +211,16 @@ public static SecurableObject parse(String fullName, SecurableObject.Type type) * * @param type The securable object type. * @param names The names of the securable object. + * @param privileges The secureable object privileges. * @return The created {@link SecurableObject} */ - static SecurableObject of(SecurableObject.Type type, String... names) { - + static SecurableObject of( + SecurableObject.Type type, List names, List privileges) { if (names == null) { throw new IllegalArgumentException("Cannot create a securable object with null names"); } - if (names.length == 0) { + if (names.isEmpty()) { throw new IllegalArgumentException("Cannot create a securable object with no names"); } @@ -244,23 +228,23 @@ static SecurableObject of(SecurableObject.Type type, String... names) { throw new IllegalArgumentException("Cannot create a securable object with no type"); } - if (names.length > 3) { + if (names.size() > 3) { throw new IllegalArgumentException( "Cannot create a securable object with the name length which is greater than 3"); } - if (names.length == 1 + if (names.size() == 1 && type != SecurableObject.Type.CATALOG && type != SecurableObject.Type.METALAKE) { throw new IllegalArgumentException( "If the length of names is 1, it must be the CATALOG or METALAKE type"); } - if (names.length == 2 && type != SecurableObject.Type.SCHEMA) { + if (names.size() == 2 && type != SecurableObject.Type.SCHEMA) { throw new IllegalArgumentException("If the length of names is 2, it must be the SCHEMA type"); } - if (names.length == 3 + if (names.size() == 3 && type != SecurableObject.Type.FILESET && type != SecurableObject.Type.TABLE && type != SecurableObject.Type.TOPIC) { @@ -268,51 +252,23 @@ static SecurableObject of(SecurableObject.Type type, String... names) { "If the length of names is 3, it must be FILESET, TABLE or TOPIC"); } - List types = Lists.newArrayList(type); - - // Find all the types of the parent securable object. - SecurableObject.Type curType = type; - List reverseNames = Lists.newArrayList(names); - Collections.reverse(reverseNames); - for (String ignored : reverseNames) { - types.add(curType); - curType = getParentSecurableObjectType(curType); - } - Collections.reverse(types); - - SecurableObject parent = null; - int level = 0; for (String name : names) { checkName(name); - - parent = new SecurableObjectImpl(parent, name, types.get(level)); - - level++; } - return parent; + return new SecurableObjectImpl(getParentFullName(names), getLastName(names), type, privileges); } - private static SecurableObject.Type getParentSecurableObjectType(SecurableObject.Type type) { - switch (type) { - case FILESET: - return SecurableObject.Type.SCHEMA; - - case TOPIC: - return SecurableObject.Type.SCHEMA; - - case TABLE: - return SecurableObject.Type.SCHEMA; - - case SCHEMA: - return SecurableObject.Type.CATALOG; + private static String getParentFullName(List names) { + if (names.size() <= 1) { + return null; + } - case CATALOG: - return SecurableObject.Type.METALAKE; + return String.join(".", names.subList(0, names.size() - 1)); + } - default: - return null; - } + private static String getLastName(List names) { + return names.get(names.size() - 1); } private static void checkName(String name) { diff --git a/api/src/test/java/com/datastrato/gravitino/authorization/TestSecurableObjects.java b/api/src/test/java/com/datastrato/gravitino/authorization/TestSecurableObjects.java index 7697c896f3d..79f773b164e 100644 --- a/api/src/test/java/com/datastrato/gravitino/authorization/TestSecurableObjects.java +++ b/api/src/test/java/com/datastrato/gravitino/authorization/TestSecurableObjects.java @@ -11,6 +11,7 @@ import static com.datastrato.gravitino.authorization.SecurableObject.Type.TABLE; import static com.datastrato.gravitino.authorization.SecurableObject.Type.TOPIC; +import com.google.common.collect.Lists; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -18,77 +19,143 @@ public class TestSecurableObjects { @Test public void testSecurableObjects() { - SecurableObject allMetalakes = SecurableObjects.ofAllMetalakes(); + SecurableObject allMetalakes = + SecurableObjects.ofAllMetalakes(Lists.newArrayList(Privileges.CreateMetalake.allow())); Assertions.assertEquals("*", allMetalakes.fullName()); Assertions.assertEquals(METALAKE, allMetalakes.type()); + Assertions.assertThrows( - IllegalArgumentException.class, () -> SecurableObjects.of(METALAKE, "*")); + IllegalArgumentException.class, + () -> + SecurableObjects.of( + METALAKE, + Lists.newArrayList("*"), + Lists.newArrayList(Privileges.UseMetalake.allow()))); - SecurableObject metalake = SecurableObjects.ofMetalake("metalake"); + SecurableObject metalake = + SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.UseMetalake.allow())); Assertions.assertEquals("metalake", metalake.fullName()); Assertions.assertEquals(METALAKE, metalake.type()); - SecurableObject anotherMetalake = SecurableObjects.of(METALAKE, "metalake"); + SecurableObject anotherMetalake = + SecurableObjects.of( + METALAKE, + Lists.newArrayList("metalake"), + Lists.newArrayList(Privileges.UseMetalake.allow())); Assertions.assertEquals(metalake, anotherMetalake); - SecurableObject catalog = SecurableObjects.ofCatalog("catalog"); + SecurableObject catalog = + SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.UseCatalog.allow())); Assertions.assertEquals("catalog", catalog.fullName()); Assertions.assertEquals(CATALOG, catalog.type()); - SecurableObject anotherCatalog = SecurableObjects.of(CATALOG, "catalog"); + SecurableObject anotherCatalog = + SecurableObjects.of( + CATALOG, + Lists.newArrayList("catalog"), + Lists.newArrayList(Privileges.UseCatalog.allow())); Assertions.assertEquals(catalog, anotherCatalog); - SecurableObject schema = SecurableObjects.ofSchema(catalog, "schema"); + SecurableObject schema = + SecurableObjects.ofSchema( + catalog, "schema", Lists.newArrayList(Privileges.UseSchema.allow())); Assertions.assertEquals("catalog.schema", schema.fullName()); Assertions.assertEquals(SCHEMA, schema.type()); - SecurableObject anotherSchema = SecurableObjects.of(SCHEMA, "catalog", "schema"); + SecurableObject anotherSchema = + SecurableObjects.of( + SCHEMA, + Lists.newArrayList("catalog", "schema"), + Lists.newArrayList(Privileges.UseSchema.allow())); Assertions.assertEquals(schema, anotherSchema); - SecurableObject table = SecurableObjects.ofTable(schema, "table"); + SecurableObject table = + SecurableObjects.ofTable(schema, "table", Lists.newArrayList(Privileges.ReadTable.allow())); Assertions.assertEquals("catalog.schema.table", table.fullName()); Assertions.assertEquals(TABLE, table.type()); - SecurableObject anotherTable = SecurableObjects.of(TABLE, "catalog", "schema", "table"); + SecurableObject anotherTable = + SecurableObjects.of( + TABLE, + Lists.newArrayList("catalog", "schema", "table"), + Lists.newArrayList(Privileges.ReadTable.allow())); Assertions.assertEquals(table, anotherTable); - SecurableObject fileset = SecurableObjects.ofFileset(schema, "fileset"); + SecurableObject fileset = + SecurableObjects.ofFileset( + schema, "fileset", Lists.newArrayList(Privileges.UseSchema.allow())); Assertions.assertEquals("catalog.schema.fileset", fileset.fullName()); Assertions.assertEquals(FILESET, fileset.type()); - SecurableObject anotherFileset = SecurableObjects.of(FILESET, "catalog", "schema", "fileset"); + SecurableObject anotherFileset = + SecurableObjects.of( + FILESET, + Lists.newArrayList("catalog", "schema", "fileset"), + Lists.newArrayList(Privileges.UseSchema.allow())); Assertions.assertEquals(fileset, anotherFileset); - SecurableObject topic = SecurableObjects.ofTopic(schema, "topic"); + SecurableObject topic = + SecurableObjects.ofTopic(schema, "topic", Lists.newArrayList(Privileges.ReadTopic.allow())); Assertions.assertEquals("catalog.schema.topic", topic.fullName()); Assertions.assertEquals(TOPIC, topic.type()); - SecurableObject anotherTopic = SecurableObjects.of(TOPIC, "catalog", "schema", "topic"); + SecurableObject anotherTopic = + SecurableObjects.of( + TOPIC, + Lists.newArrayList("catalog", "schema", "topic"), + Lists.newArrayList(Privileges.ReadTopic.allow())); Assertions.assertEquals(topic, anotherTopic); Exception e = Assertions.assertThrows( IllegalArgumentException.class, - () -> SecurableObjects.of(METALAKE, "metalake", "catalog")); + () -> + SecurableObjects.of( + METALAKE, + Lists.newArrayList("metalake", "catalog"), + Lists.newArrayList(Privileges.UseCatalog.allow()))); Assertions.assertTrue(e.getMessage().contains("length of names is 2")); e = Assertions.assertThrows( IllegalArgumentException.class, - () -> SecurableObjects.of(CATALOG, "metalake", "catalog")); + () -> + SecurableObjects.of( + CATALOG, + Lists.newArrayList("metalake", "catalog"), + Lists.newArrayList(Privileges.UseCatalog.allow()))); Assertions.assertTrue(e.getMessage().contains("length of names is 2")); e = Assertions.assertThrows( - IllegalArgumentException.class, () -> SecurableObjects.of(TABLE, "metalake")); + IllegalArgumentException.class, + () -> + SecurableObjects.of( + TABLE, + Lists.newArrayList("metalake"), + Lists.newArrayList(Privileges.ReadTable.allow()))); Assertions.assertTrue(e.getMessage().contains("the length of names is 1")); e = Assertions.assertThrows( - IllegalArgumentException.class, () -> SecurableObjects.of(TOPIC, "metalake")); + IllegalArgumentException.class, + () -> + SecurableObjects.of( + TOPIC, + Lists.newArrayList("metalake"), + Lists.newArrayList(Privileges.ReadTopic.allow()))); Assertions.assertTrue(e.getMessage().contains("the length of names is 1")); e = Assertions.assertThrows( - IllegalArgumentException.class, () -> SecurableObjects.of(FILESET, "metalake")); + IllegalArgumentException.class, + () -> + SecurableObjects.of( + FILESET, + Lists.newArrayList("metalake"), + Lists.newArrayList(Privileges.ReadFileset.allow()))); Assertions.assertTrue(e.getMessage().contains("the length of names is 1")); e = Assertions.assertThrows( IllegalArgumentException.class, - () -> SecurableObjects.of(SCHEMA, "catalog", "schema", "table")); + () -> + SecurableObjects.of( + SCHEMA, + Lists.newArrayList("catalog", "schema", "table"), + Lists.newArrayList(Privileges.UseSchema.allow()))); Assertions.assertTrue(e.getMessage().contains("the length of names is 3")); } } diff --git a/clients/client-java/src/main/java/com/datastrato/gravitino/client/DTOConverters.java b/clients/client-java/src/main/java/com/datastrato/gravitino/client/DTOConverters.java index c6dc1651032..b12af8ba85f 100644 --- a/clients/client-java/src/main/java/com/datastrato/gravitino/client/DTOConverters.java +++ b/clients/client-java/src/main/java/com/datastrato/gravitino/client/DTOConverters.java @@ -13,6 +13,7 @@ import com.datastrato.gravitino.dto.AuditDTO; import com.datastrato.gravitino.dto.CatalogDTO; import com.datastrato.gravitino.dto.MetalakeDTO; +import com.datastrato.gravitino.dto.authorization.PrivilegeDTO; import com.datastrato.gravitino.dto.authorization.SecurableObjectDTO; import com.datastrato.gravitino.dto.requests.CatalogUpdateRequest; import com.datastrato.gravitino.dto.requests.FilesetUpdateRequest; @@ -278,6 +279,16 @@ static SecurableObjectDTO toSecurableObject(SecurableObject securableObject) { return SecurableObjectDTO.builder() .withFullName(securableObject.fullName()) .withType(securableObject.type()) + .withPrivileges( + securableObject.privileges().stream() + .map( + privilege -> { + return PrivilegeDTO.builder() + .withCondition(privilege.condition()) + .withName(privilege.name()) + .build(); + }) + .toArray(PrivilegeDTO[]::new)) .build(); } } 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 5f52cbb1519..efa06c6008b 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 @@ -7,10 +7,10 @@ import com.datastrato.gravitino.MetalakeChange; 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; @@ -41,7 +41,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; @@ -426,8 +425,7 @@ public boolean deleteRole(String metalake, String role) throws NoSuchMetalakeExc * @param metalake The Metalake of the Role. * @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. + * @param securableObjects The securable objects 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. @@ -437,18 +435,15 @@ public Role createRole( String metalake, String role, Map properties, - SecurableObject securableObject, - List privileges) + List securableObjects) throws RoleAlreadyExistsException, NoSuchMetalakeException { RoleCreateRequest req = new RoleCreateRequest( role, properties, - privileges.stream() - .map(Privilege::name) - .map(Objects::toString) - .collect(Collectors.toList()), - DTOConverters.toSecurableObject(securableObject)); + securableObjects.stream() + .map(DTOConverters::toSecurableObject) + .toArray(SecurableObjectDTO[]::new)); 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..8af1dd94583 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,29 @@ 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", 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()); + Lists.newArrayList(securableObject)); + Assertions.assertEquals(1L, Privileges.CreateCatalog.allow().name().getLowBits()); + Assertions.assertEquals(0L, Privileges.CreateCatalog.allow().name().getHighBits()); Assertions.assertNotNull(createdRole); assertRole(createdRole, mockRole); @@ -84,8 +90,7 @@ public void testCreateRoles() throws Exception { metalakeName, roleName, ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()))); + Lists.newArrayList(securableObject))); Assertions.assertEquals("role already exists", ex.getMessage()); // test NoSuchMetalakeException @@ -100,8 +105,7 @@ public void testCreateRoles() throws Exception { metalakeName, roleName, ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()))); + Lists.newArrayList(securableObject))); Assertions.assertEquals("metalake not found", ex.getMessage()); // test RuntimeException @@ -114,8 +118,7 @@ public void testCreateRoles() throws Exception { metalakeName, roleName, ImmutableMap.of("k1", "v1"), - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get())), + Lists.newArrayList(securableObject)), "internal error"); } @@ -158,12 +161,11 @@ 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.CATALOG, testParentRole.securableObject().parent().type()); + SecurableObject.Type.SCHEMA, testParentRole.securableObjects().get(0).type()); + Assertions.assertEquals("catalog", testParentRole.securableObjects().get(0).parent()); + Assertions.assertEquals("catalog", testParentRole.securableObjects().get(0).parent()); } @Test @@ -188,31 +190,49 @@ public void testDeleteRoles() throws Exception { } private RoleDTO mockRoleDTO(String name) { + SecurableObject securableObject = + SecurableObjects.ofCatalog("catalog", 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( + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), + "schema", + 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().size(), + actual.securableObjects().get(0).privileges().size()); + SecurableObject expectObject = expected.securableObjects().get(0); + SecurableObject actualObject = actual.securableObjects().get(0); + int size = expectObject.privileges().size(); + for (int index = 0; index < size; index++) { + Assertions.assertEquals( + expectObject.privileges().get(0).name(), actualObject.privileges().get(0).name()); + Assertions.assertEquals( + expectObject.privileges().get(0).condition(), + actualObject.privileges().get(0).condition()); + } + 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..b9e65a06fe2 --- /dev/null +++ b/common/src/main/java/com/datastrato/gravitino/dto/authorization/PrivilegeDTO.java @@ -0,0 +1,99 @@ +/* + * 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; +import com.google.common.base.Preconditions; + +/** Data transfer object representing a privilege. */ +public class PrivilegeDTO implements Privilege { + + @JsonProperty("name") + private Name name; + + @JsonProperty("condition") + private Condition condition; + + /** Default constructor for Jackson deserialization. */ + protected PrivilegeDTO() {} + + /** + * Creates a new instance of PrivilegeDTO. + * + * @param name The name of the Privilege DTO. + * @param condition The condition of the Privilege DTO. + */ + protected PrivilegeDTO(Name name, Condition condition) { + this.name = name; + this.condition = condition; + } + + @Override + public Name name() { + return name; + } + + @Override + public String simpleString() { + if (Condition.ALLOW.equals(condition)) { + return Privileges.allow(name).simpleString(); + } else { + return Privileges.deny(name).simpleString(); + } + } + + @Override + public Condition condition() { + return condition; + } + + /** @return the builder for creating a new instance of PrivilegeDTO. */ + public static Builder builder() { + return new Builder(); + } + + /** Builder for {@link PrivilegeDTO}. */ + public static class Builder { + + private Name name; + private Condition condition; + + /** + * Sets the name of the privilege. + * + * @param name The name of the privilege. + * @return The builder instance. + */ + public Builder withName(Name name) { + this.name = name; + return this; + } + + /** + * Sets the condition of the privilege. + * + * @param condition The condition of the privilege. + * @return The builder instance. + */ + public Builder withCondition(Condition condition) { + this.condition = condition; + return this; + } + + /** + * Builds an instance of PrivilegeDTO using the builder's properties. + * + * @return An instance of PrivilegeDTO. + * @throws IllegalArgumentException If the name or condition are not set. + */ + public PrivilegeDTO build() { + Preconditions.checkArgument(name != null, "name cannot be null"); + Preconditions.checkArgument(condition != null, "condition cannot be null"); + return new PrivilegeDTO(name, condition); + } + } +} 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..5105b1e68f0 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.Collections; +import java.util.List; import javax.annotation.Nullable; import org.apache.commons.lang3.StringUtils; @@ -20,29 +23,41 @@ public class SecurableObjectDTO implements SecurableObject { @JsonProperty("type") private Type type; - private SecurableObject parent; + @JsonProperty("privileges") + private PrivilegeDTO[] privileges; + + private String parent; private String name; /** Default constructor for Jackson deserialization. */ protected SecurableObjectDTO() {} /** - * Creates a new instance of RoleDTO. + * Creates a new instance of SecurableObject DTO. * - * @param fullName The name of the Role DTO. + * @param privileges The privileges of the ScecurableObject DTO. + * @param fullName The name of the SecurableObject DTO. * @param type The type of the securable object. */ - protected SecurableObjectDTO(String fullName, Type type) { - SecurableObject securableObject = SecurableObjects.parse(fullName, type); + protected SecurableObjectDTO(String fullName, Type type, PrivilegeDTO[] privileges) { this.type = type; this.fullName = fullName; - this.parent = securableObject.parent(); - this.name = securableObject.name(); + int index = fullName.lastIndexOf("."); + + if (index == -1) { + this.parent = null; + this.name = fullName; + } else { + this.parent = fullName.substring(0, index); + this.name = fullName.substring(index + 1); + } + + this.privileges = privileges; } @Nullable @Override - public SecurableObject parent() { + public String parent() { return parent; } @@ -61,6 +76,15 @@ public Type type() { return type; } + @Override + public List privileges() { + if (privileges == null) { + return Collections.emptyList(); + } + + return Collections.unmodifiableList(Arrays.asList(privileges)); + } + /** @return the builder for creating a new instance of SecurableObjectDTO. */ public static Builder builder() { return new Builder(); @@ -70,6 +94,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 +118,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 +141,12 @@ public SecurableObjectDTO build() { Preconditions.checkArgument(type != null, "type cannot be null"); - return new SecurableObjectDTO(fullName, type); + Preconditions.checkArgument( + privileges != null && privileges.length != 0, "privileges can't be null or empty"); + + SecurableObjectDTO object = new SecurableObjectDTO(fullName, type, 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..55da063d54a 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,6 +421,27 @@ 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(); + } + + /** + * Converts a privilege implementation to a PrivilegeDTO. + * + * @param privilege The privilege implementation. + * @return The privilege DTO. + */ + public static PrivilegeDTO toDTO(Privilege privilege) { + if (privilege instanceof PrivilegeDTO) { + return (PrivilegeDTO) privilege; + } + + return PrivilegeDTO.builder() + .withName(privilege.name()) + .withCondition(privilege.condition()) .build(); } 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..80c9a5addcc 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", 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..e0add5b3f03 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()); + .addAllPrivilegeConditions( + roleEntity.securableObjects().get(0).privileges().stream() + .map(privilege -> privilege.condition().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) { + List privileges = Lists.newArrayList(); + + if (!role.getPrivilegesList().isEmpty()) { + + for (int index = 0; index < role.getPrivilegeConditionsCount(); index++) { + if (Privilege.Condition.ALLOW.name().equals(role.getPrivilegeConditions(index))) { + privileges.add(Privileges.allow(role.getPrivileges(index))); + } else { + privileges.add(Privileges.deny(role.getPrivileges(index))); + } + } + } + + SecurableObject securableObject = + SecurableObjects.parse( + role.getSecurableObjectFullName(), + SecurableObject.Type.valueOf(role.getSecurableObjectType()), + 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..04067a814e5 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_conditions as privilegeConditions," + " 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_conditions as privilegeConditions," + " 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_conditions as privilegeConditions," + " 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_conditions," + " audit_info, current_version, last_version, deleted_at)" + " VALUES(" + " #{roleMeta.roleId}," @@ -99,6 +103,7 @@ Long selectRoleIdByMetalakeIdAndName( + " #{roleMeta.securableObjectFullName}," + " #{roleMeta.securableObjectType}," + " #{roleMeta.privileges}," + + " #{roleMeta.privilegeConditions}," + " #{roleMeta.auditInfo}," + " #{roleMeta.currentVersion}," + " #{roleMeta.lastVersion}," @@ -114,6 +119,7 @@ Long selectRoleIdByMetalakeIdAndName( + " securable_object_full_name," + " securable_object_type," + " privileges," + + " privilege_conditions," + " audit_info, current_version, last_version, deleted_at)" + " VALUES(" + " #{roleMeta.roleId}," @@ -123,6 +129,7 @@ Long selectRoleIdByMetalakeIdAndName( + " #{roleMeta.securableObjectFullName}," + " #{roleMeta.securableObjectType}," + " #{roleMeta.privileges}," + + " #{roleMeta.privilegeConditions}," + " #{roleMeta.auditInfo}," + " #{roleMeta.currentVersion}," + " #{roleMeta.lastVersion}," @@ -134,6 +141,7 @@ Long selectRoleIdByMetalakeIdAndName( + " securable_object_full_name = #{roleMeta.securableObjectFullName}," + " securable_object_type = #{roleMeta.securableObjectType}," + " privileges = #{roleMeta.privileges}," + + " privilege_conditions = #{roleMeta.privilegeConditions}," + " 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..82d8ba6373a 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 privilegeConditions; private String auditInfo; private Long currentVersion; private Long lastVersion; @@ -48,6 +49,10 @@ public String getPrivileges() { return privileges; } + public String getPrivilegeConditions() { + return privilegeConditions; + } + 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(getPrivilegeConditions(), tablePO.getPrivilegeConditions()); } @Override @@ -99,7 +105,8 @@ public int hashCode() { getAuditInfo(), getCurrentVersion(), getLastVersion(), - getDeletedAt()); + getDeletedAt(), + getPrivilegeConditions()); } public static class Builder { @@ -144,6 +151,11 @@ public Builder withPrivileges(String privileges) { return this; } + public Builder withPrivilegeConditions(String privilegeConditions) { + rolePO.privilegeConditions = privilegeConditions; + return this; + } + public Builder withAuditInfo(String auditInfo) { rolePO.auditInfo = auditInfo; return this; @@ -172,7 +184,9 @@ private void validate() { rolePO.securableObjectFullName != null, "Securable object full name is required"); Preconditions.checkArgument( rolePO.securableObjectType != null, "Securable object type is required"); - Preconditions.checkArgument(rolePO.privileges != null, "Privileges is required"); + Preconditions.checkArgument(rolePO.privileges != null, "Privileges are required"); + Preconditions.checkArgument( + rolePO.privilegeConditions != null, "Privilege conditions are 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..eb89e8a64de 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()))) + .withPrivilegeConditions( + JsonUtils.anyFieldMapper() + .writeValueAsString( + roleEntity.securableObjects().get(0).privileges().stream() + .map(privilege -> privilege.condition().toString()) + .collect(Collectors.toList()))) .withAuditInfo(JsonUtils.anyFieldMapper().writeValueAsString(roleEntity.auditInfo())) .withCurrentVersion(INIT_VERSION) .withLastVersion(INIT_VERSION) @@ -881,20 +888,35 @@ public static List initializeGroupRoleRelsPOWithVersion( public static RoleEntity fromRolePO(RolePO rolePO, Namespace namespace) { try { + + List privilegeNames = + JsonUtils.anyFieldMapper() + .readValue(rolePO.getPrivileges(), new TypeReference>() {}); + List privilegeConditions = + JsonUtils.anyFieldMapper() + .readValue(rolePO.getPrivilegeConditions(), new TypeReference>() {}); + + List privileges = Lists.newArrayList(); + for (int index = 0; index < privilegeNames.size(); index++) { + if (Privilege.Condition.ALLOW.name().equals(privilegeConditions.get(index))) { + privileges.add(Privileges.allow(privilegeNames.get(index))); + } else { + privileges.add(Privileges.allow(privilegeNames.get(index))); + } + } + + SecurableObject securableObject = + SecurableObjects.parse( + rolePO.getSecurableObjectFullName(), + SecurableObject.Type.valueOf(rolePO.getSecurableObjectType()), + 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..daa78f7ee5f 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManager.java @@ -225,8 +225,8 @@ public void testCreateRole() { "metalake", "create", props, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList()); + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow()))); Assertions.assertEquals("create", role.name()); testProperties(props, role.properties()); @@ -238,8 +238,8 @@ public void testCreateRole() { "metalake", "create", props, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList())); + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())))); } @Test @@ -247,7 +247,10 @@ 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", Lists.newArrayList(Privileges.UseCatalog.allow()))); Role cachedRole = accessControlManager.getRole("metalake", "loadRole"); accessControlManager.getRoleManager().getCache().invalidateAll(); @@ -271,7 +274,10 @@ 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", Lists.newArrayList(Privileges.UseCatalog.allow()))); // 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..5ae68af773c 100644 --- a/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java +++ b/core/src/test/java/com/datastrato/gravitino/authorization/TestAccessControlManagerForPermissions.java @@ -85,8 +85,9 @@ public class TestAccessControlManagerForPermissions { .withId(1L) .withName("role") .withProperties(Maps.newHashMap()) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) - .withSecurableObject(SecurableObjects.ofCatalog(CATALOG)) + .withSecurableObject( + SecurableObjects.ofCatalog( + CATALOG, Lists.newArrayList(Privileges.UseCatalog.allow()))) .withAuditInfo(auditInfo) .build(); @@ -270,8 +271,9 @@ public void testDropRole() throws IOException { .withId(1L) .withName(anotherRole) .withProperties(Maps.newHashMap()) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) - .withSecurableObject(SecurableObjects.ofCatalog(CATALOG)) + .withSecurableObject( + SecurableObjects.ofCatalog( + CATALOG, Lists.newArrayList(Privileges.UseCatalog.allow()))) .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..fdd645526cc 100644 --- a/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java +++ b/core/src/test/java/com/datastrato/gravitino/meta/TestEntity.java @@ -266,8 +266,9 @@ public void testRole() { .withId(1L) .withName(roleName) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject( + SecurableObjects.ofCatalog( + catalogName, Lists.newArrayList(Privileges.UseCatalog.allow()))) .withProperties(map) .build(); @@ -277,17 +278,17 @@ public void testRole() { 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)); + SecurableObjects.ofCatalog(catalogName, Lists.newArrayList(Privileges.UseCatalog.allow())), + fields.get(RoleEntity.SECURABLE_OBJECT)); RoleEntity roleWithoutFields = RoleEntity.builder() .withId(1L) .withName(roleName) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog(catalogName)) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject( + SecurableObjects.ofCatalog( + catalogName, Lists.newArrayList(Privileges.UseCatalog.allow()))) .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..6d11e2ceef6 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,18 @@ 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, + Lists.newArrayList(Privileges.UseCatalog.allow(), Privileges.DropCatalog.deny())); + 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 +406,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..2dbb832c21c 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", 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..520a1e05ee9 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 @@ -228,8 +228,9 @@ public void testEntityStoreAndRetrieve() throws Exception { .withName("role") .withNamespace(AuthorizationUtils.ofRoleNamespace("metalake")) .withAuditInfo(auditInfo) - .withSecurableObject(SecurableObjects.ofCatalog("catalog")) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) + .withSecurableObject( + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow()))) .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..ff7fc638a3c 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 @@ -22,7 +22,6 @@ import com.datastrato.gravitino.Configs; import com.datastrato.gravitino.Entity; 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; @@ -765,14 +764,16 @@ public static UserEntity createUserEntity( public static RoleEntity createRoleEntity( Long id, Namespace namespace, String name, AuditInfo auditInfo) { + SecurableObject securableObject = + SecurableObjects.ofCatalog("catalog", 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(); } @@ -811,8 +812,8 @@ public static RoleEntity createRoleEntity( String name, AuditInfo auditInfo, SecurableObject securableObject, - List privileges, Map properties) { + return RoleEntity.builder() .withId(id) .withName(name) @@ -820,7 +821,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..b2759ef2d47 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 @@ -57,8 +57,8 @@ void getRoleByIdentifier() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role1", auditInfo, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), ImmutableMap.of("k1", "v1")); roleMetaService.insertRole(role1, false); @@ -82,8 +82,8 @@ void insertRole() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role1", auditInfo, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( NoSuchEntityException.class, @@ -98,8 +98,8 @@ void insertRole() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role1", auditInfo, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( AlreadyExistsException.class, () -> roleMetaService.insertRole(role1Exist, false)); @@ -111,8 +111,8 @@ void insertRole() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role1Overwrite", auditInfo, - SecurableObjects.ofCatalog("catalogOverwrite"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalogOverwrite", Lists.newArrayList(Privileges.UseCatalog.allow())), ImmutableMap.of("k2", "v2")); Assertions.assertDoesNotThrow(() -> roleMetaService.insertRole(role1Overwrite, true)); Assertions.assertEquals( @@ -146,8 +146,8 @@ void deleteRole() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role1", auditInfo, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalog", Lists.newArrayList(Privileges.UseCatalog.allow())), ImmutableMap.of("k1", "v1")); Assertions.assertThrows( NoSuchEntityException.class, @@ -166,8 +166,8 @@ void deleteRole() { AuthorizationUtils.ofRoleNamespace(metalakeName), "role2", auditInfo, - SecurableObjects.ofCatalog("catalog"), - Lists.newArrayList(Privileges.UseCatalog.get()), + SecurableObjects.ofCatalog( + "catalog", 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..40cc0f358be 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_conditions` VARCHAR(64) NOT NULL COMMENT 'securable privilege conditions', `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..e26b4fef41a 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_conditions = 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..4146b766382 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_conditions` VARCHAR(64) NOT NULL COMMENT 'securable privilege conditions', `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..147fa42abaa 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; @@ -16,6 +18,7 @@ import com.datastrato.gravitino.dto.util.DTOConverters; import com.datastrato.gravitino.metrics.MetricNames; import com.datastrato.gravitino.server.web.Utils; +import com.google.common.base.Preconditions; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.DELETE; @@ -65,23 +68,43 @@ public Response getRole(@PathParam("metalake") String metalake, @PathParam("role @ResponseMetered(name = "create-role", absolute = true) public Response createRole(@PathParam("metalake") String metalake, RoleCreateRequest request) { try { + // TODO: Supports multiple securable objects. There will be some limited support for multiple + // securable objects in the future. + // The securable objects in the same role should under the same catalog. + // If a role contains a metalake securable object, the role can't contain any other securable + // object. + Preconditions.checkArgument( + request.getSecurableObjects() != null && request.getSecurableObjects().length == 1, + "The size of securable objects must be 1"); 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(), + request.getSecurableObjects()[0].privileges().stream() + .map( + privilege -> { + if (privilege.condition().equals(Privilege.Condition.ALLOW)) { + return Privileges.allow(privilege.name()); + } else { + return Privileges.deny(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..c32e201e85f 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", 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,21 @@ 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", Lists.newArrayList(Privileges.UseCatalog.allow())) + .fullName(), + roleDTO.securableObjects().get(0).fullName()); + Assertions.assertEquals(1, roleDTO.securableObjects().get(0).privileges().size()); + Assertions.assertEquals( + Privileges.UseCatalog.allow().name(), + roleDTO.securableObjects().get(0).privileges().get(0).name()); + Assertions.assertEquals( + Privileges.UseCatalog.allow().condition(), + roleDTO.securableObjects().get(0).privileges().get(0).condition()); // 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 +157,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 +174,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 +209,16 @@ 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", Lists.newArrayList(Privileges.UseCatalog.allow())) + .fullName(), + roleDTO.securableObjects().get(0).fullName()); + Assertions.assertEquals(1, roleDTO.securableObjects().get(0).privileges().size()); + Assertions.assertEquals( + Privileges.UseCatalog.allow().name(), + roleDTO.securableObjects().get(0).privileges().get(0).name()); + Assertions.assertEquals( + Privileges.UseCatalog.allow().condition(), + roleDTO.securableObjects().get(0).privileges().get(0).condition()); // Test to throw NoSuchMetalakeException doThrow(new NoSuchMetalakeException("mock error")).when(manager).getRole(any(), any()); @@ -245,12 +265,13 @@ public void testGetRole() { } private Role buildRole(String role) { + SecurableObject catalog = + SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.UseCatalog.allow())); return RoleEntity.builder() .withId(1L) .withName(role) - .withPrivileges(Lists.newArrayList(Privileges.UseCatalog.get())) .withProperties(Collections.emptyMap()) - .withSecurableObject(SecurableObjects.ofCatalog("catalog")) + .withSecurableObject(catalog) .withAuditInfo( AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) .build();