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

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[Wesley Branton]

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.

Comment 1

[Mike Kaply [:mkaply]]

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.

Comment 2

[Andrew Swan [: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.

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1568921 - Don't allow addons to be uninstalled from AMO if policy prevents it.

Comment 4

[Shane Caraveo (:mixedpuppy)]

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?

Comment 5

[Andrew Swan [:aswan]]

(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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9de8e9eca620
Don't allow addons to be uninstalled from AMO if policy prevents it. r=aswan

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/9de8e9eca620

Comment 8

[Mike Kaply [:mkaply]]

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:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b8080b50a515

Comment 11

[Alexandru Trif, Desktop Test Engineering [:atrif] [PTO 15-29 Aug, please ni? :mboldan]]

Attachment: 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!

Comment 12

[Andrew Swan [:aswan]]

(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)

Comment 13

[Alexandru Trif, Desktop Test Engineering [:atrif] [PTO 15-29 Aug, please ni? :mboldan]]

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

Comment 14

[Mike Kaply [:mkaply]]

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:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 68.1esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/1eb4ebf64839

Comment 17

[Alexandru Trif, Desktop Test Engineering [:atrif] [PTO 15-29 Aug, please ni? :mboldan]]

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