widelands-dev team mailing list archive

[Toni Förster <toni.foerster@xxxxxxxxx>]

Toni Förster has proposed merging lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into lp:widelands with lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start as a prerequisite.

Commit message:
redesigned login box

- limit the possible characters for usernames
- draw a red box around the input field for erroneous input
- tell user were to register their username
- clicking registered checkbox focuses password field
- remove check from registered clears password field
- password field is only accessible when checkbox is clicked
- when a password is set, ***** is shown on opening

multiplayer login redesign

- only show login dialog when no name is set
- always show login box settings button

Requested reviews:
  kaputtnik (franku)
Related bugs:
  Bug #1827786 in widelands: "reconsider the metaserver loginbox"
  https://bugs.launchpad.net/widelands/+bug/1827786

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
-- 
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.

=== modified file 'src/ui_fsmenu/multiplayer.cc'
--- src/ui_fsmenu/multiplayer.cc	2019-05-11 18:50:30 +0000
+++ src/ui_fsmenu/multiplayer.cc	2019-05-12 11:12:04 +0000
@@ -58,21 +58,46 @@
 	vbox_.add_inf_space();
 	vbox_.add(&back, UI::Box::Resizing::kFullSize);
 
-	Section& s = g_options.pull_section("global");
-	auto_log_ = s.get_bool("auto_log", false);
-	if (auto_log_) {
-		showloginbox =
+	showloginbox =
 		   new UI::Button(this, "login_dialog", 0, 0, 0, 0, UI::ButtonStyle::kFsMenuSecondary,
 		                  g_gr->images().get("images/ui_basic/continue.png"), _("Show login dialog"));
-		showloginbox->sigclicked.connect(
+	showloginbox->sigclicked.connect(
 		   boost::bind(&FullscreenMenuMultiPlayer::show_internet_login, boost::ref(*this)));
-	}
 	layout();
 }
 
 /// called if the showloginbox button was pressed
 void FullscreenMenuMultiPlayer::show_internet_login() {
-	auto_log_ = false;
+	Section& s = g_options.pull_section("global");
+	LoginBox lb(*this);
+	if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {
+		nickname_ = lb.get_nickname();
+		s.set_string("nickname", nickname_);
+		/// NOTE: The password is only stored (in memory and on disk) and transmitted (over the
+		/// network
+		/// to the metaserver) as cryptographic hash. This does NOT mean that the password is
+		/// stored
+		/// securely on the local disk. While the password should be secure while transmitted to
+		/// the
+		/// metaserver (no-one can use the transmitted data to log in as the user) this is not the
+		/// case
+		/// for local storage. The stored hash of the password makes it hard to look at the
+		/// configuration
+		/// file and figure out the plaintext password to, e.g., log in on the forum. However, the
+		/// stored hash can be copied to another system and used to log in as the user on the
+		/// metaserver.
+		// Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
+		// passwords on the server are protected by SHA-1 we have to use it here, too
+		if (lb.get_password() != "*****") {
+			password_ = crypto::sha1(lb.get_password());
+			s.set_string("password_sha1", password_);
+		}
+
+		register_ = lb.registered();
+		s.set_bool("registered", lb.registered());
+	} else {
+		return;
+	}
 	internet_login();
 }
 
