GNOME Bugzilla – Bug 682544
Don't throw the user back to the lock screen if they get their password wrong
Last modified: 2012-09-04 21:40:41 UTC
Created attachment 222226 [details] [review] Giovanni's patch It's not very nice behaviour, and it gives the user more work to do. Instead, keep the login screen open and add a prompt to reenter the password: https://raw.github.com/gnome-design-team/gnome-mockups/master/system-lock-login-boot/authentication-failure.png Attaching Giovanni's patch from bug 619955.
Created attachment 222457 [details] [review] ShellUserVerifier: fix cancellation Ensure that all async callbacks check and ignore G_IO_ERROR_CANCELLED. Ensure that all runs of authentication have their own GCancellable, so that .begin() can be called multiple times on the same user verifier. Check for fingerprint reader when beginning authentication, and not when reset by GDM.
Created attachment 222458 [details] [review] Login/UnlockDialog: don't reset immediately if auth fails Instead of showing a notification, add a small message immediately below the entry, and give the user two more attempts to login, before going back to the welcome or lock screen. Rebased.
Created attachment 222473 [details] [review] ShellUserVerifier: fix cancellation Ensure that all async callbacks check and ignore G_IO_ERROR_CANCELLED. Ensure that all runs of authentication have their own GCancellable, so that .begin() can be called multiple times on the same user verifier. Check for fingerprint reader when beginning authentication, and not when reset by GDM. Fixed one problem if .cancel() was called before ever calling .begin() (could only happen after 10 minutes of no activity in gdm)
Review of attachment 222473 [details] [review]: Looks fine except for the whitespace fixes. ::: js/gdm/util.js @@ +194,3 @@ + try { + obj.call_begin_verification_for_user_finish(result); + } catch(e if e.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) { return; } Newlines @@ +208,3 @@ + try { + obj.call_begin_verification_for_user_finish(result); + } catch(e if e.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) { return; } Newlines @@ +219,3 @@ + try { + obj.call_begin_verification_finish(result); + } catch(e if e.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) { return; } Newlines
Comment on attachment 222473 [details] [review] ShellUserVerifier: fix cancellation Attachment 222473 [details] pushed as 703417a - ShellUserVerifier: fix cancellation
Review of attachment 222458 [details] [review]: Looks good. ::: js/gdm/loginDialog.js @@ +1151,3 @@ + // go back to the welcome screen. + + if (!this._userName || (++this._failCounter) == 3) { I wonder if '3' should be controlled by a GSetting. Seems like something administrators would want to tweak. At least make it not magic. ::: js/ui/unlockDialog.js @@ +226,3 @@ + + this._failCounter++; + if (this._failCounter == 3) { Eesh. Could we put this logic in the user verifier instead of being split between two places?
Created attachment 223352 [details] [review] Login/UnlockDialog: don't reset immediately if auth fails Instead of showing a notification, add a small message immediately below the entry, and give the user two more attempts to login, before going back to the welcome or lock screen.
Review of attachment 223352 [details] [review]: Minor nits, otherwise looks fine. ::: js/gdm/loginDialog.js @@ +1114,3 @@ let hold = new Batch.Hold(); + this._userName = null; Do you still need this._userName? @@ +1158,3 @@ let hold = new Batch.Hold(); + this._userName = userName; Do you still need this._userName? ::: js/gdm/util.js @@ +310,3 @@ + // For Not Listed / enterprise logins, immediately reset + // the dialog + // Otherwise, we allow three attempts. After the third, we after ALLOWED_FAILURES
Attachment 223352 [details] pushed as db20a54 - Login/UnlockDialog: don't reset immediately if auth fails