Respect 'disable enterprise CRD' policy in FetchCrdAvailabilityInfo r…

…emote command

The RemoteAccessHostAllowEnterpriseRemoteSupportConnections policy is
used to prevent an enterprise admin from starting a CRD connection;
Currently it works by failing the StartCrdSession remote command,
but it's much nicer for the enterprise admin if this policy is checked
by the FetchCrdAvailabilityInfo remote command as well, since that is
supposed to tell the admin of they can start a CRD session (and if not,
tell them why).

To make this work, I also had to introduce a pref to go with this
policy.

Manual tests performed: Start CRD session when device is:
   * at login screen.
   * in MGS, both with and without the policy set
   * in Kiosk (where the policy can not be applied).
   * in affiliated user session, both with and without the policy set.


Bug: b/282896831
Change-Id: Iaf1e1f846ef6067757fa890e47f2e1e975983443
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4909635
Reviewed-by: Sergii Bykov <sbykov@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/main@{#1210774}

chrome/browser/ash/policy/remote_commands/crd_admin_session_controller.cc

@@ -423,6 +423,8 @@ bool CrdAdminSessionController::IsCurrentSessionCurtained() const {
 void CrdAdminSessionController::RegisterLocalStatePrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kRemoteAdminWasPresent, false);
+  registry->RegisterBooleanPref(
+      prefs::kRemoteAccessHostAllowEnterpriseRemoteSupportConnections, true);
 }
 
 }  // namespace policy

chrome/browser/ash/policy/remote_commands/device_command_fetch_crd_availability_info_job.cc

@@ -4,13 +4,17 @@
 
 #include "chrome/browser/ash/policy/remote_commands/device_command_fetch_crd_availability_info_job.h"
 
+#include "base/check_deref.h"
 #include "base/functional/bind.h"
 #include "base/json/json_writer.h"
 #include "base/numerics/clamped_math.h"
 #include "base/time/time.h"
 #include "chrome/browser/ash/policy/remote_commands/crd_logging.h"
 #include "chrome/browser/ash/policy/remote_commands/crd_remote_command_utils.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/common/pref_names.h"
 #include "components/policy/core/common/remote_commands/remote_command_job.h"
