Use external constants to enable setting group policies
The chrome builders are not joined to a domain, so the
`IntegrationTest.LegacyUpdate3Web` test fails. This change allows for
the test to run as if it were joined to a domain, which allows the test
to succeed. In addition, with using external constants, the registry
is not a constraint anymore, and arbitrary group policies can be set.
Bug: 1325740
Change-Id: I27cf7fc740d14284edb44c888992761cb41bde82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3654538
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: S. Ganesh <ganesh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1006065}
diff --git a/chrome/updater/configurator.cc b/chrome/updater/configurator.cc
index e14d59c..19dac39b 100644
--- a/chrome/updater/configurator.cc
+++ b/chrome/updater/configurator.cc
@@ -57,7 +57,7 @@
Configurator::Configurator(scoped_refptr<UpdaterPrefs> prefs,
scoped_refptr<ExternalConstants> external_constants)
: prefs_(prefs),
- policy_service_(PolicyService::Create()),
+ policy_service_(PolicyService::Create(external_constants)),
external_constants_(external_constants),
activity_data_service_(
std::make_unique<ActivityDataService>(GetUpdaterScope())),
diff --git a/chrome/updater/constants.cc b/chrome/updater/constants.cc
index 00b4b21..8bde3ba 100644
--- a/chrome/updater/constants.cc
+++ b/chrome/updater/constants.cc
@@ -83,6 +83,7 @@
const char kDevOverrideKeyInitialDelay[] = "initial_delay";
const char kDevOverrideKeyServerKeepAliveSeconds[] = "server_keep_alive";
const char kDevOverrideKeyCrxVerifierFormat[] = "crx_verifier_format";
+const char kDevOverrideKeyGroupPolicies[] = "group_policies";
// Developer override file name, relative to app data directory.
const char kDevOverrideFileName[] = "overrides.json";
diff --git a/chrome/updater/constants.h b/chrome/updater/constants.h
index 15502bb2..f721596 100644
--- a/chrome/updater/constants.h
+++ b/chrome/updater/constants.h
@@ -193,6 +193,7 @@
extern const char kDevOverrideKeyInitialDelay[];
extern const char kDevOverrideKeyServerKeepAliveSeconds[];
extern const char kDevOverrideKeyCrxVerifierFormat[];
+extern const char kDevOverrideKeyGroupPolicies[];
// File name of developer overrides file.
extern const char kDevOverrideFileName[];
diff --git a/chrome/updater/external_constants.h b/chrome/updater/external_constants.h
index 8e33d3a..489fe5cb 100644
--- a/chrome/updater/external_constants.h
+++ b/chrome/updater/external_constants.h
@@ -9,6 +9,7 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h"
+#include "base/values.h"
class GURL;
@@ -42,6 +43,9 @@
// CRX format verification requirements.
virtual crx_file::VerifierFormat CrxVerifierFormat() const = 0;
+ // Overrides for the `GroupPolicyManager`.
+ virtual base::Value::DictStorage GroupPolicies() const = 0;
+
protected:
friend class base::RefCountedThreadSafe<ExternalConstants>;
scoped_refptr<ExternalConstants> next_provider_;
diff --git a/chrome/updater/external_constants_builder.cc b/chrome/updater/external_constants_builder.cc
index e4be756..c14fe1d 100644
--- a/chrome/updater/external_constants_builder.cc
+++ b/chrome/updater/external_constants_builder.cc
@@ -4,19 +4,40 @@
#include "chrome/updater/external_constants_builder.h"
+#include <algorithm>
+#include <iterator>
#include <string>
#include <vector>
#include "base/json/json_file_value_serializer.h"
#include "base/logging.h"
+#include "base/values.h"
#include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants_default.h"
+#include "chrome/updater/external_constants_override.h"
#include "chrome/updater/updater_scope.h"
#include "chrome/updater/util.h"
#include "components/crx_file/crx_verifier.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
+#include "url/gurl.h"
namespace updater {
+namespace {
+
+std::vector<std::string> StringVectorFromGURLVector(
+ const std::vector<GURL>& gurls) {
+ std::vector<std::string> ret;
+ ret.reserve(gurls.size());
+
+ std::transform(gurls.begin(), gurls.end(), std::back_inserter(ret),
+ [](const GURL& gurl) { return gurl.possibly_invalid_spec(); });
+
+ return ret;
+}
+
+} // namespace
+
ExternalConstantsBuilder::~ExternalConstantsBuilder() {
LOG_IF(WARNING, !written_) << "An ExternalConstantsBuilder with "
<< overrides_.DictSize() << " entries is being "
@@ -85,6 +106,17 @@
return *this;
}
+ExternalConstantsBuilder& ExternalConstantsBuilder::SetGroupPolicies(
+ const base::Value::DictStorage& group_policies) {
+ overrides_.SetKey(kDevOverrideKeyGroupPolicies, base::Value(group_policies));
+ return *this;
+}
+
+ExternalConstantsBuilder& ExternalConstantsBuilder::ClearGroupPolicies() {
+ overrides_.RemoveKey(kDevOverrideKeyGroupPolicies);
+ return *this;
+}
+
bool ExternalConstantsBuilder::Overwrite() {
const absl::optional<base::FilePath> base_path =
GetBaseDirectory(GetUpdaterScope());
@@ -92,6 +124,7 @@
LOG(ERROR) << "Can't find base directory; can't save constant overrides.";
return false;
}
+
const base::FilePath override_file_path =
base_path.value().AppendASCII(kDevOverrideFileName);
bool ok = JSONFileValueSerializer(override_file_path).Serialize(overrides_);
@@ -99,4 +132,27 @@
return ok;
}
+bool ExternalConstantsBuilder::Modify() {
+ scoped_refptr<ExternalConstantsOverrider> verifier =
+ ExternalConstantsOverrider::FromDefaultJSONFile(
+ CreateDefaultExternalConstants());
+ if (!verifier)
+ return Overwrite();
+
+ if (!overrides_.FindKey(kDevOverrideKeyUrl))
+ SetUpdateURL(StringVectorFromGURLVector(verifier->UpdateURL()));
+ if (!overrides_.FindKey(kDevOverrideKeyUseCUP))
+ SetUseCUP(verifier->UseCUP());
+ if (!overrides_.FindKey(kDevOverrideKeyInitialDelay))
+ SetInitialDelay(verifier->InitialDelay());
+ if (!overrides_.FindKey(kDevOverrideKeyServerKeepAliveSeconds))
+ SetServerKeepAliveSeconds(verifier->ServerKeepAliveSeconds());
+ if (!overrides_.FindKey(kDevOverrideKeyCrxVerifierFormat))
+ SetCrxVerifierFormat(verifier->CrxVerifierFormat());
+ if (!overrides_.FindKey(kDevOverrideKeyGroupPolicies))
+ SetGroupPolicies(verifier->GroupPolicies());
+
+ return Overwrite();
+}
+
} // namespace updater
diff --git a/chrome/updater/external_constants_builder.h b/chrome/updater/external_constants_builder.h
index 0746276..6c574ca 100644
--- a/chrome/updater/external_constants_builder.h
+++ b/chrome/updater/external_constants_builder.h
@@ -8,6 +8,7 @@
#include <string>
#include <vector>
+#include "base/files/file_path.h"
#include "base/values.h"
namespace crx_file {
@@ -50,6 +51,10 @@
crx_file::VerifierFormat crx_verifier_format);
ExternalConstantsBuilder& ClearCrxVerifierFormat();
+ ExternalConstantsBuilder& SetGroupPolicies(
+ const base::Value::DictStorage& group_policies);
+ ExternalConstantsBuilder& ClearGroupPolicies();
+
// Write the external constants overrides file in the default location
// with the values that have been previously set, replacing any file
// previously there. The builder remains usable, does not forget its state,
@@ -58,6 +63,10 @@
// Returns true on success, false on failure.
bool Overwrite();
+ // Blend the set values in this instance with the external constants overrides
+ // file in the default location.
+ bool Modify();
+
private:
base::Value overrides_{base::Value::Type::DICTIONARY};
bool written_ = false;
diff --git a/chrome/updater/external_constants_builder_unittest.cc b/chrome/updater/external_constants_builder_unittest.cc
index ef65b6de..4a027fd 100644
--- a/chrome/updater/external_constants_builder_unittest.cc
+++ b/chrome/updater/external_constants_builder_unittest.cc
@@ -11,6 +11,7 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/memory/scoped_refptr.h"
+#include "base/values.h"
#include "chrome/updater/constants.h"
#include "chrome/updater/external_constants.h"
#include "chrome/updater/external_constants_builder.h"
@@ -70,14 +71,20 @@
EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), kServerKeepAliveSeconds);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 0U);
}
TEST_F(ExternalConstantsBuilderTests, TestOverridingEverything) {
+ base::Value::DictStorage group_policies;
+ group_policies["a"] = base::Value(1);
+ group_policies["b"] = base::Value(2);
+
ExternalConstantsBuilder builder;
builder.SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
.SetUseCUP(false)
.SetInitialDelay(123)
- .SetServerKeepAliveSeconds(2);
+ .SetServerKeepAliveSeconds(2)
+ .SetGroupPolicies(group_policies);
EXPECT_TRUE(builder.Overwrite());
scoped_refptr<ExternalConstantsOverrider> verifier =
@@ -92,6 +99,7 @@
EXPECT_EQ(verifier->InitialDelay(), 123);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), 2);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 2U);
}
TEST_F(ExternalConstantsBuilderTests, TestPartialOverrideWithMultipleURLs) {
@@ -114,6 +122,7 @@
EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), kServerKeepAliveSeconds);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 0U);
}
TEST_F(ExternalConstantsBuilderTests, TestClearedEverything) {
@@ -127,6 +136,7 @@
.ClearUseCUP()
.ClearInitialDelay()
.ClearServerKeepAliveSeconds()
+ .ClearGroupPolicies()
.Overwrite());
scoped_refptr<ExternalConstantsOverrider> verifier =
@@ -140,15 +150,20 @@
EXPECT_EQ(verifier->InitialDelay(), kInitialDelay);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), kServerKeepAliveSeconds);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 0U);
}
TEST_F(ExternalConstantsBuilderTests, TestOverSet) {
+ base::Value::DictStorage group_policies;
+ group_policies["a"] = base::Value(1);
+
EXPECT_TRUE(
ExternalConstantsBuilder()
.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
.SetUseCUP(true)
.SetInitialDelay(123.4)
.SetServerKeepAliveSeconds(2)
+ .SetGroupPolicies(group_policies)
.SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
.SetUseCUP(false)
.SetInitialDelay(937.6)
@@ -167,16 +182,23 @@
EXPECT_EQ(verifier->InitialDelay(), 937.6);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), 3);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 1U);
}
TEST_F(ExternalConstantsBuilderTests, TestReuseBuilder) {
ExternalConstantsBuilder builder;
+
+ base::Value::DictStorage group_policies;
+ group_policies["a"] = base::Value(1);
+ group_policies["b"] = base::Value(2);
+
EXPECT_TRUE(
builder.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
.SetUseCUP(false)
.SetInitialDelay(123.4)
.SetServerKeepAliveSeconds(3)
.SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+ .SetGroupPolicies(group_policies)
.Overwrite());
scoped_refptr<ExternalConstantsOverrider> verifier =
@@ -191,11 +213,16 @@
EXPECT_EQ(verifier->InitialDelay(), 123.4);
EXPECT_EQ(verifier->ServerKeepAliveSeconds(), 3);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 2U);
+
+ base::Value::DictStorage group_policies2;
+ group_policies2["b"] = base::Value(2);
// But now we can use the builder again:
EXPECT_TRUE(builder.SetInitialDelay(92.3)
.SetServerKeepAliveSeconds(4)
.ClearUpdateURL()
+ .SetGroupPolicies(group_policies2)
.Overwrite());
// We need a new overrider to verify because it only loads once.
@@ -212,6 +239,62 @@
EXPECT_EQ(verifier2->InitialDelay(),
92.3); // Updated; update should be seen.
EXPECT_EQ(verifier2->ServerKeepAliveSeconds(), 4);
+ EXPECT_EQ(verifier2->GroupPolicies().size(), 1U);
+}
+
+TEST_F(ExternalConstantsBuilderTests, TestModify) {
+ ExternalConstantsBuilder builder;
+
+ base::Value::DictStorage group_policies;
+ group_policies["a"] = base::Value(1);
+ group_policies["b"] = base::Value(2);
+
+ EXPECT_TRUE(
+ builder.SetUpdateURL(std::vector<std::string>{"https://www.google.com"})
+ .SetUseCUP(false)
+ .SetInitialDelay(123.4)
+ .SetServerKeepAliveSeconds(3)
+ .SetUpdateURL(std::vector<std::string>{"https://www.example.com"})
+ .SetGroupPolicies(group_policies)
+ .Overwrite());
+
+ scoped_refptr<ExternalConstantsOverrider> verifier =
+ ExternalConstantsOverrider::FromDefaultJSONFile(
+ CreateDefaultExternalConstants());
+
+ EXPECT_FALSE(verifier->UseCUP());
+
+ std::vector<GURL> urls = verifier->UpdateURL();
+ ASSERT_EQ(urls.size(), 1ul);
+ EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+
+ EXPECT_EQ(verifier->InitialDelay(), 123.4);
+ EXPECT_EQ(verifier->ServerKeepAliveSeconds(), 3);
+ EXPECT_EQ(verifier->GroupPolicies().size(), 2U);
+
+ // Now we use a new builder to modify just the group policies.
+ ExternalConstantsBuilder builder2;
+
+ base::Value::DictStorage group_policies2;
+ group_policies2["b"] = base::Value(2);
+
+ EXPECT_TRUE(builder2.SetGroupPolicies(group_policies2).Modify());
+
+ // We need a new overrider to verify because it only loads once.
+ scoped_refptr<ExternalConstantsOverrider> verifier2 =
+ ExternalConstantsOverrider::FromDefaultJSONFile(
+ CreateDefaultExternalConstants());
+
+ // Only the group policies are different.
+ EXPECT_EQ(verifier2->GroupPolicies().size(), 1U);
+
+ // All the values below are unchanged.
+ EXPECT_FALSE(verifier2->UseCUP());
+ urls = verifier2->UpdateURL();
+ ASSERT_EQ(urls.size(), 1ul);
+ EXPECT_EQ(urls[0], GURL("https://www.example.com"));
+ EXPECT_EQ(verifier2->InitialDelay(), 123.4);
+ EXPECT_EQ(verifier2->ServerKeepAliveSeconds(), 3);
}
} // namespace updater
diff --git a/chrome/updater/external_constants_default.cc b/chrome/updater/external_constants_default.cc
index 2fa257f..428426f1 100644
--- a/chrome/updater/external_constants_default.cc
+++ b/chrome/updater/external_constants_default.cc
@@ -5,6 +5,7 @@
#include "chrome/updater/external_constants_default.h"
#include "base/memory/scoped_refptr.h"
+#include "base/values.h"
#include "chrome/updater/constants.h"
#include "chrome/updater/external_constants.h"
#include "chrome/updater/updater_branding.h"
@@ -35,6 +36,8 @@
return crx_file::VerifierFormat::CRX3_WITH_PUBLISHER_PROOF;
}
+ base::Value::DictStorage GroupPolicies() const override { return {}; }
+
private:
~DefaultExternalConstants() override = default;
};
diff --git a/chrome/updater/external_constants_override.cc b/chrome/updater/external_constants_override.cc
index 99929d6..41d5fbe 100644
--- a/chrome/updater/external_constants_override.cc
+++ b/chrome/updater/external_constants_override.cc
@@ -132,6 +132,19 @@
crx_format_verifier_value.GetInt());
}
+base::Value::DictStorage ExternalConstantsOverrider::GroupPolicies() const {
+ if (!override_values_.contains(kDevOverrideKeyGroupPolicies)) {
+ return next_provider_->GroupPolicies();
+ }
+
+ const base::Value& group_policies_value =
+ override_values_.at(kDevOverrideKeyGroupPolicies);
+ CHECK(group_policies_value.is_dict())
+ << "Unexpected type of override[" << kDevOverrideKeyGroupPolicies
+ << "]: " << base::Value::GetTypeName(group_policies_value.type());
+ return group_policies_value.Clone().TakeDictDeprecated();
+}
+
// static
scoped_refptr<ExternalConstantsOverrider>
ExternalConstantsOverrider::FromDefaultJSONFile(
diff --git a/chrome/updater/external_constants_override.h b/chrome/updater/external_constants_override.h
index bcfa156a..d9907ea 100644
--- a/chrome/updater/external_constants_override.h
+++ b/chrome/updater/external_constants_override.h
@@ -11,6 +11,7 @@
#include "base/containers/flat_map.h"
#include "base/memory/scoped_refptr.h"
+#include "base/values.h"
#include "chrome/updater/external_constants.h"
class GURL;
@@ -45,6 +46,7 @@
double InitialDelay() const override;
int ServerKeepAliveSeconds() const override;
crx_file::VerifierFormat CrxVerifierFormat() const override;
+ base::Value::DictStorage GroupPolicies() const override;
private:
const base::flat_map<std::string, base::Value> override_values_;
diff --git a/chrome/updater/external_constants_override_unittest.cc b/chrome/updater/external_constants_override_unittest.cc
index 21d23e9..1b82691 100644
--- a/chrome/updater/external_constants_override_unittest.cc
+++ b/chrome/updater/external_constants_override_unittest.cc
@@ -33,6 +33,7 @@
EXPECT_EQ(overrider->InitialDelay(), kInitialDelay);
EXPECT_EQ(overrider->ServerKeepAliveSeconds(), kServerKeepAliveSeconds);
+ EXPECT_EQ(overrider->GroupPolicies().size(), 0U);
}
TEST_F(ExternalConstantsOverriderTest, TestFullOverrides) {
@@ -40,10 +41,15 @@
base::Value::ListStorage url_list;
url_list.push_back(base::Value("https://www.example.com"));
url_list.push_back(base::Value("https://www.google.com"));
+ base::Value::DictStorage group_policies;
+ group_policies["a"] = base::Value(1);
+ group_policies["b"] = base::Value(2);
+
overrides[kDevOverrideKeyUseCUP] = base::Value(false);
overrides[kDevOverrideKeyUrl] = base::Value(std::move(url_list));
overrides[kDevOverrideKeyInitialDelay] = base::Value(137.1);
overrides[kDevOverrideKeyServerKeepAliveSeconds] = base::Value(1);
+ overrides[kDevOverrideKeyGroupPolicies] = base::Value(group_policies);
auto overrider = base::MakeRefCounted<ExternalConstantsOverrider>(
std::move(overrides), CreateDefaultExternalConstants());
@@ -58,6 +64,7 @@
EXPECT_EQ(overrider->InitialDelay(), 137.1);
EXPECT_EQ(overrider->ServerKeepAliveSeconds(), 1);
+ EXPECT_EQ(overrider->GroupPolicies().size(), 2U);
}
TEST_F(ExternalConstantsOverriderTest, TestOverrideUnwrappedURL) {
@@ -75,6 +82,7 @@
EXPECT_TRUE(overrider->UseCUP());
EXPECT_EQ(overrider->InitialDelay(), kInitialDelay);
EXPECT_EQ(overrider->ServerKeepAliveSeconds(), kServerKeepAliveSeconds);
+ EXPECT_EQ(overrider->GroupPolicies().size(), 0U);
}
} // namespace updater
diff --git a/chrome/updater/policy/service.cc b/chrome/updater/policy/service.cc
index f8b1e2f..35fcc9d 100644
--- a/chrome/updater/policy/service.cc
+++ b/chrome/updater/policy/service.cc
@@ -10,8 +10,10 @@
#include "base/callback.h"
#include "base/check.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
+#include "chrome/updater/external_constants.h"
#include "chrome/updater/policy/dm_policy_manager.h"
#if BUILDFLAG(IS_WIN)
@@ -208,10 +210,11 @@
return true;
}
-scoped_refptr<PolicyService> PolicyService::Create() {
+scoped_refptr<PolicyService> PolicyService::Create(
+ scoped_refptr<ExternalConstants> external_constants) {
PolicyManagerVector managers;
#if BUILDFLAG(IS_WIN)
- managers.push_back(std::make_unique<GroupPolicyManager>());
+ managers.push_back(std::make_unique<GroupPolicyManager>(external_constants));
#endif
std::unique_ptr<PolicyManagerInterface> dm_policy_manager =
CreateDMPolicyManager();
diff --git a/chrome/updater/policy/service.h b/chrome/updater/policy/service.h
index 9f56a05..38d50b57 100644
--- a/chrome/updater/policy/service.h
+++ b/chrome/updater/policy/service.h
@@ -11,6 +11,8 @@
#include "base/callback_forward.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_refptr.h"
+#include "chrome/updater/external_constants.h"
#include "chrome/updater/policy/manager.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
@@ -106,7 +108,8 @@
std::string* proxy_server) const;
// Creates an instance that takes a snapshot of policies from all providers.
- static scoped_refptr<PolicyService> Create();
+ static scoped_refptr<PolicyService> Create(
+ scoped_refptr<ExternalConstants> external_constants);
protected:
virtual ~PolicyService();
diff --git a/chrome/updater/policy/win/group_policy_manager.cc b/chrome/updater/policy/win/group_policy_manager.cc
index 30d1ebd..42d28e24 100644
--- a/chrome/updater/policy/win/group_policy_manager.cc
+++ b/chrome/updater/policy/win/group_policy_manager.cc
@@ -11,10 +11,12 @@
#include <userenv.h>
#include "base/enterprise_util.h"
+#include "base/memory/scoped_refptr.h"
#include "base/scoped_generic.h"
#include "base/strings/sys_string_conversions.h"
#include "base/values.h"
#include "base/win/registry.h"
+#include "chrome/updater/external_constants.h"
#include "chrome/updater/policy/manager.h"
#include "chrome/updater/win/win_constants.h"
@@ -75,14 +77,21 @@
} // namespace
-GroupPolicyManager::GroupPolicyManager() {
+GroupPolicyManager::GroupPolicyManager(
+ scoped_refptr<ExternalConstants> external_constants)
+ : external_constants_group_policies_(external_constants->GroupPolicies()) {
LoadAllPolicies();
}
GroupPolicyManager::~GroupPolicyManager() = default;
bool GroupPolicyManager::IsManaged() const {
- return policies_.DictSize() > 0 && base::IsManagedDevice();
+ return !policies_.DictEmpty() && IsManagedInternal();
+}
+
+bool GroupPolicyManager::IsManagedInternal() const {
+ return !external_constants_group_policies_.DictEmpty() ||
+ base::IsManagedDevice();
}
std::string GroupPolicyManager::source() const {
@@ -204,7 +213,7 @@
void GroupPolicyManager::LoadAllPolicies() {
scoped_hpolicy policy_lock;
- if (base::IsManagedDevice()) {
+ if (IsManagedInternal()) {
// GPO rules mandate a call to EnterCriticalPolicySection() before reading
// policies (and a matching LeaveCriticalPolicySection() call after read).
// Acquire the lock for managed machines because group policies are
@@ -214,6 +223,11 @@
CHECK(policy_lock.is_valid()) << "Failed to get policy lock.";
}
+ if (!external_constants_group_policies_.DictEmpty()) {
+ policies_ = external_constants_group_policies_.Clone();
+ return;
+ }
+
base::Value::DictStorage policy_storage;
for (base::win::RegistryValueIterator it(HKEY_LOCAL_MACHINE,
diff --git a/chrome/updater/policy/win/group_policy_manager.h b/chrome/updater/policy/win/group_policy_manager.h
index 0913e37..713b8066 100644
--- a/chrome/updater/policy/win/group_policy_manager.h
+++ b/chrome/updater/policy/win/group_policy_manager.h
@@ -7,7 +7,9 @@
#include <string>
+#include "base/memory/scoped_refptr.h"
#include "base/values.h"
+#include "chrome/updater/external_constants.h"
#include "chrome/updater/policy/manager.h"
namespace updater {
@@ -15,7 +17,8 @@
// The GroupPolicyManager returns policies for domain-joined machines.
class GroupPolicyManager : public PolicyManagerInterface {
public:
- GroupPolicyManager();
+ explicit GroupPolicyManager(
+ scoped_refptr<ExternalConstants> external_constants);
GroupPolicyManager(const GroupPolicyManager&) = delete;
GroupPolicyManager& operator=(const GroupPolicyManager&) = delete;
~GroupPolicyManager() override;
@@ -49,11 +52,13 @@
bool GetProxyServer(std::string* proxy_server) const override;
private:
+ bool IsManagedInternal() const;
void LoadAllPolicies();
bool GetIntPolicy(const std::string& key, int* value) const;
bool GetStringPolicy(const std::string& key, std::string* value) const;
base::Value policies_;
+ const base::Value external_constants_group_policies_;
};
} // namespace updater
diff --git a/chrome/updater/policy/win/group_policy_manager_unittest.cc b/chrome/updater/policy/win/group_policy_manager_unittest.cc
index 0191681..7f9dcea 100644
--- a/chrome/updater/policy/win/group_policy_manager_unittest.cc
+++ b/chrome/updater/policy/win/group_policy_manager_unittest.cc
@@ -48,7 +48,7 @@
TEST_F(GroupPolicyManagerTests, NoPolicySet) {
std::unique_ptr<PolicyManagerInterface> policy_manager =
- std::make_unique<GroupPolicyManager>();
+ std::make_unique<GroupPolicyManager>(CreateExternalConstants());
EXPECT_FALSE(policy_manager->IsManaged());
EXPECT_EQ(policy_manager->source(), "GroupPolicy");
@@ -142,7 +142,7 @@
key.WriteValue(L"RollbackToTargetVersion" TEST_APP_ID, 1));
std::unique_ptr<PolicyManagerInterface> policy_manager =
- std::make_unique<GroupPolicyManager>();
+ std::make_unique<GroupPolicyManager>(CreateExternalConstants());
EXPECT_EQ(policy_manager->IsManaged(), base::win::IsEnrolledToDomain());
int check_period = 0;
@@ -250,7 +250,7 @@
key.WriteValue(L"RollbackToTargetVersion" TEST_APP_ID, L"1"));
std::unique_ptr<PolicyManagerInterface> policy_manager =
- std::make_unique<GroupPolicyManager>();
+ std::make_unique<GroupPolicyManager>(CreateExternalConstants());
int check_period = 0;
EXPECT_FALSE(policy_manager->GetLastCheckPeriodMinutes(&check_period));
diff --git a/chrome/updater/test/integration_test_commands.h b/chrome/updater/test/integration_test_commands.h
index a512682..39a797bb 100644
--- a/chrome/updater/test/integration_test_commands.h
+++ b/chrome/updater/test/integration_test_commands.h
@@ -8,6 +8,7 @@
#include <string>
#include "base/memory/ref_counted.h"
+#include "base/values.h"
#include "build/build_config.h"
#include "chrome/updater/test/integration_tests_impl.h"
#include "chrome/updater/update_service.h"
@@ -28,6 +29,8 @@
: public base::RefCountedThreadSafe<IntegrationTestCommands> {
public:
virtual void EnterTestMode(const GURL& url) const = 0;
+ virtual void SetGroupPolicies(
+ const base::Value::DictStorage& values) const = 0;
virtual void Clean() const = 0;
virtual void ExpectClean() const = 0;
virtual void ExpectInstalled() const = 0;
diff --git a/chrome/updater/test/integration_test_commands_system.cc b/chrome/updater/test/integration_test_commands_system.cc
index 66ea329..8a22b04 100644
--- a/chrome/updater/test/integration_test_commands_system.cc
+++ b/chrome/updater/test/integration_test_commands_system.cc
@@ -11,10 +11,12 @@
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
+#include "base/json/json_writer.h"
#include "base/memory/scoped_refptr.h"
#include "base/notreached.h"
#include "base/path_service.h"
#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
#include "base/version.h"
#include "build/build_config.h"
#include "chrome/updater/constants.h"
@@ -37,6 +39,16 @@
namespace updater {
namespace test {
+namespace {
+
+std::string StringFromDictStorage(const base::Value::DictStorage& values) {
+ std::string values_string;
+ EXPECT_TRUE(base::JSONWriter::Write(base::Value(values), &values_string));
+ return values_string;
+}
+
+} // namespace
+
class IntegrationTestCommandsSystem : public IntegrationTestCommands {
public:
IntegrationTestCommandsSystem() = default;
@@ -68,6 +80,11 @@
RunCommand("enter_test_mode", {Param("url", url.spec())});
}
+ void SetGroupPolicies(const base::Value::DictStorage& values) const override {
+ RunCommand("set_group_policies",
+ {Param("values", StringFromDictStorage(values))});
+ }
+
void ExpectSelfUpdateSequence(ScopedServer* test_server) const override {
updater::test::ExpectSelfUpdateSequence(updater_scope_, test_server);
}
diff --git a/chrome/updater/test/integration_test_commands_user.cc b/chrome/updater/test/integration_test_commands_user.cc
index 44ebaae..421e5f1 100644
--- a/chrome/updater/test/integration_test_commands_user.cc
+++ b/chrome/updater/test/integration_test_commands_user.cc
@@ -65,6 +65,10 @@
updater::test::ExpectSelfUpdateSequence(updater_scope_, test_server);
}
+ void SetGroupPolicies(const base::Value::DictStorage& values) const override {
+ updater::test::SetGroupPolicies(values);
+ }
+
void ExpectUpdateSequence(ScopedServer* test_server,
const std::string& app_id,
const std::string& install_data_index,
diff --git a/chrome/updater/test/integration_tests.cc b/chrome/updater/test/integration_tests.cc
index acfa3d9..7cca93b 100644
--- a/chrome/updater/test/integration_tests.cc
+++ b/chrome/updater/test/integration_tests.cc
@@ -20,6 +20,7 @@
#include "base/test/task_environment.h"
#include "base/test/test_timeouts.h"
#include "base/time/time.h"
+#include "base/values.h"
#include "base/version.h"
#include "build/branding_buildflags.h"
#include "build/build_config.h"
@@ -134,6 +135,10 @@
void EnterTestMode(const GURL& url) { test_commands_->EnterTestMode(url); }
+ void SetGroupPolicies(const base::Value::DictStorage& values) {
+ test_commands_->SetGroupPolicies(values);
+ }
+
void ExpectVersionActive(const std::string& version) {
test_commands_->ExpectVersionActive(version);
}
@@ -486,35 +491,23 @@
ExpectNoUpdateSequence(&test_server, kAppId);
ExpectLegacyUpdate3WebSucceeds(kAppId, STATE_NO_UPDATE, S_OK);
+ base::Value::DictStorage group_policies;
+ group_policies["Updatetest1"] = base::Value(kPolicyAutomaticUpdatesOnly);
+ SetGroupPolicies(group_policies);
+ ExpectLegacyUpdate3WebSucceeds(
+ kAppId, STATE_ERROR, GOOPDATE_E_APP_UPDATE_DISABLED_BY_POLICY_MANUAL);
+
+ group_policies["Updatetest1"] = base::Value(kPolicyDisabled);
+ SetGroupPolicies(group_policies);
+ ExpectLegacyUpdate3WebSucceeds(kAppId, STATE_ERROR,
+ GOOPDATE_E_APP_UPDATE_DISABLED_BY_POLICY);
+
+ group_policies.clear();
+ SetGroupPolicies(group_policies);
ExpectUpdateSequence(&test_server, kAppId, "", base::Version("0.1"),
base::Version("0.2"));
ExpectLegacyUpdate3WebSucceeds(kAppId, STATE_INSTALL_COMPLETE, S_OK);
- // TODO(crbug.com/1272853) - Need administrative access to be able to write
- // under the policies key.
- if (::IsUserAnAdmin()) {
- base::win::RegKey key(HKEY_LOCAL_MACHINE, UPDATER_POLICIES_KEY,
- Wow6432(KEY_ALL_ACCESS));
-
- EXPECT_EQ(ERROR_SUCCESS,
- key.WriteValue(
- base::StrCat({L"Update", base::UTF8ToWide(kAppId)}).c_str(),
- kPolicyAutomaticUpdatesOnly));
- ExpectLegacyUpdate3WebSucceeds(
- kAppId, STATE_ERROR, GOOPDATE_E_APP_UPDATE_DISABLED_BY_POLICY_MANUAL);
-
- EXPECT_EQ(ERROR_SUCCESS,
- key.WriteValue(
- base::StrCat({L"Update", base::UTF8ToWide(kAppId)}).c_str(),
- static_cast<DWORD>(kPolicyDisabled)));
- ExpectLegacyUpdate3WebSucceeds(kAppId, STATE_ERROR,
- GOOPDATE_E_APP_UPDATE_DISABLED_BY_POLICY);
-
- EXPECT_EQ(ERROR_SUCCESS,
- base::win::RegKey(HKEY_LOCAL_MACHINE, L"", Wow6432(DELETE))
- .DeleteKey(UPDATER_POLICIES_KEY));
- }
-
Uninstall();
}
diff --git a/chrome/updater/test/integration_tests_helper.cc b/chrome/updater/test/integration_tests_helper.cc
index 45f0a734..7b318c3 100644
--- a/chrome/updater/test/integration_tests_helper.cc
+++ b/chrome/updater/test/integration_tests_helper.cc
@@ -12,6 +12,7 @@
#include "base/callback.h"
#include "base/check.h"
#include "base/command_line.h"
+#include "base/json/json_reader.h"
#include "base/logging.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
@@ -23,6 +24,7 @@
#include "base/test/test_suite.h"
#include "base/threading/thread_restrictions.h"
#include "base/threading/thread_task_runner_handle.h"
+#include "base/values.h"
#include "base/version.h"
#include "build/build_config.h"
#include "chrome/common/chrome_paths.h"
@@ -31,6 +33,7 @@
#include "chrome/updater/test/integration_tests_impl.h"
#include "chrome/updater/updater_scope.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"
#if BUILDFLAG(IS_WIN)
@@ -54,6 +57,12 @@
constexpr int kUnknownSwitch = 101;
constexpr int kBadCommand = 102;
+base::Value::DictStorage DictStorageFromString(const std::string& values) {
+ absl::optional<base::Value> results_value = base::JSONReader::Read(values);
+ EXPECT_TRUE(results_value);
+ return results_value->Clone().TakeDictDeprecated();
+}
+
template <typename... Args>
base::RepeatingCallback<bool(Args...)> WithSwitch(
const std::string& flag,
@@ -138,6 +147,19 @@
}));
}
+// Overload for base::Value::DictStorage switches.
+template <typename... Args>
+base::RepeatingCallback<bool(Args...)> WithSwitch(
+ const std::string& flag,
+ base::RepeatingCallback<bool(const base::Value::DictStorage&, Args...)>
+ callback) {
+ return WithSwitch(
+ flag,
+ base::BindLambdaForTesting([=](const std::string& flag, Args... args) {
+ return callback.Run(DictStorageFromString(flag), std::move(args)...);
+ }));
+}
+
template <typename Arg, typename... RemainingArgs>
base::RepeatingCallback<bool(RemainingArgs...)> WithArg(
Arg arg,
@@ -198,6 +220,7 @@
// then use the With* helper functions to provide its arguments.
{"clean", WithSystemScope(Wrap(&Clean))},
{"enter_test_mode", WithSwitch("url", Wrap(&EnterTestMode))},
+ {"set_group_policies", WithSwitch("values", Wrap(&SetGroupPolicies))},
{"expect_active_updater", WithSystemScope(Wrap(&ExpectActiveUpdater))},
{"expect_registered",
WithSwitch("app_id", WithSystemScope(Wrap(&ExpectRegistered)))},
diff --git a/chrome/updater/test/integration_tests_impl.cc b/chrome/updater/test/integration_tests_impl.cc
index 5982806..99b674d 100644
--- a/chrome/updater/test/integration_tests_impl.cc
+++ b/chrome/updater/test/integration_tests_impl.cc
@@ -42,6 +42,7 @@
#include "build/build_config.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/updater/constants.h"
+#include "chrome/updater/external_constants_builder.h"
#include "chrome/updater/persisted_data.h"
#include "chrome/updater/prefs.h"
#include "chrome/updater/registration_data.h"
@@ -188,6 +189,10 @@
loop.Run();
}
+void SetGroupPolicies(const base::Value::DictStorage& values) {
+ ASSERT_TRUE(ExternalConstantsBuilder().SetGroupPolicies(values).Modify());
+}
+
void ExpectVersionActive(UpdaterScope scope, const std::string& version) {
scoped_refptr<GlobalPrefs> prefs = CreateGlobalPrefs(scope);
ASSERT_NE(prefs, nullptr) << "Failed to acquire GlobalPrefs.";
diff --git a/chrome/updater/test/integration_tests_impl.h b/chrome/updater/test/integration_tests_impl.h
index d9d0bdaa..29d6a7f 100644
--- a/chrome/updater/test/integration_tests_impl.h
+++ b/chrome/updater/test/integration_tests_impl.h
@@ -12,6 +12,7 @@
#include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h"
#include "base/memory/ref_counted.h"
+#include "base/values.h"
#include "build/build_config.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
@@ -51,6 +52,9 @@
// CUP).
void EnterTestMode(const GURL& url);
+// Sets the external constants for group policies.
+void SetGroupPolicies(const base::Value::DictStorage& values);
+
// Copies the logs to a location where they can be retrieved by ResultDB.
void CopyLog(const base::FilePath& src_dir);
diff --git a/chrome/updater/test/integration_tests_mac.mm b/chrome/updater/test/integration_tests_mac.mm
index c6b3a6f7..f8e5212 100644
--- a/chrome/updater/test/integration_tests_mac.mm
+++ b/chrome/updater/test/integration_tests_mac.mm
@@ -120,7 +120,7 @@
.SetInitialDelay(0.1)
.SetServerKeepAliveSeconds(1)
.SetCrxVerifierFormat(crx_file::VerifierFormat::CRX3)
- .Overwrite());
+ .Modify());
}
absl::optional<base::FilePath> GetDataDirPath(UpdaterScope scope) {
diff --git a/chrome/updater/test/integration_tests_win.cc b/chrome/updater/test/integration_tests_win.cc
index 4c12f1b..0f38438 100644
--- a/chrome/updater/test/integration_tests_win.cc
+++ b/chrome/updater/test/integration_tests_win.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <shlobj.h>
#include <wrl/client.h>
#include <regstr.h>
@@ -245,10 +246,12 @@
.HasSwitch(kWakeSwitch));
}
} else {
- for (const wchar_t* key :
- {kRegKeyCompanyCloudManagement, kRegKeyCompanyEnrollment,
- UPDATER_POLICIES_KEY}) {
- EXPECT_FALSE(RegKeyExists(HKEY_LOCAL_MACHINE, key));
+ if (::IsUserAnAdmin()) {
+ for (const wchar_t* key :
+ {kRegKeyCompanyCloudManagement, kRegKeyCompanyEnrollment,
+ UPDATER_POLICIES_KEY}) {
+ EXPECT_FALSE(RegKeyExists(HKEY_LOCAL_MACHINE, key));
+ }
}
EXPECT_FALSE(RegKeyExists(root, UPDATER_KEY));
@@ -428,9 +431,13 @@
for (const wchar_t* key : {CLIENT_STATE_KEY, CLIENTS_KEY, UPDATER_KEY}) {
EXPECT_TRUE(DeleteRegKey(root, key));
}
- for (const wchar_t* key : {kRegKeyCompanyCloudManagement,
- kRegKeyCompanyEnrollment, UPDATER_POLICIES_KEY}) {
- EXPECT_TRUE(DeleteRegKey(HKEY_LOCAL_MACHINE, key));
+
+ if (::IsUserAnAdmin()) {
+ for (const wchar_t* key :
+ {kRegKeyCompanyCloudManagement, kRegKeyCompanyEnrollment,
+ UPDATER_POLICIES_KEY}) {
+ EXPECT_TRUE(DeleteRegKey(HKEY_LOCAL_MACHINE, key));
+ }
}
for (const CLSID& clsid :
@@ -487,7 +494,7 @@
.SetUseCUP(false)
.SetInitialDelay(0.1)
.SetCrxVerifierFormat(crx_file::VerifierFormat::CRX3)
- .Overwrite());
+ .Modify());
}
void ExpectInstalled(UpdaterScope scope) {
diff --git a/docs/updater/functional_spec.md b/docs/updater/functional_spec.md
index 2cec12ca..605209439 100644
--- a/docs/updater/functional_spec.md
+++ b/docs/updater/functional_spec.md
@@ -714,6 +714,8 @@
* `use_cup`: Whether CUP is used at all.
* `cup_public_key`: An unarmored PEM-encoded ASN.1 SubjectPublicKeyInfo with
the ecPublicKey algorithm and containing a named elliptic curve.
+* `group_policies`: Allows setting group policies, such as install and update
+ policies.
Windows: these overrides exist in registry, under
`HKLM\Software\{Company}\Update\Clients\ClientState\UpdateDev`.