Don't re-store deleted passwords on form submit on the Mac

chrome/browser/password_manager/password_store_mac_unittest.cc

 // Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/basictypes.h"
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/keychain_mock_mac.h"
 #include "chrome/browser/password_manager/password_store_mac.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
+#include "chrome/common/chrome_paths.h"
 
 using webkit_glue::PasswordForm;
+using testing::_;
+using testing::DoAll;
+using testing::WithArg;
 
-class PasswordStoreMacTest : public testing::Test {
+namespace {
+
+class MockPasswordStoreConsumer : public PasswordStoreConsumer {
+public:
+  MOCK_METHOD2(OnPasswordStoreRequestDone,
+               void(int, const std::vector<webkit_glue::PasswordForm*>&));
+};
+
+ACTION(STLDeleteElements0) {
+  STLDeleteContainerPointers(arg0.begin(), arg0.end());
+}
+
+ACTION(QuitUIMessageLoop) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  MessageLoop::current()->Quit();
+}
+
+}  // namespace
+
+#pragma mark -
+
+class PasswordStoreMacInternalsTest : public testing::Test {
  public:
   virtual void SetUp() {
     MockKeychain::KeychainTestData test_data[] = {
       // Basic HTML form.
       { kSecAuthenticationTypeHTMLForm, "some.domain.com",
         kSecProtocolTypeHTTP, NULL, 0, NULL, "20020601171500Z",
         "joe_user", "sekrit", false },
       // HTML form with path.
       { kSecAuthenticationTypeHTMLForm, "some.domain.com",
         kSecProtocolTypeHTTP, "/insecure.html", 0, NULL, "19991231235959Z",
@@ skipping 158 matching lines @@
     }
     EXPECT_EQ(expectation->preferred, form->preferred)  << test_label;
     EXPECT_EQ(expectation->ssl_valid, form->ssl_valid) << test_label;
     EXPECT_DOUBLE_EQ(expectation->creation_time,
                      form->date_created.ToDoubleT()) << test_label;
   }
 }
 
 #pragma mark -
 
-TEST_F(PasswordStoreMacTest, TestKeychainToFormTranslation) {
+TEST_F(PasswordStoreMacInternalsTest, TestKeychainToFormTranslation) {
   typedef struct {
     const PasswordForm::Scheme scheme;
     const char* signon_realm;
     const char* origin;
     const wchar_t* username;  // Set to NULL to check for a blacklist entry.
     const wchar_t* password;
     const bool ssl_valid;
     const int creation_year;
     const int creation_month;
     const int creation_day;
@@ skipping 77 matching lines @@
   {
     // Use an invalid ref, to make sure errors are reported.
     SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(99);
     PasswordForm form;
     bool parsed = internal_keychain_helpers::FillPasswordFormFromKeychainItem(
         *keychain_, keychain_item, &form);
     EXPECT_FALSE(parsed);
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestKeychainSearch) {
+TEST_F(PasswordStoreMacInternalsTest, TestKeychainSearch) {
   struct TestDataAndExpectation {
     const PasswordFormData data;
     const size_t expected_fill_matches;
     const size_t expected_merge_matches;
   };
   // Most fields are left blank because we don't care about them for searching.
   TestDataAndExpectation test_data[] = {
     // An HTML form we've seen.
     { { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
         NULL, NULL, NULL, NULL, NULL, L"joe_user", NULL, false, false, 0 },
@@ skipping 40 matching lines @@
   for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
     scoped_ptr<PasswordForm> query_form(
         CreatePasswordFormFromData(test_data[i].data));
 
     // Check matches treating the form as a fill target.
     std::vector<PasswordForm*> matching_items =
         keychain_adapter.PasswordsFillingForm(*query_form);
     EXPECT_EQ(test_data[i].expected_fill_matches, matching_items.size());
     STLDeleteElements(&matching_items);
 
-    // Check matches teating the form as a merging target.
+    // Check matches treating the form as a merging target.
+    EXPECT_EQ(test_data[i].expected_merge_matches > 0,
+              keychain_adapter.HasPasswordsMergeableWithForm(*query_form));
     matching_items = keychain_adapter.PasswordsMergeableWithForm(*query_form);
     EXPECT_EQ(test_data[i].expected_merge_matches, matching_items.size());
     STLDeleteElements(&matching_items);
 
     // None of the pre-seeded items are owned by us, so none should match an
     // owned-passwords-only search.
     matching_items = owned_keychain_adapter.PasswordsFillingForm(*query_form);
     EXPECT_EQ(0U, matching_items.size());
     STLDeleteElements(&matching_items);
   }
@@ skipping 18 matching lines @@
 
 // Changes just the signon_ream auth realm of |form|.
 static void SetPasswordFormRealm(PasswordForm* form, const char* realm) {
   GURL::Replacements replacement;
   std::string new_value(realm);
   replacement.SetPathStr(new_value);
   GURL signon_gurl = GURL(form->signon_realm);
   form->signon_realm = signon_gurl.ReplaceComponents(replacement).spec();
 }
 
-TEST_F(PasswordStoreMacTest, TestKeychainExactSearch) {
+TEST_F(PasswordStoreMacInternalsTest, TestKeychainExactSearch) {
   MacKeychainPasswordFormAdapter keychain_adapter(keychain_);
 
   PasswordFormData base_form_data[] = {
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/insecure.html",
       NULL, NULL, NULL, NULL, L"joe_user", NULL, true, false, 0 },
     { PasswordForm::SCHEME_BASIC, "http://some.domain.com:4567/low_security",
       "http://some.domain.com:4567/insecure.html",
       NULL, NULL, NULL, NULL, L"basic_auth_user", NULL, true, false, 0 },
     { PasswordForm::SCHEME_DIGEST, "https://some.domain.com/high_security",
       "https://some.domain.com",
       NULL, NULL, NULL, NULL, L"digest_auth_user", NULL, true, true, 0 },
   };
 
   for (unsigned int i = 0; i < arraysize(base_form_data); ++i) {
     // Create a base form and make sure we find a match.
     scoped_ptr<PasswordForm> base_form(CreatePasswordFormFromData(
         base_form_data[i]));
+    EXPECT_TRUE(keychain_adapter.HasPasswordsMergeableWithForm(*base_form));
     PasswordForm* match =
         keychain_adapter.PasswordExactlyMatchingForm(*base_form);
     EXPECT_TRUE(match != NULL);
     if (match) {
       EXPECT_EQ(base_form->scheme, match->scheme);
       EXPECT_EQ(base_form->origin, match->origin);
       EXPECT_EQ(base_form->username_value, match->username_value);
       delete match;
     }
 
@@ skipping 25 matching lines @@
     for (unsigned int j = 0; j < modified_forms.size(); ++j) {
       PasswordForm* match =
           keychain_adapter.PasswordExactlyMatchingForm(*modified_forms[j]);
       EXPECT_EQ(NULL, match) << "In modified version " << j << " of base form "
                              << i;
     }
     STLDeleteElements(&modified_forms);
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestKeychainAdd) {
+TEST_F(PasswordStoreMacInternalsTest, TestKeychainAdd) {
   struct TestDataAndExpectation {
     PasswordFormData data;
     bool should_succeed;
   };
   TestDataAndExpectation test_data[] = {
     // Test a variety of scheme/port/protocol/path variations.
     { { PasswordForm::SCHEME_HTML, "http://web.site.com/",
         "http://web.site.com/path/to/page.html", NULL, NULL, NULL, NULL,
         L"anonymous", L"knock-knock", false, false, 0 }, true },
     { { PasswordForm::SCHEME_HTML, "https://web.site.com/",
@@ skipping 18 matching lines @@
 
   MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_);
   owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
 
   for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
     scoped_ptr<PasswordForm> in_form(
         CreatePasswordFormFromData(test_data[i].data));
     bool add_succeeded = owned_keychain_adapter.AddPassword(*in_form);
     EXPECT_EQ(test_data[i].should_succeed, add_succeeded);
     if (add_succeeded) {
+      EXPECT_TRUE(owned_keychain_adapter.HasPasswordsMergeableWithForm(
+          *in_form));
       scoped_ptr<PasswordForm> out_form(
           owned_keychain_adapter.PasswordExactlyMatchingForm(*in_form));
       EXPECT_TRUE(out_form.get() != NULL);
       EXPECT_EQ(out_form->scheme, in_form->scheme);
       EXPECT_EQ(out_form->signon_realm, in_form->signon_realm);
       EXPECT_EQ(out_form->origin, in_form->origin);
       EXPECT_EQ(out_form->username_value, in_form->username_value);
       EXPECT_EQ(out_form->password_value, in_form->password_value);
     }
   }
@@ skipping 10 matching lines @@
     EXPECT_TRUE(keychain_adapter.AddPassword(*update_form));
     SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(2);
     PasswordForm stored_form;
     internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
                                                                 keychain_item,
                                                                 &stored_form);
     EXPECT_EQ(update_form->password_value, stored_form.password_value);
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestKeychainRemove) {
+TEST_F(PasswordStoreMacInternalsTest, TestKeychainRemove) {
   struct TestDataAndExpectation {
     PasswordFormData data;
     bool should_succeed;
   };
   TestDataAndExpectation test_data[] = {
     // Test deletion of an item that we add.
     { { PasswordForm::SCHEME_HTML, "http://web.site.com/",
         "http://web.site.com/path/to/page.html", NULL, NULL, NULL, NULL,
         L"anonymous", L"knock-knock", false, false, 0 }, true },
     // Make sure we don't delete items we don't own.
@@ skipping 18 matching lines @@
 
     MacKeychainPasswordFormAdapter keychain_adapter(keychain_);
     PasswordForm* match = keychain_adapter.PasswordExactlyMatchingForm(*form);
     EXPECT_EQ(test_data[i].should_succeed, match == NULL);
     if (match) {
       delete match;
     }
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestFormMatch) {
+TEST_F(PasswordStoreMacInternalsTest, TestFormMatch) {
   PasswordForm base_form;
   base_form.signon_realm = std::string("http://some.domain.com/");
   base_form.origin = GURL("http://some.domain.com/page.html");
   base_form.username_value = ASCIIToUTF16("joe_user");
 
   {
     // Check that everything unimportant can be changed.
     PasswordForm different_form(base_form);
     different_form.username_element = ASCIIToUTF16("username");
     different_form.submit_element = ASCIIToUTF16("submit");
@@ skipping 41 matching lines @@
   // Blacklist forms should *never* match for merging, even when identical
   // (and certainly not when only one is a blacklist entry).
   {
     PasswordForm form_a(base_form);
     form_a.blacklisted_by_user = true;
     PasswordForm form_b(form_a);
     EXPECT_FALSE(internal_keychain_helpers::FormsMatchForMerge(form_a, form_b));
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestFormMerge) {
+TEST_F(PasswordStoreMacInternalsTest, TestFormMerge) {
   // Set up a bunch of test data to use in varying combinations.
   PasswordFormData keychain_user_1 =
       { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
         "http://some.domain.com/", "", L"", L"", L"", L"joe_user", L"sekrit",
         false, false, 1010101010 };
   PasswordFormData keychain_user_1_with_path =
       { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
         "http://some.domain.com/page.html",
         "", L"", L"", L"", L"joe_user", L"otherpassword",
         false, false, 1010101010 };
@@ skipping 134 matching lines @@
     CHECK_FORMS(database_forms, test_data[DATABASE_OUTPUT][test_case],
                 test_case);
     CHECK_FORMS(merged_forms, test_data[MERGE_OUTPUT][test_case], test_case);
 
     STLDeleteElements(&keychain_forms);
     STLDeleteElements(&database_forms);
     STLDeleteElements(&merged_forms);
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestPasswordBulkLookup) {
+TEST_F(PasswordStoreMacInternalsTest, TestPasswordBulkLookup) {
   PasswordFormData db_data[] = {
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/", "http://some.domain.com/action.cgi",
       L"submit", L"username", L"password", L"joe_user", L"",
       true, false, 1212121212 },
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/page.html",
       "http://some.domain.com/handlepage.cgi",
       L"submit", L"username", L"password", L"joe_user", L"",
       true, false, 1234567890 },
@@ skipping 22 matching lines @@
   EXPECT_EQ(2U, database_forms.size());
   ASSERT_EQ(3U, merged_forms.size());
   EXPECT_EQ(ASCIIToUTF16("sekrit"), merged_forms[0]->password_value);
   EXPECT_EQ(ASCIIToUTF16("sekrit"), merged_forms[1]->password_value);
   EXPECT_EQ(true, merged_forms[2]->blacklisted_by_user);
 
   STLDeleteElements(&database_forms);
   STLDeleteElements(&merged_forms);
 }
 
-TEST_F(PasswordStoreMacTest, TestPasswordGetAll) {
+TEST_F(PasswordStoreMacInternalsTest, TestPasswordGetAll) {
   MacKeychainPasswordFormAdapter keychain_adapter(keychain_);
   MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_);
   owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
 
   // Add a few passwords of various types so that we own some.
   PasswordFormData owned_password_data[] = {
     { PasswordForm::SCHEME_HTML, "http://web.site.com/",
       "http://web.site.com/path/to/page.html", NULL, NULL, NULL, NULL,
       L"anonymous", L"knock-knock", false, false, 0 },
     { PasswordForm::SCHEME_BASIC, "http://a.site.com:2222/therealm",
@@ skipping 12 matching lines @@
   std::vector<PasswordForm*> all_passwords =
       keychain_adapter.GetAllPasswordFormPasswords();
   EXPECT_EQ(8 + arraysize(owned_password_data), all_passwords.size());
   STLDeleteElements(&all_passwords);
 
   std::vector<PasswordForm*> owned_passwords =
       owned_keychain_adapter.GetAllPasswordFormPasswords();
   EXPECT_EQ(arraysize(owned_password_data), owned_passwords.size());
   STLDeleteElements(&owned_passwords);
 }
+
+#pragma mark -
+
+class PasswordStoreMacTest : public testing::Test {
+ public:
+  PasswordStoreMacTest() : ui_thread_(ChromeThread::UI, &message_loop_) {}
+
+  virtual void SetUp() {
+    login_db_ = new LoginDatabase();
+    ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
+    FilePath db_file = db_dir_.path().AppendASCII("login.db");
+    ASSERT_TRUE(login_db_->Init(db_file));
+
+    keychain_ = new MockKeychain(3);
+
+    store_ = new PasswordStoreMac(keychain_, login_db_);
+    ASSERT_TRUE(store_->Init());
+  }
+
+  virtual void TearDown() {
+    MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask);
+    MessageLoop::current()->Run();
+  }
+
+ protected:
+  MessageLoopForUI message_loop_;
+  ChromeThread ui_thread_;
+
+  MockKeychain* keychain_;  // Owned by store_.
+  LoginDatabase* login_db_;  // Owned by store_.
+  scoped_refptr<PasswordStoreMac> store_;
+  ScopedTempDir db_dir_;
+};
+
+TEST_F(PasswordStoreMacTest, TestStoreUpdate) {
+  // Insert a password into both the database and the keychain.
+  // This is done manually, rather than through store_->AddLogin, because the
+  // Mock Keychain isn't smart enough to be able to support update generically,
+  // so some.domain.com triggers special handling to test it that make inserting
+  // fail.
+  PasswordFormData joint_data = {
+    PasswordForm::SCHEME_HTML, "http://some.domain.com/",
+    "http://some.domain.com/insecure.html", "login.cgi",
+    L"username", L"password", L"submit", L"joe_user", L"sekrit", true, false, 1
+  };
+  scoped_ptr<PasswordForm> joint_form(CreatePasswordFormFromData(joint_data));
+  login_db_->AddLogin(*joint_form);
+  MockKeychain::KeychainTestData joint_keychain_data = {
+    kSecAuthenticationTypeHTMLForm, "some.domain.com",
+    kSecProtocolTypeHTTP, "/insecure.html", 0, NULL, "20020601171500Z",
+    "joe_user", "sekrit", false };
+  keychain_->AddTestItem(joint_keychain_data);
+
+  // Insert a password into the keychain only.
+  MockKeychain::KeychainTestData keychain_only_data = {
+    kSecAuthenticationTypeHTMLForm, "keychain.only.com",
+    kSecProtocolTypeHTTP, NULL, 0, NULL, "20020601171500Z",
+    "keychain", "only", false
+  };
+  keychain_->AddTestItem(keychain_only_data);
+
+  struct UpdateData {
+    PasswordFormData form_data;
+    const char* password;  // NULL indicates no entry should be present.
+  };
+
+  // Make a series of update calls.
+  UpdateData updates[] = {
+    // Update the keychain+db passwords (the normal password update case).
+    { { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
+        "http://some.domain.com/insecure.html", "login.cgi",
+        L"username", L"password", L"submit", L"joe_user", L"53krit",
+        true, false, 2 },
+      "53krit",
+    },
+    // Update the keychain-only password; this simulates the initial use of a
+    // password stored by another browsers.
+    { { PasswordForm::SCHEME_HTML, "http://keychain.only.com/",
+        "http://keychain.only.com/login.html", "login.cgi",
+        L"username", L"password", L"submit", L"keychain", L"only",
+        true, false, 2 },
+      "only",
+    },
+    // Update a password that doesn't exist in either location. This tests the
+    // case where a form is filled, then the stored login is removed, then the
+    // form is submitted.
+    { { PasswordForm::SCHEME_HTML, "http://different.com/",
+        "http://different.com/index.html", "login.cgi",
+        L"username", L"password", L"submit", L"abc", L"123",
+        true, false, 2 },
+      NULL,
+    },
+  };
+  for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(updates); ++i) {
+    scoped_ptr<PasswordForm> form(CreatePasswordFormFromData(
+        updates[i].form_data));
+    store_->UpdateLogin(*form);
+  }
+
+  // Do a store-level query to wait for all the operations above to be done.
+  MockPasswordStoreConsumer consumer;
+  ON_CALL(consumer, OnPasswordStoreRequestDone(_, _)).WillByDefault(
+      QuitUIMessageLoop());
+  EXPECT_CALL(consumer, OnPasswordStoreRequestDone(_, _)).WillOnce(
+      DoAll(WithArg<1>(STLDeleteElements0()), QuitUIMessageLoop()));
+  store_->GetLogins(*joint_form, &consumer);
+  MessageLoop::current()->Run();
+
+  MacKeychainPasswordFormAdapter keychain_adapter(keychain_);
+  for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(updates); ++i) {
+    scoped_ptr<PasswordForm> query_form(
+        CreatePasswordFormFromData(updates[i].form_data));
+
+    std::vector<PasswordForm*> matching_items =
+        keychain_adapter.PasswordsFillingForm(*query_form);
+    if (updates[i].password) {
+      EXPECT_GT(matching_items.size(), 0U) << "iteration " << i;
+      if (matching_items.size() >= 1)
+        EXPECT_EQ(ASCIIToUTF16(updates[i].password),
+                  matching_items[0]->password_value) << "iteration " << i;
+    } else {
+      EXPECT_EQ(0U, matching_items.size()) << "iteration " << i;
+    }
+    STLDeleteElements(&matching_items);
+
+    login_db_->GetLogins(*query_form, &matching_items);
+    EXPECT_EQ(updates[i].password ? 1U : 0U, matching_items.size())
+        << "iteration " << i;
+    STLDeleteElements(&matching_items);
+  }
+}