Prevent kite tooltip icon from showing for non-supervised users

Currently, if an extension has permission updates pending the user's
approval, non-supervised users such as regular and enterprise users
will see the kite icon over the extension on the chrome://extensions
page.

This CL fixes this UI bug so that only supervised users will see the
kite icon. There should be no restrictions on the extension itself for
non-supervised users.

Bug: 1100395
Change-Id: I2d5a61e92925d093fa3ee4183c427d89dc8988b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2273684Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784278}

chrome/browser/extensions/api/developer_private/extension_info_generator.cc

@@ -570,6 +570,7 @@ void ExtensionInfoGenerator::CreateExtensionInfoHelper(
   bool permissions_increase =
       (disable_reasons & disable_reason::DISABLE_PERMISSIONS_INCREASE) != 0;
   info->disable_reasons.parent_disabled_permissions =
+      supervised_user_service_->IsChild() &&
       !supervised_user_service_
            ->GetSupervisedUserExtensionsMayRequestPermissionsPref() &&
       (custodian_approval_required || permissions_increase);

chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
+#include "base/files/file_path.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/json/json_writer.h"
 #include "base/macros.h"
@@ -22,12 +23,13 @@
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
 #include "chrome/browser/extensions/error_console/error_console.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/extensions/extension_service_test_with_install.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/permissions_test_util.h"
 #include "chrome/browser/extensions/permissions_updater.h"
 #include "chrome/browser/extensions/scripting_permissions_modifier.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/buildflags.h"
 #include "chrome/common/extensions/api/developer_private.h"
 #include "chrome/common/pref_names.h"
 #include "components/crx_file/id_util.h"
@@ -44,6 +46,12 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if BUILDFLAG(ENABLE_SUPERVISED_USERS)
+#include "chrome/browser/supervised_user/supervised_user_service.h"
+#include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_test_util.h"
+#endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
+
 namespace extensions {
 
 namespace developer = api::developer_private;
@@ -88,15 +96,20 @@ std::string SiteControlsToString(
 
 }  // namespace
 
-class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
+class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestWithInstall {
  public:
   ExtensionInfoGeneratorUnitTest() {}
   ~ExtensionInfoGeneratorUnitTest() override {}
 
  protected:
   void SetUp() override {
-    ExtensionServiceTestBase::SetUp();
-    InitializeEmptyExtensionService();
+    ExtensionServiceTestWithInstall::SetUp();
+    InitializeExtensionService(GetExtensionServiceInitParams());
+  }
+
+  // Returns the initialization parameters for the extension service.
+  virtual ExtensionServiceInitParams GetExtensionServiceInitParams() {
+    return CreateDefaultInitParams();
   }
 
   void OnInfoGenerated(std::unique_ptr<developer::ExtensionInfo>* info_out,
@@ -810,4 +823,128 @@ TEST_F(ExtensionInfoGeneratorUnitTest, Blocklisted) {
   EXPECT_EQ(developer::EXTENSION_STATE_ENABLED, info2->state);
 }
 
+// Tests that the parent_disabled_permissions disable reason is never set for
+// regular users. Prevents a regression to crbug/1100395.
+TEST_F(ExtensionInfoGeneratorUnitTest,
+       NoParentDisabledPermissionsForRegularUsers) {
+  // Preconditions.
+  ASSERT_FALSE(profile()->IsChild());
+
+  base::FilePath base_path = data_dir().AppendASCII("permissions_increase");
+  base::FilePath pem_path = base_path.AppendASCII("permissions.pem");
+  base::FilePath path = base_path.AppendASCII("v1");
+  const Extension* extension = PackAndInstallCRX(path, pem_path, INSTALL_NEW);
+  // The extension must now be installed and enabled.
+  ASSERT_TRUE(extension);
+  ASSERT_TRUE(registry()->enabled_extensions().Contains(extension->id()));
+
+  // Save the id, as |extension| will be destroyed during updating.
+  std::string extension_id = extension->id();
+
+  // Update to a new version with increased permissions.
+  path = base_path.AppendASCII("v2");
+  PackCRXAndUpdateExtension(extension_id, path, pem_path, DISABLED);
+
+  // The extension should be disabled pending approval for permission increases.
+  EXPECT_TRUE(registry()->disabled_extensions().Contains(extension_id));
+
+  // Due to a permissions increase, prefs will contain escalation information.
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+  EXPECT_TRUE(prefs->DidExtensionEscalatePermissions(extension_id));
+
+  std::unique_ptr<api::developer_private::ExtensionInfo> info =
+      GenerateExtensionInfo(extension_id);
+
+  // Verify that the kite icon error tooltip doesn't appear for regular users.
+  EXPECT_FALSE(info->disable_reasons.parent_disabled_permissions);
+}
+
+#if BUILDFLAG(ENABLE_SUPERVISED_USERS)
+// Tests for supervised users (child accounts). Supervised users are not allowed
+// to install apps or extensions unless their parent approves.
+class ExtensionInfoGeneratorUnitTestSupervised
+    : public ExtensionInfoGeneratorUnitTest {
+ public:
+  ExtensionInfoGeneratorUnitTestSupervised() = default;
+  ~ExtensionInfoGeneratorUnitTestSupervised() override = default;
+
+  SupervisedUserService* GetSupervisedUserService() {
+    return SupervisedUserServiceFactory::GetForProfile(profile());
+  }
+
+  // ExtensionInfoGeneratorUnitTest:
+  ExtensionServiceInitParams GetExtensionServiceInitParams() override {
+    ExtensionServiceInitParams params =
+        ExtensionInfoGeneratorUnitTest::GetExtensionServiceInitParams();
+    // Force a TestingPrefServiceSyncable to be created.
+    params.pref_file.clear();
+    params.profile_is_supervised = true;
+    return params;
+  }
+
+  void SetUp() override {
+    ExtensionInfoGeneratorUnitTest::SetUp();
+
+    // Set up custodians (parents) for the child.
+    supervised_user_test_util::AddCustodians(profile());
+
+    GetSupervisedUserService()->Init();
+    // Set the pref to allow the child to request extension install.
+    GetSupervisedUserService()
+        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
+  }
+};
+
+// Tests that when an extension is disabled pending permission updates, and the
+// parent has turned off the "Permissions for sites, apps and extensions"
+// toggle, then supervised users will see a kite error icon with a tooltip.
+TEST_F(ExtensionInfoGeneratorUnitTestSupervised,
+       ParentDisabledPermissionsForSupervisedUsers) {
+  // Preconditions.
+  ASSERT_TRUE(profile()->IsChild());
+
+  base::FilePath base_path = data_dir().AppendASCII("permissions_increase");
+  base::FilePath pem_path = base_path.AppendASCII("permissions.pem");
+  base::FilePath path = base_path.AppendASCII("v1");
+  const Extension* extension =
+      PackAndInstallCRX(path, pem_path, INSTALL_WITHOUT_LOAD);
+  // The extension should be installed but disabled pending custodian approval.
+  ASSERT_TRUE(extension);
+  EXPECT_TRUE(registry()->disabled_extensions().Contains(extension->id()));
+
+  // Save the id, as |extension| will be destroyed during updating.
+  std::string extension_id = extension->id();
+
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+  EXPECT_TRUE(prefs->HasDisableReason(
+      extension_id, disable_reason::DISABLE_CUSTODIAN_APPROVAL_REQUIRED));
+
+  // Simulate parent approval for the extension installation.
+  GetSupervisedUserService()->AddExtensionApproval(*extension);
+  // The extension should be enabled now.
+  EXPECT_TRUE(registry()->enabled_extensions().Contains(extension_id));
+
+  // Update to a new version with increased permissions.
+  path = base_path.AppendASCII("v2");
+  PackCRXAndUpdateExtension(extension_id, path, pem_path, DISABLED);
+
+  // The extension should be disabled pending approval for permission increases.
+  EXPECT_TRUE(registry()->disabled_extensions().Contains(extension_id));
+
+  // Due to a permission increase, prefs will contain escalation information.
+  EXPECT_TRUE(prefs->DidExtensionEscalatePermissions(extension_id));
+
+  // Simulate the parent disallowing the child from approving permission
+  // updates.
+  GetSupervisedUserService()
+      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+
+  std::unique_ptr<api::developer_private::ExtensionInfo> info =
+      GenerateExtensionInfo(extension_id);
+
+  // Verify that the kite icon error tooltip appears for supervised users.
+  EXPECT_TRUE(info->disable_reasons.parent_disabled_permissions);
+}
+
+#endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
 }  // namespace extensions

chrome/browser/supervised_user/supervised_user_service.cc

@@ -278,7 +278,7 @@ bool SupervisedUserService::IsSupervisedUserIframeFilterEnabled() const {
 }
 
 bool SupervisedUserService::IsChild() const {
-  return profile_->IsSupervised();
+  return profile_->IsChild();
 }
 
 bool SupervisedUserService::IsSupervisedUserExtensionInstallEnabled() const {
@@ -373,6 +373,9 @@ void SupervisedUserService::UpdateApprovedExtensionForTesting(
 
 bool SupervisedUserService::
     GetSupervisedUserExtensionsMayRequestPermissionsPref() const {
+  DCHECK(IsChild())
+      << "Calling GetSupervisedUserExtensionsMayRequestPermissionsPref() only "
+         "makes sense for supervised users";
   return profile_->GetPrefs()->GetBoolean(
       prefs::kSupervisedUserExtensionsMayRequestPermissions);
 }