@@ -90,37 +115,15 @@
  */
 void FullscreenMenuMultiPlayer::internet_login() {
 	Section& s = g_options.pull_section("global");
-	if (auto_log_) {
-		nickname_ = s.get_string("nickname", _("nobody"));
-		password_ = s.get_string("password_sha1", "nobody");
-		register_ = s.get_bool("registered", false);
-	} else {
-		LoginBox lb(*this);
-		if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {
-			nickname_ = lb.get_nickname();
-			/// NOTE: The password is only stored (in memory and on disk) and transmitted (over the
-			/// network
-			/// to the metaserver) as cryptographic hash. This does NOT mean that the password is
-			/// stored
-			/// securely on the local disk. While the password should be secure while transmitted to
-			/// the
-			/// metaserver (no-one can use the transmitted data to log in as the user) this is not the
-			/// case
-			/// for local storage. The stored hash of the password makes it hard to look at the
-			/// configuration
-			/// file and figure out the plaintext password to, e.g., log in on the forum. However, the
-			/// stored hash can be copied to another system and used to log in as the user on the
-			/// metaserver.
-			// Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
-			// passwords on the server are protected by SHA-1 we have to use it here, too
-			password_ = crypto::sha1(lb.get_password());
-			register_ = lb.registered();
-
-			s.set_bool("registered", lb.registered());
-			s.set_bool("auto_log", lb.set_automaticlog());
-		} else {
-			return;
-		}
+
+	nickname_ = s.get_string("nickname", "");
+	password_ = s.get_string("password_sha1", "nobody");
+	register_ = s.get_bool("registered", false);
+
+	if (nickname_.empty() || nickname_.find_first_not_of("abcdefghijklmnopqrstuvwxyz"
+			"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890@.+-_") <= nickname_.size()) {
+		show_internet_login();
+		return;
 	}
 
 	// Try to connect to the metaserver

=== modified file 'src/ui_fsmenu/multiplayer.h'
--- src/ui_fsmenu/multiplayer.h	2019-05-11 18:50:30 +0000
+++ src/ui_fsmenu/multiplayer.h	2019-05-12 11:12:04 +0000
@@ -61,7 +61,6 @@
 	std::string nickname_;
 	std::string password_;
 	bool register_;
-	bool auto_log_;
 };
 
 #endif  // end of include guard: WL_UI_FSMENU_MULTIPLAYER_H

=== modified file 'src/wui/login_box.cc'
--- src/wui/login_box.cc	2019-05-11 18:50:30 +0000
+++ src/wui/login_box.cc	2019-05-12 11:12:04 +0000
@@ -26,69 +26,65 @@
 #include "ui_basic/messagebox.h"
 
 LoginBox::LoginBox(Panel& parent)
-   : Window(&parent, "login_box", 0, 0, 500, 220, _("Metaserver login")) {
+   : Window(&parent, "login_box", 0, 0, 500, 280, _("Metaserver login")) {
 	center_to_parent();
 
 	int32_t margin = 10;
 
 	ta_nickname = new UI::Textarea(this, margin, margin, _("Nickname:"));
-	ta_password = new UI::Textarea(this, margin, 40, _("Password:"));
+	ta_password = new UI::Textarea(this, margin, 70, _("Password:"));
 	eb_nickname = new UI::EditBox(this, 150, margin, 330, 20, 2, UI::PanelStyle::kWui);
-	eb_password = new UI::EditBox(this, 150, 40, 330, 20, 2, UI::PanelStyle::kWui);
-
-	pwd_warning =
-	   new UI::MultilineTextarea(this, margin, 65, 505, 50, UI::PanelStyle::kWui,
-	                             _("WARNING: Password will be shown and saved readable!"));
-
-	cb_register = new UI::Checkbox(this, Vector2i(margin, 110), _("Log in to a registered account"),
+	eb_password = new UI::EditBox(this, 150, 70, 330, 20, 2, UI::PanelStyle::kWui);
+
+	cb_register = new UI::Checkbox(this, Vector2i(margin, 40), _("Log in to a registered account."),
 	                               "", get_inner_w() - 2 * margin);
-	cb_auto_log = new UI::Checkbox(this, Vector2i(margin, 135),
-	                               _("Automatically use this login information from now on."), "",
-	                               get_inner_w() - 2 * margin);
-
-	UI::Button* loginbtn = new UI::Button(
+
+	register_account = new UI::MultilineTextarea(this, margin, 105, 470, 140, UI::PanelStyle::kWui,
+			(boost::format(_("You need an account on the widelands website, to use a registered "
+				"account. Please visit: %s Log in to your newly created account and set an online "
+				"gaming password on your profile page."))
+				% "\n\nhttps://widelands.org/accounts/register/\n\n";).str());
+
+	loginbtn = new UI::Button(
 	   this, "login",
 	   UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 :
 	                                   (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2,
 	   get_inner_h() - 20 - margin, 200, 20, UI::ButtonStyle::kWuiPrimary, _("Login"));
-	loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));
-	UI::Button* cancelbtn = new UI::Button(
+
+	cancelbtn = new UI::Button(
 	   this, "cancel",
 	   UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2 :
 	                                   (get_inner_w() / 2 - 200) / 2,
 	   loginbtn->get_y(), 200, 20, UI::ButtonStyle::kWuiSecondary, _("Cancel"));
+
+	loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));
 	cancelbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));
+	eb_nickname->changed.connect(boost::bind(&LoginBox::change_playername, this));
+	cb_register->clickedto.connect(boost::bind(&LoginBox::clicked_register, this));
 
 	Section& s = g_options.pull_section("global");
 	eb_nickname->set_text(s.get_string("nickname", _("nobody")));
 	cb_register->set_state(s.get_bool("registered", false));
+
+	if (registered()) {
+		eb_password->set_text("*****");
+	} else {
+		eb_password->set_can_focus(false);
+		ta_password->set_color(UI_FONT_CLR_DISABLED);
+	}
+
 	eb_nickname->focus();
 }
 
