Note: This is about android only, Firefox OS works fine.
STR:
1. Go to url above.
2. Click the install button.
Observed results:
- Installing takes longer than usual.
- After a while, instead of "Launch", it says "Install" again. I click it again, I get an error that the app is already installed.
- The app is not in the android app list nor on a home screen.
- I see it in about:apps though, but with a broken icon.
- I cannot launch it from there.
Expected results:
App install and launch should succeed normally.

Now I understand the problem here. Disregard the dupe.
I do not have any knowledge of how we can push reviewer certs to a FxAndroid device right now to allow us to test packaged apps during an app review process. As such, this is a FxAndroid bug.

(In reply to Andrew Williamson [:eviljeff] from comment #9)
> Bill, I'm trying to get some further understanding of the problem underlying
> this bug. Is it effectively blocked on bug 896620? Or is it a different
> issue?
Different issue -- As far as I know, bug 896620 pertains only to desktop web runtime, I believe we have solved this problem on Android for now.

(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #10)
> (In reply to Andrew Williamson [:eviljeff] from comment #9)
> > Bill, I'm trying to get some further understanding of the problem underlying
> > this bug. Is it effectively blocked on bug 896620? Or is it a different
> > issue?
>
> Different issue -- As far as I know, bug 896620 pertains only to desktop web
> runtime, I believe we have solved this problem on Android for now.
Just to clarify, you meant that the equivalent issue in bug 896620 for Android was fixed, not that *this* bug is fixed? Because its not afaik.

(In reply to Christopher Van Wiemeersch [:cvan] from comment #13)
> Will bug 896620 fix this? I'm unclear of what the verdict was above.
No, I don't think so. That bug is specific to desktop, while this bug is for Android; and that bug is about the production certificate, while this bug is about enabling installation of apps signed with the review certificate.

Any chance I can get "reviewer" privileges on marketplace-dev.allizom.org so I can diagnose the problem? I used to have them, but now when I go to <https://marketplace-dev.allizom.org/reviewers/> I'm told "Oops! Not allowed. You tried to do something that you weren't allowed to."
My account's email address is myk@mozilla.org.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #15)
> Any chance I can get "reviewer" privileges on marketplace-dev.allizom.org so
> I can diagnose the problem? I used to have them, but now when I go to
> <https://marketplace-dev.allizom.org/reviewers/> I'm told "Oops! Not
> allowed. You tried to do something that you weren't allowed to."
Done.

The first problem is that the APK Factory service doesn't have access to the mini-manifest (nor, presumably, the package) for the app. In order for Fennec to download and install the APK, the APK Factory has to be able to access those resources. But they seem to require a session identifier that is presumably stored in a cookie.
cvan: can the Marketplace allow the APK Factory service to access those resources?
Regardless, after we fix that problem, I suspect we'll run into something like an INVALID_SIGNATURE error on webapp install because Fennec doesn't have the certificate with which the reviewer package was signed. To fix that, we need to find a way to enable reviewers to install that certificate. And Brian Smith and I just discussed doing that on desktop via an addon that installs the certificate. Presumably we could do the same thing on Android.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #17)
> Regardless, after we fix that problem, I suspect we'll run into something
> like an INVALID_SIGNATURE error on webapp install because Fennec doesn't
> have the certificate with which the reviewer package was signed. To fix
> that, we need to find a way to enable reviewers to install that certificate.
> And Brian Smith and I just discussed doing that on desktop via an addon that
> installs the certificate. Presumably we could do the same thing on Android.
Yeah I wrote an add-on that did that last year:
https://addons.mozilla.org/en-US/android/addon/mkt-app-reviewer-certs/
- it needs a little polish though.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #18)
> (In reply to Myk Melez [:myk] [@mykmelez] from comment #17)
> > The first problem is that the APK Factory service doesn't have access to the
> > mini-manifest (nor, presumably, the package) for the app.
>
> To be precise: the client's request to download the mini-manifest results in
> a "bad request" error:
Yep, the view to a mini manifest requires reviewer authentication: https://github.com/mozilla/zamboni/blob/master/mkt/reviewers/views.py#L795
I'm not sure why. Andy, do you know? Maybe we need to protect a non-public app from being exposed while under review? If that's the case, it might be a tough fix. We'd have to pass around some sort of reviewer token in lieu of sending a session cookie.
Maybe our reviewer instance (bug 963676) will solve this? The reviewer instance could maybe get some privileged API access to view mini manifests.

As you've found the view is protected by a permission ("Apps:Review") which requires a user session to associate this with.
If I remember correctly, the main reason for setting it up this way was to only allow reviewers to install non-public reviewer signed packaged apps.
Once you get the mini manifest the path to the actual signed zip is also protected by the same permission:
https://github.com/mozilla/zamboni/blob/master/mkt/reviewers/views.py#L838

(In reply to Rob Hudson [:robhudson] from comment #22)
> As you've found the view is protected by a permission ("Apps:Review") which
> requires a user session to associate this with.
>
> If I remember correctly, the main reason for setting it up this way was to
> only allow reviewers to install non-public reviewer signed packaged apps.
>
> Once you get the mini manifest the path to the actual signed zip is also
> protected by the same permission:
> https://github.com/mozilla/zamboni/blob/master/mkt/reviewers/views.py#L838
Blocking the zip is the important part, so we could probably make the mini-manifest available if that helps (I suspect it doesn't).

(In reply to Andy McKay [:andym] from comment #24)
> Even if we provided some sort of auth to grab that package, would we then be
> in a scenario where a un-reviewed priveleged app is now available on the APK
> Factory CDN without any sort of security?
Yeah we shouldn't do that both from a possible malware point of view and also that we shouldn't be making a developers code available for download until they've chosen to publish it.

We could run the reviewer APK factory behind the marketplace, use the marketplace as a proxy doing the auth for the reviewer APK factory queue. That still won't stop the auth problem between the APK factory and the marketplace. I think the only way around this is to add authentication to the APK factory.

No, I just mean keep the current behavior (which always uses the AppMarketplaceProdPublicRoot cert), and override when you hit the marketplace urls. With your patch we don't fail in the same way when trying to install a signed app from another cert right?

Comment on attachment 8383012[details][diff][review]
Patch
Review of attachment 8383012[details][diff][review]:
-----------------------------------------------------------------
::: dom/apps/src/Webapps.jsm
@@ +2999,5 @@
> aApp.downloadAvailable = false;
> throw "CERTDB_ERROR";
> }
>
> + let [result, zipReader] = yield this._openSignedPackage(aApp.manifestURL,
Does the manifest URL actually require authentication?
It would be better, theoretically, to pass in the URL of the page that actually called the installPackage() function.
See also Jonas's email on the dev-webapps list. He probably should look at this too.
Personally, I think that this "/reviewers/" thing I suggested is just a hack to get what we've currently deployed working, so it doesn't have to be perfect because we know it isn't the long-term solution.
The best long-term solution is probably to extend the "installs_allowed_from" mechanism so that it can support paths. Then, instead of signing not-yet-reviewed apps using a different root, instead we would make sure that installs_allowed_from=https://marketplace.firefox.com/reviewers. Then it would be up to marketplace.firefox.com to refuse access (require auth) to stuff under /reviewers Not only would this be simpler, but this is something more likely to go through standardization.
However, I think that we don't need to do the long-term thing right now, especially considering we have legacy stuff to be concerned with. So, I think this patch is reasonable, though, like I said above, it would be better to use the URL of the page that called installPackage instead of the manifest URL.
@@ +3064,5 @@
> + : Ci.nsIX509CertDB.AppMarketplaceDevPublicRoot;
> + break;
> +
> + default:
> + throw "INSTALL_FROM_DENIED";
What Marco is doing here seems logical to me. I don't understand why we'd want to trust the marketplace cert for any other domain. In fact, I believe we DON'T want to trust it for any other domain, because we don't want other websites to distribute marketplace-signed apps, because they may be distributing old versions of apps that have been blacklisted for security reasons or which are out of date and missing security fixes.

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #39)
> Does the manifest URL actually require authentication?
>
> It would be better, theoretically, to pass in the URL of the page that
> actually called the installPackage() function.
>
> See also Jonas's email on the dev-webapps list. He probably should look at
> this too.
The problem is that we don't store the entire install URL, but only its origin, so during updates we can't use it.
To address the problem Jonas was referring to, we could use the install origin to choose between prod and dev and the path from the manifest URL to choose between reviewers and public:
let root;
switch (installOrigin) {
case "https://marketplace.firefox.com":
root = manifestURLPath.startsWith("/reviewers/")
? Ci.nsIX509CertDB.AppMarketplaceProdReviewersRoot
: Ci.nsIX509CertDB.AppMarketplaceProdPublicRoot;
break;
case "https://marketplace-dev.allizom.org":
root = manifestURLPath.startsWith("/reviewers/")
? Ci.nsIX509CertDB.AppMarketplaceDevReviewersRoot
: Ci.nsIX509CertDB.AppMarketplaceDevPublicRoot;
break;
default:
throw "INSTALL_FROM_DENIED";
}
This way third party stores can't install apps signed by the marketplace.

(In reply to Fabrice Desré [:fabrice] from comment #41)
> If we need th full install url to do things properly, we should just store
> that.
If the install url is just an hack that will probably get ditched in the future, maybe it's better to avoid storing this information that we will not need anymore (and we don't have it for apps that are already installed, so how would we update them?)

It's not clear what is being asked of me here, so I'll just give some thoughts and hope that it covers it.
I talked with Brian the other day about this patch.
Right now it is possible for 3rd party stores to "hotlink" to privileged apps that live in the mozilla marketplace. I think that's a good thing. Keep in mind that these apps will still be updated whenever the mozilla marketplace pushes an update. Including that they'll be updated to a "this app is blocked" version if we push such an update.
We can debate if we should remove this capability. Brian had some concerns about it. But I think we should have that debate as a separate bug as it's a fairly big deal.
Right now we do *not* allow privileged apps to be hosted on other origins than the mozilla marketplace ones. I think that's how we should keep it until we've properly figured out a security model to support 3rd party marketplaces. Hardcoding this limitation in the code seems fine.
I think it's an ok idea that we use the reviewer cert whenever the path starts with "/reviewers/", like the code in comment 40 does. I don't have a better suggestion.
However I think we should *also* require that a pref has been set that enables "reviewer mode". If that pref has not been set then we should always reject review-only apps.

(In reply to Jonas Sicking (:sicking) from comment #43)
> However I think we should *also* require that a pref has been set that
> enables "reviewer mode". If that pref has not been set then we should always
> reject review-only apps.
how would the pref be set? Whatever solution we go with it has to be possible to use it on non-rooted devices (i.e. doesn't rely on building images or pushing files with adb) to be useful to our reviewer community.

(In reply to Jonas Sicking (:sicking) from comment #46)
> You should be able to set prefs through adb. I believe that bsmith had some
> script that helped the reviewer do that.
The only script that we know of from smith to help reviewers uses adb to push the certs and the prefs.js file and it doesn't work on non-rooted devices. bsmith - do you have a different one?

(In reply to Andrew Williamson [:eviljeff] from comment #47)
> (In reply to Jonas Sicking (:sicking) from comment #46)
> > You should be able to set prefs through adb. I believe that bsmith had some
> > script that helped the reviewer do that.
>
> The only script that we know of from smith to help reviewers uses adb to
> push the certs and the prefs.js file and it doesn't work on non-rooted
> devices. bsmith - do you have a different one?
okay, bsmith can't help. If someone can confirm that prefs can be set in some way on a *non-rooted* device then we're good. If not then we're back to our current position with scripts and the inability for our volunteers to actually use consumer devices.

It was my understanding that Andrew's certificate installation add-on in Comment 20 worked, or at least worked at one point. But I'm currently testing on Firefox 29 (Aurora, I don't have any updates available yet for Beta), and every time I try to install a packaged app from the reviewer tools I get "Error code: 400 - Bad Request" in the log.

(In reply to Lisa Brewster [:adora] from comment #56)
> It was my understanding that Andrew's certificate installation add-on in
> Comment 20 worked, or at least worked at one point. But I'm currently
> testing on Firefox 29 (Aurora, I don't have any updates available yet for
> Beta), and every time I try to install a packaged app from the reviewer
> tools I get "Error code: 400 - Bad Request" in the log.
it used to work - but that was before Android started doing APK wrapping. I don't think the patch to allow reviewer signed apps to actually work on Android has even landed on central yet (that's what Marco is trying to test).

(In reply to Andrew Williamson [:eviljeff] from comment #57)
> (In reply to Lisa Brewster [:adora] from comment #56)
> > It was my understanding that Andrew's certificate installation add-on in
> > Comment 20 worked, or at least worked at one point. But I'm currently
> > testing on Firefox 29 (Aurora, I don't have any updates available yet for
> > Beta), and every time I try to install a packaged app from the reviewer
> > tools I get "Error code: 400 - Bad Request" in the log.
>
> it used to work - but that was before Android started doing APK wrapping. I
> don't think the patch to allow reviewer signed apps to actually work on
> Android has even landed on central yet (that's what Marco is trying to test).
fwiw, this has just worked for me on Aurora

What steps are you using? I've tried on multiple devices, installed the add-on and checked all the boxes, and am still getting the same error when I try to install any packaged app from reviewer tools.

Which APK factory are you using?
Steps to determine
1) about:config
2) search for apk
3) Note the value of 'browser.webapps.apkFactoryUrl'
There was an issue with stage-release until 2:20pm today.
Also, which app are you installing?

In the Marketplace server logs I'm seeing:
z.reviewers:INFO Token provided for app:495338 but was not valid
This is suggesting that the token, which is a single use token, has already been used and is no longer valid. We don't have logs for the happy flow but I could add them and try to get them on prod soon which would allow us to see if, perhaps, the request is happening more than once?

(In reply to Fabrice Desré [:fabrice] from comment #70)
> Can't we get sample apps signed by the marketplace and install them?
Ok, we can't test the install origin stuff though.
I guess we need to wait for bug 880043, that is adding signed apps tests.

(In reply to Marco Castelluccio [:marco] from comment #71)
> (In reply to Fabrice Desré [:fabrice] from comment #70)
> > Can't we get sample apps signed by the marketplace and install them?
>
> Ok, we can't test the install origin stuff though.
> I guess we need to wait for bug 880043, that is adding signed apps tests.
I don't know what I'm being needinfo'd for

(In reply to Marco Castelluccio [:marco] from comment #76)
> I think we're mixing two problems here, so I've filed bug 989806.
I suspect bug 989806 is actually the only remaining issue, so I built a debug build with that patch applied and tested installation on stage, where it failed at "package open/read error: INVALID_SIGNATURE".
But that patch doesn't include certs for stage, only dev and production, so the failure is expected. Once I get access to dev (currently blocked by bug 990265), or if someone can give me access to production, I'll test there.

Ok, I tested on production with the first packaged app in the queue, and I see "package open/read error: INVALID_SIGNATURE" there as well, so bug 989806 is apparently *not* the only remaining issue. I'll keep investigating.

I just built Fennec from the Central branch with the latest patch on bug 989806, set the dom.mozApps.user_reviewer_certs pref to true, and successfully installed the first five packaged apps that are marked as Firefox for Android-compatible in the marketplace.firefox.com "apps" queue.
So I think bug 989806 is actually sufficient to resolve this bug, at least now. Perhaps other necessary fixes have landed since the last time I tested, or maybe I tested incorrectly.
I did see one case in which Marketplace told me that the app failed to install. logcat revealed that the APK Factory service response was only two bytes long. But reloading the Marketplace page and reattempting the install succeeded in retrieving and installing a valid APK.

(In reply to Austin King [:ozten] from comment #81)
> myk - can you take a peak at these and let me know if there is anything
> obviously wrong with them?
I was able to "adb install", launch, and run test.apk on a build that has the fix for bug 989806 applied, although logcat reported "[JavaScript Error: "example.com:443 uses an invalid security certificate." (which is a different issue). Subsequent launches stalled on a white screen, but that too seems like a different issue.
I was also able to install fb_test.apk after uninstalling test.apk, although that app doesn't appear to have the same name, and I couldn't figure out its name via "aapt dump", which complained "ERROR getting 'android:name' attribute", so I wasn't able to test launching and running it.

Now that blocking bug 989806 has been fixed, this should similarly be fixed, and installing apps from the reviewer details page should work on today's nightly build of Fennec.
Note that you need to use about:config to create the boolean pref dom.mozApps.use_reviewer_certs and set it to true in order to enable the functionality implemented in bug 989806.