Supervised Users should always need custodian approval for permission increase.
Remove the field trial SupervisedUserExtensionPermissionIncrease
BUG=652235
Review-Url: https://codereview.chromium.org/2396903002
Cr-Commit-Position: refs/heads/master@{#423497}
diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
index 36188eb..e037c2e 100644
--- a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
+++ b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
@@ -619,8 +619,7 @@
ExtensionPrefs* extensions_prefs = ExtensionPrefs::Get(browser_context());
- if (extensions::util::NeedCustodianApprovalForPermissionIncrease(profile) &&
- extensions_prefs->HasDisableReason(
+ if (extensions_prefs->HasDisableReason(
params->id, Extension::DISABLE_PERMISSIONS_INCREASE)) {
return RespondNow(BuildResponse(true));
}
diff --git a/chrome/browser/extensions/extension_disabled_ui.cc b/chrome/browser/extensions/extension_disabled_ui.cc
index 885fc5d..f23350b 100644
--- a/chrome/browser/extensions/extension_disabled_ui.cc
+++ b/chrome/browser/extensions/extension_disabled_ui.cc
@@ -319,9 +319,7 @@
base::string16 ExtensionDisabledGlobalError::GetBubbleViewAcceptButtonLabel() {
if (extensions::util::IsExtensionSupervised(extension_,
- service_->profile()) &&
- extensions::util::NeedCustodianApprovalForPermissionIncrease(
- service_->profile())) {
+ service_->profile())) {
// TODO(treib): Probably use a new string here once we get UX design.
// For now, just use "OK". crbug.com/461261
return l10n_util::GetStringUTF16(IDS_OK);
@@ -338,16 +336,10 @@
base::string16 ExtensionDisabledGlobalError::GetBubbleViewCancelButtonLabel() {
if (extensions::util::IsExtensionSupervised(extension_,
service_->profile())) {
- if (extensions::util::NeedCustodianApprovalForPermissionIncrease(
- service_->profile())) {
- // If the supervised user can't approve the update, then there is no
- // "cancel" button.
- return base::string16();
- } else {
- // Supervised users can not remove extensions, so use "cancel" here
- // instead of "uninstall".
- return l10n_util::GetStringUTF16(IDS_CANCEL);
- }
+ // The supervised user can't approve the update, and hence there is no
+ // "cancel" button. Return an empty string such that the "cancel" button
+ // is not shown in the dialog.
+ return base::string16();
}
return l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL);
}
@@ -358,9 +350,7 @@
void ExtensionDisabledGlobalError::BubbleViewAcceptButtonPressed(
Browser* browser) {
if (extensions::util::IsExtensionSupervised(extension_,
- service_->profile()) &&
- extensions::util::NeedCustodianApprovalForPermissionIncrease(
- service_->profile())) {
+ service_->profile())) {
return;
}
// Delay extension reenabling so this bubble closes properly.
@@ -372,16 +362,11 @@
void ExtensionDisabledGlobalError::BubbleViewCancelButtonPressed(
Browser* browser) {
- if (extensions::util::IsExtensionSupervised(extension_,
- service_->profile())) {
- // For custodian-installed extensions, this button should only exist if the
- // supervised user can approve the update. Otherwise there is only an "OK"
- // button.
- DCHECK(!extensions::util::NeedCustodianApprovalForPermissionIncrease(
- service_->profile()));
- // Supervised users may never remove custodian-installed extensions.
- return;
- }
+ // For custodian-installed extensions, this button should not exist because
+ // there is only an "OK" button.
+ // Supervised users may never remove custodian-installed extensions.
+ DCHECK(!extensions::util::IsExtensionSupervised(extension_,
+ service_->profile()));
uninstall_dialog_.reset(extensions::ExtensionUninstallDialog::Create(
service_->profile(), browser->window()->GetNativeWindow(), this));
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index a0a25a2..72b61fc 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1686,11 +1686,8 @@
#if defined(ENABLE_SUPERVISED_USERS)
// If a custodian-installed extension is disabled for a supervised user due
- // to a permissions increase, send a request to the custodian if the
- // supervised user themselves can't re-enable the extension.
+ // to a permissions increase, send a request to the custodian.
if (extensions::util::IsExtensionSupervised(extension, profile_) &&
- extensions::util::NeedCustodianApprovalForPermissionIncrease(
- profile_) &&
!ExtensionSyncService::Get(profile_)->HasPendingReenable(
extension->id(), *extension->version())) {
SupervisedUserService* supervised_user_service =
diff --git a/chrome/browser/extensions/extension_service_sync_unittest.cc b/chrome/browser/extensions/extension_service_sync_unittest.cc
index c22300b..1f4874c 100644
--- a/chrome/browser/extensions/extension_service_sync_unittest.cc
+++ b/chrome/browser/extensions/extension_service_sync_unittest.cc
@@ -1581,17 +1581,6 @@
}
protected:
- void InitNeedCustodianApprovalFieldTrial(bool enabled) {
- // Group name doesn't matter.
- base::FieldTrialList::CreateFieldTrial(
- "SupervisedUserExtensionPermissionIncrease", "group");
- std::map<std::string, std::string> params;
- params["legacy_supervised_user"] = enabled ? "true" : "false";
- params["child_account"] = enabled ? "true" : "false";
- variations::AssociateVariationParams(
- "SupervisedUserExtensionPermissionIncrease", "group", params);
- }
-
void InitSupervisedUserInitiatedExtensionInstallFeature(bool enabled) {
base::FeatureList::ClearInstanceForTesting();
std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
@@ -2000,32 +1989,8 @@
EXPECT_FALSE(IsPendingCustodianApproval(id));
}
-TEST_F(ExtensionServiceTestSupervised, UpdateWithPermissionIncreaseNoApproval) {
- InitNeedCustodianApprovalFieldTrial(false);
-
- InitServices(true /* profile_is_supervised */);
-
- MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
- supervised_user_service()->AddPermissionRequestCreator(
- base::WrapUnique(creator));
-
- std::string id = InstallPermissionsTestExtension(true /* by_custodian */);
-
- // Update to a new version with increased permissions.
- // Since we don't require the custodian's approval, no permission request
- // should be created.
- const std::string version2("2");
- EXPECT_CALL(*creator, CreateExtensionUpdateRequest(
- RequestId(id, version2), testing::_))
- .Times(0);
- UpdatePermissionsTestExtension(id, version2, DISABLED);
- EXPECT_FALSE(IsPendingCustodianApproval(id));
-}
-
TEST_F(ExtensionServiceTestSupervised,
UpdateWithPermissionIncreaseApprovalOldVersion) {
- InitNeedCustodianApprovalFieldTrial(true);
-
InitServices(true /* profile_is_supervised */);
MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
@@ -2076,8 +2041,6 @@
TEST_F(ExtensionServiceTestSupervised,
UpdateWithPermissionIncreaseApprovalMatchingVersion) {
- InitNeedCustodianApprovalFieldTrial(true);
-
InitServices(true /* profile_is_supervised */);
MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
@@ -2115,8 +2078,6 @@
TEST_F(ExtensionServiceTestSupervised,
UpdateWithPermissionIncreaseApprovalNewVersion) {
- InitNeedCustodianApprovalFieldTrial(true);
-
InitServices(true /* profile_is_supervised */);
MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
@@ -2163,7 +2124,6 @@
}
TEST_F(ExtensionServiceTestSupervised, SupervisedUserInitiatedInstalls) {
- InitNeedCustodianApprovalFieldTrial(true);
InitSupervisedUserInitiatedExtensionInstallFeature(true);
InitServices(true /* profile_is_supervised */);
@@ -2204,7 +2164,6 @@
TEST_F(ExtensionServiceTestSupervised,
UpdateSUInitiatedInstallWithoutPermissionIncrease) {
- InitNeedCustodianApprovalFieldTrial(true);
InitSupervisedUserInitiatedExtensionInstallFeature(true);
InitServices(true /* profile_is_supervised */);
@@ -2243,7 +2202,6 @@
TEST_F(ExtensionServiceTestSupervised,
UpdateSUInitiatedInstallWithPermissionIncrease) {
- InitNeedCustodianApprovalFieldTrial(true);
InitSupervisedUserInitiatedExtensionInstallFeature(true);
InitServices(true /* profile_is_supervised */);
@@ -2289,7 +2247,6 @@
TEST_F(ExtensionServiceTestSupervised,
UpdateSUInitiatedInstallWithPermissionIncreaseApprovalArrivesFirst) {
- InitNeedCustodianApprovalFieldTrial(true);
InitSupervisedUserInitiatedExtensionInstallFeature(true);
InitServices(true /* profile_is_supervised */);
diff --git a/chrome/browser/extensions/extension_util.cc b/chrome/browser/extensions/extension_util.cc
index 3260651a..df5aeb7 100644
--- a/chrome/browser/extensions/extension_util.cc
+++ b/chrome/browser/extensions/extension_util.cc
@@ -44,10 +44,6 @@
namespace util {
namespace {
-
-const char kSupervisedUserExtensionPermissionIncreaseFieldTrialName[] =
- "SupervisedUserExtensionPermissionIncrease";
-
// The entry into the prefs used to flag an extension as installed by custodian.
// It is relevant only for supervised users.
const char kWasInstalledByCustodianPrefName[] = "was_installed_by_custodian";
@@ -356,17 +352,5 @@
profile->IsSupervised();
}
-bool NeedCustodianApprovalForPermissionIncrease(const Profile* profile) {
- if (!profile->IsSupervised())
- return false;
- // Query the trial group name first, to make sure it's properly initialized.
- base::FieldTrialList::FindFullName(
- kSupervisedUserExtensionPermissionIncreaseFieldTrialName);
- std::string value = variations::GetVariationParamValue(
- kSupervisedUserExtensionPermissionIncreaseFieldTrialName,
- profile->IsChild() ? "child_account" : "legacy_supervised_user");
- return value == "true";
-}
-
} // namespace util
} // namespace extensions
diff --git a/chrome/browser/extensions/extension_util.h b/chrome/browser/extensions/extension_util.h
index 0790df2..9d99cb27 100644
--- a/chrome/browser/extensions/extension_util.h
+++ b/chrome/browser/extensions/extension_util.h
@@ -122,10 +122,6 @@
// Returns true for custodian-installed extensions in a supervised profile.
bool IsExtensionSupervised(const Extension* extension, Profile* profile);
-// Returns true if supervised users need approval from their custodian for
-// approving escalated permissions on updated extensions.
-bool NeedCustodianApprovalForPermissionIncrease(const Profile* profile);
-
} // namespace util
} // namespace extensions
