[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.

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<QString> & /* 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).
      *

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 &notes)
+{
+    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 &notes)
+{
+    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<QString> &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<ServerInfo_Warning> Servatrice_DatabaseInterface::getUserWarnHistory(const QString &userName)
 {
     QList<ServerInfo_Warning> results;

servatrice/src/servatrice_database_interface.h

@@ -224,8 +224,22 @@ public:
                             bool newPasswordNeedsHash) override;
     void setForcePasswordChange(const QString &user, bool force) override;
     QList<ServerInfo_Ban> getUserBanHistory(const QString &userName) override;
-    bool
+    bool addWarning(const QString userName,
-    addWarning(const QString userName, const QString adminName, const QString warningReason, const QString clientID);
+                    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 &notes) override;
+    Server_DatabaseInterface::UserFieldResult setAdminNotes(const QString &userName, const QString &notes) 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<QString> &userNames) override;
     QList<ServerInfo_Warning> getUserWarnHistory(const QString &userName) override;
     QList<ServerInfo_ChatMessage> getMessageLogHistory(const QString &user,
                                                        const QString &ipaddress,

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<AbstractServerSocketInterface *>(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(
+    databaseInterface->addBan(userName, address, userInfo->id(), minutes, textFromStdString(cmd.reason()),
-        "insert into {prefix}_bans (user_name, ip_address, id_admin, time_from, minutes, reason, visible_reason, "
+                              visibleReason, nameFromStdString(cmd.clientid()));
-        "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);
 
     servatrice->clientsLock.lockForRead();
     QList<QString> moderatorList = server->getOnlineModeratorList();
@@ -1760,15 +1751,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdBanFromServer(const Com
     }
 
     if (userName.isEmpty() && address.isEmpty() && (!clientId.isEmpty())) {
-        QSqlQuery *clientIdQuery =
+        QList<QString> clientIdUserNames;
-            sqlInterface->prepareQuery("select name from {prefix}_users where clientid = :client_id");
+        if (!databaseInterface->getUserNamesByClientId(nameFromStdString(cmd.clientid()), clientIdUserNames)) {
-        clientIdQuery->bindValue(":client_id", nameFromStdString(cmd.clientid()));
-        sqlInterface->execSqlQuery(clientIdQuery);
-        if (!sqlInterface->execSqlQuery(clientIdQuery)) {
             qCWarning(AbstractServerSocketInterfaceLog) << "ClientID username ban lookup failed: SQL Error";
         } else {
-            while (clientIdQuery->next()) {
+            for (const QString &name : clientIdUserNames) {
-                userName = clientIdQuery->value(0).toString();
+                userName = name;
                 AbstractServerSocketInterface *user =
                     static_cast<AbstractServerSocketInterface *>(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 =
+    const auto result = databaseInterface->setAdminFlag(userName, flag, true);
-        sqlInterface->prepareQuery("update {prefix}_users set admin = (admin | :adminlevel) where name = :username");
+    if (!result.success) {
-    query->bindValue(":adminlevel", flag);
+        logger->logMessage(QString("Failed to promote user %1: %2").arg(userName).arg(result.errorText));
-    query->bindValue(":username", userName);
-    if (!sqlInterface->execSqlQuery(query)) {
-        logger->logMessage(QString("Failed to promote user %1: %2").arg(userName).arg(query->lastError().text()));
         return false;
     }
 
@@ -3597,12 +3582,9 @@ bool AbstractServerSocketInterface::addAdminFlagToUser(const QString &userName,
 
 bool AbstractServerSocketInterface::removeAdminFlagFromUser(const QString &userName, int flag)
 {
-    QSqlQuery *query =
+    const auto result = databaseInterface->setAdminFlag(userName, flag, false);
-        sqlInterface->prepareQuery("update {prefix}_users set admin = (admin & ~ :adminlevel) where name = :username");
+    if (!result.success) {
-    query->bindValue(":adminlevel", flag);
+        logger->logMessage(QString("Failed to demote user %1: %2").arg(userName).arg(result.errorText));
-    query->bindValue(":username", userName);
-    if (!sqlInterface->execSqlQuery(query)) {
-        logger->logMessage(QString("Failed to demote user %1: %2").arg(userName).arg(query->lastError().text()));
         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");
+    QString token;
-    getUserTokenQuery->bindValue(":name", QString::fromStdString(cmd.username_to_activate()));
+    switch (databaseInterface->getUserToken(QString::fromStdString(cmd.username_to_activate()), token)) {
-    if (!sqlInterface->execSqlQuery(getUserTokenQuery)) {
+        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");
+    QString adminNotes;
-    getAdminNotesQuery->bindValue(":name", QString::fromStdString(cmd.user_name()));
+    switch (databaseInterface->getAdminNotes(QString::fromStdString(cmd.user_name()), adminNotes)) {
-    if (!sqlInterface->execSqlQuery(getAdminNotesQuery)) {
+        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 =
+    switch (databaseInterface->setAdminNotes(QString::fromStdString(cmd.user_name()),
-        sqlInterface->prepareQuery("update {prefix}_users set adminnotes = :adminnotes where name = :name");
+                                             QString::fromStdString(cmd.notes()))) {
-    updateAdminNotesQuery->bindValue(":adminnotes", QString::fromStdString(cmd.notes()));
+        case Server_DatabaseInterface::UserFieldResult::Ok:
-    updateAdminNotesQuery->bindValue(":name", QString::fromStdString(cmd.user_name()));
+            break;
-
+        case Server_DatabaseInterface::UserFieldResult::NotFound:
-    if (!sqlInterface->execSqlQuery(updateAdminNotesQuery)) {
-        return Response::RespInternalError;
-    }
-
-    if (updateAdminNotesQuery->numRowsAffected() == 0) {
             return Response::RespNameNotFound;
+        case Server_DatabaseInterface::UserFieldResult::Failed:
+            return Response::RespInternalError;
     }
 
     return Response::RespOk;