From 729f646d4d2c03c8f3f778a490c046de5904862f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Kokosi=C5=84ski?= Date: Wed, 25 Jul 2018 11:27:52 +0200 Subject: [PATCH] Fix rewrite SHOW GRANTS as a SELECT query When tables of the same name exist across different schemas, Presto lists privileges of the table from all schemas instead of the single schema mentioned in the SHOW GRANTS query. This commit fixes the issue. Extracted-From: https://github.com/prestodb/presto/pull/10904 --- .../sql/rewrite/ShowQueriesRewrite.java | 20 +++++---- .../io/prestosql/tests/hive/TestRoles.java | 42 +++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/presto-main/src/main/java/io/prestosql/sql/rewrite/ShowQueriesRewrite.java b/presto-main/src/main/java/io/prestosql/sql/rewrite/ShowQueriesRewrite.java index 4d5a01fb301e..ec3a1465524b 100644 --- a/presto-main/src/main/java/io/prestosql/sql/rewrite/ShowQueriesRewrite.java +++ b/presto-main/src/main/java/io/prestosql/sql/rewrite/ShowQueriesRewrite.java @@ -98,6 +98,7 @@ import static io.prestosql.metadata.MetadataUtil.createQualifiedObjectName; import static io.prestosql.spi.StandardErrorCode.INVALID_COLUMN_PROPERTY; import static io.prestosql.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; +import static io.prestosql.sql.ExpressionUtils.combineConjuncts; import static io.prestosql.sql.ParsingUtil.createParsingOptions; import static io.prestosql.sql.QueryUtil.aliased; import static io.prestosql.sql.QueryUtil.aliasedName; @@ -236,16 +237,19 @@ protected Node visitShowGrants(ShowGrants showGrants, Void context) session.getIdentity(), new CatalogSchemaName(catalogName, qualifiedTableName.getSchemaName())); - predicate = Optional.of(equal(identifier("table_name"), new StringLiteral(qualifiedTableName.getObjectName()))); - } - - if (catalogName == null) { - throw new SemanticException(CATALOG_NOT_SPECIFIED, showGrants, "Catalog must be specified when session catalog is not set"); + predicate = Optional.of(combineConjuncts( + equal(identifier("table_schema"), new StringLiteral(qualifiedTableName.getSchemaName())), + equal(identifier("table_name"), new StringLiteral(qualifiedTableName.getObjectName())))); } + else { + if (catalogName == null) { + throw new SemanticException(CATALOG_NOT_SPECIFIED, showGrants, "Catalog must be specified when session catalog is not set"); + } - Set allowedSchemas = listSchemas(session, metadata, accessControl, catalogName); - for (String schema : allowedSchemas) { - accessControl.checkCanShowTablesMetadata(session.getRequiredTransactionId(), session.getIdentity(), new CatalogSchemaName(catalogName, schema)); + Set allowedSchemas = listSchemas(session, metadata, accessControl, catalogName); + for (String schema : allowedSchemas) { + accessControl.checkCanShowTablesMetadata(session.getRequiredTransactionId(), session.getIdentity(), new CatalogSchemaName(catalogName, schema)); + } } return simpleQuery( diff --git a/presto-product-tests/src/main/java/io/prestosql/tests/hive/TestRoles.java b/presto-product-tests/src/main/java/io/prestosql/tests/hive/TestRoles.java index ad9976cdbbe8..52ac328647c9 100644 --- a/presto-product-tests/src/main/java/io/prestosql/tests/hive/TestRoles.java +++ b/presto-product-tests/src/main/java/io/prestosql/tests/hive/TestRoles.java @@ -670,6 +670,48 @@ public void testAdminCanShowAllGrants() } } + @Test(groups = {ROLES, AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) + public void testAdminCanShowGrantsOnlyFromCurrentSchema() + { + try { + onPrestoBob().executeQuery("CREATE TABLE hive.default.test_table_bob (foo BIGINT)"); + onPresto().executeQuery("CREATE SCHEMA hive.test"); + onPresto().executeQuery("GRANT admin TO alice"); + onPrestoAlice().executeQuery("SET ROLE ADMIN"); + onPrestoAlice().executeQuery("CREATE TABLE hive.test.test_table_bob (foo BIGINT)"); + + QueryAssert.assertThat(onPrestoAlice().executeQuery("SHOW GRANTS ON hive.default.test_table_bob")) + .containsOnly(ImmutableList.of( + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "SELECT", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "DELETE", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "UPDATE", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "INSERT", "YES", null))); + + QueryAssert.assertThat(onPrestoAlice().executeQuery("SHOW GRANTS ON hive.test.test_table_bob")) + .containsOnly(ImmutableList.of( + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "SELECT", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "DELETE", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "UPDATE", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "INSERT", "YES", null))); + QueryAssert.assertThat(onPrestoAlice().executeQuery("SELECT * FROM hive.information_schema.table_privileges where table_name = 'test_table_bob'")) + .containsOnly(ImmutableList.of( + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "SELECT", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "DELETE", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "UPDATE", "YES", null), + row("bob", "USER", "bob", "USER", "hive", "default", "test_table_bob", "INSERT", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "SELECT", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "DELETE", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "UPDATE", "YES", null), + row("alice", "USER", "alice", "USER", "hive", "test", "test_table_bob", "INSERT", "YES", null))); + } + finally { + onPresto().executeQuery("DROP TABLE hive.default.test_table_bob"); + onPrestoAlice().executeQuery("DROP TABLE hive.test.test_table_bob"); + onPresto().executeQuery("DROP SCHEMA hive.test"); + onPresto().executeQuery("REVOKE admin FROM alice"); + } + } + @Test(groups = {ROLES, AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) public void testSetRoleTablePermissions() {