Remove permissions whitelist from navigator.mozApps

VERIFIED FIXED in mozilla18

Status

Core Graveyard
DOM: Apps
VERIFIED FIXED
5 years ago
a month ago

People

(Reporter: dchan, Assigned: fabrice)

Tracking

(Blocks: 1 bug)

unspecified
mozilla18

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:-)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
navigator.mozApps still uses a whitelist for mgmt permissions checks. This should be removed if possible.


88     try {
89       let hosts = Services.prefs.getCharPref("dom.mozApps.whitelist");
90       hosts.split(",").forEach(function(aHost) {
91         Services.perms.add(Services.io.newURI(aHost, null, null),
92                            "webapps-manage",
93                            Ci.nsIPermissionManager.ALLOW_ACTION);
94       });
95     } catch(e) { }

[1] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#88
(Reporter)

Updated

5 years ago
Blocks: 774716
No longer blocks: 781870
(Assignee)

Comment 1

5 years ago
I agree. The only user are tests, but we should be able to give them the proper permission.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Actually, this permission isn't something that we are currently planning on exposing to 3rd party apps, and so I don't think that we absolutely have to fix this before shipping.

Definitely should ideally be fixed though.
blocking-basecamp: + → -
blocking-kilimanjaro: --- → +
This is the last API that uses the whitelist approach. We should really fix it.
(Assignee)

Comment 4

5 years ago
Created attachment 656648 [details] [diff] [review]
patch

I ran the tests locally, they all pass.
Assignee: nobody → fabrice
Attachment #656648 - Flags: review?(anygregor)
Comment on attachment 656648 [details] [diff] [review]
patch

> 
>-SpecialPowers.setCharPref("dom.mozApps.whitelist", "http://mochi.test:8888");
>+let io = Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService2);
>+Components.classes["@mozilla.org/permissionmanager;1"]
>+          .getService(Components.interfaces.nsIPermissionManager)
>+          .add(io.newURI("http://mochi.test:8888", null, null),
>+               "webapps-manage",
>+               Components.interfaces.nsIPermissionManager.ALLOW_ACTION);
>+

Can't you do following?
SpecialPowers.addPermission("webapps-manage", true, document);
Attachment #656648 - Flags: review?(anygregor) → review+
Note that the tests continue to pass if you simply remove the code that sets the whitelist pref, so I don't think it's necessary to give webapps-manage permission to mochi.test:8888, presumably because the tests are mochitest-chrome tests that load from chrome://mochitests (which presumably has permission because it's chrome) rather than http://mochi.test:8888 (which I think is the origin from which mochitest-plain tests load).
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/030e483f5ec1

Updated

5 years ago
QA Contact: jsmith
Whiteboard: [qa+]
https://hg.mozilla.org/mozilla-central/rev/030e483f5ec1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

5 years ago
Keywords: verifyme
Whiteboard: [qa+]

Updated

5 years ago
Status: RESOLVED → VERIFIED
Keywords: verifyme

Updated

a month ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.