diff options
Diffstat (limited to 'chrome/browser/password_manager')
4 files changed, 132 insertions, 29 deletions
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc index 97f9aa0..d5327be 100644 --- a/chrome/browser/password_manager/password_store_mac.cc +++ b/chrome/browser/password_manager/password_store_mac.cc @@ -472,13 +472,7 @@ std::vector<PasswordForm*> MatchingKeychainItems(query_form.signon_realm, query_form.scheme, NULL, NULL); - std::vector<PasswordForm*> keychain_forms = - CreateFormsFromKeychainItems(keychain_items); - for (std::vector<SecKeychainItemRef>::iterator i = keychain_items.begin(); - i != keychain_items.end(); ++i) { - keychain_->Free(*i); - } - return keychain_forms; + return ConvertKeychainItemsToForms(&keychain_items); } std::vector<PasswordForm*> @@ -489,13 +483,7 @@ std::vector<PasswordForm*> MatchingKeychainItems(query_form.signon_realm, query_form.scheme, NULL, username.c_str()); - std::vector<PasswordForm*> keychain_forms = - CreateFormsFromKeychainItems(keychain_items); - for (std::vector<SecKeychainItemRef>::iterator i = keychain_items.begin(); - i != keychain_items.end(); ++i) { - keychain_->Free(*i); - } - return keychain_forms; + return ConvertKeychainItemsToForms(&keychain_items); } PasswordForm* MacKeychainPasswordFormAdapter::PasswordExactlyMatchingForm( @@ -512,6 +500,26 @@ PasswordForm* MacKeychainPasswordFormAdapter::PasswordExactlyMatchingForm( return NULL; } +std::vector<PasswordForm*> + MacKeychainPasswordFormAdapter::GetAllPasswordFormPasswords() { + SecAuthenticationType supported_auth_types[] = { + kSecAuthenticationTypeHTMLForm, + kSecAuthenticationTypeHTTPBasic, + kSecAuthenticationTypeHTTPDigest, + }; + OSType creator = finds_only_owned_ ? kChromeKeychainCreatorCode : 0; + + std::vector<SecKeychainItemRef> matches; + for (unsigned int i = 0; i < arraysize(supported_auth_types); ++i) { + KeychainSearch keychain_search(*keychain_); + keychain_search.Init(NULL, 0, kSecProtocolTypeAny, supported_auth_types[i], + NULL, NULL, NULL, creator); + keychain_search.FindMatchingItems(&matches); + } + + return ConvertKeychainItemsToForms(&matches); +} + bool MacKeychainPasswordFormAdapter::AddPassword(const PasswordForm& form) { // We should never be trying to store a blacklist in the keychain. DCHECK(!form.blacklisted_by_user); @@ -570,17 +578,19 @@ void MacKeychainPasswordFormAdapter::SetFindsOnlyOwnedItems( } std::vector<PasswordForm*> - MacKeychainPasswordFormAdapter::CreateFormsFromKeychainItems( - const std::vector<SecKeychainItemRef>& items) { + MacKeychainPasswordFormAdapter::ConvertKeychainItemsToForms( + std::vector<SecKeychainItemRef>* items) { std::vector<PasswordForm*> keychain_forms; - for (std::vector<SecKeychainItemRef>::const_iterator i = items.begin(); - i != items.end(); ++i) { + for (std::vector<SecKeychainItemRef>::const_iterator i = items->begin(); + i != items->end(); ++i) { PasswordForm* form = new PasswordForm(); if (internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_, *i, form)) { keychain_forms.push_back(form); } + keychain_->Free(*i); } + items->clear(); return keychain_forms; } @@ -750,7 +760,18 @@ void PasswordStoreMac::RemoveLoginImpl(const PasswordForm& form) { void PasswordStoreMac::RemoveLoginsCreatedBetweenImpl( const base::Time& delete_begin, const base::Time& delete_end) { - NOTIMPLEMENTED(); + login_metadata_db_->RemoveLoginsCreatedBetween(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); } void PasswordStoreMac::GetLoginsImpl(GetLoginsRequest* request, @@ -838,6 +859,27 @@ bool PasswordStoreMac::DatabaseHasFormMatchingKeychainForm( return has_match; } +std::vector<PasswordForm*> PasswordStoreMac::GetUnusedKeychainForms() { + std::vector<PasswordForm*> database_forms; + login_metadata_db_->GetAllLogins(&database_forms, false); + + MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get()); + owned_keychain_adapter.SetFindsOnlyOwnedItems(true); + std::vector<PasswordForm*> owned_keychain_forms = + owned_keychain_adapter.GetAllPasswordFormPasswords(); + + // Run a merge; anything left in owned_keychain_forms when we are done no + // longer has a matching database entry. + std::vector<PasswordForm*> merged_forms; + internal_keychain_helpers::MergePasswordForms(&owned_keychain_forms, + &database_forms, + &merged_forms); + STLDeleteElements(&merged_forms); + STLDeleteElements(&database_forms); + + return owned_keychain_forms; +} + void PasswordStoreMac::RemoveDatabaseForms( const std::vector<PasswordForm*>& forms) { for (std::vector<PasswordForm*>::const_iterator i = forms.begin(); @@ -845,3 +887,13 @@ void PasswordStoreMac::RemoveDatabaseForms( login_metadata_db_->RemoveLogin(**i); } } + +void PasswordStoreMac::RemoveKeychainForms( + const std::vector<PasswordForm*>& forms) { + MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get()); + owned_keychain_adapter.SetFindsOnlyOwnedItems(true); + for (std::vector<PasswordForm*>::const_iterator i = forms.begin(); + i != forms.end(); ++i) { + owned_keychain_adapter.RemovePassword(**i); + } +} diff --git a/chrome/browser/password_manager/password_store_mac.h b/chrome/browser/password_manager/password_store_mac.h index 1ea24e9..7874c24 100644 --- a/chrome/browser/password_manager/password_store_mac.h +++ b/chrome/browser/password_manager/password_store_mac.h @@ -13,6 +13,11 @@ class LoginDatabaseMac; class MacKeychain; +// Implements PasswordStore on top of the OS X Keychain, with an internal +// database for extra metadata. For an overview of the interactions with the +// Keychain, as well as the rationale for some of the behaviors, see the +// Keychain integration design doc: +// http://dev.chromium.org/developers/design-documents/os-x-password-manager-keychain-integration class PasswordStoreMac : public PasswordStore { public: // Takes ownership of |keychain| and |login_db|, both of which must be @@ -41,10 +46,19 @@ class PasswordStoreMac : public PasswordStore { bool DatabaseHasFormMatchingKeychainForm( const webkit_glue::PasswordForm& form); + // Returns all the Keychain entries that we own but no longer have + // corresponding metadata for in our database. + // Caller is responsible for deleting the forms. + std::vector<webkit_glue::PasswordForm*> GetUnusedKeychainForms(); + // Removes the given forms from the database. void RemoveDatabaseForms( const std::vector<webkit_glue::PasswordForm*>& forms); + // Removes the given forms from the Keychain. + void RemoveKeychainForms( + const std::vector<webkit_glue::PasswordForm*>& forms); + scoped_ptr<MacKeychain> keychain_; scoped_ptr<LoginDatabaseMac> login_metadata_db_; diff --git a/chrome/browser/password_manager/password_store_mac_internal.h b/chrome/browser/password_manager/password_store_mac_internal.h index ba6e07b..59dd2f2 100644 --- a/chrome/browser/password_manager/password_store_mac_internal.h +++ b/chrome/browser/password_manager/password_store_mac_internal.h @@ -40,6 +40,9 @@ class MacKeychainPasswordFormAdapter { webkit_glue::PasswordForm* PasswordExactlyMatchingForm( const webkit_glue::PasswordForm& query_form); + // Returns all keychain items of types corresponding to password forms. + std::vector<webkit_glue::PasswordForm*> GetAllPasswordFormPasswords(); + // Creates a new keychain entry from |form|, or updates the password of an // existing keychain entry if there is a collision. Returns true if a keychain // entry was successfully added/updated. @@ -54,10 +57,11 @@ class MacKeychainPasswordFormAdapter { void SetFindsOnlyOwnedItems(bool finds_only_owned); private: - // Returns PasswordForms constructed from the given Keychain items. + // Returns PasswordForms constructed from the given Keychain items, calling + // MacKeychain::Free on all of the keychain items and clearing the vector. // Caller is responsible for deleting the returned forms. - std::vector<webkit_glue::PasswordForm*> CreateFormsFromKeychainItems( - const std::vector<SecKeychainItemRef>& items); + std::vector<webkit_glue::PasswordForm*> ConvertKeychainItemsToForms( + std::vector<SecKeychainItemRef>* items); // Searches |keychain| for the specific keychain entry that corresponds to the // given form, and returns it (or NULL if no match is found). The caller is diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc index 41d5d1e..c96a1a4 100644 --- a/chrome/browser/password_manager/password_store_mac_unittest.cc +++ b/chrome/browser/password_manager/password_store_mac_unittest.cc @@ -33,7 +33,7 @@ class PasswordStoreMacTest : public testing::Test { kSecProtocolTypeHTTP, NULL, 0, NULL, "20000101000000Z", "", "", true }, // De-facto negative item, type one. - {kSecAuthenticationTypeHTMLForm, "dont.remember.com", + { kSecAuthenticationTypeHTMLForm, "dont.remember.com", kSecProtocolTypeHTTP, NULL, 0, NULL, "20000101000000Z", "Password Not Stored", "", false }, // De-facto negative item, type two. @@ -55,8 +55,8 @@ class PasswordStoreMacTest : public testing::Test { "abc", "123", false }, }; - // Save one slot for use by AddInternetPassword. - unsigned int capacity = arraysize(test_data) + 1; + // Save some extra slots for use by AddInternetPassword. + unsigned int capacity = arraysize(test_data) + 3; keychain_ = new MockKeychain(capacity); for (unsigned int i = 0; i < arraysize(test_data); ++i) { @@ -485,7 +485,8 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) { owned_keychain_adapter.SetFindsOnlyOwnedItems(true); for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - PasswordForm* in_form = CreatePasswordFormFromData(test_data[i].data); + 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) { @@ -498,7 +499,6 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) { EXPECT_EQ(out_form->username_value, in_form->username_value); EXPECT_EQ(out_form->password_value, in_form->password_value); } - delete in_form; } // Test that adding duplicate item updates the existing item. @@ -508,7 +508,7 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) { "http://some.domain.com/insecure.html", NULL, NULL, NULL, NULL, L"joe_user", L"updated_password", false, false, 0 }; - PasswordForm* update_form = CreatePasswordFormFromData(data); + scoped_ptr<PasswordForm> update_form(CreatePasswordFormFromData(data)); MacKeychainPasswordFormAdapter keychain_adapter(keychain_); EXPECT_TRUE(keychain_adapter.AddPassword(*update_form)); SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(2); @@ -517,7 +517,6 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) { keychain_item, &stored_form); EXPECT_EQ(update_form->password_value, stored_form.password_value); - delete update_form; } } @@ -819,3 +818,37 @@ TEST_F(PasswordStoreMacTest, TestPasswordBulkLookup) { STLDeleteElements(&database_forms); STLDeleteElements(&merged_forms); } + +TEST_F(PasswordStoreMacTest, 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", + "http://a.site.com:2222/", NULL, NULL, NULL, NULL, + L"username", L"password", false, false, 0 }, + { PasswordForm::SCHEME_DIGEST, "https://digest.site.com/differentrealm", + "https://digest.site.com/secure.html", NULL, NULL, NULL, NULL, + L"testname", L"testpass", false, false, 0 }, + }; + for (unsigned int i = 0; i < arraysize(owned_password_data); ++i) { + scoped_ptr<PasswordForm> form(CreatePasswordFormFromData( + owned_password_data[i])); + owned_keychain_adapter.AddPassword(*form); + } + + 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); +} |