Closed Bug 1568921 Opened 5 months ago Closed 4 months ago

Addons.mozilla.org can uninstall add-ons locked with policies

Categories

(Firefox :: Enterprise Policies, defect, P1, critical)

defect

Tracking

()

VERIFIED FIXED
Firefox 70
Tracking Status
firefox-esr68 --- verified
firefox69 --- verified
firefox70 --- verified

People

(Reporter: mozilla, Assigned: mkaply)

References

()

Details

Attachments

(2 files)

Because the addons.mozilla.org website includes the ability to remove add-ons, this can be used to override enterprise policies. Regardless of whether or not the add-on is locked by a policy, it can still be removed using the website.

Instead, add-ons should only be removable if the enterprise policies allow for it.

Policies is just removing the value PERM_CAN_UNINSTALL in the addons manager.

So my wager is that this is probably an addons manager bug...

Needinfoing aswan for an opinion.

Flags: needinfo?(aswan)

The separation of responsibilities between the front-end and the add-on manager can be confusing but generally, the add-on manager uses things like the permission bit mentioned in comment 1 to tell the front-end what to offer to users, and then actual methods for things like uninstall assume the front-end only uses them when appropriate so the actual implementation don't re-check (eg, uninstall doesn't check that the addon being uninstalled has PERM_CAN_UNINSTALL, it assumes the front-end will only try to uninstall it if it does have that flag set).

In this specific case, that flag is propagated via the canUninstall property on the addon objects returned from the mozAddonManager api. AMO doesn't appear to pay attention to that property. The ideal fix here is for AMO to not offer uninstalls if that property is true (otherwise users will be given an option to take an action that should always fails).

But I've never been especially happy with the lack of backup checking in the addon manager, I would be in favor of adding a check in this case as well.

Flags: needinfo?(aswan)
Priority: -- → P1

ISTM that about:addons should have some check here if it doesn't, and that there should be some way to prevent AMO from showing a remove button. Followup bugs?

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)

ISTM that about:addons should have some check here if it doesn't, and that there should be some way to prevent AMO from showing a remove button. Followup bugs?

As mentioned in comment 2, about:addons already does check for this (at least the old one did, I hope the behavior and relevant tests were re-created in the new version). AMO has had an open issue on this for some time:
https://github.com/mozilla/addons-frontend/issues/1086

Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/9de8e9eca620
Don't allow addons to be uninstalled from AMO if policy prevents it. r=aswan
Status: UNCONFIRMED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
Assignee: nobody → mozilla

Comment on attachment 9081683 [details]
Bug 1568921 - Don't allow addons to be uninstalled from AMO if policy prevents it.

Beta/Release Uplift Approval Request

  • User impact if declined: Blocking the uninstall of extensions still allows them to be uninstalled from AMO.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Install extension from AMO via policy.
    Go to AMO and verify the Remove button returns an error.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Little used path, just adds check for uninstall flag. Has automated test.
  • String changes made/needed:
Attachment #9081683 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9081683 [details]
Bug 1568921 - Don't allow addons to be uninstalled from AMO if policy prevents it.

Fixes a gap in our enterprise policies allowing addons to be uninstalled when they shouldn't be able to. Thanks for including an automated test. Approved for 69.0b10.

Attachment #9081683 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
QA Whiteboard: [qa-triaged]
Attached image Screenshot_1.png

Hello!
Reproduced the issue with Firefox 70.0a1 (20190725215157) on Windows 10x64. The addons installed via policy and locked can be removed from AMO page.
The issue is verified fixed with Firefox 70.0a1 (20190804214813) and Firefox 69.0b10 (20190801185445) on Windows 10x64. Locked addons via policy cannot be removed from AMO page.

Although, while verifying this issue I noticed that after clicking the Remove button on AMO page and the error is thrown, the button changes to Add to Firefox. Adding the addon again makes the installing animation from AMO page entering an infinite loop. Refreshing the page fixes it. Is this the expected behavior? Should we close this as verified and fill another issue on this part? Thank you!

Flags: needinfo?(mozilla)

(In reply to Alexandru Trif, QA [:atrif] from comment #11)

Although, while verifying this issue I noticed that after clicking the Remove button on AMO page and the error is thrown, the button changes to Add to Firefox. Adding the addon again makes the installing animation from AMO page entering an infinite loop. Refreshing the page fixes it. Is this the expected behavior? Should we close this as verified and fill another issue on this part? Thank you!

That's an AMO bug, please just add a comment describing this to https://github.com/mozilla/addons-frontend/issues/1086 (or maybe https://github.com/mozilla/addons-frontend/issues/8418, those two issues look like dups to me)

Flags: needinfo?(mozilla)

Thank you for your response! As per comment 11 and comment 12, this issue is verified fixed. Commented with the issue encountered on Github.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Comment on attachment 9081683 [details]
Bug 1568921 - Don't allow addons to be uninstalled from AMO if policy prevents it.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for policy to work correctly.
  • User impact if declined: User can remove addon they weren't supposed to remove
  • Fix Landed on Version: 69,70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Only affects addons that have been marked as not uinstallable, has test.
  • String or UUID changes made by this patch:
Attachment #9081683 - Flags: approval-mozilla-esr68?

Comment on attachment 9081683 [details]
Bug 1568921 - Don't allow addons to be uninstalled from AMO if policy prevents it.

Approved for 68.1esr.

Attachment #9081683 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+

Hello! The issue is verified using Firefox 68.1.0esr (20190826132627) on Windows 10x64.

You need to log in before you can comment on or make changes to this bug.