Closed Bug 783573 Opened 12 years ago Closed 12 years ago

Remove permissions whitelist from navigator.mozApps

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set
normal

Tracking

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

VERIFIED FIXED
mozilla18
blocking-kilimanjaro +
blocking-basecamp -

People

(Reporter: dchanm+bugzilla, Assigned: fabrice)

References

Details

Attachments

(1 file)

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
Blocks: 774716
No longer blocks: basecamp-permissions
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.
Attached patch patchSplinter Review
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).
QA Contact: jsmith
Whiteboard: [qa+]
https://hg.mozilla.org/mozilla-central/rev/030e483f5ec1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Keywords: verifyme
Whiteboard: [qa+]
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: