Revert 95815 - Make extension file URL access opt-in.
This corrects an issue causing file URL access to default on for <all_urls> and file:/// permissions. We also revert all extension's "allow file access" flags to false since we can't distinguish between extensions that were installed with the bug present and those where the user clicked allow file access. Unpacked extensions will now have opt-in file access as well.
BUG=91577
TEST=ExtensionServiceTest.DefaultFileAccess
Review URL: http://codereview.chromium.org/7574017
[email protected]
Review URL: http://codereview.chromium.org/7595005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@95820 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 580870a4..ec64fdb 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -114,9 +114,9 @@
ui_test_utils::WindowedNotificationObserver load_signal(
chrome::NOTIFICATION_EXTENSION_LOADED,
Source<Profile>(browser()->profile()));
- CHECK(!service->AllowFileAccess(extension));
+ CHECK(service->AllowFileAccess(extension));
- if (fileaccess_enabled) {
+ if (!fileaccess_enabled) {
service->SetAllowFileAccess(extension, fileaccess_enabled);
load_signal.Wait();
extension = service->GetExtensionById(extension_id, false);
@@ -131,7 +131,7 @@
}
const Extension* ExtensionBrowserTest::LoadExtension(const FilePath& path) {
- return LoadExtensionWithOptions(path, false, false);
+ return LoadExtensionWithOptions(path, false, true);
}
const Extension* ExtensionBrowserTest::LoadExtensionIncognito(
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 88cedb7d6..49b892f9 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -74,11 +74,7 @@
// A preference to control whether an extension is allowed to inject script in
// pages with file URLs.
-const char kPrefAllowFileAccess[] = "newAllowFileAccess";
-// TODO(jstritar): As part of fixing http://crbug.com/91577, we revoked all
-// extension file access by renaming the pref. We should eventually clean up
-// the old flag and possibly go back to that name.
-// const char kPrefAllowFileAccessOld[] = "allowFileAccess";
+const char kPrefAllowFileAccess[] = "allowFileAccess";
// A preference set by the web store to indicate login information for
// purchased apps.
@@ -808,6 +804,12 @@
Value::CreateBooleanValue(allow));
}
+bool ExtensionPrefs::HasAllowFileAccessSetting(
+ const std::string& extension_id) const {
+ const DictionaryValue* ext = GetExtensionPref(extension_id);
+ return ext && ext->HasKey(kPrefAllowFileAccess);
+}
+
ExtensionPrefs::LaunchType ExtensionPrefs::GetLaunchType(
const std::string& extension_id,
ExtensionPrefs::LaunchType default_pref_value) {
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 8b40ad9..25c2c9a 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -207,6 +207,7 @@
// scripts into pages with file URLs.
bool AllowFileAccess(const std::string& extension_id);
void SetAllowFileAccess(const std::string& extension_id, bool allow);
+ bool HasAllowFileAccessSetting(const std::string& extension_id) const;
// Get the launch type preference. If no preference is set, return
// |default_pref_value|.
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 0ac80c7..5c05c6c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -304,7 +304,12 @@
const FilePath& extension_path, bool prompt_for_plugins) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
std::string id = Extension::GenerateIdForPath(extension_path);
- bool allow_file_access = frontend_->extension_prefs()->AllowFileAccess(id);
+ // Unpacked extensions default to allowing file access, but if that has been
+ // overridden, don't reset the value.
+ bool allow_file_access =
+ Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD);
+ if (frontend_->extension_prefs()->HasAllowFileAccessSetting(id))
+ allow_file_access = frontend_->extension_prefs()->AllowFileAccess(id);
BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
NewRunnableMethod(
@@ -1029,9 +1034,13 @@
file_util::AbsolutePath(&extension_path);
std::string id = Extension::GenerateIdForPath(extension_path);
+ bool allow_file_access =
+ Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD);
+ if (extension_prefs()->HasAllowFileAccessSetting(id))
+ allow_file_access = extension_prefs()->AllowFileAccess(id);
int flags = Extension::NO_FLAGS;
- if (extension_prefs()->AllowFileAccess(id))
+ if (allow_file_access)
flags |= Extension::ALLOW_FILE_ACCESS;
if (Extension::ShouldDoStrictErrorChecking(Extension::LOAD))
flags |= Extension::STRICT_ERROR_CHECKS;
@@ -2196,6 +2205,13 @@
initial_enable ? Extension::ENABLED : Extension::DISABLED,
from_webstore);
+ // Unpacked extensions default to allowing file access, but if that has been
+ // overridden, don't reset the value.
+ if (Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD) &&
+ !extension_prefs_->HasAllowFileAccessSetting(id)) {
+ extension_prefs_->SetAllowFileAccess(id, true);
+ }
+
NotificationService::current()->Notify(
chrome::NOTIFICATION_EXTENSION_INSTALLED,
Source<Profile>(profile_),
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index b002501..8d7ea1a5 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -1756,18 +1756,6 @@
ValidatePrefKeyCount(pref_count);
}
-// Tests that file access is OFF by default.
-TEST_F(ExtensionServiceTest, DefaultFileAccess) {
- InitializeEmptyExtensionService();
- PackAndInstallCrx(data_dir_.AppendASCII("permissions").AppendASCII("files"),
- true);
-
- EXPECT_EQ(0u, GetErrors().size());
- EXPECT_EQ(1u, service_->extensions()->size());
- std::string id = service_->extensions()->at(0)->id();
- EXPECT_FALSE(service_->extension_prefs()->AllowFileAccess(id));
-}
-
TEST_F(ExtensionServiceTest, UpdateApps) {
InitializeEmptyExtensionService();
FilePath extensions_path = data_dir_.AppendASCII("app_update");
