Commit 75b4a953 authored by Swapnil's avatar Swapnil Committed by Chromium LUCI CQ

[Extensions] Use policy-enforced update URL in c/b/extensions/

This change gives ExtensionManagement control over update URL used when
updating extensions. Previously only first install was affected, all
later updates used update URL from the extension manifest. Now update
URL from the policy can be used if extension management said so.
Update URL from policy is handled in
https://chromium-review.googlesource.com/c/chromium/src/+/2593260.
This is a follow up CL that allows usage of policy enforced update URL
in chrome/browser/extensions/.

Bug: b:175767492
Change-Id: Ib3904683fc3dcec0e4690e3c558a9346e859ae45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593101
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842496}
parent 531bc05f
...@@ -283,16 +283,20 @@ void ChromeContentVerifierDelegate::Shutdown() { ...@@ -283,16 +283,20 @@ void ChromeContentVerifierDelegate::Shutdown() {
policy_extension_reinstaller_.reset(); policy_extension_reinstaller_.reset();
} }
// static bool ChromeContentVerifierDelegate::IsFromWebstore(
bool ChromeContentVerifierDelegate::IsFromWebstore(const Extension& extension) { const Extension& extension) const {
// Use the InstallVerifier's |IsFromStore| method to avoid discrepancies // Use the InstallVerifier's |IsFromStore| method to avoid discrepancies
// between which extensions are considered in-store. // between which extensions are considered in-store.
// See https://crbug.com/766806 for details. // See https://crbug.com/766806 for details.
if (!InstallVerifier::IsFromStore(extension)) { if (!InstallVerifier::IsFromStore(extension, context_)) {
// It's possible that the webstore update url was overridden for testing // It's possible that the webstore update url was overridden for testing
// so also consider extensions with the default (production) update url // so also consider extensions with the default (production) update url
// to be from the store as well. // to be from the store as well. Therefore update URL is compared with
if (ManifestURL::GetUpdateURL(&extension) != // |GetDefaultWebstoreUpdateUrl|, not the |GetWebstoreUpdateUrl| used by
// |IsWebstoreUpdateUrl|.
ExtensionManagement* extension_management =
ExtensionManagementFactory::GetForBrowserContext(context_);
if (extension_management->GetEffectiveUpdateURL(extension) !=
extension_urls::GetDefaultWebstoreUpdateUrl()) { extension_urls::GetDefaultWebstoreUpdateUrl()) {
return false; return false;
} }
......
...@@ -85,7 +85,7 @@ class ChromeContentVerifierDelegate : public ContentVerifierDelegate { ...@@ -85,7 +85,7 @@ class ChromeContentVerifierDelegate : public ContentVerifierDelegate {
private: private:
// Returns true iff |extension| is considered extension from Chrome Web Store // Returns true iff |extension| is considered extension from Chrome Web Store
// (and therefore signed hashes may be used for its content verification). // (and therefore signed hashes may be used for its content verification).
static bool IsFromWebstore(const Extension& extension); bool IsFromWebstore(const Extension& extension) const;
// Returns information needed for content verification of |extension|. // Returns information needed for content verification of |extension|.
VerifyInfo GetVerifyInfo(const Extension& extension) const; VerifyInfo GetVerifyInfo(const Extension& extension) const;
......
...@@ -1712,7 +1712,7 @@ void ExtensionService::AddNewOrUpdatedExtension( ...@@ -1712,7 +1712,7 @@ void ExtensionService::AddNewOrUpdatedExtension(
install_flags, install_parameter, install_flags, install_parameter,
ruleset_install_prefs); ruleset_install_prefs);
delayed_installs_.Remove(extension->id()); delayed_installs_.Remove(extension->id());
if (InstallVerifier::NeedsVerification(*extension)) if (InstallVerifier::NeedsVerification(*extension, GetBrowserContext()))
InstallVerifier::Get(GetBrowserContext())->VerifyExtension(extension->id()); InstallVerifier::Get(GetBrowserContext())->VerifyExtension(extension->id());
FinishInstallation(extension); FinishInstallation(extension);
......
...@@ -181,14 +181,17 @@ bool InstallVerifier::ShouldEnforce() { ...@@ -181,14 +181,17 @@ bool InstallVerifier::ShouldEnforce() {
} }
// static // static
bool InstallVerifier::NeedsVerification(const Extension& extension) { bool InstallVerifier::NeedsVerification(const Extension& extension,
return IsFromStore(extension) && CanUseExtensionApis(extension); content::BrowserContext* context) {
return IsFromStore(extension, context) && CanUseExtensionApis(extension);
} }
// static // static
bool InstallVerifier::IsFromStore(const Extension& extension) { bool InstallVerifier::IsFromStore(const Extension& extension,
content::BrowserContext* context) {
return extension.from_webstore() || return extension.from_webstore() ||
ManifestURL::UpdatesFromGallery(&extension); ExtensionManagementFactory::GetForBrowserContext(context)
->UpdatesFromWebstore(extension);
} }
void InstallVerifier::Init() { void InstallVerifier::Init() {
...@@ -332,7 +335,7 @@ bool InstallVerifier::MustRemainDisabled(const Extension* extension, ...@@ -332,7 +335,7 @@ bool InstallVerifier::MustRemainDisabled(const Extension* extension,
if (base::Contains(InstallSigner::GetForcedNotFromWebstore(), if (base::Contains(InstallSigner::GetForcedNotFromWebstore(),
extension->id())) { extension->id())) {
verified = false; verified = false;
} else if (!IsFromStore(*extension)) { } else if (!IsFromStore(*extension, context_)) {
verified = false; verified = false;
} else if (!signature_ && (!bootstrap_check_complete_ || } else if (!signature_ && (!bootstrap_check_complete_ ||
GetStatus() < VerifyStatus::ENFORCE_STRICT)) { GetStatus() < VerifyStatus::ENFORCE_STRICT)) {
...@@ -386,7 +389,7 @@ ExtensionIdSet InstallVerifier::GetExtensionsToVerify() const { ...@@ -386,7 +389,7 @@ ExtensionIdSet InstallVerifier::GetExtensionsToVerify() const {
for (ExtensionSet::const_iterator iter = extensions->begin(); for (ExtensionSet::const_iterator iter = extensions->begin();
iter != extensions->end(); iter != extensions->end();
++iter) { ++iter) {
if (NeedsVerification(**iter)) if (NeedsVerification(**iter, context_))
result.insert((*iter)->id()); result.insert((*iter)->id());
} }
return result; return result;
......
...@@ -49,10 +49,12 @@ class InstallVerifier : public KeyedService, ...@@ -49,10 +49,12 @@ class InstallVerifier : public KeyedService,
static bool ShouldEnforce(); static bool ShouldEnforce();
// Returns whether |extension| is of a type that needs verification. // Returns whether |extension| is of a type that needs verification.
static bool NeedsVerification(const Extension& extension); static bool NeedsVerification(const Extension& extension,
content::BrowserContext* context);
// Determines if an extension claims to be from the webstore. // Determines if an extension claims to be from the webstore.
static bool IsFromStore(const Extension& extension); static bool IsFromStore(const Extension& extension,
content::BrowserContext* context);
// Initializes this object for use, including reading preferences and // Initializes this object for use, including reading preferences and
// validating the stored signature. // validating the stored signature.
......
...@@ -106,7 +106,7 @@ TEST_F(InstallVerifierTest, TestIsFromStoreAndMustRemainDisabled) { ...@@ -106,7 +106,7 @@ TEST_F(InstallVerifierTest, TestIsFromStoreAndMustRemainDisabled) {
AddExtensionAsPolicyInstalled(extension->id()); AddExtensionAsPolicyInstalled(extension->id());
EXPECT_EQ(test_case.expected_from_store_status == FROM_STORE, EXPECT_EQ(test_case.expected_from_store_status == FROM_STORE,
InstallVerifier::IsFromStore(*extension)); InstallVerifier::IsFromStore(*extension, profile()));
disable_reason::DisableReason disable_reason; disable_reason::DisableReason disable_reason;
base::string16 error; base::string16 error;
EXPECT_EQ( EXPECT_EQ(
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/extension_management.h"
#include "chrome/browser/extensions/install_verifier.h" #include "chrome/browser/extensions/install_verifier.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
...@@ -30,7 +31,6 @@ ...@@ -30,7 +31,6 @@
#include "extensions/common/manifest.h" #include "extensions/common/manifest.h"
#include "extensions/common/manifest_constants.h" #include "extensions/common/manifest_constants.h"
#include "extensions/common/manifest_handlers/background_info.h" #include "extensions/common/manifest_handlers/background_info.h"
#include "extensions/common/manifest_url_handlers.h"
#include "third_party/metrics_proto/system_profile.pb.h" #include "third_party/metrics_proto/system_profile.pb.h"
using extensions::Extension; using extensions::Extension;
...@@ -84,9 +84,9 @@ metrics::SystemProfileProto::ExtensionsState ExtensionStateAsProto( ...@@ -84,9 +84,9 @@ metrics::SystemProfileProto::ExtensionsState ExtensionStateAsProto(
// webstore, we attempt to verify with |verifier| by checking if it has been // webstore, we attempt to verify with |verifier| by checking if it has been
// explicitly deemed invalid. If |verifier| is inactive or if the extension is // explicitly deemed invalid. If |verifier| is inactive or if the extension is
// unknown to |verifier|, the local information is trusted. // unknown to |verifier|, the local information is trusted.
ExtensionState IsOffStoreExtension( ExtensionState IsOffStoreExtension(const extensions::Extension& extension,
const extensions::Extension& extension, const extensions::InstallVerifier& verifier,
const extensions::InstallVerifier& verifier) { content::BrowserContext* context) {
if (!extension.is_extension() && !extension.is_legacy_packaged_app()) if (!extension.is_extension() && !extension.is_legacy_packaged_app())
return NO_EXTENSIONS; return NO_EXTENSIONS;
...@@ -97,7 +97,7 @@ ExtensionState IsOffStoreExtension( ...@@ -97,7 +97,7 @@ ExtensionState IsOffStoreExtension(
if (verifier.AllowedByEnterprisePolicy(extension.id())) if (verifier.AllowedByEnterprisePolicy(extension.id()))
return NO_EXTENSIONS; return NO_EXTENSIONS;
if (!extensions::InstallVerifier::IsFromStore(extension)) if (!extensions::InstallVerifier::IsFromStore(extension, context))
return OFF_STORE; return OFF_STORE;
// Local information about the extension implies it is from the store. We try // Local information about the extension implies it is from the store. We try
...@@ -115,14 +115,15 @@ ExtensionState IsOffStoreExtension( ...@@ -115,14 +115,15 @@ ExtensionState IsOffStoreExtension(
// highest (as defined by the order of ExtensionState) value of each extension // highest (as defined by the order of ExtensionState) value of each extension
// in |extensions|. // in |extensions|.
ExtensionState CheckForOffStore(const extensions::ExtensionSet& extensions, ExtensionState CheckForOffStore(const extensions::ExtensionSet& extensions,
const extensions::InstallVerifier& verifier) { const extensions::InstallVerifier& verifier,
content::BrowserContext* context) {
ExtensionState state = NO_EXTENSIONS; ExtensionState state = NO_EXTENSIONS;
for (extensions::ExtensionSet::const_iterator it = extensions.begin(); for (extensions::ExtensionSet::const_iterator it = extensions.begin();
it != extensions.end() && state < OFF_STORE; it != extensions.end() && state < OFF_STORE;
++it) { ++it) {
// Combine the state of each extension, always favoring the higher state as // Combine the state of each extension, always favoring the higher state as
// defined by the order of ExtensionState. // defined by the order of ExtensionState.
state = std::max(state, IsOffStoreExtension(**it, verifier)); state = std::max(state, IsOffStoreExtension(**it, verifier, context));
} }
return state; return state;
} }
...@@ -304,7 +305,8 @@ ExtensionInstallProto::BlacklistState GetBlacklistState( ...@@ -304,7 +305,8 @@ ExtensionInstallProto::BlacklistState GetBlacklistState(
metrics::ExtensionInstallProto ConstructInstallProto( metrics::ExtensionInstallProto ConstructInstallProto(
const extensions::Extension& extension, const extensions::Extension& extension,
extensions::ExtensionPrefs* prefs, extensions::ExtensionPrefs* prefs,
base::Time last_sample_time) { base::Time last_sample_time,
extensions::ExtensionManagement* extension_management) {
ExtensionInstallProto install; ExtensionInstallProto install;
install.set_type(GetType(extension.manifest()->type())); install.set_type(GetType(extension.manifest()->type()));
install.set_install_location(GetInstallLocation(extension.location())); install.set_install_location(GetInstallLocation(extension.location()));
...@@ -315,7 +317,7 @@ metrics::ExtensionInstallProto ConstructInstallProto( ...@@ -315,7 +317,7 @@ metrics::ExtensionInstallProto ConstructInstallProto(
install.set_has_incognito_access(prefs->IsIncognitoEnabled(extension.id())); install.set_has_incognito_access(prefs->IsIncognitoEnabled(extension.id()));
install.set_is_from_store(extension.from_webstore()); install.set_is_from_store(extension.from_webstore());
install.set_updates_from_store( install.set_updates_from_store(
extensions::ManifestURL::UpdatesFromGallery(&extension)); extension_management->UpdatesFromWebstore(extension));
install.set_is_from_bookmark(extension.from_bookmark()); install.set_is_from_bookmark(extension.from_bookmark());
install.set_is_converted_from_user_script( install.set_is_converted_from_user_script(
extension.converted_from_user_script()); extension.converted_from_user_script());
...@@ -343,9 +345,11 @@ std::vector<metrics::ExtensionInstallProto> GetInstallsForProfile( ...@@ -343,9 +345,11 @@ std::vector<metrics::ExtensionInstallProto> GetInstallsForProfile(
->GenerateInstalledExtensionsSet(); ->GenerateInstalledExtensionsSet();
std::vector<ExtensionInstallProto> installs; std::vector<ExtensionInstallProto> installs;
installs.reserve(extensions->size()); installs.reserve(extensions->size());
extensions::ExtensionManagement* extension_management =
extensions::ExtensionManagementFactory::GetForBrowserContext(profile);
for (const auto& extension : *extensions) { for (const auto& extension : *extensions) {
installs.push_back( installs.push_back(ConstructInstallProto(
ConstructInstallProto(*extension, prefs, last_sample_time)); *extension, prefs, last_sample_time, extension_management));
} }
return installs; return installs;
...@@ -401,8 +405,12 @@ metrics::ExtensionInstallProto ...@@ -401,8 +405,12 @@ metrics::ExtensionInstallProto
ExtensionsMetricsProvider::ConstructInstallProtoForTesting( ExtensionsMetricsProvider::ConstructInstallProtoForTesting(
const extensions::Extension& extension, const extensions::Extension& extension,
extensions::ExtensionPrefs* prefs, extensions::ExtensionPrefs* prefs,
base::Time last_sample_time) { base::Time last_sample_time,
return ConstructInstallProto(extension, prefs, last_sample_time); Profile* profile) {
extensions::ExtensionManagement* extension_management =
extensions::ExtensionManagementFactory::GetForBrowserContext(profile);
return ConstructInstallProto(extension, prefs, last_sample_time,
extension_management);
} }
// static // static
...@@ -435,7 +443,8 @@ void ExtensionsMetricsProvider::ProvideOffStoreMetric( ...@@ -435,7 +443,8 @@ void ExtensionsMetricsProvider::ProvideOffStoreMetric(
// Combine the state from each profile, always favoring the higher state as // Combine the state from each profile, always favoring the higher state as
// defined by the order of ExtensionState. // defined by the order of ExtensionState.
state = std::max(state, CheckForOffStore(*extensions.get(), *verifier)); state = std::max(
state, CheckForOffStore(*extensions.get(), *verifier, profiles[i]));
} }
system_profile->set_offstore_extensions_state(ExtensionStateAsProto(state)); system_profile->set_offstore_extensions_state(ExtensionStateAsProto(state));
......
...@@ -50,7 +50,8 @@ class ExtensionsMetricsProvider : public metrics::MetricsProvider { ...@@ -50,7 +50,8 @@ class ExtensionsMetricsProvider : public metrics::MetricsProvider {
static metrics::ExtensionInstallProto ConstructInstallProtoForTesting( static metrics::ExtensionInstallProto ConstructInstallProtoForTesting(
const extensions::Extension& extension, const extensions::Extension& extension,
extensions::ExtensionPrefs* prefs, extensions::ExtensionPrefs* prefs,
base::Time last_sample_time); base::Time last_sample_time,
Profile* profile);
static std::vector<metrics::ExtensionInstallProto> static std::vector<metrics::ExtensionInstallProto>
GetInstallsForProfileForTesting(Profile* profile, GetInstallsForProfileForTesting(Profile* profile,
base::Time last_sample_time); base::Time last_sample_time);
......
...@@ -158,7 +158,7 @@ class ExtensionMetricsProviderInstallsTest ...@@ -158,7 +158,7 @@ class ExtensionMetricsProviderInstallsTest
ExtensionInstallProto ConstructProto(const Extension& extension) { ExtensionInstallProto ConstructProto(const Extension& extension) {
return ExtensionsMetricsProvider::ConstructInstallProtoForTesting( return ExtensionsMetricsProvider::ConstructInstallProtoForTesting(
extension, prefs_, last_sample_time_); extension, prefs_, last_sample_time_, profile());
} }
std::vector<ExtensionInstallProto> GetInstallsForProfile() { std::vector<ExtensionInstallProto> GetInstallsForProfile() {
return ExtensionsMetricsProvider::GetInstallsForProfileForTesting( return ExtensionsMetricsProvider::GetInstallsForProfileForTesting(
......
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