Accept ROLE in GRANT/REVOKE Privileges statements

Extracted-From: prestodb/presto#10904
trinodb · Jan 29, 2019 · 34ffbaa · 34ffbaa
1 parent be8189c
commit 34ffbaa
Show file tree

Hide file tree

Showing 37 changed files with 193 additions and 112 deletions.
diff --git a/presto-docs/src/main/sphinx/sql/grant.rst b/presto-docs/src/main/sphinx/sql/grant.rst
@@ -8,11 +8,9 @@ Synopsis
 .. code-block:: none
 
     GRANT ( privilege [, ...] | ( ALL PRIVILEGES ) )
-    ON [ TABLE ] table_name TO ( grantee | PUBLIC )
+    ON [ TABLE ] table_name TO ( user | USER user | ROLE role )
     [ WITH GRANT OPTION ]
 
-Usage of the term ``grantee`` denotes both users and roles.
-
 Description
 -----------
 
@@ -39,7 +37,7 @@ Grant ``SELECT`` privilege on the table ``nation`` to user ``alice``, additional
 
 Grant ``SELECT`` privilege on the table ``orders`` to everyone::
 
-    GRANT SELECT ON orders TO PUBLIC;
+    GRANT SELECT ON orders TO ROLE PUBLIC;
 
 Limitations
 -----------

diff --git a/presto-docs/src/main/sphinx/sql/revoke.rst b/presto-docs/src/main/sphinx/sql/revoke.rst
@@ -9,9 +9,7 @@ Synopsis
 
     REVOKE [ GRANT OPTION FOR ]
     ( privilege [, ...] | ALL PRIVILEGES )
-    ON [ TABLE ] table_name FROM ( grantee | PUBLIC )
-
-Usage of the term ``grantee`` denotes both users and roles.
+    ON [ TABLE ] table_name FROM ( user | USER user | ROLE role )
 
 Description
 -----------
@@ -35,7 +33,7 @@ Revoke ``INSERT`` and ``SELECT`` privileges on the table ``orders`` from user ``
 
 Revoke ``SELECT`` privilege on the table ``nation`` from everyone, additionally revoking the privilege to grant ``SELECT`` privilege::
 
-    REVOKE GRANT OPTION FOR SELECT ON nation FROM PUBLIC;
+    REVOKE GRANT OPTION FOR SELECT ON nation FROM ROLE PUBLIC;
 
 Revoke all privileges on the table ``test`` from user ``alice``::
 

diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/HiveMetadata.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/HiveMetadata.java
@@ -195,7 +195,6 @@
 import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED;
 import static io.prestosql.spi.StandardErrorCode.SCHEMA_NOT_EMPTY;
 import static io.prestosql.spi.predicate.TupleDomain.withColumnDomains;
-import static io.prestosql.spi.security.PrincipalType.ROLE;
 import static io.prestosql.spi.security.PrincipalType.USER;
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
@@ -1809,7 +1808,7 @@ public Set<String> listEnabledRoles(ConnectorSession session)
     }
 
     @Override
-    public void grantTablePrivileges(ConnectorSession session, SchemaTableName schemaTableName, Set<Privilege> privileges, String grantee, boolean grantOption)
+    public void grantTablePrivileges(ConnectorSession session, SchemaTableName schemaTableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption)
     {
         String schemaName = schemaTableName.getSchemaName();
         String tableName = schemaTableName.getTableName();
@@ -1818,11 +1817,11 @@ public void grantTablePrivileges(ConnectorSession session, SchemaTableName schem
                 .map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption))
                 .collect(toSet());
 
-        metastore.grantTablePrivileges(schemaName, tableName, getPrestoPrincipal(grantee), hivePrivilegeInfos);
+        metastore.grantTablePrivileges(schemaName, tableName, grantee, hivePrivilegeInfos);
     }
 
     @Override
