From 2608228bdddf2e048686fec21586ea00eb91c661 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 10 Nov 2026 12:16:29 +0100 Subject: [PATCH] [Server] Serve account moderation through the database interface (#7849) The moderator account commands still reached Servatrice_DatabaseInterface through its concrete class: cmdResetUserPassword ran the privilege lookup inline and called the concrete-only changeUserPasswordWithLegacyBackup, while the avatar, banner and blacklisted-banner commands called concrete-only helpers. Promote those four methods to Server_DatabaseInterface, move RemoveUserBannerResult into the interface, and add getUserAdminFlags for the reset privilege lookup. The escalation guard, audit records, had_banner response field and every response code stay in the handler. --- .../server/remote/server_database_interface.h | 31 ++++++++++++++++++ .../src/servatrice_database_interface.cpp | 16 ++++++++++ .../src/servatrice_database_interface.h | 17 +++------- servatrice/src/serversocketinterface.cpp | 32 +++++++++---------- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h index 56e3373ef..abdeab6c1 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h @@ -771,6 +771,12 @@ public: { return false; } + virtual bool changeUserPasswordWithLegacyBackup(const QString & /* user */, + const QString & /* passwordVerifier */, + const QString & /* legacyCredential */) + { + return false; + } virtual void setForcePasswordChange(const QString & /* user */, bool /* force */) { } @@ -792,6 +798,31 @@ public: { return false; } + /** @brief Outcome of removing a user's banner, so a missing user stays distinguishable. */ + enum class RemoveUserBannerResult + { + Removed, + NothingToRemove, + NoSuchUser, + DatabaseError + }; + virtual RemoveUserBannerResult removeUserBanner(const QString & /* userName */) + { + return RemoveUserBannerResult::DatabaseError; + } + virtual bool removeUserAvatar(const QString & /* userName */) + { + return false; + } + virtual int clearBlacklistedBanners() + { + return -1; + } + /** @brief The raw admin bitmask of a user; false when the query failed or the user is missing. */ + virtual bool getUserAdminFlags(const QString & /* userName */, int & /* adminFlags */) + { + return false; + } /** @brief Outcome of reading or writing one account field. */ enum class UserFieldResult { diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 96f1ae423..4f651ba08 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -2289,6 +2289,22 @@ bool Servatrice_DatabaseInterface::setUserAvatar(int userId, const QByteArray &i return execSqlQuery(query); } +bool Servatrice_DatabaseInterface::getUserAdminFlags(const QString &userName, int &adminFlags) +{ + if (!checkSql()) { + return false; + } + + QSqlQuery *query = prepareQuery("SELECT admin FROM {prefix}_users WHERE name = :name"); + query->bindValue(":name", userName); + if (!execSqlQuery(query) || !query->next()) { + return false; + } + + adminFlags = query->value(0).toInt(); + return true; +} + bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, const QString &oldPassword, bool oldPasswordNeedsHash, diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 27e8670a9..98be67994 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -216,7 +216,7 @@ public: * same password as the downgrade backup for pre-challenge clients. */ bool changeUserPasswordWithLegacyBackup(const QString &user, const QString &passwordVerifier, - const QString &legacyCredential); + const QString &legacyCredential) override; bool changeUserPassword(const QString &user, const QString &oldPassword, bool oldPasswordNeedsHash, @@ -276,19 +276,12 @@ public: // Uptime snapshot as recorded by Servatrice::statusUpdate() into the // {prefix}_uptime table. valid is false when no snapshot exists yet. UptimeSnapshot getLatestUptimeSnapshot(int serverId) override; - bool removeUserAvatar(const QString &userName); - + bool removeUserAvatar(const QString &userName) override; // Outcome of removeUserBanner(), so the caller can tell a missing user apart // from a user who simply has no banner card set. - enum class RemoveUserBannerResult - { - Removed, - NothingToRemove, - NoSuchUser, - DatabaseError, - }; - RemoveUserBannerResult removeUserBanner(const QString &userName); - int clearBlacklistedBanners(); + RemoveUserBannerResult removeUserBanner(const QString &userName) override; + int clearBlacklistedBanners() override; + bool getUserAdminFlags(const QString &userName, int &adminFlags) override; bool isCardArtAllowed(const QString &cardName, const QString &cardProviderId) override; bool addForgotPassword(const QString &user) override; bool removeForgotPassword(const QString &user) override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 7d8688db7..93bb7e585 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -2184,7 +2184,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdGetModeratorLastLogins( Response::ResponseCode AbstractServerSocketInterface::cmdResetUserPassword(const Command_ResetUserPassword &cmd, ResponseContainer &rc) { - if (!sqlInterface->checkSql()) { + if (!databaseInterface->checkSql()) { return Response::RespInternalError; } @@ -2194,12 +2194,10 @@ Response::ResponseCode AbstractServerSocketInterface::cmdResetUserPassword(const } // Look up the target user's privilege level to prevent escalation. - QSqlQuery *privQuery = sqlInterface->prepareQuery("SELECT admin FROM {prefix}_users WHERE name = :name"); - privQuery->bindValue(":name", userName); - if (!sqlInterface->execSqlQuery(privQuery) || !privQuery->next()) { + int targetAdmin = 0; + if (!databaseInterface->getUserAdminFlags(userName, targetAdmin)) { return Response::RespNameNotFound; } - const int targetAdmin = privQuery->value(0).toInt(); const bool targetIsAdmin = targetAdmin & 1; const bool targetIsMod = targetAdmin & 2; @@ -2219,10 +2217,10 @@ Response::ResponseCode AbstractServerSocketInterface::cmdResetUserPassword(const // have none, which changeUserPasswordWithLegacyBackup warns about. const QString verifier = PasswordHasher::generatePasswordVerifier(tempPassword); const QString legacyCredential = PasswordHasher::computeHash(tempPassword, PasswordHasher::generateRandomSalt()); - if (!sqlInterface->changeUserPasswordWithLegacyBackup(userName, verifier, legacyCredential)) { + if (!databaseInterface->changeUserPasswordWithLegacyBackup(userName, verifier, legacyCredential)) { return Response::RespInternalError; } - sqlInterface->setForcePasswordChange(userName, true); + databaseInterface->setForcePasswordChange(userName, true); databaseInterface->addAuditRecord(userName, this->getAddress(), QString::fromStdString(userInfo->clientid()), "PASSWORD_RESET", "Admin password reset", true); @@ -2253,7 +2251,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdResetUserPassword(const Response::ResponseCode AbstractServerSocketInterface::cmdRemoveUserAvatar(const Command_RemoveUserAvatar &cmd, ResponseContainer &rc) { - if (!sqlInterface->checkSql()) { + if (!databaseInterface->checkSql()) { return Response::RespInternalError; } @@ -2262,7 +2260,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRemoveUserAvatar(const return Response::RespContextError; } - if (!sqlInterface->removeUserAvatar(userName)) { + if (!databaseInterface->removeUserAvatar(userName)) { return Response::RespInternalError; } @@ -2278,7 +2276,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRemoveUserAvatar(const Response::ResponseCode AbstractServerSocketInterface::cmdRemoveUserBanner(const Command_RemoveUserBanner &cmd, ResponseContainer &rc) { - if (!sqlInterface->checkSql()) { + if (!databaseInterface->checkSql()) { return Response::RespInternalError; } @@ -2288,15 +2286,15 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRemoveUserBanner(const } bool hadBanner = true; - switch (sqlInterface->removeUserBanner(userName)) { - case Servatrice_DatabaseInterface::RemoveUserBannerResult::Removed: + switch (databaseInterface->removeUserBanner(userName)) { + case Server_DatabaseInterface::RemoveUserBannerResult::Removed: break; - case Servatrice_DatabaseInterface::RemoveUserBannerResult::NothingToRemove: + case Server_DatabaseInterface::RemoveUserBannerResult::NothingToRemove: hadBanner = false; break; - case Servatrice_DatabaseInterface::RemoveUserBannerResult::NoSuchUser: + case Server_DatabaseInterface::RemoveUserBannerResult::NoSuchUser: return Response::RespNameNotFound; - case Servatrice_DatabaseInterface::RemoveUserBannerResult::DatabaseError: + case Server_DatabaseInterface::RemoveUserBannerResult::DatabaseError: default: return Response::RespInternalError; } @@ -2317,11 +2315,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdClearBlacklistedBanners(const Command_ClearBlacklistedBanners & /* cmd */, ResponseContainer &rc) { - if (!sqlInterface->checkSql()) { + if (!databaseInterface->checkSql()) { return Response::RespInternalError; } - const int cleared = sqlInterface->clearBlacklistedBanners(); + const int cleared = databaseInterface->clearBlacklistedBanners(); if (cleared < 0) { return Response::RespInternalError; }