Improve IO ban information (#4544)

otland · Dec 13, 2023 · ffba1d8 · xmish · Jan 2, 2024 · ffba1d8
1 parent 0f606ee
commit ffba1d8
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 40 deletions.
diff --git a/src/ban.cpp b/src/ban.cpp
@@ -44,15 +44,15 @@ bool Ban::acceptConnection(const Connection::Address& clientIP)
  return true;
 }
 
-bool IOBan::isAccountBanned(uint32_t accountId, BanInfo& banInfo)
+const std::optional<BanInfo> IOBan::getAccountBanInfo(uint32_t accountId)
 {
  Database& db = Database::getInstance();
 
  DBResult_ptr result = db.storeQuery(fmt::format(
  "SELECT `reason`, `expires_at`, `banned_at`, `banned_by`, (SELECT `name` FROM `players` WHERE `id` = `banned_by`) AS `name` FROM `account_bans` WHERE `account_id` = {:d}",
  accountId));
  if (!result) {
- return false;
+ return std::nullopt;
  }
 
  int64_t expiresAt = result->getNumber<int64_t>("expires_at");
@@ -63,19 +63,25 @@ bool IOBan::isAccountBanned(uint32_t accountId, BanInfo& banInfo)
  accountId, db.escapeString(result->getString("reason")), result->getNumber<time_t>("banned_at"), expiresAt,
  result->getNumber<uint32_t>("banned_by")));
  g_databaseTasks.addTask(fmt::format("DELETE FROM `account_bans` WHERE `account_id` = {:d}", accountId));
- return false;
+ return std::nullopt;
  }
 
- banInfo.expiresAt = expiresAt;
- banInfo.reason = result->getString("reason");
- banInfo.bannedBy = result->getString("name");
- return true;
+ auto banInfo = std::make_optional<BanInfo>();
+ banInfo->expiresAt = expiresAt;
+
+ banInfo->reason = result->getString("reason");
+ if (banInfo->reason.empty()) {
+ banInfo->reason = "(none)";
+ }
+
+ banInfo->bannedBy = result->getString("name");
+ return banInfo;
 }
 
-bool IOBan::isIpBanned(const Connection::Address& clientIP, BanInfo& banInfo)
+const std::optional<BanInfo> IOBan::getIpBanInfo(const Connection::Address& clientIP)
 {
  if (clientIP.is_unspecified()) {
- return false;
+ return std::nullopt;
  }
 
  Database& db = Database::getInstance();
@@ -84,20 +90,26 @@ bool IOBan::isIpBanned(const Connection::Address& clientIP, BanInfo& banInfo)
  "SELECT `reason`, `expires_at`, (SELECT `name` FROM `players` WHERE `id` = `banned_by`) AS `name` FROM `ip_bans` WHERE `ip` = INET6_ATON('{:s}')",
  clientIP.to_string()));
  if (!result) {
- return false;
+ return std::nullopt;
  }
 
  int64_t expiresAt = result->getNumber<int64_t>("expires_at");
  if (expiresAt != 0 && time(nullptr) > expiresAt) {
  g_databaseTasks.addTask(
  fmt::format("DELETE FROM `ip_bans` WHERE `ip` = INET6_ATON('{:s}')", clientIP.to_string()));
- return false;
+ return std::nullopt;
  }
 
- banInfo.expiresAt = expiresAt;
- banInfo.reason = result->getString("reason");
- banInfo.bannedBy = result->getString("name");
- return true;
+ auto banInfo = std::make_optional<BanInfo>();
+ banInfo->expiresAt = expiresAt;
+
+ banInfo->reason = result->getString("reason");
+ if (banInfo->reason.empty()) {
+ banInfo->reason = "(none)";
+ }
+
+ banInfo->bannedBy = result->getString("name");
+ return banInfo;
 }
 
 bool IOBan::isPlayerNamelocked(uint32_t playerId)

diff --git a/src/ban.h b/src/ban.h
@@ -39,8 +39,8 @@ class Ban
 class IOBan
 {
 public:
- static bool isAccountBanned(uint32_t accountId, BanInfo& banInfo);
- static bool isIpBanned(const Connection::Address& clientIP, BanInfo& banInfo);
+ static const std::optional<BanInfo> getAccountBanInfo(uint32_t accountId);
+ static const std::optional<BanInfo> getIpBanInfo(const Connection::Address& clientIP);
  static bool isPlayerNamelocked(uint32_t playerId);
 };
 

diff --git a/src/protocolgame.cpp b/src/protocolgame.cpp
@@ -187,20 +187,15 @@ void ProtocolGame::login(uint32_t characterId, uint32_t accountId, OperatingSyst
  }
 
  if (!player->hasFlag(PlayerFlag_CannotBeBanned)) {
- BanInfo banInfo;
- if (IOBan::isAccountBanned(accountId, banInfo)) {
- if (banInfo.reason.empty()) {
- banInfo.reason = "(none)";
- }
-
- if (banInfo.expiresAt > 0) {
+ if (const auto& banInfo = IOBan::getAccountBanInfo(accountId)) {
 player->getAccountType() < ACCOUNT_TYPE_GAMEMASTER && g_game.getPlayerByAccount(player->getAccount())) { 
 player->getAccountType() < ACCOUNT_TYPE_GAMEMASTER && g_game.getPlayerByAccount(player->getAccount())) { 
+ if (banInfo->expiresAt > 0) {
  disconnectClient(
  fmt::format("Your account has been banned until {:s} by {:s}.\n\nReason specified:\n{:s}",
- formatDateShort(banInfo.expiresAt), banInfo.bannedBy, banInfo.reason));
+ formatDateShort(banInfo->expiresAt), banInfo->bannedBy, banInfo->reason));
  } else {
  disconnectClient(
  fmt::format("Your account has been permanently banned by {:s}.\n\nReason specified:\n{:s}",
- banInfo.bannedBy, banInfo.reason));
+ banInfo->bannedBy, banInfo->reason));
  }
  return;
  }
@@ -455,14 +450,9 @@ void ProtocolGame::onRecvFirstMessage(NetworkMessage& msg)
  return;
  }
 
- BanInfo banInfo;
- if (IOBan::isIpBanned(getIP(), banInfo)) {
- if (banInfo.reason.empty()) {
- banInfo.reason = "(none)";
- }
-
+ if (const auto& banInfo = IOBan::getIpBanInfo(getIP())) {
  disconnectClient(fmt::format("Your IP has been banned until {:s} by {:s}.\n\nReason specified:\n{:s}",
- formatDateShort(banInfo.expiresAt), banInfo.bannedBy, banInfo.reason));
+ formatDateShort(banInfo->expiresAt), banInfo->bannedBy, banInfo->reason));
  return;
  }
 

diff --git a/src/protocollogin.cpp b/src/protocollogin.cpp
@@ -166,19 +166,14 @@ void ProtocolLogin::onRecvFirstMessage(NetworkMessage& msg)
  return;
  }
 
- BanInfo banInfo;
  auto connection = getConnection();
  if (!connection) {
  return;
  }
 
- if (IOBan::isIpBanned(connection->getIP(), banInfo)) {
- if (banInfo.reason.empty()) {
- banInfo.reason = "(none)";
- }
-
+ if (const auto& banInfo = IOBan::getIpBanInfo(connection->getIP())) {
  disconnectClient(fmt::format("Your IP has been banned until {:s} by {:s}.\n\nReason specified:\n{:s}",
- formatDateShort(banInfo.expiresAt), banInfo.bannedBy, banInfo.reason),
+ formatDateShort(banInfo->expiresAt), banInfo->bannedBy, banInfo->reason),
  version);
  return;
  }