diff options
3 files changed, 1 insertions, 57 deletions
diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc index cc2aae7..49db418 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc +++ b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc @@ -44,15 +44,6 @@ class ComparablePermission { }; using ComparablePermissions = std::vector<ComparablePermission>; -void DropPermissionParameter(APIPermission::ID id, - PermissionIDSet* permissions) { - if (permissions->ContainsID(id)) { - // Erase the permission, and insert it again without a parameter. - permissions->erase(id); - permissions->insert(id); - } -} - } // namespace typedef std::set<PermissionMessage> PermissionMsgSet; @@ -185,20 +176,6 @@ bool ChromePermissionMessageProvider::IsAPIOrManifestPrivilegeIncrease( AddAPIPermissions(new_permissions, &new_ids); AddManifestPermissions(new_permissions, &new_ids); - // Ugly hack: Before M46 beta, we didn't store the parameter for settings - // override permissions in prefs (which is where |old_permissions| is coming - // from). To avoid a spurious permission increase warning, drop the parameter. - // See crbug.com/533086. - // TODO(treib,devlin): Remove this for M48, when hopefully all users will have - // updated prefs. - const APIPermission::ID kSettingsOverrideIDs[] = { - APIPermission::kHomepage, APIPermission::kSearchProvider, - APIPermission::kStartupPages}; - for (auto id : kSettingsOverrideIDs) { - DropPermissionParameter(id, &old_ids); - DropPermissionParameter(id, &new_ids); - } - // If all the IDs were already there, it's not a privilege increase. if (old_ids.Includes(new_ids)) return false; diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc index 2eab2a4..9ff56dd 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc +++ b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc @@ -131,30 +131,4 @@ TEST_F(ChromePermissionMessageProviderUnittest, EXPECT_FALSE(message1.submessages().empty()); } -// Anti-test: Check that adding a parameter to a SettingsOverridePermission -// doesn't trigger a privilege increase. This is because prior to M46 beta, we -// failed to store the parameter in the granted_permissions pref. Now we do, and -// we don't want to bother every user with a spurious permissions warning. -// See crbug.com/533086. -// TODO(treib,devlin): Remove this for M48, when hopefully all users will have -// updated prefs. -TEST_F(ChromePermissionMessageProviderUnittest, - EvilHackToSuppressSettingsOverrideParameter) { - const APIPermissionInfo* info = - PermissionsInfo::GetInstance()->GetByID(APIPermission::kSearchProvider); - - APIPermissionSet granted_permissions; - granted_permissions.insert(new SettingsOverrideAPIPermission(info)); - - APIPermissionSet actual_permissions; - actual_permissions.insert(new SettingsOverrideAPIPermission(info, "a.com")); - - EXPECT_FALSE(IsPrivilegeIncrease(granted_permissions, actual_permissions)); - - // Just to be safe: Adding the permission (with or without parameter) should - // still be considered a privilege escalation. - EXPECT_TRUE(IsPrivilegeIncrease(APIPermissionSet(), granted_permissions)); - EXPECT_TRUE(IsPrivilegeIncrease(APIPermissionSet(), actual_permissions)); -} - } // namespace extensions diff --git a/extensions/common/permissions/settings_override_permission.cc b/extensions/common/permissions/settings_override_permission.cc index 43765d2..69318d829 100644 --- a/extensions/common/permissions/settings_override_permission.cc +++ b/extensions/common/permissions/settings_override_permission.cc @@ -48,14 +48,7 @@ bool SettingsOverrideAPIPermission::FromValue( const base::Value* value, std::string* /*error*/, std::vector<std::string>* unhandled_permissions) { - // Ugly hack: |value| being null should be an error. But before M46 beta, we - // didn't store the parameter for settings override permissions in prefs. - // See crbug.com/533086. - // TODO(treib,devlin): Remove this for M48, when hopefully all users will have - // updated prefs. - // This should read: - // return value && value->GetAsString(&setting_value_); - return !value || value->GetAsString(&setting_value_); + return value && value->GetAsString(&setting_value_); } scoped_ptr<base::Value> SettingsOverrideAPIPermission::ToValue() const { |