-    public void revokeTablePrivileges(ConnectorSession session, SchemaTableName schemaTableName, Set<Privilege> privileges, String grantee, boolean grantOption)
+    public void revokeTablePrivileges(ConnectorSession session, SchemaTableName schemaTableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption)
     {
         String schemaName = schemaTableName.getSchemaName();
         String tableName = schemaTableName.getTableName();
@@ -1831,18 +1830,7 @@ public void revokeTablePrivileges(ConnectorSession session, SchemaTableName sche
                 .map(privilege -> new HivePrivilegeInfo(toHivePrivilege(privilege), grantOption))
                 .collect(toSet());
 
-        metastore.revokeTablePrivileges(schemaName, tableName, getPrestoPrincipal(grantee), hivePrivilegeInfos);
-    }
-
-    private PrestoPrincipal getPrestoPrincipal(String grantee)
-    {
-        // TODO this hack will be removed after grant for roles is introduced
-        if (grantee.equalsIgnoreCase("public")) {
-            return new PrestoPrincipal(ROLE, "public");
-        }
-        else {
-            return new PrestoPrincipal(USER, grantee);
-        }
+        metastore.revokeTablePrivileges(schemaName, tableName, grantee, hivePrivilegeInfos);
     }
 
     @Override

diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/security/LegacyAccessControl.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/security/LegacyAccessControl.java
@@ -20,6 +20,7 @@
 import io.prestosql.spi.connector.ConnectorTransactionHandle;
 import io.prestosql.spi.connector.SchemaTableName;
 import io.prestosql.spi.security.ConnectorIdentity;
+import io.prestosql.spi.security.PrestoPrincipal;
 import io.prestosql.spi.security.Privilege;
 
 import javax.inject.Inject;
@@ -188,12 +189,12 @@ public void checkCanSetCatalogSessionProperty(ConnectorTransactionHandle transac
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
     }
 }
diff --git a/...to-hive/src/main/java/io/prestosql/plugin/hive/security/PartitionsAwareAccessControl.java b/...to-hive/src/main/java/io/prestosql/plugin/hive/security/PartitionsAwareAccessControl.java
@@ -177,13 +177,13 @@ public void checkCanSetCatalogSessionProperty(ConnectorTransactionHandle transac
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
         delegate.checkCanGrantTablePrivilege(transactionHandle, identity, privilege, tableName, grantee, withGrantOption);
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
         delegate.checkCanRevokeTablePrivilege(transactionHandle, identity, privilege, tableName, revokee, grantOptionFor);
     }

diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/security/SqlStandardAccessControl.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/security/SqlStandardAccessControl.java
@@ -247,7 +247,7 @@ public void checkCanSetCatalogSessionProperty(ConnectorTransactionHandle transac
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
         if (checkTablePermission(transaction, identity, tableName, OWNERSHIP)) {
             return;
@@ -260,7 +260,7 @@ public void checkCanGrantTablePrivilege(ConnectorTransactionHandle transaction,
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(ConnectorTransactionHandle transaction, ConnectorIdentity identity, Privilege privilege, SchemaTableName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
         if (checkTablePermission(transaction, identity, tableName, OWNERSHIP)) {
             return;

diff --git a/presto-main/src/main/java/io/prestosql/execution/GrantTask.java b/presto-main/src/main/java/io/prestosql/execution/GrantTask.java
@@ -32,6 +32,7 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.util.concurrent.Futures.immediateFuture;
+import static io.prestosql.metadata.MetadataUtil.createPrincipal;
 import static io.prestosql.metadata.MetadataUtil.createQualifiedObjectName;
 import static io.prestosql.sql.analyzer.SemanticErrorCode.INVALID_PRIVILEGE;
 import static io.prestosql.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
@@ -68,10 +69,10 @@ public ListenableFuture<?> execute(Grant statement, TransactionManager transacti
 
         // verify current identity has permissions to grant permissions
         for (Privilege privilege : privileges) {
-            accessControl.checkCanGrantTablePrivilege(session.getRequiredTransactionId(), session.getIdentity(), privilege, tableName, statement.getGrantee().getValue(), statement.isWithGrantOption());
+            accessControl.checkCanGrantTablePrivilege(session.getRequiredTransactionId(), session.getIdentity(), privilege, tableName, createPrincipal(statement.getGrantee()), statement.isWithGrantOption());
         }
 
-        metadata.grantTablePrivileges(session, tableName, privileges, statement.getGrantee().getValue(), statement.isWithGrantOption());
+        metadata.grantTablePrivileges(session, tableName, privileges, createPrincipal(statement.getGrantee()), statement.isWithGrantOption());
         return immediateFuture(null);
     }
 

diff --git a/presto-main/src/main/java/io/prestosql/execution/RevokeTask.java b/presto-main/src/main/java/io/prestosql/execution/RevokeTask.java
@@ -32,6 +32,7 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.util.concurrent.Futures.immediateFuture;
+import static io.prestosql.metadata.MetadataUtil.createPrincipal;
 import static io.prestosql.metadata.MetadataUtil.createQualifiedObjectName;
 import static io.prestosql.sql.analyzer.SemanticErrorCode.INVALID_PRIVILEGE;
 import static io.prestosql.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
@@ -68,10 +69,10 @@ public ListenableFuture<?> execute(Revoke statement, TransactionManager transact
 
         // verify current identity has permissions to revoke permissions
         for (Privilege privilege : privileges) {
-            accessControl.checkCanRevokeTablePrivilege(session.getRequiredTransactionId(), session.getIdentity(), privilege, tableName, statement.getGrantee().getValue(), statement.isGrantOptionFor());
+            accessControl.checkCanRevokeTablePrivilege(session.getRequiredTransactionId(), session.getIdentity(), privilege, tableName, createPrincipal(statement.getGrantee()), statement.isGrantOptionFor());
         }
 
-        metadata.revokeTablePrivileges(session, tableName, privileges, statement.getGrantee().getValue(), statement.isGrantOptionFor());
+        metadata.revokeTablePrivileges(session, tableName, privileges, createPrincipal(statement.getGrantee()), statement.isGrantOptionFor());
         return immediateFuture(null);
     }
 

diff --git a/presto-main/src/main/java/io/prestosql/metadata/Metadata.java b/presto-main/src/main/java/io/prestosql/metadata/Metadata.java
@@ -329,12 +329,12 @@ public interface Metadata
     /**
      * Grants the specified privilege to the specified user on the specified table
      */
-    void grantTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, String grantee, boolean grantOption);
+    void grantTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption);
 
     /**
      * Revokes the specified privilege on the specified table from the specified user
      */
-    void revokeTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, String grantee, boolean grantOption);
+    void revokeTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption);
 
     /**
      * Gets the privileges for the specified table available to the given grantee

diff --git a/presto-main/src/main/java/io/prestosql/metadata/MetadataManager.java b/presto-main/src/main/java/io/prestosql/metadata/MetadataManager.java
@@ -962,7 +962,7 @@ public Set<String> listEnabledRoles(Session session, String catalog)
     }
 
     @Override
-    public void grantTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, String grantee, boolean grantOption)
+    public void grantTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption)
     {
         CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, tableName.getCatalogName());
         ConnectorId connectorId = catalogMetadata.getConnectorId();
@@ -972,7 +972,7 @@ public void grantTablePrivileges(Session session, QualifiedObjectName tableName,
     }
 
     @Override
-    public void revokeTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, String grantee, boolean grantOption)
+    public void revokeTablePrivileges(Session session, QualifiedObjectName tableName, Set<Privilege> privileges, PrestoPrincipal grantee, boolean grantOption)
     {
         CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, tableName.getCatalogName());
         ConnectorId connectorId = catalogMetadata.getConnectorId();

diff --git a/presto-main/src/main/java/io/prestosql/security/AccessControl.java b/presto-main/src/main/java/io/prestosql/security/AccessControl.java
@@ -179,14 +179,14 @@ public interface AccessControl
      *
      * @throws io.prestosql.spi.security.AccessDeniedException if not allowed
      */
-    void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String grantee, boolean withGrantOption);
+    void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal grantee, boolean withGrantOption);
 
     /**
      * Check if identity is allowed to revoke a privilege from the revokee on the specified table.
      *
      * @throws io.prestosql.spi.security.AccessDeniedException if not allowed
      */
