+++ This bug was initially created as a clone of Bug #823974 +++ Geolocation should require a manifest entry for all apps, like all other webapi permissions. If that is not present, the request should be denied. This bug is to implement in Android (Aurora, 20) if possible.

Confirmed that this doesn't magically work on Android post the patch in bug 823974. Going to generalize this geolocation and desktop notifications, as the same problem applies to both.

Jonas, if I remember right, we are expecting these prompts, right?

I believe this should be marked INVALID. Per the spreadsheet Lucas Adamski has created to track these things, we treat unprivileged, privileged, and certified apps the same -- all are allowed to do geolocation, and all must prompt when the API is invoked. There are some slight differences about whether we remember your preference, but we should never deny geolocation outright.

(In reply to Bill Walker [:bwalker] [@wfwalker] from comment #3) > I believe this should be marked INVALID. Per the spreadsheet Lucas Adamski > has created to track these things, we treat unprivileged, privileged, and > certified apps the same -- all are allowed to do geolocation, and all must > prompt when the API is invoked. There are some slight differences about > whether we remember your preference, but we should never deny geolocation > outright. No. We changed that requirement per Lucas that an app developer is required to declare geolocation and desktop notification in the permissions property in order to use the API in a web app. I can find the bug that talked about this, but security has made this a firm requirement to allow reviewers to have a clear picture of what permissions are being requested by the app.

Actually, bug 823974 has the rationale here, filed by Lucas himself. I confirmed also this is the current behavior on FF OS not too long ago as well.

Can we generalise this to Desktop also? It would be ideal if permissions didn't diverge too much.

(In reply to Andrew Williamson [:eviljeff] from comment #6) > Can we generalise this to Desktop also? It would be ideal if permissions > didn't diverge too much. Yeah, we should probably get an equivalent bug filed for this for desktop.

I have very very very strong reservations against doing this. Mostly, this seems to move us to the Android/iOS model of permissions? I don't like Android's model, and haven't met anyone who does either, including all of the security folks at Mozilla I've talked to. We've very intentionally spent a lot of time designing something different and better for both the web and apps. Why are we throwing it away?

I'll put needsinfo on Lucas to explain this. Note that this actually did go through intense debate for a while, but I believe the final rationale for going with it was due to the need to allow a reviewer to understand exactly what permissions are being requested by the app and only granting them when they are specified as a result.

Yeah, I read that above. That makes even less sense to me since we're talking about hosted apps for the most part here.

I think the rationale was covered in bug 923974 esp https://bugzilla.mozilla.org/show_bug.cgi?id=823974#c6, but to recap, the model for apps is that all potentially privacy and security impactful permissions need to be enumerated in the manifest, regardless of whether they are implicit or explicit. Geolocation was the sole and confusing exception to that, and that bug fixed that one exception. The exception made little sense since the developer controls the manifest, and if they want to use geolocation they can simply add it like every other permission they want to use.

(In reply to Lucas Adamski from comment #11) > model for apps is that all potentially privacy and security impactful > permissions need to be enumerated in the manifest, regardless of whether > they are implicit or explicit. Yes, I read that part. My question has more to do with this "model", not with whether or not we're going to be consistent. We're moving control from the user to the app developer. The manifest documentation has a great example of this: https://developer.mozilla.org/en-US/docs/Apps/Manifest#permissions i.e. that app requests contacts permissions just so it can do autocomplete in a form field, a feature that I'd gladly opt out of and that apps already have a history of abusing on iOS and Android. Rather than encouraging developers to write apps that will work with or without a permission, we've moved to a model where you can't even try the app without giving it your private info.

(In reply to Wesley Johnston (:wesj) from comment #12) > (In reply to Lucas Adamski from comment #11) > > model for apps is that all potentially privacy and security impactful > > permissions need to be enumerated in the manifest, regardless of whether > > they are implicit or explicit. > > Yes, I read that part. My question has more to do with this "model", not > with whether or not we're going to be consistent. We're moving control from > the user to the app developer. > > The manifest documentation has a great example of this: > https://developer.mozilla.org/en-US/docs/Apps/Manifest#permissions > > i.e. that app requests contacts permissions just so it can do autocomplete > in a form field, a feature that I'd gladly opt out of and that apps already > have a history of abusing on iOS and Android. Rather than encouraging > developers to write apps that will work with or without a permission, we've > moved to a model where you can't even try the app without giving it your > private info. I think you misunderstand how the model works at a fundamental level. The geolocation permission is explicit, so putting the permission in the manifest only allows the app to request geolocation from the user. The user still has to grant it. If anything it gives the user extra control because they can understand what the app *might* request of them before installing it, knowing if its not in the manifest it cannot be accessed implicitly or explicitly. Same for contacts, etc.

Ahh. OK! Great! That makes me much more comfortable. Thanks for the clarification!

(In reply to Jason Smith [:jsmith] from comment #1) > Confirmed that this doesn't magically work on Android post the patch in bug > 823974. > > Going to generalize this geolocation and desktop notifications, as the same > problem applies to both. Does anyone know why this does not magically work on Android? What about this is Android specific?

(In reply to Mark Finkle (:mfinkle) from comment #15) > (In reply to Jason Smith [:jsmith] from comment #1) > > Confirmed that this doesn't magically work on Android post the patch in bug > > 823974. > > > > Going to generalize this geolocation and desktop notifications, as the same > > problem applies to both. > > Does anyone know why this does not magically work on Android? What about > this is Android specific? Hmm...Jonas?

The reason bug 823974 doesn't fix this is because we implemented it there as part of the IPC code, which I don't think Android goes through. In general the situation around prompts on B2G is pretty bad and we've ended up with much more inconsistencies and workarounds than we should have had.

(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #11) > I think the rationale was covered in bug 923974 esp > https://bugzilla.mozilla.org/show_bug.cgi?id=823974#c6, but to recap, the > model for apps is that all potentially privacy and security impactful > permissions need to be enumerated in the manifest, regardless of whether > they are implicit or explicit. Geolocation was the sole and confusing > exception to that, and that bug fixed that one exception. The exception > made little sense since the developer controls the manifest, and if they > want to use geolocation they can simply add it like every other permission > they want to use. This is a completely broken security model which leads to most users clicking the "yea, sure, whatever" button to install the app. A much better model would be to require that the app declare that it might request permission for geolocation and still show the prompt when it it asks. That would give the user better control over their privacy and security decisions as the request comes with some context of what the app is doing.

(In reply to Brad Lassey [:blassey] from comment #18) > A much better model would be to require that the app declare that it might > request permission for geolocation and still show the prompt when it it > asks. That would give the user better control over their privacy and > security decisions as the request comes with some context of what the app is > doing. Isn't that what we're doing when this bug is fixed?? (Like we do already on FirefoxOS)

(In reply to Andrew Williamson [:eviljeff] from comment #19) > (In reply to Brad Lassey [:blassey] from comment #18) > > A much better model would be to require that the app declare that it might > > request permission for geolocation and still show the prompt when it it > > asks. That would give the user better control over their privacy and > > security decisions as the request comes with some context of what the app is > > doing. > > Isn't that what we're doing when this bug is fixed?? (Like we do already on > FirefoxOS) Yup. That's what this bug is fixing. The way the security model works is - No permission specified = automatic denial to WebAPI - Permission specified = follow the security model rules for the specific WebAPI to either automatically allow, prompt for access, or deny.

What Andrew and Jason said.

Comment on attachment 779270 [details] [diff] [review] Patch I'm assuming you are looking for reviewer. Maybe Mark can help with the review?

Comment on attachment 779270 [details] [diff] [review] Patch >diff --git a/mobile/android/components/ContentPermissionPrompt.js b/mobile/android/components/ContentPermissionPrompt.js >- if (result == Ci.nsIPermissionManager.DENY_ACTION) { >+ if (result == Ci.nsIPermissionManager.DENY_ACTION || >+ (result == Ci.nsIPermissionManager.UNKNOWN_ACTION && !!kEntities[request.type])) { No need to wrap the line for android JS

Carrying r+

This made bug 857240 perma-fail. See some of the recent TBPLbot comments there for logs. Backed out. https://hg.mozilla.org/integration/mozilla-inbound/rev/8bd708f9171b

Comment on attachment 782812 [details] [diff] [review] Patch v2 Make a Try run just to be safe

(In reply to Mark Finkle (:mfinkle) from comment #29) > Make a Try run just to be safe Aready did :)