Bug 1639347 - Refactor the OSKeyStore reauthentication into a LoginHelper method. r=MattN

* Move MP and OSKeyStore.ensureLoggedIn checks from AboutLoginParent to new requestReauth method on LoginHelper Differential Revision: https://phabricator.services.mozilla.com/D67681
2020-05-29 07:23:28 +00:00 · 2020-05-29 07:23:28 +00:00 · 6b6e23c680
--- a/browser/components/aboutlogins/AboutLoginsParent.jsm
+++ b/browser/components/aboutlogins/AboutLoginsParent.jsm
@ -323,106 +323,40 @@ class AboutLoginsParent extends JSWindowActorParent {
            "AboutLogins:MasterPasswordRequest: Message ID required for MasterPasswordRequest."
          );
        }
+        let messageText = { value: "NOT SUPPORTED" };
+        let captionText = { value: "" };

-        let loggedIn = false;
-        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;
-          }
-
-          // 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) {
-            let messageText = { value: "NOT SUPPORTED" };
-            let captionText = { value: "" };
-            // This feature is only supported on Windows and macOS
-            // but we still call in to OSKeyStore on Linux to get
-            // the proper auth_details for Telemetry.
-            // See bug 1614874 for Linux support.
-            if (OSKeyStore.canReauth()) {
-              messageId += "-" + AppConstants.platform;
-              [messageText, captionText] = await AboutLoginsL10n.formatMessages(
-                [
-                  {
-                    id: messageId,
-                  },
-                  {
-                    id: "about-logins-os-auth-dialog-caption",
-                  },
-                ]
-              );
-            }
-            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) {
-            loggedIn = false;
-            return;
-          }
-
-          // So there's a master password. But since checkPassword didn't succeed, we're logged out (per nsIPK11Token.idl).
-          try {
-            // Relogin and ask for the master password.
-            token.login(true); // 'true' means always prompt for token password. User will be prompted until
-            // clicking 'Cancel' or entering the correct password.
-          } catch (e) {
-            // An exception will be thrown if the user cancels the login prompt dialog.
-            // 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,
-          });
+        // This feature is only supported on Windows and macOS
+        // but we still call in to OSKeyStore on Linux to get
+        // the proper auth_details for Telemetry.
+        // See bug 1614874 for Linux support.
+        if (OS_AUTH_ENABLED && OSKeyStore.canReauth()) {
+          messageId += "-" + AppConstants.platform;
+          [messageText, captionText] = await AboutLoginsL10n.formatMessages([
+            {
+              id: messageId,
+            },
+            {
+              id: "about-logins-os-auth-dialog-caption",
+            },
+          ]);
        }
