Merge lp:~widelands-dev/widelands/net-checkpwd-command into lp:widelands

[Notabilis]

Checking metaserver password without doing a full login.

Currently, the online settings dialog does a full login to the metaserver to check the entered password. This branch avoid this by using a new CHECK_PWD command to verify whether the password is correct.

Should be merged after (and can't really be tested before) https://github.com/widelands/widelands_metaserver/pull/55 is deployed.

[GunChleoc (gunchleoc)]

Code LGTM, just some small nits for the comments

review: Approve

[Notabilis (notabilis27)]

Is fixed, thanks!

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5120. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/540143585.
Appveyor build 4902. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_net_checkpwd_command-4902.

[Notabilis (notabilis27)]

The related change to the metaserver has been deployed, so this branch can be tested and merged.

Bunnybot fails due to transient errors.

[GunChleoc (gunchleoc)]

Tested and working :)

@bunnybot merge force

review: Approve

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2019-05-25 08:25:55 +0000
+++ src/network/internet_gaming.cc	2019-06-01 16:31:43 +0000
@@ -235,6 +235,53 @@
 	reset();
 }
 
+bool InternetGaming::check_password(const std::string& nick,
+		   const std::string& pwd,
+		   const std::string& metaserver,
+		   uint32_t port) {
+	reset();
+
+	meta_ = metaserver;
+	port_ = port;
+	initialize_connection();
+
+	// Has to be set for the password challenge later on
+	authenticator_ = pwd;
+
+	log("InternetGaming: Verifying password.\n");
+	{
+		SendPacket s;
+		s.string(IGPCMD_CHECK_PWD);
+		s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
+		s.string(nick);
+		s.string(build_id());
+		net->send(s);
+	}
+
+	// Now let's see, whether the metaserver is answering
+	uint32_t const secs = time(nullptr);
+	state_ = CONNECTING;
+	while (kInternetGamingTimeout > time(nullptr) - secs) {
+		handle_metaserver_communication(false);
+		if (state_ != CONNECTING) {
+			if (state_ == LOBBY) {
+				SendPacket s;
+				s.string(IGPCMD_DISCONNECT);
+				s.string("CONNECTION_CLOSED");
+				net->send(s);
+				reset();
+				return true;
+			} else if (error()) {
+				reset();
+				return false;
+			}
+		}
+	}
+	log("InternetGaming: No answer from metaserver!\n");
+	reset();
+	return false;
+}
+
 /**
  * Handle situation when reading from socket failed.
  */
@@ -265,7 +312,7 @@
 }
 
 /// handles all communication between the metaserver and the client
