I have pushed a small tweak and added some more comments.

Diff comments:

> === 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() != "*****") {

Better make this empty when it's not set and make the ** a function of 
displaying it only.

> +                     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();
>  }
>  
> 
> === 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
> @@ -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("");

When the user starts typing, the password is displayed as clear text, not as 
***. This needs to be fixed in the UI::Editbox class' draw() function.

> +     } 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"

We have this same complicated comparison above pull out a bool function to 
ensure consistency.

> +             "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("");
> +     }
> +}


-- 
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.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to