From f7998deb7a8cef5d049d1d7432fd309b04267de1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Thu, 20 Aug 2026 11:15:15 +0200 Subject: [PATCH] [Server/Client/Protocol] Address PR #7091 review: security, bug, and perf fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - Promote RESET_USER_PASSWORD to admin-only dispatch (was moderator-accessible) - Reject password reset on users with equal/higher privilege than caller - Notify affected user via Event_NotifyUser::CUSTOM when password is reset - Add server-side category whitelist for reports - Drop reporter name fallback in comment/details authorization (ID-only) - Force password change: new DB column + login enforcement + client disconnect Bugs: - XSS via QTextEdit::append() → insertPlainText() in report tab and utils - allNotified initialized to true even with empty recipients list - Warning combo box: use currentData() instead of baked-in display text - Report resolution now records who resolved (resolved_by column + audit) Performance: - IP-correlation subquery: add 6-month window + LIMIT 200 - getUserSessions: clamp limit to 500 Non-blocking: - Palette-aware colors in report_utils.cpp (dark/light mode) - Report list pagination: offset/limit fields + total_count in response - SessionCommand enum gap comment for reserved values 1201-1203 Schema: 36→37 (force_password_change), 37→38 (resolved_by) Took 12 minutes Took 16 seconds --- .../remote_connection_controller.cpp | 9 ++ .../widgets/server/user/user_list_widget.cpp | 10 +-- .../src/interface/widgets/tabs/tab_report.cpp | 36 +++++--- .../widgets/utility/report_utils.cpp | 20 +++-- .../network/server/remote/server.h | 3 +- .../server/remote/server_database_interface.h | 3 + .../server/remote/server_protocolhandler.cpp | 2 + .../protocol/pb/admin_commands.proto | 8 ++ .../protocol/pb/command_report_list.proto | 2 + .../protocol/pb/moderator_commands.proto | 7 -- .../libcockatrice/protocol/pb/response.proto | 1 + .../protocol/pb/response_report_list.proto | 1 + .../protocol/pb/session_commands.proto | 1 + .../migrations/servatrice_0035_to_0036.sql | 8 +- servatrice/servatrice.sql | 5 +- .../src/servatrice_database_interface.cpp | 27 +++++- .../src/servatrice_database_interface.h | 1 + servatrice/src/serversocketinterface.cpp | 86 ++++++++++++++++--- 18 files changed, 181 insertions(+), 49 deletions(-) diff --git a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp index 4e425fb66..53dde125f 100644 --- a/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp +++ b/cockatrice/src/client/network/connection_controller/remote_connection_controller.cpp @@ -296,6 +296,15 @@ void ConnectionController::onLoginError(int r, return; } + case Response::RespPasswordChangeRequired: { + QMessageBox::information( + dialogParent, tr("Password Change Required"), + tr("An administrator has reset your password. Please contact your server administrator to obtain " + "your temporary password, then log in and change it via Account -> Change Password.")); + remoteClient->disconnectFromServer(); + return; + } + case Response::RespServerFull: { QMessageBox::critical(dialogParent, tr("Server Full"), tr("The server has reached its maximum user capacity, please check back later.")); diff --git a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp index 84ff3bf39..2534ee62c 100644 --- a/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp +++ b/cockatrice/src/interface/widgets/server/user/user_list_widget.cpp @@ -151,7 +151,7 @@ WarningDialog::WarningDialog(const QString userName, const QString clientID, QWi warnClientID = new QLineEdit(clientID); warnClientID->setMaxLength(MAX_NAME_LENGTH); warningOption = new QComboBox(); - warningOption->addItem(""); + warningOption->addItem("", ""); deleteMessages = new QCheckBox(tr("Redact all messages from this user in all rooms")); @@ -184,7 +184,7 @@ void WarningDialog::okClicked() return; } - if (warningOption->currentText().simplified().isEmpty()) { + if (warningOption->currentData().toString().simplified().isEmpty()) { QMessageBox::critical(this, tr("Error"), tr("Warning to use can not be blank, please select a valid warning to send.")); return; @@ -205,7 +205,7 @@ QString WarningDialog::getWarnID() const QString WarningDialog::getReason() const { - return warningOption->currentText().simplified(); + return warningOption->currentData().toString().simplified(); } int WarningDialog::getDeleteMessages() const @@ -216,9 +216,9 @@ int WarningDialog::getDeleteMessages() const void WarningDialog::addWarningOption(const QString warning, int startingIl) { if (startingIl > 1) { - warningOption->addItem(tr("%1 (IL %2)").arg(warning).arg(startingIl)); + warningOption->addItem(tr("%1 (IL %2)").arg(warning).arg(startingIl), warning); } else { - warningOption->addItem(warning); + warningOption->addItem(warning, warning); } } diff --git a/cockatrice/src/interface/widgets/tabs/tab_report.cpp b/cockatrice/src/interface/widgets/tabs/tab_report.cpp index ffea521d8..0b9108f9b 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_report.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_report.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -835,12 +836,13 @@ void TabReport::userInfoResponse(const Response &response) for (int i = 0; i < resp.recent_reports_size(); ++i) { const ServerInfo_Report &r = resp.recent_reports(i); QDateTime dt = QDateTime::fromSecsSinceEpoch(r.report_time()); - userContextRecentReports->append(QString("[%1] #%2 by %3 [%4]: %5") - .arg(dt.toString("yyyy-MM-dd")) - .arg(r.report_id()) - .arg(QString::fromStdString(r.reporter_name())) - .arg(QString::fromStdString(r.status())) - .arg(QString::fromStdString(r.category()))); + userContextRecentReports->moveCursor(QTextCursor::End); + userContextRecentReports->insertPlainText(QString("[%1] #%2 by %3 [%4]: %5\n") + .arg(dt.toString("yyyy-MM-dd")) + .arg(r.report_id()) + .arg(QString::fromStdString(r.reporter_name())) + .arg(QString::fromStdString(r.status())) + .arg(QString::fromStdString(r.category()))); } } } @@ -894,26 +896,32 @@ void TabReport::statsResponse(const Response &response) statsCategoriesLabel->setText(tr("By category:")); statsDetailText->clear(); - statsDetailText->append(tr("=== Top Categories ===")); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText(tr("=== Top Categories ===") + "\n"); for (int i = 0; i < resp.category_counts_size(); ++i) { const ReportCategoryCount &cc = resp.category_counts(i); QString cat = QString::fromStdString(cc.category()); cat.replace('_', ' '); cat = cat.left(1).toUpper() + cat.mid(1); - statsDetailText->append(QString(" %1: %2").arg(cat).arg(cc.count())); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText(QString(" %1: %2\n").arg(cat).arg(cc.count())); } - statsDetailText->append(tr("\n=== Most Reported Users ===")); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText("\n" + tr("=== Most Reported Users ===") + "\n"); for (int i = 0; i < resp.top_reported_users_size(); ++i) { const ReportTopUser &tu = resp.top_reported_users(i); - statsDetailText->append( - QString(" %1: %2 reports").arg(QString::fromStdString(tu.user_name())).arg(tu.count())); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText( + QString(" %1: %2 reports\n").arg(QString::fromStdString(tu.user_name())).arg(tu.count())); } - statsDetailText->append(tr("\n=== Top Reporters ===")); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText("\n" + tr("=== Top Reporters ===") + "\n"); for (int i = 0; i < resp.top_reporters_size(); ++i) { const ReportTopUser &tu = resp.top_reporters(i); - statsDetailText->append( - QString(" %1: %2 reports filed").arg(QString::fromStdString(tu.user_name())).arg(tu.count())); + statsDetailText->moveCursor(QTextCursor::End); + statsDetailText->insertPlainText( + QString(" %1: %2 reports filed\n").arg(QString::fromStdString(tu.user_name())).arg(tu.count())); } } diff --git a/cockatrice/src/interface/widgets/utility/report_utils.cpp b/cockatrice/src/interface/widgets/utility/report_utils.cpp index 260b38973..aa4558e5c 100644 --- a/cockatrice/src/interface/widgets/utility/report_utils.cpp +++ b/cockatrice/src/interface/widgets/utility/report_utils.cpp @@ -1,8 +1,11 @@ #include "report_utils.h" +#include #include #include +#include #include +#include #include namespace report_utils @@ -12,17 +15,21 @@ namespace { QColor reportStatusColor(const QString &status) { + const QColor text = qApp->palette().color(QPalette::Text); + const int luminance = (299 * text.red() + 587 * text.green() + 114 * text.blue()) / 1000; + const bool dark = luminance < 128; + if (status == "open") { - return QColor("#e74c3c"); + return dark ? QColor("#e06060") : QColor("#c0392b"); } if (status == "assigned") { - return QColor("#f39c12"); + return dark ? QColor("#e0a030") : QColor("#d68910"); } if (status == "resolved") { - return QColor("#27ae60"); + return dark ? QColor("#50c878") : QColor("#1e8449"); } if (status == "dismissed") { - return QColor("#95a5a6"); + return qApp->palette().color(QPalette::PlaceholderText); } return QColor(); } @@ -103,8 +110,9 @@ void renderReportDetails(QTextEdit *chatLogEdit, const QString author = QString::fromStdString(c.author_name()); const QString text = QString::fromStdString(c.comment_text()); const QString prefix = c.is_moderator() ? moderatorPrefix : nonModeratorPrefix; - commentsEdit->append( - QString("[%1] %2 %3:\n%4\n").arg(formatReportTime(c.comment_time()), prefix, author, text)); + commentsEdit->moveCursor(QTextCursor::End); + commentsEdit->insertPlainText( + QString("[%1] %2 %3:\n%4\n\n").arg(formatReportTime(c.comment_time()), prefix, author, text)); } } diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server.h b/libcockatrice_network/libcockatrice/network/server/remote/server.h index 12e71ebff..0ded27afa 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server.h @@ -38,7 +38,8 @@ enum AuthenticationResult UsernameInvalid, RegistrationRequired, UserIsInactive, - ClientIdRequired + ClientIdRequired, + PasswordChangeRequired }; class Server : public QObject 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 b43dbde42..1e4fc990b 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_database_interface.h @@ -172,6 +172,9 @@ public: { return false; } + virtual void setForcePasswordChange(const QString & /* user */, bool /* force */) + { + } }; #endif diff --git a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp index 2cde387bb..899df6529 100644 --- a/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp +++ b/libcockatrice_network/libcockatrice/network/server/remote/server_protocolhandler.cpp @@ -564,6 +564,8 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd return Response::RespClientIdRequired; case UserIsInactive: return Response::RespAccountNotActivated; + case PasswordChangeRequired: + return Response::RespPasswordChangeRequired; default: authState = res; usingRealPassword = needsHash; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/admin_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/admin_commands.proto index 8faaec2d2..f8b34b3f8 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/admin_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/admin_commands.proto @@ -5,6 +5,7 @@ message AdminCommand { SHUTDOWN_SERVER = 1001; RELOAD_CONFIG = 1002; ADJUST_MOD = 1003; + RESET_USER_PASSWORD = 1016; } extensions 100 to max; } @@ -37,3 +38,10 @@ message Command_AdjustMod { optional bool should_be_mod = 2; optional bool should_be_judge = 3; } + +message Command_ResetUserPassword { + extend AdminCommand { + optional Command_ResetUserPassword ext = 1016; + } + optional string user_name = 1; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_list.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_list.proto index 170e3bcbe..6993de1a7 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_list.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_report_list.proto @@ -6,4 +6,6 @@ message Command_ReportList { optional Command_ReportList ext = 1200; } optional bool unresolved_only = 1; + optional uint32 offset = 2 [default = 0]; + optional uint32 limit = 3 [default = 100]; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto index 11d16e9a4..685408830 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/moderator_commands.proto @@ -168,13 +168,6 @@ message Command_GetModeratorLastLogins { } } -message Command_ResetUserPassword { - extend ModeratorCommand { - optional Command_ResetUserPassword ext = 1016; - } - optional string user_name = 1; -} - message Command_RemoveUserAvatar { extend ModeratorCommand { optional Command_RemoveUserAvatar ext = 1017; diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto index e1f415ce6..14ba737b5 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto @@ -53,6 +53,7 @@ message Response { RespClientUpdateRequired = 35; // Client is missing features that the server is requiring RespServerFull = 36; // Server user limit reached RespEmailBlackListed = 37; // Server has blocked the email address provided for registration for some reason + RespPasswordChangeRequired = 38; // Server requires the user to change their password before proceeding } // Type of response, used to route handling on the client diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_report_list.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_report_list.proto index a5a78323b..73d9fbef8 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response_report_list.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_report_list.proto @@ -7,4 +7,5 @@ message Response_ReportList { optional Response_ReportList ext = 1210; } repeated ServerInfo_Report reports = 1; + optional uint32 total_count = 2; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index f1435c8bf..fee8c36a8 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -35,6 +35,7 @@ message SessionCommand { REPLAY_GET_CODE = 1104; REPLAY_SUBMIT_CODE = 1105; REPORT = 1200; + // 1201-1203 reserved: removed during squash REPORT_MY_LIST = 1204; REPORT_ADD_COMMENT = 1205; REPORT_DETAILS = 1206; diff --git a/servatrice/migrations/servatrice_0035_to_0036.sql b/servatrice/migrations/servatrice_0035_to_0036.sql index da5b31ec9..9727eeba5 100644 --- a/servatrice/migrations/servatrice_0035_to_0036.sql +++ b/servatrice/migrations/servatrice_0035_to_0036.sql @@ -15,6 +15,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_reports` ( `resolution_time` datetime NULL, `status` enum('open','assigned','resolved','dismissed') NOT NULL DEFAULT 'open', `assigned_to` int(7) unsigned NULL, + `resolved_by` int(7) unsigned NULL, `resolution_note` text, `notified` tinyint(1) NOT NULL DEFAULT 0, PRIMARY KEY (`id`), @@ -25,7 +26,8 @@ CREATE TABLE IF NOT EXISTS `cockatrice_reports` ( INDEX `idx_status_created` (`status`, `created_at`), FOREIGN KEY (`reporter_id`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, FOREIGN KEY (`reported_user_id`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, - FOREIGN KEY (`assigned_to`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE + FOREIGN KEY (`assigned_to`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY (`resolved_by`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; CREATE TABLE IF NOT EXISTS `cockatrice_report_comments` ( @@ -44,4 +46,8 @@ CREATE TABLE IF NOT EXISTS `cockatrice_report_comments` ( FOREIGN KEY (`author_id`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; +ALTER TABLE `cockatrice_users` + ADD COLUMN `force_password_change` tinyint(1) NOT NULL DEFAULT 0 + AFTER `passwordLastChangedDate`; + UPDATE cockatrice_schema_version SET version=36 WHERE version=35; diff --git a/servatrice/servatrice.sql b/servatrice/servatrice.sql index 8b940f706..5dbf69cbc 100644 --- a/servatrice/servatrice.sql +++ b/servatrice/servatrice.sql @@ -41,6 +41,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_users` ( `privlevelStartDate` datetime NOT NULL, `privlevelEndDate` datetime NOT NULL, `passwordLastChangedDate` datetime NOT NULL DEFAULT '0000-00-00 00:00:00', + `force_password_change` tinyint(1) NOT NULL DEFAULT 0, `leftPawnColorOverride` varchar(255), `rightPawnColorOverride` varchar(255), `card_art_params` TEXT DEFAULT NULL, @@ -248,6 +249,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_reports` ( `resolution_time` datetime NULL, `status` enum('open','assigned','resolved','dismissed') NOT NULL DEFAULT 'open', `assigned_to` int(7) unsigned NULL, + `resolved_by` int(7) unsigned NULL, `resolution_note` text, `notified` tinyint(1) NOT NULL DEFAULT 0, PRIMARY KEY (`id`), @@ -258,7 +260,8 @@ CREATE TABLE IF NOT EXISTS `cockatrice_reports` ( INDEX `idx_status_created` (`status`, `created_at`), FOREIGN KEY (`reporter_id`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, FOREIGN KEY (`reported_user_id`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, - FOREIGN KEY (`assigned_to`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE + FOREIGN KEY (`assigned_to`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE, + FOREIGN KEY (`resolved_by`) REFERENCES `cockatrice_users`(`id`) ON DELETE SET NULL ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; CREATE TABLE IF NOT EXISTS `cockatrice_report_comments` ( diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 41c7eea61..847be61da 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -340,8 +340,8 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot return UserIsBanned; } - QSqlQuery *passwordQuery = - prepareQuery("select password_sha512, active from {prefix}_users where name = :name"); + QSqlQuery *passwordQuery = prepareQuery( + "select password_sha512, active, force_password_change from {prefix}_users where name = :name"); passwordQuery->bindValue(":name", user); if (!execSqlQuery(passwordQuery)) { qCWarning(DatabaseInterfaceLog) << "Login denied: SQL error"; @@ -351,6 +351,7 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot if (passwordQuery->next()) { const QString correctPasswordSha512 = passwordQuery->value(0).toString(); const bool userIsActive = passwordQuery->value(1).toBool(); + const bool forceChange = passwordQuery->value(2).toBool(); if (!userIsActive) { qCWarning(DatabaseInterfaceLog) << "Login denied: user not active"; return UserIsInactive; @@ -362,6 +363,10 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot hashedPassword = password; } if (correctPasswordSha512 == hashedPassword) { + if (forceChange) { + qCDebug(DatabaseInterfaceLog) << "Login accepted but password change required"; + return PasswordChangeRequired; + } qCDebug(DatabaseInterfaceLog) << "Login accepted: password right"; return PasswordRight; } else { @@ -1091,6 +1096,18 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, return passwordQuery->numRowsAffected() > 0; } +void Servatrice_DatabaseInterface::setForcePasswordChange(const QString &user, bool force) +{ + if (!checkSql()) { + return; + } + + QSqlQuery *query = prepareQuery("UPDATE {prefix}_users SET force_password_change = :force WHERE name = :name"); + query->bindValue(":force", force ? 1 : 0); + query->bindValue(":name", user); + execSqlQuery(query); +} + bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user, const QString &oldPassword, bool oldPasswordNeedsHash, @@ -1383,8 +1400,10 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin } queryString.append(" OR u.name IN (SELECT DISTINCT s.user_name FROM {prefix}_sessions s " "WHERE s.ip_address IN (SELECT DISTINCT s2.ip_address FROM {prefix}_sessions s2 " - "WHERE s2.user_name = :user_name)) " - "ORDER BY u.name"); + "WHERE s2.user_name = :user_name" + " AND s2.start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH))" + " AND s.start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH)) " + "ORDER BY u.name LIMIT 200"); QSqlQuery *query = prepareQuery(queryString); query->bindValue(":user_name", userName); diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 6937ee8f6..184b201c5 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -122,6 +122,7 @@ public: bool oldPasswordNeedsHash, const QString &newPassword, bool newPasswordNeedsHash) override; + void setForcePasswordChange(const QString &user, bool force); QList getUserBanHistory(const QString userName); bool addWarning(const QString userName, const QString adminName, const QString warningReason, const QString clientID); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 95da90a71..2a8b5f0a4 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -311,8 +311,6 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedModeratorCo return cmdGetUserAlts(cmd.GetExtension(Command_GetUserAlts::ext), rc); case ModeratorCommand::GET_MODERATOR_LAST_LOGINS: return cmdGetModeratorLastLogins(cmd.GetExtension(Command_GetModeratorLastLogins::ext), rc); - case ModeratorCommand::RESET_USER_PASSWORD: - return cmdResetUserPassword(cmd.GetExtension(Command_ResetUserPassword::ext), rc); case ModeratorCommand::REMOVE_USER_AVATAR: return cmdRemoveUserAvatar(cmd.GetExtension(Command_RemoveUserAvatar::ext), rc); default: @@ -332,6 +330,8 @@ AbstractServerSocketInterface::processExtendedAdminCommand(int cmdType, const Ad return cmdReloadConfig(cmd.GetExtension(Command_ReloadConfig::ext), rc); case AdminCommand::ADJUST_MOD: return cmdAdjustMod(cmd.GetExtension(Command_AdjustMod::ext), rc); + case AdminCommand::RESET_USER_PASSWORD: + return cmdResetUserPassword(cmd.GetExtension(Command_ResetUserPassword::ext), rc); default: return Response::RespFunctionNotAllowed; } @@ -1318,11 +1318,25 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Comman } const bool unresolvedOnly = cmd.unresolved_only(); + const int offset = static_cast(cmd.offset()); + const int limit = qMin(static_cast(cmd.limit()), 1000); // Columns: 0=id, 1=reporter_name, 2=reported_user_name, 3=game_id, // 4=category, 5=description, 6=created_at, 7=status, // 8=resolution_note, 9=assigned_mod_name, // 10=room_id, 11=replay_id + QString whereClause; + if (unresolvedOnly) { + whereClause = "WHERE r.status = 'open' OR r.status = 'assigned' "; + } + + // Total count query + QSqlQuery *countQuery = sqlInterface->prepareQuery("SELECT COUNT(*) FROM {prefix}_reports r " + whereClause); + int totalCount = 0; + if (sqlInterface->execSqlQuery(countQuery) && countQuery->next()) { + totalCount = countQuery->value(0).toInt(); + } + QString queryStr = "SELECT r.id, r.reporter_name, r.reported_user_name, r.game_id, " "r.category, r.description, r.created_at, r.status, " "r.resolution_note, u.name AS assigned_mod_name, r.room_id, " @@ -1335,14 +1349,17 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportList(const Comman queryStr += "WHERE r.status = 'open' OR r.status = 'assigned' "; } - queryStr += "ORDER BY r.created_at DESC LIMIT 1000"; + queryStr += "ORDER BY r.created_at DESC LIMIT :limit OFFSET :offset"; QSqlQuery *query = sqlInterface->prepareQuery(queryStr); + query->bindValue(":limit", limit); + query->bindValue(":offset", offset); if (!sqlInterface->execSqlQuery(query)) { return Response::RespInternalError; } Response_ReportList *re = new Response_ReportList; + re->set_total_count(totalCount); while (query->next()) { ServerInfo_Report *info = re->add_reports(); @@ -1440,11 +1457,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportResolve(const Com QSqlQuery *query = sqlInterface->prepareQuery("UPDATE {prefix}_reports " "SET status = :status, resolution_note = :note, " - "resolution_time = NOW() " + "resolution_time = NOW(), resolved_by = :mod_id " "WHERE id = :id AND status IN ('open', 'assigned')"); query->bindValue(":status", newStatus); query->bindValue(":note", note); + query->bindValue(":mod_id", userInfo->id()); query->bindValue(":id", reportId); if (!sqlInterface->execSqlQuery(query)) { @@ -1455,6 +1473,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportResolve(const Com return Response::RespInvalidData; } + sqlInterface->addAuditRecord(QString::number(reportId), this->getAddress(), + QString::fromStdString(userInfo->clientid()), + dismissed ? "REPORT_DISMISSED" : "REPORT_RESOLVED", + QString("Report #%1 %2").arg(reportId).arg(newStatus), true); + // Notifying the reporter is best-effort: the resolve already succeeded. If any of the lookup // queries below fail, the report is simply left with notified = 0 and the notification is // delivered later via sendPendingReportNotifications. @@ -1740,7 +1763,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdGetUserSessions(const C } Response_UserSessions *re = new Response_UserSessions; - const QList sessions = sqlInterface->getUserSessions(userName, cmd.limit()); + const int limit = qMin(static_cast(cmd.limit()), 500); + const QList sessions = sqlInterface->getUserSessions(userName, limit); for (const ServerInfo_UserSession &session : sessions) { re->add_sessions()->CopyFrom(session); } @@ -1797,13 +1821,47 @@ Response::ResponseCode AbstractServerSocketInterface::cmdResetUserPassword(const return Response::RespContextError; } + // 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()) { + return Response::RespNameNotFound; + } + const int targetAdmin = privQuery->value(0).toInt(); + const bool targetIsAdmin = targetAdmin & 1; + const bool targetIsMod = targetAdmin & 2; + + const bool callerIsAdmin = userInfo->user_level() & ServerInfo_User::IsAdmin; + + // A moderator must not reset the password of an admin or another moderator. + if (!callerIsAdmin && (targetIsAdmin || targetIsMod)) { + return Response::RespAccessDenied; + } + const QString tempPassword = PasswordHasher::generateRandomSalt(); if (!sqlInterface->changeUserPassword(userName, tempPassword, true)) { return Response::RespInternalError; } + sqlInterface->setForcePasswordChange(userName, true); sqlInterface->addAuditRecord(userName, this->getAddress(), QString::fromStdString(userInfo->clientid()), - "PASSWORD_RESET", "Moderator password reset", true); + "PASSWORD_RESET", "Admin password reset", true); + + // Notify the affected user if they are currently online. + QReadLocker clientsLocker(&servatrice->clientsLock); + AbstractServerSocketInterface *targetSession = + static_cast(server->getUsers().value(userName)); + if (targetSession) { + Event_NotifyUser event; + event.set_type(Event_NotifyUser::CUSTOM); + event.set_custom_title(tr("Password Reset").toStdString()); + event.set_custom_content(tr("An administrator has reset your password. Please log in with the new " + "password provided to you and change it immediately.") + .toStdString()); + SessionEvent *se = targetSession->prepareSessionEvent(event); + targetSession->sendProtocolItem(*se); + delete se; + } Response_ResetUserPassword *re = new Response_ResetUserPassword; re->set_user_name(userName.toStdString()); @@ -2026,8 +2084,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportDetails(const Com const bool isMod = userInfo->user_level() & ServerInfo_User::IsModerator; const int reporterId = query->value(1).toInt(); - const QString reporterName = query->value(2).toString(); - if (reporterId != userInfo->id() && reporterName != QString::fromStdString(userInfo->name()) && !isMod) { + if (reporterId != userInfo->id() && !isMod) { return Response::RespAccessDenied; } @@ -2139,7 +2196,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportAddComment(const bool isMod = userInfo->user_level() & ServerInfo_User::IsModerator; - if (reporterId != userInfo->id() && reporterName != QString::fromStdString(userInfo->name()) && !isMod) { + // Only the reporter (by id) or a moderator may comment on a report. + if (reporterId != userInfo->id() && !isMod) { return Response::RespAccessDenied; } @@ -2184,7 +2242,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReportAddComment(const } const QString ownName = QString::fromStdString(userInfo->name()); - bool allNotified = true; + bool allNotified = !recipients.isEmpty(); QReadLocker clientsLocker(&servatrice->clientsLock); for (const QString ¬ifyName : recipients) { if (notifyName == ownName) { @@ -2788,6 +2846,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C return Response::RespWrongPassword; } + databaseInterface->setForcePasswordChange(userName, false); + return Response::RespOk; } @@ -2924,6 +2984,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con "PASSWORD_RESET", "", true); } + sqlInterface->setForcePasswordChange(nameFromStdString(cmd.user_name()), false); sqlInterface->removeForgotPassword(nameFromStdString(cmd.user_name())); return Response::RespOk; } @@ -3016,6 +3077,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdReport(const Command_Re return Response::RespInvalidData; } + static const QStringList validCategories = {"cheating", "bug_abuse", "verbal_abuse", "other"}; + if (!validCategories.contains(category.toLower())) { + return Response::RespInvalidData; + } + const int maxReportsPerDay = settingsCache->value("reporting/max_reports_per_day", 10).toInt(); if (maxReportsPerDay > 0) { QSqlQuery *countQuery =