1907844 - AddonManagerInternal._updatePromptHandler does not take into account whether the extension is force installed and extension manifest version when comparing permissions

Status: RESOLVED FIXED

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

Description

[brian.coleman]

Attachment: upgrade-prompt.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36

Steps to reproduce:

On Firefox 115 ESR:

1. Create a manifest v2 extension
2. On Windows, force install the extension by adding an entry for the extension to the ExtensionSettings Group Policy, specifying that the installation_mode be force_installed.
3. Add an optional install time permission to the extension manifest, for example add <all_urls> to the permissions key in the extension manifest
4. Upgrade the extension
5. Despite the fact that the extension installation_mode is force_installed, the user is granted the opportunity to decline the upgrade because an optional install time permission has been added to the extension

Actual results:

The user is granted the opportunity to decline the upgrade because an optional install time permission has been added to the extension

Expected results:

The user should not be granted the opportunity to decline the upgrade because an optional install time permission has been added to the extension for the following reasons.

1. The extension installation_method is force_installed, so the extension is not controlled by the user as the machine is managed by an organization using Group Policy. It should be the organization's choice whether to install/upgrade the extension and not the user's choice.
2. The extension is a manifest v2 extension. By observation, when an manifest v2 extension with an optional install time permission such as <all_urls> is force installed for the first time via Group Policy, the user is not granted the opportunity to decline the extension installation.

Comment 1

[brian.coleman]

Attachment: about-policies.png

Comment 2

[brian.coleman]

The code which performs the permissions comparison upon extension upgrade is here: https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.sys.mjs#1232

Comment 3

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

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 4

[Rob Wu [:robwu]]

Mike - what should we do here? Should we automatically grant permissions of updated extensions that are installed through the force_installed enterprise policy?

If we do decide to fix this bug, then we should also automatically grant MV3 host permissions on update (which we currently do not - bug 1893232).

Comment 5

[Mike Kaply [:mkaply]]

Yes, we should grant those permissions.

See similar issues:
Bug 1641093 - Addons installed via policy shouldn't be disabled when permissions change
and also
Bug 1904054 - When an mv3 extension is force_installed via policy, users should not be able to change host permissions

Comment 6

[Rob Wu [:robwu]]

If you're interested in working on a patch, ask me or :zombie for help if needed.

At a high level, because MV3 origin permissions are treated as optional, we should internally call ExtensionPermissions.add with the new requested permissions. We implemented the logic to do this at install time at bug 1889402, but haven't done anything with updates yet. The general case is at bug 1893232, but we can consider doing this for enterprise even without addressing the general case.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Bug 1907844 - Force permission updates for extensions installed by policy. r?willdurand!

Comment 8

[Mike Kaply [:mkaply]]

This doesn't have a test (yet).

Can someone point me to a good place to grab a test for this?

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Mike Kaply [:mkaply] from comment #8)

This doesn't have a test (yet).

Can someone point me to a good place to grab a test for this?

This setupExtensionWithUpdates helper function defined in browser_html_updates.js may provide some ideas about how to recreate the scenario I think we may need (but likely along with tweaking the approach a bit to appropriately combine it with use of the enterprise policy test helpers), there are also other helper functions in that test file that may also be useful source of inspiration for other pieces that may be needed (e.g. findUpdatesForAddonId) to trigger an update check for the test extension.

Comment 10

[Mike Kaply [:mkaply]]

[Tracking Requested - why for this release]:
I know I'm cutting this close, but this is going to be a very small enterprise only change and we have an enterprise partner that really needs this.
Hoping to get it reviewed/in nightly today or tomorrow.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e95ee971193c
Force permission updates for extensions installed by policy. r=willdurand

Comment 12

[Mike Kaply [:mkaply]]

Attachment: Bug 1907844 - Force permission updates for extensions installed by policy. r?willdurand!

Original Revision: https://phabricator.services.mozilla.com/D243175

Comment 13

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Enterprises having addon upgrade issues
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Very low
• Explanation of risk level: Change is only for enterpise policy
• String changes made/needed: None
• Is Android affected?: no

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/e95ee971193c

Comment 15

[Pulsebot]

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

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Bug 1907844 - Force permission updates for extensions installed by policy. r?willdurand!

Original Revision: https://phabricator.services.mozilla.com/D243175

Comment 17

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Extensions installed by policy get disabled
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Very low
• Explanation of risk level: enterprise only
• String changes made/needed: none
• Is Android affected?: no

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/f3251b1af9b1