From c57f1748c90ab695a4a902248ac2ad00f348282e Mon Sep 17 00:00:00 2001 From: Iris Morelle Date: Tue, 29 Jan 2019 23:39:21 -0300 Subject: [PATCH] wesnothd: Retrieve forum ban durations and send the remaining time to clients This should make it less confusing for players who might assume that their ban is permanent otherwise. If the ban is permanent the duration is not sent to clients, however. Since permanent bans can be revoked on appeal, it seems like a good idea to me to avoid using any wording that would allude to their otherwise infinite duration. Closes #3766. --- src/game_initialization/multiplayer.cpp | 23 +++++++-- src/server/forum_user_handler.cpp | 63 ++++++++++++++++++++++--- src/server/forum_user_handler.hpp | 8 +++- src/server/sample_user_handler.cpp | 4 +- src/server/sample_user_handler.hpp | 4 +- src/server/server.cpp | 29 ++++++++---- src/server/user_handler.hpp | 31 ++++++++++-- 7 files changed, 132 insertions(+), 30 deletions(-) diff --git a/src/game_initialization/multiplayer.cpp b/src/game_initialization/multiplayer.cpp index f3bbfd9e024a..0a823bdd6805 100644 --- a/src/game_initialization/multiplayer.cpp +++ b/src/game_initialization/multiplayer.cpp @@ -306,6 +306,11 @@ std::pair open_connection(std::string host) utils::string_map i18n_symbols; i18n_symbols["nick"] = login; + const bool has_extra_data = error->has_child("data"); + if(has_extra_data) { + i18n_symbols["duration"] = utils::format_timespan((*error).child("data")["duration"]); + } + if((*error)["error_code"] == MP_MUST_LOGIN) { error_message = _("You must login first."); } else if((*error)["error_code"] == MP_NAME_TAKEN_ERROR) { @@ -323,11 +328,23 @@ std::pair open_connection(std::string host) error_message = VGETTEXT("The nickname ‘$nick’ is not registered on this server.", i18n_symbols) + _(" This server disallows unregistered nicknames."); } else if((*error)["error_code"] == MP_NAME_AUTH_BAN_USER_ERROR) { - error_message = VGETTEXT("The nickname ‘$nick’ is banned on this server’s forums.", i18n_symbols); + if(has_extra_data) { + error_message = VGETTEXT("The nickname ‘$nick’ is banned on this server’s forums for $duration|.", i18n_symbols); + } else { + error_message = VGETTEXT("The nickname ‘$nick’ is banned on this server’s forums.", i18n_symbols); + } } else if((*error)["error_code"] == MP_NAME_AUTH_BAN_IP_ERROR) { - error_message = _("Your IP address is banned on this server’s forums."); + if(has_extra_data) { + error_message = VGETTEXT("Your IP address is banned on this server’s forums for $duration|.", i18n_symbols); + } else { + error_message = _("Your IP address is banned on this server’s forums."); + } } else if((*error)["error_code"] == MP_NAME_AUTH_BAN_EMAIL_ERROR) { - error_message = VGETTEXT("The email address for the nickname ‘$nick’ is banned on this server’s forums.", i18n_symbols); + if(has_extra_data) { + error_message = VGETTEXT("The email address for the nickname ‘$nick’ is banned on this server’s forums for $duration|.", i18n_symbols); + } else { + error_message = VGETTEXT("The email address for the nickname ‘$nick’ is banned on this server’s forums.", i18n_symbols); + } } else if((*error)["error_code"] == MP_PASSWORD_REQUEST) { error_message = VGETTEXT("The nickname ‘$nick’ is registered on this server.", i18n_symbols); } else if((*error)["error_code"] == MP_PASSWORD_REQUEST_FOR_LOGGED_IN_NAME) { diff --git a/src/server/forum_user_handler.cpp b/src/server/forum_user_handler.cpp index 8b3a7705535e..43f39354b8eb 100644 --- a/src/server/forum_user_handler.cpp +++ b/src/server/forum_user_handler.cpp @@ -172,7 +172,7 @@ void fuh::set_is_moderator(const std::string& name, const bool& is_moderator) { } } -fuh::BAN_TYPE fuh::user_is_banned(const std::string& name, const std::string& addr) +fuh::ban_info fuh::user_is_banned(const std::string& name, const std::string& addr) { // // NOTE: glob IP and email address bans are NOT supported yet since they @@ -186,17 +186,20 @@ fuh::BAN_TYPE fuh::user_is_banned(const std::string& name, const std::string& ad const std::string& is_extant_ban_sql = "ban_exclude = 0 AND (ban_end = 0 OR ban_end >=" + std::to_string(std::time(nullptr)) + ")"; + // TODO: retrieve full ban info in a single statement instead of issuing + // separate queries to check for a ban's existence and its duration. + try { if(!addr.empty() && prepared_statement("SELECT 1 FROM `" + db_banlist_table_ + "` WHERE UPPER(ban_ip) = UPPER(?) AND " + is_extant_ban_sql, addr)) { LOG_UH << "User '" << name << "' ip " << addr << " banned by IP address\n"; - return BAN_IP; + return retrieve_ban_info(BAN_IP, addr); } } catch(const sql_error& e) { ERR_UH << "Could not check forum bans on address '" << addr << "' :" << e.message << '\n'; - return BAN_NONE; + return {}; } - if(!user_exists(name)) return BAN_NONE; + if(!user_exists(name)) return {}; try { auto uid = get_detail_for_user(name, "user_id"); @@ -205,21 +208,67 @@ fuh::BAN_TYPE fuh::user_is_banned(const std::string& name, const std::string& ad ERR_UH << "Invalid user id for user '" << name << "'\n"; } else if(prepared_statement("SELECT 1 FROM `" + db_banlist_table_ + "` WHERE ban_userid = ? AND " + is_extant_ban_sql, uid)) { LOG_UH << "User '" << name << "' uid " << uid << " banned by uid\n"; - return BAN_USER; + return retrieve_ban_info(BAN_USER, uid); } auto email = get_detail_for_user(name, "user_email"); if(!email.empty() && prepared_statement("SELECT 1 FROM `" + db_banlist_table_ + "` WHERE UPPER(ban_email) = UPPER(?) AND " + is_extant_ban_sql, email)) { LOG_UH << "User '" << name << "' email " << email << " banned by email address\n"; - return BAN_EMAIL; + return retrieve_ban_info(BAN_EMAIL, email); } } catch(const sql_error& e) { ERR_UH << "Could not check forum bans on user '" << name << "' :" << e.message << '\n'; } - return BAN_NONE; + return {}; +} + +template +fuh::ban_info fuh::retrieve_ban_info(fuh::BAN_TYPE type, T detail) +{ + std::string col; + + switch(type) { + case BAN_USER: + col = "ban_userid"; + break; + case BAN_EMAIL: + col = "ban_email"; + break; + case BAN_IP: + col = "ban_ip"; + break; + default: + return {}; + } + + try { + return { type, retrieve_ban_duration_internal(col, detail) }; + } catch(const sql_error& e) { + // + // NOTE: + // If retrieve_ban_internal() fails to fetch the ban row, odds are the ban was + // lifted in the meantime (it's meant to be called by user_is_banned(), so we + // assume the ban expires in one second instead of returning 0 (permanent ban) + // just to err on the safe side (returning BAN_NONE would be a terrible idea, + // for that matter). + // + return { type, 1 }; + } +} + +std::time_t fuh::retrieve_ban_duration_internal(const std::string& col, const std::string& detail) +{ + const std::time_t end_time = prepared_statement("SELECT `ban_end` FROM `" + db_banlist_table_ + "` WHERE UPPER(" + col + ") = UPPER(?)", detail); + return end_time ? end_time - std::time(nullptr) : 0; +} + +std::time_t fuh::retrieve_ban_duration_internal(const std::string& col, unsigned int detail) +{ + const std::time_t end_time = prepared_statement("SELECT `ban_end` FROM `" + db_banlist_table_ + "` WHERE " + col + " = ?", detail); + return end_time ? end_time - std::time(nullptr) : 0; } std::string fuh::user_info(const std::string& name) { diff --git a/src/server/forum_user_handler.hpp b/src/server/forum_user_handler.hpp index 6faa8df00fb9..736c88c53d7c 100644 --- a/src/server/forum_user_handler.hpp +++ b/src/server/forum_user_handler.hpp @@ -69,7 +69,7 @@ class fuh : public user_handler { bool user_is_moderator(const std::string& name); void set_is_moderator(const std::string& name, const bool& is_moderator); - BAN_TYPE user_is_banned(const std::string& name, const std::string& addr); + ban_info user_is_banned(const std::string& name, const std::string& addr); // Throws user_handler::error std::string user_info(const std::string& name); @@ -91,6 +91,12 @@ class fuh : public user_handler { void set_lastlogin(const std::string& user, const time_t& lastlogin); + template + ban_info retrieve_ban_info(BAN_TYPE, T detail); + + std::time_t retrieve_ban_duration_internal(const std::string& col, const std::string& detail); + std::time_t retrieve_ban_duration_internal(const std::string& col, unsigned int detail); + std::string db_name_, db_host_, db_user_, db_password_, db_users_table_, db_banlist_table_, db_extra_table_; typedef std::unique_ptr mysql_result; diff --git a/src/server/sample_user_handler.cpp b/src/server/sample_user_handler.cpp index a02bd7d830cf..a53042e8cf46 100644 --- a/src/server/sample_user_handler.cpp +++ b/src/server/sample_user_handler.cpp @@ -95,9 +95,9 @@ void suh::set_is_moderator(const std::string& name, const bool& is_moderator) { users_[name].is_moderator = is_moderator; } -suh::BAN_TYPE suh::user_is_banned(const std::string&, const std::string&) { +suh::ban_info suh::user_is_banned(const std::string&, const std::string&) { // FIXME: stub - return BAN_NONE; + return {}; } void suh::set_mail(const std::string& user, const std::string& mail) { diff --git a/src/server/sample_user_handler.hpp b/src/server/sample_user_handler.hpp index 51e2c101988b..88b7b5c36a42 100644 --- a/src/server/sample_user_handler.hpp +++ b/src/server/sample_user_handler.hpp @@ -19,8 +19,6 @@ #include #include -#include - /** * An example of how to implement user_handler. * If you use this on anything real, you are insane. @@ -43,7 +41,7 @@ class suh : public user_handler { bool user_is_moderator(const std::string& name); void set_is_moderator(const std::string& name, const bool& is_moderator); - BAN_TYPE user_is_banned(const std::string& name, const std::string&); + ban_info user_is_banned(const std::string& name, const std::string&); std::string user_info(const std::string& name); diff --git a/src/server/server.cpp b/src/server/server.cpp index 0515f90e6dd1..4d74df0b4632 100644 --- a/src/server/server.cpp +++ b/src/server/server.cpp @@ -723,16 +723,19 @@ bool server::is_login_allowed(socket_ptr socket, const simple_wml::node* const l } const bool is_moderator = user_handler_ && user_handler_->user_is_moderator(username); - const user_handler::BAN_TYPE auth_ban = user_handler_ - ? user_handler_->user_is_banned(username, client_address(socket)) - : user_handler::BAN_NONE; + user_handler::ban_info auth_ban; - if(auth_ban) { + if(user_handler_) { + auth_ban = user_handler_->user_is_banned(username, client_address(socket)); + } + + if(auth_ban.type) { std::string ban_type_desc; std::string ban_reason; const char* msg_numeric; + std::string ban_duration = std::to_string(auth_ban.duration); - switch(auth_ban) { + switch(auth_ban.type) { case user_handler::BAN_USER: ban_type_desc = "account"; msg_numeric = MP_NAME_AUTH_BAN_USER_ERROR; @@ -754,10 +757,18 @@ bool server::is_login_allowed(socket_ptr socket, const simple_wml::node* const l ban_reason = ban_type_desc; } + ban_reason += " (" + ban_duration + ")"; + if(!is_moderator) { LOG_SERVER << client_address(socket) << "\t" << username << "\tis banned by user_handler (" << ban_type_desc << ")\n"; - async_send_error(socket, "You are banned from this server: " + ban_reason, msg_numeric); + if(auth_ban.duration) { + // Temporary ban + async_send_error(socket, "You are banned from this server: " + ban_reason, msg_numeric, {{"duration", ban_duration}}); + } else { + // Permanent ban + async_send_error(socket, "You are banned from this server: " + ban_reason, msg_numeric); + } return false; } else { LOG_SERVER << client_address(socket) << "\t" << username << "\tis banned by user_handler (" << ban_type_desc @@ -802,7 +813,7 @@ bool server::is_login_allowed(socket_ptr socket, const simple_wml::node* const l "If you no longer want to be automatically authenticated use '/query signout'."); } - if(auth_ban) { + if(auth_ban.type) { send_server_message(socket, "You are currently banned by the forum administration."); } @@ -851,7 +862,7 @@ bool server::authenticate( // This name is registered and no password provided if(password.empty()) { if(!name_taken) { - send_password_request(socket, "The nickname '" + username + "' is registered on this server.", + send_password_request(socket, "The nickname '" + username + "' is registered on this server.", username, version, MP_PASSWORD_REQUEST); } else { send_password_request(socket, @@ -909,7 +920,7 @@ bool server::authenticate( "You have made too many failed login attempts.", MP_TOO_MANY_ATTEMPTS_ERROR); } else { send_password_request(socket, - "The password you provided for the nickname '" + username + "' was incorrect.", username,version, + "The password you provided for the nickname '" + username + "' was incorrect.", username,version, MP_INCORRECT_PASSWORD_ERROR); } diff --git a/src/server/user_handler.hpp b/src/server/user_handler.hpp index d3b78c340953..34e67f32a4ea 100644 --- a/src/server/user_handler.hpp +++ b/src/server/user_handler.hpp @@ -18,6 +18,7 @@ class config; #include "exceptions.hpp" +#include #include /** @@ -108,12 +109,32 @@ class user_handler { /** Mark this user as a moderator */ virtual void set_is_moderator(const std::string& name, const bool& is_moderator) =0; + /** Ban type values */ enum BAN_TYPE { - BAN_NONE, - BAN_USER, - BAN_IP, - BAN_EMAIL, + BAN_NONE, /**< Not a ban */ + BAN_USER, /**< User account/name ban */ + BAN_IP, /**< IP address ban */ + BAN_EMAIL, /**< Account email address ban */ + }; + + /** Ban status description */ + struct ban_info + { + BAN_TYPE type; /**< Ban type */ + std::time_t duration; /**< Ban duration (0 if permanent) */ + + ban_info() + : type(BAN_NONE) + , duration(0) + { + } + + ban_info(BAN_TYPE ptype, std::time_t pduration) + : type(ptype) + , duration(pduration) + { + } }; /** @@ -123,7 +144,7 @@ class user_handler { * subclass. Regular IP ban checks are done by @a server_base * instead. */ - virtual BAN_TYPE user_is_banned(const std::string& name, const std::string& addr="") = 0; + virtual ban_info user_is_banned(const std::string& name, const std::string& addr="") = 0; struct error : public game::error { error(const std::string& message) : game::error(message) {}