Fix checking access for the SYSTEM command.

src/Access/Common/AccessType.h

@@ -145,14 +145,14 @@ enum class AccessType
     M(SYSTEM_RELOAD_EMBEDDED_DICTIONARIES, "RELOAD EMBEDDED DICTIONARIES", GLOBAL, SYSTEM_RELOAD) /* implicitly enabled by the grant SYSTEM_RELOAD_DICTIONARY ON *.* */\
     M(SYSTEM_RELOAD, "", GROUP, SYSTEM) \
     M(SYSTEM_RESTART_DISK, "SYSTEM RESTART DISK", GLOBAL, SYSTEM) \
-    M(SYSTEM_MERGES, "SYSTEM STOP MERGES, SYSTEM START MERGES, STOP_MERGES, START MERGES", TABLE, SYSTEM) \
+    M(SYSTEM_MERGES, "SYSTEM STOP MERGES, SYSTEM START MERGES, STOP MERGES, START MERGES", TABLE, SYSTEM) \
     M(SYSTEM_TTL_MERGES, "SYSTEM STOP TTL MERGES, SYSTEM START TTL MERGES, STOP TTL MERGES, START TTL MERGES", TABLE, SYSTEM) \
     M(SYSTEM_FETCHES, "SYSTEM STOP FETCHES, SYSTEM START FETCHES, STOP FETCHES, START FETCHES", TABLE, SYSTEM) \
     M(SYSTEM_MOVES, "SYSTEM STOP MOVES, SYSTEM START MOVES, STOP MOVES, START MOVES", TABLE, SYSTEM) \
     M(SYSTEM_DISTRIBUTED_SENDS, "SYSTEM STOP DISTRIBUTED SENDS, SYSTEM START DISTRIBUTED SENDS, STOP DISTRIBUTED SENDS, START DISTRIBUTED SENDS", TABLE, SYSTEM_SENDS) \
-    M(SYSTEM_REPLICATED_SENDS, "SYSTEM STOP REPLICATED SENDS, SYSTEM START REPLICATED SENDS, STOP_REPLICATED_SENDS, START REPLICATED SENDS", TABLE, SYSTEM_SENDS) \
+    M(SYSTEM_REPLICATED_SENDS, "SYSTEM STOP REPLICATED SENDS, SYSTEM START REPLICATED SENDS, STOP REPLICATED SENDS, START REPLICATED SENDS", TABLE, SYSTEM_SENDS) \
     M(SYSTEM_SENDS, "SYSTEM STOP SENDS, SYSTEM START SENDS, STOP SENDS, START SENDS", GROUP, SYSTEM) \
-    M(SYSTEM_REPLICATION_QUEUES, "SYSTEM STOP REPLICATION QUEUES, SYSTEM START REPLICATION QUEUES, STOP_REPLICATION_QUEUES, START REPLICATION QUEUES", TABLE, SYSTEM) \
+    M(SYSTEM_REPLICATION_QUEUES, "SYSTEM STOP REPLICATION QUEUES, SYSTEM START REPLICATION QUEUES, STOP REPLICATION QUEUES, START REPLICATION QUEUES", TABLE, SYSTEM) \
     M(SYSTEM_DROP_REPLICA, "DROP REPLICA", TABLE, SYSTEM) \
     M(SYSTEM_SYNC_REPLICA, "SYNC REPLICA", TABLE, SYSTEM) \
     M(SYSTEM_RESTART_REPLICA, "RESTART REPLICA", TABLE, SYSTEM) \

src/Interpreters/InterpreterSystemQuery.cpp

