Remove ActiveDirectory code from BrowserPolicyConnectorAsh
As documented in crrev.com/c/4463118, it is safe to assume that
InstallAttributes::IsActiveDirectoryManaged() always returns false.
This CL removes respective active directory related code blocks.
It also removes a browser test from
chrome/browser/ash/policy/active_directory to break a circular
dependency. Note that we intend to remove ALL files from that
directory in b/274762979 anyway.
Bug: b/279364186 b/274762979
Change-Id: Icb295677d4d96074bba60c9f012b3b4ed6079286
Tests: Existing unit and browser tests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4466053
Reviewed-by: Igor <igorcov@chromium.org>
Commit-Queue: Roland Bock <rbock@google.com>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1135951}
diff --git a/chrome/browser/ash/policy/active_directory/component_active_directory_policy_browsertest.cc b/chrome/browser/ash/policy/active_directory/component_active_directory_policy_browsertest.cc
deleted file mode 100644
index f7e7c073..0000000
--- a/chrome/browser/ash/policy/active_directory/component_active_directory_policy_browsertest.cc
+++ /dev/null
@@ -1,220 +0,0 @@
-// Copyright 2018 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <string>
-
-#include "ash/constants/ash_switches.h"
-#include "base/command_line.h"
-#include "base/files/file_path.h"
-#include "base/functional/bind.h"
-#include "base/memory/ref_counted.h"
-#include "base/path_service.h"
-#include "chrome/browser/extensions/extension_browsertest.h"
-#include "chrome/browser/policy/profile_policy_connector.h"
-#include "chrome/common/chrome_paths.h"
-#include "chromeos/ash/components/cryptohome/cryptohome_parameters.h"
-#include "chromeos/ash/components/dbus/login_manager/policy_descriptor.pb.h"
-#include "chromeos/ash/components/dbus/session_manager/fake_session_manager_client.h"
-#include "chromeos/ash/components/install_attributes/stub_install_attributes.h"
-#include "components/policy/core/common/cloud/cloud_policy_constants.h"
-#include "components/policy/core/common/cloud/test/policy_builder.h"
-#include "components/policy/core/common/policy_service.h"
-#include "components/user_manager/user_names.h"
-#include "content/public/test/browser_test.h"
-#include "extensions/common/extension.h"
-#include "extensions/test/extension_test_message_listener.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace policy {
-
-namespace {
-
-constexpr char kTestDomain[] = "test_domain";
-constexpr char kTestDeviceId[] = "test_device_id";
-
-constexpr char kTestExtensionId[] = "kjmkgkdkpedkejedfhmfcenooemhbpbo";
-constexpr base::FilePath::CharType kTestExtensionPath[] =
- FILE_PATH_LITERAL("extensions/managed_extension");
-
-constexpr char kTestExtension2Id[] = "behllobkkfkfnphdnhnkndlbkcpglgmj";
-constexpr base::FilePath::CharType kTestExtension2Path[] =
- FILE_PATH_LITERAL("extensions/managed_extension2");
-
-constexpr char kTestPolicy[] = R"({
- "Policy": {
- "Name": "disable_all_the_things"
- }
-})";
-constexpr char kTestPolicyJSON[] = R"({"Name":"disable_all_the_things"})";
-
-constexpr char kTestPolicy2[] = R"({
- "Policy": {
- "Another": "turn_it_off"
- }
-})";
-constexpr char kTestPolicy2JSON[] = R"({"Another":"turn_it_off"})";
-
-constexpr char kEmptyPolicy[] = "{}";
-
-void ExpectSuccess(base::OnceClosure callback, bool result) {
- EXPECT_TRUE(result);
- std::move(callback).Run();
-}
-
-} // namespace
-
-class ComponentActiveDirectoryPolicyTest
- : public extensions::ExtensionBrowserTest {
- protected:
- ComponentActiveDirectoryPolicyTest()
- : install_attributes_(
- ash::StubInstallAttributes::CreateActiveDirectoryManaged(
- kTestDomain,
- kTestDeviceId)) {
- builder_.policy_data().set_policy_type(
- dm_protocol::kChromeExtensionPolicyType);
- builder_.policy_data().set_settings_entity_id(kTestExtensionId);
- }
-
- void SetUpCommandLine(base::CommandLine* command_line) override {
- ExtensionBrowserTest::SetUpCommandLine(command_line);
-
- // Log in as Active Directory user.
- command_line->AppendSwitchASCII(::ash::switches::kLoginUser,
- ::user_manager::kStubAdUserEmail);
-
- // Without this, user manager code will shut down Chrome since it can't
- // find any policy.
- command_line->AppendSwitchASCII(
- ::ash::switches::kAllowFailedPolicyFetchForTest, "true");
- }
-
- void SetUpOnMainThread() override {
- ExtensionBrowserTest::SetUpOnMainThread();
-
- // Install the initial extension.
- ExtensionTestMessageListener ready_listener("ready");
- event_listener_ = std::make_unique<ExtensionTestMessageListener>(
- "event", ReplyBehavior::kWillReply);
- extension_ = LoadExtension(kTestExtensionPath);
- ASSERT_TRUE(extension_.get());
- ASSERT_EQ(kTestExtensionId, extension_->id());
- EXPECT_TRUE(ready_listener.WaitUntilSatisfied());
-
- // Store test extension policy
- StorePolicy(kTestExtensionId, kTestPolicy);
- RefreshPolicies();
-
- // The extension will receive an update event.
- EXPECT_TRUE(event_listener_->WaitUntilSatisfied());
- }
-
- void TearDownOnMainThread() override {
- event_listener_.reset();
- ExtensionBrowserTest::TearDownOnMainThread();
- }
-
- scoped_refptr<const extensions::Extension> LoadExtension(
- const base::FilePath::CharType* path) {
- base::FilePath full_path;
- if (!base::PathService::Get(chrome::DIR_TEST_DATA, &full_path)) {
- ADD_FAILURE();
- return nullptr;
- }
- scoped_refptr<const extensions::Extension> extension(
- ExtensionBrowserTest::LoadExtension(full_path.Append(path)));
- if (!extension.get()) {
- ADD_FAILURE();
- return nullptr;
- }
- return extension;
- }
-
- void StorePolicy(const char* extension_id, const char* policy) {
- const AccountId& account_id = user_manager::StubAdAccountId();
- login_manager::PolicyDescriptor descriptor;
- descriptor.set_account_type(login_manager::ACCOUNT_TYPE_USER);
- descriptor.set_account_id(cryptohome::Identification(account_id).id());
- descriptor.set_domain(login_manager::POLICY_DOMAIN_EXTENSIONS);
- descriptor.set_component_id(extension_id);
-
- builder_.set_payload(policy);
- builder_.Build();
- base::RunLoop run_loop;
- ash::FakeSessionManagerClient::Get()->StorePolicy(
- descriptor, builder_.GetBlob(),
- base::BindOnce(&ExpectSuccess, run_loop.QuitClosure()));
- run_loop.Run();
- }
-
- void RefreshPolicies() {
- ProfilePolicyConnector* profile_connector =
- browser()->profile()->GetProfilePolicyConnector();
- PolicyService* policy_service = profile_connector->policy_service();
- base::RunLoop run_loop;
- policy_service->RefreshPolicies(run_loop.QuitClosure());
- run_loop.Run();
- }
-
- scoped_refptr<const extensions::Extension> extension_;
- std::unique_ptr<ExtensionTestMessageListener> event_listener_;
- ash::ScopedStubInstallAttributes install_attributes_;
- ComponentActiveDirectoryPolicyBuilder builder_;
-};
-
-// Checks policy "Name" is set with expected value.
-IN_PROC_BROWSER_TEST_F(ComponentActiveDirectoryPolicyTest,
- FetchExtensionPolicy) {
- // Read the initial policy.
- ExtensionTestMessageListener policy_listener(kTestPolicyJSON);
- event_listener_->Reply("get-policy-Name");
- EXPECT_TRUE(policy_listener.WaitUntilSatisfied());
-}
-
-// Uploads "Another" and verifies it. Also verifies that "Name" doesn't exist
-// anymore.
-IN_PROC_BROWSER_TEST_F(ComponentActiveDirectoryPolicyTest,
- UpdateExtensionPolicy) {
- // Update and reload policy and make sure that the update event was received.
- event_listener_->Reply("idle");
- event_listener_->Reset();
- StorePolicy(kTestExtensionId, kTestPolicy2);
- RefreshPolicies();
- EXPECT_TRUE(event_listener_->WaitUntilSatisfied());
-
- // This policy was removed.
- ExtensionTestMessageListener policy_listener1(kEmptyPolicy,
- ReplyBehavior::kWillReply);
- event_listener_->Reply("get-policy-Name");
- EXPECT_TRUE(policy_listener1.WaitUntilSatisfied());
-
- ExtensionTestMessageListener policy_listener2(kTestPolicy2JSON);
- policy_listener1.Reply("get-policy-Another");
- EXPECT_TRUE(policy_listener2.WaitUntilSatisfied());
-}
-
-IN_PROC_BROWSER_TEST_F(ComponentActiveDirectoryPolicyTest,
- InstallNewExtension) {
- event_listener_->Reply("idle");
- event_listener_.reset();
-
- // Store policy 2 for extension 2.
- StorePolicy(kTestExtension2Id, kTestPolicy2);
-
- // Installing a new extension should trigger a schema update, which should
- // trigger a policy refresh.
- ExtensionTestMessageListener result_listener("ok");
- result_listener.set_failure_message("fail");
- scoped_refptr<const extensions::Extension> extension2 =
- LoadExtension(kTestExtension2Path);
- ASSERT_TRUE(extension2.get());
- ASSERT_EQ(kTestExtension2Id, extension2->id());
-
- // This extension only sends the 'policy' signal once it receives the policy,
- // and after verifying it has the expected value. Otherwise it sends 'fail'.
- EXPECT_TRUE(result_listener.WaitUntilSatisfied());
-}
-
-} // namespace policy
diff --git a/chrome/browser/ash/policy/core/browser_policy_connector_ash.cc b/chrome/browser/ash/policy/core/browser_policy_connector_ash.cc
index 9ec9b90..3dabaaca 100644
--- a/chrome/browser/ash/policy/core/browser_policy_connector_ash.cc
+++ b/chrome/browser/ash/policy/core/browser_policy_connector_ash.cc
@@ -110,11 +110,6 @@
return MarketSegment::UNKNOWN;
}
-// Checks whether forced re-enrollment is enabled.
-bool IsForcedReEnrollmentEnabled() {
- return AutoEnrollmentTypeChecker::IsFREEnabled();
-}
-
std::unique_ptr<ash::attestation::AttestationFlow> CreateAttestationFlow() {
return std::make_unique<ash::attestation::AttestationFlowAdaptive>(
std::make_unique<ash::attestation::AttestationCAClient>());
@@ -143,37 +138,24 @@
ash::DeviceSettingsService::Get(), ash::InstallAttributes::Get(),
CreateBackgroundTaskRunner());
- if (ash::InstallAttributes::Get()->IsActiveDirectoryManaged()) {
- ash::UpstartClient::Get()->StartAuthPolicyService();
+ state_keys_broker_ = std::make_unique<ServerBackedStateKeysBroker>(
+ ash::SessionManagerClient::Get());
- device_active_directory_policy_manager_ =
- new DeviceActiveDirectoryPolicyManager(
- std::move(device_cloud_policy_store));
- providers_for_init_.push_back(
- base::WrapUnique<ConfigurationPolicyProvider>(
- device_active_directory_policy_manager_.get()));
- } else {
- state_keys_broker_ = std::make_unique<ServerBackedStateKeysBroker>(
- ash::SessionManagerClient::Get());
+ const base::FilePath device_policy_external_data_path =
+ base::PathService::CheckedGet(ash::DIR_DEVICE_POLICY_EXTERNAL_DATA);
- const base::FilePath device_policy_external_data_path =
- base::PathService::CheckedGet(ash::DIR_DEVICE_POLICY_EXTERNAL_DATA);
+ auto external_data_manager =
+ std::make_unique<DevicePolicyCloudExternalDataManager>(
+ base::BindRepeating(&GetChromePolicyDetails),
+ CreateBackgroundTaskRunner(), device_policy_external_data_path,
+ device_cloud_policy_store.get());
- auto external_data_manager =
- std::make_unique<DevicePolicyCloudExternalDataManager>(
- base::BindRepeating(&GetChromePolicyDetails),
- CreateBackgroundTaskRunner(), device_policy_external_data_path,
- device_cloud_policy_store.get());
-
- device_cloud_policy_manager_ = new DeviceCloudPolicyManagerAsh(
- std::move(device_cloud_policy_store),
- std::move(external_data_manager),
- base::SingleThreadTaskRunner::GetCurrentDefault(),
- state_keys_broker_.get());
- providers_for_init_.push_back(
- base::WrapUnique<ConfigurationPolicyProvider>(
- device_cloud_policy_manager_.get()));
- }
+ device_cloud_policy_manager_ = new DeviceCloudPolicyManagerAsh(
+ std::move(device_cloud_policy_store), std::move(external_data_manager),
+ base::SingleThreadTaskRunner::GetCurrentDefault(),
+ state_keys_broker_.get());
+ providers_for_init_.push_back(base::WrapUnique<ConfigurationPolicyProvider>(
+ device_cloud_policy_manager_.get()));
}
global_user_cloud_policy_provider_ = new ProxyPolicyProvider();
@@ -204,34 +186,14 @@
RestartDeviceCloudPolicyInitializer();
}
- if (!ash::InstallAttributes::Get()->IsActiveDirectoryManaged()) {
- device_local_account_policy_service_ =
- std::make_unique<DeviceLocalAccountPolicyService>(
- ash::SessionManagerClient::Get(), ash::DeviceSettingsService::Get(),
- ash::CrosSettings::Get(),
- affiliated_invalidation_service_provider_.get(),
- CreateBackgroundTaskRunner(), CreateBackgroundTaskRunner(),
- CreateBackgroundTaskRunner(), url_loader_factory);
- device_local_account_policy_service_->Connect(device_management_service());
- } else if (IsForcedReEnrollmentEnabled()) {
- // Initialize state keys and enrollment ID upload mechanisms to DM Server in
- // Active Directory mode.
- state_keys_broker_ = std::make_unique<ServerBackedStateKeysBroker>(
- ash::SessionManagerClient::Get());
- active_directory_device_state_uploader_ =
- std::make_unique<ActiveDirectoryDeviceStateUploader>(
- /*client_id=*/GetInstallAttributes()->GetDeviceId(),
- device_management_service(), state_keys_broker_.get(),
- url_loader_factory, std::make_unique<DMTokenStorage>(local_state),
- local_state);
- active_directory_device_state_uploader_->Init();
-
- // Initialize the manager that will start the migration of Chromad devices
- // into cloud management, when all pre-requisites are met.
- active_directory_migration_manager_ =
- std::make_unique<ActiveDirectoryMigrationManager>(local_state);
- active_directory_migration_manager_->Init();
- }
+ device_local_account_policy_service_ =
+ std::make_unique<DeviceLocalAccountPolicyService>(
+ ash::SessionManagerClient::Get(), ash::DeviceSettingsService::Get(),
+ ash::CrosSettings::Get(),
+ affiliated_invalidation_service_provider_.get(),
+ CreateBackgroundTaskRunner(), CreateBackgroundTaskRunner(),
+ CreateBackgroundTaskRunner(), url_loader_factory);
+ device_local_account_policy_service_->Connect(device_management_service());
if (device_cloud_policy_manager_) {
device_cloud_policy_invalidator_ =
diff --git a/chrome/browser/ash/policy/core/browser_policy_connector_ash.h b/chrome/browser/ash/policy/core/browser_policy_connector_ash.h
index bd9ed32..cc31eb4 100644
--- a/chrome/browser/ash/policy/core/browser_policy_connector_ash.h
+++ b/chrome/browser/ash/policy/core/browser_policy_connector_ash.h
@@ -98,6 +98,10 @@
bool IsCloudManaged() const;
// Checks whether this is an Active Directory managed enterprise device.
+ // In theory, this can still yield true for a few left-over AD devices.
+ // However, starting in M114, it is considered safe to assume that this
+ // function always returns false.
+ // TODO(b/279364186) Remove.
bool IsActiveDirectoryManaged() const;
// Returns the enterprise enrollment domain if device is managed.
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 50e420fc..51a46bb 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -4037,7 +4037,6 @@
"../browser/ash/os_url_handler_browsertest.cc",
"../browser/ash/platform_keys/key_permissions/key_permissions_manager_browsertest.cc",
"../browser/ash/platform_keys/platform_keys_service_browsertest.cc",
- "../browser/ash/policy/active_directory/component_active_directory_policy_browsertest.cc",
"../browser/ash/policy/affiliation/affiliation_mixin.cc",
"../browser/ash/policy/affiliation/affiliation_mixin.h",
"../browser/ash/policy/affiliation/affiliation_test_helper.cc",
