Add weak passwords section to Password Check page

This CL updates the UI in the chrome://settings/passswords/check and
adds to it a new weak password section.
Screenshot of Password Check page before and after changes:
https://bugs.chromium.org/p/chromium/issues/detail?id=1119752#c15

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

chrome/app/settings_strings.grdp

@@ -340,7 +340,7 @@
   <message name="IDS_SETTINGS_CHECK_PASSWORDS_CANCELED" desc="Message for when the password check was canceled by the user.">
     Canceled
   </message>
-  <message name="IDS_SETTINGS_CHECKED_PASSWORDS" desc="Title above amount of found compromised passwords after password bulk check.">
+  <message name="IDS_SETTINGS_CHECKED_PASSWORDS" desc="Title above amount of found insecure passwords after password bulk check.">
     Checked passwords
   </message>
   <message name="IDS_SETTINGS_CHECK_PASSWORDS_DESCRIPTION" desc="Explanation of the passwords bulk check feature found within the password settings.">
@@ -379,6 +379,13 @@
   <message name="IDS_SETTINGS_COMPROMISED_PASSWORDS_ADVICE" desc="Description of what user should do after compromised passwords are found.">
     Change these passwords immediately to keep your account safe:
   </message>
+  <message name="IDS_SETTINGS_WEAK_PASSWORDS" desc="Title for list of weak credentials after passwords bulk check.">
+    Weak passwords
+  </message>
+  <message name="IDS_SETTINGS_WEAK_PASSWORDS_DESCRIPTION" desc="Information about why weak passwords are a security issue and what user should do to fix the problem. Has a link.">
+    Weak passwords are easy to guess. Make sure you're creating strong passwords.
+    <ph name="BEGIN_LINK">&lt;br/&gt;&lt;a target='_blank' href='$1'&gt;</ph>See more security tips.<ph name="END_LINK">&lt;/a&gt;</ph>
+  </message>
   <message name="IDS_SETTINGS_CHANGE_PASSWORD_BUTTON" desc="Button inside password check section which opens url for changing leaked password.">
     Change password
   </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_WEAK_PASSWORDS.png.sha1

+428225b74a1b2102df1b1d6b7cf6270ca3803e00
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_WEAK_PASSWORDS_DESCRIPTION.png.sha1

+428225b74a1b2102df1b1d6b7cf6270ca3803e00
\ No newline at end of file

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

@@ -46,6 +46,11 @@
         width: 100%;
       }
 
+      #weakPasswordsDescription {
+        display: block;
+        text-align: left;
+      }
+
     </style>
     <!-- The banner is visible if no insecure password was found (yet). -->
     <template is="dom-if"
@@ -99,7 +104,7 @@
         $i18n{noCompromisedCredentials}
       </div>
     </div>
-    <div id="passwordCheckBody"
+    <div id="compromisedCredentialsBody"
         hidden$="[[!hasLeakedCredentials_(leakedPasswords)]]">
       <div class="cr-row first">
         <h2>$i18n{compromisedPasswords}</h2>
@@ -119,6 +124,35 @@
                 on-change-password-clicked="onChangePasswordClick_"
 <if expr="chromeos">
                 token-request-manager="[[tokenRequestManager_]]"
+</if>
+                on-already-changed-password-click="onAlreadyChangedClick_">
+            </password-check-list-item>
+          </template>
+        </iron-list>
+      </div>
+    </div>
+    <!-- TODO(crbug.com/1119752): |weakCredentialsBody| is almost a copy-paste
+    of |compromisedCredentialsBody|. Clean it up by creating a polymer element
+    for this. -->
+    <div id="weakCredentialsBody" hidden$="[[!hasWeakCredentials_(weakPasswords)]]">
+      <div class="cr-row first">
+        <h2>$i18n{weakPasswords}</h2>
+      </div>
+      <div class="list-frame vertical-list">
+        <div class="list-item secondary" id="weakPasswordsDescription">
+          $i18nRaw{weakPasswordsDescription}
+        </div>
+      </div>
+      <div class="list-frame first">
+        <iron-list id="weakPasswordList" items="[[weakPasswords]]">
+          <template>
+            <password-check-list-item item="[[item]]"
+                on-more-actions-click="onMoreActionsClick_"
+                clicked-change-password=
+                "[[clickedChangePassword_(item, clickedChangePasswordIds_.size)]]"
+                on-change-password-clicked="onChangePasswordClick_"
+<if expr="chromeos">
+                token-request-manager="[[tokenRequestManager_]]"
 </if>
                 on-already-changed-password-click="onAlreadyChangedClick_">
             </password-check-list-item>

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