-void InternetGaming::handle_metaserver_communication() {
+void InternetGaming::handle_metaserver_communication(bool relogin_on_error) {
 	if (error())
 		return;
 	try {
@@ -278,7 +325,7 @@
 			// Process all available packets
 			std::unique_ptr<RecvPacket> packet = net->try_receive();
 			if (packet) {
-				handle_packet(*packet);
+				handle_packet(*packet, relogin_on_error);
 			} else {
 				// Nothing more to receive
 				break;
@@ -315,7 +362,7 @@
 			set_error();
 			waittimeout_ = std::numeric_limits<int32_t>::max();
 			log("InternetGaming: reached a timeout for an awaited answer of the metaserver!\n");
-			if (!relogin()) {
+			if (relogin_on_error && !relogin()) {
 				// Do not try to relogin again automatically.
 				reset();
 				set_error();
@@ -328,7 +375,7 @@
 	if (time(nullptr) - lastping_ > 240) {
 		// Try to relogin
 		set_error();
-		if (!relogin()) {
+		if (relogin_on_error && !relogin()) {
 			// Do not try to relogin again automatically.
 			reset();
 			set_error();
@@ -337,7 +384,7 @@
 }
 
 /// Handle one packet received from the metaserver.
-void InternetGaming::handle_packet(RecvPacket& packet) {
+void InternetGaming::handle_packet(RecvPacket& packet, bool relogin_on_error) {
 	std::string cmd = packet.string();
 
 	// First check if everything is fine or whether the metaserver broke up with the client.
@@ -347,7 +394,7 @@
 		if (reason == "CLIENT_TIMEOUT") {
 			// Try to relogin
 			set_error();
-			if (!relogin()) {
+			if (relogin_on_error && !relogin()) {
 				// Do not try to relogin again automatically.
 				reset();
 				set_error();
@@ -405,6 +452,12 @@
 			    asctime(gmtime(&now)));
 			return;
 
+		} else if (cmd == IGPCMD_PWD_OK) {
+			const time_t now = time(nullptr);
+			log("InternetGaming: Password check successful at UTC %s", asctime(gmtime(&now)));
+			state_ = LOBBY;
+			return;
+
 		} else if (cmd == IGPCMD_ERROR) {
 			std::string errortype = packet.string();
 			if (errortype != IGPCMD_LOGIN && errortype != IGPCMD_PWD_CHALLENGE) {

=== modified file 'src/network/internet_gaming.h'
--- src/network/internet_gaming.h	2019-05-15 09:56:36 +0000
+++ src/network/internet_gaming.h	2019-06-01 16:31:43 +0000
@@ -79,6 +79,23 @@
 	bool relogin();
 	void logout(const std::string& msgcode = "CONNECTION_CLOSED");
 
+	/**
+	 * Connects to the metaserver and checks the password without logging in.
+	 *
+	 * Note that the user might be logged in with another username and as unregistered
+	 * if the user account is already in use by another client.
+	 * @warning Resets the current connection.
+	 * @param nick The username.
+	 * @param pwd The password.
+	 * @param metaserver The hostname of the metaserver.
+	 * @param port The port number of the metaserver.
+	 * @return Whether the password was valid.
+	 */
+	bool check_password(const std::string& nick,
+	           const std::string& pwd,
+	           const std::string& metaserver,
+	           uint32_t port);
+
 	/// \returns whether the client is logged in
 	bool logged_in() {
 		return (state_ == LOBBY) || (state_ == CONNECTING) || (state_ == IN_GAME);
@@ -92,7 +109,7 @@
 		clientupdate_ = true;
 	}
 
-	void handle_metaserver_communication();
+	void handle_metaserver_communication(bool relogin_on_error = true);
 
 	// Game specific functions
 	/**
@@ -202,7 +219,7 @@
 	 */
 	void create_second_connection();
 
-	void handle_packet(RecvPacket& packet);
+	void handle_packet(RecvPacket& packet, bool relogin_on_error = true);
 	void handle_failed_read();
 
 	// conversion functions

=== modified file 'src/network/internet_gaming_protocol.h'
--- src/network/internet_gaming_protocol.h	2019-02-23 11:00:49 +0000
+++ src/network/internet_gaming_protocol.h	2019-06-01 16:31:43 +0000
@@ -39,8 +39,9 @@
  * 3: Between build 19 and build 20 - Added network relay for internet games
  * 4: Between build 19 and build 20 - Using CHAP for password authentication
  * 5: Build 20 - Removed obsolete TELL_IP, modifications on user and game listing [supported]
+ * 6: Between build 20 and build 21 - Added CHECK_PWD and PWD_OK commands
  */
-constexpr unsigned int kInternetGamingProtocolVersion = 5;
+constexpr unsigned int kInternetGamingProtocolVersion = 6;
 
 /**
  * The default timeout time after which the client tries to resend a package or even finally closes
@@ -182,7 +183,29 @@
 static const std::string IGPCMD_LOGIN = "LOGIN";
 
 /**
- * This is sent by the metaserver after a IGPCMD_LOGIN by a registered client.
+ * The client tries to check the password of the user without doing a full login.
+ * Should be sent without logging in before.
+ *
+ * Payload:
+ * \li string:    protocol version (see kInternetGamingProtocolVersion)
+ * \li string:    client name
+ * \li string:    build_id of the client
+ *
+ * A IGPCMD_PWD_CHALLENGE exchange follows before the server replies with a IGPCMD_PWD_OK
+ * or IGPCMD_ERROR message.
+ *
+ * If the password is correct, the metaserver replies with a IGPCMD_PWD_OK command
+ * with the following payload:
+ * \li string:    client name. Will be the same as sent before.
+ * \li string:    clients rights (see client rights section above)
+ *
+ * If the password is wrong or some other error occurred, \ref IGPCMD_ERROR is returned.
+ */
+static const std::string IGPCMD_CHECK_PWD = "CHECK_PWD";
+static const std::string IGPCMD_PWD_OK = "PWD_OK";
+
+/**
+ * This is sent by the metaserver after a IGPCMD_LOGIN or IGPCMD_CHECK_PWD by a registered client.
  * This is the first message of the a protocol similar to the challenge handshake authentication
  * protocol (CHAP) for secure transmission of the users password.
  * The server sends the nonce for hashing:

=== modified file 'src/wui/login_box.cc'
--- src/wui/login_box.cc	2019-05-29 12:49:27 +0000
+++ src/wui/login_box.cc	2019-06-01 16:31:43 +0000
@@ -188,15 +188,13 @@
 	const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());
 	uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);
 	std::string password = crypto::sha1(eb_password->text());
-	InternetGaming::ref().login(get_nickname(), password, true, meta, port);
 
-	if (!InternetGaming::ref().logged_in()) {
+	if (!InternetGaming::ref().check_password(get_nickname(), password, meta, port)) {
 		// something went wrong -> show the error message
 		// idealy it is about the wrong password
 		ChatMessage msg = InternetGaming::ref().get_messages().back();
 		UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);
 		wmb.run<UI::Panel::Returncodes>();
-		InternetGaming::ref().reset();
 		eb_password->set_text("");
 		eb_password->focus();
 		return false;