Adding ftp ws/wss permissions should not trigger extension permission warnings on extension upgrade

VERIFIED FIXED in Firefox 66

Status

P2
enhancement
VERIFIED FIXED
5 months ago
2 months ago

People

(Reporter: alexeiatyahoodotcom+mzllbgzll, Assigned: robwu)

Tracking

63 Branch
mozilla66
Dependency tree / graph

Firefox Tracking Flags

(firefox66 verified)

Details

(Whiteboard: [permission])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 months ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36

Steps to reproduce:

We have recently had another (still ongoing) incident with HTTPS Everywhere. A third of HTTPS Everywhere's users are no longer upgrading to the latest version because of a trivial permissions change: HTTPS Everywhere already had *://*/* and asked for ftp://*/*. You can see some of the fallout in https://github.com/EFForg/https-everywhere/issues/16377 and more in HTTPS Everywhere's addons.mozilla.org reviews.

A similar issue is blocking Privacy Badger from properly filtering web socket requests (https://github.com/EFForg/privacybadger/issues/1010). Privacy Badger currently asks for http/https permissions, but needs ws/wss (or <all_urls>). However triggering a permissions dialog on upgrade is unacceptable as Privacy Badger will effectively lose a third of its userbase.


Actual results:

Users saw "Access your data on all websites" (https://user-images.githubusercontent.com/156799/44497426-0822b980-a62e-11e8-8db8-1bb942de3f11.png) and freaked out.


Expected results:

Adding ftp/ws/wss permissions (or replacing http/https with <all_urls>) should not trigger any new permission warnings on upgrade.
(Reporter)

Comment 1

5 months ago
This issue is related to bug 1411999 and bug 1441693.
(Reporter)

Comment 2

5 months ago
There is also the question of whether setting "match_about_blank" on a content script declaration will trigger any new warnings (https://bugzilla.mozilla.org/show_bug.cgi?id=1411999#c9). It should not!
Priority: -- → P3

Updated

5 months ago
Severity: normal → enhancement
Whiteboard: [permission]
(Reporter)

Comment 3

5 months ago
I'm having trouble reproducing this permissions bug in testing.

I set up a self-hosted, unsigned copy of an extension that asks for the "*://*/*" permission in the extension manifest. I then configured Firefox to accept unsigned extensions, installed the self-hosted extension, uploaded an updated version that adds the "ftp://*/*" permission, updated the updates manifest to point to the new version, and forced an update in about:addons. The extension updated, but no permissions dialog appeared.

I was able to reproduce getting the permissions dialog when I added "unlimitedStorage", however.
(Reporter)

Updated

5 months ago
See Also: → bug 1411999

Comment 4

4 months ago
We’re having similar issues in the latest version of Ghostery (v8.2.5) in which we added manifest permissions for `ws://` and `wss://`. The upgrade causes Firefox to throw a permission dialog, which is misleading and redundant since users have already allowed `http://` and `https://` host permissions on installation.  As Alexei mentioned, this is needlessly scaring users and forcing them to re-accept a permission that they’ve already accepted. 

You can see the pushback from Ghostery users on our review board:

https://user-images.githubusercontent.com/4699516/49228878-54626000-f3ba-11e8-8c89-7fa233300492.png

Additionally, some users aren’t getting the permission prompt in the url bar, but instead it’s buried in the toolbar hamburger menu. If they don’t see it, it goes away when you close Firefox.

https://user-images.githubusercontent.com/4699516/49228286-013bdd80-f3b9-11e8-8863-c939acdbdfca.png

For the time being, we are forced to remove WebSocket filter support from the Firefox version of Ghostery. The upgrade conversion rate has been very low. 

In cases where users have already consented to allow host permissions for an extension, could a fix be implemented so that these users are not re-prompted when a host permission is changed?
(Assignee)

Updated

3 months ago
Duplicate of this bug: 1445581
(Reporter)

Comment 6

3 months ago
I'd like to point out that none of these permissions warnings gotchas are a problem in Chrome.
(Assignee)

Updated

3 months ago
Assignee: nobody → rob
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P3 → P2
(Assignee)

Comment 7

3 months ago
str
STR for QA:

1. Install https://addons.mozilla.org/en-US/firefox/addon/https-everywhere/versions/2018.6.21
2. Visit about:addons and click on Extensions.
3. Click on the gears icon and "Check for Updates".
4. Click on "Install updates" at the "Available updates" tab.

Expected:
- Updated without additional prompt

Actual:
- New "Access your data on all websites" permission warning as reported in comment 0.
(Reporter)

Comment 8

3 months ago
To be clear, this isn't just about "ftp", but ws/wss (Privacy Badger needs to add them but can't, not without losing a large percentage of its user base which is unacceptable: https://github.com/EFForg/privacybadger/issues/1010#issuecomment-442260262) and really any other what should be trivial upgrade. I suggest taking a hard look at all the differences in permissions warning logic Firefox has with Chrome, removing as many as possible and documenting the rest (bug 1411999). There should also be a simple way of testing what happens with permissions warnings on extension update (bug 1505510).

Thank you for working on this!
(Assignee)

Comment 10

3 months ago
Depends on D5527

Depends on D14962
(Assignee)

Comment 11

3 months ago
If an extension with the "mozillaAddons" permission is updated, the
permission diffing logic should support restricted schemes.
Otherwise the MatchPattern will throw and prevent the update from being
installed.

`Extension.comparePermissions` is called with the result of
`.userPermissions`, which in turn is equivalent to the result of the
`manifestPermissions` getter. This already filters out restricted
schemes if needed. Therefore we can unconditionally use
`restrictSchemes:false` in `comparePermissions`.

Depends on D14963
Attachment #9032400 - Attachment description: Bug 1504018 - Unit tests for permission warnings on update → Bug 1504018 - Skip host permissions for which a warning has been shown before
Attachment #9032401 - Attachment description: Bug 1504018 - Add restrictSchemes:false to comparePermissions → Bug 1504018 - Support unrestricted schemes in permission warnings
Attachment #9032399 - Attachment is obsolete: true
Blocks: 1515697

Comment 12

2 months ago
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/dc4bd4ee4c22
Skip host permissions for which a warning has been shown before r=aswan
https://hg.mozilla.org/integration/autoland/rev/8583d9d48387
Support unrestricted schemes in permission warnings r=aswan

Comment 13

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Updated

2 months ago
Depends on: 1519626

Comment 14

2 months ago

Verified using multiple profiles and addon updates including 'HTTPS everywhere' on Nightly 66.0a1 (20190117095319) running on Windows 10 x64 bit following the steps described on comment #7, upon update no prompts were displayed, also verified that the issue is indeed reproducible on earlier versions (64.0 ).

Status: RESOLVED → VERIFIED
status-firefox66: fixed → verified
You need to log in before you can comment on or make changes to this bug.