Passwords PSL Matching on Gnome: overwrite realm and origin

chrome/browser/password_manager/native_backend_gnome_x_unittest.cc

Index: chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
diff --git a/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc b/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
index 66f9902a45d239cb971395b63745f309bb415271..cd6201a82c76b8c9d0e22b524b951586ba520ffe 100644
--- a/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
+++ b/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
@@ -282,6 +282,11 @@ class MockGnomeKeyringLoader : public GnomeKeyringLoader {
 
 class NativeBackendGnomeTest : public testing::Test {
  protected:
+  enum UpdateType {  // Used in CheckPSLUpdate().
+    UPDATE_BY_UPDATELOGIN,
+    UPDATE_BY_ADDLOGIN,
+  };
+
   NativeBackendGnomeTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
         db_thread_(BrowserThread::DB) {
@@ -396,16 +401,13 @@ class NativeBackendGnomeTest : public testing::Test {
     CheckStringAttribute(item, "application", app_string);
   }
 
-  // Checks (using EXPECT_* macros), that |credentials| are accessible for
-  // filling in for a page with |origin| iff
-  // |should_credential_be_available_to_url| is true.
-  void CheckCredentialAvailability(const PasswordForm& credentials,
-                                   const std::string& url,
-                                   bool should_credential_be_available_to_url) {
-    password_manager::PSLMatchingHelper helper;
-    ASSERT_TRUE(helper.IsMatchingEnabled())
-        << "PSL matching needs to be enabled.";
-
+  // Saves |credentials| and then gets login for origin and realm |url|. Returns
+  // true when something is found, and in such case copies the result to
+  // |result| when |result| is not NULL. (Note that there can be max. 1 result,
+  // derived from |credentials|.)
+  bool CheckCredentialAvailability(const PasswordForm& credentials,
+                                   const GURL& url,
+                                   PasswordForm* result) {
     NativeBackendGnome backend(321);
     backend.Init();
 
@@ -417,8 +419,8 @@ class NativeBackendGnomeTest : public testing::Test {
                    credentials));
 
     PasswordForm target_form;
-    target_form.origin = GURL(url);
-    target_form.signon_realm = url;
+    target_form.origin = url;
+    target_form.signon_realm = url.spec();
     std::vector<PasswordForm*> form_list;
     BrowserThread::PostTask(
         BrowserThread::DB,
@@ -434,10 +436,131 @@ class NativeBackendGnomeTest : public testing::Test {
     if (mock_keyring_items.size() > 0)
       CheckMockKeyringItem(&mock_keyring_items[0], credentials, "chrome-321");
 
-    if (should_credential_be_available_to_url)
-      EXPECT_EQ(1u, form_list.size());
-    else
-      EXPECT_EQ(0u, form_list.size());
+    if (form_list.empty())
+      return false;
+    EXPECT_EQ(1u, form_list.size());
+    if (result)
+      *result = *form_list[0];
+    STLDeleteElements(&form_list);
+    return true;
+  }
+
+  // Test that updating does not use PSL matching: Add a www.facebook.com
+  // password, then use PSL matching to get a copy of it for m.facebook.com, and
+  // add that copy as well. Now update the www.facebook.com password -- the
+  // m.facebook.com password should not get updated. Depending on the argument,
+  // the credential update is done via UpdateLogin or AddLogin.

[Garrett Casto, 2014/04/24 06:15:24]

Depending on what we end up doing with sync, it's not totally clear that calling
AddLogin for a an existing credential is going to be valid in the future. That
being said, for now it can still happen so it's reasonable to test.

[vabr (Chromium), 2014/04/24 08:33:47]

I agree. Added a TODO comment below to make it easier for the person who bans
AddLogin from acting on existing logins and observes PSLUpdatingStrictAddLogin
being broken.

+  void CheckPSLUpdate(UpdateType update_type) {
+    password_manager::PSLMatchingHelper helper;
+    ASSERT_TRUE(helper.IsMatchingEnabled());
+
+    NativeBackendGnome backend(321);
+    backend.Init();
+
+    // Add |form_facebook_| to saved logins.
+    BrowserThread::PostTask(
+        BrowserThread::DB,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin),
+                   base::Unretained(&backend),
+                   form_facebook_));
+
+    // Get the PSL-matched copy of the saved login for m.facebook.
+    const GURL kMobileURL("http://m.facebook.com/");
+    PasswordForm m_facebook_lookup;
+    m_facebook_lookup.origin = kMobileURL;
+    m_facebook_lookup.signon_realm = kMobileURL.spec();
+    std::vector<PasswordForm*> form_list;
+    BrowserThread::PostTask(
+        BrowserThread::DB,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&NativeBackendGnome::GetLogins),
+                   base::Unretained(&backend),
+                   m_facebook_lookup,
+                   &form_list));
+    RunBothThreads();
+    EXPECT_EQ(1u, mock_keyring_items.size());
+    EXPECT_EQ(1u, form_list.size());
+    PasswordForm m_facebook = *form_list[0];
+    STLDeleteElements(&form_list);
+    EXPECT_EQ(kMobileURL, m_facebook.origin);
+    EXPECT_EQ(kMobileURL.spec(), m_facebook.signon_realm);
+
+    // Add the PSL-matched copy to saved logins.
+    BrowserThread::PostTask(
+        BrowserThread::DB,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin),
+                   base::Unretained(&backend),
+                   m_facebook));
+    RunBothThreads();
+    EXPECT_EQ(2u, mock_keyring_items.size());
+
+    // Update www.facebook.com login.
+    PasswordForm new_facebook(form_facebook_);
+    const base::string16 kOldPassword(form_facebook_.password_value);
+    const base::string16 kNewPassword(UTF8ToUTF16("new_b"));
+    EXPECT_NE(kOldPassword, kNewPassword);
+    new_facebook.password_value = kNewPassword;
+    switch (update_type) {
+      case UPDATE_BY_UPDATELOGIN:
+        BrowserThread::PostTask(
+            BrowserThread::DB,
+            FROM_HERE,
+            base::Bind(base::IgnoreResult(&NativeBackendGnome::UpdateLogin),
+                       base::Unretained(&backend),
+                       new_facebook));
+        break;
+      case UPDATE_BY_ADDLOGIN:
+        BrowserThread::PostTask(
+            BrowserThread::DB,
+            FROM_HERE,
+            base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin),
+                       base::Unretained(&backend),
+                       new_facebook));
+        break;
+    }
+
+    RunBothThreads();
+    EXPECT_EQ(2u, mock_keyring_items.size());
+
+    // Check that m.facebook.com login was not modified by the update.
+    BrowserThread::PostTask(
+        BrowserThread::DB,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&NativeBackendGnome::GetLogins),
+                   base::Unretained(&backend),
+                   m_facebook_lookup,
+                   &form_list));
+    RunBothThreads();
+    // There should be two results -- the exact one, and the PSL-matched one.
+    EXPECT_EQ(2u, form_list.size());
+    size_t index_non_psl = 0;
+    if (!form_list[index_non_psl]->original_signon_realm.empty())
+      index_non_psl = 1;
+    EXPECT_EQ(kMobileURL, form_list[index_non_psl]->origin);
+    EXPECT_EQ(kMobileURL.spec(), form_list[index_non_psl]->signon_realm);
+    EXPECT_EQ(kOldPassword, form_list[index_non_psl]->password_value);
+    STLDeleteElements(&form_list);
+
+    // Check that www.facebook.com login was modified by the update.
+    BrowserThread::PostTask(
+        BrowserThread::DB,
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&NativeBackendGnome::GetLogins),
+                   base::Unretained(&backend),
+                   form_facebook_,
+                   &form_list));
+    RunBothThreads();
+    // There should be two results -- the exact one, and the PSL-matched one.
+    EXPECT_EQ(2u, form_list.size());
+    index_non_psl = 0;
+    if (!form_list[index_non_psl]->original_signon_realm.empty())
+      index_non_psl = 1;
+    EXPECT_EQ(form_facebook_.origin, form_list[index_non_psl]->origin);
+    EXPECT_EQ(form_facebook_.signon_realm,
+              form_list[index_non_psl]->signon_realm);
+    EXPECT_EQ(kNewPassword, form_list[index_non_psl]->password_value);
     STLDeleteElements(&form_list);
   }
 
