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
1=== modified file 'src/network/internet_gaming.cc'
2--- src/network/internet_gaming.cc 2019-05-25 08:25:55 +0000
3+++ src/network/internet_gaming.cc 2019-06-01 16:31:43 +0000
4@@ -235,6 +235,53 @@
5 reset();
6 }
7
8+bool InternetGaming::check_password(const std::string& nick,
9+ const std::string& pwd,
10+ const std::string& metaserver,
11+ uint32_t port) {
12+ reset();
13+
14+ meta_ = metaserver;
15+ port_ = port;
16+ initialize_connection();
17+
18+ // Has to be set for the password challenge later on
19+ authenticator_ = pwd;
20+
21+ log("InternetGaming: Verifying password.\n");
22+ {
23+ SendPacket s;
24+ s.string(IGPCMD_CHECK_PWD);
25+ s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
26+ s.string(nick);
27+ s.string(build_id());
28+ net->send(s);
29+ }
30+
31+ // Now let's see, whether the metaserver is answering
32+ uint32_t const secs = time(nullptr);
33+ state_ = CONNECTING;
34+ while (kInternetGamingTimeout > time(nullptr) - secs) {
35+ handle_metaserver_communication(false);
36+ if (state_ != CONNECTING) {
37+ if (state_ == LOBBY) {
38+ SendPacket s;
39+ s.string(IGPCMD_DISCONNECT);
40+ s.string("CONNECTION_CLOSED");
41+ net->send(s);
42+ reset();
43+ return true;
44+ } else if (error()) {
45+ reset();
46+ return false;
47+ }
48+ }
49+ }
50+ log("InternetGaming: No answer from metaserver!\n");
51+ reset();
52+ return false;
53+}
54+
55 /**
56 * Handle situation when reading from socket failed.
57 */
58@@ -265,7 +312,7 @@
59 }
60
61 /// handles all communication between the metaserver and the client
62-void InternetGaming::handle_metaserver_communication() {
63+void InternetGaming::handle_metaserver_communication(bool relogin_on_error) {
64 if (error())
65 return;
66 try {
67@@ -278,7 +325,7 @@
68 // Process all available packets
69 std::unique_ptr<RecvPacket> packet = net->try_receive();
70 if (packet) {
71- handle_packet(*packet);
72+ handle_packet(*packet, relogin_on_error);
73 } else {
74 // Nothing more to receive
75 break;
76@@ -315,7 +362,7 @@
77 set_error();
78 waittimeout_ = std::numeric_limits<int32_t>::max();
79 log("InternetGaming: reached a timeout for an awaited answer of the metaserver!\n");
80- if (!relogin()) {
81+ if (relogin_on_error && !relogin()) {
82 // Do not try to relogin again automatically.
83 reset();
84 set_error();
85@@ -328,7 +375,7 @@
86 if (time(nullptr) - lastping_ > 240) {
87 // Try to relogin
88 set_error();
89- if (!relogin()) {
90+ if (relogin_on_error && !relogin()) {
91 // Do not try to relogin again automatically.
92 reset();
93 set_error();
94@@ -337,7 +384,7 @@
95 }
96
97 /// Handle one packet received from the metaserver.
98-void InternetGaming::handle_packet(RecvPacket& packet) {
99+void InternetGaming::handle_packet(RecvPacket& packet, bool relogin_on_error) {
100 std::string cmd = packet.string();
101
102 // First check if everything is fine or whether the metaserver broke up with the client.
103@@ -347,7 +394,7 @@
104 if (reason == "CLIENT_TIMEOUT") {
105 // Try to relogin
106 set_error();
107- if (!relogin()) {
108+ if (relogin_on_error && !relogin()) {
109 // Do not try to relogin again automatically.
110 reset();
111 set_error();
112@@ -405,6 +452,12 @@
113 asctime(gmtime(&now)));
114 return;
115
116+ } else if (cmd == IGPCMD_PWD_OK) {
117+ const time_t now = time(nullptr);
118+ log("InternetGaming: Password check successful at UTC %s", asctime(gmtime(&now)));
119+ state_ = LOBBY;
120+ return;
121+
122 } else if (cmd == IGPCMD_ERROR) {
123 std::string errortype = packet.string();
124 if (errortype != IGPCMD_LOGIN && errortype != IGPCMD_PWD_CHALLENGE) {
125
126=== modified file 'src/network/internet_gaming.h'
127--- src/network/internet_gaming.h 2019-05-15 09:56:36 +0000
128+++ src/network/internet_gaming.h 2019-06-01 16:31:43 +0000
129@@ -79,6 +79,23 @@
130 bool relogin();
131 void logout(const std::string& msgcode = "CONNECTION_CLOSED");
132
133+ /**
134+ * Connects to the metaserver and checks the password without logging in.
135+ *
136+ * Note that the user might be logged in with another username and as unregistered
137+ * if the user account is already in use by another client.
138+ * @warning Resets the current connection.
139+ * @param nick The username.
140+ * @param pwd The password.
141+ * @param metaserver The hostname of the metaserver.
142+ * @param port The port number of the metaserver.
143+ * @return Whether the password was valid.
144+ */
145+ bool check_password(const std::string& nick,
146+ const std::string& pwd,
147+ const std::string& metaserver,
148+ uint32_t port);
149+
150 /// \returns whether the client is logged in
151 bool logged_in() {
152 return (state_ == LOBBY) || (state_ == CONNECTING) || (state_ == IN_GAME);
153@@ -92,7 +109,7 @@
154 clientupdate_ = true;
155 }
156
157- void handle_metaserver_communication();
158+ void handle_metaserver_communication(bool relogin_on_error = true);
159
160 // Game specific functions
161 /**
162@@ -202,7 +219,7 @@
163 */
164 void create_second_connection();
165
166- void handle_packet(RecvPacket& packet);
167+ void handle_packet(RecvPacket& packet, bool relogin_on_error = true);
168 void handle_failed_read();
169
170 // conversion functions
171
172=== modified file 'src/network/internet_gaming_protocol.h'
173--- src/network/internet_gaming_protocol.h 2019-02-23 11:00:49 +0000
174+++ src/network/internet_gaming_protocol.h 2019-06-01 16:31:43 +0000
175@@ -39,8 +39,9 @@
176 * 3: Between build 19 and build 20 - Added network relay for internet games
177 * 4: Between build 19 and build 20 - Using CHAP for password authentication
178 * 5: Build 20 - Removed obsolete TELL_IP, modifications on user and game listing [supported]
179+ * 6: Between build 20 and build 21 - Added CHECK_PWD and PWD_OK commands
180 */
181-constexpr unsigned int kInternetGamingProtocolVersion = 5;
182+constexpr unsigned int kInternetGamingProtocolVersion = 6;
183
184 /**
185 * The default timeout time after which the client tries to resend a package or even finally closes
186@@ -182,7 +183,29 @@
187 static const std::string IGPCMD_LOGIN = "LOGIN";
188
189 /**
190- * This is sent by the metaserver after a IGPCMD_LOGIN by a registered client.
191+ * The client tries to check the password of the user without doing a full login.
192+ * Should be sent without logging in before.
193+ *
194+ * Payload:
195+ * \li string: protocol version (see kInternetGamingProtocolVersion)
196+ * \li string: client name
197+ * \li string: build_id of the client
198+ *
199+ * A IGPCMD_PWD_CHALLENGE exchange follows before the server replies with a IGPCMD_PWD_OK
200+ * or IGPCMD_ERROR message.
201+ *
202+ * If the password is correct, the metaserver replies with a IGPCMD_PWD_OK command
203+ * with the following payload:
204+ * \li string: client name. Will be the same as sent before.
205+ * \li string: clients rights (see client rights section above)
206+ *
207+ * If the password is wrong or some other error occurred, \ref IGPCMD_ERROR is returned.
208+ */
209+static const std::string IGPCMD_CHECK_PWD = "CHECK_PWD";
210+static const std::string IGPCMD_PWD_OK = "PWD_OK";
211+
212+/**
213+ * This is sent by the metaserver after a IGPCMD_LOGIN or IGPCMD_CHECK_PWD by a registered client.
214 * This is the first message of the a protocol similar to the challenge handshake authentication
215 * protocol (CHAP) for secure transmission of the users password.
216 * The server sends the nonce for hashing:
217
218=== modified file 'src/wui/login_box.cc'
219--- src/wui/login_box.cc 2019-05-29 12:49:27 +0000
220+++ src/wui/login_box.cc 2019-06-01 16:31:43 +0000
221@@ -188,15 +188,13 @@
222 const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());
223 uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);
224 std::string password = crypto::sha1(eb_password->text());
225- InternetGaming::ref().login(get_nickname(), password, true, meta, port);
226
227- if (!InternetGaming::ref().logged_in()) {
228+ if (!InternetGaming::ref().check_password(get_nickname(), password, meta, port)) {
229 // something went wrong -> show the error message
230 // idealy it is about the wrong password
231 ChatMessage msg = InternetGaming::ref().get_messages().back();
232 UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);
233 wmb.run<UI::Panel::Returncodes>();
234- InternetGaming::ref().reset();
235 eb_password->set_text("");
236 eb_password->focus();
237 return false;

Subscribers

People subscribed via source and target branches

to status/vote changes: