diff options
author | mkwst <mkwst@chromium.org> | 2014-12-17 02:10:25 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-12-17 10:12:00 +0000 |
commit | d885f216d2e645534ffb8dbb935a3947ff3f7718 (patch) | |
tree | 43609436b646d66fab130518509390e6a048c111 | |
parent | 1736bcf4e918f13c78ade1d446c7222c720d4d6a (diff) | |
download | chromium_src-d885f216d2e645534ffb8dbb935a3947ff3f7718.zip chromium_src-d885f216d2e645534ffb8dbb935a3947ff3f7718.tar.gz chromium_src-d885f216d2e645534ffb8dbb935a3947ff3f7718.tar.bz2 |
Credential Manager: Pipe federated credentials through to UI layer.
This patch adjusts the credential manager dispatcher's filters for responses
to 'request()' which attempt to grab federated credentials, and lays the
groundwork for doing something useful with them in the UI layer.
Currently that groundwork just consists of piping the values around and
ensuring that nothing explodes. Future CLs will adjust the UI to ensure
that users can make a meaningful choice.
BUG=400674
R=vabr@chromium.org,vasilii@chromium.org
Review URL: https://codereview.chromium.org/809693002
Cr-Commit-Position: refs/heads/master@{#308771}
10 files changed, 71 insertions, 40 deletions
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 8ed58b0..7059e73 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc @@ -227,15 +227,19 @@ bool ChromePasswordManagerClient::PromptUserToSavePassword( } bool ChromePasswordManagerClient::PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const password_manager::CredentialInfo&)> callback) { - // Take ownership of all the password form objects in the |results| vector. - ScopedVector<autofill::PasswordForm> entries; - entries.assign(forms.begin(), forms.end()); + // Take ownership of all the password form objects in the forms vectors. + ScopedVector<autofill::PasswordForm> local_entries; + local_entries.assign(local_forms.begin(), local_forms.end()); + ScopedVector<autofill::PasswordForm> federated_entries; + federated_entries.assign(federated_forms.begin(), federated_forms.end()); + ManagePasswordsUIController* manage_passwords_ui_controller = ManagePasswordsUIController::FromWebContents(web_contents()); - return manage_passwords_ui_controller->OnChooseCredentials(entries.Pass(), - callback); + return manage_passwords_ui_controller->OnChooseCredentials( + local_entries.Pass(), federated_entries.Pass(), callback); } void ChromePasswordManagerClient::AutomaticPasswordSave( diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index bae9ce2..753bf262 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h @@ -53,7 +53,8 @@ class ChromePasswordManagerClient bool PromptUserToSavePassword( scoped_ptr<password_manager::PasswordFormManager> form_to_save) override; bool PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const password_manager::CredentialInfo&)> callback) override; void AutomaticPasswordSave(scoped_ptr<password_manager::PasswordFormManager> diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc index 00a4f7f..513bd1e 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc @@ -108,12 +108,14 @@ void ManagePasswordsUIController::OnPasswordSubmitted( } bool ManagePasswordsUIController::OnChooseCredentials( - ScopedVector<autofill::PasswordForm> credentials, + ScopedVector<autofill::PasswordForm> local_credentials, + ScopedVector<autofill::PasswordForm> federated_credentials, base::Callback<void(const password_manager::CredentialInfo&)> callback){ - DCHECK(!credentials.empty()); + // TODO(vasilii): Do something clever with |federated_credentials|. + DCHECK(!local_credentials.empty() || !federated_credentials.empty()); form_manager_.reset(); - origin_ = credentials[0]->origin; - new_password_forms_.swap(credentials); + origin_ = local_credentials[0]->origin; + new_password_forms_.swap(local_credentials); // The map is useless because usernames may overlap. password_form_map_.clear(); state_ = password_manager::ui::CREDENTIAL_REQUEST_AND_BUBBLE_STATE; diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h index faf3fb8..716c355 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h @@ -45,9 +45,11 @@ class ManagePasswordsUIController scoped_ptr<password_manager::PasswordFormManager> form_manager); // Called when the site asks user to choose from credentials. This triggers - // the UI to prompt the user. |credentials| shouldn't be empty. + // the UI to prompt the user. |local_credentials| and |federated_credentials| + // shouldn't both be empty. bool OnChooseCredentials( - ScopedVector<autofill::PasswordForm> credentials, + ScopedVector<autofill::PasswordForm> local_credentials, + ScopedVector<autofill::PasswordForm> federated_credentials, base::Callback<void(const password_manager::CredentialInfo&)> callback); // Called when the password will be saved automatically, but we still wish to diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc index 1a5e874..e2781fe 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc @@ -338,10 +338,12 @@ TEST_F(ManagePasswordsUIControllerTest, AutomaticPasswordSave) { } TEST_F(ManagePasswordsUIControllerTest, ChooseCredential) { - ScopedVector<autofill::PasswordForm> credentials; - credentials.push_back(new autofill::PasswordForm(test_form())); + ScopedVector<autofill::PasswordForm> local_credentials; + local_credentials.push_back(new autofill::PasswordForm(test_form())); + ScopedVector<autofill::PasswordForm> federated_credentials; EXPECT_TRUE(controller()->OnChooseCredentials( - credentials.Pass(), + local_credentials.Pass(), + federated_credentials.Pass(), base::Bind(&ManagePasswordsUIControllerTest::CredentialCallback, base::Unretained(this)))); EXPECT_EQ(password_manager::ui::CREDENTIAL_REQUEST_AND_BUBBLE_STATE, @@ -364,10 +366,12 @@ TEST_F(ManagePasswordsUIControllerTest, ChooseCredential) { } TEST_F(ManagePasswordsUIControllerTest, ChooseCredentialCancel) { - ScopedVector<autofill::PasswordForm> credentials; - credentials.push_back(new autofill::PasswordForm(test_form())); + ScopedVector<autofill::PasswordForm> local_credentials; + local_credentials.push_back(new autofill::PasswordForm(test_form())); + ScopedVector<autofill::PasswordForm> federated_credentials; EXPECT_TRUE(controller()->OnChooseCredentials( - credentials.Pass(), + local_credentials.Pass(), + federated_credentials.Pass(), base::Bind(&ManagePasswordsUIControllerTest::CredentialCallback, base::Unretained(this)))); EXPECT_EQ(password_manager::ui::CREDENTIAL_REQUEST_AND_BUBBLE_STATE, diff --git a/components/password_manager/content/browser/content_credential_manager_dispatcher.cc b/components/password_manager/content/browser/content_credential_manager_dispatcher.cc index 8620389..445e55b 100644 --- a/components/password_manager/content/browser/content_credential_manager_dispatcher.cc +++ b/components/password_manager/content/browser/content_credential_manager_dispatcher.cc @@ -134,22 +134,28 @@ void ContentCredentialManagerDispatcher::OnGetPasswordStoreResults( const std::vector<autofill::PasswordForm*>& results) { DCHECK(pending_request_); + std::set<std::string> federations; + for (const GURL& origin : pending_request_->federations) + federations.insert(origin.spec()); + // We own the PasswordForm instances, so we're responsible for cleaning - // up the instances we don't add to |filtered_results|. We'll dump them - // into a ScopedVector and allow it to delete the PasswordForms upon - // destruction. - std::vector<autofill::PasswordForm*> filtered_results; + // up the instances we don't add to |local_results| or |federated_results|. + // We'll dump them into a ScopedVector and allow it to delete the + // PasswordForms upon destruction. + std::vector<autofill::PasswordForm*> local_results; + std::vector<autofill::PasswordForm*> federated_results; ScopedVector<autofill::PasswordForm> discarded_results; for (autofill::PasswordForm* form : results) { // TODO(mkwst): Extend this filter to include federations. - if (form->origin == pending_request_->origin) { - filtered_results.push_back(form); - } else { + if (form->origin == pending_request_->origin) + local_results.push_back(form); + else if (federations.count(form->origin.spec()) != 0) + federated_results.push_back(form); + else discarded_results.push_back(form); - } } - if (filtered_results.empty() || + if ((local_results.empty() && federated_results.empty()) || web_contents()->GetLastCommittedURL().GetOrigin() != pending_request_->origin) { SendCredential(pending_request_->id, CredentialInfo()); @@ -157,7 +163,8 @@ void ContentCredentialManagerDispatcher::OnGetPasswordStoreResults( } if (!client_->PromptUserToChooseCredentials( - filtered_results, + local_results, + federated_results, base::Bind(&ContentCredentialManagerDispatcher::SendCredential, base::Unretained(this), pending_request_->id))) { SendCredential(pending_request_->id, CredentialInfo()); diff --git a/components/password_manager/content/browser/content_credential_manager_dispatcher_unittest.cc b/components/password_manager/content/browser/content_credential_manager_dispatcher_unittest.cc index 4e86291..9d64b9e 100644 --- a/components/password_manager/content/browser/content_credential_manager_dispatcher_unittest.cc +++ b/components/password_manager/content/browser/content_credential_manager_dispatcher_unittest.cc @@ -50,14 +50,18 @@ class TestPasswordManagerClient } bool PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const password_manager::CredentialInfo&)> callback) override { - EXPECT_FALSE(forms.empty()); + EXPECT_FALSE(local_forms.empty() && federated_forms.empty()); did_prompt_user_to_choose_ = true; - ScopedVector<autofill::PasswordForm> entries; - entries.assign(forms.begin(), forms.end()); - password_manager::CredentialInfo info(*entries[0]); + ScopedVector<autofill::PasswordForm> local_entries; + local_entries.assign(local_forms.begin(), local_forms.end()); + ScopedVector<autofill::PasswordForm> federated_entries; + federated_entries.assign(federated_forms.begin(), federated_forms.end()); + // TODO(vasilii): Do something clever with |federated_forms|. + password_manager::CredentialInfo info(*local_entries[0]); base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, info)); return true; diff --git a/components/password_manager/core/browser/password_manager_client.h b/components/password_manager/core/browser/password_manager_client.h index 43f014d..c068bb1 100644 --- a/components/password_manager/core/browser/password_manager_client.h +++ b/components/password_manager/core/browser/password_manager_client.h @@ -84,9 +84,11 @@ class PasswordManagerClient { // Returns true if the prompt is indeed displayed. If the prompt is not // displayed, returns false and does not call |callback|. // |callback| should be invoked with the chosen form. - // Note: The implementation takes ownership of all PasswordForms in |forms|. + // Note: The implementation takes ownership of all PasswordForms in + // |local_forms| and |federated_forms|. virtual bool PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const CredentialInfo&)> callback) = 0; // Called when a password is saved in an automated fashion. Embedder may diff --git a/components/password_manager/core/browser/stub_password_manager_client.cc b/components/password_manager/core/browser/stub_password_manager_client.cc index e64c8a8..7ca89d0 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.cc +++ b/components/password_manager/core/browser/stub_password_manager_client.cc @@ -33,10 +33,14 @@ bool StubPasswordManagerClient::PromptUserToSavePassword( } bool StubPasswordManagerClient::PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const password_manager::CredentialInfo&)> callback) { - ScopedVector<autofill::PasswordForm> entries; - entries.assign(forms.begin(), forms.end()); + // Take ownership of all the password form objects in the forms vectors. + ScopedVector<autofill::PasswordForm> local_entries; + local_entries.assign(local_forms.begin(), local_forms.end()); + ScopedVector<autofill::PasswordForm> federated_entries; + federated_entries.assign(federated_forms.begin(), federated_forms.end()); return false; } diff --git a/components/password_manager/core/browser/stub_password_manager_client.h b/components/password_manager/core/browser/stub_password_manager_client.h index b32ff87..0f4013e 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.h +++ b/components/password_manager/core/browser/stub_password_manager_client.h @@ -25,7 +25,8 @@ class StubPasswordManagerClient : public PasswordManagerClient { bool PromptUserToSavePassword( scoped_ptr<PasswordFormManager> form_to_save) override; bool PromptUserToChooseCredentials( - const std::vector<autofill::PasswordForm*>& forms, + const std::vector<autofill::PasswordForm*>& local_forms, + const std::vector<autofill::PasswordForm*>& federated_forms, base::Callback<void(const password_manager::CredentialInfo&)> callback) override; void AutomaticPasswordSave( |