+/// think function of the UI (main loop)
+void LoginBox::think() {
+	verify_input();
+}
+
 /**
  * called, if "login" is pressed.
  */
 void LoginBox::clicked_ok() {
-	// Check if all needed input fields are valid
-	if (eb_nickname->text().empty()) {
-		UI::WLMessageBox mb(
-		   this, _("Empty Nickname"), _("Please enter a nickname!"), UI::WLMessageBox::MBoxType::kOk);
-		mb.run<UI::Panel::Returncodes>();
-		return;
-	}
-	if (eb_nickname->text().find(' ') <= eb_nickname->text().size()) {
-		UI::WLMessageBox mb(this, _("Space in Nickname"),
-		                    _("Sorry, but spaces are not allowed in nicknames!"),
-		                    UI::WLMessageBox::MBoxType::kOk);
-		mb.run<UI::Panel::Returncodes>();
-		return;
-	}
-	if (eb_password->text().empty() && cb_register->get_state()) {
-		UI::WLMessageBox mb(this, _("Empty Password"), _("Please enter your password!"),
-		                    UI::WLMessageBox::MBoxType::kOk);
-		mb.run<UI::Panel::Returncodes>();
-		return;
-	}
 	end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
 }
 
@@ -97,6 +93,11 @@
 	end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
 }
 
+/// Calles when nickname was changed
+void LoginBox::change_playername() {
+	cb_register->set_state(false);
+}
+
 bool LoginBox::handle_key(bool down, SDL_Keysym code) {
 	if (down) {
 		switch (code.sym) {
@@ -113,3 +114,44 @@
 	}
 	return UI::Panel::handle_key(down, code);
 }
+
+void LoginBox::clicked_register() {
+	if (cb_register->get_state()) {
+		ta_password->set_color(UI_FONT_CLR_DISABLED);
+		eb_password->set_can_focus(false);
+		eb_password->set_text("");
+	} else {
+		ta_password->set_color(UI_FONT_CLR_FG);
+		eb_password->set_can_focus(true);
+		eb_password->focus();
+	}
+}
+
+void LoginBox::verify_input() {
+	// Check if all needed input fields are valid
+	loginbtn->set_enabled(true);
+	eb_nickname->set_tooltip("");
+	eb_password->set_tooltip("");
+	eb_nickname->set_warning(false);
+
+	if (eb_nickname->text().empty()) {
+		eb_nickname->set_warning(true);
+		eb_nickname->set_tooltip(_("Please enter a nickname!"));
+		loginbtn->set_enabled(false);
+	} else if (eb_nickname->text().find_first_not_of("abcdefghijklmnopqrstuvwxyz"
+		"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890@.+-_") <= eb_nickname->text().size()) {
+			eb_nickname->set_warning(true);
+			eb_nickname->set_tooltip(_("Enter a valid nickname. This value may contain only "
+													  "English letters, numbers, and @ . + - _ characters."));
+			loginbtn->set_enabled(false);
+	}
+
+	if (eb_password->text().empty() && cb_register->get_state()) {
+		eb_password->set_tooltip(_("Please enter your password!"));
+		loginbtn->set_enabled(false);
+	}
+
+	if (eb_password->has_focus() && eb_password->text() == "*****") {
+		eb_password->set_text("");
+	}
+}

=== modified file 'src/wui/login_box.h'
--- src/wui/login_box.h	2019-05-11 18:50:30 +0000
+++ src/wui/login_box.h	2019-05-12 11:12:04 +0000
@@ -29,6 +29,8 @@
 struct LoginBox : public UI::Window {
 	explicit LoginBox(UI::Panel&);
 
+	void think() override;
+
 	std::string get_nickname() {
 		return eb_nickname->text();
 	}
@@ -38,24 +40,25 @@
 	bool registered() {
 		return cb_register->get_state();
 	}
-	bool set_automaticlog() {
-		return cb_auto_log->get_state();
-	}
 
 	/// Handle keypresses
 	bool handle_key(bool down, SDL_Keysym code) override;
 
 private:
+	void change_playername();
 	void clicked_back();
 	void clicked_ok();
+	void clicked_register();
+	void verify_input();
 
+	UI::Button* loginbtn;
+	UI::Button* cancelbtn;
 	UI::EditBox* eb_nickname;
 	UI::EditBox* eb_password;
 	UI::Checkbox* cb_register;
-	UI::Checkbox* cb_auto_log;
 	UI::Textarea* ta_nickname;
 	UI::Textarea* ta_password;
-	UI::MultilineTextarea* pwd_warning;
+	UI::MultilineTextarea* register_account;
 };
 
 #endif  // end of include guard: WL_WUI_LOGIN_BOX_H