Closed Bug 1058319 Opened 5 years ago Closed 5 years ago

Add a geolocation-noprompt permission

Categories

(Firefox OS Graveyard :: Runtime, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0+, firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S3 (29aug)
blocking-b2g 2.0+
Tracking Status
firefox32 --- wontfix
firefox33 --- wontfix
firefox34 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

Attachments

(2 files, 2 obsolete files)

That will let certified apps use the geolocation api without prompting the user. This is needed for FMD.
Attached patch geolocation-noprompt.patch (obsolete) — Splinter Review
Attachment #8478694 - Flags: review?(anygregor)
Attached patch geolocation-noprompt.patch (obsolete) — Splinter Review
Tested by setting geolocation-noprompt in the camera app and checking that the geolocation was included in the picture's meta-data.
Attachment #8478694 - Attachment is obsolete: true
Attachment #8478694 - Flags: review?(anygregor)
Attachment #8478721 - Flags: review?(anygregor)
Comment on attachment 8478721 [details] [diff] [review]
geolocation-noprompt.patch

Review of attachment 8478721 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8478721 - Flags: review?(anygregor) → review+
Adding Jonas and Andrew so they are aware that we made this change.
Flags: needinfo?(overholt)
Flags: needinfo?(jonas)
[Blocking Requested - why for this release]:
This is a very low risk change that improves the security of the fmd app (see https://bugzilla.mozilla.org/show_bug.cgi?id=1058330#c1).
blocking-b2g: --- → 2.0?
Comment on attachment 8478721 [details] [diff] [review]
geolocation-noprompt.patch

Review of attachment 8478721 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/apps/src/PermissionsTable.jsm
@@ +38,5 @@
>                               certified: PROMPT_ACTION
>                             },
> +                           "geolocation-noprompt": {
> +                             app: PROMPT_ACTION,
> +                             privileged: PROMPT_ACTION,

Shouldn't these two be set to DENY? Not that important, but seems like the right thing to do.
(In reply to Jonas Sicking (:sicking) from comment #7)
> Comment on attachment 8478721 [details] [diff] [review]
> geolocation-noprompt.patch
> 
> Review of attachment 8478721 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/apps/src/PermissionsTable.jsm
> @@ +38,5 @@
> >                               certified: PROMPT_ACTION
> >                             },
> > +                           "geolocation-noprompt": {
> > +                             app: PROMPT_ACTION,
> > +                             privileged: PROMPT_ACTION,
> 
> Shouldn't these two be set to DENY? Not that important, but seems like the
> right thing to do.

We can do that if you really care, but I didn't think of any reason do make it different from the default geolocation one.
silently treating geolocation-noprompt as geolocation seems bad. So yeah, I think we should DENY.

(Separately, I wish that we rejected installing any apps that ask for a permission which has DENY for the apps app-type, but that's of course separate)
Flags: needinfo?(jonas)
sorry had to backout this change since i guess this caused https://tbpl.mozilla.org/php/getParsedLog.php?id=46751241&tree=B2g-Inbound
Flags: needinfo?(overholt)
blocking-b2g: 2.0? → 2.0+
Attached patch patch v2Splinter Review
I changed the permissions to deny for non-certified apps, and fixed an issue we had with substitute permissions in the reverse table. We were overriding the non-substituted version and end up with a bad value in isExplicitInPermissionsTable().
Attachment #8478721 - Attachment is obsolete: true
Attachment #8480877 - Flags: review?(anygregor)
Attachment #8480877 - Flags: review?(anygregor) → review+
Requires Fabrice's patch
Attachment #8480888 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c108c1bcce79
https://hg.mozilla.org/mozilla-central/rev/e7529ddd2e28
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S3 (29aug)
You need to log in before you can comment on or make changes to this bug.