CASSANDRA-17027: Allow to grant permission for all tables in a keyspace #1263

blerer · 2021-10-11T12:40:27Z

In some cases it is useful to prevent users to alter or drop a keyspace
while allowing them to create new tables.
This patch add support for a new DataResource below KEYSPACE but above
TABLE. The syntax to grant permission at this level in ALL TABLES IN
KEYSPACE.

adelapena · 2021-10-12T12:20:25Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

Nit: there are a few double spaces around here

Suggested change

assertUnauthorizedQuery("User user has no SELECT permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));

assertUnauthorizedQuery("User user has no MODIFY permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));

assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));

assertUnauthorizedQuery("User user has no DROP permission on <table " + table + "> or any of its parents",

assertUnauthorizedQuery("User user has no SELECT permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));

assertUnauthorizedQuery("User user has no MODIFY permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));

assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));

assertUnauthorizedQuery("User user has no DROP permission on <table " + table + "> or any of its parents",

adelapena · 2021-10-12T12:21:26Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

Nit: there are a few double spaces around here

Suggested change

assertUnauthorizedQuery("User user has no SELECT permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));

assertUnauthorizedQuery("User user has no MODIFY permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));

assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));

assertUnauthorizedQuery("User user has no DROP permission on <table " + table + "> or any of its parents",

assertUnauthorizedQuery("User user has no SELECT permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));

assertUnauthorizedQuery("User user has no MODIFY permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));

assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",

formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));

assertUnauthorizedQuery("User user has no DROP permission on <table " + table + "> or any of its parents",

adelapena · 2021-10-12T12:27:42Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

I understand that this is here to generate a new currentTable() for the calls to formatQuery below, although it seems incidentally no-op here, is this right? Could we add a brief inline comment like create a new table name for the next calls to formatQuery?

adelapena · 2021-10-12T12:30:59Z

test/unit/org/apache/cassandra/auth/RoleOptionsTest.java

Do we need the changes in this file?

adelapena · 2021-10-12T12:32:20Z

test/unit/org/apache/cassandra/cql3/CQLTester.java

It's great that we have this available! :)

adelapena · 2021-10-12T12:33:11Z

src/java/org/apache/cassandra/service/ClientState.java

I guess it should be ensureAllTablesPermission

adelapena · 2021-10-12T14:37:30Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

Maybe we could add coverage for some other queries, such as as UPDATE and DELETE as it's done here, and perhaps even create/drop indexes and MVs, wdyt?

ekaterinadimitrova2 · 2021-10-12T19:19:55Z

NEWS.txt

Suggested change

- A new ALL TABLES IN KEYSPACE resource has been added. It allows to grant permission for all tables and user types

in a keyspace while preventing the user to use those permission on the keyspace itself.

- 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.

ekaterinadimitrova2 · 2021-10-12T19:31:00Z

pylib/cqlshlib/test/test_cqlsh_completion.py

Suggested change

self.trycompletions(

"GR",

immediate='ANT ')

self.trycompletions("GR",

immediate='ANT ')

I would probably keep the formatting for this and all following invocations similar to the one used in the other tests in the same class.

ekaterinadimitrova2 · 2021-10-12T19:48:44Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

Maybe we can add before the class @SuppressWarnings("SingleCharacterStringConcatenation") to remove the noise from the warnings?

This is warning seems to be an IDEA warning so I am reluctant to add annotation for those things to the code base.

ekaterinadimitrova2 · 2021-10-12T21:18:32Z

test/unit/org/apache/cassandra/auth/GrantAndRevokeTest.java

I am not sure, didn't you mean it to be 0 actually for disable?

Any number less or equal to zero will disable caching. I will change it to zero if it makes things less confusing.

Yes, while it works it is documented (both cassandra.yaml and the website documentation) to use 0 for disable and a quick check shows me that In other unit tests (JMXAuthTest, UFAuthTest) we use 0. So I think it will indeed make it less confusing and more consistent if we keep it 0. Thank you :-)

In some cases it is useful to prevent users to alter or drop a keyspace while allowing them to create new tables. This patch add support for a new DataResource below KEYSPACE but above TABLE. The syntax to grant permission at this level in ALL TABLES IN KEYSPACE.

blerer · 2021-10-20T12:32:48Z

Committed manually

We cannot go to DC yet because we need a two phase roll out where any component that reads sai indexes can already read DC, but we haven't deployed the DC reading code yet (there are still pending fixes).

adelapena reviewed Oct 12, 2021

View reviewed changes

ekaterinadimitrova2 reviewed Oct 12, 2021

View reviewed changes

blerer added 2 commits October 18, 2021 18:25

CircleCI: DO NOT MERGE

c7e0064

blerer force-pushed the CASSANDRA-17027 branch from 1b89219 to c7e0064 Compare October 18, 2021 16:27

missing fixes

ea3d20d

blerer closed this Oct 20, 2021

blerer deleted the CASSANDRA-17027 branch October 20, 2021 12:36

-        assertUnauthorizedQuery("User user has no SELECT permission on <table " +  table + "> or any of its parents",
-                                formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));
-        assertUnauthorizedQuery("User user has no MODIFY permission on <table " +  table + "> or any of its parents",
-                                formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));
-        assertUnauthorizedQuery("User user has no ALTER permission on <table " +  table + "> or any of its parents",
-                                formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));
-        assertUnauthorizedQuery("User user has no DROP permission on <table " +  table + "> or any of its parents",
+        assertUnauthorizedQuery("User user has no SELECT permission on <table " + table + "> or any of its parents",
+                                formatQuery(KEYSPACE_PER_TEST, "SELECT * FROM %s WHERE pk = 1 AND ck = 1"));
+        assertUnauthorizedQuery("User user has no MODIFY permission on <table " + table + "> or any of its parents",
+                                formatQuery(KEYSPACE_PER_TEST, "TRUNCATE TABLE %s"));
+        assertUnauthorizedQuery("User user has no ALTER permission on <table " + table + "> or any of its parents",
+                                formatQuery(KEYSPACE_PER_TEST, "ALTER TABLE %s ADD val_2 int"));
+        assertUnauthorizedQuery("User user has no DROP permission on <table " + table + "> or any of its parents",

-    - A new ALL TABLES IN KEYSPACE resource has been added. It allows to grant permission for all tables and user types
-      in a keyspace while preventing the user to use those permission on the keyspace itself.
+    - 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.

-        self.trycompletions(
-            "GR",
-            immediate='ANT ')
+        self.trycompletions("GR",
+                                          immediate='ANT ')

CASSANDRA-17027: Allow to grant permission for all tables in a keyspace #1263

CASSANDRA-17027: Allow to grant permission for all tables in a keyspace #1263

Uh oh!

Conversation

blerer commented Oct 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekaterinadimitrova2 Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blerer commented Oct 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ekaterinadimitrova2 Oct 18, 2021 •

edited

Loading