[Extensions] Apply policy host settings when re-enabling the extensions
On policy update, host settings (permissions) are updates for enabled
extensions only. Therefore it could be possible that policy changes
while the extension was disables, so it will have outdated host
permissions.
This CL fixes this by applying permissions right after extension is
enabled back. Also it removes some small code duplication in
ExtensionSettings and PermissionUpdater.
Bug: 982966
Change-Id: Ief08c4ec6b568ec31209890061592d03f5792b6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3038799
Commit-Queue: Oleg Davydov <burunduk@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905668}
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 5ebd66e0..f1a9f5c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1109,6 +1109,10 @@
// according to header file once diffs have settled down.
void ExtensionService::PostActivateExtension(
scoped_refptr<const Extension> extension) {
+ // Update policy permissions in case they were changed while extension was not
+ // active.
+ PermissionsUpdater(profile()).ApplyPolicyHostRestrictions(*extension);
+
// TODO(kalman): Convert ExtensionSpecialStoragePolicy to a
// BrowserContextKeyedService and use ExtensionRegistryObserver.
profile_->GetExtensionSpecialStoragePolicy()->GrantRightsForExtension(
@@ -1191,7 +1195,7 @@
management->GetDefaultPolicyAllowedHosts());
for (const auto& extension : registry_->enabled_extensions()) {
- SetPolicySettingsForExtension(extension.get());
+ PermissionsUpdater(profile()).ApplyPolicyHostRestrictions(*extension);
}
// Loop through the disabled extension list, find extensions to re-enable
@@ -1844,19 +1848,6 @@
MaybeFinishDelayedInstallations();
}
-void ExtensionService::SetPolicySettingsForExtension(
- const Extension* extension) {
- ExtensionManagement* management =
- ExtensionManagementFactory::GetForBrowserContext(profile());
- if (management->UsesDefaultPolicyHostRestrictions(extension)) {
- PermissionsUpdater(profile()).SetUsesDefaultHostRestrictions(extension);
- } else {
- PermissionsUpdater(profile()).SetPolicyHostRestrictions(
- extension, management->GetPolicyBlockedHosts(extension),
- management->GetPolicyAllowedHosts(extension));
- }
-}
-
const Extension* ExtensionService::GetPendingExtensionUpdate(
const std::string& id) const {
return delayed_installs_.GetByID(id);
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 376fb41..aa2beca5 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -530,11 +530,6 @@
// Common helper to finish installing the given extension.
void FinishInstallation(const Extension* extension);
- // Sets the policy settings for the extension basically
- // by delegating this to the permission_data_updater.
- // Holds for default and policy settings.
- void SetPolicySettingsForExtension(const Extension* extension);
-
// Disables the extension if the privilege level has increased
// (e.g., due to an upgrade).
void CheckPermissionsIncrease(const Extension* extension,
diff --git a/chrome/browser/extensions/permissions_updater.cc b/chrome/browser/extensions/permissions_updater.cc
index 3d5583c..1c85fe6 100644
--- a/chrome/browser/extensions/permissions_updater.cc
+++ b/chrome/browser/extensions/permissions_updater.cc
@@ -420,6 +420,19 @@
std::move(completion_callback));
}
+void PermissionsUpdater::ApplyPolicyHostRestrictions(
+ const Extension& extension) {
+ ExtensionManagement* management =
+ ExtensionManagementFactory::GetForBrowserContext(browser_context_);
+ if (management->UsesDefaultPolicyHostRestrictions(&extension)) {
+ SetUsesDefaultHostRestrictions(&extension);
+ } else {
+ SetPolicyHostRestrictions(&extension,
+ management->GetPolicyBlockedHosts(&extension),
+ management->GetPolicyAllowedHosts(&extension));
+ }
+}
+
void PermissionsUpdater::SetPolicyHostRestrictions(
const Extension* extension,
const URLPatternSet& runtime_blocked_hosts,
@@ -540,15 +553,7 @@
if ((init_flag_ & INIT_FLAG_TRANSIENT) == 0) {
update_active_permissions = true;
// Apply per-extension policy if set.
- ExtensionManagement* management =
- ExtensionManagementFactory::GetForBrowserContext(browser_context_);
- if (!management->UsesDefaultPolicyHostRestrictions(extension)) {
- SetPolicyHostRestrictions(extension,
- management->GetPolicyBlockedHosts(extension),
- management->GetPolicyAllowedHosts(extension));
- } else {
- SetUsesDefaultHostRestrictions(extension);
- }
+ ApplyPolicyHostRestrictions(*extension);
}
SetPermissions(extension, std::move(granted_permissions),
diff --git a/chrome/browser/extensions/permissions_updater.h b/chrome/browser/extensions/permissions_updater.h
index 1f6e8fd..17d70995 100644
--- a/chrome/browser/extensions/permissions_updater.h
+++ b/chrome/browser/extensions/permissions_updater.h
@@ -123,6 +123,10 @@
void RemovePermissionsUnsafe(const Extension* extension,
const PermissionSet& permissions);
+ // Fetches the policy settings from the ExtensionManagement service and
+ // applies them to the extension.
+ void ApplyPolicyHostRestrictions(const Extension& extension);
+
// Sets list of hosts |extension| may not interact with (overrides default).
void SetPolicyHostRestrictions(const Extension* extension,
const URLPatternSet& runtime_blocked_hosts,
diff --git a/chrome/browser/policy/extension_policy_browsertest.cc b/chrome/browser/policy/extension_policy_browsertest.cc
index 96b35b6..4083023 100644
--- a/chrome/browser/policy/extension_policy_browsertest.cc
+++ b/chrome/browser/policy/extension_policy_browsertest.cc
@@ -2206,6 +2206,41 @@
extension_prefs->GetDisableReasons(kGoodCrxId));
}
+// Verifies that policy host block/allow settings are applied even when
+// extension is disabled.
+IN_PROC_BROWSER_TEST_F(ExtensionPolicyTest, ExtensionBlockedHostWhenDisabled) {
+ GURL test_url = GURL("http://www.google.com");
+ std::string* error = nullptr;
+ int tab_id = 1;
+
+ const extensions::Extension* extension = InstallExtension(kGoodCrxName);
+ ASSERT_TRUE(extension);
+ {
+ extensions::URLPatternSet new_hosts;
+ new_hosts.AddOrigin(URLPattern::SCHEME_ALL, test_url);
+ extension->permissions_data()->UpdateTabSpecificPermissions(
+ tab_id, extensions::PermissionSet(extensions::APIPermissionSet(),
+ extensions::ManifestPermissionSet(),
+ std::move(new_hosts),
+ extensions::URLPatternSet()));
+ }
+
+ ASSERT_TRUE(extension_service()->IsExtensionEnabled(extension->id()));
+
+ ASSERT_TRUE(
+ extension->permissions_data()->CanAccessPage(test_url, tab_id, error));
+
+ DisableExtension(extension->id());
+ {
+ extensions::ExtensionManagementPolicyUpdater pref(&provider_);
+ pref.AddPolicyBlockedHost(extension->id(), "*://*.google.com");
+ }
+ extension_service()->EnableExtension(extension->id());
+
+ EXPECT_FALSE(
+ extension->permissions_data()->CanAccessPage(test_url, tab_id, error));
+}
+
// Similar to ExtensionPolicyTest but sets the WebAppInstallForceList policy
// before the browser is started.
class WebAppInstallForceListPolicyTest : public ExtensionPolicyTest {
@@ -2482,10 +2517,10 @@
extensions::URLPatternSet new_hosts;
new_hosts.AddOrigin(url_scheme, url);
extension->permissions_data()->UpdateTabSpecificPermissions(
- 1, extensions::PermissionSet(extensions::APIPermissionSet(),
- extensions::ManifestPermissionSet(),
- std::move(new_hosts),
- extensions::URLPatternSet()));
+ tab_id, extensions::PermissionSet(extensions::APIPermissionSet(),
+ extensions::ManifestPermissionSet(),
+ std::move(new_hosts),
+ extensions::URLPatternSet()));
}
MockConfigurationPolicyProvider* GetProfile1Policy() {
