Support CRD to manually launched kiosk session

This has always been blocked, but nobody can think of a good reason why it would be blocked, so we decided to allow it. Fixed: b/240562417 Test: unit_tests --gtest_filter="DeviceCommandStartCrd*" Change-Id: Ia8dd596edb01122c226356bf9b910bd640137388 Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811575 Reviewed-by: Ben Franz <bfranz@chromium.org> Commit-Queue: Jeroen Dhollander <jeroendh@google.com> Cr-Commit-Position: refs/heads/main@{#1032469}
chromium · Aug 8, 2022 · ea81a9c · ea81a9c
1 parent f0c87a6
commit ea81a9c
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 25 deletions.
diff --git a/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_job.cc b/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_job.cc
@@ -421,9 +421,9 @@ bool DeviceCommandStartCrdSessionJob::UserTypeSupportsCrd() const {
     case UserType::kAffiliatedUser:
     case UserType::kAutoLaunchedKiosk:
     case UserType::kManagedGuestSession:
+    case UserType::kManuallyLaunchedKiosk:
       return true;
     case UserType::kNoUser:
-    case UserType::kNonAutoLaunchedKiosk:
     case UserType::kOther:
       return false;
   }
@@ -442,7 +442,7 @@ DeviceCommandStartCrdSessionJob::GetUserType() const {
     if (IsRunningAutoLaunchedKiosk())
       return UserType::kAutoLaunchedKiosk;
     else
-      return UserType::kNonAutoLaunchedKiosk;
+      return UserType::kManuallyLaunchedKiosk;
   }
 
   if (user_manager->IsLoggedInAsPublicAccount())
@@ -463,10 +463,11 @@ DeviceCommandStartCrdSessionJob::GetUmaSessionType() const {
       return UmaSessionType::kAffiliatedUser;
     case UserType::kManagedGuestSession:
       return UmaSessionType::kManagedGuestSession;
+    case UserType::kManuallyLaunchedKiosk:
+      return UmaSessionType::kManuallyLaunchedKiosk;
     case UserType::kNoUser:
       // TODO(b/236689277): Introduce UmaSessionType::kNoLocalUser.
       return UmaSessionType::kMaxValue;
-    case UserType::kNonAutoLaunchedKiosk:
     case UserType::kOther:
       NOTREACHED();
       return UmaSessionType::kMaxValue;
@@ -507,8 +508,8 @@ bool DeviceCommandStartCrdSessionJob::ShouldShowConfirmationDialog() const {
     case UserType::kManagedGuestSession:
       return true;
     case UserType::kAutoLaunchedKiosk:
+    case UserType::kManuallyLaunchedKiosk:
     case UserType::kNoUser:
-    case UserType::kNonAutoLaunchedKiosk:
     case UserType::kOther:
       return false;
   }
@@ -532,9 +533,9 @@ bool DeviceCommandStartCrdSessionJob::ShouldTerminateUponInput() const {
       //      user input.
       return false;
     case UserType::kAutoLaunchedKiosk:
+    case UserType::kManuallyLaunchedKiosk:
       return !acked_user_presence_;
     case UserType::kNoUser:
-    case UserType::kNonAutoLaunchedKiosk:
     case UserType::kOther:
       // This method will only be called for user types for which we support
       // CRD sessions.
@@ -562,8 +563,8 @@ const char* DeviceCommandStartCrdSessionJob::UserTypeToString(
   switch (value) {
     case UserType::kAutoLaunchedKiosk:
       return "kAutoLaunchedKiosk";
-    case UserType::kNonAutoLaunchedKiosk:
-      return "kNonAutoLaunchedKiosk";
+    case UserType::kManuallyLaunchedKiosk:
+      return "kManuallyLaunchedKiosk";
     case UserType::kNoUser:
       return "kNoUser";
     case UserType::kAffiliatedUser:

diff --git a/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_job.h b/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_job.h
@@ -95,12 +95,13 @@ class DeviceCommandStartCrdSessionJob : public RemoteCommandJob {
   // fetch an oauth token.
   void SetOAuthTokenForTest(const std::string& token);
 
-  // This enum can't be renumbered because its logged to UMA.
+  // This enum can't be renumbered because it is logged to UMA.
   enum class UmaSessionType {
     kAutoLaunchedKiosk = 0,
     kAffiliatedUser = 1,
     kManagedGuestSession = 2,
-    kMaxValue = kManagedGuestSession
+    kManuallyLaunchedKiosk = 3,
+    kMaxValue = kManuallyLaunchedKiosk
   };
 
  protected:
@@ -116,7 +117,7 @@ class DeviceCommandStartCrdSessionJob : public RemoteCommandJob {
 
   enum class UserType {
     kAutoLaunchedKiosk,
-    kNonAutoLaunchedKiosk,
+    kManuallyLaunchedKiosk,
     kNoUser,
     kAffiliatedUser,
     kManagedGuestSession,

diff --git a/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_unittest.cc b/chrome/browser/ash/policy/remote_commands/device_command_start_crd_session_unittest.cc
@@ -270,6 +270,12 @@ class DeviceCommandStartCrdSessionJobTest : public ash::DeviceSettingsTestBase {
         ->set_current_app_was_auto_launched_with_zero_delay_for_testing(true);
   }
 
+  void LogInAsManuallyLaunchedKioskAppUser() {
+    LogInAsKioskAppUser();
+    ash::KioskAppManager::Get()
+        ->set_current_app_was_auto_launched_with_zero_delay_for_testing(false);
+  }
+
   void SetDeviceIdleTime(int idle_time_in_sec) {
     user_activity_detector_->set_last_activity_time_for_test(
         base::TimeTicks::Now() - base::Seconds(idle_time_in_sec));
@@ -439,18 +445,17 @@ TEST_F(DeviceCommandStartCrdSessionJobTest, ShouldFailForRegularUser) {
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldFailForKioskUserWithoutAutoLaunch) {
+       ShouldSucceedForManuallyLaunchedKioskUser) {
   LogInAsKioskAppUser();
 
   ash::KioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(false);
 
-  EXPECT_ERROR(RunJobAndWaitForResult(),
-               DeviceCommandStartCrdSessionJob::FAILURE_UNSUPPORTED_USER_TYPE);
+  EXPECT_SUCCESS(RunJobAndWaitForResult());
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldSucceedForKioskUserWithZeroDelayAutoLaunch) {
+       ShouldSucceedForAutoLaunchedKioskUser) {
   LogInAsKioskAppUser();
   ash::KioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(true);
@@ -459,19 +464,18 @@ TEST_F(DeviceCommandStartCrdSessionJobTest,
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldFailForArcKioskUserWithoutAutoLaunch) {
+       ShouldSucceedForManuallyLaunchedArcKioskUser) {
   SetOAuthToken(kTestOAuthToken);
 
   LogInAsArcKioskAppUser();
   ash::ArcKioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(false);
 
-  EXPECT_ERROR(RunJobAndWaitForResult(),
-               DeviceCommandStartCrdSessionJob::FAILURE_UNSUPPORTED_USER_TYPE);
+  EXPECT_SUCCESS(RunJobAndWaitForResult());
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldSucceedForArcKioskUserWithZeroDelayAutoLaunch) {
+       ShouldSucceedForAutoLaunchedArcKioskUser) {
   LogInAsArcKioskAppUser();
   ash::ArcKioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(true);
@@ -480,17 +484,16 @@ TEST_F(DeviceCommandStartCrdSessionJobTest,
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldFailForWebKioskUserWithoutAutoLaunch) {
+       ShouldSucceedForManuallyLaunchedWebKioskUser) {
   LogInAsWebKioskAppUser();
   ash::WebKioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(false);
 
-  EXPECT_ERROR(RunJobAndWaitForResult(),
-               DeviceCommandStartCrdSessionJob::FAILURE_UNSUPPORTED_USER_TYPE);
+  EXPECT_SUCCESS(RunJobAndWaitForResult());
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldSucceedForWebKioskUserWithZeroDelayAutoLaunch) {
+       ShouldSucceedForAutoLaunchedWebKioskUser) {
   LogInAsWebKioskAppUser();
   ash::WebKioskAppManager::Get()
       ->set_current_app_was_auto_launched_with_zero_delay_for_testing(true);
@@ -585,7 +588,7 @@ TEST_F(DeviceCommandStartCrdSessionJobTest,
 
 TEST_F(
     DeviceCommandStartCrdSessionJobTest,
-    ShouldPassTerminateUponInputTrueToDelegateForKioskUserIfAckedUserPresenceSetFalse) {
+    ShouldPassTerminateUponInputTrueToDelegateForAutolaunchedKioskIfAckedUserPresenceSetFalse) {
   LogInAsAutoLaunchedKioskAppUser();
 
   EXPECT_SUCCESS(
@@ -597,7 +600,7 @@ TEST_F(
 
 TEST_F(
     DeviceCommandStartCrdSessionJobTest,
-    ShouldPassTerminateUponInputFalseToDelegateForKioskUserIfAckedUserPresenceSetTrue) {
+    ShouldPassTerminateUponInputFalseToDelegateForAutolaunchedKioskIfAckedUserPresenceSetTrue) {
   LogInAsAutoLaunchedKioskAppUser();
 
   EXPECT_SUCCESS(
@@ -607,6 +610,30 @@ TEST_F(
             crd_host_delegate().session_parameters().terminate_upon_input);
 }
 
+TEST_F(
+    DeviceCommandStartCrdSessionJobTest,
+    ShouldPassTerminateUponInputTrueToDelegateForManuallylaunchedKioskIfAckedUserPresenceSetFalse) {
+  LogInAsManuallyLaunchedKioskAppUser();
+
+  EXPECT_SUCCESS(
+      RunJobAndWaitForResult(Payload().Set("acked_user_presence", false)));
+
+  EXPECT_EQ(true,
+            crd_host_delegate().session_parameters().terminate_upon_input);
+}
+
+TEST_F(
+    DeviceCommandStartCrdSessionJobTest,
+    ShouldPassTerminateUponInputFalseToDelegateForManuallyLaunchedKioskIfAckedUserPresenceSetTrue) {
+  LogInAsManuallyLaunchedKioskAppUser();
+
+  EXPECT_SUCCESS(
+      RunJobAndWaitForResult(Payload().Set("ackedUserPresence", true)));
+
+  EXPECT_EQ(false,
+            crd_host_delegate().session_parameters().terminate_upon_input);
+}
+
 TEST_F(DeviceCommandStartCrdSessionJobTest,
        ShouldPassShowConfirmationDialogFalseToDelegateForKioskUser) {
   LogInAsAutoLaunchedKioskAppUser();
@@ -673,7 +700,7 @@ TEST_F(DeviceCommandStartCrdSessionJobTest,
 }
 
 TEST_F(DeviceCommandStartCrdSessionJobTest,
-       ShouldSendSuccessUmaLogWhenAutoLaunchKioskConnects) {
+       ShouldSendSuccessUmaLogWhenAutoLaunchedKioskConnects) {
   base::HistogramTester histogram_tester;
 
   LogInAsAutoLaunchedKioskAppUser();
@@ -687,6 +714,21 @@ TEST_F(DeviceCommandStartCrdSessionJobTest,
       UmaSessionType::kAutoLaunchedKiosk, 1);
 }
 
+TEST_F(DeviceCommandStartCrdSessionJobTest,
+       ShouldSendSuccessUmaLogWhenManuallyLaunchedKioskConnects) {
+  base::HistogramTester histogram_tester;
+
+  LogInAsManuallyLaunchedKioskAppUser();
+  crd_host_delegate().SetHasActiveSession(true);
+  RunJobAndWaitForResult();
+
+  histogram_tester.ExpectUniqueSample(
+      "Enterprise.DeviceRemoteCommand.Crd.Result", ResultCode::SUCCESS, 1);
+  histogram_tester.ExpectUniqueSample(
+      "Enterprise.DeviceRemoteCommand.Crd.SessionType",
+      UmaSessionType::kManuallyLaunchedKiosk, 1);
+}
+
 TEST_F(DeviceCommandStartCrdSessionJobTest,
        ShouldSendSuccessUmaLogWhenAffiliatedUserConnects) {
   base::HistogramTester histogram_tester;

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -30726,6 +30726,7 @@ Called by update_document_policy_enum.py.-->
   <int value="0" label="kAutoLaunchedKiosk"/>
   <int value="1" label="kAffiliatedUser"/>
   <int value="2" label="kManagedGuestSession"/>
+  <int value="3" label="kManuallyLaunchedKiosk"/>
 </enum>
 
 <enum name="EnterpriseDeviceManagementStatus">