@@ -496,24 +619,38 @@ TEST_F(NativeBackendGnomeTest, BasicListLogins) {
 
 // Save a password for www.facebook.com and see it suggested for m.facebook.com.
 TEST_F(NativeBackendGnomeTest, PSLMatchingPositive) {
-  CheckCredentialAvailability(form_facebook_,
-                              "http://m.facebook.com/",
-                              /*should_credential_be_available_to_url=*/true);
+  PasswordForm result;
+  const GURL kMobileURL("http://m.facebook.com/");
+  password_manager::PSLMatchingHelper helper;
+  ASSERT_TRUE(helper.IsMatchingEnabled());
+  EXPECT_TRUE(CheckCredentialAvailability(form_facebook_, kMobileURL, &result));
+  EXPECT_EQ(kMobileURL, result.origin);
+  EXPECT_EQ(kMobileURL.spec(), result.signon_realm);
 }
 
 // Save a password for www.facebook.com and see it not suggested for
 // m-facebook.com.
 TEST_F(NativeBackendGnomeTest, PSLMatchingNegativeDomainMismatch) {
-  CheckCredentialAvailability(form_facebook_,
-                              "http://m-facebook.com/",
-                              /*should_credential_be_available_to_url=*/false);
+  password_manager::PSLMatchingHelper helper;
+  ASSERT_TRUE(helper.IsMatchingEnabled());
+  EXPECT_FALSE(CheckCredentialAvailability(
+      form_facebook_, GURL("http://m-facebook.com/"), NULL));
 }
 
 // Test PSL matching is off for domains excluded from it.
 TEST_F(NativeBackendGnomeTest, PSLMatchingDisabledDomains) {
-  CheckCredentialAvailability(form_google_,
-                              "http://one.google.com/",
-                              /*should_credential_be_available_to_url=*/false);
+  password_manager::PSLMatchingHelper helper;
+  ASSERT_TRUE(helper.IsMatchingEnabled());
+  EXPECT_FALSE(CheckCredentialAvailability(
+      form_google_, GURL("http://one.google.com/"), NULL));
+}
+
+TEST_F(NativeBackendGnomeTest, PSLUpdatingStrictUpdateLogin) {
+  CheckPSLUpdate(UPDATE_BY_UPDATELOGIN);
+}
+
+TEST_F(NativeBackendGnomeTest, PSLUpdatingStrictAddLogin) {
+  CheckPSLUpdate(UPDATE_BY_ADDLOGIN);
 }
 
 TEST_F(NativeBackendGnomeTest, BasicUpdateLogin) {