Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(196)

Unified Diff: chrome/browser/extensions/content_verifier_browsertest.cc

Issue 2299203004: Attempt to repair corrupt enterprise policy force-installed extensions (Closed)
Patch Set: switched to using installsource, addressed review comments Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/content_verifier_browsertest.cc
diff --git a/chrome/browser/extensions/content_verifier_browsertest.cc b/chrome/browser/extensions/content_verifier_browsertest.cc
index f688dd4712708f4c9b1023b3580a2c2482d06acc..9b9dce766e90c29a5ad8565e7feb97a5a561404e 100644
--- a/chrome/browser/extensions/content_verifier_browsertest.cc
+++ b/chrome/browser/extensions/content_verifier_browsertest.cc
@@ -6,52 +6,104 @@
#include <set>
#include <string>
+#include "base/callback_helpers.h"
#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
#include "base/scoped_observer.h"
+#include "base/strings/string_split.h"
+#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_service.h"
#include "chrome/common/chrome_switches.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/content_verifier.h"
#include "extensions/browser/content_verify_job.h"
+#include "extensions/browser/crx_file_info.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_registry_observer.h"
+#include "extensions/browser/external_install_info.h"
+#include "extensions/browser/external_provider_interface.h"
+#include "extensions/browser/management_policy.h"
+#include "extensions/browser/updater/extension_downloader.h"
+#include "extensions/browser/updater/extension_downloader_test_delegate.h"
+#include "extensions/browser/updater/manifest_fetch_data.h"
+#include "extensions/common/extension_urls.h"
namespace extensions {
namespace {
-// Helper for observing extension unloads.
-class UnloadObserver : public ExtensionRegistryObserver {
+// Helper for observing extension registry events.
+class RegistryObserver : public ExtensionRegistryObserver {
public:
- explicit UnloadObserver(ExtensionRegistry* registry) : observer_(this) {
+ explicit RegistryObserver(ExtensionRegistry* registry) : observer_(this) {
observer_.Add(registry);
}
- ~UnloadObserver() override {}
+ ~RegistryObserver() override {}
+
+ // Waits until we've seen an unload for extension with |id|, returning true
+ // if we saw one or false otherwise (typically because of test timeout).
+ bool WaitForUnload(const ExtensionId& id) {
+ if (base::ContainsKey(unloaded_, id))
+ return true;
+
+ base::RunLoop run_loop;
+ awaited_unload_id_ = id;
+ quit_closure_ = run_loop.QuitClosure();
+ run_loop.Run();
+ return base::ContainsKey(unloaded_, id);
+ }
- void WaitForUnload(const ExtensionId& id) {
- if (base::ContainsKey(observed_, id))
- return;
+ // Same as WaitForUnload, but for an install.
+ bool WaitForInstall(const ExtensionId& id) {
+ if (base::ContainsKey(installed_, id))
+ return true;
- ASSERT_TRUE(loop_runner_.get() == NULL);
- awaited_id_ = id;
- loop_runner_ = new content::MessageLoopRunner();
- loop_runner_->Run();
+ base::RunLoop run_loop;
+ awaited_install_id_ = id;
+ quit_closure_ = run_loop.QuitClosure();
+ run_loop.Run();
+ return base::ContainsKey(installed_, id);
}
+ // ExtensionRegistryObserver
void OnExtensionUnloaded(content::BrowserContext* browser_context,
const Extension* extension,
UnloadedExtensionInfo::Reason reason) override {
- observed_.insert(extension->id());
- if (awaited_id_ == extension->id())
- loop_runner_->Quit();
+ unloaded_.insert(extension->id());
+ if (awaited_unload_id_ == extension->id()) {
+ awaited_unload_id_.clear();
+ base::ResetAndReturn(&quit_closure_).Run();
+ }
+ }
+
+ void OnExtensionInstalled(content::BrowserContext* browser_context,
+ const Extension* extension,
+ bool is_update) override {
+ installed_.insert(extension->id());
+ if (awaited_install_id_ == extension->id()) {
+ awaited_install_id_.clear();
+ base::ResetAndReturn(&quit_closure_).Run();
+ }
}
private:
- ExtensionId awaited_id_;
- std::set<ExtensionId> observed_;
- scoped_refptr<content::MessageLoopRunner> loop_runner_;
- ScopedObserver<ExtensionRegistry, UnloadObserver> observer_;
+ // The id we're waiting for a load/install of respectively.
+ ExtensionId awaited_unload_id_;
+ ExtensionId awaited_install_id_;
+
+ // The quit closure for stopping a running RunLoop, if we're waiting.
+ base::Closure quit_closure_;
+
+ // The extension id's we've seen unloaded and installed, respectively.
+ std::set<ExtensionId> unloaded_;
+ std::set<ExtensionId> installed_;
+
+ ScopedObserver<ExtensionRegistry, RegistryObserver> observer_;
+
+ DISALLOW_COPY_AND_ASSIGN(RegistryObserver);
};
// Helper for forcing ContentVerifyJob's to return an error.
@@ -250,6 +302,133 @@ void VerifierObserver::OnFetchComplete(const std::string& extension_id,
loop_runner_->Quit();
}
+// This lets us intercept requests for update checks of extensions, and
+// substitute a local file as a simulated response.
+class DownloaderTestDelegate : public ExtensionDownloaderTestDelegate {
+ public:
+ DownloaderTestDelegate() {}
+
+ // This makes it so that update check requests for |extension_id| will return
+ // a downloaded file of |crx_path| that is claimed to have version
+ // |version_string|.
+ void AddResponse(const ExtensionId& extension_id,
+ const std::string& version_string,
+ const base::FilePath& crx_path) {
+ responses_[extension_id] = std::make_pair(version_string, crx_path);
+ }
+
+ const std::vector<std::unique_ptr<ManifestFetchData>>& requests() {
+ return requests_;
+ }
+
+ // ExtensionDownloaderTestDelegate:
+ void StartUpdateCheck(
+ ExtensionDownloader* downloader,
+ ExtensionDownloaderDelegate* delegate,
+ std::unique_ptr<ManifestFetchData> fetch_data) override {
+ requests_.push_back(std::move(fetch_data));
+ const ManifestFetchData* data = requests_.back().get();
+
+ for (const auto& id : data->extension_ids()) {
+ if (ContainsKey(responses_, id)) {
+ // We use PostTask here instead of calling OnExtensionDownloadFinished
+ // immeditately, because the calling code isn't expecting a synchronous
+ // response (in non-test situations there are at least 2 network
+ // requests needed before a file could be returned).
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::Bind(
+ &ExtensionDownloaderDelegate::OnExtensionDownloadFinished,
+ base::Unretained(delegate),
+ CRXFileInfo(id, responses_[id].second),
+ false /* pass_file_ownership */, GURL(), responses_[id].first,
+ ExtensionDownloaderDelegate::PingResult(), data->request_ids(),
+ ExtensionDownloaderDelegate::InstallCallback()));
+ }
+ }
+ }
+
+ private:
+ // The requests we've received.
+ std::vector<std::unique_ptr<ManifestFetchData>> requests_;
+
+ // The prepared responses - this maps an extension id to a (version string,
+ // crx file path) pair.
+ std::map<std::string, std::pair<ExtensionId, base::FilePath>> responses_;
+
+ DISALLOW_COPY_AND_ASSIGN(DownloaderTestDelegate);
+};
+
+// This lets us simulate the behavior of an enterprise policy that wants
+// a given extension to be installed via the webstore.
+class TestExternalProvider : public ExternalProviderInterface {
+ public:
+ TestExternalProvider(VisitorInterface* visitor,
+ const ExtensionId& extension_id)
+ : visitor_(visitor), extension_id_(extension_id) {}
+
+ ~TestExternalProvider() override {}
+
+ // ExternalProviderInterface:
+ void ServiceShutdown() override {}
+
+ void VisitRegisteredExtension() override {
+ visitor_->OnExternalExtensionUpdateUrlFound(
+ ExternalInstallInfoUpdateUrl(
+ extension_id_, std::string() /* install_parameter */,
+ base::MakeUnique<GURL>(extension_urls::GetWebstoreUpdateUrl()),
+ Manifest::EXTERNAL_POLICY_DOWNLOAD, 0 /* creation_flags */,
+ true /* mark_acknowledged */),
+ true /* is_initial_load */);
+ visitor_->OnExternalProviderReady(this);
+ }
+
+ bool HasExtension(const ExtensionId& id) const override {
+ return id == std::string("npnbmohejbjohgpjnmjagbafnjhkmgko");
+ }
+
+ bool GetExtensionDetails(
+ const ExtensionId& id,
+ Manifest::Location* location,
+ std::unique_ptr<base::Version>* version) const override {
+ ADD_FAILURE() << "Unexpected GetExtensionDetails call; id:" << id;
+ return false;
+ }
+
+ bool IsReady() const override { return true; }
+
+ private:
+ VisitorInterface* visitor_;
+ ExtensionId extension_id_;
+ base::Closure quit_closure_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestExternalProvider);
+};
+
+// This lets us simulate a policy-installed extension being "force" installed;
+// ie a user is not allowed to manually uninstall/disable it.
+class ForceInstallProvider : public ManagementPolicy::Provider {
+ public:
+ explicit ForceInstallProvider(const ExtensionId& id) : id_(id) {}
+ ~ForceInstallProvider() override {}
+
+ std::string GetDebugPolicyProviderName() const override {
+ return "ForceInstallProvider";
+ }
+
+ // MananagementPolicy::Provider:
+ bool UserMayModifySettings(const Extension* extension,
+ base::string16* error) const override {
+ return extension->id() != id_;
+ }
+
+ private:
+ // The extension id we want to disallow uninstall/disable for.
+ ExtensionId id_;
+
+ DISALLOW_COPY_AND_ASSIGN(ForceInstallProvider);
+};
+
} // namespace
class ContentVerifierTest : public ExtensionBrowserTest {
@@ -280,7 +459,7 @@ class ContentVerifierTest : public ExtensionBrowserTest {
std::string id = "npnbmohejbjohgpjnmjagbafnjhkmgko";
delegate_.set_id(id);
unload_observer_.reset(
- new UnloadObserver(ExtensionRegistry::Get(profile())));
+ new RegistryObserver(ExtensionRegistry::Get(profile())));
const Extension* extension = InstallExtensionFromWebstore(
test_data_dir_.AppendASCII("content_verifier/v1.crx"), 1);
ASSERT_TRUE(extension);
@@ -294,7 +473,7 @@ class ContentVerifierTest : public ExtensionBrowserTest {
AddTabAtIndexToBrowser(browser(), 1, page_url_, ui::PAGE_TRANSITION_LINK,
false);
- unload_observer_->WaitForUnload(id);
+ EXPECT_TRUE(unload_observer_->WaitForUnload(id));
ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
int reasons = prefs->GetDisableReasons(id);
EXPECT_TRUE(reasons & Extension::DISABLE_CORRUPTED);
@@ -306,7 +485,7 @@ class ContentVerifierTest : public ExtensionBrowserTest {
protected:
JobDelegate delegate_;
- std::unique_ptr<UnloadObserver> unload_observer_;
+ std::unique_ptr<RegistryObserver> unload_observer_;
GURL page_url_;
};
@@ -423,4 +602,64 @@ IN_PROC_BROWSER_TEST_F(ContentVerifierTest, ContentScripts) {
ContentVerifyJob::SetObserverForTests(NULL);
}
+// Tests the case of a corrupt extension that is force-installed by policy and
+// should not be allowed to be manually uninstalled/disabled by the user.
+IN_PROC_BROWSER_TEST_F(ContentVerifierTest, PolicyCorrupted) {
+ ExtensionSystem* system = ExtensionSystem::Get(profile());
+ ExtensionService* service = system->extension_service();
+
+ // The id of our test extension.
+ std::string id("npnbmohejbjohgpjnmjagbafnjhkmgko");
+
+ // Setup fake policy and update check objects.
+ ForceInstallProvider policy(id);
+ DownloaderTestDelegate downloader;
+ system->management_policy()->RegisterProvider(&policy);
+ ExtensionDownloader::set_test_delegate(&downloader);
+ service->AddProviderForTesting(
+ base::MakeUnique<TestExternalProvider>(service, id));
+
+ base::FilePath crx_path =
+ test_data_dir_.AppendASCII("content_verifier/v1.crx");
+ const Extension* extension =
+ InstallExtension(crx_path, 1, Manifest::EXTERNAL_POLICY_DOWNLOAD);
+ EXPECT_NE(extension, nullptr);
+
+ downloader.AddResponse(id, extension->VersionString(), crx_path);
+
+ RegistryObserver registry_observer(ExtensionRegistry::Get(profile()));
+ ContentVerifier* verifier = system->content_verifier();
+ verifier->VerifyFailed(extension->id(), ContentVerifyJob::HASH_MISMATCH);
+
+ // Make sure the extension first got disabled due to corruption.
+ EXPECT_TRUE(registry_observer.WaitForUnload(id));
+ ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+ int reasons = prefs->GetDisableReasons(id);
+ EXPECT_TRUE(reasons & Extension::DISABLE_CORRUPTED);
+
+ // Make sure the extension then got re-installed, and that after reinstall it
+ // is no longer disabled due to corruption.
+ EXPECT_TRUE(registry_observer.WaitForInstall(id));
+ reasons = prefs->GetDisableReasons(id);
+ EXPECT_FALSE(reasons & Extension::DISABLE_CORRUPTED);
+
+ // Make sure that the update check request properly included a parameter
+ // indicating that this was a corrupt policy reinstall.
+ bool found = false;
+ for (const auto& request : downloader.requests()) {
+ if (request->Includes(id)) {
+ std::string query = request->full_url().query();
+ for (const auto& part : base::SplitString(
+ query, "&", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL)) {
+ if (base::StartsWith(part, "x=", base::CompareCase::SENSITIVE) &&
+ part.find(std::string("id%3D") + id) != std::string::npos) {
+ found = true;
+ EXPECT_NE(std::string::npos, part.find("installsource%3Dreinstall"));
+ }
+ }
+ }
+ }
+ EXPECT_TRUE(found);
+}
+
} // namespace extensions
« no previous file with comments | « chrome/browser/extensions/chrome_content_verifier_delegate.cc ('k') | chrome/browser/extensions/extension_service.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698