Treat extension download from untrusted sources as regular download

This CL treats extension downloads from untrusted sources as regular
downloads. That means these downloads will be prompted, show up in
downloads page and download shelf as normal downloads.

BUG=756825

Change-Id: I515bba2fa0a64198a56a06ce38864ef59ad18c9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761374Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689091}

chrome/browser/download/download_crx_util.cc

@@ -64,6 +64,17 @@ std::unique_ptr<ExtensionInstallPrompt> CreateExtensionInstallPrompt(
   }
 }
 
+bool OffStoreInstallAllowedByPrefs(Profile* profile, const DownloadItem& item) {
+  // TODO(aa): RefererURL is cleared in some cases, for example when going
+  // between secure and non-secure URLs. It would be better if DownloadItem
+  // tracked the initiating page explicitly.
+  LOG(ERROR) << "OffStoreInstallAllowedByPrefs************"
+             << g_allow_offstore_install_for_testing;
+  return g_allow_offstore_install_for_testing ||
+         extensions::ExtensionManagementFactory::GetForBrowserContext(profile)
+             ->IsOffstoreInstallAllowed(item.GetURL(), item.GetReferrerUrl());
+}
+
 }  // namespace
 
 // Tests can call this method to inject a mock ExtensionInstallPrompt
@@ -134,13 +145,9 @@ bool IsExtensionDownload(const DownloadItem& download_item) {
   }
 }
 
