Implement PasswordStore::GetLoginsForSameOrganizationName.

components/password_manager/core/browser/login_database_unittest.cc

Index: components/password_manager/core/browser/login_database_unittest.cc
diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc
index b15468f57bbbb4768c7fd2e107d07dfe461721b5..f852331a4dae2a17ac63925343f9f16afa5b5bc8 100644
--- a/components/password_manager/core/browser/login_database_unittest.cc
+++ b/components/password_manager/core/browser/login_database_unittest.cc
@@ -21,6 +21,7 @@
 #include "build/build_config.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/os_crypt/os_crypt_mocker.h"
+#include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/psl_matching_helper.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
@@ -789,6 +790,145 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingRegexp) {
   EXPECT_EQ(0U, result.size());
 }
 
+TEST_F(LoginDatabaseTest,
+       GetLoginsForSameOrganizationName_OnlyWebHTTPFormsAreConsidered) {
+  const struct {
+    const PasswordFormData form_data;
+    const char* other_queried_signon_realm;
+    bool expected_matches_itself;
+    bool expected_matches_other_realm;
+  } kTestCases[] = {
+      {{PasswordForm::SCHEME_HTML, "https://example.com/",
+        "https://example.com/origin", "", L"", L"", L"", L"u", L"p", false, 1},
+       nullptr,
+       true,
+       true},
+      {{PasswordForm::SCHEME_BASIC, "http://example.com/realm",
+        "http://example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
+       nullptr,
+       false,
+       false},
+      {{PasswordForm::SCHEME_OTHER, "ftp://example.com/realm",
+        "ftp://example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
+       "http://example.com/realm",
+       false,
+       false},
+      {{PasswordForm::SCHEME_HTML,
+        "federation://example.com/accounts.google.com",
+        "https://example.com/orgin", "", L"", L"", L"", L"u",
+        kTestingFederatedLoginMarker, false, 1},
+       "http://example.com/",
+       false,
+       false},

[vasilii, 2017/05/24 17:48:38]

actually this should be true. Or we don't care about federated credentials
because they are not filled?

[engedy, 2017/05/24 19:27:12]

That was my thinking, but I can be convinced otherwise. Alternatively, we could
record them somewhere else.

@Maxim, what do you think?

[vasilii, 2017/05/26 10:53:36]

I agree with your thinking

[engedy, 2017/05/26 10:58:43]

Acknowledged.

+      {{PasswordForm::SCHEME_HTML, "android://hash@example.com/",
+        "android://hash@example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
+       "http://example.com/",
+       false,
+       false},
+  };
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(test_case.form_data.signon_realm);
+
+    std::unique_ptr<PasswordForm> form =
+        CreatePasswordFormFromDataForTesting(test_case.form_data);
+    ASSERT_EQ(AddChangeForForm(*form), db().AddLogin(*form));
+
+    std::vector<std::unique_ptr<PasswordForm>> same_organization_forms;
+    EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
+        form->signon_realm, &same_organization_forms));
+    EXPECT_EQ(test_case.expected_matches_itself ? 1u : 0u,

[vasilii, 2017/05/24 17:48:38]

Doesn't test_case.expected_matches_itself just work?

[engedy, 2017/05/24 19:27:12]

It happens to work, but I think it's less clear to the reader. What can they
expect from the EXPECT_EQ macro? Does it perform normal integral promotion, or
will it convert the size() to bool? (In the latter case possibly hiding errors
where there are 2 elements, not one.)

What do you think?

[vasilii, 2017/05/26 10:53:36]

If EXPECT_EQ goes crazy with converting the second parameter to bool then your
check will pass as well ;-)
Up to you. Assuming EXPECT_EQ just does "if (expected == actual)" we are safe.

[engedy, 2017/05/26 10:58:43]

Yeah, right. Shortened this.

+              same_organization_forms.size());
+
+    if (test_case.other_queried_signon_realm) {
+      same_organization_forms.clear();
+      EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
+          test_case.other_queried_signon_realm, &same_organization_forms));
+      EXPECT_EQ(test_case.expected_matches_other_realm ? 1u : 0u,
+                same_organization_forms.size());
+    }
+
+    ASSERT_TRUE(db().RemoveLogin(*form));
+  }
+}
+
+TEST_F(LoginDatabaseTest, GetLoginsForSameOrganizationName_DetailsOfMatching) {
+  const struct {
+    const char* saved_signon_realm;
+    const char* queried_signon_realm;
+    bool expected_matches;
+  } kTestCases[] = {
+      // PSL matches are also same-organization-name matches.
+      {"http://psl.example.com/", "http://example.com/", true},
+      {"http://example.com/", "http://sub.example.com/", true},
+      {"https://a.b.example.co.uk/", "https://c.d.e.example.co.uk/", true},
+
+      // Non-PSL but same-organization-name matches. Also an illustration why it
+      // would be unsafe to offer these credentials for filling.
+      {"https://example.com/", "https://example.co.uk/", true},
+      {"https://example.co.uk/", "https://example.com/", true},
+      {"https://a.example.appspot.com/", "https://b.example.co.uk/", true},
+
+      // Same-organization-name matches are HTTP/HTTPS-agnostic.
+      {"https://example.com/", "http://example.com/", true},
+      {"http://example.com/", "https://example.com/", true},
+
+      {"http://www.foo-bar.com/", "http://sub.foo-bar.com", true},
+      {"http://www.foo_bar.com/", "http://sub.foo_bar.com", true},
+      {"http://www.foo-bar.com/", "http://sub.foo%2Dbar.com", true},
+      {"http://www.foo%21bar.com/", "http://sub.foo!bar.com", true},
+      {"http://a.xn--sztr-7na0i.co.uk/", "http://xn--sztr-7na0i.com/", true},
+      {"http://a.xn--sztr-7na0i.co.uk/", "http://www.sz\xc3\xb3t\xc3\xa1r.com/",
+       true},
+
+      {"http://www.foo+bar.com/", "http://sub.foo+bar.com", true},
+      {"http://www.foooobar.com/", "http://sub.foo+bar.com", false},
+      {"http://www.fobar.com/", "http://sub.foo?bar.com", false},
+      {"http://www.foozbar.com/", "http://sub.foo.bar.com", false},
+      {"http://www.foozbar.com/", "http://sub.foo[a-z]bar.com", false},
+
+      {"https://notexample.com/", "https://example.com/", false},
+      {"https://a.notexample.com/", "https://example.com/", false},
+      {"https://example.com/", "https://notexample.com/", false},
+      {"https://example.com/", "https://example.bar.com/", false},
+      {"https://example.foo.com/", "https://example.com/", false},
+      {"https://example.foo.com/", "https://example.bar.com/", false},
+
+      // URLs without host portions, hosts without registry controlled domains
+      // or hosts consisting of a registry.
+      {"http://localhost/", "http://localhost/", false},
+      {"https://example/", "https://example/", false},
+      {"https://co.uk/", "https://co.uk/", false},
+      {"https://example/", "https://example.com/", false},
+      {"https://a.example/", "https://example.com/", false},
+      {"https://example.com/", "https://example/", false},
+      {"https://127.0.0.1/", "https://127.0.0.1/", false},
+      {"https:/[3ffe:2a00:100:7031::1]/", "https:/[3ffe:2a00:100:7031::1]/",
+       false},
+
+      // Queried |signon-realms| are invalid URIs.
+      {"https://example.com/", "", false},
+      {"https://example.com/", "bad url", false},
+      {"https://example.com/", "https://", false},
+      {"https://example.com/", "http://www.foo;bar.com", false},

[vasilii, 2017/05/24 17:48:38]

Also seems useful
 {"https://example.com/", "example", false},

[engedy, 2017/05/24 19:27:12]

Done.

+  };
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(test_case.saved_signon_realm);
+    SCOPED_TRACE(test_case.queried_signon_realm);
+
+    std::unique_ptr<PasswordForm> form = CreatePasswordFormFromDataForTesting(
+        {PasswordForm::SCHEME_HTML, test_case.saved_signon_realm,
+         test_case.saved_signon_realm, "", L"", L"", L"", L"u", L"p", true, 1});
+    std::vector<std::unique_ptr<PasswordForm>> result;
+    ASSERT_EQ(AddChangeForForm(*form), db().AddLogin(*form));
+    EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
+        test_case.queried_signon_realm, &result));
+    EXPECT_EQ(test_case.expected_matches ? 1u : 0u, result.size());
+    ASSERT_TRUE(db().RemoveLogin(*form));
+  }
+}
+
 static bool AddTimestampedLogin(LoginDatabase* db,
                                 std::string url,
                                 const std::string& unique_string,