+
+        let { isAuthorized, telemetryEvent } = await LoginHelper.requestReauth(
+          this.browsingContext.embedderElement,
+          OS_AUTH_ENABLED,
+          AboutLogins._authExpirationTime,
+          messageText.value,
+          captionText.value
+        );
+        if (isAuthorized) {
+          const AUTH_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
+          AboutLogins._authExpirationTime = Date.now() + AUTH_TIMEOUT_MS;
+        }
+        this.sendAsyncMessage("AboutLogins:MasterPasswordResponse", {
+          result: isAuthorized,
+          telemetryEvent,
+        });
        break;
      }
      case "AboutLogins:Subscribe": {
--- a/browser/components/preferences/tests/browser_masterpassword.js
+++ b/browser/components/preferences/tests/browser_masterpassword.js
@ -1,4 +1,5 @@
 ChromeUtils.import("resource://testing-common/OSKeyStoreTestUtils.jsm", this);
+ChromeUtils.import("resource://gre/modules/OSKeyStore.jsm", this);

 add_task(async function() {
  let prefs = await openPreferencesViaOpenPreferencesAPI("panePrivacy", {
@ -36,7 +37,7 @@ add_task(async function() {
  ok(button.disabled, "master password button should be disabled by default");

  let masterPasswordNextState = false;
-  if (OSKeyStoreTestUtils.canTestOSKeyStoreLogin()) {
+  if (OSKeyStoreTestUtils.canTestOSKeyStoreLogin() && OSKeyStore.canReauth()) {
    let osAuthDialogShown = OSKeyStoreTestUtils.waitForOSKeyStoreLogin(false);
    checkbox.click();
    info("waiting for os auth dialog to appear and get canceled");
@ -58,7 +59,7 @@ add_task(async function() {
  }

  masterPasswordNextState = true;
-  if (OSKeyStoreTestUtils.canTestOSKeyStoreLogin()) {
+  if (OSKeyStoreTestUtils.canTestOSKeyStoreLogin() && OSKeyStore.canReauth()) {
    let osAuthDialogShown = OSKeyStoreTestUtils.waitForOSKeyStoreLogin(true);
    checkbox.click();
    info("waiting for os auth dialog to appear");
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@ -20,6 +20,11 @@ const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { XPCOMUtils } = ChromeUtils.import(
  "resource://gre/modules/XPCOMUtils.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "OSKeyStore",
+  "resource://gre/modules/OSKeyStore.jsm"
+);

 /**
 * Contains functions shared by different Login Manager components.
@ -1147,6 +1152,112 @@ this.LoginHelper = {
    return token.hasPassword;
  },

+  /**
+   * Shows the Master Password prompt if enabled, or the
+   * OS auth dialog otherwise.
+   * @param {Element} browser
+   *        The <browser> that the prompt should be shown on
+   * @param OSReauthEnabled Boolean indicating if OS reauth should be tried
+   * @param expirationTime Optional timestamp indicating next required re-authentication
+   * @param messageText Formatted and localized string to be displayed when the OS auth dialog is used.
+   * @param captionText Formatted and localized string to be displayed when the OS auth dialog is used.
+   */
+  async requestReauth(
+    browser,
+    OSReauthEnabled,
+    expirationTime,
+    messageText,
+    captionText
+  ) {
+    let isAuthorized = false;
+    let telemetryEvent;
+
+    // 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();
+
+    // Do we have a recent authorization?
+    if (expirationTime && Date.now() < expirationTime) {
+      isAuthorized = true;
+      telemetryEvent = {
+        object: token.hasPassword ? "master_password" : "os_auth",
+        method: "reauthenticate",
+        value: "success_no_prompt",
+      };
+      return {
+        isAuthorized,
+        telemetryEvent,
+      };
+    }
+
+    // Default to true if there is no master password and OS reauth is not available
+    if (!token.hasPassword && !OSReauthEnabled) {
+      isAuthorized = true;
+      telemetryEvent = {
+        object: "os_auth",
+        method: "reauthenticate",
+        value: "success_disabled",
+      };
+      return {
+        isAuthorized,
+        telemetryEvent,
+      };
+    }
+    // Use the OS auth dialog if there is no master password
+    if (!token.hasPassword && OSReauthEnabled) {
+      let result = await OSKeyStore.ensureLoggedIn(
+        messageText,
+        captionText,
+        browser.ownerGlobal,
+        false
+      );
+      isAuthorized = result.authenticated;
+      telemetryEvent = {
+        object: "os_auth",
+        method: "reauthenticate",
+        value: result.auth_details,
+      };
+      return {
+        isAuthorized,
+        telemetryEvent,
+      };
+    }
+    // We'll attempt to re-auth via Master Password, force a log-out
+    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) {
+      isAuthorized = false;
+      return {
+        isAuthorized,
+        telemetryEvent,
+      };
+    }
+
+    // So there's a master password. But since checkPassword didn't succeed, we're logged out (per nsIPK11Token.idl).
+    try {
+      // Relogin and ask for the master password.
+      token.login(true); // 'true' means always prompt for token password. User will be prompted until
+      // clicking 'Cancel' or entering the correct password.
+    } catch (e) {
+      // An exception will be thrown if the user cancels the login prompt dialog.
+      // User is also logged out of Software Security Device.
+    }
+    isAuthorized = token.isLoggedIn();
+    telemetryEvent = {
+      object: "master_password",
+      method: "reauthenticate",
+      value: isAuthorized ? "success" : "fail",
+    };
+    return {
+      isAuthorized,
+      telemetryEvent,
+    };
+  },
+
  /**
   * Send a notification when stored data is changed.
   */
--- a/toolkit/modules/OSKeyStore.jsm
+++ b/toolkit/modules/OSKeyStore.jsm
@ -103,7 +103,7 @@ var OSKeyStore = {
  async _reauthInTests() {
    // Skip this reauth because there is no way to mock the
    // native dialog in the testing environment, for now.
-    log.debug("_ensureReauth: _testReauth: ", this._testReauth);
+    log.debug("_reauthInTests: _testReauth: ", this._testReauth);
    switch (this._testReauth) {
      case "pass":
        Services.obs.notifyObservers(
@ -201,9 +201,6 @@ var OSKeyStore = {
      ) {
        unlockPromise = this._reauthInTests();
      } else if (this.canReauth()) {
-        // The OS auth dialog is not supported on macOS < 10.12
-        // (Darwin 16) due to various issues (bug 1622304 and bug 1622303).
-
        // On Windows, this promise rejects when the user cancels login dialog, see bug 1502121.
        // On macOS this resolves to false, so we would need to check it.
        unlockPromise = osReauthenticator
--- a/toolkit/modules/tests/modules/OSKeyStoreTestUtils.jsm
+++ b/toolkit/modules/tests/modules/OSKeyStoreTestUtils.jsm
@ -6,6 +6,9 @@
 var EXPORTED_SYMBOLS = ["OSKeyStoreTestUtils"];

 ChromeUtils.import("resource://gre/modules/OSKeyStore.jsm", this);
+const { AppConstants } = ChromeUtils.import(
+  "resource://gre/modules/AppConstants.jsm"
+);
 ChromeUtils.defineModuleGetter(
  this,
  "UpdateUtils",
@ -16,6 +19,10 @@ const { TestUtils } = ChromeUtils.import(
  "resource://testing-common/TestUtils.jsm"
 );

+// Debug builds will be treated as "official" builds for the purposes of the automated testing for behavior of OSKeyStore.ensureLoggedIn
+// We want to ensure that we catch test failures that would only otherwise show up in official builds
+const isCanaryBuildForOSKeyStore = AppConstants.DEBUG;
+
 var OSKeyStoreTestUtils = {
  TEST_ONLY_REAUTH: "toolkit.osKeyStore.unofficialBuildOnlyLogin",

@ -37,11 +44,12 @@ var OSKeyStoreTestUtils = {
   * Checks whether or not the test can be run by bypassing
   * the OS login dialog. We do not want the user to be able to
   * do so with in official builds.
-   * @returns {boolean} True if the test can be preformed.
+   * @returns {boolean} True if the test can be performed.
   */
  canTestOSKeyStoreLogin() {
    return (
-      UpdateUtils.getUpdateChannel(false) == "default" && OSKeyStore.canReauth()
+      UpdateUtils.getUpdateChannel(false) == "default" &&
+      !isCanaryBuildForOSKeyStore
    );
  },