Add access control check while listing views

trinodb · Apr 1, 2020 · f922c85 · f922c85
1 parent 0c13d16
commit f922c85
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 4 deletions.
diff --git a/presto-hive/src/test/java/io/prestosql/plugin/hive/TestHiveIntegrationSmokeTest.java b/presto-hive/src/test/java/io/prestosql/plugin/hive/TestHiveIntegrationSmokeTest.java
@@ -4733,6 +4733,37 @@ public void testShowColumnMetadata()
         assertUpdate("DROP TABLE " + tableName);
     }
 
+    @Test
+    public void testShowViews()
+    {
+        String viewName = "test_show_views";
+
+        Session testSession = testSessionBuilder()
+                .setIdentity(Identity.ofUser("test_view_access_owner"))
+                .setCatalog(getSession().getCatalog().get())
+                .setSchema(getSession().getSchema().get())
+                .build();
+
+        assertUpdate("CREATE VIEW " + viewName + " AS SELECT abs(1) as whatever");
+
+        String showViews = format("SELECT * FROM information_schema.views WHERE table_name = '%s'", viewName);
+        assertQuery(
+                format("SELECT table_name FROM information_schema.views WHERE table_name = '%s'", viewName),
+                format("VALUES '%s'", viewName));
+
+        executeExclusively(() -> {
+            try {
+                getQueryRunner().getAccessControl().denyTables(table -> false);
+                assertQueryReturnsEmptyResult(testSession, showViews);
+            }
+            finally {
+                getQueryRunner().getAccessControl().reset();
+            }
+        });
+
+        assertUpdate("DROP VIEW " + viewName);
+    }
+
     @Test
     public void testShowTablePrivileges()
     {

diff --git a/...n/src/main/java/io/prestosql/connector/informationschema/InformationSchemaPageSource.java b/...n/src/main/java/io/prestosql/connector/informationschema/InformationSchemaPageSource.java
@@ -17,7 +17,6 @@
 import com.google.common.collect.ImmutableList;
 import io.prestosql.Session;
 import io.prestosql.metadata.Metadata;
-import io.prestosql.metadata.QualifiedObjectName;
 import io.prestosql.metadata.QualifiedTablePrefix;
 import io.prestosql.security.AccessControl;
 import io.prestosql.spi.Page;
@@ -53,6 +52,7 @@
 import static com.google.common.collect.Sets.union;
 import static io.prestosql.connector.informationschema.InformationSchemaMetadata.defaultPrefixes;
 import static io.prestosql.connector.informationschema.InformationSchemaMetadata.isTablesEnumeratingTable;
+import static io.prestosql.metadata.MetadataListing.getViews;
 import static io.prestosql.metadata.MetadataListing.listSchemas;
 import static io.prestosql.metadata.MetadataListing.listTableColumns;
 import static io.prestosql.metadata.MetadataListing.listTablePrivileges;
@@ -288,11 +288,11 @@ private void addTablesRecords(QualifiedTablePrefix prefix)
 
     private void addViewsRecords(QualifiedTablePrefix prefix)
     {
-        for (Map.Entry<QualifiedObjectName, ConnectorViewDefinition> entry : metadata.getViews(session, prefix).entrySet()) {
+        for (Map.Entry<SchemaTableName, ConnectorViewDefinition> entry : getViews(session, metadata, accessControl, prefix).entrySet()) {
             addRecord(
-                    entry.getKey().getCatalogName(),
+                    prefix.getCatalogName(),
                     entry.getKey().getSchemaName(),
-                    entry.getKey().getObjectName(),
+                    entry.getKey().getTableName(),
                     entry.getValue().getOriginalSql());
             if (isLimitExhausted()) {
                 return;

diff --git a/presto-main/src/main/java/io/prestosql/metadata/MetadataListing.java b/presto-main/src/main/java/io/prestosql/metadata/MetadataListing.java
@@ -22,6 +22,7 @@
 import io.prestosql.security.AccessControl;
 import io.prestosql.spi.connector.CatalogSchemaTableName;
 import io.prestosql.spi.connector.ColumnMetadata;
+import io.prestosql.spi.connector.ConnectorViewDefinition;
 import io.prestosql.spi.connector.SchemaTableName;
 import io.prestosql.spi.security.GrantInfo;
 
@@ -75,6 +76,18 @@ public static Set<SchemaTableName> listViews(Session session, Metadata metadata,
         return accessControl.filterTables(session.toSecurityContext(), prefix.getCatalogName(), tableNames);
     }
 
+    public static Map<SchemaTableName, ConnectorViewDefinition> getViews(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
+    {
+        Map<SchemaTableName, ConnectorViewDefinition> views = metadata.getViews(session, prefix).entrySet().stream()
+                .collect(toImmutableMap(entry -> entry.getKey().asSchemaTableName(), Entry::getValue));
+
+        Set<SchemaTableName> accessible = accessControl.filterTables(session.toSecurityContext(), prefix.getCatalogName(), views.keySet());
+
+        return views.entrySet().stream()
+                .filter(entry -> accessible.contains(entry.getKey()))
+                .collect(toImmutableMap(Entry::getKey, Entry::getValue));
+    }
+
     public static Set<GrantInfo> listTablePrivileges(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix)
     {
         List<GrantInfo> grants = metadata.listTablePrivileges(session, prefix);

diff --git a/presto-main/src/main/java/io/prestosql/testing/TestingAccessControlManager.java b/presto-main/src/main/java/io/prestosql/testing/TestingAccessControlManager.java
@@ -20,6 +20,7 @@
 import io.prestosql.security.SecurityContext;
 import io.prestosql.spi.connector.CatalogSchemaName;
 import io.prestosql.spi.connector.CatalogSchemaTableName;
+import io.prestosql.spi.connector.SchemaTableName;
 import io.prestosql.spi.security.Identity;
 import io.prestosql.spi.security.ViewExpression;
 import io.prestosql.spi.type.Type;
@@ -112,6 +113,7 @@ public class TestingAccessControlManager
     private final Map<RowFilterKey, List<ViewExpression>> rowFilters = new HashMap<>();
     private final Map<ColumnMaskKey, List<ViewExpression>> columnMasks = new HashMap<>();
     private Predicate<String> deniedCatalogs = s -> true;
+    private Predicate<SchemaTableName> deniedTables = s -> true;
 
     @Inject
     public TestingAccessControlManager(TransactionManager transactionManager)
@@ -155,6 +157,7 @@ public void reset()
     {
         denyPrivileges.clear();
         deniedCatalogs = s -> true;
+        deniedTables = s -> true;
         rowFilters.clear();
         columnMasks.clear();
     }
@@ -164,6 +167,11 @@ public void denyCatalogs(Predicate<String> deniedCatalogs)
         this.deniedCatalogs = this.deniedCatalogs.and(deniedCatalogs);
     }
 
+    public void denyTables(Predicate<SchemaTableName> deniedTables)
+    {
+        this.deniedTables = this.deniedTables.and(deniedTables);
+    }
+
     @Override
     public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
     {
@@ -174,6 +182,17 @@ public Set<String> filterCatalogs(Identity identity, Set<String> catalogs)
                         .collect(toImmutableSet()));
     }
 
+    @Override
+    public Set<SchemaTableName> filterTables(SecurityContext context, String catalogName, Set<SchemaTableName> tableNames)
+    {
+        return super.filterTables(
+                context,
+                catalogName,
+                tableNames.stream()
+                        .filter(this.deniedTables)
+                        .collect(toImmutableSet()));
+    }
+
     @Override
     public void checkCanImpersonateUser(Identity identity, String userName)
     {