Closed Bug 797574 Opened 7 years ago Closed 7 years ago

PermissionsInstaller.jsm broken for package apps

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set

Tracking

(blocking-basecamp:+, firefox18 fixed)

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed

People

(Reporter: dchanm+bugzilla, Assigned: fabrice)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

Package app installation was recently changed by bug 789527 . This invalidated one of the assumptions that PermissionsInstaller relied on.

app.manifest is no longer defined for packaged apps [1]

The manifest now resides at app.updateManifest. The Webapps.jsm code accounts for this [2]. Other parts of the code that rely on ManifestHelper may break without a similar isPackage check.


Dump from PermissionsInstaller.jsm before ManifestHelper is called
I/Gecko   (  438): app is {"installOrigin":"http://mochi.test:8888","origin":"app://{79c967f9-30e1-4d20-99aa-b7b6aa1f6058}","manifestURL":"http://example.com/tests/webapi/apps/webapp.sjs","updateManifest":{"name":"Super Crazy Basic App","installs_allowed_from":["*"],"type":"certified","package_path":"http://example.com/tests/webapi/app.zip","permissions":{"geolocation":{"description":"geolocate"},"contacts":{"description":"contacts","access":"readwrite"}}},"etag":null,"receipts":[],"categories":[],"removable":true,"id":"{79c967f9-30e1-4d20-99aa-b7b6aa1f6058}","installTime":1349300320014,"lastUpdateCheck":1349300320015}


[1] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.js#229
[2] - http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#897
Summary: PermissionInstaller.jsm broken for package apps → PermissionsInstaller.jsm broken for package apps
So, the question is: do we want to have the permissions in the mini-manifest and install them from here, or do we wait for the package download to use the "normal" manifest?
We should not be putting the permissions in the minimanifest. And absolutely not be making PermissionsInstaller.jsm use permissions enumerated in the mini manifest.

The list of permissions granted to an app needs to be signed by the store. This can only happen in the package.

This is exactly why we don't want to put permissions into the mini manifest, even for informative purposes, since there's a risk that someone would actually use it to make security decisions.

So we can't go through the permission-installation code until the package has been downloaded.
Assignee: nobody → fabrice
blocking-basecamp: ? → +
Attached patch patchSplinter Review
With this patch we update permissions in the following situations:
- Packaged apps: we have downloaded the package or applied a new update.
- Hosted apps: the installation is confirmed, or we updated.

For hosted apps with an app-cache, I'm not sure whether to defer the permissions install/update up to the moment the app-cache is downloaded.
Attachment #668588 - Flags: review?(anygregor)
What patch is this based on? The argument list for InstallPermissions doesn't match with mxr.
(In reply to Gregor Wagner [:gwagner] from comment #4)
> What patch is this based on? The argument list for InstallPermissions
> doesn't match with mxr.

I build the aApp object with only the properties used (.manifest, .orgin and .manifestURL, and the 3rd parameter is optional. I will update the function documentation though since it doesn't match the implementation anymore.
Attachment #668588 - Flags: review?(anygregor) → review+
https://hg.mozilla.org/mozilla-central/rev/87fc9650f607
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Keywords: verifyme
QA Contact: jsmith
QA Contact: jsmith
Keywords: verifyme
Whiteboard: [qa-]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.