diff options
author | vasilii@chromium.org <vasilii@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-17 21:12:45 +0000 |
---|---|---|
committer | vasilii@chromium.org <vasilii@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-17 21:12:45 +0000 |
commit | 1d26103fdc24c560547262954029bd278859fd44 (patch) | |
tree | 42ab18cfb516f147be642a5127baac4b76e18d6b | |
parent | 99b73c36bfe37307cb5a1aabcc4ea3a32ddd54aa (diff) | |
download | chromium_src-1d26103fdc24c560547262954029bd278859fd44.zip chromium_src-1d26103fdc24c560547262954029bd278859fd44.tar.gz chromium_src-1d26103fdc24c560547262954029bd278859fd44.tar.bz2 |
Support to remove passwords by date_synced timestamp.
The new method RemoveLoginsSyncedBetween() works exactly like the existing RemoveLoginsCreatedBetween().
BUG=362679
TBR=zea@chromium.org
Review URL: https://codereview.chromium.org/335893002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277863 0039d316-1c4b-4281-b951-d872f2087c98
24 files changed, 727 insertions, 185 deletions
diff --git a/chrome/browser/password_manager/native_backend_gnome_x.cc b/chrome/browser/password_manager/native_backend_gnome_x.cc index 6b4236a..7dd651e 100644 --- a/chrome/browser/password_manager/native_backend_gnome_x.cc +++ b/chrome/browser/password_manager/native_backend_gnome_x.cc @@ -642,26 +642,17 @@ bool NativeBackendGnome::RemoveLogin(const PasswordForm& form) { return true; } -bool NativeBackendGnome::RemoveLoginsCreatedBetween( - const base::Time& delete_begin, - const base::Time& delete_end) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); - bool ok = true; - // We could walk the list and delete items as we find them, but it is much - // easier to build the list and use RemoveLogin() to delete them. - PasswordFormList forms; - if (!GetAllLogins(&forms)) - return false; +bool NativeBackendGnome::RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) { + return RemoveLoginsBetween( + delete_begin, delete_end, CREATION_TIMESTAMP, NULL); +} - for (size_t i = 0; i < forms.size(); ++i) { - if (delete_begin <= forms[i]->date_created && - (delete_end.is_null() || forms[i]->date_created < delete_end)) { - if (!RemoveLogin(*forms[i])) - ok = false; - } - delete forms[i]; - } - return ok; +bool NativeBackendGnome::RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) { + return RemoveLoginsBetween(delete_begin, delete_end, SYNC_TIMESTAMP, changes); } bool NativeBackendGnome::GetLogins(const PasswordForm& form, @@ -683,27 +674,10 @@ bool NativeBackendGnome::GetLogins(const PasswordForm& form, return true; } -bool NativeBackendGnome::GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, +bool NativeBackendGnome::GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); - // We could walk the list and add items as we find them, but it is much - // easier to build the list and then filter the results. - PasswordFormList all_forms; - if (!GetAllLogins(&all_forms)) - return false; - - forms->reserve(forms->size() + all_forms.size()); - for (size_t i = 0; i < all_forms.size(); ++i) { - if (get_begin <= all_forms[i]->date_created && - (get_end.is_null() || all_forms[i]->date_created < get_end)) { - forms->push_back(all_forms[i]); - } else { - delete all_forms[i]; - } - } - - return true; + return GetLoginsBetween(get_begin, get_end, CREATION_TIMESTAMP, forms); } bool NativeBackendGnome::GetAutofillableLogins(PasswordFormList* forms) { @@ -753,6 +727,61 @@ bool NativeBackendGnome::GetAllLogins(PasswordFormList* forms) { return true; } +bool NativeBackendGnome::GetLoginsBetween(base::Time get_begin, + base::Time get_end, + TimestampToCompare date_to_compare, + PasswordFormList* forms) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + // We could walk the list and add items as we find them, but it is much + // easier to build the list and then filter the results. + PasswordFormList all_forms; + if (!GetAllLogins(&all_forms)) + return false; + + base::Time autofill::PasswordForm::*date_member = + date_to_compare == CREATION_TIMESTAMP + ? &autofill::PasswordForm::date_created + : &autofill::PasswordForm::date_synced; + for (size_t i = 0; i < all_forms.size(); ++i) { + if (get_begin <= all_forms[i]->*date_member && + (get_end.is_null() || all_forms[i]->*date_member < get_end)) { + forms->push_back(all_forms[i]); + } else { + delete all_forms[i]; + } + } + + return true; +} + +bool NativeBackendGnome::RemoveLoginsBetween( + base::Time get_begin, + base::Time get_end, + TimestampToCompare date_to_compare, + password_manager::PasswordStoreChangeList* changes) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); + if (changes) + changes->clear(); + // We could walk the list and delete items as we find them, but it is much + // easier to build the list and use RemoveLogin() to delete them. + ScopedVector<autofill::PasswordForm> forms; + if (!GetLoginsBetween(get_begin, get_end, date_to_compare, &forms.get())) + return false; + + bool ok = true; + for (size_t i = 0; i < forms.size(); ++i) { + if (RemoveLogin(*forms[i])) { + if (changes) { + changes->push_back(password_manager::PasswordStoreChange( + password_manager::PasswordStoreChange::REMOVE, *forms[i])); + } + } else { + ok = false; + } + } + return ok; +} + std::string NativeBackendGnome::GetProfileSpecificAppString() const { // Originally, the application string was always just "chrome" and used only // so that we had *something* to search for since GNOME Keyring won't search diff --git a/chrome/browser/password_manager/native_backend_gnome_x.h b/chrome/browser/password_manager/native_backend_gnome_x.h index c5d2c2e..67d84a5 100644 --- a/chrome/browser/password_manager/native_backend_gnome_x.h +++ b/chrome/browser/password_manager/native_backend_gnome_x.h @@ -87,17 +87,26 @@ class NativeBackendGnome : public PasswordStoreX::NativeBackend, const autofill::PasswordForm& form, password_manager::PasswordStoreChangeList* changes) OVERRIDE; virtual bool RemoveLogin(const autofill::PasswordForm& form) OVERRIDE; - virtual bool RemoveLoginsCreatedBetween( - const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE; + virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) OVERRIDE; + virtual bool RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) OVERRIDE; virtual bool GetLogins(const autofill::PasswordForm& form, PasswordFormList* forms) OVERRIDE; - virtual bool GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, + virtual bool GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) OVERRIDE; virtual bool GetAutofillableLogins(PasswordFormList* forms) OVERRIDE; virtual bool GetBlacklistLogins(PasswordFormList* forms) OVERRIDE; private: + enum TimestampToCompare { + CREATION_TIMESTAMP, + SYNC_TIMESTAMP, + }; + // Adds a login form without checking for one to replace first. bool RawAddLogin(const autofill::PasswordForm& form); @@ -107,6 +116,20 @@ class NativeBackendGnome : public PasswordStoreX::NativeBackend, // Helper for GetLoginsCreatedBetween(). bool GetAllLogins(PasswordFormList* forms); + // Retrieves password created/synced in the time interval. Returns |true| if + // the operation succeeded. + bool GetLoginsBetween(base::Time get_begin, + base::Time get_end, + TimestampToCompare date_to_compare, + PasswordFormList* forms); + + // Removes password created/synced in the time interval. Returns |true| if the + // operation succeeded. |changes| will contain the changes applied. + bool RemoveLoginsBetween(base::Time get_begin, + base::Time get_end, + TimestampToCompare date_to_compare, + password_manager::PasswordStoreChangeList* changes); + // Generates a profile-specific app string based on profile_id_. std::string GetProfileSpecificAppString() const; 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 cf8f2214..fb045ca 100644 --- a/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc +++ b/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc @@ -309,6 +309,13 @@ void CheckPasswordChanges(const PasswordStoreChangeList& expected_list, } } +void CheckPasswordChangesWithResult(const PasswordStoreChangeList* expected, + const PasswordStoreChangeList* actual, + bool result) { + EXPECT_TRUE(result); + CheckPasswordChanges(*expected, *actual); +} + } // anonymous namespace class NativeBackendGnomeTest : public testing::Test { @@ -948,4 +955,65 @@ TEST_F(NativeBackendGnomeTest, ListLoginsAppends) { CheckMockKeyringItem(&mock_keyring_items[0], form_google_, "chrome-42"); } +TEST_F(NativeBackendGnomeTest, RemoveLoginsSyncedBetween) { + NativeBackendGnome backend(42); + backend.Init(); + + base::Time now = base::Time::Now(); + base::Time next_day = now + base::TimeDelta::FromDays(1); + form_google_.date_synced = now; + form_isc_.date_synced = next_day; + form_google_.date_created = base::Time(); + form_isc_.date_created = base::Time(); + + BrowserThread::PostTask( + BrowserThread::DB, + FROM_HERE, + base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin), + base::Unretained(&backend), + form_google_)); + BrowserThread::PostTask( + BrowserThread::DB, + FROM_HERE, + base::Bind(base::IgnoreResult(&NativeBackendGnome::AddLogin), + base::Unretained(&backend), + form_isc_)); + + PasswordStoreChangeList expected_changes; + expected_changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, form_google_)); + PasswordStoreChangeList changes; + BrowserThread::PostTaskAndReplyWithResult( + BrowserThread::DB, + FROM_HERE, + base::Bind(&NativeBackendGnome::RemoveLoginsSyncedBetween, + base::Unretained(&backend), + base::Time(), + next_day, + &changes), + base::Bind(&CheckPasswordChangesWithResult, &expected_changes, &changes)); + RunBothThreads(); + + EXPECT_EQ(1u, mock_keyring_items.size()); + if (mock_keyring_items.size() > 0) + CheckMockKeyringItem(&mock_keyring_items[0], form_isc_, "chrome-42"); + + // Remove form_isc_. + expected_changes.clear(); + expected_changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, form_isc_)); + BrowserThread::PostTaskAndReplyWithResult( + BrowserThread::DB, + FROM_HERE, + base::Bind(&NativeBackendGnome::RemoveLoginsSyncedBetween, + base::Unretained(&backend), + next_day, + base::Time(), + &changes), + base::Bind(&CheckPasswordChangesWithResult, &expected_changes, &changes)); + RunBothThreads(); + + EXPECT_EQ(0u, mock_keyring_items.size()); +} + // TODO(mdm): add more basic tests here at some point. diff --git a/chrome/browser/password_manager/native_backend_kwallet_x.cc b/chrome/browser/password_manager/native_backend_kwallet_x.cc index d8816e5..e493d0d 100644 --- a/chrome/browser/password_manager/native_backend_kwallet_x.cc +++ b/chrome/browser/password_manager/native_backend_kwallet_x.cc @@ -356,94 +356,18 @@ bool NativeBackendKWallet::RemoveLogin(const PasswordForm& form) { return ok; } -bool NativeBackendKWallet::RemoveLoginsCreatedBetween( - const base::Time& delete_begin, - const base::Time& delete_end) { - int wallet_handle = WalletHandle(); - if (wallet_handle == kInvalidKWalletHandle) - return false; - - // We could probably also use readEntryList here. - std::vector<std::string> realm_list; - { - dbus::MethodCall method_call(kKWalletInterface, "entryList"); - dbus::MessageWriter builder(&method_call); - builder.AppendInt32(wallet_handle); // handle - builder.AppendString(folder_name_); // folder - builder.AppendString(app_name_); // appid - scoped_ptr<dbus::Response> response( - kwallet_proxy_->CallMethodAndBlock( - &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT)); - if (!response.get()) { - LOG(ERROR) << "Error contacting kwalletd (entryList)"; - return false; - } - dbus::MessageReader reader(response.get()); - dbus::MessageReader array(response.get()); - if (!reader.PopArray(&array)) { - LOG(ERROR) << "Error reading response from kwalletd (entryList): " - << response->ToString(); - return false; - } - while (array.HasMoreData()) { - std::string realm; - if (!array.PopString(&realm)) { - LOG(ERROR) << "Error reading response from kwalletd (entryList): " - << response->ToString(); - return false; - } - realm_list.push_back(realm); - } - } - - bool ok = true; - for (size_t i = 0; i < realm_list.size(); ++i) { - const std::string& signon_realm = realm_list[i]; - dbus::MethodCall method_call(kKWalletInterface, "readEntry"); - dbus::MessageWriter builder(&method_call); - builder.AppendInt32(wallet_handle); // handle - builder.AppendString(folder_name_); // folder - builder.AppendString(signon_realm); // key - builder.AppendString(app_name_); // appid - scoped_ptr<dbus::Response> response( - kwallet_proxy_->CallMethodAndBlock( - &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT)); - if (!response.get()) { - LOG(ERROR) << "Error contacting kwalletd (readEntry)"; - continue; - } - dbus::MessageReader reader(response.get()); - const uint8_t* bytes = NULL; - size_t length = 0; - if (!reader.PopArrayOfBytes(&bytes, &length)) { - LOG(ERROR) << "Error reading response from kwalletd (readEntry): " - << response->ToString(); - continue; - } - if (!bytes || !CheckSerializedValue(bytes, length, signon_realm)) - continue; - - // Can't we all just agree on whether bytes are signed or not? Please? - Pickle pickle(reinterpret_cast<const char*>(bytes), length); - PasswordFormList all_forms; - DeserializeValue(signon_realm, pickle, &all_forms); - - PasswordFormList kept_forms; - kept_forms.reserve(all_forms.size()); - for (size_t i = 0; i < all_forms.size(); ++i) { - if (delete_begin <= all_forms[i]->date_created && - (delete_end.is_null() || all_forms[i]->date_created < delete_end)) { - delete all_forms[i]; - } else { - kept_forms.push_back(all_forms[i]); - } - } +bool NativeBackendKWallet::RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) { + password_manager::PasswordStoreChangeList changes; + return RemoveLoginsBetween( + delete_begin, delete_end, CREATION_TIMESTAMP, &changes); +} - if (!SetLoginsList(kept_forms, signon_realm, wallet_handle)) - ok = false; - STLDeleteElements(&kept_forms); - } - return ok; +bool NativeBackendKWallet::RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) { + return RemoveLoginsBetween(delete_begin, delete_end, SYNC_TIMESTAMP, changes); } bool NativeBackendKWallet::GetLogins(const PasswordForm& form, @@ -454,13 +378,14 @@ bool NativeBackendKWallet::GetLogins(const PasswordForm& form, return GetLoginsList(forms, form.signon_realm, wallet_handle); } -bool NativeBackendKWallet::GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, +bool NativeBackendKWallet::GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) { int wallet_handle = WalletHandle(); if (wallet_handle == kInvalidKWalletHandle) return false; - return GetLoginsList(forms, get_begin, get_end, wallet_handle); + return GetLoginsList( + forms, get_begin, get_end, wallet_handle, CREATION_TIMESTAMP); } bool NativeBackendKWallet::GetAutofillableLogins(PasswordFormList* forms) { @@ -571,16 +496,21 @@ bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms, bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms, const base::Time& begin, const base::Time& end, - int wallet_handle) { + int wallet_handle, + TimestampToCompare date_to_compare) { PasswordFormList all_forms; if (!GetAllLogins(&all_forms, wallet_handle)) return false; // We have to read all the entries, and then filter them here. + base::Time autofill::PasswordForm::*date_member = + date_to_compare == CREATION_TIMESTAMP + ? &autofill::PasswordForm::date_created + : &autofill::PasswordForm::date_synced; forms->reserve(forms->size() + all_forms.size()); for (size_t i = 0; i < all_forms.size(); ++i) { - if (begin <= all_forms[i]->date_created && - (end.is_null() || all_forms[i]->date_created < end)) { + if (begin <= all_forms[i]->*date_member && + (end.is_null() || all_forms[i]->*date_member < end)) { forms->push_back(all_forms[i]); } else { delete all_forms[i]; @@ -709,6 +639,106 @@ bool NativeBackendKWallet::SetLoginsList(const PasswordFormList& forms, return ret == 0; } +bool NativeBackendKWallet::RemoveLoginsBetween( + base::Time delete_begin, + base::Time delete_end, + TimestampToCompare date_to_compare, + password_manager::PasswordStoreChangeList* changes) { + DCHECK(changes); + changes->clear(); + int wallet_handle = WalletHandle(); + if (wallet_handle == kInvalidKWalletHandle) + return false; + + // We could probably also use readEntryList here. + std::vector<std::string> realm_list; + { + dbus::MethodCall method_call(kKWalletInterface, "entryList"); + dbus::MessageWriter builder(&method_call); + builder.AppendInt32(wallet_handle); // handle + builder.AppendString(folder_name_); // folder + builder.AppendString(app_name_); // appid + scoped_ptr<dbus::Response> response(kwallet_proxy_->CallMethodAndBlock( + &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT)); + if (!response.get()) { + LOG(ERROR) << "Error contacting kwalletd (entryList)"; + return false; + } + dbus::MessageReader reader(response.get()); + dbus::MessageReader array(response.get()); + if (!reader.PopArray(&array)) { + LOG(ERROR) << "Error reading response from kwalletd (entryList): " + << response->ToString(); + return false; + } + while (array.HasMoreData()) { + std::string realm; + if (!array.PopString(&realm)) { + LOG(ERROR) << "Error reading response from kwalletd (entryList): " + << response->ToString(); + return false; + } + realm_list.push_back(realm); + } + } + + bool ok = true; + for (size_t i = 0; i < realm_list.size(); ++i) { + const std::string& signon_realm = realm_list[i]; + dbus::MethodCall method_call(kKWalletInterface, "readEntry"); + dbus::MessageWriter builder(&method_call); + builder.AppendInt32(wallet_handle); // handle + builder.AppendString(folder_name_); // folder + builder.AppendString(signon_realm); // key + builder.AppendString(app_name_); // appid + scoped_ptr<dbus::Response> response(kwallet_proxy_->CallMethodAndBlock( + &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT)); + if (!response.get()) { + LOG(ERROR) << "Error contacting kwalletd (readEntry)"; + continue; + } + dbus::MessageReader reader(response.get()); + const uint8_t* bytes = NULL; + size_t length = 0; + if (!reader.PopArrayOfBytes(&bytes, &length)) { + LOG(ERROR) << "Error reading response from kwalletd (readEntry): " + << response->ToString(); + continue; + } + if (!bytes || !CheckSerializedValue(bytes, length, signon_realm)) + continue; + + // Can't we all just agree on whether bytes are signed or not? Please? + Pickle pickle(reinterpret_cast<const char*>(bytes), length); + PasswordFormList all_forms; + DeserializeValue(signon_realm, pickle, &all_forms); + + PasswordFormList kept_forms; + kept_forms.reserve(all_forms.size()); + base::Time autofill::PasswordForm::*date_member = + date_to_compare == CREATION_TIMESTAMP + ? &autofill::PasswordForm::date_created + : &autofill::PasswordForm::date_synced; + for (size_t i = 0; i < all_forms.size(); ++i) { + if (delete_begin <= all_forms[i]->*date_member && + (delete_end.is_null() || all_forms[i]->*date_member < delete_end)) { + changes->push_back(password_manager::PasswordStoreChange( + password_manager::PasswordStoreChange::REMOVE, *all_forms[i])); + delete all_forms[i]; + } else { + kept_forms.push_back(all_forms[i]); + } + } + + if (!SetLoginsList(kept_forms, signon_realm, wallet_handle)) { + ok = false; + changes->clear(); + } + STLDeleteElements(&kept_forms); + } + return ok; +} + // static void NativeBackendKWallet::SerializeValue(const PasswordFormList& forms, Pickle* pickle) { diff --git a/chrome/browser/password_manager/native_backend_kwallet_x.h b/chrome/browser/password_manager/native_backend_kwallet_x.h index 2567ba0e..4e7f689 100644 --- a/chrome/browser/password_manager/native_backend_kwallet_x.h +++ b/chrome/browser/password_manager/native_backend_kwallet_x.h @@ -46,12 +46,16 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend { const autofill::PasswordForm& form, password_manager::PasswordStoreChangeList* changes) OVERRIDE; virtual bool RemoveLogin(const autofill::PasswordForm& form) OVERRIDE; - virtual bool RemoveLoginsCreatedBetween( - const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE; + virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) OVERRIDE; + virtual bool RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) OVERRIDE; virtual bool GetLogins(const autofill::PasswordForm& form, PasswordFormList* forms) OVERRIDE; - virtual bool GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, + virtual bool GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) OVERRIDE; virtual bool GetAutofillableLogins(PasswordFormList* forms) OVERRIDE; virtual bool GetBlacklistLogins(PasswordFormList* forms) OVERRIDE; @@ -75,6 +79,11 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend { PERMANENT_FAIL // Init failed, and is not likely to work later either. }; + enum TimestampToCompare { + CREATION_TIMESTAMP, + SYNC_TIMESTAMP, + }; + // Initialization. bool StartKWalletd(); InitResult InitWallet(); @@ -92,11 +101,12 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend { bool autofillable, int wallet_handle); - // Reads PasswordForms from the wallet created in the given time range. + // Reads PasswordForms from the wallet created/synced in the given time range. bool GetLoginsList(PasswordFormList* forms, const base::Time& begin, const base::Time& end, - int wallet_handle); + int wallet_handle, + TimestampToCompare date_to_compare); // Helper for some of the above GetLoginsList() methods. bool GetAllLogins(PasswordFormList* forms, int wallet_handle); @@ -108,6 +118,13 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend { const std::string& signon_realm, int wallet_handle); + // Removes password created/synced in the time interval. Returns |true| if the + // operation succeeded. |changes| will contain the changes applied. + bool RemoveLoginsBetween(base::Time delete_begin, + base::Time delete_end, + TimestampToCompare date_to_compare, + password_manager::PasswordStoreChangeList* changes); + // Opens the wallet and ensures that the "Chrome Form Data" folder exists. // Returns kInvalidWalletHandle on error. int WalletHandle(); diff --git a/chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc b/chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc index 63db11b..1f1713d 100644 --- a/chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc +++ b/chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc @@ -171,12 +171,17 @@ class NativeBackendKWalletTestBase : public testing::Test { const PasswordForm& actual); static void CheckPasswordChanges(const PasswordStoreChangeList& expected, const PasswordStoreChangeList& actual); + static void CheckPasswordChangesWithResult( + const PasswordStoreChangeList* expected, + const PasswordStoreChangeList* actual, + bool result); PasswordForm old_form_google_; PasswordForm form_google_; PasswordForm form_isc_; }; +// static void NativeBackendKWalletTestBase::CheckPasswordForm( const PasswordForm& expected, const PasswordForm& actual) { EXPECT_EQ(expected.origin, actual.origin); @@ -197,6 +202,7 @@ void NativeBackendKWalletTestBase::CheckPasswordForm( EXPECT_EQ(expected.date_synced, actual.date_synced); } +// static void NativeBackendKWalletTestBase::CheckPasswordChanges( const PasswordStoreChangeList& expected, const PasswordStoreChangeList& actual) { @@ -207,6 +213,15 @@ void NativeBackendKWalletTestBase::CheckPasswordChanges( } } +// static +void NativeBackendKWalletTestBase::CheckPasswordChangesWithResult( + const PasswordStoreChangeList* expected, + const PasswordStoreChangeList* actual, + bool result) { + EXPECT_TRUE(result); + CheckPasswordChanges(*expected, *actual); +} + class NativeBackendKWalletTest : public NativeBackendKWalletTestBase { protected: NativeBackendKWalletTest() @@ -792,6 +807,73 @@ TEST_F(NativeBackendKWalletTest, ListLoginsAppends) { CheckPasswordForms("Chrome Form Data (42)", expected); } +TEST_F(NativeBackendKWalletTest, RemoveLoginsSyncedBetween) { + NativeBackendKWalletStub backend(42); + EXPECT_TRUE(backend.InitWithBus(mock_session_bus_)); + + base::Time now = base::Time::Now(); + base::Time next_day = now + base::TimeDelta::FromDays(1); + form_google_.date_synced = now; + form_isc_.date_synced = next_day; + form_google_.date_created = base::Time(); + form_isc_.date_created = base::Time(); + + BrowserThread::PostTask( + BrowserThread::DB, + FROM_HERE, + base::Bind(base::IgnoreResult(&NativeBackendKWalletStub::AddLogin), + base::Unretained(&backend), + form_google_)); + BrowserThread::PostTask( + BrowserThread::DB, + FROM_HERE, + base::Bind(base::IgnoreResult(&NativeBackendKWalletStub::AddLogin), + base::Unretained(&backend), + form_isc_)); + + PasswordStoreChangeList expected_changes; + expected_changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, form_google_)); + PasswordStoreChangeList changes; + BrowserThread::PostTaskAndReplyWithResult( + BrowserThread::DB, + FROM_HERE, + base::Bind(&NativeBackendKWalletStub::RemoveLoginsSyncedBetween, + base::Unretained(&backend), + base::Time(), + next_day, + &changes), + base::Bind(&NativeBackendKWalletTest::CheckPasswordChangesWithResult, + &expected_changes, + &changes)); + RunDBThread(); + + std::vector<const PasswordForm*> forms; + forms.push_back(&form_isc_); + ExpectationArray expected; + expected.push_back(make_pair(std::string(form_isc_.signon_realm), forms)); + CheckPasswordForms("Chrome Form Data (42)", expected); + + // Remove form_isc_. + expected_changes.clear(); + expected_changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, form_isc_)); + BrowserThread::PostTaskAndReplyWithResult( + BrowserThread::DB, + FROM_HERE, + base::Bind(&NativeBackendKWalletStub::RemoveLoginsSyncedBetween, + base::Unretained(&backend), + next_day, + base::Time(), + &changes), + base::Bind(&NativeBackendKWalletTest::CheckPasswordChangesWithResult, + &expected_changes, + &changes)); + RunDBThread(); + + CheckPasswordForms("Chrome Form Data (42)", ExpectationArray()); +} + // TODO(mdm): add more basic tests here at some point. // (For example tests for storing >1 password per realm pickle.) diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc index c46c5d8..0c006db 100644 --- a/chrome/browser/password_manager/password_store_mac.cc +++ b/chrome/browser/password_manager/password_store_mac.cc @@ -965,6 +965,40 @@ PasswordStoreChangeList PasswordStoreMac::RemoveLoginsCreatedBetweenImpl( return changes; } +PasswordStoreChangeList PasswordStoreMac::RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) { + PasswordStoreChangeList changes; + std::vector<PasswordForm*> forms; + if (login_metadata_db_->GetLoginsSyncedBetween( + delete_begin, delete_end, &forms)) { + if (login_metadata_db_->RemoveLoginsSyncedBetween(delete_begin, + delete_end)) { + // We can't delete from the Keychain by date because we may be sharing + // items with database entries that weren't in the delete range. Instead, + // we find all the Keychain items we own but aren't using any more and + // delete those. + std::vector<PasswordForm*> orphan_keychain_forms = + GetUnusedKeychainForms(); + // This is inefficient, since we have to re-look-up each keychain item + // one at a time to delete it even though the search step already had a + // list of Keychain item references. If this turns out to be noticeably + // slow we'll need to rearchitect to allow the search and deletion steps + // to share. + RemoveKeychainForms(orphan_keychain_forms); + STLDeleteElements(&orphan_keychain_forms); + + for (std::vector<PasswordForm*>::const_iterator it = forms.begin(); + it != forms.end(); + ++it) { + changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, **it)); + } + } + } + return changes; +} + void PasswordStoreMac::GetLoginsImpl( const autofill::PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/chrome/browser/password_manager/password_store_mac.h b/chrome/browser/password_manager/password_store_mac.h index 87cc61b..fd1d4be 100644 --- a/chrome/browser/password_manager/password_store_mac.h +++ b/chrome/browser/password_manager/password_store_mac.h @@ -60,6 +60,9 @@ class PasswordStoreMac : public password_manager::PasswordStore { virtual password_manager::PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl(const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE; + virtual password_manager::PasswordStoreChangeList + RemoveLoginsSyncedBetweenImpl(base::Time delete_begin, + base::Time delete_end) OVERRIDE; virtual void GetLoginsImpl( const autofill::PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/chrome/browser/password_manager/password_store_x.cc b/chrome/browser/password_manager/password_store_x.cc index 8a6fc74..2fbe63c 100644 --- a/chrome/browser/password_manager/password_store_x.cc +++ b/chrome/browser/password_manager/password_store_x.cc @@ -110,6 +110,21 @@ PasswordStoreChangeList PasswordStoreX::RemoveLoginsCreatedBetweenImpl( return changes; } +PasswordStoreChangeList PasswordStoreX::RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) { + CheckMigration(); + PasswordStoreChangeList changes; + if (use_native_backend() && + backend_->RemoveLoginsSyncedBetween(delete_begin, delete_end, &changes)) { + allow_fallback_ = false; + } else if (allow_default_store()) { + changes = PasswordStoreDefault::RemoveLoginsSyncedBetweenImpl(delete_begin, + delete_end); + } + return changes; +} + namespace { struct LoginLessThan { bool operator()(const PasswordForm* a, const PasswordForm* b) { diff --git a/chrome/browser/password_manager/password_store_x.h b/chrome/browser/password_manager/password_store_x.h index 585f64d..edc2c4e 100644 --- a/chrome/browser/password_manager/password_store_x.h +++ b/chrome/browser/password_manager/password_store_x.h @@ -46,12 +46,21 @@ class PasswordStoreX : public password_manager::PasswordStoreDefault { const autofill::PasswordForm& form, password_manager::PasswordStoreChangeList* changes) = 0; virtual bool RemoveLogin(const autofill::PasswordForm& form) = 0; - virtual bool RemoveLoginsCreatedBetween(const base::Time& delete_begin, - const base::Time& delete_end) = 0; + + // Removes all logins created/synced from |delete_begin| onwards (inclusive) + // and before |delete_end|. You may use a null Time value to do an unbounded + // delete in either direction. + virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) = 0; + virtual bool RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) = 0; + virtual bool GetLogins(const autofill::PasswordForm& form, PasswordFormList* forms) = 0; - virtual bool GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, + virtual bool GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) = 0; virtual bool GetAutofillableLogins(PasswordFormList* forms) = 0; virtual bool GetBlacklistLogins(PasswordFormList* forms) = 0; @@ -79,6 +88,9 @@ class PasswordStoreX : public password_manager::PasswordStoreDefault { virtual password_manager::PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl(const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE; + virtual password_manager::PasswordStoreChangeList + RemoveLoginsSyncedBetweenImpl(base::Time delete_begin, + base::Time delete_end) OVERRIDE; virtual void GetLoginsImpl( const autofill::PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/chrome/browser/password_manager/password_store_x_unittest.cc b/chrome/browser/password_manager/password_store_x_unittest.cc index 3882ba0..4a67b5d 100644 --- a/chrome/browser/password_manager/password_store_x_unittest.cc +++ b/chrome/browser/password_manager/password_store_x_unittest.cc @@ -68,9 +68,15 @@ class FailingBackend : public PasswordStoreX::NativeBackend { } virtual bool RemoveLogin(const PasswordForm& form) OVERRIDE { return false; } - virtual bool RemoveLoginsCreatedBetween( - const base::Time& delete_begin, - const base::Time& delete_end) OVERRIDE { + virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) OVERRIDE { + return false; + } + + virtual bool RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) OVERRIDE { return false; } @@ -79,8 +85,8 @@ class FailingBackend : public PasswordStoreX::NativeBackend { return false; } - virtual bool GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, + virtual bool GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) OVERRIDE { return false; } @@ -121,9 +127,8 @@ class MockBackend : public PasswordStoreX::NativeBackend { return true; } - virtual bool RemoveLoginsCreatedBetween( - const base::Time& delete_begin, - const base::Time& delete_end) OVERRIDE { + virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin, + base::Time delete_end) OVERRIDE { for (size_t i = 0; i < all_forms_.size(); ++i) { if (delete_begin <= all_forms_[i].date_created && (delete_end.is_null() || all_forms_[i].date_created < delete_end)) @@ -132,6 +137,22 @@ class MockBackend : public PasswordStoreX::NativeBackend { return true; } + virtual bool RemoveLoginsSyncedBetween( + base::Time delete_begin, + base::Time delete_end, + password_manager::PasswordStoreChangeList* changes) OVERRIDE { + DCHECK(changes); + for (size_t i = 0; i < all_forms_.size(); ++i) { + if (delete_begin <= all_forms_[i].date_synced && + (delete_end.is_null() || all_forms_[i].date_synced < delete_end)) { + changes->push_back(password_manager::PasswordStoreChange( + password_manager::PasswordStoreChange::REMOVE, all_forms_[i])); + erase(i--); + } + } + return true; + } + virtual bool GetLogins(const PasswordForm& form, PasswordFormList* forms) OVERRIDE { for (size_t i = 0; i < all_forms_.size(); ++i) @@ -140,8 +161,8 @@ class MockBackend : public PasswordStoreX::NativeBackend { return true; } - virtual bool GetLoginsCreatedBetween(const base::Time& get_begin, - const base::Time& get_end, + virtual bool GetLoginsCreatedBetween(base::Time get_begin, + base::Time get_end, PasswordFormList* forms) OVERRIDE { for (size_t i = 0; i < all_forms_.size(); ++i) if (get_begin <= all_forms_[i].date_created && diff --git a/chrome/browser/sync/test/integration/passwords_helper.cc b/chrome/browser/sync/test/integration/passwords_helper.cc index 6a3fb0f..0dee0eb 100644 --- a/chrome/browser/sync/test/integration/passwords_helper.cc +++ b/chrome/browser/sync/test/integration/passwords_helper.cc @@ -64,6 +64,16 @@ class PasswordStoreConsumerHelper DISALLOW_COPY_AND_ASSIGN(PasswordStoreConsumerHelper); }; +// PasswordForm::date_synced is a local field. Therefore it may be different +// across clients. +void ClearSyncDateField(std::vector<PasswordForm>* forms) { + for (std::vector<PasswordForm>::iterator it = forms->begin(); + it != forms->end(); + ++it) { + it->date_synced = base::Time(); + } +} + } // namespace namespace passwords_helper { @@ -137,6 +147,7 @@ bool ProfileContainsSamePasswordFormsAsVerifier(int index) { std::vector<PasswordForm> forms; GetLogins(GetVerifierPasswordStore(), verifier_forms); GetLogins(GetPasswordStore(index), forms); + ClearSyncDateField(&forms); bool result = password_manager::ContainsSamePasswordForms(verifier_forms, forms); if (!result) { @@ -159,6 +170,8 @@ bool ProfilesContainSamePasswordForms(int index_a, int index_b) { std::vector<PasswordForm> forms_b; GetLogins(GetPasswordStore(index_a), forms_a); GetLogins(GetPasswordStore(index_b), forms_b); + ClearSyncDateField(&forms_a); + ClearSyncDateField(&forms_b); bool result = password_manager::ContainsSamePasswordForms(forms_a, forms_b); if (!result) { LOG(ERROR) << "Password forms in Profile" << index_a << ":"; diff --git a/components/password_manager/core/browser/login_database.cc b/components/password_manager/core/browser/login_database.cc index 5bb071f..90526e2 100644 --- a/components/password_manager/core/browser/login_database.cc +++ b/components/password_manager/core/browser/login_database.cc @@ -354,19 +354,20 @@ PasswordStoreChangeList LoginDatabase::UpdateLogin(const PasswordForm& form) { // Replacement is necessary to deal with updating imported credentials. See // crbug.com/349138 for details. sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE, - "UPDATE OR REPLACE logins SET " - "action_url = ?, " - "password_value = ?, " - "ssl_valid = ?, " - "preferred = ?, " - "possible_usernames = ?, " - "times_used = ?, " - "submit_element = ? " - "WHERE origin_url = ? AND " - "username_element = ? AND " - "username_value = ? AND " - "password_element = ? AND " - "signon_realm = ?")); + "UPDATE OR REPLACE logins SET " + "action_url = ?, " + "password_value = ?, " + "ssl_valid = ?, " + "preferred = ?, " + "possible_usernames = ?, " + "times_used = ?, " + "submit_element = ?, " + "date_synced = ? " + "WHERE origin_url = ? AND " + "username_element = ? AND " + "username_value = ? AND " + "password_element = ? AND " + "signon_realm = ?")); s.BindString(0, form.action.spec()); s.BindBlob(1, encrypted_password.data(), static_cast<int>(encrypted_password.length())); @@ -376,12 +377,13 @@ PasswordStoreChangeList LoginDatabase::UpdateLogin(const PasswordForm& form) { s.BindBlob(4, pickle.data(), pickle.size()); s.BindInt(5, form.times_used); s.BindString16(6, form.submit_element); + s.BindInt64(7, form.date_synced.ToInternalValue()); - s.BindString(7, form.origin.spec()); - s.BindString16(8, form.username_element); - s.BindString16(9, form.username_value); - s.BindString16(10, form.password_element); - s.BindString(11, form.signon_realm); + s.BindString(8, form.origin.spec()); + s.BindString16(9, form.username_element); + s.BindString16(10, form.username_value); + s.BindString16(11, form.password_element); + s.BindString(12, form.signon_realm); if (!s.Run()) return PasswordStoreChangeList(); @@ -425,6 +427,19 @@ bool LoginDatabase::RemoveLoginsCreatedBetween(const base::Time delete_begin, return s.Run(); } +bool LoginDatabase::RemoveLoginsSyncedBetween(base::Time delete_begin, + base::Time delete_end) { + sql::Statement s(db_.GetCachedStatement( + SQL_FROM_HERE, + "DELETE FROM logins WHERE date_synced >= ? AND date_synced < ?")); + s.BindInt64(0, delete_begin.ToInternalValue()); + s.BindInt64(1, + delete_end.is_null() ? base::Time::Max().ToInternalValue() + : delete_end.ToInternalValue()); + + return s.Run(); +} + LoginDatabase::EncryptionResult LoginDatabase::InitPasswordFormFromStatement( PasswordForm* form, sql::Statement& s) const { @@ -603,6 +618,38 @@ bool LoginDatabase::GetLoginsCreatedBetween( return s.Succeeded(); } +bool LoginDatabase::GetLoginsSyncedBetween( + const base::Time begin, + const base::Time end, + std::vector<autofill::PasswordForm*>* forms) const { + DCHECK(forms); + sql::Statement s(db_.GetCachedStatement( + SQL_FROM_HERE, + "SELECT origin_url, action_url, username_element, username_value, " + "password_element, password_value, submit_element, signon_realm, " + "ssl_valid, preferred, date_created, blacklisted_by_user, " + "scheme, password_type, possible_usernames, times_used, form_data, " + "use_additional_auth, date_synced FROM logins " + "WHERE date_synced >= ? AND date_synced < ?" + "ORDER BY origin_url")); + s.BindInt64(0, begin.ToInternalValue()); + s.BindInt64(1, + end.is_null() ? base::Time::Max().ToInternalValue() + : end.ToInternalValue()); + + while (s.Step()) { + scoped_ptr<PasswordForm> new_form(new PasswordForm()); + EncryptionResult result = InitPasswordFormFromStatement(new_form.get(), s); + if (result == ENCRYPTION_RESULT_SERVICE_FAILURE) + return false; + if (result == ENCRYPTION_RESULT_ITEM_FAILURE) + continue; + DCHECK(result == ENCRYPTION_RESULT_SUCCESS); + forms->push_back(new_form.release()); + } + return s.Succeeded(); +} + bool LoginDatabase::GetAutofillableLogins( std::vector<PasswordForm*>* forms) const { return GetAllLoginsWithBlacklistSetting(false, forms); diff --git a/components/password_manager/core/browser/login_database.h b/components/password_manager/core/browser/login_database.h index 8fcf846..bfcf495 100644 --- a/components/password_manager/core/browser/login_database.h +++ b/components/password_manager/core/browser/login_database.h @@ -54,6 +54,12 @@ class LoginDatabase { bool RemoveLoginsCreatedBetween(const base::Time delete_begin, const base::Time delete_end); + // Removes all logins synced from |delete_begin| onwards (inclusive) and + // before |delete_end|. You may use a null Time value to do an unbounded + // delete in either direction. + bool RemoveLoginsSyncedBetween(base::Time delete_begin, + base::Time delete_end); + // Loads a list of matching password forms into the specified vector |forms|. // The list will contain all possibly relevant entries to the observed |form|, // including blacklisted matches. @@ -64,8 +70,16 @@ class LoginDatabase { // You may use a null Time value to do an unbounded search in either // direction. bool GetLoginsCreatedBetween( - const base::Time begin, - const base::Time end, + base::Time begin, + base::Time end, + std::vector<autofill::PasswordForm*>* forms) const; + + // Loads all logins synced from |begin| onwards (inclusive) and before |end|. + // You may use a null Time value to do an unbounded search in either + // direction. + bool GetLoginsSyncedBetween( + base::Time begin, + base::Time end, std::vector<autofill::PasswordForm*>* forms) const; // Loads the complete list of autofillable password forms (i.e., not blacklist diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc index 8cef06e..b1093c9 100644 --- a/components/password_manager/core/browser/login_database_unittest.cc +++ b/components/password_manager/core/browser/login_database_unittest.cc @@ -577,9 +577,11 @@ TEST_F(LoginDatabaseTest, TestPublicSuffixDomainMatchingRegexp) { EXPECT_EQ(0U, result.size()); } -static bool AddTimestampedLogin(LoginDatabase* db, std::string url, +static bool AddTimestampedLogin(LoginDatabase* db, + std::string url, const std::string& unique_string, - const base::Time& time) { + const base::Time& time, + bool date_is_creation) { // Example password form. PasswordForm form; form.origin = GURL(url + std::string("/LoginAuth")); @@ -588,7 +590,10 @@ static bool AddTimestampedLogin(LoginDatabase* db, std::string url, form.password_element = ASCIIToUTF16(unique_string); form.submit_element = ASCIIToUTF16("signIn"); form.signon_realm = url; - form.date_created = time; + if (date_is_creation) + form.date_created = time; + else + form.date_synced = time; return db->AddLogin(form) == AddChangeForForm(form); } @@ -610,11 +615,11 @@ TEST_F(LoginDatabaseTest, ClearPrivateData_SavedPasswords) { base::TimeDelta one_day = base::TimeDelta::FromDays(1); // Create one with a 0 time. - EXPECT_TRUE(AddTimestampedLogin(&db_, "1", "foo1", base::Time())); + EXPECT_TRUE(AddTimestampedLogin(&db_, "1", "foo1", base::Time(), true)); // Create one for now and +/- 1 day. - EXPECT_TRUE(AddTimestampedLogin(&db_, "2", "foo2", now - one_day)); - EXPECT_TRUE(AddTimestampedLogin(&db_, "3", "foo3", now)); - EXPECT_TRUE(AddTimestampedLogin(&db_, "4", "foo4", now + one_day)); + EXPECT_TRUE(AddTimestampedLogin(&db_, "2", "foo2", now - one_day, true)); + EXPECT_TRUE(AddTimestampedLogin(&db_, "3", "foo3", now, true)); + EXPECT_TRUE(AddTimestampedLogin(&db_, "4", "foo4", now + one_day, true)); // Verify inserts worked. EXPECT_TRUE(db_.GetAutofillableLogins(&result)); @@ -642,6 +647,49 @@ TEST_F(LoginDatabaseTest, ClearPrivateData_SavedPasswords) { EXPECT_EQ(0U, result.size()); } +TEST_F(LoginDatabaseTest, RemoveLoginsSyncedBetween) { + ScopedVector<autofill::PasswordForm> result; + + base::Time now = base::Time::Now(); + base::TimeDelta one_day = base::TimeDelta::FromDays(1); + + // Create one with a 0 time. + EXPECT_TRUE(AddTimestampedLogin(&db_, "1", "foo1", base::Time(), false)); + // Create one for now and +/- 1 day. + EXPECT_TRUE(AddTimestampedLogin(&db_, "2", "foo2", now - one_day, false)); + EXPECT_TRUE(AddTimestampedLogin(&db_, "3", "foo3", now, false)); + EXPECT_TRUE(AddTimestampedLogin(&db_, "4", "foo4", now + one_day, false)); + + // Verify inserts worked. + EXPECT_TRUE(db_.GetAutofillableLogins(&result.get())); + EXPECT_EQ(4U, result.size()); + result.clear(); + + // Get everything from today's date and on. + EXPECT_TRUE(db_.GetLoginsSyncedBetween(now, base::Time(), &result.get())); + ASSERT_EQ(2U, result.size()); + EXPECT_EQ("3", result[0]->signon_realm); + EXPECT_EQ("4", result[1]->signon_realm); + result.clear(); + + // Delete everything from today's date and on. + db_.RemoveLoginsSyncedBetween(now, base::Time()); + + // Should have deleted half of what we inserted. + EXPECT_TRUE(db_.GetAutofillableLogins(&result.get())); + ASSERT_EQ(2U, result.size()); + EXPECT_EQ("1", result[0]->signon_realm); + EXPECT_EQ("2", result[1]->signon_realm); + result.clear(); + + // Delete with 0 date (should delete all). + db_.RemoveLoginsSyncedBetween(base::Time(), now); + + // Verify nothing is left. + EXPECT_TRUE(db_.GetAutofillableLogins(&result.get())); + EXPECT_EQ(0U, result.size()); +} + TEST_F(LoginDatabaseTest, BlacklistedLogins) { std::vector<PasswordForm*> result; @@ -814,6 +862,7 @@ TEST_F(LoginDatabaseTest, UpdateOverlappingCredentials) { // Simulate the user changing their password. complete_form.password_value = ASCIIToUTF16("new_password"); + complete_form.date_synced = base::Time::Now(); EXPECT_EQ(UpdateChangeForForm(complete_form), db_.UpdateLogin(complete_form)); // Both still exist now. diff --git a/components/password_manager/core/browser/mock_password_store.h b/components/password_manager/core/browser/mock_password_store.h index 8fc8794..0c5da0c 100644 --- a/components/password_manager/core/browser/mock_password_store.h +++ b/components/password_manager/core/browser/mock_password_store.h @@ -36,6 +36,8 @@ class MockPasswordStore : public PasswordStore { PasswordStoreChangeList(const autofill::PasswordForm&)); MOCK_METHOD2(RemoveLoginsCreatedBetweenImpl, PasswordStoreChangeList(const base::Time&, const base::Time&)); + MOCK_METHOD2(RemoveLoginsSyncedBetweenImpl, + PasswordStoreChangeList(base::Time, base::Time)); MOCK_METHOD3(GetLoginsImpl, void(const autofill::PasswordForm& form, PasswordStore::AuthorizationPromptPolicy prompt_policy, diff --git a/components/password_manager/core/browser/password_store.cc b/components/password_manager/core/browser/password_store.cc index 0dd5684..3af03003 100644 --- a/components/password_manager/core/browser/password_store.cc +++ b/components/password_manager/core/browser/password_store.cc @@ -112,6 +112,17 @@ void PasswordStore::RemoveLoginsCreatedBetween(const base::Time& delete_begin, this, delete_begin, delete_end))); } +void PasswordStore::RemoveLoginsSyncedBetween(base::Time delete_begin, + base::Time delete_end) { + ScheduleTask( + base::Bind(&PasswordStore::WrapModificationTask, + this, + base::Bind(&PasswordStore::RemoveLoginsSyncedBetweenImpl, + this, + delete_begin, + delete_end))); +} + void PasswordStore::GetLogins( const PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/components/password_manager/core/browser/password_store.h b/components/password_manager/core/browser/password_store.h index 6bb063d..f4fd83e 100644 --- a/components/password_manager/core/browser/password_store.h +++ b/components/password_manager/core/browser/password_store.h @@ -142,6 +142,10 @@ class PasswordStore : protected PasswordStoreSync, virtual void RemoveLoginsCreatedBetween(const base::Time& delete_begin, const base::Time& delete_end); + // Removes all logins synced in the given date range. + virtual void RemoveLoginsSyncedBetween(base::Time delete_begin, + base::Time delete_end); + // Searches for a matching PasswordForm, and notifies |consumer| on // completion. The request will be cancelled if the consumer is destroyed. // |prompt_policy| indicates whether it's permissible to prompt the user to @@ -215,6 +219,11 @@ class PasswordStore : protected PasswordStoreSync, virtual PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl( const base::Time& delete_begin, const base::Time& delete_end) = 0; + // Synchronous implementation to remove the given logins. + virtual PasswordStoreChangeList RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) = 0; + typedef base::Callback<void(const std::vector<autofill::PasswordForm*>&)> ConsumerCallbackRunner; // Owns all PasswordForms in the vector. diff --git a/components/password_manager/core/browser/password_store_default.cc b/components/password_manager/core/browser/password_store_default.cc index 92cab5a..b14e185 100644 --- a/components/password_manager/core/browser/password_store_default.cc +++ b/components/password_manager/core/browser/password_store_default.cc @@ -70,6 +70,25 @@ PasswordStoreChangeList PasswordStoreDefault::RemoveLoginsCreatedBetweenImpl( return changes; } +PasswordStoreChangeList PasswordStoreDefault::RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) { + std::vector<PasswordForm*> forms; + PasswordStoreChangeList changes; + if (login_db_->GetLoginsSyncedBetween(delete_begin, delete_end, &forms)) { + if (login_db_->RemoveLoginsSyncedBetween(delete_begin, delete_end)) { + for (std::vector<PasswordForm*>::const_iterator it = forms.begin(); + it != forms.end(); + ++it) { + changes.push_back( + PasswordStoreChange(PasswordStoreChange::REMOVE, **it)); + } + } + } + STLDeleteElements(&forms); + return changes; +} + void PasswordStoreDefault::GetLoginsImpl( const autofill::PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/components/password_manager/core/browser/password_store_default.h b/components/password_manager/core/browser/password_store_default.h index 766f38d..a2a14d4 100644 --- a/components/password_manager/core/browser/password_store_default.h +++ b/components/password_manager/core/browser/password_store_default.h @@ -36,6 +36,9 @@ class PasswordStoreDefault : public PasswordStore { const autofill::PasswordForm& form) OVERRIDE; virtual PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl( const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE; + virtual PasswordStoreChangeList RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) OVERRIDE; virtual void GetLoginsImpl( const autofill::PasswordForm& form, AuthorizationPromptPolicy prompt_policy, diff --git a/components/password_manager/core/browser/password_syncable_service.cc b/components/password_manager/core/browser/password_syncable_service.cc index 61d1d40..8cf3817 100644 --- a/components/password_manager/core/browser/password_syncable_service.cc +++ b/components/password_manager/core/browser/password_syncable_service.cc @@ -221,6 +221,7 @@ syncer::SyncError PasswordSyncableService::ProcessSyncChanges( ScopedVector<autofill::PasswordForm> new_sync_entries; ScopedVector<autofill::PasswordForm> updated_sync_entries; ScopedVector<autofill::PasswordForm> deleted_entries; + base::Time time_now = base::Time::Now(); for (syncer::SyncChangeList::const_iterator it = change_list.begin(); it != change_list.end(); @@ -231,10 +232,12 @@ syncer::SyncError PasswordSyncableService::ProcessSyncChanges( form.get()); switch (it->change_type()) { case syncer::SyncChange::ACTION_ADD: { + form->date_synced = time_now; new_sync_entries.push_back(form.release()); break; } case syncer::SyncChange::ACTION_UPDATE: { + form->date_synced = time_now; updated_sync_entries.push_back(form.release()); break; } @@ -368,10 +371,12 @@ void PasswordSyncableService::CreateOrUpdateEntry( // Check whether the data from sync is already in the password store. PasswordEntryMap::iterator existing_local_entry_iter = umatched_data_from_password_db->find(tag); + base::Time time_now = base::Time::Now(); if (existing_local_entry_iter == umatched_data_from_password_db->end()) { // The sync data is not in the password store, so we need to create it in // the password store. Add the entry to the new_entries list. scoped_ptr<autofill::PasswordForm> new_password(new autofill::PasswordForm); + new_password->date_synced = time_now; PasswordFromSpecifics(password_specifics, new_password.get()); new_sync_entries->push_back(new_password.release()); } else { @@ -384,6 +389,7 @@ void PasswordSyncableService::CreateOrUpdateEntry( case IDENTICAL: break; case SYNC: + new_password->date_synced = time_now; updated_sync_entries->push_back(new_password.release()); break; case LOCAL: diff --git a/components/password_manager/core/browser/password_syncable_service_unittest.cc b/components/password_manager/core/browser/password_syncable_service_unittest.cc index b64dcb4..684cfdc 100644 --- a/components/password_manager/core/browser/password_syncable_service_unittest.cc +++ b/components/password_manager/core/browser/password_syncable_service_unittest.cc @@ -82,6 +82,25 @@ SyncChange CreateSyncChange(const autofill::PasswordForm& password, return SyncChange(FROM_HERE, type, data); } +class FormFinder { + public: + explicit FormFinder(const autofill::PasswordForm& form) : form_(form) {} + ~FormFinder() {} + + bool operator()(const autofill::PasswordForm& form) const; + + private: + const autofill::PasswordForm form_; +}; + +bool FormFinder::operator()(const autofill::PasswordForm& form) const { + return form.origin == form_.origin && + form.username_element == form_.username_element && + form.username_value == form_.username_value && + form.password_element == form_.password_element && + form.signon_realm == form_.signon_realm; +} + // A testable implementation of the |PasswordSyncableService| that mocks // out all interaction with the password database. class MockPasswordSyncableService : public PasswordSyncableService { @@ -267,9 +286,15 @@ PasswordStoreChangeList PasswordStoreDataVerifier::VerifyChange( PasswordStoreChange::Type type, const autofill::PasswordForm& password, std::vector<autofill::PasswordForm>* password_list) { - std::vector<autofill::PasswordForm>::iterator it = - std::find(password_list->begin(), password_list->end(), password); + std::vector<autofill::PasswordForm>::iterator it = std::find_if( + password_list->begin(), password_list->end(), FormFinder(password)); EXPECT_NE(password_list->end(), it); + PasswordsEqual(GetPasswordSpecifics(SyncDataFromPassword(*it)), + GetPasswordSpecifics(SyncDataFromPassword(password))); + if (type != PasswordStoreChange::REMOVE) { + EXPECT_FALSE(password.date_synced.is_null()) << password.signon_realm; + EXPECT_FALSE(password.date_synced.is_max()) << password.signon_realm; + } password_list->erase(it); return PasswordStoreChangeList(1, PasswordStoreChange(type, password)); } diff --git a/components/password_manager/core/browser/test_password_store.cc b/components/password_manager/core/browser/test_password_store.cc index 555eb70..3a524a8 100644 --- a/components/password_manager/core/browser/test_password_store.cc +++ b/components/password_manager/core/browser/test_password_store.cc @@ -111,6 +111,13 @@ PasswordStoreChangeList TestPasswordStore::RemoveLoginsCreatedBetweenImpl( return changes; } +PasswordStoreChangeList TestPasswordStore::RemoveLoginsSyncedBetweenImpl( + base::Time begin, + base::Time end) { + PasswordStoreChangeList changes; + return changes; +} + bool TestPasswordStore::FillAutofillableLogins( std::vector<autofill::PasswordForm*>* forms) { return true; diff --git a/components/password_manager/core/browser/test_password_store.h b/components/password_manager/core/browser/test_password_store.h index fb0b477..de9fb20 100644 --- a/components/password_manager/core/browser/test_password_store.h +++ b/components/password_manager/core/browser/test_password_store.h @@ -61,6 +61,9 @@ class TestPasswordStore : public PasswordStore { virtual void ReportMetricsImpl() OVERRIDE {} virtual PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl( const base::Time& begin, const base::Time& end) OVERRIDE; + virtual PasswordStoreChangeList RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) OVERRIDE; virtual void GetAutofillableLoginsImpl( PasswordStore::GetLoginsRequest* request) OVERRIDE {} virtual void GetBlacklistLoginsImpl( |