This is an automated email from the ASF dual-hosted git repository. blerer pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push: new 1858932 Allow GRANT/REVOKE multiple permissions in a single statement 1858932 is described below commit 185893256f10c14207bffe49ae733fb1a970aec5 Author: Francisco Guerrero <francisco.guerr...@apple.com> AuthorDate: Fri Oct 8 15:05:24 2021 -0700 Allow GRANT/REVOKE multiple permissions in a single statement patch by Francisco Guerrero; reviewed by Benjamin Lerer and Yifan Cai for CASSANDRA-17030 This commit allows GRANT/REVOKE statement to support multiple permissions with a single statement. For example, ``` GRANT MODIFY, SELECT ON KEYSPACE field TO manager; GRANT ALTER, DROP ON ROLE role1 TO role2; ``` --- CHANGES.txt | 1 + NEWS.txt | 3 + pylib/cqlshlib/cql3handling.py | 12 ++- pylib/cqlshlib/test/test_cqlsh_completion.py | 42 +++++++--- src/antlr/Parser.g | 6 +- .../apache/cassandra/auth/GrantAndRevokeTest.java | 20 +---- .../validation/miscellaneous/RoleSyntaxTest.java | 97 +++++++++++++--------- 7 files changed, 109 insertions(+), 72 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 624ae44..0103be2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Allow to GRANT or REVOKE multiple permissions in a single statement (CASSANDRA-17030) * Allow to grant permission for all tables in a keyspace (CASSANDRA-17027) * Log time spent writing keys during compaction (CASSANDRA-17037) * Make nodetool compactionstats and sstable_tasks consistent (CASSANDRA-16976) diff --git a/NEWS.txt b/NEWS.txt index 162241c..5c48685 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -38,6 +38,9 @@ using the provided 'sstableupgrade' tool. New features ------------ + - Support for multiple permission in a single GRANT/REVOKE/LIST statement has been added. It allows to + grant/revoke/list multiple permissions using a single statement by providing a list of comma-separated + permissions. - A new ALL TABLES IN KEYSPACE resource has been added. It allows to grant permissions for all tables and user types in a keyspace while preventing the user to use those permissions on the keyspace itself. - Added support for type casting in the WHERE clause components and in the values of INSERT and UPDATE statements. diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py index a99e779..5a9e498 100644 --- a/pylib/cqlshlib/cql3handling.py +++ b/pylib/cqlshlib/cql3handling.py @@ -1512,7 +1512,7 @@ syntax_rules += r''' | "EXECUTE" ; -<permissionExpr> ::= ( <permission> "PERMISSION"? ) +<permissionExpr> ::= ( [newpermission]=<permission> "PERMISSION"? ( "," [newpermission]=<permission> "PERMISSION"? )* ) | ( "ALL" "PERMISSIONS"? ) ; @@ -1547,6 +1547,16 @@ syntax_rules += r''' ''' +@completer_for('permissionExpr', 'newpermission') +def permission_completer(ctxt, _): + new_permissions = set([permission.upper() for permission in ctxt.get_binding('newpermission')]) + all_permissions = set([permission.arg for permission in ctxt.ruleset['permission'].arg]) + suggestions = all_permissions - new_permissions + if len(suggestions) == 0: + return [Hint('No more permissions here.')] + return suggestions + + @completer_for('username', 'name') def username_name_completer(ctxt, cass): def maybe_quote(name): diff --git a/pylib/cqlshlib/test/test_cqlsh_completion.py b/pylib/cqlshlib/test/test_cqlsh_completion.py index bc82033..d2cabdb 100644 --- a/pylib/cqlshlib/test/test_cqlsh_completion.py +++ b/pylib/cqlshlib/test/test_cqlsh_completion.py @@ -834,15 +834,23 @@ class TestCqlshCompletion(CqlshCompletionCase): choices=['ALL', 'ALTER', 'AUTHORIZE', 'CREATE', 'DESCRIBE', 'DROP', 'EXECUTE', 'MODIFY', 'SELECT'], other_choices_ok=True) self.trycompletions("GRANT MODIFY ", - choices=['ON', 'PERMISSION']) + choices=[',', 'ON', 'PERMISSION']) self.trycompletions("GRANT MODIFY P", - immediate='ERMISSION ON ') - self.trycompletions("GRANT MODIFY PERMISSION O", + immediate='ERMISSION ') + self.trycompletions("GRANT MODIFY PERMISSION ", + choices=[',', 'ON']) + self.trycompletions("GRANT MODIFY PERMISSION, ", + choices=['ALTER', 'AUTHORIZE', 'CREATE', 'DESCRIBE', 'DROP', 'EXECUTE', 'SELECT']) + self.trycompletions("GRANT MODIFY PERMISSION, D", + choices=['DESCRIBE', 'DROP']) + self.trycompletions("GRANT MODIFY PERMISSION, DR", + immediate='OP ') + self.trycompletions("GRANT MODIFY PERMISSION, DROP O", immediate='N ') - self.trycompletions("GRANT MODIFY ON ", + self.trycompletions("GRANT MODIFY, DROP ON ", choices=['ALL', 'KEYSPACE', 'MBEANS', 'ROLE', 'FUNCTION', 'MBEAN', 'TABLE'], other_choices_ok=True) - self.trycompletions("GRANT MODIFY ON ALL ", + self.trycompletions("GRANT MODIFY, DROP ON ALL ", choices=['KEYSPACES', 'TABLES'], other_choices_ok=True) self.trycompletions("GRANT MODIFY PERMISSION ON KEY", @@ -857,18 +865,28 @@ class TestCqlshCompletion(CqlshCompletionCase): choices=['ALL', 'ALTER', 'AUTHORIZE', 'CREATE', 'DESCRIBE', 'DROP', 'EXECUTE', 'MODIFY', 'SELECT'], other_choices_ok=True) self.trycompletions("REVOKE MODIFY ", - choices=['ON', 'PERMISSION']) + choices=[',', 'ON', 'PERMISSION']) self.trycompletions("REVOKE MODIFY P", - immediate='ERMISSION ON ') - self.trycompletions("REVOKE MODIFY PERMISSION O", + immediate='ERMISSION ') + self.trycompletions("REVOKE MODIFY PERMISSION ", + choices=[',', 'ON']) + self.trycompletions("REVOKE MODIFY PERMISSION, ", + choices=['ALTER', 'AUTHORIZE', 'CREATE', 'DESCRIBE', 'DROP', 'EXECUTE', 'SELECT']) + self.trycompletions("REVOKE MODIFY PERMISSION, D", + choices=['DESCRIBE', 'DROP']) + self.trycompletions("REVOKE MODIFY PERMISSION, DR", + immediate='OP ') + self.trycompletions("REVOKE MODIFY PERMISSION, DROP ", + choices=[',', 'ON', 'PERMISSION']) + self.trycompletions("REVOKE MODIFY PERMISSION, DROP O", immediate='N ') - self.trycompletions("REVOKE MODIFY PERMISSION ON ", + self.trycompletions("REVOKE MODIFY PERMISSION, DROP ON ", choices=['ALL', 'KEYSPACE', 'MBEANS', 'ROLE', 'FUNCTION', 'MBEAN', 'TABLE'], other_choices_ok=True) - self.trycompletions("REVOKE MODIFY ON ALL ", + self.trycompletions("REVOKE MODIFY, DROP ON ALL ", choices=['KEYSPACES', 'TABLES'], other_choices_ok=True) - self.trycompletions("REVOKE MODIFY PERMISSION ON KEY", + self.trycompletions("REVOKE MODIFY PERMISSION, DROP ON KEY", immediate='SPACE ') - self.trycompletions("REVOKE MODIFY PERMISSION ON KEYSPACE system_tr", + self.trycompletions("REVOKE MODIFY PERMISSION, DROP ON KEYSPACE system_tr", immediate='aces FROM ') diff --git a/src/antlr/Parser.g b/src/antlr/Parser.g index caeedde..efc8664 100644 --- a/src/antlr/Parser.g +++ b/src/antlr/Parser.g @@ -1037,7 +1037,7 @@ truncateStatement returns [TruncateStatement stmt] ; /** - * GRANT <permission> ON <resource> TO <rolename> + * GRANT <permission>[, <permission>]* | ALL ON <resource> TO <rolename> */ grantPermissionsStatement returns [GrantPermissionsStatement stmt] : K_GRANT @@ -1050,7 +1050,7 @@ grantPermissionsStatement returns [GrantPermissionsStatement stmt] ; /** - * REVOKE <permission> ON <resource> FROM <rolename> + * REVOKE <permission>[, <permission>]* | ALL ON <resource> FROM <rolename> */ revokePermissionsStatement returns [RevokePermissionsStatement stmt] : K_REVOKE @@ -1105,7 +1105,7 @@ permission returns [Permission perm] permissionOrAll returns [Set<Permission> perms] : K_ALL ( K_PERMISSIONS )? { $perms = Permission.ALL; } - | p=permission ( K_PERMISSION )? { $perms = EnumSet.of($p.perm); } + | p=permission ( K_PERMISSION )? { $perms = EnumSet.of($p.perm); } ( ',' p=permission ( K_PERMISSION )? { $perms.add($p.perm); } )* ; resource returns [IResource res] diff --git a/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java b/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java index 181a039..2d1cded 100644 --- a/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java +++ b/test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java @@ -89,10 +89,7 @@ public class GrantAndRevokeTest extends CQLTester useSuperUser(); - executeNet("GRANT ALTER ON KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT DROP ON KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT SELECT ON KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT MODIFY ON KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); + executeNet("GRANT ALTER, DROP, SELECT, MODIFY ON KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); useUser(user, pass); @@ -132,10 +129,7 @@ public class GrantAndRevokeTest extends CQLTester useSuperUser(); - executeNet("REVOKE ALTER ON KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE DROP ON KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE SELECT ON KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE MODIFY ON KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); + executeNet("REVOKE ALTER, DROP, MODIFY, SELECT ON KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); table = KEYSPACE_PER_TEST + "." + createTable(KEYSPACE_PER_TEST, "CREATE TABLE %s (pk int, ck int, val int, val_2 text, PRIMARY KEY (pk, ck))"); type = KEYSPACE_PER_TEST + "." + createType(KEYSPACE_PER_TEST, "CREATE TYPE %s (a int, b text)"); @@ -215,10 +209,7 @@ public class GrantAndRevokeTest extends CQLTester useSuperUser(); - executeNet("GRANT ALTER ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT DROP ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT SELECT ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); - executeNet("GRANT MODIFY ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); + executeNet("GRANT ALTER, DROP, SELECT, MODIFY ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " TO " + user); useUser(user, pass); @@ -261,10 +252,7 @@ public class GrantAndRevokeTest extends CQLTester useSuperUser(); - executeNet("REVOKE ALTER ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE DROP ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE SELECT ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); - executeNet("REVOKE MODIFY ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); + executeNet("REVOKE ALTER, DROP, SELECT, MODIFY ON ALL TABLES IN KEYSPACE " + KEYSPACE_PER_TEST + " FROM " + user); table = KEYSPACE_PER_TEST + "." + createTable(KEYSPACE_PER_TEST, "CREATE TABLE %s (pk int, ck int, val int, val_2 text, PRIMARY KEY (pk, ck))"); index = KEYSPACE_PER_TEST + '.' + createIndex(KEYSPACE_PER_TEST, "CREATE INDEX ON %s (val_2)"); diff --git a/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/RoleSyntaxTest.java b/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/RoleSyntaxTest.java index f72e3dc..fecb344 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/RoleSyntaxTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/miscellaneous/RoleSyntaxTest.java @@ -17,6 +17,8 @@ */ package org.apache.cassandra.cql3.validation.miscellaneous; +import java.util.Arrays; + import org.junit.Assert; import org.junit.Test; @@ -25,8 +27,9 @@ import org.apache.cassandra.cql3.CQLTester; public class RoleSyntaxTest extends CQLTester { - private final String NO_QUOTED_USERNAME = "Quoted strings are are not supported for user names " + - "and USER is deprecated, please use ROLE"; + private static final String NO_QUOTED_USERNAME = "Quoted strings are are not supported for user names " + + "and USER is deprecated, please use ROLE"; + @Test public void standardOptionsSyntaxTest() throws Throwable { @@ -107,49 +110,63 @@ public class RoleSyntaxTest extends CQLTester @Test public void grantRevokePermissionsSyntaxTest() throws Throwable { - // grant/revoke on RoleResource - assertValidSyntax("GRANT ALTER ON ROLE r1 TO r2"); - assertValidSyntax("GRANT ALTER ON ROLE 'r1' TO \"r2\""); - assertValidSyntax("GRANT ALTER ON ROLE \"r1\" TO 'r2'"); - assertValidSyntax("GRANT ALTER ON ROLE $$r1$$ TO $$ r '2' $$"); - assertValidSyntax("REVOKE ALTER ON ROLE r1 FROM r2"); - assertValidSyntax("REVOKE ALTER ON ROLE 'r1' FROM \"r2\""); - assertValidSyntax("REVOKE ALTER ON ROLE \"r1\" FROM 'r2'"); - assertValidSyntax("REVOKE ALTER ON ROLE $$r1$$ FROM $$ r '2' $$"); - - // grant/revoke on DataResource - assertValidSyntax("GRANT SELECT ON KEYSPACE ks TO r1"); - assertValidSyntax("GRANT SELECT ON KEYSPACE ks TO 'r1'"); - assertValidSyntax("GRANT SELECT ON KEYSPACE ks TO \"r1\""); - assertValidSyntax("GRANT SELECT ON KEYSPACE ks TO $$ r '1' $$"); - assertValidSyntax("REVOKE SELECT ON KEYSPACE ks FROM r1"); - assertValidSyntax("REVOKE SELECT ON KEYSPACE ks FROM 'r1'"); - assertValidSyntax("REVOKE SELECT ON KEYSPACE ks FROM \"r1\""); - assertValidSyntax("REVOKE SELECT ON KEYSPACE ks FROM $$ r '1' $$"); + for (String r1 : Arrays.asList("r1", "'r1'", "\"r1\"", "$$r1$$")) + { + for (String r2 : Arrays.asList("r2", "\"r2\"", "'r2'", "$$ r '2' $$")) + { + // grant/revoke on RoleResource + assertValidSyntax(String.format("GRANT ALTER ON ROLE %s TO %s", r1, r2)); + assertValidSyntax(String.format("GRANT ALTER PERMISSION ON ROLE %s TO %s", r1, r2)); + assertValidSyntax(String.format("REVOKE ALTER ON ROLE %s FROM %s", r1, r2)); + assertValidSyntax(String.format("REVOKE ALTER PERMISSION ON ROLE %s FROM %s", r1, r2)); + + // grant/revoke multiple permissions in a single statement + assertValidSyntax(String.format("GRANT CREATE, ALTER ON ROLE %s TO %s", r1, r2)); + assertValidSyntax(String.format("GRANT CREATE PERMISSION, ALTER PERMISSION ON ROLE %s TO %s", r1, r2)); + assertValidSyntax(String.format("REVOKE CREATE, ALTER ON ROLE %s FROM %s", r1, r2)); + assertValidSyntax(String.format("REVOKE CREATE PERMISSION, ALTER PERMISSION ON ROLE %s FROM %s", r1, r2)); + } + } + + for (String r1 : Arrays.asList("r1", "'r1'", "\"r1\"", "$$r1$$", "$$ r '1' $$")) + { + // grant/revoke on DataResource + assertValidSyntax(String.format("GRANT SELECT ON KEYSPACE ks TO %s", r1)); + assertValidSyntax(String.format("GRANT SELECT PERMISSION ON KEYSPACE ks TO %s", r1)); + assertValidSyntax(String.format("REVOKE SELECT ON KEYSPACE ks FROM %s", r1)); + assertValidSyntax(String.format("REVOKE SELECT PERMISSION ON KEYSPACE ks FROM %s", r1)); + + // grant/revoke multiple permissions in a single statement + assertValidSyntax(String.format("GRANT MODIFY, SELECT ON KEYSPACE ks TO %s", r1)); + assertValidSyntax(String.format("GRANT MODIFY PERMISSION, SELECT PERMISSION ON KEYSPACE ks TO %s", r1)); + assertValidSyntax(String.format("GRANT MODIFY, SELECT ON ALL KEYSPACES TO %s", r1)); + assertValidSyntax(String.format("GRANT MODIFY PERMISSION, SELECT PERMISSION ON ALL KEYSPACES TO %s", r1)); + assertValidSyntax(String.format("REVOKE MODIFY, SELECT ON KEYSPACE ks FROM %s", r1)); + assertValidSyntax(String.format("REVOKE MODIFY PERMISSION, SELECT PERMISSION ON KEYSPACE ks FROM %s", r1)); + assertValidSyntax(String.format("REVOKE MODIFY, SELECT ON ALL KEYSPACES FROM %s", r1)); + assertValidSyntax(String.format("REVOKE MODIFY PERMISSION, SELECT PERMISSION ON ALL KEYSPACES FROM %s", r1)); + } } @Test public void listPermissionsSyntaxTest() throws Throwable { - assertValidSyntax("LIST ALL PERMISSIONS ON ALL ROLES OF r1"); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL ROLES OF 'r1'"); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL ROLES OF \"r1\""); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL ROLES OF $$ r '1' $$"); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE 'r1' OF r2"); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE \"r1\" OF r2"); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE $$ r '1' $$ OF r2"); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE 'r1' OF 'r2'"); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE \"r1\" OF \"r2\""); - assertValidSyntax("LIST ALL PERMISSIONS ON ROLE $$r1$$ OF $$ r '2' $$"); - - assertValidSyntax("LIST ALL PERMISSIONS ON ALL KEYSPACES OF r1"); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL KEYSPACES OF 'r1'"); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL KEYSPACES OF \"r1\""); - assertValidSyntax("LIST ALL PERMISSIONS ON ALL KEYSPACES OF $$ r '1' $$"); - assertValidSyntax("LIST ALL PERMISSIONS OF r1"); - assertValidSyntax("LIST ALL PERMISSIONS OF 'r1'"); - assertValidSyntax("LIST ALL PERMISSIONS OF \"r1\""); - assertValidSyntax("LIST ALL PERMISSIONS OF $$ r '1' $$"); + for (String r1 : Arrays.asList("r1", "'r1'", "\"r1\"", "$$r1$$", "$$ r '1' $$")) + { + assertValidSyntax(String.format("LIST ALL PERMISSIONS ON ALL ROLES OF %s", r1)); + assertValidSyntax(String.format("LIST ALL PERMISSIONS ON ALL KEYSPACES OF %s", r1)); + assertValidSyntax(String.format("LIST ALL PERMISSIONS OF %s", r1)); + assertValidSyntax(String.format("LIST MODIFY PERMISSION ON KEYSPACE ks OF %s", r1)); + assertValidSyntax(String.format("LIST MODIFY, SELECT OF %s", r1)); + assertValidSyntax(String.format("LIST MODIFY, SELECT PERMISSION ON KEYSPACE ks OF %s", r1)); + + for (String r2 : Arrays.asList("r2", "\"r2\"", "'r2'", "$$ r '2' $$")) + { + assertValidSyntax(String.format("LIST ALL PERMISSIONS ON ROLE %s OF %s", r1, r2)); + assertValidSyntax(String.format("LIST ALTER PERMISSION ON ROLE %s OF %s", r1, r2)); + assertValidSyntax(String.format("LIST ALTER, DROP PERMISSION ON ROLE %s OF %s", r1, r2)); + } + } } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org