Commit a30ddd4a authored by Dominique Fauteux-Chapleau's avatar Dominique Fauteux-Chapleau Committed by Commit Bot

Clear policies from memory and storage on CBCM unenrollment

The browser test is updated to observe the core directly since it now
gets disconnected by unenrollment. Also, refactor
browser_dm_token_storag* classes by moving the post task function to
the base class and the subclass now only provide the store function
binding.

Bug: 1020301, 1020299
Change-Id: I98a62c90fcb8e7ca30dc733bd2b19fb6632e9ec9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940575
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: default avatarTien Mai <tienmai@chromium.org>
Reviewed-by: default avatarOwen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721228}
parent 78196eae
......@@ -178,6 +178,14 @@ void BrowserDMTokenStorage::InitIfNeeded() {
should_display_error_message_on_failure_ = InitEnrollmentErrorOption();
}
void BrowserDMTokenStorage::SaveDMToken(const std::string& token) {
auto task = SaveDMTokenTask(token, RetrieveClientId());
auto reply = base::BindOnce(&BrowserDMTokenStorage::OnDMTokenStored,
weak_factory_.GetWeakPtr());
base::PostTaskAndReplyWithResult(SaveDMTokenTaskRunner().get(), FROM_HERE,
std::move(task), std::move(reply));
}
std::string BrowserDMTokenStorage::InitSerialNumber() {
// GetHardwareInfo is asynchronous, but we need this synchronously. This call
// will only happens once, as we cache the value. This will eventually be
......
......@@ -21,6 +21,10 @@
#include "base/system/sys_info.h"
#include "components/policy/core/common/cloud/dm_token.h"
namespace base {
class TaskRunner;
}
namespace policy {
// Manages storing and retrieving tokens and client ID used to enroll browser
......@@ -32,6 +36,7 @@ namespace policy {
// called.
class BrowserDMTokenStorage {
public:
using StoreTask = base::OnceCallback<bool()>;
using StoreCallback = base::OnceCallback<void(bool success)>;
// Returns the global singleton object. Must be called from the UI thread.
......@@ -101,8 +106,15 @@ class BrowserDMTokenStorage {
// Gets the boolean value that determines if error message will be displayed
// when enrollment fails.
virtual bool InitEnrollmentErrorOption() = 0;
// Saves the DM token. This implementation is platform dependant.
virtual void SaveDMToken(const std::string& token) = 0;
// Function called by |SaveDMToken| that returns if the operation was a
// success. This implementation is platform dependent.
virtual StoreTask SaveDMTokenTask(const std::string& token,
const std::string& client_id) = 0;
// Can optionally be overridden by platform implementation if a specific task
// runner should be used by |SaveDMToken|.
virtual scoped_refptr<base::TaskRunner> SaveDMTokenTaskRunner() = 0;
// Saves the DM token.
void SaveDMToken(const std::string& token);
// Will be called after the DM token is stored.
StoreCallback store_callback_;
......@@ -117,6 +129,8 @@ class BrowserDMTokenStorage {
SEQUENCE_CHECKER(sequence_checker_);
base::WeakPtrFactory<BrowserDMTokenStorage> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(BrowserDMTokenStorage);
};
......
......@@ -21,6 +21,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/syslog_logging.h"
#include "base/task/post_task.h"
#include "base/task/task_traits.h"
#include "base/task_runner_util.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/sequenced_task_runner_handle.h"
......@@ -81,7 +82,9 @@ BrowserDMTokenStorage* BrowserDMTokenStorage::Get() {
return storage.get();
}
BrowserDMTokenStorageLinux::BrowserDMTokenStorageLinux() {}
BrowserDMTokenStorageLinux::BrowserDMTokenStorageLinux()
: task_runner_(
base::CreateTaskRunner({base::ThreadPool(), base::MayBlock()})) {}
BrowserDMTokenStorageLinux::~BrowserDMTokenStorageLinux() {}
......@@ -167,13 +170,15 @@ bool BrowserDMTokenStorageLinux::InitEnrollmentErrorOption() {
kEnrollmentMandatoryOption;
}
void BrowserDMTokenStorageLinux::SaveDMToken(const std::string& token) {
std::string client_id = RetrieveClientId();
base::PostTaskAndReplyWithResult(
FROM_HERE, {base::ThreadPool(), base::MayBlock()},
base::BindOnce(&StoreDMTokenInUserDataDir, token, client_id),
base::BindOnce(&BrowserDMTokenStorage::OnDMTokenStored,
weak_factory_.GetWeakPtr()));
BrowserDMTokenStorage::StoreTask BrowserDMTokenStorageLinux::SaveDMTokenTask(
const std::string& token,
const std::string& client_id) {
return base::BindOnce(&StoreDMTokenInUserDataDir, token, client_id);
}
scoped_refptr<base::TaskRunner>
BrowserDMTokenStorageLinux::SaveDMTokenTaskRunner() {
return task_runner_;
}
std::string BrowserDMTokenStorageLinux::ReadMachineIdFile() {
......
......@@ -32,13 +32,14 @@ class BrowserDMTokenStorageLinux : public BrowserDMTokenStorage {
std::string InitEnrollmentToken() override;
std::string InitDMToken() override;
bool InitEnrollmentErrorOption() override;
void SaveDMToken(const std::string& token) override;
StoreTask SaveDMTokenTask(const std::string& token,
const std::string& client_id) override;
scoped_refptr<base::TaskRunner> SaveDMTokenTaskRunner() override;
// Returns the content of "/etc/machine-id". Virtual for tests.
virtual std::string ReadMachineIdFile();
// This should always be the last member of the class.
base::WeakPtrFactory<BrowserDMTokenStorageLinux> weak_factory_{this};
scoped_refptr<base::TaskRunner> task_runner_;
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageLinuxTest, InitClientId);
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageLinuxTest, InitEnrollmentToken);
......
......@@ -32,10 +32,11 @@ class BrowserDMTokenStorageMac : public BrowserDMTokenStorage {
std::string InitEnrollmentToken() override;
std::string InitDMToken() override;
bool InitEnrollmentErrorOption() override;
void SaveDMToken(const std::string& token) override;
StoreTask SaveDMTokenTask(const std::string& token,
const std::string& client_id) override;
scoped_refptr<base::TaskRunner> SaveDMTokenTaskRunner() override;
// This should always be the last member of the class.
base::WeakPtrFactory<BrowserDMTokenStorageMac> weak_factory_;
scoped_refptr<base::TaskRunner> task_runner_;
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageMacTest, InitClientId);
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageMacTest, InitEnrollmentToken);
......
......@@ -172,7 +172,9 @@ BrowserDMTokenStorage* BrowserDMTokenStorage::Get() {
return storage.get();
}
BrowserDMTokenStorageMac::BrowserDMTokenStorageMac() : weak_factory_(this) {}
BrowserDMTokenStorageMac::BrowserDMTokenStorageMac()
: task_runner_(
base::CreateTaskRunner({base::ThreadPool(), base::MayBlock()})) {}
BrowserDMTokenStorageMac::~BrowserDMTokenStorageMac() {}
......@@ -231,13 +233,15 @@ bool BrowserDMTokenStorageMac::InitEnrollmentErrorOption() {
return IsEnrollmentMandatoryByFile().value_or(false);
}
void BrowserDMTokenStorageMac::SaveDMToken(const std::string& token) {
std::string client_id = RetrieveClientId();
base::PostTaskAndReplyWithResult(
FROM_HERE, {base::ThreadPool(), base::MayBlock()},
base::BindOnce(&StoreDMTokenInDirAppDataDir, token, client_id),
base::BindOnce(&BrowserDMTokenStorage::OnDMTokenStored,
weak_factory_.GetWeakPtr()));
BrowserDMTokenStorage::StoreTask BrowserDMTokenStorageMac::SaveDMTokenTask(
const std::string& token,
const std::string& client_id) {
return base::BindOnce(&StoreDMTokenInDirAppDataDir, token, client_id);
}
scoped_refptr<base::TaskRunner>
BrowserDMTokenStorageMac::SaveDMTokenTaskRunner() {
return task_runner_;
}
} // namespace policy
......@@ -197,12 +197,15 @@ bool BrowserDMTokenStorageWin::InitEnrollmentErrorOption() {
return InstallUtil::ShouldCloudManagementBlockOnFailure();
}
void BrowserDMTokenStorageWin::SaveDMToken(const std::string& token) {
base::PostTaskAndReplyWithResult(
com_sta_task_runner_.get(), FROM_HERE,
base::BindOnce(&StoreDMTokenInRegistry, token),
base::BindOnce(&BrowserDMTokenStorage::OnDMTokenStored,
weak_factory_.GetWeakPtr()));
BrowserDMTokenStorage::StoreTask BrowserDMTokenStorageWin::SaveDMTokenTask(
const std::string& token,
const std::string& client_id) {
return base::BindOnce(&StoreDMTokenInRegistry, token);
}
scoped_refptr<base::TaskRunner>
BrowserDMTokenStorageWin::SaveDMTokenTaskRunner() {
return com_sta_task_runner_;
}
// static
......
......@@ -32,13 +32,12 @@ class BrowserDMTokenStorageWin : public BrowserDMTokenStorage {
std::string InitEnrollmentToken() override;
std::string InitDMToken() override;
bool InitEnrollmentErrorOption() override;
void SaveDMToken(const std::string& token) override;
StoreTask SaveDMTokenTask(const std::string& token,
const std::string& client_id) override;
scoped_refptr<base::TaskRunner> SaveDMTokenTaskRunner() override;
scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
// This should always be the last member of the class.
base::WeakPtrFactory<BrowserDMTokenStorageWin> weak_factory_{this};
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageWinTest, InitClientId);
FRIEND_TEST_ALL_PREFIXES(BrowserDMTokenStorageWinTest, InitEnrollmentToken);
// TODO(crbug.com/907589): Remove once no longer in use.
......
......@@ -292,6 +292,36 @@ bool ChromeBrowserCloudManagementController::
cloud_management_register_watcher_->IsDialogShowing();
}
void ChromeBrowserCloudManagementController::UnenrollBrowser() {
// Invalidate DM token in storage.
BrowserDMTokenStorage::Get()->InvalidateDMToken(base::BindOnce(
&ChromeBrowserCloudManagementController::InvalidateDMTokenCallback,
base::Unretained(this)));
}
void ChromeBrowserCloudManagementController::InvalidatePolicies() {
// Reset policies.
if (policy_fetcher_) {
policy_fetcher_->RemoveClientObserver(this);
policy_fetcher_->Disconnect();
}
// This causes the scheduler to stop refreshing itself since the DM token is
// no longer valid.
if (report_scheduler_)
report_scheduler_->OnDMTokenUpdated();
}
void ChromeBrowserCloudManagementController::InvalidateDMTokenCallback(
bool success) {
if (success) {
DVLOG(1) << "Successfully invalidated the DM token";
InvalidatePolicies();
} else {
DVLOG(1) << "Failed to invalidate the DM token";
}
}
void ChromeBrowserCloudManagementController::OnPolicyFetched(
CloudPolicyClient* client) {
// Ignored.
......@@ -306,16 +336,8 @@ void ChromeBrowserCloudManagementController::OnClientError(
CloudPolicyClient* client) {
// DM_STATUS_SERVICE_DEVICE_NOT_FOUND being the last status implies the
// browser has been unenrolled.
if (client->status() == DM_STATUS_SERVICE_DEVICE_NOT_FOUND) {
// Invalidate DM token in storage.
BrowserDMTokenStorage::Get()->InvalidateDMToken(
base::BindOnce([](bool success) {
if (success)
DVLOG(1) << "Successfully invalidated the DM token";
else
DVLOG(1) << "Failed to invalidate the DM token";
}));
}
if (client->status() == DM_STATUS_SERVICE_DEVICE_NOT_FOUND)
UnenrollBrowser();
}
void ChromeBrowserCloudManagementController::NotifyPolicyRegisterFinished(
......
......@@ -90,6 +90,8 @@ class ChromeBrowserCloudManagementController
// Returns whether the enterprise startup dialog is being diaplayed.
bool IsEnterpriseStartupDialogShowing();
void UnenrollBrowser();
// CloudPolicyClient::Observer implementation:
void OnPolicyFetched(CloudPolicyClient* client) override;
void OnRegistrationStateChanged(CloudPolicyClient* client) override;
......@@ -105,6 +107,9 @@ class ChromeBrowserCloudManagementController
const std::string& dm_token,
const std::string& client_id);
void InvalidatePolicies();
void InvalidateDMTokenCallback(bool success);
void CreateReportSchedulerAsync(
scoped_refptr<base::SequencedTaskRunner> task_runner);
void CreateReportScheduler();
......
......@@ -145,7 +145,7 @@ class ChromeBrowserExtraSetUp : public ChromeBrowserMainExtraParts {
};
// Two observers that quit run_loop when policy is fetched and stored or in case
// there is any error.
// the core is disconnected in case of error.
class PolicyFetchStoreObserver : public CloudPolicyStore::Observer {
public:
PolicyFetchStoreObserver(CloudPolicyStore* store,
......@@ -168,35 +168,31 @@ class PolicyFetchStoreObserver : public CloudPolicyStore::Observer {
DISALLOW_COPY_AND_ASSIGN(PolicyFetchStoreObserver);
};
class PolicyFetchClientObserver : public CloudPolicyClient::Observer {
class PolicyFetchCoreObserver : public CloudPolicyCore::Observer {
public:
PolicyFetchClientObserver(CloudPolicyClient* client,
base::OnceClosure quit_closure)
: client_(client), quit_closure_(std::move(quit_closure)) {
client_->AddObserver(this);
PolicyFetchCoreObserver(CloudPolicyCore* core, base::OnceClosure quit_closure)
: core_(core), quit_closure_(std::move(quit_closure)) {
core_->AddObserver(this);
}
~PolicyFetchClientObserver() override { client_->RemoveObserver(this); }
~PolicyFetchCoreObserver() override { core_->RemoveObserver(this); }
void OnPolicyFetched(CloudPolicyClient* client) override {
std::move(quit_closure_).Run();
}
void OnCoreConnected(CloudPolicyCore* core) override {}
void OnRegistrationStateChanged(CloudPolicyClient* client) override {}
void OnRefreshSchedulerStarted(CloudPolicyCore* core) override {}
void OnClientError(CloudPolicyClient* client) override {
void OnCoreDisconnecting(CloudPolicyCore* core) override {
// This is called when policy fetching fails and is used in
// ChromeBrowserCloudManagementController to unenroll the browser. The
// status must be DM_STATUS_SERVICE_DEVICE_NOT_FOUND for this to happen.
EXPECT_EQ(client->status(), DM_STATUS_SERVICE_DEVICE_NOT_FOUND);
EXPECT_EQ(core->client()->status(), DM_STATUS_SERVICE_DEVICE_NOT_FOUND);
std::move(quit_closure_).Run();
}
void OnRemoteCommandsServiceStarted(CloudPolicyCore* core) override {}
private:
CloudPolicyClient* client_;
CloudPolicyCore* core_;
base::OnceClosure quit_closure_;
std::unique_ptr<PolicyFetchStoreObserver> store_observer_;
DISALLOW_COPY_AND_ASSIGN(PolicyFetchClientObserver);
};
} // namespace
......@@ -607,13 +603,13 @@ IN_PROC_BROWSER_TEST_P(MachineLevelUserCloudPolicyPolicyFetchTest, Test) {
if (manager->core()->client()->last_policy_timestamp().is_null()) {
base::RunLoop run_loop;
// Listen to store event which is fired after policy validation if token is
// valid. Otherwise listen to the client event because there is no store
// event.
std::unique_ptr<PolicyFetchClientObserver> client_observer;
// valid. Otherwise listen to the core since it gets disconnected by
// unenrollment.
std::unique_ptr<PolicyFetchCoreObserver> core_observer;
std::unique_ptr<PolicyFetchStoreObserver> store_observer;
if (dm_token() == kInvalidDMToken) {
client_observer = std::make_unique<PolicyFetchClientObserver>(
manager->core()->client(), run_loop.QuitClosure());
core_observer = std::make_unique<PolicyFetchCoreObserver>(
manager->core(), run_loop.QuitClosure());
} else {
store_observer = std::make_unique<PolicyFetchStoreObserver>(
manager->store(), run_loop.QuitClosure());
......
......@@ -115,7 +115,10 @@ MachineLevelUserCloudPolicyFetcher::MachineLevelUserCloudPolicyFetcher(
}
MachineLevelUserCloudPolicyFetcher::~MachineLevelUserCloudPolicyFetcher() {
policy_manager_->core()->service()->RemoveObserver(this);
// The pointers need to be checked since they might be invalidated from a
// |Disconnect| call.
if (policy_manager_->core() && policy_manager_->core()->service())
policy_manager_->core()->service()->RemoveObserver(this);
}
void MachineLevelUserCloudPolicyFetcher::SetupRegistrationAndFetchPolicy(
......@@ -142,6 +145,14 @@ void MachineLevelUserCloudPolicyFetcher::RemoveClientObserver(
policy_manager_->RemoveClientObserver(observer);
}
void MachineLevelUserCloudPolicyFetcher::Disconnect() {
if (policy_manager_) {
if (policy_manager_->core() && policy_manager_->core()->service())
policy_manager_->core()->service()->RemoveObserver(this);
policy_manager_->DisconnectAndRemovePolicy();
}
}
void MachineLevelUserCloudPolicyFetcher::
OnCloudPolicyServiceInitializationCompleted() {
// Client will be registered before policy fetch. A non-registered client
......
......@@ -82,6 +82,10 @@ class MachineLevelUserCloudPolicyFetcher : public CloudPolicyService::Observer {
void AddClientObserver(CloudPolicyClient::Observer* observer);
void RemoveClientObserver(CloudPolicyClient::Observer* observer);
// Shuts down |policy_manager_| (removes and stops refreshing the cached cloud
// policy).
void Disconnect();
// CloudPolicyService::Observer:
void OnCloudPolicyServiceInitializationCompleted() override;
......
......@@ -3,6 +3,9 @@
// found in the LICENSE file.
#include "chrome/browser/policy/fake_browser_dm_token_storage.h"
#include "base/bind.h"
#include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
namespace policy {
......@@ -59,8 +62,16 @@ bool FakeBrowserDMTokenStorage::InitEnrollmentErrorOption() {
return enrollment_error_option_;
}
void FakeBrowserDMTokenStorage::SaveDMToken(const std::string& token) {
OnDMTokenStored(storage_enabled_);
FakeBrowserDMTokenStorage::StoreTask FakeBrowserDMTokenStorage::SaveDMTokenTask(
const std::string& token,
const std::string& client_id) {
return base::BindOnce([](bool enabled) -> bool { return enabled; },
storage_enabled_);
}
scoped_refptr<base::TaskRunner>
FakeBrowserDMTokenStorage::SaveDMTokenTaskRunner() {
return base::ThreadTaskRunnerHandle::Get();
}
} // namespace policy
......@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_POLICY_FAKE_BROWSER_DM_TOKEN_STORAGE_H_
#define CHROME_BROWSER_POLICY_FAKE_BROWSER_DM_TOKEN_STORAGE_H_
#include "base/memory/weak_ptr.h"
#include "chrome/browser/policy/browser_dm_token_storage.h"
namespace policy {
......@@ -33,7 +34,9 @@ class FakeBrowserDMTokenStorage : public BrowserDMTokenStorage {
std::string InitEnrollmentToken() override;
std::string InitDMToken() override;
bool InitEnrollmentErrorOption() override;
void SaveDMToken(const std::string& token) override;
StoreTask SaveDMTokenTask(const std::string& token,
const std::string& client_id) override;
scoped_refptr<base::TaskRunner> SaveDMTokenTaskRunner() override;
private:
std::string client_id_ = "";
......
......@@ -77,6 +77,23 @@ void MachineLevelUserCloudPolicyManager::RemoveClientObserver(
client()->RemoveObserver(observer);
}
void MachineLevelUserCloudPolicyManager::DisconnectAndRemovePolicy() {
if (external_data_manager_)
external_data_manager_->Disconnect();
core()->Disconnect();
// store_->Clear() will publish the updated, empty policy. The component
// policy service must be cleared before OnStoreLoaded() is issued, so that
// component policies are also empty at CheckAndPublishPolicy().
ClearAndDestroyComponentCloudPolicyService();
// When the |store_| is cleared, it informs the |external_data_manager_| that
// all external data references have been removed, causing the
// |external_data_manager_| to clear its cache as well.
store_->Clear();
}
void MachineLevelUserCloudPolicyManager::Init(SchemaRegistry* registry) {
DVLOG(1) << "Machine level cloud policy manager initialized";
// Call to grand-parent's Init() instead of parent's is intentional.
......
......@@ -32,7 +32,7 @@ class POLICY_EXPORT MachineLevelUserCloudPolicyManager
~MachineLevelUserCloudPolicyManager() override;
// Initializes the cloud connection. |local_state| must stay valid until this
// object is deleted.
// object is deleted or DisconnectAndRemovePolicy gets called.
void Connect(PrefService* local_state,
std::unique_ptr<CloudPolicyClient> client);
......@@ -45,6 +45,10 @@ class POLICY_EXPORT MachineLevelUserCloudPolicyManager
MachineLevelUserCloudPolicyStore* store() { return store_.get(); }
// Shuts down the MachineLevelUserCloudPolicyManager (removes and stops
// refreshing the cached cloud policy).
void DisconnectAndRemovePolicy();
// ConfigurationPolicyProvider:
void Init(SchemaRegistry* registry) override;
void Shutdown() override;
......
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