From 21f6d032e6c4ceddd5a758c175c707d7b7265732 Mon Sep 17 00:00:00 2001 From: xiyuan Date: Tue, 6 Dec 2016 12:36:06 -0800 Subject: [PATCH] kiosk: Make consumer kiosk mode disabled by default Add a "enable-consumer-kiosk" switch. Consumer kiosk mode is disabled by default and only enabled when the switch is present. When consumer kiosk mode is disabled, the hotkey to enable consumer kiosk auto launch is disabled and the management webui in chrome://extensions page is hidden. BUG=670429 Review-Url: https://codereview.chromium.org/2548603004 Cr-Commit-Position: refs/heads/master@{#436699} --- .../chromeos/app_mode/kiosk_app_manager.cc | 20 +++++ .../chromeos/app_mode/kiosk_app_manager.h | 2 + .../app_mode/kiosk_app_manager_browsertest.cc | 85 ++++++++++--------- .../chromeos/login/kiosk_browsertest.cc | 13 +-- .../chromeos/login/ui/webui_login_view.cc | 9 +- .../chromeos/login/signin_screen_handler.cc | 1 + .../chromeos/kiosk_apps_browsertest.js | 30 +++++++ .../extensions/chromeos/kiosk_apps_handler.cc | 24 ++++-- chromeos/chromeos_switches.cc | 3 + chromeos/chromeos_switches.h | 1 + 10 files changed, 132 insertions(+), 56 deletions(-) diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc b/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc index f4be7534481935..a24c7bf3383250 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc @@ -10,6 +10,7 @@ #include "base/barrier_closure.h" #include "base/bind.h" +#include "base/command_line.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" @@ -35,6 +36,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension_constants.h" #include "chromeos/chromeos_paths.h" +#include "chromeos/chromeos_switches.h" #include "chromeos/cryptohome/async_method_caller.h" #include "chromeos/cryptohome/cryptohome_parameters.h" #include "chromeos/dbus/dbus_thread_manager.h" @@ -183,6 +185,12 @@ void KioskAppManager::RemoveObsoleteCryptohomes() { base::Bind(&PerformDelayedCryptohomeRemovals)); } +// static +bool KioskAppManager::IsConsumerKioskEnabled() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kEnableConsumerKiosk); +} + KioskAppManager::App::App(const KioskAppData& data, bool is_extension_pending, bool auto_launched_with_zero_delay) @@ -255,6 +263,12 @@ void KioskAppManager::AddAppForTest( void KioskAppManager::EnableConsumerKioskAutoLaunch( const KioskAppManager::EnableKioskAutoLaunchCallback& callback) { + if (!IsConsumerKioskEnabled()) { + if (!callback.is_null()) + callback.Run(false); + return; + } + policy::BrowserPolicyConnectorChromeOS* connector = g_browser_process->platform_part()->browser_policy_connector_chromeos(); connector->GetInstallAttributes()->LockDevice( @@ -268,6 +282,12 @@ void KioskAppManager::EnableConsumerKioskAutoLaunch( void KioskAppManager::GetConsumerKioskAutoLaunchStatus( const KioskAppManager::GetConsumerKioskAutoLaunchStatusCallback& callback) { + if (!IsConsumerKioskEnabled()) { + if (!callback.is_null()) + callback.Run(CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); + return; + } + policy::BrowserPolicyConnectorChromeOS* connector = g_browser_process->platform_part()->browser_policy_connector_chromeos(); connector->GetInstallAttributes()->ReadImmutableAttributes( diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_manager.h b/chrome/browser/chromeos/app_mode/kiosk_app_manager.h index aeae5dfd1328cd..7aac2e7f43f19b 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_manager.h +++ b/chrome/browser/chromeos/app_mode/kiosk_app_manager.h @@ -108,6 +108,8 @@ class KioskAppManager : public KioskAppDataDelegate, // Removes cryptohomes which could not be removed during the previous session. static void RemoveObsoleteCryptohomes(); + static bool IsConsumerKioskEnabled(); + // Initiates reading of consumer kiosk mode auto-launch status. void GetConsumerKioskAutoLaunchStatus( const GetConsumerKioskAutoLaunchStatusCallback& callback); diff --git a/chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc b/chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc index 7218eace37a105..05c9c4564752e8 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc @@ -31,6 +31,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/test/base/in_process_browser_test.h" +#include "chromeos/chromeos_switches.h" #include "chromeos/settings/cros_settings_names.h" #include "components/prefs/scoped_user_pref_update.h" #include "content/public/test/test_utils.h" @@ -780,79 +781,81 @@ IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, UpdateAndRemoveApp) { } IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, EnableConsumerKiosk) { - std::unique_ptr status( - new KioskAppManager::ConsumerKioskAutoLaunchStatus( - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED)); - std::unique_ptr locked(new bool(false)); + // Consumer kiosk is disabled by default. Enable it for test. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableConsumerKiosk); + + KioskAppManager::ConsumerKioskAutoLaunchStatus status = + KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED; + bool locked = false; scoped_refptr runner = new content::MessageLoopRunner; - manager()->GetConsumerKioskAutoLaunchStatus( - base::Bind(&ConsumerKioskAutoLaunchStatusCheck, - status.get(), - runner->QuitClosure())); + manager()->GetConsumerKioskAutoLaunchStatus(base::Bind( + &ConsumerKioskAutoLaunchStatusCheck, &status, runner->QuitClosure())); runner->Run(); - EXPECT_EQ(*status.get(), - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_CONFIGURABLE); + EXPECT_EQ(status, KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_CONFIGURABLE); scoped_refptr runner2 = new content::MessageLoopRunner; manager()->EnableConsumerKioskAutoLaunch( - base::Bind(&ConsumerKioskModeLockCheck, - locked.get(), - runner2->QuitClosure())); + base::Bind(&ConsumerKioskModeLockCheck, &locked, runner2->QuitClosure())); runner2->Run(); - EXPECT_TRUE(*locked.get()); + EXPECT_TRUE(locked); scoped_refptr runner3 = new content::MessageLoopRunner; - manager()->GetConsumerKioskAutoLaunchStatus( - base::Bind(&ConsumerKioskAutoLaunchStatusCheck, - status.get(), - runner3->QuitClosure())); + manager()->GetConsumerKioskAutoLaunchStatus(base::Bind( + &ConsumerKioskAutoLaunchStatusCheck, &status, runner3->QuitClosure())); runner3->Run(); - EXPECT_EQ(*status.get(), - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_ENABLED); + EXPECT_EQ(status, KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_ENABLED); +} + +IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, ConsumerKioskDisabled) { + KioskAppManager::ConsumerKioskAutoLaunchStatus status = + KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_CONFIGURABLE; + + scoped_refptr runner = + new content::MessageLoopRunner; + manager()->GetConsumerKioskAutoLaunchStatus(base::Bind( + &ConsumerKioskAutoLaunchStatusCheck, &status, runner->QuitClosure())); + runner->Run(); + EXPECT_EQ(status, KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); } IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, PreventEnableConsumerKioskForEnterprise) { - // First, lock the device as enterprise. + // Consumer kiosk is disabled by default. Enable it for test. + base::CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kEnableConsumerKiosk); + + // Lock the device as enterprise. EXPECT_EQ(LockDeviceForEnterprise(), InstallAttributes::LOCK_SUCCESS); - std::unique_ptr status( - new KioskAppManager::ConsumerKioskAutoLaunchStatus( - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED)); - std::unique_ptr locked(new bool(true)); + KioskAppManager::ConsumerKioskAutoLaunchStatus status = + KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED; + bool locked = true; scoped_refptr runner = new content::MessageLoopRunner; - manager()->GetConsumerKioskAutoLaunchStatus( - base::Bind(&ConsumerKioskAutoLaunchStatusCheck, - status.get(), - runner->QuitClosure())); + manager()->GetConsumerKioskAutoLaunchStatus(base::Bind( + &ConsumerKioskAutoLaunchStatusCheck, &status, runner->QuitClosure())); runner->Run(); - EXPECT_EQ(*status.get(), - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); + EXPECT_EQ(status, KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); scoped_refptr runner2 = new content::MessageLoopRunner; manager()->EnableConsumerKioskAutoLaunch( - base::Bind(&ConsumerKioskModeLockCheck, - locked.get(), - runner2->QuitClosure())); + base::Bind(&ConsumerKioskModeLockCheck, &locked, runner2->QuitClosure())); runner2->Run(); - EXPECT_FALSE(*locked.get()); + EXPECT_FALSE(locked); scoped_refptr runner3 = new content::MessageLoopRunner; - manager()->GetConsumerKioskAutoLaunchStatus( - base::Bind(&ConsumerKioskAutoLaunchStatusCheck, - status.get(), - runner3->QuitClosure())); + manager()->GetConsumerKioskAutoLaunchStatus(base::Bind( + &ConsumerKioskAutoLaunchStatusCheck, &status, runner3->QuitClosure())); runner3->Run(); - EXPECT_EQ(*status.get(), - KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); + EXPECT_EQ(status, KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_DISABLED); } IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, diff --git a/chrome/browser/chromeos/login/kiosk_browsertest.cc b/chrome/browser/chromeos/login/kiosk_browsertest.cc index e859b0478fd48e..b2767924b6981b 100644 --- a/chrome/browser/chromeos/login/kiosk_browsertest.cc +++ b/chrome/browser/chromeos/login/kiosk_browsertest.cc @@ -542,6 +542,9 @@ class KioskTest : public OobeBaseTest { void SetUpCommandLine(base::CommandLine* command_line) override { OobeBaseTest::SetUpCommandLine(command_line); fake_cws_->Init(embedded_test_server()); + + if (use_consumer_kiosk_mode_) + command_line->AppendSwitch(switches::kEnableConsumerKiosk); } void LaunchApp(const std::string& app_id, bool diagnostic_mode) { @@ -714,15 +717,13 @@ class KioskTest : public OobeBaseTest { } void EnableConsumerKioskMode() { - std::unique_ptr locked(new bool(false)); + bool locked = false; scoped_refptr runner = new content::MessageLoopRunner; - KioskAppManager::Get()->EnableConsumerKioskAutoLaunch( - base::Bind(&ConsumerKioskModeAutoStartLockCheck, - locked.get(), - runner->QuitClosure())); + KioskAppManager::Get()->EnableConsumerKioskAutoLaunch(base::Bind( + &ConsumerKioskModeAutoStartLockCheck, &locked, runner->QuitClosure())); runner->Run(); - EXPECT_TRUE(*locked.get()); + EXPECT_TRUE(locked); } KioskAppManager::ConsumerKioskAutoLaunchStatus diff --git a/chrome/browser/chromeos/login/ui/webui_login_view.cc b/chrome/browser/chromeos/login/ui/webui_login_view.cc index a83d5c8f085fa1..a9d53c95f68a43 100644 --- a/chrome/browser/chromeos/login/ui/webui_login_view.cc +++ b/chrome/browser/chromeos/login/ui/webui_login_view.cc @@ -19,6 +19,7 @@ #include "base/values.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/accessibility/accessibility_util.h" +#include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h" #include "chrome/browser/chromeos/login/ui/login_display_host_impl.h" #include "chrome/browser/chromeos/login/ui/proxy_settings_dialog.h" #include "chrome/browser/chromeos/login/ui/web_contents_set_background_color.h" @@ -191,9 +192,11 @@ WebUILoginView::WebUILoginView() { accel_map_[ui::Accelerator( ui::VKEY_A, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN | ui::EF_SHIFT_DOWN)] = kAccelNameEnrollmentAd; - accel_map_[ui::Accelerator(ui::VKEY_K, - ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN)] = - kAccelNameKioskEnable; + if (KioskAppManager::IsConsumerKioskEnabled()) { + accel_map_[ui::Accelerator(ui::VKEY_K, + ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN)] = + kAccelNameKioskEnable; + } accel_map_[ui::Accelerator(ui::VKEY_V, ui::EF_ALT_DOWN)] = kAccelNameVersion; accel_map_[ui::Accelerator(ui::VKEY_R, diff --git a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc index 2934a79d083c61..9751f73df43e8d 100644 --- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc +++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc @@ -1224,6 +1224,7 @@ void SigninScreenHandler::HandleToggleKioskEnableScreen() { policy::BrowserPolicyConnectorChromeOS* connector = g_browser_process->platform_part()->browser_policy_connector_chromeos(); if (delegate_ && !connector->IsEnterpriseManaged() && + KioskAppManager::IsConsumerKioskEnabled() && LoginDisplayHost::default_host()) { delegate_->ShowKioskEnableScreen(); } diff --git a/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_browsertest.js b/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_browsertest.js index 01003f65ff101f..5a7b9f29924129 100644 --- a/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_browsertest.js +++ b/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_browsertest.js @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +GEN('#include "base/command_line.h"'); + /** * TestFixture for kiosk app settings WebUI testing. * @extends {testing.Test} @@ -17,6 +19,11 @@ KioskAppSettingsWebUITest.prototype = { */ browsePreload: 'chrome://extensions-frame/', + /** @override */ + commandLineSwitches: [{ + switchName: 'enable-consumer-kiosk', + }], + /** * Mock settings data. * @private @@ -233,3 +240,26 @@ TEST_F('KioskAppSettingsWebUITest', 'testAllowDisableBailout', function() { extensions.KioskAppsOverlay.setSettings(this.settings_); expectFalse(checkbox.disabled); }); + +/** + * TestFixture for kiosk app settings when consumer kiosk is disabled. + * @extends {testing.Test} + * @constructor + */ +function NoConsumerKioskWebUITest() {} + +NoConsumerKioskWebUITest.prototype = { + __proto__: KioskAppSettingsWebUITest.prototype, + + /** @override */ + commandLineSwitches: [], + + /** @override */ + setUp: function() {} +}; + +// Test kiosk app settings are not visible when consumer kiosk is disabled. +TEST_F('NoConsumerKioskWebUITest', 'settingsHidden', function() { + assertEquals(this.browsePreload, document.location.href); + assertTrue($('add-kiosk-app').hidden); +}); diff --git a/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.cc b/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.cc index 67a9cc32c7e7b3..c7fc87d376f61f 100644 --- a/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.cc +++ b/chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.cc @@ -19,6 +19,7 @@ #include "base/sys_info.h" #include "base/values.h" #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h" +#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/grit/chromium_strings.h" #include "chrome/grit/generated_resources.h" @@ -213,12 +214,23 @@ void KioskAppsHandler::OnKioskExtensionDownloadFailed( void KioskAppsHandler::OnGetConsumerKioskAutoLaunchStatus( chromeos::KioskAppManager::ConsumerKioskAutoLaunchStatus status) { initialized_ = true; - is_kiosk_enabled_ = user_manager::UserManager::Get()->IsCurrentUserOwner() || - !base::SysInfo::IsRunningOnChromeOS(); - - is_auto_launch_enabled_ = - status == KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_ENABLED || - !base::SysInfo::IsRunningOnChromeOS(); + if (KioskAppManager::IsConsumerKioskEnabled()) { + if (!base::SysInfo::IsRunningOnChromeOS()) { + // Enable everything when running on a dev box. + is_kiosk_enabled_ = true; + is_auto_launch_enabled_ = true; + } else { + // Enable consumer kiosk for owner and enable auto launch if configured. + is_kiosk_enabled_ = + ProfileHelper::IsOwnerProfile(Profile::FromWebUI(web_ui())); + is_auto_launch_enabled_ = + status == KioskAppManager::CONSUMER_KIOSK_AUTO_LAUNCH_ENABLED; + } + } else { + // Otherwise, consumer kiosk is disabled. + is_kiosk_enabled_ = false; + is_auto_launch_enabled_ = false; + } if (is_kiosk_enabled_) { base::DictionaryValue kiosk_params; diff --git a/chromeos/chromeos_switches.cc b/chromeos/chromeos_switches.cc index a022c32b2c8620..37cec4e0489117 100644 --- a/chromeos/chromeos_switches.cc +++ b/chromeos/chromeos_switches.cc @@ -217,6 +217,9 @@ const char kEnableArc[] = "enable-arc"; // Enables ARC OptIn flow in OOBE. const char kEnableArcOOBEOptIn[] = "enable-arc-oobe-optin"; +// Enables consume kiosk mode. +const char kEnableConsumerKiosk[] = "enable-consumer-kiosk"; + // Enables Data Saver prompt on cellular networks. const char kEnableDataSaverPrompt[] = "enable-datasaver-prompt"; diff --git a/chromeos/chromeos_switches.h b/chromeos/chromeos_switches.h index cd6426501be453..66c8278a160559 100644 --- a/chromeos/chromeos_switches.h +++ b/chromeos/chromeos_switches.h @@ -77,6 +77,7 @@ CHROMEOS_EXPORT extern const char kEafeUrl[]; CHROMEOS_EXPORT extern const char kEnableAd[]; CHROMEOS_EXPORT extern const char kEnableArc[]; CHROMEOS_EXPORT extern const char kEnableArcOOBEOptIn[]; +CHROMEOS_EXPORT extern const char kEnableConsumerKiosk[]; CHROMEOS_EXPORT extern const char kEnableDataSaverPrompt[]; CHROMEOS_EXPORT extern const char kEnableExperimentalAccessibilityFeatures[]; CHROMEOS_EXPORT extern const char kEnableExtensionAssetsSharing[];