Add ability to use weak check in the frontend

This CL updates PasswordManagerProxy and adds basic properties to the
PasswordCheckBehavior to provide the ability to use passwords check for
weakness in the frontend. It also updates password_check a bit to
verify that basic logic works.

Bug: 1119752
Change-Id: I95133f63094fafa7ac105ebba3c1df357cb7bf1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421372Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Cr-Commit-Position: refs/heads/master@{#809731}

chrome/app/settings_strings.grdp

@@ -352,6 +352,12 @@
       =1 {{COUNT} compromised password}
       other {{COUNT} compromised passwords}}
   </message>
+  <message name="IDS_SETTINGS_INSECURE_PASSWORDS_COUNT" desc="Number of insecure passwords present in the database">
+    {COUNT, plural,
+      =0 {No security issues found}
+      =1 {Found {COUNT} security issue}
+      other {Found {COUNT} security issues}}
+  </message>
   <message name="IDS_SETTINGS_CHECK_PASSWORDS_AGAIN" desc="Button to start bulk password check manually in passwords check section.">
     Check again
   </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_INSECURE_PASSWORDS_COUNT.png.sha1

+121c305725484c75be7ab0ed6f627abdfc5023ac
\ No newline at end of file

chrome/browser/resources/settings/autofill_page/password_check.html

@@ -7,7 +7,7 @@
         width: 1.6em;
       }
 
