1642267 - The AutoAdminLogon and RequireSignon telemetry data is only recorded for successful authentications

Status: VERIFIED FIXED

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

Comment 1

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

Attachment: Bug 1642267 - Record the AutoAdminLogon and RequireSignon values in extra_keys so we can monitor these values regardless of success/fail for authentication attempts.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29a0b5a4c69f
Record the AutoAdminLogon and RequireSignon values in extra_keys so we can monitor these values regardless of success/fail for authentication attempts. r=MattN

Comment 3

[Dorel Luca [:dluca]]

Backed out changeset 29a0b5a4c69f (bug 1642267) for XPCShell in toolkit/modules/tests/xpcshell/test_osKeyStore.js

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304501116&repo=autoland&lineNumber=3141

Also fails on Browser-chrome:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=304502277&repo=autoland&lineNumber=3110

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=29a0b5a4c69fe49d7b447080f7400b11d4279307

Backout:
https://hg.mozilla.org/integration/autoland/rev/613497adb044d6c701ffb3786046f65985a4ac0a

Comment 4

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

Attachment: Data review form

Comment 5

[Chris H-C :chutten]

Comment on attachment 9153188 [details]
Data review form

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, :jaws and :loines are responsible.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.

---
Result: datareview+

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce622c9ded1c
Record the AutoAdminLogon and RequireSignon values in extra_keys so we can monitor these values regardless of success/fail for authentication attempts. r=MattN

Comment 7

[Atila Butkovits]

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

Comment 8

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified if the telemetry is correctly registered when the OS auth dialog is triggered by clicking the "Copy Password", "Show Password". "Edit Login" and "Export Logins". I have tested this on the latest Nightly 79.0a1 build (Build ID: 20200604092907) on Windows 10 x64 version 1903, Windows 8.1 x32 and Windows 7 x32. I have also verified that the telemetry registered on Mac 10.14.6 and Ubuntu 18.04 x64 are not affected.

Here are the results when an OS password is set:

1. Telemetry registered if AutoAdmin is NOT set and the Require Signon is set:
   reauthenticate os_auth success {"auto_admin": "false", "require_signon": "true"}
   reauthenticate os_auth fail {"auto_admin": "false", "require_signon": "true"}

2. Telemetry registered if AutoAdmin is set and the Require Signon is set:
   reauthenticate os_auth success {"auto_admin": "true", "require_signon": "true"}
   reauthenticate os_auth fail {"auto_admin": "true", "require_signon": "true"}

3. Telemetry registered if AutoAdmin is NOT set and the Require Signon is NOT set:
   reauthenticate os_auth success {"auto_admin": "false", "require_signon": "false"}
   reauthenticate os_auth fail {"auto_admin": "false", "require_signon": "false"}

4. Telemetry registered if AutoAdmin is set and the Require Signon is NOT set:
   reauthenticate os_auth success {"auto_admin": "true", "require_signon": "false"}
   reauthenticate os_auth fail {"auto_admin": "true", "require_signon": "false"}

If there is no OS password set the following event is sent for each of the above cases:

• reauthenticate os_auth success_no_password {"auto_admin": "false", "require_signon": "true"}
  @Jared is it necessary to have the extra info in the telemetry event if no OS password is set?

Comment 9

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

(In reply to Cosmin Muntean [:cmuntean], Ecosystem QA from comment #8)

If there is no OS password set the following event is sent for each of the above cases:

• reauthenticate os_auth success_no_password {"auto_admin": "false", "require_signon": "true"}
  @Jared is it necessary to have the extra info in the telemetry event if no OS password is set?

It's probably won't be used, but it's no harm in collecting it in this case. I would be extremely surprised if we saw lots of reports of no password being set but auto_admin was true, for example. But collecting this data will allow us to see anomalies like this. More likely would be seeing no password being set and require_signon set to false.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jaws, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

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

Comment on attachment 9153052 [details]
Bug 1642267 - Record the AutoAdminLogon and RequireSignon values in extra_keys so we can monitor these values regardless of success/fail for authentication attempts.

Beta/Release Uplift Approval Request

• User impact if declined: Mozilla will not be able to understand why some users fail to login and thus we will not be able to make changes to our software in a shorter timeframe.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Records in telemetry data that we were already using
• String changes made/needed: none

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9153052 [details]
Bug 1642267 - Record the AutoAdminLogon and RequireSignon values in extra_keys so we can monitor these values regardless of success/fail for authentication attempts.

approved for 78.0b8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/312e22df46bd

Comment 14

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified if the telemetry is correctly registered when the OS auth dialog is triggered by clicking the "Copy Password", "Show Password". and "Edit Login". I have verified this on the latest Beta 78.0b8 (Build ID: 20200616235426) on Windows 10 x64 version 1903, Windows 8.1 x32 and Windows 7 x64. I have also verified that the telemetry registered on Mac 10.14.6 and Ubuntu 18.04 x64 are not affected.

Here are the results when an OS password is set:

1. Telemetry registered if AutoAdmin is NOT set and the Require Signon is set:
   reauthenticate os_auth success {"auto_admin": "false", "require_signon": "true"}
   reauthenticate os_auth fail {"auto_admin": "false", "require_signon": "true"}

2. Telemetry registered if AutoAdmin is set and the Require Signon is set:
   reauthenticate os_auth success {"auto_admin": "true", "require_signon": "true"}
   reauthenticate os_auth fail {"auto_admin": "true", "require_signon": "true"}

3. Telemetry registered if AutoAdmin is NOT set and the Require Signon is NOT set:
   reauthenticate os_auth success {"auto_admin": "false", "require_signon": "false"}
   reauthenticate os_auth fail {"auto_admin": "false", "require_signon": "false"}

4. Telemetry registered if AutoAdmin is set and the Require Signon is NOT set:
   reauthenticate os_auth success {"auto_admin": "true", "require_signon": "false"}
   reauthenticate os_auth fail {"auto_admin": "true", "require_signon": "false"}

If there is no OS password set the following event is sent for each of the above cases:

• reauthenticate os_auth success_no_password {"auto_admin": "false", "require_signon": "true"}