From 8c8dd0c0723dc227ed2173f3618d596bcae7c2d4 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 10 Nov 2026 12:16:28 +0100 Subject: [PATCH] [Server] Serve admin actions through the database interface (#7847) cmdBanFromServer, the admin-flag helpers, cmdForceActivateUser and the admin-notes commands still prepared their statements inline through Servatrice_DatabaseInterface, and addWarning was only reachable through the concrete Servatrice class. Add setAdminFlag, getUserToken, getAdminNotes, setAdminNotes, addBan and getUserNamesByClientId to Server_DatabaseInterface with fail-closed defaults, promote addWarning, and model the outcomes as UserFieldResult and AdminFlagUpdateResult. Notifications, the audit record, the ban disconnect fan-out and every response code stay in the handler. --- .../server/remote/server_database_interface.h | 52 +++++++ .../src/servatrice_database_interface.cpp | 133 ++++++++++++++++++ .../src/servatrice_database_interface.h | 18 ++- servatrice/src/serversocketinterface.cpp | 107 ++++++-------- 4 files changed, 244 insertions(+), 66 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 1f3351ac2..1d18603a3 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h @@ -774,6 +774,58 @@ public: virtual void setForcePasswordChange(const QString & /* user */, bool /* force */) { } + /** @brief Outcome of reading or writing one account field. */ + enum class UserFieldResult + { + Ok, + NotFound, + Failed + }; + /** @brief Outcome of granting or revoking admin flags; the caller logs the error text. */ + struct AdminFlagUpdateResult + { + bool success = false; + QString errorText; + }; + virtual AdminFlagUpdateResult setAdminFlag(const QString & /* userName */, int /* flag */, bool /* add */) + { + return {}; + } + virtual UserFieldResult getUserToken(const QString & /* userName */, QString & /* token */) + { + return UserFieldResult::Failed; + } + virtual UserFieldResult getAdminNotes(const QString & /* userName */, QString & /* notes */) + { + return UserFieldResult::Failed; + } + virtual UserFieldResult setAdminNotes(const QString & /* userName */, const QString & /* notes */) + { + return UserFieldResult::Failed; + } + /** @brief Record a ban; the caller proceeds even when the insert fails, as before. */ + virtual bool addBan(const QString & /* userName */, + const QString & /* address */, + int /* adminId */, + int /* minutes */, + const QString & /* reason */, + const QString & /* visibleReason */, + const QString & /* clientId */) + { + return false; + } + /** @brief All user names carrying a client id; false when the lookup failed. */ + virtual bool getUserNamesByClientId(const QString & /* clientId */, QList & /* userNames */) + { + return false; + } + virtual bool addWarning(const QString /* userName */, + const QString /* adminName */, + const QString /* warningReason */, + const QString /* clientID */) + { + return false; + } /** * @brief Whether the given card art may be shown (card art rules). * diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index c60d6feae..6d8eb4588 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -2437,6 +2437,139 @@ bool Servatrice_DatabaseInterface::addWarning(const QString userName, return true; } +Server_DatabaseInterface::AdminFlagUpdateResult +Servatrice_DatabaseInterface::setAdminFlag(const QString &userName, int flag, bool add) +{ + AdminFlagUpdateResult result; + if (!checkSql()) { + return result; + } + + QSqlQuery *query = + add ? prepareQuery("update {prefix}_users set admin = (admin | :adminlevel) where name = :username") + : prepareQuery("update {prefix}_users set admin = (admin & ~ :adminlevel) where name = :username"); + query->bindValue(":adminlevel", flag); + query->bindValue(":username", userName); + if (!execSqlQuery(query)) { + result.errorText = query->lastError().text(); + return result; + } + + result.success = true; + return result; +} + +Server_DatabaseInterface::UserFieldResult Servatrice_DatabaseInterface::getUserToken(const QString &userName, + QString &token) +{ + if (!checkSql()) { + return UserFieldResult::Failed; + } + + QSqlQuery *query = prepareQuery("select token from {prefix}_users WHERE name = :name"); + query->bindValue(":name", userName); + if (!execSqlQuery(query)) { + return UserFieldResult::Failed; + } + + if (!query->next()) { + return UserFieldResult::NotFound; + } + + token = query->value(0).toString(); + return UserFieldResult::Ok; +} + +Server_DatabaseInterface::UserFieldResult Servatrice_DatabaseInterface::getAdminNotes(const QString &userName, + QString ¬es) +{ + if (!checkSql()) { + return UserFieldResult::Failed; + } + + QSqlQuery *query = prepareQuery("select adminnotes from {prefix}_users WHERE name = :name"); + query->bindValue(":name", userName); + if (!execSqlQuery(query)) { + return UserFieldResult::Failed; + } + + if (!query->next()) { + return UserFieldResult::NotFound; + } + + notes = query->value(0).toString(); + return UserFieldResult::Ok; +} + +Server_DatabaseInterface::UserFieldResult Servatrice_DatabaseInterface::setAdminNotes(const QString &userName, + const QString ¬es) +{ + if (!checkSql()) { + return UserFieldResult::Failed; + } + + QSqlQuery *query = prepareQuery("update {prefix}_users set adminnotes = :adminnotes where name = :name"); + query->bindValue(":adminnotes", notes); + query->bindValue(":name", userName); + + if (!execSqlQuery(query)) { + return UserFieldResult::Failed; + } + + if (query->numRowsAffected() == 0) { + return UserFieldResult::NotFound; + } + + return UserFieldResult::Ok; +} + +bool Servatrice_DatabaseInterface::addBan(const QString &userName, + const QString &address, + int adminId, + int minutes, + const QString &reason, + const QString &visibleReason, + const QString &clientId) +{ + if (!checkSql()) { + return false; + } + + QSqlQuery *query = prepareQuery( + "insert into {prefix}_bans (user_name, ip_address, id_admin, time_from, minutes, reason, visible_reason, " + "clientid) values(:user_name, :ip_address, :id_admin, NOW(), :minutes, :reason, :visible_reason, :client_id)"); + query->bindValue(":user_name", userName); + query->bindValue(":ip_address", address); + query->bindValue(":id_admin", adminId); + query->bindValue(":minutes", minutes); + query->bindValue(":reason", reason); + query->bindValue(":visible_reason", visibleReason); + query->bindValue(":client_id", clientId); + return execSqlQuery(query); +} + +bool Servatrice_DatabaseInterface::getUserNamesByClientId(const QString &clientId, QList &userNames) +{ + if (!checkSql()) { + return false; + } + + QSqlQuery *query = prepareQuery("select name from {prefix}_users where clientid = :client_id"); + query->bindValue(":client_id", clientId); + // The original call executed the query twice: the first result was discarded and the + // second governed the loop. Both executions are kept so the failure behaviour is identical. + execSqlQuery(query); + if (!execSqlQuery(query)) { + return false; + } + + while (query->next()) { + userNames.append(query->value(0).toString()); + } + + return true; +} + QList Servatrice_DatabaseInterface::getUserWarnHistory(const QString &userName) { QList results; diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 076ab6523..60e86f285 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -224,8 +224,22 @@ public: bool newPasswordNeedsHash) override; void setForcePasswordChange(const QString &user, bool force) override; QList getUserBanHistory(const QString &userName) override; - bool - addWarning(const QString userName, const QString adminName, const QString warningReason, const QString clientID); + bool addWarning(const QString userName, + const QString adminName, + const QString warningReason, + const QString clientID) override; + Server_DatabaseInterface::AdminFlagUpdateResult setAdminFlag(const QString &userName, int flag, bool add) override; + Server_DatabaseInterface::UserFieldResult getUserToken(const QString &userName, QString &token) override; + Server_DatabaseInterface::UserFieldResult getAdminNotes(const QString &userName, QString ¬es) override; + Server_DatabaseInterface::UserFieldResult setAdminNotes(const QString &userName, const QString ¬es) override; + bool addBan(const QString &userName, + const QString &address, + int adminId, + int minutes, + const QString &reason, + const QString &visibleReason, + const QString &clientId) override; + bool getUserNamesByClientId(const QString &clientId, QList &userNames) override; QList getUserWarnHistory(const QString &userName) override; QList getMessageLogHistory(const QString &user, const QString &ipaddress, diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 801a40de8..9a944b870 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1668,7 +1668,7 @@ void AbstractServerSocketInterface::removeSaidMessages(const QString &userName, Response::ResponseCode AbstractServerSocketInterface::cmdWarnUser(const Command_WarnUser &cmd, ResponseContainer & /*rc*/) { - if (!sqlInterface->checkSql()) { // sql database is required, without database there are no moderators anyway + if (!databaseInterface->checkSql()) { // sql database is required, without database there are no moderators anyway return Response::RespInternalError; } @@ -1681,7 +1681,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdWarnUser(const Command_ removeSaidMessages(userName, amountRemove); } - if (sqlInterface->addWarning(userName, sendingModerator, warningReason, clientId)) { + if (databaseInterface->addWarning(userName, sendingModerator, warningReason, clientId)) { servatrice->clientsLock.lockForRead(); AbstractServerSocketInterface *user = static_cast(server->getUsers().value(userName)); @@ -1713,7 +1713,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdWarnUser(const Command_ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Command_BanFromServer &cmd, ResponseContainer & /*rc*/) { - if (!sqlInterface->checkSql()) { + if (!databaseInterface->checkSql()) { return Response::RespInternalError; } @@ -1735,17 +1735,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com address = ""; } - QSqlQuery *query = sqlInterface->prepareQuery( - "insert into {prefix}_bans (user_name, ip_address, id_admin, time_from, minutes, reason, visible_reason, " - "clientid) values(:user_name, :ip_address, :id_admin, NOW(), :minutes, :reason, :visible_reason, :client_id)"); - query->bindValue(":user_name", userName); - query->bindValue(":ip_address", address); - query->bindValue(":id_admin", userInfo->id()); - query->bindValue(":minutes", minutes); - query->bindValue(":reason", textFromStdString(cmd.reason())); - query->bindValue(":visible_reason", visibleReason); - query->bindValue(":client_id", nameFromStdString(cmd.clientid())); - sqlInterface->execSqlQuery(query); + databaseInterface->addBan(userName, address, userInfo->id(), minutes, textFromStdString(cmd.reason()), + visibleReason, nameFromStdString(cmd.clientid())); servatrice->clientsLock.lockForRead(); QList moderatorList = server->getOnlineModeratorList(); @@ -1760,15 +1751,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com } if (userName.isEmpty() && address.isEmpty() && (!clientId.isEmpty())) { - QSqlQuery *clientIdQuery = - sqlInterface->prepareQuery("select name from {prefix}_users where clientid = :client_id"); - clientIdQuery->bindValue(":client_id", nameFromStdString(cmd.clientid())); - sqlInterface->execSqlQuery(clientIdQuery); - if (!sqlInterface->execSqlQuery(clientIdQuery)) { + QList clientIdUserNames; + if (!databaseInterface->getUserNamesByClientId(nameFromStdString(cmd.clientid()), clientIdUserNames)) { qCWarning(AbstractServerSocketInterfaceLog) << "ClientID username ban lookup failed: SQL Error"; } else { - while (clientIdQuery->next()) { - userName = clientIdQuery->value(0).toString(); + for (const QString &name : clientIdUserNames) { + userName = name; AbstractServerSocketInterface *user = static_cast(server->getUsers().value(userName)); if (user && !userList.contains(user)) { @@ -3573,12 +3561,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReloadConfig(const Comm bool AbstractServerSocketInterface::addAdminFlagToUser(const QString &userName, int flag) { - QSqlQuery *query = - sqlInterface->prepareQuery("update {prefix}_users set admin = (admin | :adminlevel) where name = :username"); - query->bindValue(":adminlevel", flag); - query->bindValue(":username", userName); - if (!sqlInterface->execSqlQuery(query)) { - logger->logMessage(QString("Failed to promote user %1: %2").arg(userName).arg(query->lastError().text())); + const auto result = databaseInterface->setAdminFlag(userName, flag, true); + if (!result.success) { + logger->logMessage(QString("Failed to promote user %1: %2").arg(userName).arg(result.errorText)); return false; } @@ -3597,12 +3582,9 @@ bool AbstractServerSocketInterface::addAdminFlagToUser(const QString &userName, bool AbstractServerSocketInterface::removeAdminFlagFromUser(const QString &userName, int flag) { - QSqlQuery *query = - sqlInterface->prepareQuery("update {prefix}_users set admin = (admin & ~ :adminlevel) where name = :username"); - query->bindValue(":adminlevel", flag); - query->bindValue(":username", userName); - if (!sqlInterface->execSqlQuery(query)) { - logger->logMessage(QString("Failed to demote user %1: %2").arg(userName).arg(query->lastError().text())); + const auto result = databaseInterface->setAdminFlag(userName, flag, false); + if (!result.success) { + logger->logMessage(QString("Failed to demote user %1: %2").arg(userName).arg(result.errorText)); return false; } @@ -3686,17 +3668,17 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForceActivateUser(const ResponseContainer &rc) { // Determine if user exists - auto *getUserTokenQuery = sqlInterface->prepareQuery("select token from {prefix}_users WHERE name = :name"); - getUserTokenQuery->bindValue(":name", QString::fromStdString(cmd.username_to_activate())); - if (!sqlInterface->execSqlQuery(getUserTokenQuery)) { - // Internal server error - return Response::RespInternalError; + QString token; + switch (databaseInterface->getUserToken(QString::fromStdString(cmd.username_to_activate()), token)) { + case Server_DatabaseInterface::UserFieldResult::Ok: + break; + case Server_DatabaseInterface::UserFieldResult::NotFound: + // User doesn't exist + return Response::RespNameNotFound; + case Server_DatabaseInterface::UserFieldResult::Failed: + // Internal server error + return Response::RespInternalError; } - if (!getUserTokenQuery->next()) { - // User doesn't exist - return Response::RespNameNotFound; - } - const auto &token = getUserTokenQuery->value(0).toString(); // Add audit log that Moderator activated account on behalf of user const auto &msg = QString("Attempt Force Activation by %1").arg(QString::fromStdString(cmd.moderator_name())); @@ -3715,17 +3697,17 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForceActivateUser(const Response::ResponseCode AbstractServerSocketInterface::cmdGetAdminNotes(const Command_GetAdminNotes &cmd, ResponseContainer &rc) { - auto *getAdminNotesQuery = sqlInterface->prepareQuery("select adminnotes from {prefix}_users WHERE name = :name"); - getAdminNotesQuery->bindValue(":name", QString::fromStdString(cmd.user_name())); - if (!sqlInterface->execSqlQuery(getAdminNotesQuery)) { - // Internal server error - return Response::RespInternalError; + QString adminNotes; + switch (databaseInterface->getAdminNotes(QString::fromStdString(cmd.user_name()), adminNotes)) { + case Server_DatabaseInterface::UserFieldResult::Ok: + break; + case Server_DatabaseInterface::UserFieldResult::NotFound: + // User doesn't exist + return Response::RespNameNotFound; + case Server_DatabaseInterface::UserFieldResult::Failed: + // Internal server error + return Response::RespInternalError; } - if (!getAdminNotesQuery->next()) { - // User doesn't exist - return Response::RespNameNotFound; - } - const auto &adminNotes = getAdminNotesQuery->value(0).toString(); Response_GetAdminNotes *re = new Response_GetAdminNotes; re->set_user_name(cmd.user_name()); @@ -3738,17 +3720,14 @@ Response::ResponseCode AbstractServerSocketInterface::cmdGetAdminNotes(const Com Response::ResponseCode AbstractServerSocketInterface::cmdUpdateAdminNotes(const Command_UpdateAdminNotes &cmd, ResponseContainer & /*rc*/) { - auto *updateAdminNotesQuery = - sqlInterface->prepareQuery("update {prefix}_users set adminnotes = :adminnotes where name = :name"); - updateAdminNotesQuery->bindValue(":adminnotes", QString::fromStdString(cmd.notes())); - updateAdminNotesQuery->bindValue(":name", QString::fromStdString(cmd.user_name())); - - if (!sqlInterface->execSqlQuery(updateAdminNotesQuery)) { - return Response::RespInternalError; - } - - if (updateAdminNotesQuery->numRowsAffected() == 0) { - return Response::RespNameNotFound; + switch (databaseInterface->setAdminNotes(QString::fromStdString(cmd.user_name()), + QString::fromStdString(cmd.notes()))) { + case Server_DatabaseInterface::UserFieldResult::Ok: + break; + case Server_DatabaseInterface::UserFieldResult::NotFound: + return Response::RespNameNotFound; + case Server_DatabaseInterface::UserFieldResult::Failed: + return Response::RespInternalError; } return Response::RespOk;