Refactor canCreateView security checks

hasGrantOptionForPrivilege cannot be used in security checks for createView because it doesn't consider the session role. Extracted-From: prestodb/presto#10904
trinodb · Jan 29, 2019 · 53b13c9 · 53b13c9
1 parent 709de9d
commit 53b13c9
Showing 1 changed file with 14 additions and 9 deletions.
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
@@ -188,23 +188,23 @@ public void checkCanRenameColumn(ConnectorTransactionHandle transaction, Connect
     public void checkCanSelectFromColumns(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName, Set<String> columnNames)
     {
         // TODO: Implement column level access control
-        if (!checkTablePermission(transaction, identity, tableName, SELECT)) {
+        if (!checkTablePermission(transaction, identity, tableName, SELECT, false)) {
             denySelectTable(tableName.toString());
         }
     }
 
     @Override
     public void checkCanInsertIntoTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName)
     {
-        if (!checkTablePermission(transaction, identity, tableName, INSERT)) {
+        if (!checkTablePermission(transaction, identity, tableName, INSERT, false)) {
             denyInsertTable(tableName.toString());
         }
     }
 
     @Override
     public void checkCanDeleteFromTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName)
     {
-        if (!checkTablePermission(transaction, identity, tableName, DELETE)) {
+        if (!checkTablePermission(transaction, identity, tableName, DELETE, false)) {
             denyDeleteTable(tableName.toString());
         }
     }
@@ -228,11 +228,10 @@ public void checkCanDropView(ConnectorTransactionHandle transaction, ConnectorId
     @Override
     public void checkCanCreateViewWithSelectFromColumns(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName, Set<String> columnNames)
     {
+        checkCanSelectFromColumns(transaction, identity, tableName, columnNames);
+
         // TODO implement column level access control
-        if (!checkTablePermission(transaction, identity, tableName, SELECT)) {
-            denySelectTable(tableName.toString());
-        }
-        if (!hasGrantOptionForPrivilege(transaction, identity, Privilege.SELECT, tableName)) {
+        if (!checkTablePermission(transaction, identity, tableName, SELECT, true)) {
             denyCreateViewWithSelect(tableName.toString(), identity);
         }
     }
@@ -377,10 +376,15 @@ private boolean isDatabaseOwner(ConnectorTransactionHandle transaction, Connecto
 
     private boolean isTableOwner(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName)
     {
-        return checkTablePermission(transaction, identity, tableName, OWNERSHIP);
+        return checkTablePermission(transaction, identity, tableName, OWNERSHIP, false);
     }
 
-    private boolean checkTablePermission(ConnectorTransactionHandle transaction, ConnectorIdentity identity, SchemaTableName tableName, HivePrivilege requiredPrivilege)
+    private boolean checkTablePermission(
+            ConnectorTransactionHandle transaction,
+            ConnectorIdentity identity,
+            SchemaTableName tableName,
+            HivePrivilege requiredPrivilege,
+            boolean grantOptionRequired)
     {
         if (isAdmin(transaction, identity)) {
             return true;
@@ -397,6 +401,7 @@ private boolean checkTablePermission(ConnectorTransactionHandle transaction, Con
         SemiTransactionalHiveMetastore metastore = metastoreProvider.apply(((HiveTransactionHandle) transaction));
         return listApplicableTablePrivileges(metastore, tableName.getSchemaName(), tableName.getTableName(), new PrestoPrincipal(USER, identity.getUser()))
                 .stream()
+                .filter(privilegeInfo -> !grantOptionRequired || privilegeInfo.isGrantOption())
                 .anyMatch(privilegeInfo -> privilegeInfo.getHivePrivilege().equals(requiredPrivilege));
     }