Bug 1628029 - Add telemetry events for OS authentication. r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D70069

browser/components/aboutlogins/AboutLoginsChild.jsm

@@ -137,7 +137,7 @@ class AboutLoginsChild extends JSWindowActorChild {
         break;
       }
       case "AboutLoginsRecordTelemetryEvent": {
-        let { method, object, extra = {} } = event.detail;
+        let { method, object, extra = {}, value = null } = event.detail;
 
         if (method == "open_management") {
           let { docShell } = this.browsingContext;
@@ -163,7 +163,7 @@ class AboutLoginsChild extends JSWindowActorChild {
             TELEMETRY_EVENT_CATEGORY,
             method,
             object,
-            null,
+            value,
             extra
           );
         } catch (ex) {
@@ -198,7 +198,26 @@ class AboutLoginsChild extends JSWindowActorChild {
     switch (message.name) {
       case "AboutLogins:MasterPasswordResponse":
         if (masterPasswordPromise) {
-          masterPasswordPromise.resolve(message.data);
+          masterPasswordPromise.resolve(message.data.result);
+          try {
+            let {
+              method,
+              object,
+              extra = {},
+              value = null,
+            } = message.data.telemetryEvent;
+            Services.telemetry.recordEvent(
+              TELEMETRY_EVENT_CATEGORY,
+              method,
+              object,
+              value,
+              extra
+            );
+          } catch (ex) {
+            Cu.reportError(
+              "AboutLoginsChild: error recording telemetry event: " + ex.message
+            );
+          }
         }
         break;
       case "AboutLogins:Setup":

browser/components/aboutlogins/AboutLoginsParent.jsm

@@ -323,51 +323,74 @@ class AboutLoginsParent extends JSWindowActorParent {
           );
         }
 
-        if (Date.now() < AboutLogins._authExpirationTime) {
-          this.sendAsyncMessage("AboutLogins:MasterPasswordResponse", true);
-          return;
-        }
-
-        // This does no harm if master password isn't set.
-        let tokendb = Cc["@mozilla.org/security/pk11tokendb;1"].createInstance(
-          Ci.nsIPK11TokenDB
-        );
-        let token = tokendb.getInternalKeyToken();
-
         let loggedIn = false;
-        // Use the OS auth dialog if there is no master password
-        if (!token.hasPassword && !OS_AUTH_ENABLED) {
-          loggedIn = true;
-        } else if (!token.hasPassword && OS_AUTH_ENABLED) {
-          if (AppConstants.platform == "macosx") {
-            // OS Auth dialogs on macOS must only provide the "reason" that the prompt
-            // is being displayed.
-            messageId += "-macosx";
+        let telemetryEvent;
+
+        try {
+          // This does no harm if master password isn't set.
+          let tokendb = Cc[
+            "@mozilla.org/security/pk11tokendb;1"
+          ].createInstance(Ci.nsIPK11TokenDB);
+          let token = tokendb.getInternalKeyToken();
+
+          if (Date.now() < AboutLogins._authExpirationTime) {
+            loggedIn = true;
+            telemetryEvent = {
+              object: token.hasPassword ? "master_password" : "os_auth",
+              method: "reauthenticate",
+              value: "success_no_prompt",
+            };
+            return;
           }
-          let [messageText, captionText] = await AboutLoginsL10n.formatMessages(
-            [
+
+          // Use the OS auth dialog if there is no master password
+          if (!token.hasPassword && !OS_AUTH_ENABLED) {
+            loggedIn = true;
+            telemetryEvent = {
+              object: "os_auth",
+              method: "reauthenticate",
+              value: "success_disabled",
+            };
+            return;
+          }
+          if (!token.hasPassword && OS_AUTH_ENABLED) {
+            if (AppConstants.platform == "macosx") {
+              // OS Auth dialogs on macOS must only provide the "reason" that the prompt
+              // is being displayed.
+              messageId += "-macosx";
+            }
+            let [
+              messageText,
+              captionText,
+            ] = await AboutLoginsL10n.formatMessages([
               {
                 id: messageId,
               },
               {
                 id: "about-logins-os-auth-dialog-caption",
               },
-            ]
-          );
-          loggedIn = await OSKeyStore.ensureLoggedIn(
-            messageText.value,
-            captionText.value,
-            ownerGlobal,
-            false
-          );
-        } else {
+            ]);
+            let result = await OSKeyStore.ensureLoggedIn(
+              messageText.value,
+              captionText.value,
+              ownerGlobal,
+              false
+            );
+            loggedIn = result.authenticated;
+            telemetryEvent = {
+              object: "os_auth",
+              method: "reauthenticate",
+              value: result.auth_details,
+            };
+            return;
+          }
           // Force a log-out of the Master Password.
           token.checkPassword("");
 
           // If a master password prompt is already open, just exit early and return false.
           // The user can re-trigger it after responding to the already open dialog.
           if (Services.logins.uiBusy) {
-            this.sendAsyncMessage("AboutLogins:MasterPasswordResponse", false);
+            loggedIn = false;
             return;
           }
 
@@ -381,13 +404,21 @@ class AboutLoginsParent extends JSWindowActorParent {
             // User is also logged out of Software Security Device.
           }
           loggedIn = token.isLoggedIn();
+          telemetryEvent = {
+            object: "master_password",
+            method: "reauthenticate",
+            value: loggedIn ? "success" : "fail",
+          };
+        } finally {
+          if (loggedIn) {
+            const AUTH_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
+            AboutLogins._authExpirationTime = Date.now() + AUTH_TIMEOUT_MS;
+          }
+          this.sendAsyncMessage("AboutLogins:MasterPasswordResponse", {
+            result: loggedIn,
+            telemetryEvent,
+          });
         }
-
-        if (loggedIn) {
-          const AUTH_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
-          AboutLogins._authExpirationTime = Date.now() + AUTH_TIMEOUT_MS;
-        }
-        this.sendAsyncMessage("AboutLogins:MasterPasswordResponse", loggedIn);
         break;
       }
       case "AboutLogins:Subscribe": {

browser/components/aboutlogins/tests/browser/browser_aaa_eventTelemetry_run_first.js

@@ -82,10 +82,13 @@ add_task(async function test_telemetry_events() {
       copyButton.click();
     });
     await reauthObserved;
-    await LoginTestUtils.telemetry.waitForEventCount(4);
+    // When reauth is observed an extra telemetry event will be recorded
+    // for the reauth, hence the event count increasing by 2 here, and later
+    // in the test as well.
+    await LoginTestUtils.telemetry.waitForEventCount(5);
   }
   let nextTelemetryEventCount = OSKeyStoreTestUtils.canTestOSKeyStoreLogin()
-    ? 5
+    ? 6
     : 4;
 
   let promiseNewTab = BrowserTestUtils.waitForNewTab(
@@ -107,6 +110,7 @@ add_task(async function test_telemetry_events() {
     let reauthObserved = forceAuthTimeoutAndWaitForOSKeyStoreLogin({
       loginResult: true,
     });
+    nextTelemetryEventCount++; // An extra event is observed for the reauth event.
     await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() {
       let loginItem = content.document.querySelector("login-item");
       let revealCheckbox = loginItem.shadowRoot.querySelector(
@@ -127,15 +131,14 @@ add_task(async function test_telemetry_events() {
     });
     await LoginTestUtils.telemetry.waitForEventCount(nextTelemetryEventCount++);
 
-    reauthObserved = forceAuthTimeoutAndWaitForOSKeyStoreLogin({
-      loginResult: true,
-    });
+    // Don't force the auth timeout here to check that `auth_skipped: true` is set as
+    // in `extra`.
+    nextTelemetryEventCount++; // An extra event is observed for the reauth event.
     await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() {
       let loginItem = content.document.querySelector("login-item");
       let editButton = loginItem.shadowRoot.querySelector(".edit-button");
       editButton.click();
     });
-    await reauthObserved;
     await LoginTestUtils.telemetry.waitForEventCount(nextTelemetryEventCount++);
 
     await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() {
@@ -238,10 +241,13 @@ add_task(async function test_telemetry_events() {
     [true, "pwmgr", "open_management", "direct"],
     [true, "pwmgr", "select", "existing_login", null, { breached: "true" }],
     [true, "pwmgr", "copy", "username", null, { breached: "true" }],
+    [testOSAuth, "pwmgr", "reauthenticate", "os_auth", "success"],
     [testOSAuth, "pwmgr", "copy", "password", null, { breached: "true" }],
     [true, "pwmgr", "open_site", "existing_login", null, { breached: "true" }],
+    [testOSAuth, "pwmgr", "reauthenticate", "os_auth", "success"],
     [testOSAuth, "pwmgr", "show", "password", null, { breached: "true" }],
     [testOSAuth, "pwmgr", "hide", "password", null, { breached: "true" }],
+    [testOSAuth, "pwmgr", "reauthenticate", "os_auth", "success_no_prompt"],
     [testOSAuth, "pwmgr", "edit", "existing_login", null, { breached: "true" }],
     [testOSAuth, "pwmgr", "save", "existing_login", null, { breached: "true" }],
     [true, "pwmgr", "delete", "existing_login", null, { breached: "true" }],

browser/components/preferences/in-content/privacy.js

@@ -1952,7 +1952,7 @@ var gPrivacyPane = {
         window,
         false
       );
-      if (!loggedIn) {
+      if (!loggedIn.authenticated) {
         return;
       }
     }

browser/extensions/formautofill/FormAutofillParent.jsm

@@ -341,7 +341,7 @@ class FormAutofillParent extends JSWindowActorParent {
         break;
       }
       case "FormAutofill:SaveCreditCard": {
-        if (!(await FormAutofillUtils.ensureLoggedIn())) {
+        if (!(await FormAutofillUtils.ensureLoggedIn()).authenticated) {
           log.warn("User canceled encryption login");
           return undefined;
         }
@@ -690,7 +690,7 @@ class FormAutofillParent extends JSWindowActorParent {
         return;
       }
 
-      if (!(await FormAutofillUtils.ensureLoggedIn())) {
+      if (!(await FormAutofillUtils.ensureLoggedIn()).authenticated) {
         log.warn("User canceled encryption login");
         return;
       }

browser/extensions/formautofill/content/manageDialog.js

@@ -372,6 +372,7 @@ class ManageCreditCards extends ManageRecords {
     if (
       !creditCard ||
       (await FormAutofillUtils.ensureLoggedIn(reauthPasswordPromptMessage))
+        .authenticated
     ) {
       let decryptedCCNumObj = {};
       if (creditCard && creditCard["cc-number-encrypted"]) {

toolkit/components/telemetry/Events.yaml

@@ -607,16 +607,35 @@ pwmgr:
     record_in_processes: [main, content]
     release_channel_collection: opt-out
     expiry_version: never
+  reauthenticate:
+    description: >
+      Measure how often users are asked to authenticate with their Operating System or Master Password to gain access to stored passwords.
+      Possible values are as follows,
+        "success_no_prompt" should be used when the feature is enabled but no prompt is given to the user because they have recently authenticated.
+        "success_disabled" is used when the feature is disabled.
+        "success_unsupported_platform" should be set when the user attempts to authenticate on an unsupported platform.
+    objects: [
+      "master_password",
+      "os_auth",
+    ]
+    methods: ["reauthenticate"]
+    bug_numbers:
+      - 1628029
+    expiry_version: never
+    notification_emails: ["loines@mozilla.com", "passwords-dev@mozilla.org", "jaws@mozilla.com"]
+    release_channel_collection: opt-out
+    products:
+      - "firefox"
+    record_in_processes: [content]
   mgmt_interaction:
     description: >
-      These events record interactions on the about:logins page. Sort methods have an accompanying
-      value that specifies what order the list of logins is sorted with: {last_changed, last_used, title}.
+      These events record interactions on the about:logins page.
     extra_keys:
       breached: >
         Whether the login is marked as breached or not. If a login is both breached and vulnerable, it will only be reported as breached.
       vulnerable: >
         Whether the login is marked as vulnerable or not. If a login is both breached and vulnerable, it will only be reported as breached.
-      sort_key: The key that is used for sorting the login-list
+      sort_key: The key that is used for sorting the login-list. Should only be set with the "sort" method.
     objects: [
       "existing_login",
       "list",
@@ -643,13 +662,12 @@ pwmgr:
     bug_numbers:
       - 1548463
       - 1600958
+      - 1549115
     expiry_version: never
     notification_emails: ["loines@mozilla.com", "passwords-dev@mozilla.org", "jaws@mozilla.com"]
     release_channel_collection: opt-out
     products:
       - "firefox"
-      - "fennec"
-      - "geckoview"
     record_in_processes: [content]
   autocomplete_field:
     objects: ["generatedpassword"]

toolkit/modules/OSKeyStore.jsm

@@ -93,7 +93,7 @@ var OSKeyStore = {
           "oskeystore-testonly-reauth",
           "pass"
         );
-        break;
+        return { authenticated: true, auth_details: "success" };
       case "cancel":
         Services.obs.notifyObservers(
           null,
@@ -145,9 +145,9 @@ var OSKeyStore = {
    *                                  the key storage. If we start creating keys on macOS by running
    *                                  this code we'll potentially have to do extra work to cleanup
    *                                  the mess later.
-   * @returns {Promise<boolean>}      True if it's logged in or no password is set
-   *                                  and false if it's still not logged in (prompt
-   *                                  canceled or other error).
+   * @returns {Promise<Object>}       Object with the following properties:
+   *                                    authenticated: {boolean} Set to true if the user successfully authenticated.
+   *                                    auth_details: {String?} Details of the authentication result.
    */
   async ensureLoggedIn(
     reauth = false,
@@ -201,17 +201,21 @@ var OSKeyStore = {
                 Cr.NS_ERROR_FAILURE
               );
             }
+            return { authenticated: true, auth_details: "success" };
           });
       } else {
         log.debug("ensureLoggedIn: Skipping reauth on unsupported platforms");
-        unlockPromise = Promise.resolve();
+        unlockPromise = Promise.resolve({
+          authenticated: true,
+          auth_details: "success_unsupported_platform",
+        });
       }
     } else {
-      unlockPromise = Promise.resolve();
+      unlockPromise = Promise.resolve({ authenticated: true });
     }
 
     if (generateKeyIfNotAvailable) {
-      unlockPromise = unlockPromise.then(async () => {
+      unlockPromise = unlockPromise.then(async reauthResult => {
         if (!(await nativeOSKeyStore.asyncSecretAvailable(this.STORE_LABEL))) {
           log.debug(
             "ensureLoggedIn: Secret unavailable, attempt to generate new secret."
@@ -227,23 +231,24 @@ var OSKeyStore = {
               recoveryPhrase.length
           );
         }
+        return reauthResult;
       });
     }
 
     unlockPromise = unlockPromise.then(
-      () => {
+      reauthResult => {
         log.debug("ensureLoggedIn: Logged in");
         this._pendingUnlockPromise = null;
         this._isLocked = false;
 
-        return true;
+        return reauthResult;
       },
       err => {
         log.debug("ensureLoggedIn: Not logged in", err);
         this._pendingUnlockPromise = null;
         this._isLocked = true;
 
-        return false;
+        return { authenticated: false, auth_details: "fail" };
       }
     );
 
@@ -267,7 +272,7 @@ var OSKeyStore = {
    * @returns {Promise<string>}           resolves to the decrypted string, or rejects otherwise.
    */
   async decrypt(cipherText, reauth = false) {
-    if (!(await this.ensureLoggedIn(reauth))) {
+    if (!(await this.ensureLoggedIn(reauth)).authenticated) {
       throw Components.Exception(
         "User canceled OS unlock entry",
         Cr.NS_ERROR_ABORT
@@ -287,7 +292,7 @@ var OSKeyStore = {
    * @returns {Promise<string>} resolves to the encrypted string (with algorithm), otherwise rejects.
    */
   async encrypt(plainText) {
-    if (!(await this.ensureLoggedIn())) {
+    if (!(await this.ensureLoggedIn()).authenticated) {
       throw Components.Exception(
         "User canceled OS unlock entry",
         Cr.NS_ERROR_ABORT

toolkit/modules/tests/xpcshell/test_osKeyStore.js

@@ -34,7 +34,11 @@ const testText = "test string";
 let cipherText;
 
 add_task(async function test_encrypt_decrypt() {
-  Assert.equal(await OSKeyStore.ensureLoggedIn(), true, "Started logged in.");
+  Assert.equal(
+    (await OSKeyStore.ensureLoggedIn()).authenticated,
+    true,
+    "Started logged in."
+  );
 
   cipherText = await OSKeyStore.encrypt(testText);
   Assert.notEqual(testText, cipherText);
@@ -67,7 +71,7 @@ add_task(async function test_reauth() {
   reauthObserved = OSKeyStoreTestUtils.waitForOSKeyStoreLogin(false);
   await new Promise(resolve => TestUtils.executeSoon(resolve));
   Assert.equal(
-    await OSKeyStore.ensureLoggedIn("test message"),
+    (await OSKeyStore.ensureLoggedIn("test message")).authenticated,
     false,
     "Reauth cancelled."
   );
@@ -82,7 +86,7 @@ add_task(async function test_reauth() {
   reauthObserved = OSKeyStoreTestUtils.waitForOSKeyStoreLogin(true);
   await new Promise(resolve => TestUtils.executeSoon(resolve));
   Assert.equal(
-    await OSKeyStore.ensureLoggedIn("test message"),
+    (await OSKeyStore.ensureLoggedIn("test message")).authenticated,
     true,
     "Reauth logged in."
   );