accounts.google.com and myaccount.google.com should be PSL matched.

Now no google.com domains are PSL matched. This CL looses this for
Google sign-in and change password forms domains.

Bug: 1001086

Change-Id: I103d560272fd9986364e6625adb0389fec947df2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1754004
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693934}

components/password_manager/core/browser/login_database.cc

@@ -1351,8 +1351,7 @@ bool LoginDatabase::GetLogins(
   const GURL signon_realm(form.signon_realm);
   std::string registered_domain = GetRegistryControlledDomain(signon_realm);
   const bool should_PSL_matching_apply =
-      form.scheme == PasswordForm::Scheme::kHtml &&
-      ShouldPSLDomainMatchingApply(registered_domain);
+      !registered_domain.empty() && form.scheme == PasswordForm::Scheme::kHtml;
   const bool should_federated_apply =
       form.scheme == PasswordForm::Scheme::kHtml;
   DCHECK(!get_statement_.empty());

components/password_manager/core/browser/login_database_unittest.cc

@@ -612,17 +612,12 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingShouldMatchingApply) {
   EXPECT_TRUE(db().GetAutofillableLogins(&result));
   EXPECT_EQ(0U, result.size());
 
-  // Example password form.
+  // Saved password form on Google sign-in page.
   PasswordForm form;
   form.origin = GURL("https://accounts.google.com/");
-  form.action = GURL("https://accounts.google.com/login");
-  form.username_element = ASCIIToUTF16("username");
   form.username_value = ASCIIToUTF16("test@gmail.com");
-  form.password_element = ASCIIToUTF16("password");
   form.password_value = ASCIIToUTF16("test");
-  form.submit_element = ASCIIToUTF16("");
   form.signon_realm = "https://accounts.google.com/";
-  form.preferred = false;
   form.scheme = PasswordForm::Scheme::kHtml;
 
   // Add it and make sure it is there.
@@ -633,20 +628,26 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingShouldMatchingApply) {
 
   // Match against an exact copy.
   EXPECT_TRUE(db().GetLogins(PasswordStore::FormDigest(form), &result));
-  EXPECT_EQ(1U, result.size());
+  ASSERT_EQ(1U, result.size());
+  EXPECT_EQ(form.signon_realm, result[0]->signon_realm);
   result.clear();
 
-  // We go to a different site on the same domain where feature is not needed.
+  // Google change password should match to the saved sign-in form.
   PasswordStore::FormDigest form2 = {PasswordForm::Scheme::kHtml,
+                                     "https://myaccount.google.com/",
+                                     GURL("https://myaccount.google.com/")};
+
+  EXPECT_TRUE(db().GetLogins(form2, &result));
+  ASSERT_EQ(1U, result.size());
+  EXPECT_EQ(form.signon_realm, result[0]->signon_realm);
+  EXPECT_TRUE(result[0]->is_public_suffix_match);
+
+  // There should be no PSL match on other subdomains.
+  PasswordStore::FormDigest form3 = {PasswordForm::Scheme::kHtml,
                                      "https://some.other.google.com/",
                                      GURL("https://some.other.google.com/")};
 
-  // Match against the other site. Should not match since feature should not be
-  // enabled for this domain.
-  ASSERT_FALSE(ShouldPSLDomainMatchingApply(
-      GetRegistryControlledDomain(GURL(form2.signon_realm))));
-
-  EXPECT_TRUE(db().GetLogins(form2, &result));
+  EXPECT_TRUE(db().GetLogins(form3, &result));
   EXPECT_EQ(0U, result.size());
 }
 
