Skip to content

GitLab

T
tangled
Commit b8dffe56 authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit bot
Browse files
Options

[Extensions] Enable the scripts-require-action feature based on all-urls pref 

The scripts-require-action feature should affect all extensions that don't have
the "can run on all urls" preference. When an extension is installed, it should
have the "all urls" flag by default if the switch is off.

This is necessary in order to allow syncing the all urls flag.

BUG=454832

Review URL: https://codereview.chromium.org/874683005

Cr-Commit-Position: refs/heads/master@{#315153}
parent 681ceae0
Hide whitespace changes
Inline Side-by-side
Showing with 199 additions and 139 deletions
chrome/browser/extensions/active_script_controller.cc
View file @ b8dffe56
...@@ -37,31 +37,11 @@ ...@@ -37,31 +37,11 @@
namespace extensions { namespace extensions {
namespace {
// Returns true if the extension should be regarded as a "permitted" extension
// for the case of metrics. We need this because we only actually withhold
// permissions if the switch is enabled, but want to record metrics in all
// cases.
// "ExtensionWouldHaveHadHostPermissionsWithheldIfSwitchWasOn()" would be
// more accurate, but too long.
bool ShouldRecordExtension(const Extension* extension) {
return extension->ShouldDisplayInExtensionSettings() &&
!Manifest::IsPolicyLocation(extension->location()) &&
!Manifest::IsComponentLocation(extension->location()) &&
!PermissionsData::CanExecuteScriptEverywhere(extension) &&
extension->permissions_data()
->active_permissions()
->ShouldWarnAllHosts();
}
} // namespace
ActiveScriptController::ActiveScriptController( ActiveScriptController::ActiveScriptController(
content::WebContents* web_contents) content::WebContents* web_contents)
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
browser_context_(web_contents->GetBrowserContext()), browser_context_(web_contents->GetBrowserContext()),
enabled_(FeatureSwitch::scripts_require_action()->IsEnabled()), was_used_on_page_(false),
extension_registry_observer_(this) { extension_registry_observer_(this) {
CHECK(web_contents); CHECK(web_contents);
extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_)); extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_));
...@@ -143,7 +123,7 @@ void ActiveScriptController::OnClicked(const Extension* extension) { ...@@ -143,7 +123,7 @@ void ActiveScriptController::OnClicked(const Extension* extension) {
} }
bool ActiveScriptController::WantsToRun(const Extension* extension) { bool ActiveScriptController::WantsToRun(const Extension* extension) {
return enabled_ && pending_requests_.count(extension->id()) > 0; return pending_requests_.count(extension->id()) > 0;
} }
PermissionsData::AccessType PermissionsData::AccessType
...@@ -152,11 +132,6 @@ ActiveScriptController::RequiresUserConsentForScriptInjection( ...@@ -152,11 +132,6 @@ ActiveScriptController::RequiresUserConsentForScriptInjection(
UserScript::InjectionType type) { UserScript::InjectionType type) {
CHECK(extension); CHECK(extension);
// If the feature is not enabled, we automatically allow all extensions to
// run scripts.
if (!enabled_)
permitted_extensions_.insert(extension->id());
// Allow the extension if it's been explicitly granted permission. // Allow the extension if it's been explicitly granted permission.
if (permitted_extensions_.count(extension->id()) > 0) if (permitted_extensions_.count(extension->id()) > 0)
return PermissionsData::ACCESS_ALLOWED; return PermissionsData::ACCESS_ALLOWED;
...@@ -187,6 +162,8 @@ void ActiveScriptController::RequestScriptInjection( ...@@ -187,6 +162,8 @@ void ActiveScriptController::RequestScriptInjection(
// to run. // to run.
if (list.size() == 1u) if (list.size() == 1u)
NotifyChange(extension); NotifyChange(extension);
was_used_on_page_ = true;
} }
void ActiveScriptController::RunPendingForExtension( void ActiveScriptController::RunPendingForExtension(
...@@ -253,8 +230,9 @@ void ActiveScriptController::OnRequestScriptInjectionPermission( ...@@ -253,8 +230,9 @@ void ActiveScriptController::OnRequestScriptInjectionPermission(
// ran (because this feature is not enabled). Add the extension to the list of // ran (because this feature is not enabled). Add the extension to the list of
// permitted extensions (for metrics), and return immediately. // permitted extensions (for metrics), and return immediately.
if (request_id == -1) { if (request_id == -1) {
if (ShouldRecordExtension(extension)) { if (util::ScriptsMayRequireActionForExtension(
DCHECK(!enabled_); extension,
extension->permissions_data()->active_permissions().get())) {
permitted_extensions_.insert(extension->id()); permitted_extensions_.insert(extension->id());
} }
return; return;
...@@ -313,9 +291,9 @@ void ActiveScriptController::LogUMA() const { ...@@ -313,9 +291,9 @@ void ActiveScriptController::LogUMA() const {
"Extensions.ActiveScriptController.ShownActiveScriptsOnPage", "Extensions.ActiveScriptController.ShownActiveScriptsOnPage",
pending_requests_.size()); pending_requests_.size());
// We only log the permitted extensions metric if the feature is enabled, // We only log the permitted extensions metric if the feature was used at all
// because otherwise the data will be boring (100% allowed). // on the page, because otherwise the data will be boring.
if (enabled_) { if (was_used_on_page_) {
UMA_HISTOGRAM_COUNTS_100( UMA_HISTOGRAM_COUNTS_100(
"Extensions.ActiveScriptController.PermittedExtensions", "Extensions.ActiveScriptController.PermittedExtensions",
permitted_extensions_.size()); permitted_extensions_.size());
...@@ -344,6 +322,7 @@ void ActiveScriptController::DidNavigateMainFrame( ...@@ -344,6 +322,7 @@ void ActiveScriptController::DidNavigateMainFrame(
LogUMA(); LogUMA();
permitted_extensions_.clear(); permitted_extensions_.clear();
pending_requests_.clear(); pending_requests_.clear();
was_used_on_page_ = false;
} }
void ActiveScriptController::OnExtensionUnloaded( void ActiveScriptController::OnExtensionUnloaded(
......
chrome/browser/extensions/active_script_controller.h
View file @ b8dffe56
...@@ -130,10 +130,9 @@ class ActiveScriptController : public content::WebContentsObserver, ...@@ -130,10 +130,9 @@ class ActiveScriptController : public content::WebContentsObserver,
// The associated browser context. // The associated browser context.
content::BrowserContext* browser_context_; content::BrowserContext* browser_context_;
// Whether or not the ActiveScriptController is enabled (corresponding to the // Whether or not the feature was used for any extensions. This may not be the
// kActiveScriptEnforcement switch). If it is not, it acts as an empty shell, // case if the user never enabled the scripts-require-action flag.
// always allowing scripts to run and never displaying actions. bool was_used_on_page_;
bool enabled_;
// The map of extension_id:pending_request of all pending requests. // The map of extension_id:pending_request of all pending requests.
PendingRequestMap pending_requests_; PendingRequestMap pending_requests_;
......
chrome/browser/extensions/crx_installer_browsertest.cc
View file @ b8dffe56
...@@ -598,42 +598,4 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, ManagementPolicy) { ...@@ -598,42 +598,4 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, ManagementPolicy) {
EXPECT_FALSE(InstallExtension(crx_path, 0)); EXPECT_FALSE(InstallExtension(crx_path, 0));
} }
IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, WithheldElevationCheck) {
// Enable consent flag and install extension. The <all_hosts> permission will
// be withheld.
scoped_ptr<FeatureSwitch::ScopedOverride> enable_scripts_switch(
new FeatureSwitch::ScopedOverride(
FeatureSwitch::scripts_require_action(), true));
const char kManifest[] =
"{"
" \"name\": \"Withheld test\","
" \"version\": \"1.0\","
" \"permissions\": ["
" \"http://*/*\""
" ],"
" \"manifest_version\": 2"
"}";
TestExtensionDir dir;
dir.WriteManifest(kManifest);
base::FilePath crx_path = dir.Pack();
EXPECT_FALSE(crx_path.empty());
const Extension* extension = InstallExtension(crx_path, 1);
EXPECT_TRUE(base::PathExists(extension->path()));
std::string extension_id = extension->id();
ExtensionRegistry* registry = ExtensionRegistry::Get(
browser()->profile());
EXPECT_TRUE(registry->enabled_extensions().GetByID(extension_id));
// Disable consent flag and reinstall extension. It should now be disabled
// because previously withheld permissions are now being requested.
enable_scripts_switch.reset();
extension = InstallExtension(crx_path, -1);
EXPECT_FALSE(registry->enabled_extensions().GetByID(extension_id));
EXPECT_TRUE(registry->disabled_extensions().GetByID(extension_id));
EXPECT_TRUE(ExtensionPrefs::Get(browser()->profile())->GetDisableReasons(
extension_id) & Extension::DISABLE_PERMISSIONS_INCREASE);
}
} // namespace extensions } // namespace extensions
chrome/browser/extensions/extension_ui_unittest.cc
View file @ b8dffe56
...@@ -329,23 +329,22 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) { ...@@ -329,23 +329,22 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
scoped_ptr<base::DictionaryValue> value(handler()->CreateExtensionDetailValue( scoped_ptr<base::DictionaryValue> value(handler()->CreateExtensionDetailValue(
all_urls_extension.get(), std::vector<ExtensionPage>(), NULL)); all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
bool result = false; bool result = false;
const std::string kWantsAllUrls = "wantsAllUrls"; const std::string kShowAllUrls = "showAllUrls";
const std::string kAllowAllUrls = "allowAllUrls"; const std::string kAllowAllUrls = "allowAllUrls";
// The extension should want all urls, but not currently have it. // The extension should want all urls, but not currently have it.
EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_TRUE(result); EXPECT_TRUE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_FALSE(result);
// Give the extension all urls. // Give the extension all urls.
util::SetAllowedScriptingOnAllUrls( util::SetAllowedScriptingOnAllUrls(all_urls_extension->id(), profile(), true);
all_urls_extension->id(), profile(), true);
// Now the extension should both want and have all urls. // Now the extension should both want and have all urls.
value.reset(handler()->CreateExtensionDetailValue( value.reset(handler()->CreateExtensionDetailValue(
all_urls_extension.get(), std::vector<ExtensionPage>(), NULL)); all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_TRUE(result); EXPECT_TRUE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_TRUE(result); EXPECT_TRUE(result);
...@@ -353,20 +352,34 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) { ...@@ -353,20 +352,34 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
// The other extension should neither want nor have all urls. // The other extension should neither want nor have all urls.
value.reset(handler()->CreateExtensionDetailValue( value.reset(handler()->CreateExtensionDetailValue(
no_urls_extension.get(), std::vector<ExtensionPage>(), NULL)); no_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_FALSE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_FALSE(result);
// Revoke the first extension's permissions.
util::SetAllowedScriptingOnAllUrls(
all_urls_extension->id(), profile(), false);
// Turn off the switch and load another extension (so permissions are // Turn off the switch and load another extension (so permissions are
// re-initialized). // re-initialized).
enable_scripts_switch.reset(); enable_scripts_switch.reset();
// Even though the extension has the all urls preference, the checkbox // Since the extension doesn't have access to all urls (but normally would),
// shouldn't show up with the switch off. // the extension should have the "want" flag even with the switch off.
value.reset(handler()->CreateExtensionDetailValue( value.reset(handler()->CreateExtensionDetailValue(
all_urls_extension.get(), std::vector<ExtensionPage>(), NULL)); all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_TRUE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_FALSE(result);
// If we grant the extension all urls, then it should no longer "want" it,
// since it is back in its normal state (with the switch off).
util::SetAllowedScriptingOnAllUrls(all_urls_extension->id(), profile(), true);
value.reset(handler()->CreateExtensionDetailValue(
all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_FALSE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_TRUE(result); EXPECT_TRUE(result);
...@@ -379,10 +392,10 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) { ...@@ -379,10 +392,10 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
// show up without the switch. // show up without the switch.
value.reset(handler()->CreateExtensionDetailValue( value.reset(handler()->CreateExtensionDetailValue(
all_urls_extension.get(), std::vector<ExtensionPage>(), NULL)); all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kShowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_FALSE(result);
EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result)); EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
EXPECT_FALSE(result); EXPECT_TRUE(result);
} }
} // namespace extensions } // namespace extensions
chrome/browser/extensions/extension_util.cc
View file @ b8dffe56
...@@ -70,6 +70,35 @@ std::string ReloadExtensionIfEnabled(const std::string& extension_id, ...@@ -70,6 +70,35 @@ std::string ReloadExtensionIfEnabled(const std::string& extension_id,
return id; return id;
} }
// Sets the preference for scripting on all urls to |allowed|, optionally
// updating the extension's active permissions (based on |update_permissions|).
void SetAllowedScriptingOnAllUrlsHelper(
content::BrowserContext* context,
const std::string& extension_id,
bool allowed,
bool update_permissions) {
// TODO(devlin): Right now, we always need to have a value for this pref.
// Once the scripts-require-action feature launches, we can change the set
// to be null if false.
ExtensionPrefs::Get(context)->UpdateExtensionPref(
extension_id,
kExtensionAllowedOnAllUrlsPrefName,
new base::FundamentalValue(allowed));
if (update_permissions) {
const Extension* extension =
ExtensionRegistry::Get(context)->enabled_extensions().GetByID(
extension_id);
if (extension) {
PermissionsUpdater updater(context);
if (allowed)
updater.GrantWithheldImpliedAllHosts(extension);
else
updater.WithholdImpliedAllHosts(extension);
}
}
}
} // namespace } // namespace
bool IsIncognitoEnabled(const std::string& extension_id, bool IsIncognitoEnabled(const std::string& extension_id,
...@@ -176,46 +205,42 @@ void SetAllowFileAccess(const std::string& extension_id, ...@@ -176,46 +205,42 @@ void SetAllowFileAccess(const std::string& extension_id,
bool AllowedScriptingOnAllUrls(const std::string& extension_id, bool AllowedScriptingOnAllUrls(const std::string& extension_id,
content::BrowserContext* context) { content::BrowserContext* context) {
bool allowed = false; bool allowed = false;
return ExtensionPrefs::Get(context)->ReadPrefAsBoolean( ExtensionPrefs* prefs = ExtensionPrefs::Get(context);
extension_id, if (!prefs->ReadPrefAsBoolean(extension_id,
kExtensionAllowedOnAllUrlsPrefName, kExtensionAllowedOnAllUrlsPrefName,
&allowed) && &allowed)) {
allowed; // If there is no value present, we make one, defaulting it to the value of
// the 'scripts require action' flag. If the flag is on, then the extension
// does not have permission to script on all urls by default.
allowed = DefaultAllowedScriptingOnAllUrls();
SetAllowedScriptingOnAllUrlsHelper(context, extension_id, allowed, false);
}
return allowed;
} }
void SetAllowedScriptingOnAllUrls(const std::string& extension_id, void SetAllowedScriptingOnAllUrls(const std::string& extension_id,
content::BrowserContext* context, content::BrowserContext* context,
bool allowed) { bool allowed) {
if (allowed == AllowedScriptingOnAllUrls(extension_id, context)) if (allowed != AllowedScriptingOnAllUrls(extension_id, context))
return; // Nothing to do here. SetAllowedScriptingOnAllUrlsHelper(context, extension_id, allowed, true);
}
ExtensionPrefs::Get(context)->UpdateExtensionPref(
extension_id,
kExtensionAllowedOnAllUrlsPrefName,
allowed ? new base::FundamentalValue(true) : NULL);
const Extension* extension = bool DefaultAllowedScriptingOnAllUrls() {
ExtensionRegistry::Get(context)->enabled_extensions().GetByID( return !FeatureSwitch::scripts_require_action()->IsEnabled();
extension_id);
if (extension) {
PermissionsUpdater updater(context);
if (allowed)
updater.GrantWithheldImpliedAllHosts(extension);
else
updater.WithholdImpliedAllHosts(extension);
}
} }
bool ScriptsMayRequireActionForExtension(const Extension* extension) { bool ScriptsMayRequireActionForExtension(
// An extension requires user action to execute scripts iff the switch to do const Extension* extension,
// so is enabled, the extension shows up in chrome:extensions (so the user can const PermissionSet* permissions) {
// grant withheld permissions), the extension is not part of chrome or // An extension may require user action to execute scripts iff the extension
// corporate policy, and also not on the scripting whitelist. // shows up in chrome:extensions (so the user can grant withheld permissions),
return FeatureSwitch::scripts_require_action()->IsEnabled() && // is not part of chrome or corporate policy, not on the scripting whitelist,
extension->ShouldDisplayInExtensionSettings() && // and requires enough permissions that we should withhold them.
return extension->ShouldDisplayInExtensionSettings() &&
!Manifest::IsPolicyLocation(extension->location()) && !Manifest::IsPolicyLocation(extension->location()) &&
!Manifest::IsComponentLocation(extension->location()) && !Manifest::IsComponentLocation(extension->location()) &&
!PermissionsData::CanExecuteScriptEverywhere(extension); !PermissionsData::CanExecuteScriptEverywhere(extension) &&
permissions->ShouldWarnAllHosts();
} }
bool IsAppLaunchable(const std::string& extension_id, bool IsAppLaunchable(const std::string& extension_id,
......
chrome/browser/extensions/extension_util.h
View file @ b8dffe56
...@@ -26,6 +26,7 @@ namespace extensions { ...@@ -26,6 +26,7 @@ namespace extensions {
class Extension; class Extension;
struct ExtensionInfo; struct ExtensionInfo;
class PermissionSet;
namespace util { namespace util {
...@@ -68,6 +69,9 @@ void SetAllowFileAccess(const std::string& extension_id, ...@@ -68,6 +69,9 @@ void SetAllowFileAccess(const std::string& extension_id,
bool AllowedScriptingOnAllUrls(const std::string& extension_id, bool AllowedScriptingOnAllUrls(const std::string& extension_id,
content::BrowserContext* context); content::BrowserContext* context);
// Returns the default value for being allowed to script on all urls.
bool DefaultAllowedScriptingOnAllUrls();
// Sets whether the extension with |extension_id| is allowed to execute scripts // Sets whether the extension with |extension_id| is allowed to execute scripts
// on all urls (exempting chrome:// urls, etc) without explicit user consent. // on all urls (exempting chrome:// urls, etc) without explicit user consent.
// This should only be used with FeatureSwitch::scripts_require_action() // This should only be used with FeatureSwitch::scripts_require_action()
...@@ -77,8 +81,12 @@ void SetAllowedScriptingOnAllUrls(const std::string& extension_id, ...@@ -77,8 +81,12 @@ void SetAllowedScriptingOnAllUrls(const std::string& extension_id,
bool allowed); bool allowed);
// Returns true if the --scripts-require-action flag would possibly affect // Returns true if the --scripts-require-action flag would possibly affect
// the given |extension|. // the given |extension| and |permissions|. We pass in the |permissions|
bool ScriptsMayRequireActionForExtension(const Extension* extension); // explicitly, as we may need to check with permissions other than the ones
// that are currently on the extension's PermissionsData.
bool ScriptsMayRequireActionForExtension(
const Extension* extension,
const PermissionSet* permissions);
// Returns true if |extension_id| can be launched (possibly only after being // Returns true if |extension_id| can be launched (possibly only after being
// enabled). // enabled).
......
chrome/browser/extensions/permissions_updater.cc
View file @ b8dffe56
...@@ -182,14 +182,14 @@ void PermissionsUpdater::InitializePermissions(const Extension* extension) { ...@@ -182,14 +182,14 @@ void PermissionsUpdater::InitializePermissions(const Extension* extension) {
bounded_active = GetBoundedActivePermissions(extension, active_permissions); bounded_active = GetBoundedActivePermissions(extension, active_permissions);
} }
// Withhold permissions if the switch applies to this extension. // Determine whether or not to withhold host permissions.
// Non-transient extensions also must not have the preference to allow bool should_withhold_permissions = false;
// scripting on all urls. if (util::ScriptsMayRequireActionForExtension(extension,
bool should_withhold_permissions = bounded_active.get())) {
util::ScriptsMayRequireActionForExtension(extension); should_withhold_permissions =
if ((init_flag_ & INIT_FLAG_TRANSIENT) == 0) { init_flag_ & INIT_FLAG_TRANSIENT ?
should_withhold_permissions &= !util::DefaultAllowedScriptingOnAllUrls() :
!util::AllowedScriptingOnAllUrls(extension->id(), browser_context_); !util::AllowedScriptingOnAllUrls(extension->id(), browser_context_);
} }
URLPatternSet granted_explicit_hosts; URLPatternSet granted_explicit_hosts;
......
chrome/browser/extensions/permissions_updater_unittest.cc
View file @ b8dffe56
...@@ -11,10 +11,12 @@ ...@@ -11,10 +11,12 @@
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_service_test_base.h" #include "chrome/browser/extensions/extension_service_test_base.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/permissions_updater.h" #include "chrome/browser/extensions/permissions_updater.h"
#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension_test_util.h" #include "chrome/common/extensions/extension_test_util.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/crx_file/id_util.h"
#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
...@@ -36,7 +38,8 @@ namespace { ...@@ -36,7 +38,8 @@ namespace {
scoped_refptr<const Extension> CreateExtensionWithPermissions( scoped_refptr<const Extension> CreateExtensionWithPermissions(
const std::set<URLPattern>& scriptable_hosts, const std::set<URLPattern>& scriptable_hosts,
const std::set<URLPattern>& explicit_hosts, const std::set<URLPattern>& explicit_hosts,
Manifest::Location location) { Manifest::Location location,
const std::string& name) {
ListBuilder scriptable_host_list; ListBuilder scriptable_host_list;
for (std::set<URLPattern>::const_iterator pattern = scriptable_hosts.begin(); for (std::set<URLPattern>::const_iterator pattern = scriptable_hosts.begin();
pattern != scriptable_hosts.end(); pattern != scriptable_hosts.end();
...@@ -59,12 +62,13 @@ scoped_refptr<const Extension> CreateExtensionWithPermissions( ...@@ -59,12 +62,13 @@ scoped_refptr<const Extension> CreateExtensionWithPermissions(
.SetLocation(location) .SetLocation(location)
.SetManifest( .SetManifest(
DictionaryBuilder() DictionaryBuilder()
.Set("name", "extension") .Set("name", name)
.Set("description", "foo") .Set("description", "foo")
.Set("manifest_version", 2) .Set("manifest_version", 2)
.Set("version", "0.1.2.3") .Set("version", "0.1.2.3")
.Set("content_scripts", ListBuilder().Append(script.Pass())) .Set("content_scripts", ListBuilder().Append(script.Pass()))
.Set("permissions", explicit_host_list.Pass())) .Set("permissions", explicit_host_list.Pass()))
.SetID(crx_file::id_util::GenerateId(name))
.Build(); .Build();
} }
...@@ -300,7 +304,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) { ...@@ -300,7 +304,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) {
all_host_patterns, safe_patterns); all_host_patterns, safe_patterns);
scoped_refptr<const Extension> extension = CreateExtensionWithPermissions( scoped_refptr<const Extension> extension = CreateExtensionWithPermissions(
all_patterns, all_patterns, Manifest::INTERNAL); all_patterns, all_patterns, Manifest::INTERNAL, "a");
const PermissionsData* permissions_data = extension->permissions_data(); const PermissionsData* permissions_data = extension->permissions_data();
PermissionsUpdater updater(profile_.get()); PermissionsUpdater updater(profile_.get());
updater.InitializePermissions(extension.get()); updater.InitializePermissions(extension.get());
...@@ -359,7 +363,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) { ...@@ -359,7 +363,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) {
// Creating a component extension should result in no withheld permissions. // Creating a component extension should result in no withheld permissions.
extension = CreateExtensionWithPermissions( extension = CreateExtensionWithPermissions(
all_patterns, all_patterns, Manifest::COMPONENT); all_patterns, all_patterns, Manifest::COMPONENT, "b");
permissions_data = extension->permissions_data(); permissions_data = extension->permissions_data();
updater.InitializePermissions(extension.get()); updater.InitializePermissions(extension.get());
EXPECT_TRUE(SetsAreEqual( EXPECT_TRUE(SetsAreEqual(
...@@ -380,7 +384,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) { ...@@ -380,7 +384,7 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) {
// Without the switch, we shouldn't withhold anything. // Without the switch, we shouldn't withhold anything.
switch_override.reset(); switch_override.reset();
extension = CreateExtensionWithPermissions( extension = CreateExtensionWithPermissions(
all_patterns, all_patterns, Manifest::INTERNAL); all_patterns, all_patterns, Manifest::INTERNAL, "c");
permissions_data = extension->permissions_data(); permissions_data = extension->permissions_data();
updater.InitializePermissions(extension.get()); updater.InitializePermissions(extension.get());
EXPECT_TRUE(SetsAreEqual( EXPECT_TRUE(SetsAreEqual(
...@@ -399,4 +403,73 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) { ...@@ -399,4 +403,73 @@ TEST_F(PermissionsUpdaterTest, WithholdAllHosts) {
.empty()); .empty());
} }
// Tests that withholding all hosts behaves properly with extensions installed
// when the switch is turned on and off.
TEST_F(PermissionsUpdaterTest, WithholdAllHostsWithTransientSwitch) {
InitializeEmptyExtensionService();
URLPattern all_hosts(URLPattern::SCHEME_ALL, "<all_urls>");
std::set<URLPattern> all_host_patterns;
all_host_patterns.insert(all_hosts);
scoped_refptr<const Extension> extension_a = CreateExtensionWithPermissions(
all_host_patterns, all_host_patterns, Manifest::INTERNAL, "a");
PermissionsUpdater updater(profile());
updater.InitializePermissions(extension_a.get());
const PermissionsData* permissions_data = extension_a->permissions_data();
// Since the extension was created without the switch on, it should default
// to having all urls access.
EXPECT_TRUE(SetsAreEqual(
permissions_data->active_permissions()->scriptable_hosts().patterns(),
all_host_patterns));
EXPECT_TRUE(
permissions_data->withheld_permissions()->scriptable_hosts().is_empty());
EXPECT_TRUE(util::AllowedScriptingOnAllUrls(extension_a->id(), profile()));
// Enable the switch, and re-init permission for the extension.
scoped_ptr<FeatureSwitch::ScopedOverride> switch_override(
new FeatureSwitch::ScopedOverride(FeatureSwitch::scripts_require_action(),
FeatureSwitch::OVERRIDE_ENABLED));
updater.InitializePermissions(extension_a.get());
// Since the extension was installed when the switch was off, it should still
// have the all urls pref.
permissions_data = extension_a->permissions_data();
EXPECT_TRUE(SetsAreEqual(
permissions_data->active_permissions()->scriptable_hosts().patterns(),
all_host_patterns));
EXPECT_TRUE(
permissions_data->withheld_permissions()->scriptable_hosts().is_empty());
EXPECT_TRUE(util::AllowedScriptingOnAllUrls(extension_a->id(), profile()));
// Load a new extension, which also has all urls. Since the switch is now on,
// the permissions should be withheld.
scoped_refptr<const Extension> extension_b = CreateExtensionWithPermissions(
all_host_patterns, all_host_patterns, Manifest::INTERNAL, "b");
updater.InitializePermissions(extension_b.get());
permissions_data = extension_b->permissions_data();
EXPECT_TRUE(
permissions_data->active_permissions()->scriptable_hosts().is_empty());
EXPECT_TRUE(SetsAreEqual(
permissions_data->withheld_permissions()->scriptable_hosts().patterns(),
all_host_patterns));
EXPECT_FALSE(util::AllowedScriptingOnAllUrls(extension_b->id(), profile()));
// Disable the switch, and reload the extension.
switch_override.reset();
updater.InitializePermissions(extension_b.get());
// Since the extension was installed with the switch on, it should still be
// restricted with the switch off.
permissions_data = extension_b->permissions_data();
EXPECT_TRUE(
permissions_data->active_permissions()->scriptable_hosts().is_empty());
EXPECT_TRUE(SetsAreEqual(
permissions_data->withheld_permissions()->scriptable_hosts().patterns(),
all_host_patterns));
EXPECT_FALSE(util::AllowedScriptingOnAllUrls(extension_b->id(), profile()));
}
} // namespace extensions } // namespace extensions
chrome/browser/resources/extensions/extension_list.js
View file @ b8dffe56
...@@ -51,6 +51,7 @@ ...@@ -51,6 +51,7 @@
* prettifiedPath: (string|undefined), * prettifiedPath: (string|undefined),
* recommendedInstall: boolean, * recommendedInstall: boolean,
* runtimeErrors: (Array.<RuntimeError>|undefined), * runtimeErrors: (Array.<RuntimeError>|undefined),
* showAllUrls: boolean,
* suspiciousInstall: boolean, * suspiciousInstall: boolean,
* terminated: boolean, * terminated: boolean,
* updateRequiredByPolicy: boolean, * updateRequiredByPolicy: boolean,
...@@ -58,7 +59,6 @@ ...@@ -58,7 +59,6 @@
* views: Array.<{renderViewId: number, renderProcessId: number, * views: Array.<{renderViewId: number, renderProcessId: number,
* path: string, incognito: boolean, * path: string, incognito: boolean,
* generatedBackgroundPage: boolean}>, * generatedBackgroundPage: boolean}>,
* wantsAllUrls: boolean,
* wantsErrorCollection: boolean, * wantsErrorCollection: boolean,
* wantsFileAccess: boolean, * wantsFileAccess: boolean,
* warnings: (Array|undefined)}} * warnings: (Array|undefined)}}
...@@ -258,7 +258,7 @@ cr.define('options', function() { ...@@ -258,7 +258,7 @@ cr.define('options', function() {
// The 'allow on all urls' checkbox. This should only be visible if // The 'allow on all urls' checkbox. This should only be visible if
// active script restrictions are enabled. If they are not enabled, no // active script restrictions are enabled. If they are not enabled, no
// extensions should want all urls. // extensions should want all urls.
if (extension.wantsAllUrls) { if (extension.showAllUrls) {
var allUrls = node.querySelector('.all-urls-control'); var allUrls = node.querySelector('.all-urls-control');
allUrls.addEventListener('click', function(e) { allUrls.addEventListener('click', function(e) {
chrome.send('extensionSettingsAllowOnAllUrls', chrome.send('extensionSettingsAllowOnAllUrls',
......
chrome/browser/ui/webui/extensions/extension_settings_handler.cc
View file @ b8dffe56
...@@ -335,16 +335,17 @@ base::DictionaryValue* ExtensionSettingsHandler::CreateExtensionDetailValue( ...@@ -335,16 +335,17 @@ base::DictionaryValue* ExtensionSettingsHandler::CreateExtensionDetailValue(
} }
extension_data->Set("dependentExtensions", dependents_list); extension_data->Set("dependentExtensions", dependents_list);
// Extensions only want all URL access if: // We show the "all urls" checkbox if:
// - The feature is enabled for the given extension. // - The feature is enabled for the given extension.
// - The extension has access to enough urls that we can't just let it run // - The extension has access to enough urls that we can't just let it run
// on those specified in the permissions. // on those specified in the permissions.
bool wants_all_urls = bool show_all_urls =
util::ScriptsMayRequireActionForExtension(extension) && (FeatureSwitch::scripts_require_action()->IsEnabled() &&
(extension->permissions_data()->HasWithheldImpliedAllHosts() || util::ScriptsMayRequireActionForExtension(
util::AllowedScriptingOnAllUrls( extension,
extension->id(), extension_service_->GetBrowserContext())); extension->permissions_data()->active_permissions().get())) ||
extension_data->SetBoolean("wantsAllUrls", wants_all_urls); extension->permissions_data()->HasWithheldImpliedAllHosts();
extension_data->SetBoolean("showAllUrls", show_all_urls);
extension_data->SetBoolean( extension_data->SetBoolean(
"allowAllUrls", "allowAllUrls",
util::AllowedScriptingOnAllUrls( util::AllowedScriptingOnAllUrls(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment