783573 - Remove permissions whitelist from navigator.mozApps

Status: VERIFIED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[David Chan [:dchan]]

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

Comment 1

[[:fabrice] Fabrice Desré]

I agree. The only user are tests, but we should be able to give them the proper permission.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 3

[Gregor Wagner [:gwagner]]

This is the last API that uses the whitelist approach. We should really fix it.

Comment 4

[[:fabrice] Fabrice Desré]

Attachment: patch

I ran the tests locally, they all pass.

Comment 5

[Gregor Wagner [:gwagner]]

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

Comment 6

[Myk Melez [:myk] [@mykmelez]]

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

Comment 7

[[:fabrice] Fabrice Desré]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/030e483f5ec1