-bool OffStoreInstallAllowedByPrefs(Profile* profile, const DownloadItem& item) {
-  // TODO(aa): RefererURL is cleared in some cases, for example when going
-  // between secure and non-secure URLs. It would be better if DownloadItem
-  // tracked the initiating page explicitly.
-  return g_allow_offstore_install_for_testing ||
-         extensions::ExtensionManagementFactory::GetForBrowserContext(profile)
-             ->IsOffstoreInstallAllowed(item.GetURL(), item.GetReferrerUrl());
+bool IsTrustedExtensionDownload(Profile* profile, const DownloadItem& item) {
+  return IsExtensionDownload(item) &&
+         OffStoreInstallAllowedByPrefs(profile, item);
 }
 
 std::unique_ptr<base::AutoReset<bool>> OverrideOffstoreInstallAllowedForTesting(

chrome/browser/download/download_crx_util.h

@@ -47,10 +47,9 @@ scoped_refptr<extensions::CrxInstaller> OpenChromeExtension(
 // scripts to be extension downloads, since we convert those automatically.
 bool IsExtensionDownload(const download::DownloadItem& download_item);
 
-// Checks whether an extension download should be allowed to proceed because the
-// installation site is whitelisted in prefs.
-bool OffStoreInstallAllowedByPrefs(Profile* profile,
-                                   const download::DownloadItem& item);
+// Checks whether a download is an extension from a whitelisted site in prefs.
+bool IsTrustedExtensionDownload(Profile* profile,
+                                const download::DownloadItem& item);
 
 // Allows tests to override whether offstore extension installs are allowed
 // for testing purposes.

chrome/browser/download/download_crx_util_android.cc

@@ -14,8 +14,8 @@ bool IsExtensionDownload(const download::DownloadItem& download_item) {
   return false;
 }
 
-bool OffStoreInstallAllowedByPrefs(Profile* profile,
-                                   const download::DownloadItem& item) {
+bool IsTrustedExtensionDownload(Profile* profile,
+                                const download::DownloadItem& item) {
   // Extensions are not supported on Android, return the safe default.
   return false;
 }

chrome/browser/download/download_history.cc

@@ -39,6 +39,7 @@
 #include "base/task/post_task.h"
 #include "build/build_config.h"
 #include "chrome/browser/download/download_crx_util.h"
+#include "chrome/browser/profiles/profile.h"
 #include "components/download/public/common/download_features.h"
 #include "components/download/public/common/download_item.h"
 #include "components/download/public/common/download_utils.h"
@@ -361,11 +362,15 @@ void DownloadHistory::MaybeAddToHistory(download::DownloadItem* item) {
   bool removing = removing_ids_.find(download_id) != removing_ids_.end();
 
   // TODO(benjhayden): Remove IsTemporary().
-  if (download_crx_util::IsExtensionDownload(*item) ||
+  if ((notifier_.GetManager() &&
+       download_crx_util::IsTrustedExtensionDownload(
+           Profile::FromBrowserContext(
+               notifier_.GetManager()->GetBrowserContext()),
+           *item)) ||
       item->IsTemporary() ||
-      (data->state() != DownloadHistoryData::NOT_PERSISTED) ||
-      removing)
+      (data->state() != DownloadHistoryData::NOT_PERSISTED) || removing) {
     return;
+  }
 
   data->SetState(DownloadHistoryData::PERSISTING);
   // Keep the info for in-progress download, so we can check whether history DB

chrome/browser/download/download_item_model.cc

@@ -292,13 +292,13 @@ bool DownloadItemModel::ShouldRemoveFromShelfWhenComplete() const {
       if (IsDangerous())
         return false;
 
-      // If the download is an extension, temporary, or will be opened
+      // If the download is a trusted extension, temporary, or will be opened
       // automatically, then it should be removed from the shelf on completion.
       // TODO(asanka): The logic for deciding opening behavior should be in a
       //               central location. http://crbug.com/167702
-      return (download_crx_util::IsExtensionDownload(*download_) ||
-              download_->IsTemporary() ||
-              download_->GetOpenWhenComplete() ||
+      return (download_crx_util::IsTrustedExtensionDownload(profile(),
+                                                            *download_) ||
+              download_->IsTemporary() || download_->GetOpenWhenComplete() ||
               download_->ShouldOpenFileBasedOnExtension());
 
     case DownloadItem::COMPLETE:
@@ -322,7 +322,7 @@ bool DownloadItemModel::ShouldRemoveFromShelfWhenComplete() const {
 
 bool DownloadItemModel::ShouldShowDownloadStartedAnimation() const {
   return !download_->IsSavePackageDownload() &&
-      !download_crx_util::IsExtensionDownload(*download_);
+         !download_crx_util::IsTrustedExtensionDownload(profile(), *download_);
 }
 
 bool DownloadItemModel::ShouldShowInShelf() const {

chrome/browser/download/download_target_determiner.cc

@@ -1015,9 +1015,9 @@ DownloadConfirmationReason DownloadTargetDeterminer::NeedsConfirmation(
     return DownloadConfirmationReason::SAVE_AS;
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-  // Don't prompt for extension downloads.
-  if (download_crx_util::IsExtensionDownload(*download_) ||
-      filename.MatchesExtension(extensions::kExtensionFileExtension))
+  // Don't prompt for extension downloads if the installation site is white
+  // listed.
+  if (download_crx_util::IsTrustedExtensionDownload(GetProfile(), *download_))
     return DownloadConfirmationReason::NONE;
 #endif
 
@@ -1051,15 +1051,10 @@ DownloadFileType::DangerLevel DownloadTargetDeterminer::GetDangerLevel(
       !download_->GetForcedFilePath().empty())
     return DownloadFileType::NOT_DANGEROUS;
 
-  const bool is_extension_download =
-      download_crx_util::IsExtensionDownload(*download_);
-
   // User-initiated extension downloads from pref-whitelisted sources are not
   // considered dangerous.
   if (download_->HasUserGesture() &&
-      is_extension_download &&
-      download_crx_util::OffStoreInstallAllowedByPrefs(
-          GetProfile(), *download_)) {
+      download_crx_util::IsTrustedExtensionDownload(GetProfile(), *download_)) {
     return DownloadFileType::NOT_DANGEROUS;
   }
 

chrome/browser/download/download_target_determiner_unittest.cc

@@ -23,6 +23,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_confirmation_result.h"
+#include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_prompt_status.h"
 #include "chrome/browser/download/download_stats.h"
@@ -1431,9 +1432,40 @@ TEST_F(DownloadTargetDeterminerTest, ContinueWithConfirmation_SaveAs) {
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 // These test cases are run with "Prompt for download" user preference set to
-// true. Automatic extension downloads shouldn't cause prompting.
+// true. For non-trusted extensions, download should cause prompting.
 // Android doesn't support extensions.
-TEST_F(DownloadTargetDeterminerTest, PromptAlways_Extension) {
+TEST_F(DownloadTargetDeterminerTest, PromptAlways_NonTrustedExtension) {
+  const DownloadTestCase kPromptingTestCases[] = {
+      {// 0: Automatic Browser Extension download. - Shouldn't prompt for
+       //    browser extension downloads even if "Prompt for download"
+       //    preference is set.
+       AUTOMATIC, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS, "http://example.com/foo.kindabad",
+       extensions::Extension::kMimeType, FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("foo.crx"), DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+       EXPECT_CRDOWNLOAD},
+
+      {// 1: Automatic User Script - Shouldn't prompt for user script downloads
+       //    even if "Prompt for download" preference is set.
+       AUTOMATIC, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+       DownloadFileType::NOT_DANGEROUS, "http://example.com/foo.user.js", "",
+       FILE_PATH_LITERAL(""),
+
+       FILE_PATH_LITERAL("foo.user.js"),
+       DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+       EXPECT_CRDOWNLOAD},
+  };
+
+  SetPromptForDownload(true);
+  RunTestCasesWithActiveItem(kPromptingTestCases,
+                             base::size(kPromptingTestCases));
+}
+
+// Trusted extension download should not cause prompting.
+TEST_F(DownloadTargetDeterminerTest, PromptAlways_TrustedExtension) {
   const DownloadTestCase kPromptingTestCases[] = {
       {// 0: Automatic Browser Extension download. - Shouldn't prompt for
        //    browser extension downloads even if "Prompt for download"
@@ -1458,6 +1490,8 @@ TEST_F(DownloadTargetDeterminerTest, PromptAlways_Extension) {
        EXPECT_CRDOWNLOAD},
   };
 
+  auto allow_offstore_install =
+      download_crx_util::OverrideOffstoreInstallAllowedForTesting(true);
   SetPromptForDownload(true);
   RunTestCasesWithActiveItem(kPromptingTestCases,
                              base::size(kPromptingTestCases));

chrome/browser/ui/webui/downloads/downloads_list_tracker.cc

@@ -335,8 +335,12 @@ void DownloadsListTracker::SetChunkSizeForTesting(size_t chunk_size) {
 }
 
 bool DownloadsListTracker::ShouldShow(const DownloadItem& item) const {
-  return !download_crx_util::IsExtensionDownload(item) && !item.IsTemporary() &&
-         !item.IsTransient() && !item.GetFileNameToReportUser().empty() &&
+  return !download_crx_util::IsTrustedExtensionDownload(
+             Profile::FromBrowserContext(
+                 GetMainNotifierManager()->GetBrowserContext()),
+             item) &&
+         !item.IsTemporary() && !item.IsTransient() &&
+         !item.GetFileNameToReportUser().empty() &&
          !item.GetTargetFilePath().empty() && !item.GetURL().is_empty() &&
          DownloadItemModel(const_cast<DownloadItem*>(&item))
              .ShouldShowInShelf() &&