-      iron-icon.has-leaks {
+      iron-icon.has-security-issues {
         --iron-icon-fill-color: var(--google-red-600);
         background: radial-gradient(circle 1.1em at 1.1em,
                                     #FCE8E6 100%,
@@ -21,7 +21,7 @@
       }
 
       @media (prefers-color-scheme: dark) {
-        iron-icon.has-leaks {
+        iron-icon.has-security-issues {
           --iron-icon-fill-color: var(--google-red-refresh-300);
           background: radial-gradient(circle 1.1em at 1.1em,
                                     var(--google-grey-900) 100%,
@@ -29,7 +29,7 @@
         }
       }
 
-      iron-icon.no-leaks {
+      iron-icon.no-security-issues {
         --iron-icon-fill-color: var(--google-blue-600);
         background-size: 16px 16px;
       }
@@ -38,7 +38,7 @@
         display: none;
       }
 
-      #leakCheckHeader {
+      #securityCheckHeader {
         border-bottom: var(--cr-separator-line);
       }
 
@@ -47,8 +47,9 @@
       }
 
     </style>
-    <!-- The banner is visible if no compromised password was found (yet). -->
-    <template is="dom-if" if="[[shouldShowBanner_(status, leakedPasswords)]]">
+    <!-- The banner is visible if no insecure password was found (yet). -->
+    <template is="dom-if"
+        if="[[shouldShowBanner_(status, leakedPasswords, weakPasswords)]]">
       <picture>
         <source srcset="[[bannerImageSrc_(1, status)]]"
                 media="(prefers-color-scheme: dark)">
@@ -57,11 +58,11 @@
     </template>
 
     <!-- The header showing progress or result of the check-->
-    <div class="cr-row first two-line" id="leakCheckHeader">
+    <div class="cr-row first two-line" id="securityCheckHeader">
       <!-- If the password check concluded, show only a status Icon. -->
       <template is="dom-if" if="[[!isCheckInProgress_(status)]]">
-        <iron-icon class$="[[getStatusIconClass_(status, leakedPasswords)]]"
-                   icon="[[getStatusIcon_(status, leakedPasswords)]]">
+        <iron-icon class$="[[getStatusIconClass_(status, leakedPasswords, weakPasswords)]]"
+                   icon="[[getStatusIcon_(status, leakedPasswords, weakPasswords)]]">
         </iron-icon>
       </template>
 
@@ -80,8 +81,8 @@
           </span>
         </div>
         <div class="secondary" id="subtitle"
-            hidden$="[[!showsPasswordsCount_(status, leakedPasswords)]]">
-          [[compromisedPasswordsCount]]
+            hidden$="[[!showsPasswordsCount_(status, leakedPasswords, weakPasswords)]]">
+          [[getPasswordsCount_(insecurePasswordsCount, compromisedPasswordsCount)]]
         </div>
       </div>
       <cr-button id="controlPasswordCheckButton"

chrome/browser/resources/settings/autofill_page/password_check.js

@@ -274,6 +274,24 @@ Polymer({
     return !!this.leakedPasswords.length;
   },
 
+  /**
+   * Returns true if there are any weak credentials.
+   * @return {boolean}
+   * @private
+   */
+  hasWeakCredentials_() {
+    return !!this.weakPasswords.length;
+  },
+
+  /**
+   * Returns true if there are any insecure credentials.
+   * @return {boolean}
+   * @private
+   */
+  hasInsecureCredentials_() {
+    return !!this.leakedPasswords.length || !!this.weakPasswords.length;
+  },
+
   /**
    * @param {!CustomEvent<{moreActionsButton: !HTMLElement}>} event
    * @private
@@ -374,10 +392,10 @@ Polymer({
    * @private
    */
   getStatusIcon_() {
-    if (!this.hasLeaksOrErrors_()) {
+    if (!this.hasInsecureCredentialsOrErrors_()) {
       return 'settings:check-circle';
     }
-    if (this.hasLeakedCredentials_()) {
+    if (this.hasInsecureCredentials_()) {
       return 'cr:warning';
     }
     return 'cr:info';
@@ -389,11 +407,11 @@ Polymer({
    * @private
    */
   getStatusIconClass_() {
-    if (!this.hasLeaksOrErrors_()) {
-      return this.waitsForFirstCheck_() ? 'hidden' : 'no-leaks';
+    if (!this.hasInsecureCredentialsOrErrors_()) {
+      return this.waitsForFirstCheck_() ? 'hidden' : 'no-security-issues';
     }
-    if (this.hasLeakedCredentials_()) {
-      return 'has-leaks';
+    if (this.hasInsecureCredentials_()) {
+      return 'has-security-issues';
     }
     return '';
   },
@@ -537,21 +555,21 @@ Polymer({
    * @private
    */
   shouldShowBanner_() {
-    if (this.hasLeakedCredentials_()) {
+    if (this.hasInsecureCredentials_()) {
       return false;
     }
     return this.status.state === CheckState.CANCELED ||
-        !this.hasLeaksOrErrors_();
+        !this.hasInsecureCredentialsOrErrors_();
   },
 
   /**
-   * Returns true if there are leaked credentials or the status is unexpected
+   * Returns true if there are insecure credentials or the status is unexpected
    * for a regular password check.
    * @return {boolean}
    * @private
    */
-  hasLeaksOrErrors_() {
-    if (this.hasLeakedCredentials_()) {
+  hasInsecureCredentialsOrErrors_() {
+    if (this.hasInsecureCredentials_()) {
       return true;
     }
     switch (this.status.state) {
@@ -571,13 +589,13 @@ Polymer({
   },
 
   /**
-   * Returns true if there are leaked credentials or the status is unexpected
+   * Returns true if there are insecure credentials or the status is unexpected
    * for a regular password check.
    * @return {boolean}
    * @private
    */
   showsPasswordsCount_() {
-    if (this.hasLeakedCredentials_()) {
+    if (this.hasInsecureCredentials_()) {
       return true;
     }
     switch (this.status.state) {
@@ -597,6 +615,20 @@ Polymer({
         this.status.state);
   },
 
+  /**
+   * Returns count of insecure credentials, if |passwordsWeaknessCheckEnabled|
+   * is true, otherwise, returns count of compromised credentials.
+   * @return {string}
+   * @private
+   */
+  getPasswordsCount_() {
+    if (this.passwordsWeaknessCheckEnabled) {
+      return this.insecurePasswordsCount;
+    } else {
+      return this.compromisedPasswordsCount;
+    }
+  },
+
   /**
    * Returns true iff the leak check was performed at least once before.
    * @return {boolean}

chrome/browser/resources/settings/autofill_page/password_check_behavior.js

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import {assert} from 'chrome://resources/js/assert.m.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
 
 import {PasswordManagerImpl, PasswordManagerProxy} from './password_manager_proxy.js';
@@ -23,6 +24,11 @@ export const PasswordCheckBehavior = {
      */
     compromisedPasswordsCount: String,
 
+    /**
+     * The number of insecure passwords as a formatted string.
+     */
+    insecurePasswordsCount: String,
+
     /**
      * An array of leaked passwords to display.
      * @type {!Array<!PasswordManagerProxy.InsecureCredential>}
@@ -32,6 +38,15 @@ export const PasswordCheckBehavior = {
       value: () => [],
     },
 
+    /**
+     * An array of weak passwords to display.
+     * @type {!Array<!PasswordManagerProxy.InsecureCredential>}
+     */
+    weakPasswords: {
+      type: Array,
+      value: () => [],
+    },
+
     /**
      * The status indicates progress and affects banner, title and icon.
      * @type {!PasswordManagerProxy.PasswordCheckStatus}
@@ -49,6 +64,17 @@ export const PasswordCheckBehavior = {
       type: Boolean,
       value: true,
     },
+
+    /**
+     * Returns true if passwords weakness check is enabled.
+     * @type {boolean}
+     */
+    passwordsWeaknessCheckEnabled: {
+      type: Boolean,
+      value() {
+        return loadTimeData.getBoolean('passwordsWeaknessCheck');
+      }
+    },
   },
 
   /**
@@ -56,6 +82,11 @@ export const PasswordCheckBehavior = {
    */
   leakedCredentialsListener_: null,
 
+  /**
+   * @private {?function(!PasswordManagerProxy.InsecureCredentials):void}
+   */
+  weakCredentialsListener_: null,
+
   /**
    * @private {?function(!PasswordManagerProxy.PasswordCheckStatus):void}
    */
@@ -78,6 +109,25 @@ export const PasswordCheckBehavior = {
           .then(count => {
             this.compromisedPasswordsCount = count;
           });
+      PluralStringProxyImpl.getInstance()
+          .getPluralString(
+              'insecurePasswords',
+              this.leakedPasswords.length + this.weakPasswords.length)
+          .then(count => {
+            this.insecurePasswordsCount = count;
+          });
+    };
+
+    this.weakCredentialsListener_ = weakCredentials => {
+      this.weakPasswords = weakCredentials;
+
+      PluralStringProxyImpl.getInstance()
+          .getPluralString(
+              'insecurePasswords',
+              this.leakedPasswords.length + this.weakPasswords.length)
+          .then(count => {
+            this.insecurePasswordsCount = count;
+          });
     };
 
     this.passwordManager = PasswordManagerImpl.getInstance();
@@ -85,11 +135,15 @@ export const PasswordCheckBehavior = {
         this.statusChangedListener_);
     this.passwordManager.getCompromisedCredentials().then(
         this.leakedCredentialsListener_);
+    this.passwordManager.getWeakCredentials().then(
+        this.weakCredentialsListener_);
 
     this.passwordManager.addPasswordCheckStatusListener(
         this.statusChangedListener_);
     this.passwordManager.addCompromisedCredentialsListener(
         this.leakedCredentialsListener_);
+    this.passwordManager.addWeakCredentialsListener(
+        this.weakCredentialsListener_);
   },
 
   /** @override */
@@ -100,6 +154,9 @@ export const PasswordCheckBehavior = {
     this.passwordManager.removeCompromisedCredentialsListener(
         assert(this.leakedCredentialsListener_));
     this.leakedCredentialsListener_ = null;
+    this.passwordManager.removeWeakCredentialsListener(
+        assert(this.weakCredentialsListener_));
+    this.weakCredentialsListener_ = null;
   },
 
   /**

chrome/browser/resources/settings/autofill_page/password_manager_proxy.js

@@ -203,6 +203,12 @@ export class PasswordManagerProxy {
    */
   getCompromisedCredentials() {}
 
+  /**
+   * Requests the latest information about weak credentials.
+   * @return {!Promise<(PasswordManagerProxy.InsecureCredentials)>}
+   */
+  getWeakCredentials() {}
+
   /**
    * Returns the current status of the check via |callback|.
    * @return {!Promise<(PasswordManagerProxy.PasswordCheckStatus)>}
@@ -229,6 +235,20 @@ export class PasswordManagerProxy {
    */
   removeCompromisedCredentialsListener(listener) {}
 
+  /**
+   * Add an observer to the weak passwords change.
+   * @param {function(!PasswordManagerProxy.InsecureCredentials):void}
+   *      listener
+   */
+  addWeakCredentialsListener(listener) {}
+
+  /**
+   * Remove an observer to the weak passwords change.
+   * @param {function(!PasswordManagerProxy.InsecureCredentials):void}
+   *     listener
+   */
+  removeWeakCredentialsListener(listener) {}
+
   /**
    * Add an observer to the passwords check status change.
    * @param {function(!PasswordManagerProxy.PasswordCheckStatus):void} listener
@@ -528,6 +548,13 @@ export class PasswordManagerImpl {
     });
   }
 
+  /** @override */
+  getWeakCredentials() {
+    return new Promise(resolve => {
+      chrome.passwordsPrivate.getWeakCredentials(resolve);
+    });
+  }
+
   /** @override */
   removeInsecureCredential(insecureCredential) {
     chrome.passwordsPrivate.removeInsecureCredential(insecureCredential);
@@ -545,6 +572,16 @@ export class PasswordManagerImpl {
         listener);
   }
 
+  /** @override */
+  addWeakCredentialsListener(listener) {
+    chrome.passwordsPrivate.onWeakCredentialsChanged.addListener(listener);
+  }
+
+  /** @override */
+  removeWeakCredentialsListener(listener) {
+    chrome.passwordsPrivate.onWeakCredentialsChanged.removeListener(listener);
+  }
+
   /** @override */
   addPasswordCheckStatusListener(listener) {
     chrome.passwordsPrivate.onPasswordCheckStatusChanged.addListener(listener);

chrome/browser/ui/webui/settings/settings_ui.cc

@@ -320,6 +320,8 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
   auto plural_string_handler = std::make_unique<PluralStringHandler>();
   plural_string_handler->AddLocalizedString(
       "compromisedPasswords", IDS_SETTINGS_COMPROMISED_PASSWORDS_COUNT);
+  plural_string_handler->AddLocalizedString(
+      "insecurePasswords", IDS_SETTINGS_INSECURE_PASSWORDS_COUNT);
   web_ui->AddMessageHandler(std::move(plural_string_handler));
 
   // Add the metrics handler to write uma stats.

chrome/test/data/webui/settings/password_check_test.js

@@ -10,9 +10,10 @@ import 'chrome://settings/lazy_load.js';
 import {isChromeOS} from 'chrome://resources/js/cr.m.js';
 import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
+import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {OpenWindowProxyImpl, PasswordManagerImpl, PasswordManagerProxy, Router, routes} from 'chrome://settings/settings.js';
-import {makeCompromisedCredential,  makePasswordCheckStatus} from 'chrome://test/settings/passwords_and_autofill_fake_data.js';
+import {makeCompromisedCredential, makeInsecureCredential, makePasswordCheckStatus} from 'chrome://test/settings/passwords_and_autofill_fake_data.js';
 import {getSyncAllPrefs,simulateSyncStatus} from 'chrome://test/settings/sync_test_util.m.js';
 import {TestOpenWindowProxy} from 'chrome://test/settings/test_open_window_proxy.js';
 import {TestPasswordManagerProxy} from 'chrome://test/settings/test_password_manager_proxy.js';
@@ -493,8 +494,8 @@ suite('PasswordsCheckSection', function() {
     const spinner = checkPasswordSection.$$('paper-spinner-lite');
     expectFalse(isElementVisible(spinner));
     assertTrue(isElementVisible(icon));
-    expectFalse(icon.classList.contains('has-leaks'));
-    expectTrue(icon.classList.contains('no-leaks'));
+    expectFalse(icon.classList.contains('has-security-issues'));
+    expectTrue(icon.classList.contains('no-security-issues'));
   });
 
   // Tests that there is neither spinner nor icon if the check hasn't run yet.
@@ -527,8 +528,8 @@ suite('PasswordsCheckSection', function() {
     const spinner = checkPasswordSection.$$('paper-spinner-lite');
     expectFalse(isElementVisible(spinner));
     assertTrue(isElementVisible(icon));
-    expectTrue(icon.classList.contains('has-leaks'));
-    expectFalse(icon.classList.contains('no-leaks'));
+    expectTrue(icon.classList.contains('has-security-issues'));
+    expectFalse(icon.classList.contains('no-security-issues'));
   });
 
   // Tests that the spinner is replaced with a warning on errors.
@@ -545,8 +546,8 @@ suite('PasswordsCheckSection', function() {
     const spinner = checkPasswordSection.$$('paper-spinner-lite');
     expectFalse(isElementVisible(spinner));
     assertTrue(isElementVisible(icon));
-    expectFalse(icon.classList.contains('has-leaks'));
-    expectFalse(icon.classList.contains('no-leaks'));
+    expectFalse(icon.classList.contains('has-security-issues'));
+    expectFalse(icon.classList.contains('no-security-issues'));
   });
 
   // Tests that the spinner replaces any icon while the check is running.
@@ -619,6 +620,54 @@ suite('PasswordsCheckSection', function() {
     expectEquals(section.i18n('checkPasswordsProgress', 2, 5), title.innerText);
   });
 
+  // If passwords weakness check is enabled, shows count of insecure
+  // credentials.
+  test('showInsecurePasswordsCount', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    const data = passwordManager.data;
+    data.leakedCredentials = [
+      makeCompromisedCredential('one.com', 'test4', 'LEAKED'),
+    ];
+    data.weakCredentials = [
+      makeInsecureCredential('one.com', 'test4'),
+    ];
+
+    const section = createCheckPasswordSection();
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+    const subtitle = section.$.subtitle;
+    assertTrue(isElementVisible(subtitle));
+
+    return PluralStringProxyImpl.getInstance()
+        .getPluralString('insecurePasswords', 2)
+        .then(count => {
+          expectEquals(count, subtitle.textContent.trim());
+        });
+  });
+
+  // If passwords weakness check is disabled, shows count of compromised
+  // credentials.
+  test('showCompromisedPasswordsCount', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: false});
+    const data = passwordManager.data;
+    data.leakedCredentials = [
+      makeCompromisedCredential('one.com', 'test4', 'LEAKED'),
+    ];
+
+    const section = createCheckPasswordSection();
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    const subtitle = section.$.subtitle;
+    assertTrue(isElementVisible(subtitle));
+
+    return PluralStringProxyImpl.getInstance()
+        .getPluralString('compromisedPasswords', 1)
+        .then(count => {
+          expectEquals(count, subtitle.textContent.trim());
+        });
+  });
+
   // When canceled, show string explaining that and already found leak
   // count.
   test('showProgressAndLeaksAfterCanceled', async function() {

chrome/test/data/webui/settings/passwords_and_autofill_fake_data.js

@@ -244,33 +244,46 @@ export function createCreditCardEntry() {
 }
 
 /**
- * Creates a new compromised credential.
+ * Creates a new insecure credential.
  * @param {string} url
  * @param {string} username
- * @param {chrome.passwordsPrivate.CompromiseType} type
  * @param {number=} id
- * @param {number=} elapsedMinSinceCompromise
  * @return {chrome.passwordsPrivate.InsecureCredential}
  * @private
  */
-export function makeCompromisedCredential(
-    url, username, type, id, elapsedMinSinceCompromise) {
+export function makeInsecureCredential(url, username, id) {
   return {
     id: id || 0,
     formattedOrigin: url,
     changePasswordUrl: `http://${url}/`,
     username: username,
-    compromisedInfo: {
-      compromiseTime: Date.now() - (elapsedMinSinceCompromise * 60000),
-      elapsedTimeSinceCompromise: `${elapsedMinSinceCompromise} minutes ago`,
-      compromiseType: type,
-    },
     detailedOrigin: '',
     isAndroidCredential: false,
     signonRealm: '',
   };
 }
 
+/**
+ * Creates a new compromised credential.
+ * @param {string} url
+ * @param {string} username
+ * @param {chrome.passwordsPrivate.CompromiseType} type
+ * @param {number=} id
+ * @param {number=} elapsedMinSinceCompromise
+ * @return {chrome.passwordsPrivate.InsecureCredential}
+ * @private
+ */
+export function makeCompromisedCredential(
+    url, username, type, id, elapsedMinSinceCompromise) {
+  const credential = makeInsecureCredential(url, username, id);
+  credential.compromisedInfo = {
+    compromiseTime: Date.now() - (elapsedMinSinceCompromise * 60000),
+    elapsedTimeSinceCompromise: `${elapsedMinSinceCompromise} minutes ago`,
+    compromiseType: type,
+  };
+  return credential;
+}
+
 /**
  * Creates a new password check status.
  * @param {!chrome.passwordsPrivate.PasswordCheckState=} state

chrome/test/data/webui/settings/test_password_manager_proxy.js

@@ -47,6 +47,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       'startBulkPasswordCheck',
       'stopBulkPasswordCheck',
       'getCompromisedCredentials',
+      'getWeakCredentials',
       'getPasswordCheckStatus',
       'getPlaintextInsecurePassword',
       'changeInsecureCredential',
@@ -70,6 +71,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       passwords: [],
       exceptions: [],
       leakedCredentials: [],
+      weakCredentials: [],
       checkStatus: makePasswordCheckStatus(),
     };
 
@@ -80,6 +82,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       addExceptionListChangedListener: null,
       requestPlaintextPassword: null,
       addCompromisedCredentialsListener: null,
+      addWeakCredentialsListener: null,
       addAccountStorageOptInStateListener: null,
     };
 
@@ -241,6 +244,12 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
     return Promise.resolve(this.data.leakedCredentials);
   }
 
+  /** @override */
+  getWeakCredentials() {
+    this.methodCalled('getWeakCredentials');
+    return Promise.resolve(this.data.weakCredentials);
+  }
+
   /** @override */
   getPasswordCheckStatus() {
     this.methodCalled('getPasswordCheckStatus');
@@ -255,6 +264,14 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
   /** @override */
   removeCompromisedCredentialsListener(listener) {}
 
+  /** @override */
+  addWeakCredentialsListener(listener) {
+    this.lastCallback.addWeakCredentialsListener = listener;
+  }
+
+  /** @override */
+  removeWeakCredentialsListener(listener) {}
+
   /** @override */
   addPasswordCheckStatusListener(listener) {
     this.lastCallback.addPasswordCheckStatusListener = listener;