You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2021/10/22 12:37:23 UTC

[cassandra] branch trunk updated: Allow GRANT/REVOKE multiple permissions in a single statement

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 <fr...@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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org