Skip to content

Commit

Permalink
Support profile level remote command job.
Browse files Browse the repository at this point in the history
Create a new profile picker helper class. It can accept
`ProfileManager` for CBCM remote command. Or a `Profile` instance
for profile level remote command.

Note that one job can only be one of the two states above.

The helper class now is only used for `ClearBrowsingDataJob`.


Change-Id: I06f5298ce8c0779892822c4e745b68f7e0204b40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4776180
Reviewed-by: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184313}
  • Loading branch information
Owen Min authored and Chromium LUCI CQ committed Aug 16, 2023
1 parent fdb6289 commit 0d9183b
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 52 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4608,6 +4608,8 @@ static_library("browser") {
"enterprise/remote_commands/cbcm_remote_commands_factory.h",
"enterprise/remote_commands/clear_browsing_data_job.cc",
"enterprise/remote_commands/clear_browsing_data_job.h",
"enterprise/remote_commands/job_profile_picker.cc",
"enterprise/remote_commands/job_profile_picker.h",
"first_run/upgrade_util.cc",
"first_run/upgrade_util.h",
"lifetime/switch_utils.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
#include "base/json/json_writer.h"
#include "base/task/single_thread_task_runner.h"
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace enterprise_commands {
Expand All @@ -20,7 +18,6 @@ namespace {

const char kFailedTypesPath[] = "failed_data_types";

const char kProfilePathField[] = "profile_path";
const char kClearCacheField[] = "clear_cache";
const char kClearCookiesField[] = "clear_cookies";

Expand Down Expand Up @@ -60,7 +57,9 @@ std::string CreatePayload(uint64_t failed_data_types) {
} // namespace

ClearBrowsingDataJob::ClearBrowsingDataJob(ProfileManager* profile_manager)
: profile_manager_(profile_manager) {}
: job_profile_picker_(profile_manager) {}
ClearBrowsingDataJob::ClearBrowsingDataJob(Profile* profile)
: job_profile_picker_(profile) {}

ClearBrowsingDataJob::~ClearBrowsingDataJob() = default;

Expand All @@ -79,41 +78,9 @@ bool ClearBrowsingDataJob::ParseCommandPayload(
return false;
const base::Value::Dict& dict = root->GetDict();

const std::string* path = dict.FindString(kProfilePathField);
if (!path)
if (!job_profile_picker_.ParseCommandPayload(dict)) {
return false;

// On Windows, file paths are wstring as opposed to string on other platforms.
// On POSIX platforms other than MacOS and ChromeOS, the encoding is unknown.
//
// This path is sent from the server, which obtained it from Chrome in a
// previous report, and Chrome casts the path as UTF8 using UTF8Unsafe before
// sending it (see BrowserReportGeneratorDesktop::GenerateProfileInfo).
// Because of that, the best thing we can do everywhere is try to get the
// path from UTF8, and ending up with an invalid path will fail later in
// RunImpl when we attempt to get the profile from the path.
profile_path_ = base::FilePath::FromUTF8Unsafe(*path);
#if BUILDFLAG(IS_WIN)
// For Windows machines, the path that Chrome reports for the profile is
// "Normalized" to all lower-case on the reporting server. This means that
// when the server sends the command, the path will be all lower case and
// the profile manager won't be able to use it as a key. To avoid this issue,
// This code will iterate over all profile paths and find the one that matches
// in a case-insensitive comparison. If this doesn't find one, RunImpl will
// fail in the same manner as if the profile didn't exist, which is the
// expected behavior.
ProfileAttributesStorage& storage =
profile_manager_->GetProfileAttributesStorage();
for (ProfileAttributesEntry* entry : storage.GetAllProfilesAttributes()) {
base::FilePath entry_path = entry->GetPath();

if (base::FilePath::CompareEqualIgnoreCase(profile_path_.value(),
entry_path.value())) {
profile_path_ = entry_path;
break;
}
}
#endif

// Not specifying these fields is equivalent to setting them to false.
clear_cache_ = dict.FindBool(kClearCacheField).value_or(false);
Expand All @@ -123,16 +90,14 @@ bool ClearBrowsingDataJob::ParseCommandPayload(
}

void ClearBrowsingDataJob::RunImpl(CallbackWithResult result_callback) {
DCHECK(profile_manager_);

uint64_t types = 0;
if (clear_cache_)
types |= content::BrowsingDataRemover::DATA_TYPE_CACHE;

if (clear_cookies_)
types |= content::BrowsingDataRemover::DATA_TYPE_COOKIES;

Profile* profile = profile_manager_->GetProfileByPath(profile_path_);
Profile* profile = job_profile_picker_.GetProfile();
if (!profile) {
// If the payload's profile path doesn't correspond to an existing profile,
// there's nothing to do. The most likely scenario is that the profile was
Expand Down Expand Up @@ -167,7 +132,7 @@ void ClearBrowsingDataJob::RunImpl(CallbackWithResult result_callback) {

void ClearBrowsingDataJob::OnBrowsingDataRemoverDone(
uint64_t failed_data_types) {
Profile* profile = profile_manager_->GetProfileByPath(profile_path_);
Profile* profile = job_profile_picker_.GetProfile();
DCHECK(profile);

content::BrowsingDataRemover* remover = profile->GetBrowsingDataRemover();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "chrome/browser/enterprise/remote_commands/job_profile_picker.h"
#include "components/policy/core/common/remote_commands/remote_command_job.h"
#include "content/public/browser/browsing_data_remover.h"

class ProfileManager;
class Profile;

namespace enterprise_commands {

Expand All @@ -20,6 +22,7 @@ class ClearBrowsingDataJob : public policy::RemoteCommandJob,
public content::BrowsingDataRemover::Observer {
public:
explicit ClearBrowsingDataJob(ProfileManager* profile_manager);
explicit ClearBrowsingDataJob(Profile* profile);
~ClearBrowsingDataJob() override;

private:
Expand All @@ -31,15 +34,12 @@ class ClearBrowsingDataJob : public policy::RemoteCommandJob,
// content::BrowsingDataRemover::Observer:
void OnBrowsingDataRemoverDone(uint64_t failed_data_types) override;

base::FilePath profile_path_;
bool clear_cache_;
bool clear_cookies_;

// RunImpl callback which will be invoked by OnBrowsingDataRemoverDone.
CallbackWithResult result_callback_;

// Non-owned pointer to the ProfileManager of the current browser process.
raw_ptr<ProfileManager> profile_manager_;
JobProfilePicker job_profile_picker_;
};

} // namespace enterprise_commands
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,28 @@ enterprise_management::RemoteCommand CreateCommandProto(
return command_proto;
}

std::unique_ptr<ClearBrowsingDataJob> CreateJob(
enterprise_management::RemoteCommand command_proto,
ProfileManager* profile_manager) {
auto job = std::make_unique<ClearBrowsingDataJob>(profile_manager);
void InitJob(ClearBrowsingDataJob* job,
const enterprise_management::RemoteCommand& command_proto) {
EXPECT_TRUE(job->Init(base::TimeTicks::Now(), command_proto,
enterprise_management::SignedData{}));
EXPECT_EQ(kUniqueID, job->unique_id());
EXPECT_EQ(policy::RemoteCommandJob::NOT_STARTED, job->status());
}

std::unique_ptr<ClearBrowsingDataJob> CreateJob(
const enterprise_management::RemoteCommand& command_proto,
ProfileManager* profile_manager) {
auto job = std::make_unique<ClearBrowsingDataJob>(profile_manager);
InitJob(job.get(), command_proto);

return job;
}

std::unique_ptr<ClearBrowsingDataJob> CreateJob(
const enterprise_management::RemoteCommand& command_proto,
Profile* profile) {
auto job = std::make_unique<ClearBrowsingDataJob>(profile);
InitJob(job.get(), command_proto);

return job;
}
Expand Down Expand Up @@ -100,8 +114,8 @@ class ClearBrowsingDataJobTest : public ::testing::Test {

void RunUntilIdle() { task_environment_->RunUntilIdle(); }

void AddTestingProfile() {
profile_manager_->CreateTestingProfile(kProfileName);
TestingProfile* AddTestingProfile() {
return profile_manager_->CreateTestingProfile(kProfileName);
}

base::FilePath GetTestProfilePath() {
Expand Down Expand Up @@ -289,6 +303,28 @@ TEST_F(ClearBrowsingDataJobTest, SuccessClearNeither) {
EXPECT_TRUE(done);
}

TEST_F(ClearBrowsingDataJobTest, SucessClearWithProfile) {
base::RunLoop run_loop;
TestingProfile* profile = AddTestingProfile();
auto job = CreateJob(CreateCommandProto(GetTestProfilePath().AsUTF8Unsafe(),
/* clear_cache= */ true,
/* clear_cookies= */ false),
profile);
bool done = false;
// Run should return true because the command will be successfully posted,
// but status of the command will be |FAILED| when |finished_callback| is
// invoked.
EXPECT_TRUE(job->Run(base::Time::Now(), base::TimeTicks::Now(),
base::BindLambdaForTesting([&] {
EXPECT_EQ(policy::RemoteCommandJob::SUCCEEDED,
job->status());
done = true;
run_loop.Quit();
})));
run_loop.Run();
EXPECT_TRUE(done);
}

// For Windows machines, the path that Chrome reports for the profile is
// "Normalized" to all lower-case on the reporting server. This means that
// when the server sends the command, the path will be all lower case and
Expand Down
81 changes: 81 additions & 0 deletions chrome/browser/enterprise/remote_commands/job_profile_picker.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/enterprise/remote_commands/job_profile_picker.h"

#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h"

namespace enterprise_commands {
namespace {

const char kProfilePathField[] = "profile_path";

} // namespace

JobProfilePicker::JobProfilePicker(Profile* profile)
: profile_or_profile_manager_(profile) {}
JobProfilePicker::JobProfilePicker(ProfileManager* profile_manager)
: profile_or_profile_manager_(profile_manager) {}

JobProfilePicker::~JobProfilePicker() = default;

bool JobProfilePicker::ParseCommandPayload(
const base::Value::Dict& command_payload) {
if (absl::holds_alternative<raw_ptr<Profile>>(profile_or_profile_manager_)) {
return true;
}

const std::string* path = command_payload.FindString(kProfilePathField);
if (!path) {
return false;
}

// On Windows, file paths are wstring as opposed to string on other platforms.
// On POSIX platforms other than MacOS and ChromeOS, the encoding is unknown.
//
// This path is sent from the server, which obtained it from Chrome in a
// previous report, and Chrome casts the path as UTF8 using UTF8Unsafe before
// sending it (see BrowserReportGeneratorDesktop::GenerateProfileInfo).
// Because of that, the best thing we can do everywhere is try to get the
// path from UTF8, and ending up with an invalid path will fail later in
// RunImpl when we attempt to get the profile from the path.
profile_path_ = base::FilePath::FromUTF8Unsafe(*path);
#if BUILDFLAG(IS_WIN)
// For Windows machines, the path that Chrome reports for the profile is
// "Normalized" to all lower-case on the reporting server. This means that
// when the server sends the command, the path will be all lower case and
// the profile manager won't be able to use it as a key. To avoid this issue,
// This code will iterate over all profile paths and find the one that matches
// in a case-insensitive comparison. If this doesn't find one, RunImpl will
// fail in the same manner as if the profile didn't exist, which is the
// expected behavior.
ProfileAttributesStorage& storage =
absl::get<raw_ptr<ProfileManager>>(profile_or_profile_manager_)
->GetProfileAttributesStorage();
for (ProfileAttributesEntry* entry : storage.GetAllProfilesAttributes()) {
base::FilePath entry_path = entry->GetPath();

if (base::FilePath::CompareEqualIgnoreCase(profile_path_.value(),
entry_path.value())) {
profile_path_ = entry_path;
break;
}
}
#endif
return true;
}

Profile* JobProfilePicker::GetProfile() {
if (absl::holds_alternative<raw_ptr<Profile>>(profile_or_profile_manager_)) {
return absl::get<raw_ptr<Profile>>(profile_or_profile_manager_);
}
return absl::get<raw_ptr<ProfileManager>>(profile_or_profile_manager_)
->GetProfileByPath(profile_path_);
}

} // namespace enterprise_commands
40 changes: 40 additions & 0 deletions chrome/browser/enterprise/remote_commands/job_profile_picker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_ENTERPRISE_REMOTE_COMMANDS_JOB_PROFILE_PICKER_H_
#define CHROME_BROWSER_ENTERPRISE_REMOTE_COMMANDS_JOB_PROFILE_PICKER_H_

#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "base/values.h"
#include "third_party/abseil-cpp/absl/types/variant.h"

class ProfileManager;
class Profile;

namespace enterprise_commands {
// A helper class that allow remote command job select the correct profile to
// execute the command, regardless the command come with CBCM or Profile
// management.
class JobProfilePicker {
public:
explicit JobProfilePicker(Profile* profile);
explicit JobProfilePicker(ProfileManager* profile);
JobProfilePicker(const JobProfilePicker&) = delete;
JobProfilePicker& operator=(const JobProfilePicker&) = delete;
~JobProfilePicker();

bool ParseCommandPayload(const base::Value::Dict& command_payload);
Profile* GetProfile();

private:
absl::variant<raw_ptr<Profile>, raw_ptr<ProfileManager>>
profile_or_profile_manager_;

base::FilePath profile_path_;
};

} // namespace enterprise_commands

#endif // CHROME_BROWSER_ENTERPRISE_REMOTE_COMMANDS_JOB_PROFILE_PICKER_H_

0 comments on commit 0d9183b

Please sign in to comment.