+#include "components/prefs/pref_service.h"
 
 namespace policy {
 
@@ -41,8 +45,17 @@ base::Value::List GetSupportedSessionTypes(bool is_in_managed_environment) {
   return result;
 }
 
+bool IsAllowedByPolicy() {
+  return CHECK_DEREF(g_browser_process->local_state())
+      .GetBoolean(
+          prefs::kRemoteAccessHostAllowEnterpriseRemoteSupportConnections);
+}
+
 CrdSessionAvailability GetRemoteSupportAvailability(
     UserSessionType current_user_session) {
+  if (!IsAllowedByPolicy()) {
+    return CrdSessionAvailability::UNAVAILABLE_DISABLED_BY_POLICY;
+  }
   if (!UserSessionSupportsRemoteSupport(current_user_session)) {
     return CrdSessionAvailability::UNAVAILABLE_UNSUPPORTED_USER_SESSION_TYPE;
   }
@@ -52,6 +65,9 @@ CrdSessionAvailability GetRemoteSupportAvailability(
 CrdSessionAvailability GetRemoteAccessAvailability(
     bool is_in_managed_environment,
     UserSessionType current_user_session) {
+  if (!IsAllowedByPolicy()) {
+    return CrdSessionAvailability::UNAVAILABLE_DISABLED_BY_POLICY;
+  }
   if (!is_in_managed_environment) {
     return CrdSessionAvailability::UNAVAILABLE_UNMANAGED_ENVIRONMENT;
   }

chrome/browser/ash/policy/remote_commands/device_command_fetch_crd_availability_info_job_unittest.cc

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/check_deref.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/test_future.h"
 #include "base/test/values_test_util.h"
@@ -20,6 +21,8 @@
 #include "chrome/browser/ash/policy/remote_commands/fake_cros_network_config.h"
 #include "chrome/browser/ash/policy/remote_commands/user_session_type_test_util.h"
 #include "chrome/browser/ash/settings/device_settings_test_helper.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/user_activity/user_activity_detector.h"
@@ -40,6 +43,7 @@ using test::TestSessionType;
 using testing::Not;
 
 constexpr int64_t kUniqueID = 111222333444;
+constexpr TestSessionType kAnySessionType = TestSessionType::kGuestSession;
 
 RemoteCommand GenerateCommandProto() {
   RemoteCommand command_proto;
@@ -102,6 +106,8 @@ class DeviceCommandFetchCrdAvailabilityInfoJobTest
   void SetUp() override {
     DeviceSettingsTestBase::SetUp();
 
+    ASSERT_TRUE(profile_manager_.SetUp());
+
     user_activity_detector_ = std::make_unique<ui::UserActivityDetector>();
     arc_kiosk_app_manager_ = std::make_unique<ash::ArcKioskAppManager>();
     web_kiosk_app_manager_ = std::make_unique<ash::WebKioskAppManager>();
@@ -161,6 +167,14 @@ class DeviceCommandFetchCrdAvailabilityInfoJobTest
             .SetOncSource(OncSource::kDevicePolicy));
   }
 
+  void StartSessionOfType(TestSessionType user_session_type) {
+    test::StartSessionOfType(user_session_type, user_manager());
+  }
+
+  void DisablePref(const char* pref_name) {
+    profile_manager_.local_state()->Get()->SetBoolean(pref_name, false);
+  }
+
  private:
   std::unique_ptr<ash::ArcKioskAppManager> arc_kiosk_app_manager_;
   std::unique_ptr<ash::WebKioskAppManager> web_kiosk_app_manager_;
@@ -170,6 +184,8 @@ class DeviceCommandFetchCrdAvailabilityInfoJobTest
   std::unique_ptr<ui::UserActivityDetector> user_activity_detector_;
 
   test::ScopedFakeCrosNetworkConfig fake_cros_network_config_;
+
+  TestingProfileManager profile_manager_{TestingBrowserProcess::GetGlobal()};
 };
 
 // Fixture for tests parameterized over the possible session types
@@ -231,13 +247,27 @@ TEST_F(DeviceCommandFetchCrdAvailabilityInfoJobTest,
               DictionaryHasValue("isInManagedEnvironment", base::Value(false)));
 }
 
+TEST_F(DeviceCommandFetchCrdAvailabilityInfoJobTest,
+       ShouldRespectDisabledByPolicy) {
+  StartSessionOfType(kAnySessionType);
+
+  DisablePref(prefs::kRemoteAccessHostAllowEnterpriseRemoteSupportConnections);
+
+  Result result = CreateAndRunJob();
+
+  EXPECT_EQ(ParseJsonDict(result.payload).FindInt("remoteAccessAvailability"),
+            CrdSessionAvailability::UNAVAILABLE_DISABLED_BY_POLICY);
+  EXPECT_EQ(ParseJsonDict(result.payload).FindInt("remoteSupportAvailability"),
+            CrdSessionAvailability::UNAVAILABLE_DISABLED_BY_POLICY);
+}
+
 TEST_P(DeviceCommandFetchCrdAvailabilityInfoJobTestParameterizedOverSessionType,
        ShouldReturnUserSessionType) {
   TestSessionType session_type = GetParam();
   SCOPED_TRACE(base::StringPrintf("Testing session type %s",
                                   SessionTypeToString(session_type)));
 
-  StartSessionOfType(session_type, user_manager());
+  StartSessionOfType(session_type);
 
   Result result = CreateAndRunJob();
 
@@ -275,7 +305,7 @@ TEST_P(DeviceCommandFetchCrdAvailabilityInfoJobTestParameterizedOverSessionType,
   SCOPED_TRACE(base::StringPrintf("Testing session type %s",
                                   SessionTypeToString(session_type)));
 
-  StartSessionOfType(session_type, user_manager());
+  StartSessionOfType(session_type);
 
   AddActiveManagedNetwork();
   Result result = CreateAndRunJob();
@@ -317,7 +347,7 @@ TEST_P(DeviceCommandFetchCrdAvailabilityInfoJobTestParameterizedOverSessionType,
   fake_cros_network_config().SetActiveNetworks(
       {CreateNetwork().SetOncSource(OncSource::kNone)});
 
-  StartSessionOfType(session_type, user_manager());
+  StartSessionOfType(session_type);
 
   Result result = CreateAndRunJob();
 
@@ -336,7 +366,7 @@ TEST_P(DeviceCommandFetchCrdAvailabilityInfoJobTestParameterizedOverSessionType,
   SCOPED_TRACE(base::StringPrintf("Testing session type %s",
                                   SessionTypeToString(session_type)));
 
-  StartSessionOfType(session_type, user_manager());
+  StartSessionOfType(session_type);
 
   Result result = CreateAndRunJob();
 
@@ -371,7 +401,7 @@ TEST_P(DeviceCommandFetchCrdAvailabilityInfoJobTestParameterizedOverSessionType,
                                   SessionTypeToString(session_type)));
 
   AddActiveManagedNetwork();
-  StartSessionOfType(session_type, user_manager());
+  StartSessionOfType(session_type);
 
   Result result = CreateAndRunJob();
 

chrome/browser/ash/policy/remote_commands/user_session_type_test_util.cc

@@ -121,7 +121,7 @@ TestingProfile* StartSessionOfTypeWithProfile(
   AccountId account_id = CreateUserOfType(session_type, user_manager);
   user_manager.LoginUser(account_id);
   return profile_manager.CreateTestingProfile(account_id.GetUserEmail(),
-                                              /* is_main_profile= */ true);
+                                              /*is_main_profile=*/true);
 }
 
 }  // namespace policy::test

chrome/browser/policy/configuration_policy_handler_list_factory.cc

@@ -1839,6 +1839,9 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
   { key::kKioskTroubleshootingToolsEnabled,
     prefs::kKioskTroubleshootingToolsEnabled,
     base::Value::Type::BOOLEAN },
+  { key::kRemoteAccessHostAllowEnterpriseRemoteSupportConnections,
+    prefs::kRemoteAccessHostAllowEnterpriseRemoteSupportConnections,
+    base::Value::Type::BOOLEAN },
   { key::kRealTimeDownloadProtectionRequestAllowed,
     prefs::kRealTimeDownloadProtectionRequestAllowedByPolicy,
     base::Value::Type::BOOLEAN },

chrome/common/pref_names.h

@@ -2945,6 +2945,13 @@ inline constexpr char kKioskTroubleshootingToolsEnabled[] =
 // browser window.
 inline constexpr char kNewWindowsInKioskAllowed[] =
     "new_windows_in_kiosk_allowed";
+
+// A boolean pref which determines whether a remote admin can start a CRD
+// connection through the 'start crd session' remote command.
+inline constexpr char
+    kRemoteAccessHostAllowEnterpriseRemoteSupportConnections[] =
+        "enterprise_remote_support_connections_allowed";
+
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
 // *************** SERVICE PREFS ***************

components/policy/proto/device_management_backend.proto

@@ -77,6 +77,9 @@ enum CrdSessionAvailability {
   UNAVAILABLE_UNMANAGED_ENVIRONMENT = 3;
   // The CRD session type is unsupported by device os version
   UNAVAILABLE_UNSUPPORTED_DEVICE_OS_VERSION = 4;
+  // CRD is disabled on the device by the 'enterprise remote support connection'
+  // policy.
+  UNAVAILABLE_DISABLED_BY_POLICY = 5;
 }
 
 // Data along with a cryptographic signature verifying their authenticity.

components/policy/test/data/policy_test_cases.json

@@ -708,7 +708,45 @@
     "reason_for_missing_test": "Not used by the browser, handled by CRD host."
   },
   "RemoteAccessHostAllowEnterpriseRemoteSupportConnections": {
-    "reason_for_missing_test": "Not used by the browser, handled by CRD host."
+    "os": [
+      "chromeos_ash"
+    ],
+    "policy_pref_mapping_tests": [
+      {
+        "note": "Check default value (no policies set)",
+        "policies": {},
+        "prefs": {
+          "enterprise_remote_support_connections_allowed": {
+            "location": "local_state",
+            "default_value": true
+          }
+        }
+      },
+      {
+        "note": "Check true policy value",
+        "policies": {
+          "RemoteAccessHostAllowEnterpriseRemoteSupportConnections": true
+        },
+        "prefs": {
+          "enterprise_remote_support_connections_allowed": {
+            "location": "local_state",
+            "value": true
+          }
+        }
+      },
+      {
+        "note": "Check false policy value",
+        "policies": {
+          "RemoteAccessHostAllowEnterpriseRemoteSupportConnections": false
+        },
+        "prefs": {
+          "enterprise_remote_support_connections_allowed": {
+            "location": "local_state",
+            "value": false
+          }
+        }
+      }
+    ]
   },
   "PrintingEnabled": {
     "os": [