diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 48b26be65deddf..272a9e63396bdd 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc @@ -18,6 +18,7 @@ #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_version_info.h" +#include "chrome/common/url_constants.h" #include "components/autofill/content/common/autofill_messages.h" #include "components/autofill/core/browser/password_generator.h" #include "components/autofill/core/common/password_form.h" @@ -28,6 +29,7 @@ #include "components/password_manager/core/browser/password_manager_internals_service.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/common/password_manager_switches.h" +#include "content/public/browser/navigation_entry.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" @@ -81,6 +83,24 @@ bool ChromePasswordManagerClient::IsAutomaticPasswordSavingEnabled() const { chrome::VersionInfo::CHANNEL_UNKNOWN; } +bool ChromePasswordManagerClient::IsPasswordManagerEnabledForCurrentPage() + const { + if (EnabledForSyncSignin()) + return true; + + DCHECK(web_contents()); + content::NavigationEntry* entry = + web_contents()->GetController().GetLastCommittedEntry(); + if (!entry) { + // TODO(gcasto): Determine if fix for crbug.com/388246 is relevant here. + return true; + } + // Do not fill nor save password when a user is signing in for sync. This + // is because users need to remember their password if they are syncing as + // this is effectively their master password. + return entry->GetURL().host() != chrome::kChromeUIChromeSigninHost; +} + void ChromePasswordManagerClient::PromptUserToSavePassword( password_manager::PasswordFormManager* form_to_save) { if (IsTheHotNewBubbleUIEnabled()) { @@ -333,3 +353,19 @@ bool ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled() { // The bubble should be the default case that runs on the bots. return group_name != "Infobar"; } + +bool ChromePasswordManagerClient::EnabledForSyncSignin() { + CommandLine* command_line = CommandLine::ForCurrentProcess(); + if (command_line->HasSwitch( + password_manager::switches::kDisableManagerForSyncSignin)) + return false; + + if (command_line->HasSwitch( + password_manager::switches::kEnableManagerForSyncSignin)) + return true; + + // Default is enabled. + std::string group_name = + base::FieldTrialList::FindFullName("PasswordManagerStateForSyncSignin"); + return group_name != "Disabled"; +} diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index b9a3103077df00..cce2773f5292f4 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h @@ -39,6 +39,7 @@ class ChromePasswordManagerClient // PasswordManagerClient implementation. virtual bool IsAutomaticPasswordSavingEnabled() const OVERRIDE; + virtual bool IsPasswordManagerEnabledForCurrentPage() const OVERRIDE; virtual void PromptUserToSavePassword( password_manager::PasswordFormManager* form_to_save) OVERRIDE; virtual void PasswordWasAutofilled( @@ -81,6 +82,9 @@ class ChromePasswordManagerClient // the sad old Infobar UI. static bool IsTheHotNewBubbleUIEnabled(); + // Returns true if the password manager should be enabled during sync signin. + static bool EnabledForSyncSignin(); + private: ChromePasswordManagerClient(content::WebContents* web_contents, autofill::AutofillClient* autofill_client); diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc index c6b1747a467328..0ca48b3d5aba57 100644 --- a/components/password_manager/core/browser/password_manager.cc +++ b/components/password_manager/core/browser/password_manager.cc @@ -111,9 +111,14 @@ void PasswordManager::SetFormHasGeneratedPassword(const PasswordForm& form) { // TODO(gcasto): Add UMA stats to track this. } +bool PasswordManager::IsEnabledForCurrentPage() const { + return !driver_->DidLastPageLoadEncounterSSLErrors() && + client_->IsPasswordManagerEnabledForCurrentPage(); +} + bool PasswordManager::IsSavingEnabledForCurrentPage() const { return *password_manager_enabled_ && !driver_->IsOffTheRecord() && - !driver_->DidLastPageLoadEncounterSSLErrors(); + IsEnabledForCurrentPage(); } void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) { @@ -318,8 +323,7 @@ void PasswordManager::OnPasswordFormsParsed( void PasswordManager::CreatePendingLoginManagers( const std::vector& forms) { - // Don't try to autofill or save passwords in the presence of SSL errors. - if (driver_->DidLastPageLoadEncounterSSLErrors()) + if (!IsEnabledForCurrentPage()) return; // Copy the weak pointers to the currently known login managers for comparison diff --git a/components/password_manager/core/browser/password_manager.h b/components/password_manager/core/browser/password_manager.h index 45a7f93fa47ac6..62471d7f972ea3 100644 --- a/components/password_manager/core/browser/password_manager.h +++ b/components/password_manager/core/browser/password_manager.h @@ -116,6 +116,11 @@ class PasswordManager : public LoginModel { MAX_FAILURE_VALUE }; + // Returns if the password manager is enabled for this page. There are certain + // situations (e.g. bad SSL cert) where we disable the password manager + // temporarily. + bool IsEnabledForCurrentPage() const; + // Log failure for UMA. Logs additional metrics if the |form_origin| // corresponds to one of the top, explicitly monitored websites. For some // values of |failure| also sends logs to the internals page through |logger|, diff --git a/components/password_manager/core/browser/password_manager_client.cc b/components/password_manager/core/browser/password_manager_client.cc index ddadd2d4758e60..fe5a7de7036086 100644 --- a/components/password_manager/core/browser/password_manager_client.cc +++ b/components/password_manager/core/browser/password_manager_client.cc @@ -10,6 +10,10 @@ bool PasswordManagerClient::IsAutomaticPasswordSavingEnabled() const { return false; } +bool PasswordManagerClient::IsPasswordManagerEnabledForCurrentPage() const { + return true; +} + base::FieldTrial::Probability PasswordManagerClient::GetProbabilityForExperiment( const std::string& experiment_name) { diff --git a/components/password_manager/core/browser/password_manager_client.h b/components/password_manager/core/browser/password_manager_client.h index e1c765e4507554..a3579e789aef85 100644 --- a/components/password_manager/core/browser/password_manager_client.h +++ b/components/password_manager/core/browser/password_manager_client.h @@ -30,6 +30,10 @@ class PasswordManagerClient { // The default return value is false. virtual bool IsAutomaticPasswordSavingEnabled() const; + // If the password manager should work for the current page. Default + // always returns true. + virtual bool IsPasswordManagerEnabledForCurrentPage() const; + // Informs the embedder of a password form that can be saved if the user // allows it. The embedder is not required to prompt the user if it decides // that this form doesn't need to be saved. diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc index 7b95a3c32cb874..558089cd46bd16 100644 --- a/components/password_manager/core/browser/password_manager_unittest.cc +++ b/components/password_manager/core/browser/password_manager_unittest.cc @@ -42,6 +42,7 @@ namespace { class MockPasswordManagerClient : public StubPasswordManagerClient { public: + MOCK_CONST_METHOD0(IsPasswordManagerEnabledForCurrentPage, bool()); MOCK_METHOD1(PromptUserToSavePassword, void(PasswordFormManager*)); MOCK_METHOD0(GetPasswordStore, PasswordStore*()); MOCK_METHOD0(GetPrefs, PrefService*()); @@ -82,6 +83,8 @@ class PasswordManagerTest : public testing::Test { EXPECT_CALL(*store_, ReportMetrics()).Times(AnyNumber()); CHECK(store_->Init(syncer::SyncableService::StartSyncFlare())); + EXPECT_CALL(client_, IsPasswordManagerEnabledForCurrentPage()) + .WillRepeatedly(Return(true)); EXPECT_CALL(client_, GetPasswordStore()).WillRepeatedly(Return(store_)); EXPECT_CALL(client_, GetPrefs()).WillRepeatedly(Return(&prefs_)); EXPECT_CALL(client_, GetDriver()).WillRepeatedly(Return(&driver_)); @@ -625,4 +628,24 @@ TEST_F(PasswordManagerTest, AutofillingNotEnabledOnSSLErrors) { manager()->OnPasswordFormsParsed(forms); } +TEST_F(PasswordManagerTest, SavingDisabledIfManagerDisabled) { + EXPECT_CALL(client_, IsPasswordManagerEnabledForCurrentPage()) + .WillRepeatedly(Return(false)); + EXPECT_FALSE(manager()->IsSavingEnabledForCurrentPage()); +} + +TEST_F(PasswordManagerTest, AutofillingDisabledIfManagerDisabled) { + EXPECT_CALL(client_, IsPasswordManagerEnabledForCurrentPage()) + .WillRepeatedly(Return(false)); + + // Let us pretend some forms were found on a website. + std::vector forms; + forms.push_back(MakeSimpleForm()); + + // Feed those forms to |manager()| and check that it does not try to find + // matching saved credentials for the forms. + EXPECT_CALL(*store_.get(), GetLogins(_, _, _)).Times(Exactly(0)); + manager()->OnPasswordFormsParsed(forms); +} + } // namespace password_manager diff --git a/components/password_manager/core/common/password_manager_switches.cc b/components/password_manager/core/common/password_manager_switches.cc index c5b167b9315f9a..31cba89fb6aebe 100644 --- a/components/password_manager/core/common/password_manager_switches.cc +++ b/components/password_manager/core/common/password_manager_switches.cc @@ -8,6 +8,15 @@ namespace password_manager { namespace switches { +// Disable both saving and filling for the sync signin form. +const char kDisableManagerForSyncSignin[] = + "disable-manager-for-sync-signin"; + +// Enable saving and filling for the sync signin form. Currently the default +// behavior. +const char kEnableManagerForSyncSignin[] = + "enable-manager-for-sync-signin"; + // Disables the save-password prompt. Passwords are then saved automatically, // without asking the user. const char kEnableAutomaticPasswordSaving[] = diff --git a/components/password_manager/core/common/password_manager_switches.h b/components/password_manager/core/common/password_manager_switches.h index a80f4b4df883c9..a3eed084b7fc08 100644 --- a/components/password_manager/core/common/password_manager_switches.h +++ b/components/password_manager/core/common/password_manager_switches.h @@ -12,6 +12,8 @@ namespace switches { // All switches in alphabetical order. The switches should be documented // alongside the definition of their values in the .cc file. +extern const char kDisableManagerForSyncSignin[]; +extern const char kEnableManagerForSyncSignin[]; extern const char kEnableAutomaticPasswordSaving[]; } // namespace switches