Limit the use of KioskAppExternalLoader to Chrome App Kiosk only

KioskAppExternalLoader is used to load apps and extensions for a Chrome
App Kiosk session. It registers a callback and wait until app info is
updated.

This class should be only used in Chrome App Kiosk, but not in other two
Kiosk types. But according to the current implementation, the loader is
enabled unexpectedly and blocks the uninstallation of force-installed
extensions when they are removed by admins. Therefore, the check logic
should be limited to a proper scope.

Bug: 1153440
Change-Id: I8bf79fa553dfbea5e271b7f06fc6ff0dffb33495
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575118Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Anatoliy Potapchuk <apotapchuk@chromium.org>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Commit-Queue: Anqing Zhao <anqing@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836300}

chrome/browser/chromeos/app_mode/startup_app_launcher_unittest.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/chromeos/app_mode/test_kiosk_extension_builder.h"
 #include "chrome/browser/chromeos/extensions/test_external_cache.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -36,6 +37,7 @@
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/account_id/account_id.h"
 #include "components/session_manager/core/session_manager.h"
+#include "components/user_manager/scoped_user_manager.h"
 #include "components/version_info/channel.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -351,6 +353,7 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
 
     extensions::ExtensionServiceTestBase::SetUp();
 
+    InitializeKioskAppUser();
     InitializeEmptyExtensionService();
     external_apps_loader_handler_ = std::make_unique<TestKioskLoaderVisitor>(
         browser_context(), registry(), service());
@@ -554,6 +557,18 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
   }
 
  protected:
+  void InitializeKioskAppUser() {
+    const AccountId kiosk_account_id(
+        AccountId::FromUserEmail(kTestUserAccount));
+    auto fake_user_manager_ =
+        std::make_unique<chromeos::FakeChromeUserManager>();
+    fake_user_manager_->AddKioskAppUser(kiosk_account_id);
+    fake_user_manager_->LoginUser(kiosk_account_id);
+
+    user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
+        std::move(fake_user_manager_));
+  }
+
   TestAppLaunchDelegate startup_launch_delegate_;
 
   std::unique_ptr<KioskAppLauncher> startup_app_launcher_;
@@ -573,6 +588,8 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
   std::unique_ptr<extensions::ExternalProviderImpl> primary_app_provider_;
   std::unique_ptr<extensions::ExternalProviderImpl> secondary_apps_provider_;
 
+  std::unique_ptr<user_manager::ScopedUserManager> user_manager_enabler_;
+
   DISALLOW_COPY_AND_ASSIGN(StartupAppLauncherTest);
 };
 

chrome/browser/extensions/external_provider_impl.cc

@@ -664,42 +664,44 @@ void ExternalProviderImpl::CreateExternalProviders(
     provider_list->push_back(std::move(policy_provider));
   }
 
-  // Load the KioskAppExternalProvider when running in kiosk mode.
+  // Load the KioskAppExternalProvider when running in the Chrome App kiosk
+  // mode.
   if (chrome::IsRunningInForcedAppMode()) {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-    // Kiosk primary app external provider.
-    // For enterprise managed kiosk apps, change the location to
-    // "force-installed by policy".
-    policy::BrowserPolicyConnectorChromeOS* const connector =
-        g_browser_process->platform_part()->browser_policy_connector_chromeos();
-    Manifest::Location location = Manifest::EXTERNAL_PREF;
-    if (connector && connector->IsEnterpriseManaged())
-      location = Manifest::EXTERNAL_POLICY;
-
-    std::unique_ptr<ExternalProviderImpl> kiosk_app_provider(
-        new ExternalProviderImpl(
-            service,
-            base::MakeRefCounted<chromeos::KioskAppExternalLoader>(
-                chromeos::KioskAppExternalLoader::AppClass::kPrimary),
-            profile, location, Manifest::INVALID_LOCATION,
-            Extension::NO_FLAGS));
-    kiosk_app_provider->set_auto_acknowledge(true);
-    kiosk_app_provider->set_install_immediately(true);
-    kiosk_app_provider->set_allow_updates(true);
-    provider_list->push_back(std::move(kiosk_app_provider));
-
-    // Kiosk secondary app external provider.
-    std::unique_ptr<ExternalProviderImpl> secondary_kiosk_app_provider(
-        new ExternalProviderImpl(
-            service,
-            base::MakeRefCounted<chromeos::KioskAppExternalLoader>(
-                chromeos::KioskAppExternalLoader::AppClass::kSecondary),
-            profile, Manifest::EXTERNAL_PREF, Manifest::EXTERNAL_PREF_DOWNLOAD,
-            Extension::NO_FLAGS));
-    secondary_kiosk_app_provider->set_auto_acknowledge(true);
-    secondary_kiosk_app_provider->set_install_immediately(true);
-    secondary_kiosk_app_provider->set_allow_updates(true);
-    provider_list->push_back(std::move(secondary_kiosk_app_provider));
+    if (user && user->GetType() == user_manager::USER_TYPE_KIOSK_APP) {
+      // Kiosk primary app external provider.
+      // For enterprise managed kiosk apps, change the location to
+      // "force-installed by policy".
+      policy::BrowserPolicyConnectorChromeOS* const connector =
+          g_browser_process->platform_part()
+              ->browser_policy_connector_chromeos();
+      Manifest::Location location = Manifest::EXTERNAL_PREF;
+      if (connector && connector->IsEnterpriseManaged())
+        location = Manifest::EXTERNAL_POLICY;
+
+      auto kiosk_app_provider = std::make_unique<ExternalProviderImpl>(
+          service,
+          base::MakeRefCounted<chromeos::KioskAppExternalLoader>(
+              chromeos::KioskAppExternalLoader::AppClass::kPrimary),
+          profile, location, Manifest::INVALID_LOCATION, Extension::NO_FLAGS);
+      kiosk_app_provider->set_auto_acknowledge(true);
+      kiosk_app_provider->set_install_immediately(true);
+      kiosk_app_provider->set_allow_updates(true);
+      provider_list->push_back(std::move(kiosk_app_provider));
+
+      // Kiosk secondary app external provider.
+      auto secondary_kiosk_app_provider =
+          std::make_unique<ExternalProviderImpl>(
+              service,
+              base::MakeRefCounted<chromeos::KioskAppExternalLoader>(
+                  chromeos::KioskAppExternalLoader::AppClass::kSecondary),
+              profile, Manifest::EXTERNAL_PREF,
+              Manifest::EXTERNAL_PREF_DOWNLOAD, Extension::NO_FLAGS);
+      secondary_kiosk_app_provider->set_auto_acknowledge(true);
+      secondary_kiosk_app_provider->set_install_immediately(true);
+      secondary_kiosk_app_provider->set_allow_updates(true);
+      provider_list->push_back(std::move(secondary_kiosk_app_provider));
+    }
 #endif
     return;
   }