@@ -686,8 +687,6 @@ TEST_F(LoginDatabaseTest, TestFederatedMatchingWithoutPSLMatching) {
   EXPECT_THAT(result, testing::ElementsAre(Pointee(form)));
 
   // Match against the second one.
-  ASSERT_FALSE(ShouldPSLDomainMatchingApply(
-      GetRegistryControlledDomain(GURL(form2.signon_realm))));
   form_request.origin = form2.origin;
   form_request.signon_realm = form2.signon_realm;
   EXPECT_TRUE(db().GetLogins(form_request, &result));

components/password_manager/core/browser/psl_matching_helper.cc

@@ -18,6 +18,14 @@ using autofill::PasswordForm;
 
 namespace password_manager {
 
+namespace {
+bool IsAllowedForPSLMatchedGoogleDomain(const GURL& url) {
+  return url.DomainIs("myaccount.google.com") ||
+         url.DomainIs("accounts.google.com");
+}
+
+}  // namespace
+
 std::ostream& operator<<(std::ostream& out, MatchResult result) {
   switch (result) {
     case MatchResult::NO_MATCH:
@@ -61,35 +69,28 @@ MatchResult GetMatchResult(const PasswordForm& form,
 
   // PSL and federated matches only apply to HTML forms.
   if (form_digest.scheme != PasswordForm::Scheme::kHtml ||
-      form.scheme != PasswordForm::Scheme::kHtml)
+      form.scheme != PasswordForm::Scheme::kHtml) {
     return MatchResult::NO_MATCH;
+  }
 
-  const bool allow_psl_match = ShouldPSLDomainMatchingApply(
-      GetRegistryControlledDomain(GURL(form_digest.signon_realm)));
-  const bool allow_federated_match = !form.federation_origin.opaque();
-
-  if (allow_psl_match &&
-      IsPublicSuffixDomainMatch(form.signon_realm, form_digest.signon_realm))
+  if (IsPublicSuffixDomainMatch(form.signon_realm, form_digest.signon_realm))
     return MatchResult::PSL_MATCH;
 
+  const bool allow_federated_match = !form.federation_origin.opaque();
   if (allow_federated_match &&
       IsFederatedRealm(form.signon_realm, form_digest.origin) &&
-      form.origin.GetOrigin() == form_digest.origin.GetOrigin())
+      form.origin.GetOrigin() == form_digest.origin.GetOrigin()) {
     return MatchResult::FEDERATED_MATCH;
+  }
 
-  if (allow_psl_match && allow_federated_match &&
-      IsFederatedPSLMatch(form.signon_realm, form.origin, form_digest.origin))
+  if (allow_federated_match &&
+      IsFederatedPSLMatch(form.signon_realm, form.origin, form_digest.origin)) {
     return MatchResult::FEDERATED_PSL_MATCH;
+  }
 
   return MatchResult::NO_MATCH;
 }
 
-bool ShouldPSLDomainMatchingApply(
-    const std::string& registry_controlled_domain) {
-  return !registry_controlled_domain.empty() &&
-         registry_controlled_domain != "google.com";
-}
-
 bool IsPublicSuffixDomainMatch(const std::string& url1,
                                const std::string& url2) {
   GURL gurl1(url1);
@@ -101,6 +102,12 @@ bool IsPublicSuffixDomainMatch(const std::string& url1,
   if (gurl1 == gurl2)
     return true;
 
+  if (gurl1.DomainIs("google.com") && gurl2.DomainIs("google.com")) {
+    return gurl1.scheme() == gurl2.scheme() && gurl1.port() == gurl2.port() &&
+           IsAllowedForPSLMatchedGoogleDomain(gurl1) &&
+           IsAllowedForPSLMatchedGoogleDomain(gurl2);
+  }
+
   std::string domain1(GetRegistryControlledDomain(gurl1));
   std::string domain2(GetRegistryControlledDomain(gurl2));
 

components/password_manager/core/browser/psl_matching_helper.h

@@ -54,18 +54,6 @@ bool IsFederatedPSLMatch(const std::string& form_signon_realm,
 MatchResult GetMatchResult(const autofill::PasswordForm& form,
                            const PasswordStore::FormDigest& form_digest);
 
-// Using the public suffix list for matching the origin is only needed for
-// websites that do not have a single hostname for entering credentials. It
-// would be better for their users if they did, but until then we help them find
-// credentials across different hostnames. We know that accounts.google.com is
-// the only hostname we should be accepting credentials on for any domain under
-// google.com, so we can apply a tighter policy for that domain. For owners of
-// domains where a single hostname is always used when your users are entering
-// their credentials, please contact palmer@chromium.org, nyquist@chromium.org
-// or file a bug at http://crbug.com/ to be added here.
-bool ShouldPSLDomainMatchingApply(
-    const std::string& registry_controlled_domain);
-
 // Two URLs are considered a Public Suffix Domain match if they have the same
 // scheme, ports, and their registry controlled domains are equal. If one or
 // both arguments do not describe valid URLs, returns false.

components/password_manager/core/browser/psl_matching_helper_unittest.cc

@@ -91,8 +91,14 @@ TEST(PSLMatchingUtilsTest, GetMatchResultPSL) {
       {"https://www.facebook.com/", "https://m.facebook.com",
        MatchResult::PSL_MATCH},
 
-      // Don't apply PSL matching to Google domains.
-      {"https://google.com/", "https://maps.google.com/",
+      // Google sign-in and change password pages are PSL matched.
+      {"https://accounts.google.com/", "https://myaccount.google.com/",
+       MatchResult::PSL_MATCH},
+
+      // Don't apply PSL matching to other Google domains.
+      {"https://accounts.google.com/", "https://maps.google.com/",
+       MatchResult::NO_MATCH},
+      {"https://subdomain1.google.com/", "https://maps.google.com/",
        MatchResult::NO_MATCH},
 
       // Scheme mismatch.