@@ -141,12 +141,17 @@ void InterpreterSystemQuery::startStopAction(StorageActionBlockType action_type,
     auto manager = getContext()->getActionLocksManager();
     manager->cleanExpired();
 
+    auto access = getContext()->getAccess();
+    auto required_access_type = getRequiredAccessType(action_type);
+
     if (volume_ptr && action_type == ActionLocks::PartsMerge)
     {
+        access->checkAccess(required_access_type);
         volume_ptr->setAvoidMergesUserOverride(!start);
     }
     else if (table_id)
     {
+        access->checkAccess(required_access_type, table_id.database_name, table_id.table_name);
         auto table = DatabaseCatalog::instance().tryGetTable(table_id, getContext());
         if (table)
         {
@@ -161,7 +166,6 @@ void InterpreterSystemQuery::startStopAction(StorageActionBlockType action_type,
     }
     else
     {
-        auto access = getContext()->getAccess();
         for (auto & elem : DatabaseCatalog::instance().getDatabases())
         {
             for (auto iterator = elem.second->getTablesIterator(getContext()); iterator->isValid(); iterator->next())
@@ -170,14 +174,9 @@ void InterpreterSystemQuery::startStopAction(StorageActionBlockType action_type,
                 if (!table)
                     continue;
 
-                if (!access->isGranted(getRequiredAccessType(action_type), elem.first, iterator->name()))
+                if (!access->isGranted(required_access_type, elem.first, iterator->name()))
                 {
-                    LOG_INFO(
+                    LOG_INFO(log, "Access {} denied, skipping {}.{}", toString(required_access_type), elem.first, iterator->name());
-                        log,
-                        "Access {} denied, skipping {}.{}",
-                        toString(getRequiredAccessType(action_type)),
-                        elem.first,
-                        iterator->name());
                     continue;
                 }
 
@@ -422,8 +421,7 @@ BlockIO InterpreterSystemQuery::execute()
             restartReplicas(system_context);
             break;
         case Type::RESTART_REPLICA:
-            if (!tryRestartReplica(table_id, system_context))
+            restartReplica(table_id, system_context);
-                throw Exception(ErrorCodes::BAD_ARGUMENTS, table_is_not_replicated.data(), table_id.getNameForLogs());
             break;
         case Type::RESTORE_REPLICA:
             restoreReplica();
@@ -483,8 +481,6 @@ void InterpreterSystemQuery::restoreReplica()
 
 StoragePtr InterpreterSystemQuery::tryRestartReplica(const StorageID & replica, ContextMutablePtr system_context, bool need_ddl_guard)
 {
-    getContext()->checkAccess(AccessType::SYSTEM_RESTART_REPLICA, replica);
-
     auto table_ddl_guard = need_ddl_guard
         ? DatabaseCatalog::instance().getDDLGuard(replica.getDatabaseName(), replica.getTableName())
         : nullptr;
@@ -529,15 +525,36 @@ StoragePtr InterpreterSystemQuery::tryRestartReplica(const StorageID & replica,
     return table;
 }
 
+void InterpreterSystemQuery::restartReplica(const StorageID & replica, ContextMutablePtr system_context)
+{
+    getContext()->checkAccess(AccessType::SYSTEM_RESTART_REPLICA, replica);
+    if (!tryRestartReplica(replica, system_context))
+        throw Exception(ErrorCodes::BAD_ARGUMENTS, table_is_not_replicated.data(), replica.getNameForLogs());
+}
+
 void InterpreterSystemQuery::restartReplicas(ContextMutablePtr system_context)
 {
     std::vector<StorageID> replica_names;
     auto & catalog = DatabaseCatalog::instance();
 
+    auto access = getContext()->getAccess();
+    bool access_is_granted_globally = access->isGranted(AccessType::SYSTEM_RESTART_REPLICA);
+
     for (auto & elem : catalog.getDatabases())
+    {
         for (auto it = elem.second->getTablesIterator(getContext()); it->isValid(); it->next())
+        {
             if (dynamic_cast<const StorageReplicatedMergeTree *>(it->table().get()))
+            {
+                if (!access_is_granted_globally && !access->isGranted(AccessType::SYSTEM_RESTART_REPLICA, elem.first, it->name()))
+                {
+                    LOG_INFO(log, "Access {} denied, skipping {}.{}", "SYSTEM RESTART REPLICA", elem.first, it->name());
+                    continue;
+                }
                 replica_names.emplace_back(it->databaseName(), it->name());
+            }
+        }
+    }
 
     if (replica_names.empty())
         return;
@@ -583,14 +600,22 @@ void InterpreterSystemQuery::dropReplica(ASTSystemQuery & query)
     }
     else if (query.is_drop_whole_replica)
     {
-        getContext()->checkAccess(AccessType::SYSTEM_DROP_REPLICA);
         auto databases = DatabaseCatalog::instance().getDatabases();
+        auto access = getContext()->getAccess();
+        bool access_is_granted_globally = access->isGranted(AccessType::SYSTEM_DROP_REPLICA);
 
         for (auto & elem : databases)
         {
             DatabasePtr & database = elem.second;
             for (auto iterator = database->getTablesIterator(getContext()); iterator->isValid(); iterator->next())
+            {
+                if (!access_is_granted_globally && !access->isGranted(AccessType::SYSTEM_DROP_REPLICA, elem.first, iterator->name()))
+                {
+                    LOG_INFO(log, "Access {} denied, skipping {}.{}", "SYSTEM DROP REPLICA", elem.first, iterator->name());
+                    continue;
+                }
                 dropReplicaImpl(query, iterator->table());
+            }
             LOG_TRACE(log, "Dropped replica {} from database {}", query.replica, backQuoteIfNeed(database->getDatabaseName()));
         }
     }

src/Interpreters/InterpreterSystemQuery.h

@@ -47,6 +47,7 @@ private:
     /// Returns pointer to a newly created table if the restart was successful
     StoragePtr tryRestartReplica(const StorageID & replica, ContextMutablePtr context, bool need_ddl_guard = true);
 
+    void restartReplica(const StorageID & replica, ContextMutablePtr system_context);
     void restartReplicas(ContextMutablePtr system_context);
     void syncReplica(ASTSystemQuery & query);
 

tests/queries/0_stateless/01271_show_privileges.reference

@@ -99,14 +99,14 @@ SYSTEM RELOAD FUNCTION	['SYSTEM RELOAD FUNCTIONS','RELOAD FUNCTION','RELOAD FUNC
 SYSTEM RELOAD EMBEDDED DICTIONARIES	['RELOAD EMBEDDED DICTIONARIES']	GLOBAL	SYSTEM RELOAD
 SYSTEM RELOAD	[]	\N	SYSTEM
 SYSTEM RESTART DISK	['SYSTEM RESTART DISK']	GLOBAL	SYSTEM
-SYSTEM MERGES	['SYSTEM STOP MERGES','SYSTEM START MERGES','STOP_MERGES','START MERGES']	TABLE	SYSTEM
+SYSTEM MERGES	['SYSTEM STOP MERGES','SYSTEM START MERGES','STOP MERGES','START MERGES']	TABLE	SYSTEM
 SYSTEM TTL MERGES	['SYSTEM STOP TTL MERGES','SYSTEM START TTL MERGES','STOP TTL MERGES','START TTL MERGES']	TABLE	SYSTEM
 SYSTEM FETCHES	['SYSTEM STOP FETCHES','SYSTEM START FETCHES','STOP FETCHES','START FETCHES']	TABLE	SYSTEM
 SYSTEM MOVES	['SYSTEM STOP MOVES','SYSTEM START MOVES','STOP MOVES','START MOVES']	TABLE	SYSTEM
 SYSTEM DISTRIBUTED SENDS	['SYSTEM STOP DISTRIBUTED SENDS','SYSTEM START DISTRIBUTED SENDS','STOP DISTRIBUTED SENDS','START DISTRIBUTED SENDS']	TABLE	SYSTEM SENDS
-SYSTEM REPLICATED SENDS	['SYSTEM STOP REPLICATED SENDS','SYSTEM START REPLICATED SENDS','STOP_REPLICATED_SENDS','START REPLICATED SENDS']	TABLE	SYSTEM SENDS
+SYSTEM REPLICATED SENDS	['SYSTEM STOP REPLICATED SENDS','SYSTEM START REPLICATED SENDS','STOP REPLICATED SENDS','START REPLICATED SENDS']	TABLE	SYSTEM SENDS
 SYSTEM SENDS	['SYSTEM STOP SENDS','SYSTEM START SENDS','STOP SENDS','START SENDS']	\N	SYSTEM
-SYSTEM REPLICATION QUEUES	['SYSTEM STOP REPLICATION QUEUES','SYSTEM START REPLICATION QUEUES','STOP_REPLICATION_QUEUES','START REPLICATION QUEUES']	TABLE	SYSTEM
+SYSTEM REPLICATION QUEUES	['SYSTEM STOP REPLICATION QUEUES','SYSTEM START REPLICATION QUEUES','STOP REPLICATION QUEUES','START REPLICATION QUEUES']	TABLE	SYSTEM
 SYSTEM DROP REPLICA	['DROP REPLICA']	TABLE	SYSTEM
 SYSTEM SYNC REPLICA	['SYNC REPLICA']	TABLE	SYSTEM
 SYSTEM RESTART REPLICA	['RESTART REPLICA']	TABLE	SYSTEM

tests/testflows/rbac/regression.py

@@ -27,7 +27,6 @@ issue_17653 = "https://github.com/ClickHouse/ClickHouse/issues/17653"
 issue_17655 = "https://github.com/ClickHouse/ClickHouse/issues/17655"
 issue_17766 = "https://github.com/ClickHouse/ClickHouse/issues/17766"
 issue_18110 = "https://github.com/ClickHouse/ClickHouse/issues/18110"
-issue_18206 = "https://github.com/ClickHouse/ClickHouse/issues/18206"
 issue_21083 = "https://github.com/ClickHouse/ClickHouse/issues/21083"
 issue_21084 = "https://github.com/ClickHouse/ClickHouse/issues/21084"
 issue_25413 = "https://github.com/ClickHouse/ClickHouse/issues/25413"
@@ -122,20 +121,6 @@ xfails = {
         [(Fail, issue_17655)],
     "privileges/public tables/sensitive tables":
         [(Fail, issue_18110)],
-    "privileges/system merges/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system ttl merges/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system moves/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system sends/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system fetches/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system restart replica/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
-    "privileges/system replication queues/:/:/:/:/SYSTEM:":
-        [(Fail, issue_18206)],
     "privileges/: row policy/nested live:":
         [(Fail, issue_21083)],
     "privileges/: row policy/nested mat:":