chrome/browser/extensions/external_provider_impl_chromeos_unittest.cc

@@ -48,6 +48,8 @@ const char kExternalAppId[] = "kekdneafjmhmndejhmbcadfiiofngffo";
 const char kStandaloneAppId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kStandaloneChildAppId[] = "hcglmfcclpfgljeaiahehebeoaiicbko";
 
+const char kTestUserAccount[] = "user@test";
+
 class ExternalProviderImplChromeOSTest : public ExtensionServiceTestBase {
  public:
   ExternalProviderImplChromeOSTest()
@@ -116,6 +118,25 @@ class ExternalProviderImplChromeOSTest : public ExtensionServiceTestBase {
     }
   }
 
+  void ValidateExternalProviderCountInAppMode(int expected_count) {
+    base::CommandLine* command = base::CommandLine::ForCurrentProcess();
+    command->AppendSwitchASCII(switches::kForceAppMode, std::string());
+    command->AppendSwitchASCII(switches::kAppId, std::string("app_id"));
+
+    InitializeEmptyExtensionService();
+
+    ProviderCollection providers;
+    extensions::ExternalProviderImpl::CreateExternalProviders(
+        service_, profile_.get(), service_->pending_extension_manager(),
+        &providers);
+
+    EXPECT_EQ(providers.size(), expected_count);
+  }
+
+  chromeos::FakeChromeUserManager* fake_user_manager() const {
+    return fake_user_manager_;
+  }
+
  private:
   std::unique_ptr<base::ScopedPathOverride> external_externsions_overrides_;
   chromeos::system::ScopedFakeStatisticsProvider fake_statistics_provider_;
@@ -263,4 +284,39 @@ TEST_F(ExternalProviderImplChromeOSTest, PriorityCompleted) {
   EXPECT_TRUE(registry()->GetInstalledExtension(kStandaloneAppId));
 }
 
+// Validate the external providers enabled in the Chrome App Kiosk session. The
+// expected number should be 3.
+// - |policy_provider|.
+// - |kiosk_app_provider|.
+// - |secondary_kiosk_app_provider|.
+TEST_F(ExternalProviderImplChromeOSTest, ChromeAppKiosk) {
+  const AccountId kiosk_account_id(AccountId::FromUserEmail(kTestUserAccount));
+  fake_user_manager()->AddKioskAppUser(kiosk_account_id);
+  fake_user_manager()->LoginUser(kiosk_account_id);
+
+  ValidateExternalProviderCountInAppMode(3);
+}
+
+// Validate the external providers enabled in the ARC++ App Kiosk session. The
+// expected number should be only 1.
+// - |policy_provider|.
+TEST_F(ExternalProviderImplChromeOSTest, ArcAppKiosk) {
+  const AccountId kiosk_account_id(AccountId::FromUserEmail(kTestUserAccount));
+  fake_user_manager()->AddArcKioskAppUser(kiosk_account_id);
+  fake_user_manager()->LoginUser(kiosk_account_id);
+
+  ValidateExternalProviderCountInAppMode(1);
+}
+
+// Validate the external providers enabled in the Web App Kiosk session. The
+// expected number should be only 1.
+// - |policy_provider|.
+TEST_F(ExternalProviderImplChromeOSTest, WebAppKiosk) {
+  const AccountId kiosk_account_id(AccountId::FromUserEmail(kTestUserAccount));
+  fake_user_manager()->AddWebKioskAppUser(kiosk_account_id);
+  fake_user_manager()->LoginUser(kiosk_account_id);
+
+  ValidateExternalProviderCountInAppMode(1);
+}
+
 }  // namespace extensions