diff --git a/auth/service.cc b/auth/service.cc index 24accd66b9..e9609094a4 100644 --- a/auth/service.cc +++ b/auth/service.cc @@ -676,7 +676,8 @@ future get_permissions(const service& ser, const authenticated_u } bool is_protected(const service& ser, command_desc cmd) noexcept { - if (cmd.type_ == command_desc::type::ALTER_WITH_OPTS) { + if (cmd.type_ == command_desc::type::ALTER_WITH_OPTS || + cmd.type_ == command_desc::type::ALTER_SYSTEM_WITH_ALLOWED_OPTS) { return false; // Table attributes are OK to modify; see #7057. } return ser.underlying_role_manager().protected_resources().contains(cmd.resource) diff --git a/auth/service.hh b/auth/service.hh index 73765992b0..eac2f61d21 100644 --- a/auth/service.hh +++ b/auth/service.hh @@ -224,6 +224,7 @@ struct command_desc { const ::auth::resource& resource; ///< Resource impacted by this command. enum class type { ALTER_WITH_OPTS, ///< Command is ALTER ... WITH ... + ALTER_SYSTEM_WITH_ALLOWED_OPTS, OTHER } type_ = type::OTHER; }; diff --git a/cql3/statements/alter_table_statement.cc b/cql3/statements/alter_table_statement.cc index f8bec2f467..115e66e156 100644 --- a/cql3/statements/alter_table_statement.cc +++ b/cql3/statements/alter_table_statement.cc @@ -57,8 +57,16 @@ uint32_t alter_table_statement::get_bound_terms() const { future<> alter_table_statement::check_access(query_processor& qp, const service::client_state& state) const { using cdt = auth::command_desc::type; - return state.has_column_family_access(keyspace(), column_family(), auth::permission::ALTER, - _type == type::opts ? cdt::ALTER_WITH_OPTS : cdt::OTHER); + auto type = cdt::OTHER; + if (_type == type::opts) { + // can modify only KW_MEMTABLE_FLUSH_PERIOD property for system tables (see issue #21223) + if (is_system_keyspace(keyspace()) && _properties->count() == 1 && _properties->has_property(cf_prop_defs::KW_MEMTABLE_FLUSH_PERIOD)) { + type = cdt::ALTER_SYSTEM_WITH_ALLOWED_OPTS; + } else { + type = cdt::ALTER_WITH_OPTS; + } + } + return state.has_column_family_access(keyspace(), column_family(), auth::permission::ALTER, type); } static data_type validate_alter(const schema& schema, const column_definition& def, const cql3_type& validator) diff --git a/cql3/statements/property_definitions.hh b/cql3/statements/property_definitions.hh index 0733a9269b..11f354c4a1 100644 --- a/cql3/statements/property_definitions.hh +++ b/cql3/statements/property_definitions.hh @@ -70,6 +70,10 @@ public: static int32_t to_int(sstring key, std::optional value, int32_t default_value); static long to_long(sstring key, std::optional value, long default_value); + + size_t count() const { + return _properties.size(); + } }; } diff --git a/service/client_state.cc b/service/client_state.cc index 76ce2d03c5..b0ae51b2ce 100644 --- a/service/client_state.cc +++ b/service/client_state.cc @@ -125,7 +125,7 @@ future<> service::client_state::has_access(const sstring& ks, auth::command_desc // prevent system keyspace modification auto name = ks; std::transform(name.begin(), name.end(), name.begin(), ::tolower); - if (is_system_keyspace(name)) { + if (is_system_keyspace(name) && cmd.type_ != auth::command_desc::type::ALTER_SYSTEM_WITH_ALLOWED_OPTS) { return make_exception_future<>(exceptions::unauthorized_exception(ks + " keyspace is not user-modifiable.")); } diff --git a/test/cqlpy/test_system_tables.py b/test/cqlpy/test_system_tables.py index cf432e8e18..8b9994f110 100644 --- a/test/cqlpy/test_system_tables.py +++ b/test/cqlpy/test_system_tables.py @@ -11,6 +11,7 @@ from .util import new_test_table import pytest from . import nodetool +from cassandra.protocol import Unauthorized ############################################################################# # system.size_estimates.partitions_count @@ -133,3 +134,15 @@ def test_partitions_estimate_only_deletions(cassandra_bug, cql, test_keyspace): print(count) # Count should be close to 0, not to N assert count < N/1.25 + +# See issue #21223 +# Test possibility to set 'memtable_flush_period_in_ms' option for system tables +# and this option only: it is impossible to modify it with any other options together +def test_alter_system_table_properties(cql, test_keyspace): + with pytest.raises(Unauthorized): + cql.execute("ALTER TABLE system.compaction_history WITH comment = ''") + + with pytest.raises(Unauthorized): + cql.execute("ALTER TABLE system.compaction_history WITH memtable_flush_period_in_ms = 80000 AND comment = ''") + + cql.execute("ALTER TABLE system.compaction_history WITH memtable_flush_period_in_ms = 80000")