1658768 - Locked extension added by policy is able to be removed

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[thetfordb]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

I created a configuration profile using the policy templates here: https://github.com/mozilla/policy-templates/blob/master/mac/org.mozilla.firefox.plist

I added an extension to the Extension -> Install array and then added the identifier to the Extension -> Locked array. I assigned the configuration profile to my test Mac.

On my test Mac, I removed the existing Firefox profile. I launched Firefox. The extension installed and was "locked". I went to manage add-ons. I clicked the "..." button on the right of the Enabled extension list for my particular extension. A selection list appears with "Can't be removed [Why?]", "Report", and "Manage". If I click to the right of the "Why" text on the "Can't be removed [Why?]" line, the addon is uninstalled.

Actual results:

The user is prompted to confirm uninstalling the addon and then the addon is uninstalled.

Expected results:

No uninstall action should be performed since the addon/extension is "locked" by policy. If I click anywhere else on that link, no action is performed. Clicking why takes me to the KB article for "Cannot remove an add-on (extension or theme)" as it should.

I don't know why clicking to the right of the word "Why?" performs an uninstall.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Andrew Swan [:aswan]]

Shane, this sounds like a regression from bug 1603227?

Comment 3

[Shane Caraveo (:mixedpuppy)]

Well, it seems like two issues here.

The first is a UI issue, clicking to the right of "why" is initiating an uninstall. bug 1603227 wouldn't have had any effect on UI.

The second issue is that despite the UI being reachable, ISTM should not allow an actual uninstall. Enterprise policies are checked after[1] those changes from bug 1603227, thus overriding them. Either the policy is not set correctly, or there is a problem checking the uninstall permission when uninstalling is initiated. I'll have to dig into that further.

[1] https://searchfox.org/mozilla-central/rev/bfdb6a1ef893e4c926ce52700ed187a9743603ce/toolkit/mozapps/extensions/internal/XPIDatabase.jsm#762

Comment 4

[Alexandra Martin]

Attachment: ext_test.mobileconfig

I've managed to reproduce the issue on MacOS 10.14 with the latest Firefox Release 80.0.1 (20200831163820) and Beta 81.0b7 (20200906164749) using the configuration profile from https://gist.github.com/paulgalow/9ffc4840586a3a9b47ec4c068de96928 and only changing Extension -> Install array to have the latest extension version. With the provided steps and with the made configuration profile, the extension is not installing on latest Nightly 82.0a1 (20200907094115) and I am not sure if I can change something in the configuration profile to make that happen.

Comment 6

[Shane Caraveo (:mixedpuppy)]

This is an issue original with the work on supporting the remove action in the html rewrite, bug 1580962. Some part of the remove element is clickable even though disabled.

Patch will ensure that the addon has uninstall permission prior to removing, that will catch any instance like this.

Manually tested with the policy:

{
  "policies": {
    "Extensions": {
      "Locked":  ["[e-mail]"]
    }
  }
}

Comment 7

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1658768 check permission before uninstalling an addon from about:addons

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/341ae1d83639
check permission before uninstalling an addon from about:addons r=rpl

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/341ae1d83639

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Patch for ESR 78

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fix specifically around policy, consistency with 90
User impact if declined: Can uninstall addon even if policy says no.
Fix Landed on Version: 90
Risk to taking this patch (and alternatives if risky): Low (has test).

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9225013 [details] [diff] [review]
Patch for ESR 78

Approved for 78.12esr.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/5d270f8c0460