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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@95815 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_browsertest.cc

@@ -114,9 +114,9 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithOptions(
     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::LoadExtensionWithOptions(
 }
 
 const Extension* ExtensionBrowserTest::LoadExtension(const FilePath& path) {
-  return LoadExtensionWithOptions(path, false, true);
+  return LoadExtensionWithOptions(path, false, false);
 }
 
 const Extension* ExtensionBrowserTest::LoadExtensionIncognito(

chrome/browser/extensions/extension_prefs.cc

@@ -74,7 +74,11 @@ const char kPrefIncognitoEnabled[] = "incognito";
 
 // A preference to control whether an extension is allowed to inject script in
 // pages with file URLs.
-const char kPrefAllowFileAccess[] = "allowFileAccess";
+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";
 
 // A preference set by the web store to indicate login information for
 // purchased apps.
@@ -804,12 +808,6 @@ void ExtensionPrefs::SetAllowFileAccess(const std::string& extension_id,
                       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) {

chrome/browser/extensions/extension_prefs.h

@@ -207,7 +207,6 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
   // 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|.

chrome/browser/extensions/extension_service.cc

@@ -304,12 +304,7 @@ void ExtensionServiceBackend::CheckExtensionFileAccess(
     const FilePath& extension_path, bool prompt_for_plugins) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::string id = Extension::GenerateIdForPath(extension_path);
-  // 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);
+  bool allow_file_access = frontend_->extension_prefs()->AllowFileAccess(id);
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(
@@ -1034,13 +1029,9 @@ void ExtensionService::LoadExtensionFromCommandLine(
   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 (allow_file_access)
+  if (extension_prefs()->AllowFileAccess(id))
     flags |= Extension::ALLOW_FILE_ACCESS;
   if (Extension::ShouldDoStrictErrorChecking(Extension::LOAD))
     flags |= Extension::STRICT_ERROR_CHECKS;
@@ -2205,13 +2196,6 @@ void ExtensionService::OnExtensionInstalled(
       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_),

chrome/browser/extensions/extension_service_unittest.cc

@@ -1756,6 +1756,18 @@ TEST_F(ExtensionServiceTest, InstallApps) {
   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");

chrome/common/extensions/extension.h

@@ -287,12 +287,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
            location == Extension::COMPONENT;
   }
 
-  // Unpacked extensions start off with file access since they are a developer
-  // feature.
-  static inline bool ShouldAlwaysAllowFileAccess(Location location) {
-    return location == Extension::LOAD;
-  }
-
   // See Type definition above.
   Type GetType() const;
 

chrome/test/data/extensions/good/Preferences

@@ -10,7 +10,7 @@
               "host": ["file://*", "http://*.google.com/*", "https://*.google.com/*", "http://*.news.com/*"]
             },
             "state": 1,
-            "allowFileAccess": true,
+            "newAllowFileAccess": true,
             "manifest": {
               "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDuUZGKCDbff6IRaxa4Pue7PPkxwPaNhGT3JEqppEsNWFjM80imEdqMbf3lrWqEfaHgaNku7nlpwPO1mu3/4Hr+XdNa5MhfnOnuPee4hyTLwOs3Vzz81wpbdzUxZSi2OmqMyI5oTaBYICfNHLwcuc65N5dbt6WKGeKgTpp4v7j7zwIDAQAB",
               "version": "1.0.0.0",

chrome/test/data/extensions/permissions/files/manifest.json

+{
+  "name": "file access extension",
+  "version": "1",
+  "permissions": ["file:///*"]
+}