Commit f37f3978 authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

[b4p] Suppress move bubble for the primary account password

If the password of the primary signed-in account is stored locally (i.e.
in the profile store), the bubble that offers moving it into the account
should not be shown, since storing the password for an account within
that same account doesn't make sense. Moving other Google account
passwords is still valid.

Bug: 1110894
Change-Id: I2be5b30ecb121e5db91e3ff33b98cba51af6430b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339980
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795438}
parent 5e4e3423
...@@ -4,16 +4,36 @@ ...@@ -4,16 +4,36 @@
#include "components/password_manager/core/browser/password_manager_client_helper.h" #include "components/password_manager/core/browser/password_manager_client_helper.h"
#include "base/strings/utf_string_conversions.h"
#include "components/password_manager/core/browser/password_bubble_experiment.h" #include "components/password_manager/core/browser/password_bubble_experiment.h"
#include "components/password_manager/core/browser/password_feature_manager.h" #include "components/password_manager/core/browser/password_feature_manager.h"
#include "components/password_manager/core/browser/password_form_manager_for_ui.h" #include "components/password_manager/core/browser/password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_manager.h" #include "components/password_manager/core/browser/password_manager.h"
#include "components/password_manager/core/browser/password_sync_util.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h" #include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "google_apis/gaia/gaia_auth_util.h"
namespace password_manager { namespace password_manager {
namespace {
bool IsPrimaryAccountSignIn(const signin::IdentityManager& identity_manager,
const base::string16& username,
const std::string& signon_realm) {
CoreAccountInfo primary_account = identity_manager.GetPrimaryAccountInfo(
signin::ConsentLevel::kNotRequired);
return sync_util::IsGaiaCredentialPage(signon_realm) &&
!primary_account.IsEmpty() &&
gaia::AreEmailsSame(base::UTF16ToUTF8(username),
primary_account.email);
}
} // namespace
PasswordManagerClientHelper::PasswordManagerClientHelper( PasswordManagerClientHelper::PasswordManagerClientHelper(
PasswordManagerClient* delegate) PasswordManagerClient* delegate)
: delegate_(delegate) { : delegate_(delegate) {
...@@ -85,12 +105,27 @@ bool PasswordManagerClientHelper::ShouldPromptToEnableAutoSignIn() const { ...@@ -85,12 +105,27 @@ bool PasswordManagerClientHelper::ShouldPromptToEnableAutoSignIn() const {
bool PasswordManagerClientHelper::ShouldPromptToMovePasswordToAccount( bool PasswordManagerClientHelper::ShouldPromptToMovePasswordToAccount(
const PasswordFormManagerForUI& submitted_manager) const { const PasswordFormManagerForUI& submitted_manager) const {
return delegate_->GetPasswordFeatureManager() if (!delegate_->GetPasswordFeatureManager()
->ShouldShowAccountStorageBubbleUi() && ->ShouldShowAccountStorageBubbleUi()) {
delegate_->GetPasswordFeatureManager()->GetDefaultPasswordStore() == return false;
autofill::PasswordForm::Store::kAccountStore && }
submitted_manager.IsMovableToAccountStore() && if (delegate_->GetPasswordFeatureManager()->GetDefaultPasswordStore() ==
!delegate_->IsIncognito(); autofill::PasswordForm::Store::kProfileStore) {
return false;
}
if (!submitted_manager.IsMovableToAccountStore())
return false;
if (delegate_->IsIncognito())
return false;
// It's not useful to store the password for the primary account inside
// that same account.
if (IsPrimaryAccountSignIn(
*delegate_->GetIdentityManager(),
submitted_manager.GetPendingCredentials().username_value,
submitted_manager.GetPendingCredentials().signon_realm)) {
return false;
}
return true;
} }
} // namespace password_manager } // namespace password_manager
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/mock_password_form_manager_for_ui.h" #include "components/password_manager/core/browser/mock_password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_form_manager.h" #include "components/password_manager/core/browser/password_form_manager.h"
...@@ -19,6 +20,8 @@ ...@@ -19,6 +20,8 @@
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "google_apis/gaia/gaia_urls.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -31,7 +34,8 @@ using testing::AnyNumber; ...@@ -31,7 +34,8 @@ using testing::AnyNumber;
using testing::NiceMock; using testing::NiceMock;
using testing::Return; using testing::Return;
constexpr char kTestUsername[] = "test@gmail.com"; constexpr char kTestUsername[] = "user";
constexpr char kGaiaAccountEmail[] = "user@gmail.com";
constexpr char kTestPassword[] = "T3stP@$$w0rd"; constexpr char kTestPassword[] = "T3stP@$$w0rd";
constexpr char kTestOrigin[] = "https://example.com/"; constexpr char kTestOrigin[] = "https://example.com/";
...@@ -46,6 +50,7 @@ class MockPasswordManagerClient : public StubPasswordManagerClient { ...@@ -46,6 +50,7 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
MOCK_METHOD(void, PromptUserToEnableAutosignin, (), (override)); MOCK_METHOD(void, PromptUserToEnableAutosignin, (), (override));
MOCK_METHOD(PrefService*, GetPrefs, (), (const, override)); MOCK_METHOD(PrefService*, GetPrefs, (), (const, override));
MOCK_METHOD(bool, IsIncognito, (), (const, override)); MOCK_METHOD(bool, IsIncognito, (), (const, override));
MOCK_METHOD(signin::IdentityManager*, GetIdentityManager, (), (override));
}; };
PasswordForm CreateForm(std::string username, PasswordForm CreateForm(std::string username,
...@@ -81,13 +86,23 @@ class PasswordManagerClientHelperTest : public testing::Test { ...@@ -81,13 +86,23 @@ class PasswordManagerClientHelperTest : public testing::Test {
prefs_.SetBoolean(prefs::kWasAutoSignInFirstRunExperienceShown, false); prefs_.SetBoolean(prefs::kWasAutoSignInFirstRunExperienceShown, false);
prefs_.SetBoolean(prefs::kCredentialsEnableAutosignin, true); prefs_.SetBoolean(prefs::kCredentialsEnableAutosignin, true);
ON_CALL(client_, GetPrefs()).WillByDefault(Return(&prefs_)); ON_CALL(client_, GetPrefs()).WillByDefault(Return(&prefs_));
ON_CALL(*client(), GetIdentityManager)
.WillByDefault(Return(identity_test_environment()->identity_manager()));
identity_test_environment()->SetUnconsentedPrimaryAccount(
kGaiaAccountEmail);
} }
~PasswordManagerClientHelperTest() override = default; ~PasswordManagerClientHelperTest() override = default;
protected: protected:
PasswordManagerClientHelper* helper() { return &helper_; } PasswordManagerClientHelper* helper() { return &helper_; }
MockPasswordManagerClient* client() { return &client_; } MockPasswordManagerClient* client() { return &client_; }
signin::IdentityTestEnvironment* identity_test_environment() {
return &identity_test_environment_;
}
base::test::TaskEnvironment task_environment_;
signin::IdentityTestEnvironment identity_test_environment_;
NiceMock<MockPasswordManagerClient> client_; NiceMock<MockPasswordManagerClient> client_;
PasswordManagerClientHelper helper_; PasswordManagerClientHelper helper_;
TestingPrefServiceSimple prefs_; TestingPrefServiceSimple prefs_;
...@@ -182,4 +197,22 @@ TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForUnmovableForm) { ...@@ -182,4 +197,22 @@ TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForUnmovableForm) {
CreateFormManager(&form, /*is_movable=*/false)); CreateFormManager(&form, /*is_movable=*/false));
} }
TEST_F(PasswordManagerClientHelperTest, NoPromptToMoveForGaiaAccountForm) {
base::test::ScopedFeatureList account_storage_feature;
account_storage_feature.InitAndEnableFeature(
password_manager::features::kEnablePasswordsAccountStorage);
EXPECT_CALL(*client()->GetPasswordFeatureManager(),
ShouldShowAccountStorageBubbleUi)
.WillOnce(Return(true));
EXPECT_CALL(*client()->GetPasswordFeatureManager(), GetDefaultPasswordStore)
.WillOnce(Return(autofill::PasswordForm::Store::kAccountStore));
EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0);
const PasswordForm gaia_account_form = CreateForm(
kGaiaAccountEmail, kTestPassword, GaiaUrls::GetInstance()->gaia_url());
helper()->NotifySuccessfulLoginWithExistingPassword(
CreateFormManager(&gaia_account_form, /*is_movable=*/true));
}
} // namespace password_manager } // namespace password_manager
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment