From 5d9615c3feb82991f7f4ac8297b3104d6b8f9fc4 Mon Sep 17 00:00:00 2001 From: Christina Wallin Date: Tue, 28 Nov 2017 14:51:06 +0100 Subject: [PATCH] Add SHOW CURRENT ROLES Instead of select * from information_schema.roles, SHOW CURRENT ROLES rewrites to select * from information_schema.enabled_roles. All users can see what roles they're currently using, so no need for access control checks. Extracted-From: https://github.com/prestodb/presto/pull/10904 --- .../src/main/sphinx/sql/show-roles.rst | 9 +++++-- .../sql/rewrite/ShowQueriesRewrite.java | 17 +++++++++---- .../antlr4/io/prestosql/sql/parser/SqlBase.g4 | 2 +- .../java/io/prestosql/sql/SqlFormatter.java | 6 ++++- .../io/prestosql/sql/parser/AstBuilder.java | 3 ++- .../java/io/prestosql/sql/tree/ShowRoles.java | 23 ++++++++++++------ .../prestosql/sql/parser/TestSqlParser.java | 13 +++++++--- .../sql/parser/TestStatementBuilder.java | 2 ++ .../io/prestosql/tests/hive/TestRoles.java | 24 ++++++++++++++++++- 9 files changed, 79 insertions(+), 20 deletions(-) diff --git a/presto-docs/src/main/sphinx/sql/show-roles.rst b/presto-docs/src/main/sphinx/sql/show-roles.rst index 23d4f5c6429a..a15e72c614c1 100644 --- a/presto-docs/src/main/sphinx/sql/show-roles.rst +++ b/presto-docs/src/main/sphinx/sql/show-roles.rst @@ -7,9 +7,14 @@ Synopsis .. code-block:: none - SHOW ROLES [ FROM catalog ] + SHOW [CURRENT] ROLES [ FROM catalog ] Description ----------- -List the roles in ``catalog`` or in the current catalog. +``SHOW ROLES`` lists all the roles in ``catalog`` or in the +current catalog if ``catalog`` is not specified. + +``SHOW CURRENT ROLES`` lists the enabled roles for the session +in ``catalog, or in the current catalog if ``catalog`` is not +specified. 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 1687c42a8efc..9751ede22332 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 @@ -83,6 +83,7 @@ import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.ImmutableList.toImmutableList; import static io.prestosql.connector.informationSchema.InformationSchemaMetadata.TABLE_COLUMNS; +import static io.prestosql.connector.informationSchema.InformationSchemaMetadata.TABLE_ENABLED_ROLES; import static io.prestosql.connector.informationSchema.InformationSchemaMetadata.TABLE_ROLES; import static io.prestosql.connector.informationSchema.InformationSchemaMetadata.TABLE_SCHEMATA; import static io.prestosql.connector.informationSchema.InformationSchemaMetadata.TABLE_TABLES; @@ -265,10 +266,18 @@ protected Node visitShowRoles(ShowRoles node, Void context) } String catalog = node.getCatalog().map(c -> c.getValue().toLowerCase(ENGLISH)).orElseGet(() -> session.getCatalog().get()); - accessControl.checkCanShowRoles(session.getRequiredTransactionId(), session.getIdentity(), catalog); - return simpleQuery( - selectList(aliasedName("role_name", "Role")), - from(catalog, TABLE_ROLES)); + + if (node.isCurrent()) { + return simpleQuery( + selectList(aliasedName("role_name", "Role")), + from(catalog, TABLE_ENABLED_ROLES)); + } + else { + accessControl.checkCanShowRoles(session.getRequiredTransactionId(), session.getIdentity(), catalog); + return simpleQuery( + selectList(aliasedName("role_name", "Role")), + from(catalog, TABLE_ROLES)); + } } @Override diff --git a/presto-parser/src/main/antlr4/io/prestosql/sql/parser/SqlBase.g4 b/presto-parser/src/main/antlr4/io/prestosql/sql/parser/SqlBase.g4 index 30642cc640d5..00e0e15abbb5 100644 --- a/presto-parser/src/main/antlr4/io/prestosql/sql/parser/SqlBase.g4 +++ b/presto-parser/src/main/antlr4/io/prestosql/sql/parser/SqlBase.g4 @@ -99,7 +99,7 @@ statement | SHOW COLUMNS (FROM | IN) qualifiedName #showColumns | SHOW STATS FOR qualifiedName #showStats | SHOW STATS FOR '(' querySpecification ')' #showStatsForQuery - | SHOW ROLES ((FROM | IN) identifier)? #showRoles + | SHOW CURRENT? ROLES ((FROM | IN) identifier)? #showRoles | DESCRIBE qualifiedName #showColumns | DESC qualifiedName #showColumns | SHOW FUNCTIONS #showFunctions diff --git a/presto-parser/src/main/java/io/prestosql/sql/SqlFormatter.java b/presto-parser/src/main/java/io/prestosql/sql/SqlFormatter.java index 1b093a364d5a..eb83278592a4 100644 --- a/presto-parser/src/main/java/io/prestosql/sql/SqlFormatter.java +++ b/presto-parser/src/main/java/io/prestosql/sql/SqlFormatter.java @@ -1263,7 +1263,11 @@ public Void visitShowGrants(ShowGrants node, Integer indent) @Override protected Void visitShowRoles(ShowRoles node, Integer context) { - builder.append("SHOW ROLES"); + builder.append("SHOW "); + if (node.isCurrent()) { + builder.append("CURRENT "); + } + builder.append("ROLES"); if (node.getCatalog().isPresent()) { builder.append(" FROM ") diff --git a/presto-parser/src/main/java/io/prestosql/sql/parser/AstBuilder.java b/presto-parser/src/main/java/io/prestosql/sql/parser/AstBuilder.java index 4c216f661698..7e654f208c0c 100644 --- a/presto-parser/src/main/java/io/prestosql/sql/parser/AstBuilder.java +++ b/presto-parser/src/main/java/io/prestosql/sql/parser/AstBuilder.java @@ -942,7 +942,8 @@ public Node visitShowRoles(SqlBaseParser.ShowRolesContext context) { return new ShowRoles( getLocation(context), - getIdentifierIfPresent(context.identifier())); + getIdentifierIfPresent(context.identifier()), + context.CURRENT() != null); } @Override diff --git a/presto-parser/src/main/java/io/prestosql/sql/tree/ShowRoles.java b/presto-parser/src/main/java/io/prestosql/sql/tree/ShowRoles.java index 226f06bbaea4..79400afcf65b 100644 --- a/presto-parser/src/main/java/io/prestosql/sql/tree/ShowRoles.java +++ b/presto-parser/src/main/java/io/prestosql/sql/tree/ShowRoles.java @@ -26,21 +26,23 @@ public class ShowRoles extends Statement { private final Optional catalog; + private final boolean current; - public ShowRoles(Optional catalog) + public ShowRoles(Optional catalog, boolean current) { - this(Optional.empty(), catalog); + this(Optional.empty(), catalog, current); } - public ShowRoles(NodeLocation location, Optional catalog) + public ShowRoles(NodeLocation location, Optional catalog, boolean current) { - this(Optional.of(location), catalog); + this(Optional.of(location), catalog, current); } - public ShowRoles(Optional location, Optional catalog) + public ShowRoles(Optional location, Optional catalog, boolean current) { super(location); this.catalog = requireNonNull(catalog, "catalog is null"); + this.current = current; } public Optional getCatalog() @@ -48,6 +50,11 @@ public Optional getCatalog() return catalog; } + public boolean isCurrent() + { + return current; + } + @Override public R accept(AstVisitor visitor, C context) { @@ -63,7 +70,7 @@ public List getChildren() @Override public int hashCode() { - return Objects.hash(catalog); + return Objects.hash(catalog, current); } @Override @@ -76,7 +83,8 @@ public boolean equals(Object obj) return false; } ShowRoles o = (ShowRoles) obj; - return Objects.equals(catalog, o.catalog); + return Objects.equals(catalog, o.catalog) && + current == o.current; } @Override @@ -84,6 +92,7 @@ public String toString() { return toStringHelper(this) .add("catalog", catalog) + .add("current", current) .toString(); } } diff --git a/presto-parser/src/test/java/io/prestosql/sql/parser/TestSqlParser.java b/presto-parser/src/test/java/io/prestosql/sql/parser/TestSqlParser.java index 65d390371b13..7b839a2dbcda 100644 --- a/presto-parser/src/test/java/io/prestosql/sql/parser/TestSqlParser.java +++ b/presto-parser/src/test/java/io/prestosql/sql/parser/TestSqlParser.java @@ -1448,11 +1448,18 @@ public void testShowRoles() throws Exception { assertStatement("SHOW ROLES", - new ShowRoles(Optional.empty())); + new ShowRoles(Optional.empty(), false)); assertStatement("SHOW ROLES FROM foo", - new ShowRoles(Optional.of(new Identifier("foo")))); + new ShowRoles(Optional.of(new Identifier("foo")), false)); assertStatement("SHOW ROLES IN foo", - new ShowRoles(Optional.of(new Identifier("foo")))); + new ShowRoles(Optional.of(new Identifier("foo")), false)); + + assertStatement("SHOW CURRENT ROLES", + new ShowRoles(Optional.empty(), true)); + assertStatement("SHOW CURRENT ROLES FROM foo", + new ShowRoles(Optional.of(new Identifier("foo")), true)); + assertStatement("SHOW CURRENT ROLES IN foo", + new ShowRoles(Optional.of(new Identifier("foo")), true)); } @Test diff --git a/presto-parser/src/test/java/io/prestosql/sql/parser/TestStatementBuilder.java b/presto-parser/src/test/java/io/prestosql/sql/parser/TestStatementBuilder.java index 175ae2e02ac8..46d966ebb4a8 100644 --- a/presto-parser/src/test/java/io/prestosql/sql/parser/TestStatementBuilder.java +++ b/presto-parser/src/test/java/io/prestosql/sql/parser/TestStatementBuilder.java @@ -248,6 +248,8 @@ public void testStatementBuilder() printStatement("show grants"); printStatement("show roles"); printStatement("show roles from foo"); + printStatement("show current roles"); + printStatement("show current roles from foo"); printStatement("prepare p from select * from (select * from T) \"A B\""); 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 72b3ff1347d4..37dbf043ad2f 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 @@ -491,7 +491,7 @@ public void testSetAdminRole() row("admin")); } - @Test(groups = {HIVE_CONNECTOR, ROLES, AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) + @Test(groups = {ROLES, AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) public void testShowRoles() { QueryAssert.assertThat(onPresto().executeQuery("SHOW ROLES")) @@ -514,6 +514,28 @@ public void testShowRoles() row("role1")); } + @Test(groups = {ROLES, AUTHORIZATION, PROFILE_SPECIFIC_TESTS}) + public void testShowCurrentRoles() + { + QueryAssert.assertThat(onPresto().executeQuery("SHOW CURRENT ROLES")) + .containsOnly( + row("public")); + onPresto().executeQuery("CREATE ROLE role1"); + onPresto().executeQuery("CREATE ROLE role2"); + onPresto().executeQuery("GRANT role1 TO alice"); + onPresto().executeQuery("GRANT role2 TO alice"); + QueryAssert.assertThat(onPrestoAlice().executeQuery("SHOW CURRENT ROLES")) + .containsOnly( + row("public"), + row("role1"), + row("role2")); + onPrestoAlice().executeQuery("SET ROLE role2"); + QueryAssert.assertThat(onPrestoAlice().executeQuery("SHOW CURRENT ROLES")) + .containsOnly( + row("public"), + row("role2")); + } + private static QueryExecutor onPrestoAlice() { return connectToPresto("alice@presto");