Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Making Chromoting use no GPO provider in PolicyLoaderWin.
Browse files Browse the repository at this point in the history
The old Chromoting code (policy_hack before crrev.com/830193002) used to
always read Chromoting policies from the registry.  The new code (using
components/policy) ignores contents of the registry on non-domain-joined
machines.  This old-vs-new difference in behavior was unintentional.
The current changelist ensures that Chromoting's old behavior is preserved.

BUG=460734
TEST=1) components_unittests.exe, 2) remoting_unittests.exe, 3) remoting_unittests.exe --gtest_filter=*PolicyWatcher*Real* -v=1 copied and run from a non-domain-joined, test VM to verify that Chromoting gets its policies from the registry (HKLM\SOFTWARE\Policies\Google\Chrome) in this scenario.

Review URL: https://codereview.chromium.org/947353002

Cr-Commit-Position: refs/heads/master@{#317865}
(cherry picked from commit bccb49e)

TBR=dcaiafa

Review URL: https://codereview.chromium.org/959033002

Cr-Commit-Position: refs/branch-heads/2311@{#40}
Cr-Branched-From: 09b7de5-refs/heads/master@{#317474}
  • Loading branch information
jamiewalch committed Feb 26, 2015
1 parent ef10f3c commit 41afc82
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 7 deletions.
7 changes: 4 additions & 3 deletions components/policy/core/common/policy_loader_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,10 @@ scoped_ptr<PolicyBundle> PolicyLoaderWin::Load() {
// timeout on it more aggressively. For now, there's no justification for
// the additional effort this would introduce.

if (is_enterprise || !ReadPolicyFromGPO(scope, &gpo_dict, &status)) {
VLOG_IF(1, !is_enterprise) << "Failed to read GPO files for " << scope
<< " falling back to registry.";
bool is_registry_forced = is_enterprise || gpo_provider_ == nullptr;
if (is_registry_forced || !ReadPolicyFromGPO(scope, &gpo_dict, &status)) {
VLOG_IF(1, !is_registry_forced) << "Failed to read GPO files for "
<< scope << " falling back to registry.";
gpo_dict.ReadRegistry(kScopes[i].hive, chrome_policy_key_);
}

Expand Down
3 changes: 3 additions & 0 deletions components/policy/core/common/policy_loader_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class POLICY_EXPORT PolicyLoaderWin
// The PReg file name used by GPO.
static const base::FilePath::CharType kPRegFileName[];

// Passing |gpo_provider| equal nullptr forces all reads to go through the
// registry. This is undesirable for Chrome (see crbug.com/259236), but
// needed for some other use cases (i.e. Chromoting - see crbug.com/460734).
PolicyLoaderWin(scoped_refptr<base::SequencedTaskRunner> task_runner,
const base::string16& chrome_policy_key,
AppliedGPOListProvider* gpo_provider);
Expand Down
20 changes: 18 additions & 2 deletions components/policy/core/common/policy_loader_win_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,8 @@ class PolicyLoaderWinTest : public PolicyTestBase,
.AppendASCII("data")
.AppendASCII("policy")
.AppendASCII("gpo");

gpo_list_provider_ = this;
}

// AppliedGPOListProvider:
Expand Down Expand Up @@ -740,7 +742,8 @@ class PolicyLoaderWinTest : public PolicyTestBase,
}

bool Matches(const PolicyBundle& expected) {
PolicyLoaderWin loader(loop_.message_loop_proxy(), kTestPolicyKey, this);
PolicyLoaderWin loader(loop_.message_loop_proxy(), kTestPolicyKey,
gpo_list_provider_);
scoped_ptr<PolicyBundle> loaded(
loader.InitialLoad(schema_registry_.schema_map()));
return loaded->Equals(expected);
Expand Down Expand Up @@ -783,6 +786,7 @@ class PolicyLoaderWinTest : public PolicyTestBase,
PGROUP_POLICY_OBJECT gpo_list_;
DWORD gpo_list_status_;
base::FilePath test_data_dir_;
AppliedGPOListProvider* gpo_list_provider_;
};

const base::char16 PolicyLoaderWinTest::kTestPolicyKey[] =
Expand Down Expand Up @@ -1047,7 +1051,19 @@ TEST_F(PolicyLoaderWinTest, AppliedPolicyInDomain) {
gpo_list_ = &gpo;
gpo_list_status_ = ERROR_SUCCESS;

PolicyBundle empty;
EXPECT_TRUE(MatchesRegistrySentinel());
}

TEST_F(PolicyLoaderWinTest, GpoProviderNotSpecified) {
base::win::SetDomainStateForTesting(false);
InstallRegistrySentinel();
base::FilePath gpo_dir(test_data_dir_.AppendASCII("empty"));
GROUP_POLICY_OBJECT gpo;
InitGPO(&gpo, 0, gpo_dir, NULL, NULL);
gpo_list_ = &gpo;
gpo_list_status_ = ERROR_SUCCESS;
gpo_list_provider_ = nullptr;

EXPECT_TRUE(MatchesRegistrySentinel());
}

Expand Down
5 changes: 3 additions & 2 deletions remoting/host/policy_watcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ scoped_ptr<PolicyWatcher> PolicyWatcher::Create(
// Chromium.
scoped_ptr<policy::AsyncPolicyLoader> policy_loader;
#if defined(OS_WIN)
policy_loader = policy::PolicyLoaderWin::Create(
file_task_runner, L"SOFTWARE\\Policies\\Google\\Chrome");
policy_loader.reset(new policy::PolicyLoaderWin(
file_task_runner, L"SOFTWARE\\Policies\\Google\\Chrome",
nullptr)); // nullptr = don't use GPO / always read from the registry.
#elif defined(OS_MACOSX)
CFStringRef bundle_id = CFSTR("com.google.Chrome");
policy_loader.reset(new policy::PolicyLoaderMac(
Expand Down

0 comments on commit 41afc82

Please sign in to comment.