801996 - Install a packaged app that requests permissions that are not available to this app type results in NS_ERROR_FAILURE - Denied

Status: RESOLVED INVALID

(Core Graveyard :: DOM: Apps, defect)

Description

[Jason Smith [:jsmith]]

Steps:

1. Go to http://people.mozilla.com/~fdesre/openwebapps/test.html
2. Select install package

Expected:

The packaged app specified in the package.manifest should be installed.

Actual:

The packaged app fails to install without any notification at all. Checking the logcat, I see the following:

10-15 20:03:59.448: E/GeckoConsole(411): [JavaScript Error: "NS_ERROR_FAILURE: Denied" {file: "jar:file:///system/b2g/omni.ja!/components/Webapps.js" line: 667}]

Comment 1

[Jason Smith [:jsmith]]

The error indicated here appears to be thrown if the app does not have sufficient privileges:

  set oninstall(aCallback) {
    if (this.hasPrivileges)
      this._oninstall = aCallback;
    else
      throw new Components.Exception("Denied", Cr.NS_ERROR_FAILURE);
  },

Why is that the case?

Comment 2

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #1)
> The error indicated here appears to be thrown if the app does not have
> sufficient privileges:
> 
>   set oninstall(aCallback) {
>     if (this.hasPrivileges)
>       this._oninstall = aCallback;
>     else
>       throw new Components.Exception("Denied", Cr.NS_ERROR_FAILURE);
>   },
> 
> Why is that the case?

Wait. I know what happened here after looking at the zip file:

  "permissions": [
    "contacts",
    "systemXHR"
  ],

A non-privileged app I don't believe has access to contacts, so this definitely shouldn't be installable. 

The error fired is right, although we could improve on this by throwing an error callback with PERMISSION_DENIED.

Comment 3

[Jason Smith [:jsmith]]

We really should not be dumping a NS_ERROR_FAILURE and should instead be failing safely with an error callback indicating permission denied.

Comment 4

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

Not blocking on this specifically. If we don't have error handling that sounds like a separate bug that might be a blocker

Comment 5

[Jason Smith [:jsmith]]

(In reply to Jonas Sicking (:sicking) from comment #4)
> Not blocking on this specifically. If we don't have error handling that
> sounds like a separate bug that might be a blocker

I don't follow. Right now, a NS_ERROR_FAILURE is fired if the app does not have permissions as an exception, which is messy from being interpreted from a JS caller's perspective (example: installing web apps from . If the implementation suggestion isn't the way to go, then I'll genericize the bug and mark it for re-triage. But right now, this isn't acceptable if the marketplace guys run into this (specifically in the review process), which is definitely possible.

Comment 6

[Jason Smith [:jsmith]]

Feel free to morph the bug to whatever is the right acceptable approach for v1. Geo pointed out it was a minus based on the idea I came up with, but feel free to morph to whatever is acceptable for v1.

Comment 7

[[:fabrice] Fabrice Desré]

(In reply to Jason Smith [:jsmith] from comment #1)
> The error indicated here appears to be thrown if the app does not have
> sufficient privileges:
> 
>   set oninstall(aCallback) {
>     if (this.hasPrivileges)
>       this._oninstall = aCallback;
>     else
>       throw new Components.Exception("Denied", Cr.NS_ERROR_FAILURE);
>   },
> 
> Why is that the case?

This is unrelated to app install. The page is trying to use parts of the API that need the webapps-manage permission, and fail to do so. That's fine.

Comment 8

[[:fabrice] Fabrice Desré]

(In reply to Jason Smith [:jsmith] from comment #2)

> 
> Wait. I know what happened here after looking at the zip file:
> 
>   "permissions": [
>     "contacts",
>     "systemXHR"
>   ],
> 
> A non-privileged app I don't believe has access to contacts, so this
> definitely shouldn't be installable. 
> 
> The error fired is right, although we could improve on this by throwing an
> error callback with PERMISSION_DENIED.

This is very possible that we don't bubble an exception from PermissionInstaller.jsm - I'll take a look and update this bug accordingly.