diff --git a/CHANGES.txt b/CHANGES.txt index 77689676b0..7ba25c90f8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,6 +41,7 @@ * Fix resource cleanup after SAI query timeouts (CASSANDRA-19177) * Suppress CVE-2023-6481 (CASSANDRA-19184) Merged from 4.1: + * 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/test/test_cqlsh_completion.py b/pylib/cqlshlib/test/test_cqlsh_completion.py index a3ca07a902..a53df5905d 100644 --- a/pylib/cqlshlib/test/test_cqlsh_completion.py +++ b/pylib/cqlshlib/test/test_cqlsh_completion.py @@ -1042,11 +1042,16 @@ class TestCqlshCompletion(CqlshCompletionCase): def test_complete_in_alter_role(self): self.trycompletions('ALTER ROLE ', choices=['', 'IF', '']) + self.trycompletions('ALTER ROLE IF ', immediate='EXISTS ') self.trycompletions('ALTER ROLE foo ', immediate='WITH ') self.trycompletions('ALTER ROLE foo WITH ', choices=['ACCESS', 'HASHED', 'LOGIN', 'OPTIONS', 'PASSWORD', 'SUPERUSER']) self.trycompletions('ALTER ROLE foo WITH ACCESS TO ', choices=['ALL', 'DATACENTERS']) self.trycompletions('ALTER ROLE foo WITH ACCESS FROM ', choices=['ALL', 'CIDRS']) + def test_complete_in_create_user(self): + self.trycompletions('CREATE USER ', choices=['', 'IF']) + self.trycompletions('CREATE USER IF ', immediate='NOT EXISTS ') + def test_complete_in_drop_role(self): self.trycompletions('DROP ROLE ', choices=['', 'IF', '']) diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java index aadf6c788d..74ec25f2df 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterRoleStatement.java @@ -114,6 +114,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); diff --git a/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java b/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java index 6bb83b3b63..a6adafcd01 100644 --- a/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java +++ b/test/unit/org/apache/cassandra/auth/CreateAndAlterRoleTest.java @@ -18,13 +18,21 @@ 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.InvalidQueryException; import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.config.DatabaseDescriptor; 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; @@ -129,4 +137,35 @@ public class CreateAndAlterRoleTest extends CQLTester executeNet("SELECT key FROM system.local"); } + + @Test + public void createAlterRoleIfExists() + { + 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 roles = getAllRoles(); + assertTrue(roles.contains("does_not_exist_yet")); + // not created - CASSANDRA-19749 + assertFalse(roles.contains("also_does_not_exist_yet")); + } + + private Set getAllRoles() + { + ResultSet rows = executeNet("SELECT role FROM system_auth.roles"); + Set roles = new HashSet<>(); + rows.forEach(row -> roles.add(row.getString(0))); + return roles; + } }