-    void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String revokee, boolean grantOptionFor);
+    void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal revokee, boolean grantOptionFor);
 
     /**
      * Check if identity is allowed to set the specified system property.

diff --git a/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java b/presto-main/src/main/java/io/prestosql/security/AccessControlManager.java
@@ -466,7 +466,7 @@ public void checkCanCreateViewWithSelectFromColumns(TransactionId transactionId,
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
         requireNonNull(identity, "identity is null");
         requireNonNull(tableName, "tableName is null");
@@ -483,7 +483,7 @@ public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity id
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
         requireNonNull(identity, "identity is null");
         requireNonNull(tableName, "tableName is null");

diff --git a/presto-main/src/main/java/io/prestosql/security/AllowAllAccessControl.java b/presto-main/src/main/java/io/prestosql/security/AllowAllAccessControl.java
@@ -137,12 +137,12 @@ public void checkCanCreateViewWithSelectFromColumns(TransactionId transactionId,
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
     }
 

diff --git a/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java b/presto-main/src/main/java/io/prestosql/security/AllowAllSystemAccessControl.java
@@ -17,6 +17,7 @@
 import io.prestosql.spi.connector.CatalogSchemaTableName;
 import io.prestosql.spi.connector.SchemaTableName;
 import io.prestosql.spi.security.Identity;
+import io.prestosql.spi.security.PrestoPrincipal;
 import io.prestosql.spi.security.Privilege;
 import io.prestosql.spi.security.SystemAccessControl;
 import io.prestosql.spi.security.SystemAccessControlFactory;
@@ -178,12 +179,12 @@ public void checkCanSetCatalogSessionProperty(Identity identity, String catalogN
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, PrestoPrincipal grantee, boolean withGrantOption)
     {
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, PrestoPrincipal revokee, boolean grantOptionFor)
     {
     }
 }
diff --git a/presto-main/src/main/java/io/prestosql/security/DenyAllAccessControl.java b/presto-main/src/main/java/io/prestosql/security/DenyAllAccessControl.java
@@ -185,13 +185,13 @@ public void checkCanCreateViewWithSelectFromColumns(TransactionId transactionId,
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal grantee, boolean withGrantOption)
     {
         denyGrantTablePrivilege(privilege.name(), tableName.toString());
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(TransactionId transactionId, Identity identity, Privilege privilege, QualifiedObjectName tableName, PrestoPrincipal revokee, boolean grantOptionFor)
     {
         denyRevokeTablePrivilege(privilege.name(), tableName.toString());
     }

diff --git a/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java b/presto-main/src/main/java/io/prestosql/security/FileBasedSystemAccessControl.java
@@ -23,6 +23,7 @@
 import io.prestosql.spi.connector.CatalogSchemaTableName;
 import io.prestosql.spi.connector.SchemaTableName;
 import io.prestosql.spi.security.Identity;
+import io.prestosql.spi.security.PrestoPrincipal;
 import io.prestosql.spi.security.Privilege;
 import io.prestosql.spi.security.SystemAccessControl;
 import io.prestosql.spi.security.SystemAccessControlFactory;
@@ -303,12 +304,12 @@ public void checkCanSetCatalogSessionProperty(Identity identity, String catalogN
     }
 
     @Override
-    public void checkCanGrantTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, String grantee, boolean withGrantOption)
+    public void checkCanGrantTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, PrestoPrincipal grantee, boolean withGrantOption)
     {
     }
 
     @Override
-    public void checkCanRevokeTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, String revokee, boolean grantOptionFor)
+    public void checkCanRevokeTablePrivilege(Identity identity, Privilege privilege, CatalogSchemaTableName table, PrestoPrincipal revokee, boolean grantOptionFor)
     {
     }
 }