@@ -280,7 +280,7 @@ Polymer({
    * @private
    */
   hasWeakCredentials_() {
-    return !!this.weakPasswords.length;
+    return this.passwordsWeaknessCheckEnabled && !!this.weakPasswords.length;
   },
 
   /**
@@ -289,7 +289,7 @@ Polymer({
    * @private
    */
   hasInsecureCredentials_() {
-    return !!this.leakedPasswords.length || !!this.weakPasswords.length;
+    return !!this.leakedPasswords.length || this.hasWeakCredentials_();
   },
 
   /**

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

@@ -6,25 +6,25 @@
         --iron-icon-fill-color: var(--text-color-action);
       }
 
-      #leakedPassword {
+      #insecurePassword {
         background-color: transparent;
         border: none;
         font-size: inherit;
         margin-inline-start: 4px;
       }
 
-      #leaked-item {
+      #insecure-item {
         margin-bottom: 12px;
         margin-top: 12px;
       }
 
-      #leaked-info {
+      #insecure-info {
         display: flex;
         flex: 2;
         width: 0;
       }
 
-      #leakUsername {
+      #insecureUsername {
         align-items: baseline;
         display: flex;
         flex: 1;
@@ -59,43 +59,46 @@
         flex-direction: column;
       }
 
-      #leakOrigin {
+      #insecureOrigin {
         direction: rtl;
         display: flex;
         justify-content: flex-end;
       }
 
-      :host-context([dir='rtl']) #leakOrigin {
+      :host-context([dir='rtl']) #insecureOrigin {
         justify-content: flex-start;
       }
     </style>
-    <div class="list-item" id="leaked-item" focus-row-container>
+    <div class="list-item" id="insecure-item" focus-row-container>
       <site-favicon url="[[item.changePasswordUrl]]"></site-favicon>
-      <div id="leaked-info">
+      <div id="insecure-info">
         <div id="info-column" class="no-min-width">
-          <div id="leakOrigin" class="no-min-width">
+          <div id="insecureOrigin" class="no-min-width">
             <span class="text-elide">
               <!-- This bdo tag is necessary to fix the display of domains
                 starting with numbers. -->
               <bdo dir="ltr">[[item.formattedOrigin]]</bdo>
             </span>
           </div>
-          <div class="no-min-width" id="leakUsername" >
+          <div class="no-min-width" id="insecureUsername" >
             <span class="no-min-width text-elide secondary">
               [[item.username]]
             </span>
-            <input class="no-min-width secondary text-elide" id="leakedPassword"
-                focus-row-control focus-type="passwordField" readonly
+            <input class="no-min-width secondary text-elide"
+                id="insecurePassword" focus-row-control
+                focus-type="passwordField" readonly
                 type="[[getInputType_(isPasswordVisible_)]]"
                 value="[[password_]]" on-click="onReadonlyInputTap_"
                 disabled$="[[!isPasswordVisible_]]">
           </div>
-          <div class="secondary" id="leakType">
-            [[getCompromiseType_(item)]]
-          </div>
-          <div class="secondary" id="elapsedTime">
-            [[item.compromisedInfo.elapsedTimeSinceCompromise]]
-          </div>
+          <template is="dom-if" if="[[isCompromisedItem_(item)]]">
+            <div class="secondary" id="leakType">
+              [[getCompromiseType_(item)]]
+            </div>
+            <div class="secondary" id="elapsedTime">
+              [[item.compromisedInfo.elapsedTimeSinceCompromise]]
+            </div>
+          </template>
         </div>
       </div>
       <template is="dom-if" if="[[item.changePasswordUrl]]">

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

@@ -73,6 +73,15 @@ Polymer({
     this.passwordManager_ = PasswordManagerImpl.getInstance();
   },
 
+  /**
+   * Returns true if |item| is compromised credential, otherwise returns false.
+   * @return {boolean}
+   * @private
+   */
+  isCompromisedItem_() {
+    return !!this.item.compromisedInfo;
+  },
+
   /**
    * @return {string}
    * @private

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

@@ -831,6 +831,7 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
       {"compromisedPasswords", IDS_SETTINGS_COMPROMISED_PASSWORDS},
       {"compromisedPasswordsDescription",
        IDS_SETTINGS_COMPROMISED_PASSWORDS_ADVICE},
+      {"weakPasswords", IDS_SETTINGS_WEAK_PASSWORDS},
       {"changePasswordButton", IDS_SETTINGS_CHANGE_PASSWORD_BUTTON},
       {"changePasswordInApp", IDS_SETTINGS_CHANGE_PASSWORD_IN_APP_LABEL},
       {"leakedPassword", IDS_SETTINGS_COMPROMISED_PASSWORD_REASON_LEAKED},
@@ -1036,6 +1037,11 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
                          autofill::payments::GetManageInstrumentsUrl().spec());
   html_source->AddString("paymentMethodsLearnMoreURL",
                          chrome::kPaymentMethodsLearnMoreURL);
+  html_source->AddString(
+      "weakPasswordsDescription",
+      l10n_util::GetStringFUTF16(
+          IDS_SETTINGS_WEAK_PASSWORDS_DESCRIPTION,
+          base::ASCIIToUTF16(chrome::kSeeMoreSecurityTipsURL)));
   // The warning message that will be shown if there is a content setting
   // pattern with a wildcard in it. The check for wildcards is done on the js
   // side.

chrome/common/url_constants.cc

@@ -246,6 +246,9 @@ const char kSafeBrowsingHelpCenterURL[] =
 const char kSafetyTipHelpCenterURL[] =
     "https://support.google.com/chrome/?p=safety_tip";
 
+const char kSeeMoreSecurityTipsURL[] =
+    "https://support.google.com/accounts/answer/32040";
+
 const char kSettingsSearchHelpURL[] =
     "https://support.google.com/chrome/?p=settings_search_help";
 

chrome/common/url_constants.h

@@ -215,6 +215,10 @@ extern const char kSafeBrowsingHelpCenterURL[];
 // "Learn more" URL for safety tip bubble.
 extern const char kSafetyTipHelpCenterURL[];
 
+// The URL for the "See more security tips" with advices how to create a strong
+// password.
+extern const char kSeeMoreSecurityTipsURL[];
+
 // Help URL for the settings page's search feature.
 extern const char kSettingsSearchHelpURL[];
 

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

@@ -89,36 +89,79 @@ function shuffleArray(array) {
 }
 
 /**
- * Helper method that validates a that elements in the compromised credentials
- * list match the expected data.
+ * Helper method to convert |CompromiseType| to string.
+ * @param {!chrome.passwordsPrivate.CompromiseType} compromiseType
+ * @return {string}
+ * @private
+ */
+function getCompromiseType(compromiseType) {
+  switch (compromiseType) {
+    case chrome.passwordsPrivate.CompromiseType.PHISHED:
+      return loadTimeData.getString('phishedPassword');
+    case chrome.passwordsPrivate.CompromiseType.LEAKED:
+      return loadTimeData.getString('leakedPassword');
+    case chrome.passwordsPrivate.CompromiseType.PHISHED_AND_LEAKED:
+      return loadTimeData.getString('phishedAndLeakedPassword');
+  }
+}
+
+/**
+ * Helper method that validates a that elements in the insecure credentials list
+ * match the expected data.
  * @param {!Element} checkPasswordSection The section element that will be
  *     checked.
- * @param {!Array<!chrome.passwordsPrivate.InsecureCredential>}
- * passwordList The expected data.
+ * @param {!Array<!chrome.passwordsPrivate.InsecureCredential>} passwordList The
+ *     expected data.
+ * @param {boolean} isCompromised If true, check compromised info for each
+ *     insecure credential.
  * @private
  */
-function validateLeakedPasswordsList(
-    checkPasswordSection, compromisedCredentials) {
-  const listElements = checkPasswordSection.$.leakedPasswordList;
-  assertEquals(listElements.items.length, compromisedCredentials.length);
+function validateInsecurePasswordsList(
+    checkPasswordSection, insecureCredentials, isCompromised) {
+  const listElements = isCompromised ?
+      checkPasswordSection.$.leakedPasswordList :
+      checkPasswordSection.$.weakPasswordList;
+  assertEquals(listElements.items.length, insecureCredentials.length);
   const nodes = checkPasswordSection.shadowRoot.querySelectorAll(
       'password-check-list-item');
-  for (let index = 0; index < compromisedCredentials.length; ++index) {
+  for (let index = 0; index < insecureCredentials.length; ++index) {
     const node = nodes[index];
     assertTrue(!!node);
     assertEquals(
-        node.$.elapsedTime.textContent.trim(),
-        compromisedCredentials[index]
-            .compromisedInfo.elapsedTimeSinceCompromise);
-    assertEquals(
-        node.$.leakUsername.textContent.trim(),
-        compromisedCredentials[index].username);
+        node.$.insecureUsername.textContent.trim(),
+        insecureCredentials[index].username);
     assertEquals(
-        node.$.leakOrigin.textContent.trim(),
-        compromisedCredentials[index].formattedOrigin);
+        node.$.insecureOrigin.textContent.trim(),
+        insecureCredentials[index].formattedOrigin);
+
+    if (isCompromised) {
+      assertEquals(
+          node.shadowRoot.querySelector('#elapsedTime').textContent.trim(),
+          insecureCredentials[index]
+              .compromisedInfo.elapsedTimeSinceCompromise);
+      assertEquals(
+          node.shadowRoot.querySelector('#leakType').textContent.trim(),
+          getCompromiseType(
+              insecureCredentials[index].compromisedInfo.compromiseType));
+    }
   }
 }
 
+/**
+ * Helper method that validates a that elements in the compromised credentials
+ * list match the expected data.
+ * @param {!Element} checkPasswordSection The section element that will be
+ *     checked.
+ * @param {!Array<!chrome.passwordsPrivate.InsecureCredential>} passwordList The
+ *     expected data.
+ * @private
+ */
+function validateLeakedPasswordsList(
+    checkPasswordSection, compromisedCredentials) {
+  validateInsecurePasswordsList(
+      checkPasswordSection, compromisedCredentials, true);
+}
+
 suite('PasswordsCheckSection', function() {
   /** @type {TestPasswordManagerProxy} */
   let passwordManager = null;
@@ -349,7 +392,7 @@ suite('PasswordsCheckSection', function() {
 
     await passwordManager.whenCalled('getPasswordCheckStatus');
     flush();
-    assertFalse(isElementVisible(section.$.passwordCheckBody));
+    assertFalse(isElementVisible(section.$.compromisedCredentialsBody));
     assertTrue(isElementVisible(section.$.noCompromisedCredentials));
   });
 
@@ -363,7 +406,7 @@ suite('PasswordsCheckSection', function() {
     const checkPasswordSection = createCheckPasswordSection();
     await passwordManager.whenCalled('getCompromisedCredentials');
     flush();
-    assertFalse(checkPasswordSection.$.passwordCheckBody.hidden);
+    assertFalse(checkPasswordSection.$.compromisedCredentialsBody.hidden);
     assertTrue(checkPasswordSection.$.noCompromisedCredentials.hidden);
     validateLeakedPasswordsList(checkPasswordSection, leakedPasswords);
   });
@@ -407,7 +450,7 @@ suite('PasswordsCheckSection', function() {
 
     await passwordManager.whenCalled('getCompromisedCredentials');
     flush();
-    assertFalse(checkPasswordSection.$.passwordCheckBody.hidden);
+    assertFalse(checkPasswordSection.$.compromisedCredentialsBody.hidden);
     const listElement = checkPasswordSection.$$('password-check-list-item');
     const menu = checkPasswordSection.$.moreActionsMenu;
 
@@ -668,6 +711,48 @@ suite('PasswordsCheckSection', function() {
         });
   });
 
+  // Verify that weak passwords section is shown, if |passwordsWeaknessCheck|
+  // flag is enabled.
+  test('showWeakPasswords', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    const weakCredentials = [
+      makeInsecureCredential('one.com', 'test1'),
+      makeInsecureCredential('two.com', 'test2'),
+    ];
+    passwordManager.data.weakCredentials = weakCredentials;
+
+    const section = createCheckPasswordSection();
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    assertTrue(isElementVisible(section.$.weakCredentialsBody));
+    validateInsecurePasswordsList(section, weakCredentials, false);
+  });
+
+  // Verify that weak passwords section is hidden, if no weak credentials were
+  // found.
+  test('noWeakPasswords', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    const section = createCheckPasswordSection();
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    assertFalse(isElementVisible(section.$.weakCredentialsBody));
+  });
+
+  // Verify that weak passwords section is hidden, if |passwordsWeaknessCheck|
+  // flag is disabled.
+  test('hideWeakPasswords', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: false});
+    passwordManager.data.weakCredentials =
+        [makeInsecureCredential('one.com', 'test1')];
+    const section = createCheckPasswordSection();
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    assertFalse(isElementVisible(section.$.weakCredentialsBody));
+  });
+
   // When canceled, show string explaining that and already found leak
   // count.
   test('showProgressAndLeaksAfterCanceled', async function() {
@@ -1146,8 +1231,8 @@ suite('PasswordsCheckSection', function() {
     flush();
     const listElements = checkPasswordSection.$.leakedPasswordList;
     const node = listElements.children[1];
-    assertEquals('password', node.$.leakedPassword.type);
-    assertNotEquals('test4', node.$.leakedPassword.value);
+    assertEquals('password', node.$.insecurePassword.type);
+    assertNotEquals('test4', node.$.insecurePassword.value);
 
     // Open the more actions menu and click 'Show Password'.
     node.$.more.click();
@@ -1162,15 +1247,15 @@ suite('PasswordsCheckSection', function() {
     const {reason} =
         await passwordManager.whenCalled('getPlaintextInsecurePassword');
     expectEquals(chrome.passwordsPrivate.PlaintextReason.VIEW, reason);
-    assertEquals('text', node.$.leakedPassword.type);
-    assertEquals('test4', node.$.leakedPassword.value);
+    assertEquals('text', node.$.insecurePassword.type);
+    assertEquals('test4', node.$.insecurePassword.value);
 
     // Open the more actions menu and click 'Hide Password'.
     node.$.more.click();
     checkPasswordSection.$.menuShowPassword.click();
 
-    assertEquals('password', node.$.leakedPassword.type);
-    assertNotEquals('test4', node.$.leakedPassword.value);
+    assertEquals('password', node.$.insecurePassword.type);
+    assertNotEquals('test4', node.$.insecurePassword.value);
   });
 
   // Verify if getPlaintext fails password will not be shown
@@ -1183,16 +1268,16 @@ suite('PasswordsCheckSection', function() {
     flush();
     const listElements = checkPasswordSection.$.leakedPasswordList;
     const node = listElements.children[1];
-    assertEquals('password', node.$.leakedPassword.type);
-    assertNotEquals('test4', node.$.leakedPassword.value);
+    assertEquals('password', node.$.insecurePassword.type);
+    assertNotEquals('test4', node.$.insecurePassword.value);
 
     // Open the more actions menu and click 'Show Password'.
     node.$.more.click();
     checkPasswordSection.$.menuShowPassword.click();
     await passwordManager.whenCalled('getPlaintextInsecurePassword');
     // Verify that password field didn't change
-    assertEquals('password', node.$.leakedPassword.type);
-    assertNotEquals('test4', node.$.leakedPassword.value);
+    assertEquals('password', node.$.insecurePassword.type);
+    assertNotEquals('test4', node.$.insecurePassword.value);
   });
 
   // Verify that clicking "Change password" reveals "Already changed password"
@@ -1260,15 +1345,15 @@ suite('PasswordsCheckSection', function() {
       await eventToPromise('close', checkPasswordSection.$.moreActionsMenu);
 
       // Verify that password field didn't change
-      assertEquals('password', node.$.leakedPassword.type);
-      assertNotEquals('test4', node.$.leakedPassword.value);
+      assertEquals('password', node.$.insecurePassword.type);
+      assertNotEquals('test4', node.$.insecurePassword.value);
 
       passwordManager.plaintextPassword_ = 'test4';
       node.tokenRequestManager.resolve();
       await passwordManager.whenCalled('getPlaintextInsecurePassword');
 
-      assertEquals('text', node.$.leakedPassword.type);
-      assertEquals('test4', node.$.leakedPassword.value);
+      assertEquals('text', node.$.insecurePassword.type);
+      assertEquals('test4', node.$.insecurePassword.value);
     });
   }