1632786 - Canceling the OS auth dialog is wrongly counted as an invalid login attempt

Status: VERIFIED FIXED

(Firefox :: about:logins, defect, P2)

Description

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Attachment: canceling the OS auth dialog is counted as invalid login.gif

[Affected versions]:

• Nightly 77.0a1;
• Beta 76.0b8;

[Affected Platforms]:

• Windows 10 x64;
• Windows 7 x64;
• Windows 8.1 x32;

[Prerequisites]:

• Have an OS password set.
• Have at least one login saved.
• Have an account lockout policy set to 5 invalid logon attempts.

[Steps to reproduce]:

1. Open the latest Nightly Firefox browser.
2. Navigate to the "about:logins" page and select a saved login.
3. Click on the "Show Password" button.
4. Click the "Cancel" button of the OS auth dialog.
5. Repeat 4 more times the step 3 and 4.
6. Click again the "Show Password" button.
7. In the OS auth dialog enter the valid password.
8. Observe the behavior.

[Expected result]:

• The password is shown.

[Actual result]:

• "The referenced account is currently locked out and may not be logged on to." message is displayed and the password is not shown.

[Notes]:

• Also, the "pwmgr reauthenticate os_auth fail" telemetry event is registered when the OS auth dialog is canceled. This telemetry event is also sent on Mac OS when the auth dialog is canceled.
• Attached a screen recording with the issue.

Comment 1

[Matthew N. [:MattN]]

What do you think about this?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

The lockout is because the blank password auth attempt is happening on each show of the dialog. That will be fixed by bug 1633090.

The line at https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/toolkit/modules/OSKeyStore.jsm#258 needs to be changed to fix the Telemetry issue that was mentioned in the notes. We will either need to add a "canceled" value or remove it.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Turns out this is less of a bug than was originally thought. Both for Master Password as well as OS auth (Windows and Linux), the only way to exit the dialog without a valid authentication attempt is through cancel. So there is no separation between "fail" or "cancel", just misleading terminology.

I will attach a patch that changes "fail" to "cancel" and add more to the Events.yaml description.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1632786 - Improve the documentation of the pwmgr.reauthenticate Event description. r?MattN

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

This change will cause some inconsistencies with Telemetry data as users update, but the Telemetry probe is recent enough that we may be OK with changing the name.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2a5480f2344
Improve the documentation of the pwmgr.reauthenticate Event description. r=MattN

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/a2a5480f2344

Comment 8

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this issue and the description is correctly updated in the probe dictionary. Considering this I will mark this issue as verified - fixed.