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 into lp:widelands with lp:~widelands-dev/widelands/bug-1825932-open-games 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 onopening

multiplay 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/+merge/367100
-- 
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1825932-open-games.

=== modified file 'src/ui_fsmenu/multiplayer.cc'
--- src/ui_fsmenu/multiplayer.cc	2019-02-23 11:00:49 +0000
+++ src/ui_fsmenu/multiplayer.cc	2019-05-08 12:06:02 +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,14 @@
  */
 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_ == " ") {
+		show_internet_login();
+		return;
 	}
 
 	// Try to connect to the metaserver

=== modified file 'src/ui_fsmenu/multiplayer.h'
--- src/ui_fsmenu/multiplayer.h	2019-02-23 11:00:49 +0000
+++ src/ui_fsmenu/multiplayer.h	2019-05-08 12:06:02 +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-02-23 11:00:49 +0000
+++ src/wui/login_box.cc	2019-05-08 12:06:02 +0000
@@ -26,7 +26,7 @@
 #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, 260, _("Metaserver login")) {
 	center_to_parent();
 
 	int32_t margin = 10;
@@ -36,59 +36,52 @@
 	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"),
+	cb_register = new UI::Checkbox(this, Vector2i(margin, 70), _("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,
+			_("To register an account, please visit our website: \n\n"
+				"https://widelands.org/accounts/register/ \n\n"
+				"Log in to your newly created account and set an \n"
+				"online gaming password on your profile page."));
+
+	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)));
 
 	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);
+	}
+
 	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);
 }
 
@@ -113,3 +106,43 @@
 	}
 	return UI::Panel::handle_key(down, code);
 }
+
+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);
+	eb_password->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);
+	}
+
+	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_warning(true);
+		eb_password->set_tooltip(_("Please enter your password!"));
+		eb_password->focus();
+		loginbtn->set_enabled(false);
+	}
+
+	if (!eb_password->text().empty() && !cb_register->get_state()) {
+		eb_password->set_text("");
+		eb_password->set_can_focus(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-02-23 11:00:49 +0000
+++ src/wui/login_box.h	2019-05-08 12:06:02 +0000
@@ -29,6 +29,10 @@
 struct LoginBox : public UI::Window {
 	explicit LoginBox(UI::Panel&);
 
+	void think() override;
+
+	void verify_input();
+
 	std::string get_nickname() {
 		return eb_nickname->text();
 	}
@@ -38,9 +42,6 @@
 	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;
@@ -49,13 +50,14 @@
 	void clicked_back();
 	void clicked_ok();
 
+	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