Do not create a role if ALTER ROLE IF EXISTS operates on non-existing role
patch by Stefan Miklosovic; reviewed by Brandon Williams for CASSANDRA-19749
diff --git a/CHANGES.txt b/CHANGES.txt
index cdceaa8..1ade2c0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.1.6
+ * Do not create a role if ALTER ROLE IF EXISTS operates on non-existing role (CASSANDRA-19749)
* Use OpOrder in repairIterator to ensure we don't lose memtables mid-paxos repair (Cassandra-19668)
* Refresh stale paxos commit (CASSANDRA-19617)
* Reduce info logging from automatic paxos repair (CASSANDRA-19445)
diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py
index 03e06d8..bbaea48 100644
--- a/pylib/cqlshlib/cql3handling.py
+++ b/pylib/cqlshlib/cql3handling.py
@@ -1483,7 +1483,7 @@
| <unreservedKeyword>
;
-<createRoleStatement> ::= "CREATE" "ROLE" <rolename>
+<createRoleStatement> ::= "CREATE" "ROLE" ( "IF" "NOT" "EXISTS" )? <rolename>
( "WITH" <roleProperty> ("AND" <roleProperty>)*)?
;
diff --git a/pylib/cqlshlib/test/test_cqlsh_completion.py b/pylib/cqlshlib/test/test_cqlsh_completion.py
index af9d05e..731b987 100644
--- a/pylib/cqlshlib/test/test_cqlsh_completion.py
+++ b/pylib/cqlshlib/test/test_cqlsh_completion.py
@@ -952,3 +952,12 @@
def test_complete_in_alter_role(self):
self.trycompletions('ALTER ROLE ', choices=['<identifier>', 'IF', '<quotedName>'])
+ self.trycompletions('ALTER ROLE IF ', immediate='EXISTS ')
+
+ def test_complete_in_create_user(self):
+ self.trycompletions('CREATE USER ', choices=['<username>', 'IF'])
+ self.trycompletions('CREATE USER IF ', immediate='NOT EXISTS ')
+
+ def test_complete_in_create_role(self):
+ self.trycompletions('CREATE ROLE ', choices=['<identifier>', '<quotedName>', 'IF'])
+ self.trycompletions('CREATE ROLE IF ', immediate='NOT EXISTS ')
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java
index eb0e3e0..ccbaeb2 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java
@@ -105,6 +105,9 @@
public ResultMessage execute(ClientState state) throws RequestValidationException, RequestExecutionException
{
+ if (ifExists && !DatabaseDescriptor.getRoleManager().isExistingRole(role))
+ return null;
+
if (!opts.isEmpty())
DatabaseDescriptor.getRoleManager().alterRole(state.getUser(), role, opts);
if (dcPermissions != null)
diff --git a/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java b/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java
index 33ae129..5b079b4 100644
--- a/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java
+++ b/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java
@@ -18,13 +18,20 @@
package org.apache.cassandra.auth;
+import java.util.HashSet;
+import java.util.Set;
+
import org.junit.BeforeClass;
import org.junit.Test;
+import com.datastax.driver.core.ResultSet;
import com.datastax.driver.core.exceptions.AuthenticationException;
+import com.datastax.driver.core.exceptions.InvalidQueryException;
import org.apache.cassandra.Util;
import org.apache.cassandra.cql3.CQLTester;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mindrot.jbcrypt.BCrypt.gensalt;
import static org.mindrot.jbcrypt.BCrypt.hashpw;
@@ -101,7 +108,7 @@
String.format("CREATE USER %s WITH hashed password '%s'",
user1, "this_is_an_invalid_hash"));
executeNet(String.format("CREATE USER %s WITH hashed password '%s'", user1, hashedPassword));
- executeNet(String.format("CREATE USER %s WITH password '%s'", user2, plainTextPwd));
+ executeNet(String.format("CREATE USER %s WITH password '%s'", user2, plainTextPwd));
useUser(user1, plainTextPwd);
@@ -125,6 +132,37 @@
executeNetWithAuthSpin("SELECT key FROM system.local");
}
+ @Test
+ public void createAlterRoleIfExists() throws Throwable
+ {
+ useSuperUser();
+
+ executeNet("CREATE ROLE IF NOT EXISTS does_not_exist_yet");
+ assertTrue(getAllRoles().contains("does_not_exist_yet"));
+
+ // execute one more time
+ executeNet("CREATE ROLE IF NOT EXISTS does_not_exist_yet");
+
+ assertThatThrownBy(() -> executeNet("CREATE ROLE does_not_exist_yet"))
+ .isInstanceOf(InvalidQueryException.class)
+ .hasMessageContaining("does_not_exist_yet already exists");
+
+ // alter non-existing is no-op when "if exists" is specified
+ executeNet("ALTER ROLE IF EXISTS also_does_not_exist_yet WITH LOGIN = true");
+ Set<String> roles = getAllRoles();
+ assertTrue(roles.contains("does_not_exist_yet"));
+ // not created - CASSANDRA-19749
+ assertFalse(roles.contains("also_does_not_exist_yet"));
+ }
+
+ private Set<String> getAllRoles() throws Throwable
+ {
+ ResultSet rows = executeNet("SELECT role FROM system_auth.roles");
+ Set<String> roles = new HashSet<>();
+ rows.forEach(row -> roles.add(row.getString(0)));
+ return roles;
+ }
+
/**
* Altering or creating auth may take some time to be effective
*
