Closed Bug 1759737 Opened 2 years ago Closed 2 years ago

Prevent XPInstalls if they are not triggered by a user gesture

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
100 Branch
Tracking Status
firefox100 --- verified

People

(Reporter: rpl, Assigned: rpl)

References

(Blocks 1 open bug)

Details

(Whiteboard: [addons-jira])

Attachments

(2 files)

Triggering an XPI installation prompts the user, and so it should be treated as other popups and permissions prompts that a webpage are able to trigger and require a user gesture.

Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/9a5b9d5e5a20
Require user gesture on XPInstalls requests. r=mixedpuppy
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

Backed out changeset 9a5b9d5e5a20 (Bug 1759737) for causing screenshots failures on browser_permissionPrompts.js.
Backout link
Push with failures
Failure Log

Status: RESOLVED → REOPENED
Flags: needinfo?(lgreco)
Resolution: FIXED → ---

Backout merged to central. Changeset here

My apologies, browser_permissionPrompts.js didn't got caught form my push to try and so we didn't notice it did also need to be updated to account for the new user activation requirement to be able to successfully trigger the add-on install flow.

I've been able to trigger the same failure locally and I've attached a separate patch which does make the test to pass consistently again (D141474).

I've also pushed it to try here to confirm that is the case also when executed on the build infrastructure:

Flags: needinfo?(lgreco)

Push to try also confirmed that the patch from D141474 did fix the failure while running it on the build infrastructure as it was doing when I tried it locally:

Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/0965adbf0a03
Require user gesture on XPInstalls requests. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/3a526b204177
mozscreenshots clicks should be detected as user activated to trigger the addon install flow. r=mixedpuppy
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Blocks: 1752998
See Also: → 1755964
Blocks: 1761775

Verified the fix on the latest Nightly (100.0a1/20220328213646) under Windows 10 x64, Ubuntu 16.04 LTS and macOS 11.3.1.

Installation flows (both the ones started from JS code by calling InstallTrigger.install and the ones started by navigating to an extension .xpi) now require explicit user gesture to be activated.

Unsolicited installations (such as when calling window.location = "..." and InstallTrigger.install(...) on page load) fail with no doorhanger being shown and errors logged to web/browser console:

Testing has been performed on an example test page and 2 real websites which self host add-ons: https://www.grammarly.com/browser/firefox and https://www.zotero.org/download/ .

Status: RESOLVED → VERIFIED

I'm linking the Webcompat Regression https://github.com/webcompat/web-bugs/issues/101685, which is related to this change (and not Bug 1754441):

The install page from www.dashlane.com is hitting the error raised if InstallTrigger.install is being called outside of handing the user input.

As part of the QA verification related to this change, Alex did also test a number of known extension that are often installed from the extension developer website through the "third party install flow" and www.dashlane.com was one of the websites tested explicitly:

The QA verification on www.dashlane.com determined that:

  • no install is triggered while the user activation requirement is enabled.
  • temporarily flipping off the user activation requirement allows the installation to complete successfully.
  • the InstallTrigger warnings are being logged in the console in both cases (because the install flow is being triggered using InstallTrigger.install, but the feature is still enabled and so it is not what prevents the install flow to be triggered successfully)

We raise an explicit error when we reject an addon installation due to the user activation requirement, but that is not visible in the webconsole because it is being caught by the website itself:

              InstallTrigger.install(n, (function (r, n) {
                  0 === n && e.stateChanged('STATE_INSTALLING'),
                  t(null, 0 === n)
                }))
              } catch (e) {
                t(e)
              }

Adding a breakpoint on the catch block would show that the InstallTrigger.install call raised an exception with the error message: "InstallTrigger.install can only be called from a user input handler".

Based on an initial look to the stack trace of that call, I suspect that the InstallTrigger.install may not be detected as handling user input because it is called later as part of some asynchronous execution and so the user input flag may be there anymore when the call to InstallTrigger.install is reached.

Updating the docs related to self-hosting WebExtensions xpi files and the related communications to the extension developers (highlighting this change along with the future removal of the InstallTrigger deprecation) are the next steps planned.

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

Attachment

General

Created:
Updated:
Size: