Closed Bug 1632786 Opened 3 years ago Closed 3 years ago

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

Categories

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

Desktop
All
defect

Tracking

()

VERIFIED FIXED
Firefox 77
Tracking Status
firefox75 --- unaffected
firefox76 --- wontfix
firefox77 --- verified

People

(Reporter: cmuntean, Assigned: jaws)

References

Details

Attachments

(2 files)

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

What do you think about this?

Flags: needinfo?(jaws)

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.

Flags: needinfo?(jaws)
Blocks: 1628029
Priority: -- → P2
Assignee: nobody → jaws
Status: NEW → ASSIGNED

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.

Attachment #9144790 - Attachment description: Bug 1632786 - Rename 'fail' to 'cancel' to better describe what the user action was (it is not possible to 'fail' these dialogs, they will continue propmting until correct credentials are provided or the user cancels). r?MattN → Bug 1632786 - Rename 'fail' to 'cancel' to better describe what the user action was (it is not possible to 'fail' these dialogs, they will continue prompting until correct credentials are provided or the user cancels). r?MattN

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.

Attachment #9144790 - Attachment description: Bug 1632786 - Rename 'fail' to 'cancel' to better describe what the user action was (it is not possible to 'fail' these dialogs, they will continue prompting until correct credentials are provided or the user cancels). r?MattN → Bug 1632786 - Improve the documentation of the pwmgr.reauthenticate Event description. r?MattN
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/a2a5480f2344
Improve the documentation of the pwmgr.reauthenticate Event description. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.