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

Proposed by Notabilis
Status: Merged
Merged at revision: 9138
Proposed branch: lp:~widelands-dev/widelands/net-checkpwd-command
Merge into: lp:widelands
Diff against target: 237 lines (+104/-13)
4 files modified
src/network/internet_gaming.cc (+59/-6)
src/network/internet_gaming.h (+19/-2)
src/network/internet_gaming_protocol.h (+25/-2)
src/wui/login_box.cc (+1/-3)
To merge this branch: bzr merge lp:~widelands-dev/widelands/net-checkpwd-command
Reviewer Review Type Date Requested Status
GunChleoc Approve
Review via email: mp+368225@code.launchpad.net

Commit message

Checking metaserver password without doing a full login.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

Code LGTM, just some small nits for the comments

review: Approve
9137. By Notabilis

Fixing language.

Revision history for this message
Notabilis (notabilis27) wrote :

Is fixed, thanks!

Revision history for this message
bunnybot (widelandsofficial) wrote :

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.

Revision history for this message
Notabilis (notabilis27) wrote :

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

Bunnybot fails due to transient errors.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Tested and working :)

@bunnybot merge force

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
235 reset();235 reset();
236}236}
237237
238bool InternetGaming::check_password(const std::string& nick,
239 const std::string& pwd,
240 const std::string& metaserver,
241 uint32_t port) {
242 reset();
243
244 meta_ = metaserver;
245 port_ = port;
246 initialize_connection();
247
248 // Has to be set for the password challenge later on
249 authenticator_ = pwd;
250
251 log("InternetGaming: Verifying password.\n");
252 {
253 SendPacket s;
254 s.string(IGPCMD_CHECK_PWD);
255 s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
256 s.string(nick);
257 s.string(build_id());
258 net->send(s);
259 }
260
261 // Now let's see, whether the metaserver is answering
262 uint32_t const secs = time(nullptr);
263 state_ = CONNECTING;
264 while (kInternetGamingTimeout > time(nullptr) - secs) {
265 handle_metaserver_communication(false);
266 if (state_ != CONNECTING) {
267 if (state_ == LOBBY) {
268 SendPacket s;
269 s.string(IGPCMD_DISCONNECT);
270 s.string("CONNECTION_CLOSED");
271 net->send(s);
272 reset();
273 return true;
274 } else if (error()) {
275 reset();
276 return false;
277 }
278 }
279 }
280 log("InternetGaming: No answer from metaserver!\n");
281 reset();
282 return false;
283}
284
238/**285/**
239 * Handle situation when reading from socket failed.286 * Handle situation when reading from socket failed.
240 */287 */
@@ -265,7 +312,7 @@
265}312}
266313
267/// handles all communication between the metaserver and the client314/// handles all communication between the metaserver and the client
268void InternetGaming::handle_metaserver_communication() {315void InternetGaming::handle_metaserver_communication(bool relogin_on_error) {
269 if (error())316 if (error())
270 return;317 return;
271 try {318 try {
@@ -278,7 +325,7 @@
278 // Process all available packets325 // Process all available packets
279 std::unique_ptr<RecvPacket> packet = net->try_receive();326 std::unique_ptr<RecvPacket> packet = net->try_receive();
280 if (packet) {327 if (packet) {
281 handle_packet(*packet);328 handle_packet(*packet, relogin_on_error);
282 } else {329 } else {
283 // Nothing more to receive330 // Nothing more to receive
284 break;331 break;
@@ -315,7 +362,7 @@
315 set_error();362 set_error();
316 waittimeout_ = std::numeric_limits<int32_t>::max();363 waittimeout_ = std::numeric_limits<int32_t>::max();
317 log("InternetGaming: reached a timeout for an awaited answer of the metaserver!\n");364 log("InternetGaming: reached a timeout for an awaited answer of the metaserver!\n");
318 if (!relogin()) {365 if (relogin_on_error && !relogin()) {
319 // Do not try to relogin again automatically.366 // Do not try to relogin again automatically.
320 reset();367 reset();
321 set_error();368 set_error();
@@ -328,7 +375,7 @@
328 if (time(nullptr) - lastping_ > 240) {375 if (time(nullptr) - lastping_ > 240) {
329 // Try to relogin376 // Try to relogin
330 set_error();377 set_error();
331 if (!relogin()) {378 if (relogin_on_error && !relogin()) {
332 // Do not try to relogin again automatically.379 // Do not try to relogin again automatically.
333 reset();380 reset();
334 set_error();381 set_error();
@@ -337,7 +384,7 @@
337}384}
338385
339/// Handle one packet received from the metaserver.386/// Handle one packet received from the metaserver.
340void InternetGaming::handle_packet(RecvPacket& packet) {387void InternetGaming::handle_packet(RecvPacket& packet, bool relogin_on_error) {
341 std::string cmd = packet.string();388 std::string cmd = packet.string();
342389
343 // First check if everything is fine or whether the metaserver broke up with the client.390 // First check if everything is fine or whether the metaserver broke up with the client.
@@ -347,7 +394,7 @@
347 if (reason == "CLIENT_TIMEOUT") {394 if (reason == "CLIENT_TIMEOUT") {
348 // Try to relogin395 // Try to relogin
349 set_error();396 set_error();
350 if (!relogin()) {397 if (relogin_on_error && !relogin()) {
351 // Do not try to relogin again automatically.398 // Do not try to relogin again automatically.
352 reset();399 reset();
353 set_error();400 set_error();
@@ -405,6 +452,12 @@
405 asctime(gmtime(&now)));452 asctime(gmtime(&now)));
406 return;453 return;
407454
455 } else if (cmd == IGPCMD_PWD_OK) {
456 const time_t now = time(nullptr);
457 log("InternetGaming: Password check successful at UTC %s", asctime(gmtime(&now)));
458 state_ = LOBBY;
459 return;
460
408 } else if (cmd == IGPCMD_ERROR) {461 } else if (cmd == IGPCMD_ERROR) {
409 std::string errortype = packet.string();462 std::string errortype = packet.string();
410 if (errortype != IGPCMD_LOGIN && errortype != IGPCMD_PWD_CHALLENGE) {463 if (errortype != IGPCMD_LOGIN && errortype != IGPCMD_PWD_CHALLENGE) {
411464
=== 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 @@
79 bool relogin();79 bool relogin();
80 void logout(const std::string& msgcode = "CONNECTION_CLOSED");80 void logout(const std::string& msgcode = "CONNECTION_CLOSED");
8181
82 /**
83 * Connects to the metaserver and checks the password without logging in.
84 *
85 * Note that the user might be logged in with another username and as unregistered
86 * if the user account is already in use by another client.
87 * @warning Resets the current connection.
88 * @param nick The username.
89 * @param pwd The password.
90 * @param metaserver The hostname of the metaserver.
91 * @param port The port number of the metaserver.
92 * @return Whether the password was valid.
93 */
94 bool check_password(const std::string& nick,
95 const std::string& pwd,
96 const std::string& metaserver,
97 uint32_t port);
98
82 /// \returns whether the client is logged in99 /// \returns whether the client is logged in
83 bool logged_in() {100 bool logged_in() {
84 return (state_ == LOBBY) || (state_ == CONNECTING) || (state_ == IN_GAME);101 return (state_ == LOBBY) || (state_ == CONNECTING) || (state_ == IN_GAME);
@@ -92,7 +109,7 @@
92 clientupdate_ = true;109 clientupdate_ = true;
93 }110 }
94111
95 void handle_metaserver_communication();112 void handle_metaserver_communication(bool relogin_on_error = true);
96113
97 // Game specific functions114 // Game specific functions
98 /**115 /**
@@ -202,7 +219,7 @@
202 */219 */
203 void create_second_connection();220 void create_second_connection();
204221
205 void handle_packet(RecvPacket& packet);222 void handle_packet(RecvPacket& packet, bool relogin_on_error = true);
206 void handle_failed_read();223 void handle_failed_read();
207224
208 // conversion functions225 // conversion functions
209226
=== 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 @@
39 * 3: Between build 19 and build 20 - Added network relay for internet games39 * 3: Between build 19 and build 20 - Added network relay for internet games
40 * 4: Between build 19 and build 20 - Using CHAP for password authentication40 * 4: Between build 19 and build 20 - Using CHAP for password authentication
41 * 5: Build 20 - Removed obsolete TELL_IP, modifications on user and game listing [supported]41 * 5: Build 20 - Removed obsolete TELL_IP, modifications on user and game listing [supported]
42 * 6: Between build 20 and build 21 - Added CHECK_PWD and PWD_OK commands
42 */43 */
43constexpr unsigned int kInternetGamingProtocolVersion = 5;44constexpr unsigned int kInternetGamingProtocolVersion = 6;
4445
45/**46/**
46 * The default timeout time after which the client tries to resend a package or even finally closes47 * The default timeout time after which the client tries to resend a package or even finally closes
@@ -182,7 +183,29 @@
182static const std::string IGPCMD_LOGIN = "LOGIN";183static const std::string IGPCMD_LOGIN = "LOGIN";
183184
184/**185/**
185 * This is sent by the metaserver after a IGPCMD_LOGIN by a registered client.186 * The client tries to check the password of the user without doing a full login.
187 * Should be sent without logging in before.
188 *
189 * Payload:
190 * \li string: protocol version (see kInternetGamingProtocolVersion)
191 * \li string: client name
192 * \li string: build_id of the client
193 *
194 * A IGPCMD_PWD_CHALLENGE exchange follows before the server replies with a IGPCMD_PWD_OK
195 * or IGPCMD_ERROR message.
196 *
197 * If the password is correct, the metaserver replies with a IGPCMD_PWD_OK command
198 * with the following payload:
199 * \li string: client name. Will be the same as sent before.
200 * \li string: clients rights (see client rights section above)
201 *
202 * If the password is wrong or some other error occurred, \ref IGPCMD_ERROR is returned.
203 */
204static const std::string IGPCMD_CHECK_PWD = "CHECK_PWD";
205static const std::string IGPCMD_PWD_OK = "PWD_OK";
206
207/**
208 * This is sent by the metaserver after a IGPCMD_LOGIN or IGPCMD_CHECK_PWD by a registered client.
186 * This is the first message of the a protocol similar to the challenge handshake authentication209 * This is the first message of the a protocol similar to the challenge handshake authentication
187 * protocol (CHAP) for secure transmission of the users password.210 * protocol (CHAP) for secure transmission of the users password.
188 * The server sends the nonce for hashing:211 * The server sends the nonce for hashing:
189212
=== 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 @@
188 const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());188 const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());
189 uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);189 uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);
190 std::string password = crypto::sha1(eb_password->text());190 std::string password = crypto::sha1(eb_password->text());
191 InternetGaming::ref().login(get_nickname(), password, true, meta, port);
192191
193 if (!InternetGaming::ref().logged_in()) {192 if (!InternetGaming::ref().check_password(get_nickname(), password, meta, port)) {
194 // something went wrong -> show the error message193 // something went wrong -> show the error message
195 // idealy it is about the wrong password194 // idealy it is about the wrong password
196 ChatMessage msg = InternetGaming::ref().get_messages().back();195 ChatMessage msg = InternetGaming::ref().get_messages().back();
197 UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);196 UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);
198 wmb.run<UI::Panel::Returncodes>();197 wmb.run<UI::Panel::Returncodes>();
199 InternetGaming::ref().reset();
200 eb_password->set_text("");198 eb_password->set_text("");
201 eb_password->focus();199 eb_password->focus();
202 return false;200 return false;

Subscribers

People subscribed via source and target branches

to status/vote changes: