w3c / geolocation

W3C Geolocation API
https://www.w3.org/TR/geolocation/
81 stars 56 forks source link

Restrict interfaces to secure contexts? #25

Closed foolip closed 5 years ago

foolip commented 5 years ago

In Intent to Implement and Ship: Remove [NoInterfaceObject] from Geolocation API interfaces @mikewest asks:

We limit usage of the Geolocation API to secure contexts. While we don't mark navigator.geolocation as [SecureContext], it seems reasonable to mark these new interfaces as such. Is that a change we could bring into the spec?

This seems reasonable to me. However, a hindrance is that the spec currently doesn't mention secure contexts at all. http://w3c.github.io/geolocation-api/#dom-navigatorgeolocation-getcurrentposition doesn't even seem to have a blanket "reject the request if you feel like it" allowance that some specs do, seemingly requiring the API to work on non-secure contexts.

To just add [SecureContext] would leave things in a pretty strange state, so perhaps the algorithms should be modified at the same time to reject requests for non-secure contexts.

annevk commented 5 years ago

Why would adding it leave things in a strange state?

foolip commented 5 years ago

Oh, I was influenced by thinking about https://github.com/w3c/deviceorientation/issues/47 here, but there aren't any events involved here. (I thought we'd end up firing events but not exposed interfaces if taking the spec literally.)

marcoscaceres commented 5 years ago

This change shouldn't be controversial, as no engine allows usage of geolocation outside of a secure context... the "strange state" is that the implementation is exposed in non-secure contexts. I guess we could add a note about that, but still add [SecureContext].

annevk commented 5 years ago

I'm not sure I follow what is being proposed at this point. [SecureContext] does affect exposure of whatever it is applied on. If you don't want that, you need to use prose instead.

foolip commented 5 years ago

There are three options that I can see:

annevk commented 5 years ago

It seems we cannot add [SecureContext] to Geolocation or GeolocationPositionError unless we add it everywhere, right? Returning instances of interfaces that are not exposed (and not supposed to be available) would be quite strange.

I'd support adding it to the remainder ASAP though and if we can get away with not exposing navigator.geolocation that'd be ideal.

foolip commented 5 years ago

What remains would be PositionOptions, GeolocationPosition and GeolocationCoordinates. Adding [SecureContext] to just those would be fine, but would call for a "this is silly for historical reasons" comment.

Adding [SecureContext] everywhere would indeed be ideal, but probably breaks something. In a quick search in httparchive, I was surprised to not find any such cases quickly, most finds were libraries that do handle navigator.geolocation not existing correctly. So it would at least take more research to prove this undoable, or someone can prove it possible by doing it :)

marcoscaceres commented 5 years ago

let’s start with the non-risky SecureContext additions. The limited API does no harm in legacy HTTP only sites, so maybe it’s ok just to leave it for now as to prevent potential breakage.

marcoscaceres commented 5 years ago

Sent https://github.com/w3c/geolocation-api/pull/34 for review...