Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4327)

Unified Diff: chrome/browser/extensions/extension_service_unittest.cc

Issue 595363002: Add policy controlled permission block list for extensions (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@ext-fix
Patch Set: fix memory leaks Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/extensions/extension_service.cc ('k') | chrome/browser/extensions/extension_system_impl.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/extensions/extension_service_unittest.cc
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index d23b98ec30b357e8496fa22eaee6197d56ccdf68..0fde625d54b5699d7babda5a2c4a4fd0dee8830e 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -55,6 +55,7 @@
#include "chrome/browser/extensions/pack_extension_job.h"
#include "chrome/browser/extensions/pending_extension_info.h"
#include "chrome/browser/extensions/pending_extension_manager.h"
+#include "chrome/browser/extensions/permissions_updater.h"
#include "chrome/browser/extensions/test_blacklist.h"
#include "chrome/browser/extensions/test_extension_system.h"
#include "chrome/browser/extensions/unpacked_installer.h"
@@ -99,6 +100,7 @@
#include "extensions/common/feature_switch.h"
#include "extensions/common/manifest_constants.h"
#include "extensions/common/manifest_handlers/background_info.h"
+#include "extensions/common/manifest_handlers/permissions_parser.h"
#include "extensions/common/manifest_url_handlers.h"
#include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h"
@@ -189,6 +191,7 @@ const char theme2_crx[] = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
const char permissions_crx[] = "eagpmdpfmaekmmcejjbmjoecnejeiiin";
const char unpacked[] = "cbcdidchbppangcjoddlpdjlenngjldk";
const char updates_from_webstore[] = "akjooamlhcgeopfifcmlggaebeocgokj";
+const char permissions_blocklist[] = "noffkehfcaggllbcojjbopcmlhcnhcdn";
struct ExtensionsOrder {
bool operator()(const scoped_refptr<const Extension>& a,
@@ -767,6 +770,16 @@ class ExtensionServiceTest : public extensions::ExtensionServiceTestBase,
json_blacklist, gpu_info);
}
+ // Grants all optional permissions stated in manifest to active permission
+ // set for extension |id|.
+ void GrantAllOptionalPermissions(const std::string& id) {
+ const Extension* extension = service()->GetInstalledExtension(id);
+ scoped_refptr<const PermissionSet> all_optional_permissions =
+ extensions::PermissionsParser::GetOptionalPermissions(extension);
+ extensions::PermissionsUpdater perms_updater(profile());
+ perms_updater.AddPermissions(extension, all_optional_permissions.get());
+ }
+
// Helper method to set up a WindowedNotificationObserver to wait for a
// specific CrxInstaller to finish if we don't know the value of the
// |installer| yet.
@@ -3843,6 +3856,219 @@ TEST_F(ExtensionServiceTest, ManagementPolicyRequiresEnable) {
EXPECT_EQ(0u, registry()->disabled_extensions().size());
}
+// Tests that extensions with conflicting required permissions by enterprise
+// policy cannot be installed.
+TEST_F(ExtensionServiceTest, PolicyBlockedPermissionNewExtensionInstall) {
+ InitializeEmptyExtensionServiceWithTestingPrefs();
+ base::FilePath path = data_dir().AppendASCII("permissions_blocklist");
+
+ {
+ // Update policy to block one of the required permissions of target.
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.AddBlockedPermission("*", "tabs");
+ }
+
+ // The extension should be failed to install.
+ PackAndInstallCRX(path, INSTALL_FAILED);
+
+ {
+ // Update policy to block one of the optional permissions instead.
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.ClearBlockedPermissions("*");
+ pref.AddBlockedPermission("*", "history");
+ }
+
+ // The extension should succeed to install this time.
+ std::string id = PackAndInstallCRX(path, INSTALL_NEW)->id();
+
+ // Uninstall the extension and update policy to block some arbitrary
+ // unknown permission.
+ UninstallExtension(id, false);
+ {
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.ClearBlockedPermissions("*");
+ pref.AddBlockedPermission("*", "unknown.permission.for.testing");
+ }
+
+ // The extension should succeed to install as well.
+ PackAndInstallCRX(path, INSTALL_NEW);
+}
+
+// Tests that extension supposed to be force installed but with conflicting
+// required permissions cannot be installed.
+TEST_F(ExtensionServiceTest, PolicyBlockedPermissionConflictsWithForceInstall) {
+ InitializeEmptyExtensionServiceWithTestingPrefs();
+
+ // Pack the crx file.
+ base::FilePath path = data_dir().AppendASCII("permissions_blocklist");
+ base::FilePath pem_path = data_dir().AppendASCII("permissions_blocklist.pem");
+ base::ScopedTempDir temp_dir;
+ EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
+ base::FilePath crx_path = temp_dir.path().AppendASCII("temp.crx");
+
+ PackCRX(path, pem_path, crx_path);
+
+ {
+ // Block one of the required permissions.
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.AddBlockedPermission("*", "tabs");
+ }
+
+ // Use MockExtensionProvider to simulate force installing extension.
+ MockExtensionProvider* provider =
+ new MockExtensionProvider(service(), Manifest::EXTERNAL_POLICY_DOWNLOAD);
+ AddMockExternalProvider(provider);
+ provider->UpdateOrAddExtension(permissions_blocklist, "1.0", crx_path);
+
+ {
+ // Attempts to force install this extension.
+ content::WindowedNotificationObserver observer(
+ extensions::NOTIFICATION_CRX_INSTALLER_DONE,
+ content::NotificationService::AllSources());
+ service()->CheckForExternalUpdates();
+ observer.Wait();
+ }
+
+ // The extension should not be installed.
+ ASSERT_FALSE(service()->GetInstalledExtension(permissions_blocklist));
+
+ // Remove this extension from pending extension manager as we would like to
+ // give another attempt later.
+ service()->pending_extension_manager()->Remove(permissions_blocklist);
+
+ {
+ // Clears the permission block list.
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.ClearBlockedPermissions("*");
+ }
+
+ {
+ // Attempts to force install this extension again.
+ content::WindowedNotificationObserver observer(
+ extensions::NOTIFICATION_CRX_INSTALLER_DONE,
+ content::NotificationService::AllSources());
+ service()->CheckForExternalUpdates();
+ observer.Wait();
+ }
+
+ const Extension* installed =
+ service()->GetInstalledExtension(permissions_blocklist);
+ ASSERT_TRUE(installed);
+ EXPECT_EQ(installed->location(), Manifest::EXTERNAL_POLICY_DOWNLOAD);
+}
+
+// Tests that newer versions of an extension with conflicting required
+// permissions by enterprise policy cannot be updated to.
+TEST_F(ExtensionServiceTest, PolicyBlockedPermissionExtensionUpdate) {
+ InitializeEmptyExtensionServiceWithTestingPrefs();
+
+ base::FilePath path = data_dir().AppendASCII("permissions_blocklist");
+ base::FilePath path2 = data_dir().AppendASCII("permissions_blocklist2");
+ base::FilePath pem_path = data_dir().AppendASCII("permissions_blocklist.pem");
+
+ // Install 'permissions_blocklist'.
+ const Extension* installed = PackAndInstallCRX(path, pem_path, INSTALL_NEW);
+ EXPECT_EQ(installed->id(), permissions_blocklist);
+
+ {
+ // Block one of the required permissions of 'permissions_blocklist2'.
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.AddBlockedPermission("*", "downloads");
+ }
+
+ // Install 'permissions_blocklist' again, should be updated.
+ const Extension* updated = PackAndInstallCRX(path, pem_path, INSTALL_UPDATED);
+ EXPECT_EQ(updated->id(), permissions_blocklist);
+
+ std::string old_version = updated->VersionString();
+
+ // Attempts to update to 'permissions_blocklist2' should fail.
+ PackAndInstallCRX(path2, pem_path, INSTALL_FAILED);
+
+ // Verify that the old version is still enabled.
+ updated = service()->GetExtensionById(permissions_blocklist, false);
+ ASSERT_TRUE(updated);
+ EXPECT_EQ(old_version, updated->VersionString());
+}
+
+// Tests that policy update with additional permissions blocked revoke
+// conflicting granted optional permissions and unload extensions with
+// conflicting required permissions, including the force installed ones.
+TEST_F(ExtensionServiceTest, PolicyBlockedPermissionPolicyUpdate) {
+ InitializeEmptyExtensionServiceWithTestingPrefs();
+
+ base::FilePath path = data_dir().AppendASCII("permissions_blocklist");
+ base::FilePath path2 = data_dir().AppendASCII("permissions_blocklist2");
+ base::FilePath pem_path = data_dir().AppendASCII("permissions_blocklist.pem");
+
+ // Pack the crx file.
+ base::ScopedTempDir temp_dir;
+ EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
+ base::FilePath crx_path = temp_dir.path().AppendASCII("temp.crx");
+
+ PackCRX(path2, pem_path, crx_path);
+
+ // Install two arbitary extensions with specified manifest.
+ std::string ext1 = PackAndInstallCRX(path, INSTALL_NEW)->id();
+ std::string ext2 = PackAndInstallCRX(path2, INSTALL_NEW)->id();
+ ASSERT_NE(ext1, permissions_blocklist);
+ ASSERT_NE(ext2, permissions_blocklist);
+ ASSERT_NE(ext1, ext2);
+
+ // Force install another extension with known id and same manifest as 'ext2'.
+ std::string ext2_forced = permissions_blocklist;
+ MockExtensionProvider* provider =
+ new MockExtensionProvider(service(), Manifest::EXTERNAL_POLICY_DOWNLOAD);
+ AddMockExternalProvider(provider);
+ provider->UpdateOrAddExtension(ext2_forced, "2.0", crx_path);
+
+ content::WindowedNotificationObserver observer(
+ extensions::NOTIFICATION_CRX_INSTALLER_DONE,
+ content::NotificationService::AllSources());
+ service()->CheckForExternalUpdates();
+ observer.Wait();
+
+ extensions::ExtensionRegistry* registry =
+ extensions::ExtensionRegistry::Get(profile());
+
+ // Verify all three extensions are installed and enabled.
+ ASSERT_TRUE(registry->enabled_extensions().GetByID(ext1));
+ ASSERT_TRUE(registry->enabled_extensions().GetByID(ext2));
+ ASSERT_TRUE(registry->enabled_extensions().GetByID(ext2_forced));
+
+ // Grant all optional permissions to each extension.
+ GrantAllOptionalPermissions(ext1);
+ GrantAllOptionalPermissions(ext2);
+ GrantAllOptionalPermissions(ext2_forced);
+
+ scoped_refptr<const PermissionSet> active_permissions(
+ ExtensionPrefs::Get(profile())->GetActivePermissions(ext1));
+ EXPECT_TRUE(active_permissions->HasAPIPermission(
+ extensions::APIPermission::kDownloads));
+
+ // Set policy to block 'downloads' permission.
+ {
+ ManagementPrefUpdater pref(profile_->GetTestingPrefService());
+ pref.AddBlockedPermission("*", "downloads");
+ }
+
+ base::RunLoop().RunUntilIdle();
+
+ // 'ext1' should still be enabled, but with 'downloads' permission revoked.
+ EXPECT_TRUE(registry->enabled_extensions().GetByID(ext1));
+ active_permissions =
+ ExtensionPrefs::Get(profile())->GetActivePermissions(ext1);
+ EXPECT_FALSE(active_permissions->HasAPIPermission(
+ extensions::APIPermission::kDownloads));
+
+ // 'ext2' should be disabled because one of its required permissions is
+ // blocked.
+ EXPECT_FALSE(registry->enabled_extensions().GetByID(ext2));
+
+ // 'ext2_forced' should be handled the same as 'ext2'
+ EXPECT_FALSE(registry->enabled_extensions().GetByID(ext2_forced));
+}
+
// Flaky on windows; http://crbug.com/309833
#if defined(OS_WIN)
#define MAYBE_ExternalExtensionAutoAcknowledgement DISABLED_ExternalExtensionAutoAcknowledgement
« no previous file with comments | « chrome/browser/extensions/extension_service.cc ('k') | chrome/browser/extensions/extension_system_impl.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698