Commit 9bebc67b authored by Findit's avatar Findit

Revert "Apply crOS Account Manager policy to remove Secondary Accounts"

This reverts commit 66b27048.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 644435 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNjZiMjcwNDhlZWQ4YTYzZjdlNzgzZWQ5YWY0NmNlMjJlZjk3M2M5OQw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/12017

Sample Failed Step: non_single_process_mash_browser_tests

Sample Flaky Test: AccountManagerPolicyControllerTest.ExistingSecondaryAccountsAreRemovedAfterPolicyApplication

Original change's description:
> Apply crOS Account Manager policy to remove Secondary Accounts
> 
> If SecondaryGoogleAccountSigninAllowed policy is set to false, it sets
> the kSecondaryGoogleAccountSigninAllowed preference to false. This
> should result in any existing Secondary Accounts in Chrome OS Account
> Manager being removed.
> 
> Please check https://crbug.com/934492#c6 for a Design Document.
> 
> Bug: 934492
> Test: browser_tests --gtest_filter="*AccountManagerPolicyControllerTest*"
> Change-Id: Ib620a8347b9859dae0245957a0e240490d450545
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538185
> Commit-Queue: Kush Sinha <sinhak@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#644435}

Change-Id: Iddc86e15909b54317dda77323bfd1cbb972c1767
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 934492
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1542136
Cr-Commit-Position: refs/heads/master@{#644893}
parent 56ff77b4
......@@ -306,10 +306,6 @@ source_set("chromeos") {
"accessibility/switch_access_panel.h",
"account_manager/account_manager_migrator.cc",
"account_manager/account_manager_migrator.h",
"account_manager/account_manager_policy_controller.cc",
"account_manager/account_manager_policy_controller.h",
"account_manager/account_manager_policy_controller_factory.cc",
"account_manager/account_manager_policy_controller_factory.h",
"account_manager/account_manager_util.cc",
"account_manager/account_manager_util.h",
"account_manager/account_migration_runner.cc",
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller.h"
#include <string>
#include "base/callback.h"
#include "base/sequence_checker.h"
#include "chrome/browser/chromeos/account_manager/account_manager_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/components/account_manager/account_manager.h"
#include "chromeos/constants/chromeos_pref_names.h"
#include "components/prefs/pref_service.h"
namespace chromeos {
AccountManagerPolicyController::AccountManagerPolicyController(
Profile* profile,
AccountManager* account_manager,
const AccountId& device_account_id)
: profile_(profile),
account_manager_(account_manager),
device_account_id_(device_account_id),
weak_factory_(this) {}
AccountManagerPolicyController::~AccountManagerPolicyController() {
pref_change_registrar_.RemoveAll();
}
void AccountManagerPolicyController::Start() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!chromeos::IsAccountManagerAvailable(profile_))
return;
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
chromeos::prefs::kSecondaryGoogleAccountSigninAllowed,
base::BindRepeating(&AccountManagerPolicyController::OnPrefChanged,
weak_factory_.GetWeakPtr()));
// Take any necessary initial action based on the current state of the pref.
OnPrefChanged();
}
void AccountManagerPolicyController::OnPrefChanged() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (profile_->GetPrefs()->GetBoolean(
chromeos::prefs::kSecondaryGoogleAccountSigninAllowed)) {
return;
}
account_manager_->GetAccounts(
base::BindOnce(&AccountManagerPolicyController::OnGetAccounts,
weak_factory_.GetWeakPtr()));
}
void AccountManagerPolicyController::OnGetAccounts(
const std::vector<AccountManager::Account>& accounts) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// The objective here is to remove all Secondary Accounts in Chrome OS
// Account Manager. When this policy / pref is applied, all account
// additions to Chrome OS Account Manager are blocked. Hence, we do not need
// to take care of the case where accounts are being added to Account
// Manager, while we are removing them from here. We can simply retrieve the
// current list of accounts from Account Manager and then issue calls to
// remove all Secondary Accounts.
for (const auto& account : accounts) {
if (account.key.account_type !=
account_manager::AccountType::ACCOUNT_TYPE_GAIA) {
// |kSecondaryGoogleAccountSigninAllowed| applies only to Gaia accounts.
// Ignore other types of accounts.
continue;
}
if (device_account_id_.GetAccountType() == AccountType::GOOGLE &&
account.key.id == device_account_id_.GetGaiaId()) {
// Do not remove the Device Account.
continue;
}
// This account is a Secondary Gaia account. Remove it.
account_manager_->RemoveAccount(account.key);
}
}
} // namespace chromeos
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_H_
#define CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_H_
#include <vector>
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chromeos/components/account_manager/account_manager.h"
#include "components/account_id/account_id.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_change_registrar.h"
class Profile;
namespace chromeos {
class AccountManager;
class AccountManagerPolicyController : public KeyedService {
public:
AccountManagerPolicyController(Profile* profile,
AccountManager* account_manager,
const AccountId& device_account_id);
~AccountManagerPolicyController() override;
// Starts applying the behaviour required by |chromeos::AccountManager|
// specific prefs and policies.
void Start();
private:
// Callback handler for |AccountManager::GetAccounts|.
void OnGetAccounts(const std::vector<AccountManager::Account>&);
// Callback for handling changes in |kSecondaryGoogleAccountSigninAllowed|
// pref.
void OnPrefChanged();
// Non-owning pointers.
Profile* const profile_;
AccountManager* const account_manager_;
const AccountId device_account_id_;
// For listening on Pref changes.
PrefChangeRegistrar pref_change_registrar_;
SEQUENCE_CHECKER(sequence_checker_);
base::WeakPtrFactory<AccountManagerPolicyController> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(AccountManagerPolicyController);
};
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_H_
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/files/scoped_temp_dir.h"
#include "base/test/bind_test_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller.h"
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller_factory.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/testing_profile.h"
#include "chromeos/components/account_manager/account_manager_factory.h"
#include "chromeos/constants/chromeos_pref_names.h"
#include "components/user_manager/scoped_user_manager.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chromeos {
namespace {
constexpr char kFakePrimaryUsername[] = "test-primary@example.com";
constexpr char kFakeSecondaryUsername[] = "test-secondary@example.com";
constexpr char kFakeSecondaryGaiaId[] = "fake-secondary-gaia-id";
} // namespace
class AccountManagerPolicyControllerTest : public InProcessBrowserTest {
public:
AccountManagerPolicyControllerTest() = default;
~AccountManagerPolicyControllerTest() override = default;
void SetUpOnMainThread() override {
// Prep private fields.
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
TestingProfile::Builder profile_builder;
profile_builder.SetPath(temp_dir_.GetPath().AppendASCII("TestProfile"));
profile_builder.SetProfileName(kFakePrimaryUsername);
profile_ = IdentityTestEnvironmentProfileAdaptor::
CreateProfileForIdentityTestEnvironment(profile_builder);
AccountManagerFactory* factory =
g_browser_process->platform_part()->GetAccountManagerFactory();
account_manager_ = factory->GetAccountManager(profile()->GetPath().value());
identity_test_environment_adaptor_ =
std::make_unique<IdentityTestEnvironmentProfileAdaptor>(profile_.get());
// Prep the Primary account.
auto* identity_test_env =
identity_test_environment_adaptor_->identity_test_env();
const AccountInfo primary_account_info =
identity_test_env->MakePrimaryAccountAvailable(kFakePrimaryUsername);
auto user_manager = std::make_unique<chromeos::FakeChromeUserManager>();
AccountId account_id = AccountId::FromUserEmailGaiaId(
primary_account_info.email, primary_account_info.gaia);
user_manager->AddUser(account_id);
user_manager->LoginUser(account_id);
scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
std::move(user_manager));
// Add accounts in Account Manager.
account_manager_->UpsertAccount(
AccountManager::AccountKey{
primary_account_info.gaia,
account_manager::AccountType::ACCOUNT_TYPE_GAIA},
primary_account_info.email, AccountManager::kInvalidToken);
account_manager_->UpsertAccount(
AccountManager::AccountKey{
kFakeSecondaryGaiaId,
account_manager::AccountType::ACCOUNT_TYPE_GAIA},
kFakeSecondaryUsername, AccountManager::kInvalidToken);
AccountManagerPolicyControllerFactory::GetForBrowserContext(profile());
}
void TearDownOnMainThread() override {
identity_test_environment_adaptor_.reset();
scoped_user_manager_.reset();
profile_.reset();
}
std::vector<AccountManager::Account> GetAccountManagerAccounts() {
DCHECK(account_manager_);
std::vector<AccountManager::Account> accounts;
base::RunLoop run_loop;
account_manager_->GetAccounts(base::BindLambdaForTesting(
[&accounts, &run_loop](
const std::vector<AccountManager::Account>& stored_accounts) {
accounts = stored_accounts;
run_loop.Quit();
}));
run_loop.Run();
return accounts;
}
Profile* profile() { return profile_.get(); }
identity::IdentityManager* identity_manager() {
return identity_test_environment_adaptor_->identity_test_env()
->identity_manager();
}
private:
base::ScopedTempDir temp_dir_;
// Non-owning pointer.
AccountManager* account_manager_ = nullptr;
std::unique_ptr<Profile> profile_;
std::unique_ptr<IdentityTestEnvironmentProfileAdaptor>
identity_test_environment_adaptor_;
std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
DISALLOW_COPY_AND_ASSIGN(AccountManagerPolicyControllerTest);
};
IN_PROC_BROWSER_TEST_F(AccountManagerPolicyControllerTest,
ExistingSecondaryAccountsAreNotRemovedIfPolicyIsNotSet) {
std::vector<AccountManager::Account> accounts = GetAccountManagerAccounts();
// We should have at least 1 Secondary Account.
const std::vector<AccountManager::Account>::size_type initial_num_accounts =
accounts.size();
ASSERT_GT(initial_num_accounts, 1UL);
// Use default policy value for |kSecondaryGoogleAccountSigninAllowed|
// (|true|).
profile()->GetPrefs()->SetBoolean(
chromeos::prefs::kSecondaryGoogleAccountSigninAllowed, true);
// All accounts must be intact.
accounts = GetAccountManagerAccounts();
EXPECT_EQ(initial_num_accounts, accounts.size());
}
IN_PROC_BROWSER_TEST_F(
AccountManagerPolicyControllerTest,
ExistingSecondaryAccountsAreRemovedAfterPolicyApplication) {
std::vector<AccountManager::Account> accounts = GetAccountManagerAccounts();
// We should have at least 1 Secondary Account.
ASSERT_GT(accounts.size(), 1UL);
// Disallow secondary account sign-ins.
profile()->GetPrefs()->SetBoolean(
chromeos::prefs::kSecondaryGoogleAccountSigninAllowed, false);
// Secondary Accounts must be removed.
accounts = GetAccountManagerAccounts();
ASSERT_EQ(accounts.size(), 1UL);
EXPECT_EQ(ProfileHelper::Get()
->GetUserByProfile(profile())
->GetAccountId()
.GetGaiaId(),
identity_manager()->GetPrimaryAccountInfo().gaia);
EXPECT_EQ(ProfileHelper::Get()
->GetUserByProfile(profile())
->GetAccountId()
.GetGaiaId(),
accounts[0].key.id);
}
} // namespace chromeos
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller_factory.h"
#include "base/no_destructor.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/components/account_manager/account_manager_factory.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
namespace chromeos {
// static
AccountManagerPolicyController*
AccountManagerPolicyControllerFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<AccountManagerPolicyController*>(
GetInstance()->GetServiceForBrowserContext(context, true));
}
// static
AccountManagerPolicyControllerFactory*
AccountManagerPolicyControllerFactory::GetInstance() {
static base::NoDestructor<AccountManagerPolicyControllerFactory> instance;
return instance.get();
}
AccountManagerPolicyControllerFactory::AccountManagerPolicyControllerFactory()
: BrowserContextKeyedServiceFactory(
"AccountManagerPolicyController",
BrowserContextDependencyManager::GetInstance()) {}
AccountManagerPolicyControllerFactory::
~AccountManagerPolicyControllerFactory() = default;
KeyedService* AccountManagerPolicyControllerFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
Profile* const profile = Profile::FromBrowserContext(context);
chromeos::AccountManagerFactory* factory =
g_browser_process->platform_part()->GetAccountManagerFactory();
chromeos::AccountManager* account_manager =
factory->GetAccountManager(profile->GetPath().value());
if (!account_manager)
return nullptr;
user_manager::User* const user =
ProfileHelper::Get()->GetUserByProfile(profile);
if (!user)
return nullptr;
AccountManagerPolicyController* const service =
new AccountManagerPolicyController(profile, account_manager,
user->GetAccountId());
// Auto-start the Service.
service->Start();
return service;
}
} // namespace chromeos
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_FACTORY_H_
#define CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_FACTORY_H_
#include <vector>
#include "base/macros.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
namespace base {
template <typename T>
class NoDestructor;
} // namespace base
namespace chromeos {
class AccountManagerPolicyController;
class AccountManagerPolicyControllerFactory
: public BrowserContextKeyedServiceFactory {
public:
// Gets the instance of the service associated with |context|.
static AccountManagerPolicyController* GetForBrowserContext(
content::BrowserContext* context);
// Gets the singleton instance of the factory
// (|AccountManagerPolicyControllerFactory|).
static AccountManagerPolicyControllerFactory* GetInstance();
private:
friend class base::NoDestructor<AccountManagerPolicyControllerFactory>;
AccountManagerPolicyControllerFactory();
~AccountManagerPolicyControllerFactory() override;
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
DISALLOW_COPY_AND_ASSIGN(AccountManagerPolicyControllerFactory);
};
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MANAGER_POLICY_CONTROLLER_FACTORY_H_
......@@ -125,7 +125,6 @@
#if defined(OS_CHROMEOS)
#include "chrome/browser/browser_process_platform_part_chromeos.h"
#include "chrome/browser/chromeos/account_manager/account_manager_policy_controller_factory.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/cryptohome/cryptohome_client.h"
......@@ -1365,11 +1364,6 @@ void ProfileManager::DoFinalInitForServices(Profile* profile,
#endif
AccessibilityLabelsServiceFactory::GetForProfile(profile)->Init();
#if defined(OS_CHROMEOS)
chromeos::AccountManagerPolicyControllerFactory::GetForBrowserContext(
profile);
#endif
}
void ProfileManager::DoFinalInitLogging(Profile* profile) {
......
......@@ -1739,7 +1739,6 @@ test("browser_tests") {
"../browser/chromeos/accessibility/sticky_keys_browsertest.cc",
"../browser/chromeos/accessibility/switch_access_browsertest.cc",
"../browser/chromeos/accessibility/touch_exploration_controller_browsertest.cc",
"../browser/chromeos/account_manager/account_manager_policy_controller_browsertest.cc",
"../browser/chromeos/app_mode/arc/arc_kiosk_app_manager_browsertest.cc",
"../browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc",
"../browser/chromeos/app_mode/kiosk_app_update_service_browsertest.cc",
......
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