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

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)

pylib/cqlshlib/cql3handling.py

@@ -1483,7 +1483,7 @@ syntax_rules += r'''
              | <unreservedKeyword>
              ;
 
-<createRoleStatement> ::= "CREATE" "ROLE" <rolename>
+<createRoleStatement> ::= "CREATE" "ROLE" ( "IF" "NOT" "EXISTS" )? <rolename>
                               ( "WITH" <roleProperty> ("AND" <roleProperty>)*)?
                         ;
 

pylib/cqlshlib/test/test_cqlsh_completion.py

@@ -952,3 +952,12 @@ class TestCqlshCompletion(CqlshCompletionCase):
 
     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 ')

src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java

@@ -105,6 +105,9 @@ public class AlterRoleStatement extends AuthenticationStatement
 
     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)

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 @@ public class CreateAndAlterRoleTest extends CQLTester
                              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 @@ public class CreateAndAlterRoleTest extends CQLTester
         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
      *