Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define restrictions on device-info permission. #380

Closed
jan-ivar opened this issue Aug 4, 2016 · 20 comments
Closed

Define restrictions on device-info permission. #380

jan-ivar opened this issue Aug 4, 2016 · 20 comments

Comments

@jan-ivar
Copy link
Member

jan-ivar commented Aug 4, 2016

(Follow-up from #372) Why have a list-devices permission separate from persistent gum permission?

It used to mean persistent gum permission OR active gum, but we've since broken out the OR active gum part, so it's now identical to persistent gum permission, except it's a separately settable permission AFAICT (which is not a neutral change).

It's not clear when or where this permission gets set or whether it gets persisted or whether it can be set differently from the other permission. If it's persisted then that affects the behavior of enumerateDevice labels and the devicechange event in ways that differ from how it used to be (which was only during active gum or after persistent gum permission basically).

@alvestrand
Copy link
Contributor

The reason I broke it out as a separate permission is because gUM permissions can be device linked, so that there can be many of them, while the list-devices permission is always across all devices; it seemed cleaner to me to describe it as a separate permission.

We also have use cases where apps will want to list devices before deciding which device to ask permission to use; we don't have any API that allows this to happen yet, but I thought it was cleaner to describe it like this so that an API can be added later.

At the moment, it is described as being set whenever a persistent gUM is set. Clearing it is not described; it would be consistent to clear it when all data for the origin is cleared.

@jan-ivar
Copy link
Member Author

jan-ivar commented Aug 9, 2016

@alvestrand But it's not the same thing only cleaner, it's different, and I don't remember any discussions on it, therefore I think it only fair to remove it until such discussion has happened.

The old language already covered multiple devices: "If none of the local devices are attached to an active MediaStreamTrack in the current browsing context, and if no persistent permission to access these local devices has been granted to the page's origin", (then no label).

This meant a page would stop seeing labels at a well-defined point: when the user revoked permission for the last device a page has access to, or the script's last active stream ended, whichever happened later. This clarity has disappeared.

At the moment, it is described as being set whenever a persistent gUM is set.

Could you point me to the specific language?

If we add an API later, shouldn't we discuss any permissions it may need at that time?

@alvestrand
Copy link
Contributor

alvestrand commented Aug 10, 2016

Hm. I have a mismatch between the specs. gUM uses "list-devices", but permissions uses "device-info".

Setting the device-info permission was part of my original November 16 proposal (the doc attached is still at https://docs.google.com/document/d/13c4hTlm2XgVYpxfGL1a8fcvI1CAUdIgd662DfElk_ow/edit#heading=h.fb7kn49jp9ff ) - but I seem to have missed it on my various document updates - the text is now in neither document. I'll fix that.

The original github bug was #305

About the "stop seeing labels at a well-defined point" - the reason labels were hidden was a fingerprint concern. Once the fingerprint is taken (by exposing the label), I simply don't see the point of hiding them again (short of a full reset of info for that origin). It just makes life more complicated.

@jan-ivar
Copy link
Member Author

I guess what bugs me is that nowhere in #319 is there any mention or discussion of these behavioral changes. Its subject, "Text incorporating the Permissions API's definition of permission handling", makes it sound like a behaviorally neutral editorial PR. It makes me wonder if anyone noticed.

Whatever happened to arguing for a change before it can be adopted?

the reason labels were hidden was a fingerprint concern. Once the fingerprint is taken (by exposing the label), I simply don't see the point of hiding them again (short of a full reset of info for that origin). It just makes life more complicated.

Oh so list-devices is persisted? That's a bigger change than I thought.

This affects devicechange events as well, btw.

@jan-ivar
Copy link
Member Author

So is the motivating driver here to create a permission that no user will bother to reset?

@alvestrand
Copy link
Contributor

If you can find a specified behavior for persistence of the right to list names on entities in the spec before November 2015, feel free to point it out. In my opinion, we've gone from "if you open a device, you'll get access to the label (and then magic happens)" to "if you open a device, a permission is created so that you'll have access to the label, and later handling of that permission is done according to the rules for permissions".

I don't call that a behavioral change.

@jan-ivar
Copy link
Member Author

If you can find a specified behavior for persistence of the right to list names on entities in the spec before November 2015, feel free to point it out.

Here from September 25th is what I already quoted above: ("If none of the local devices are attached to an active MediaStreamTrack in the current browsing context, and if no persistent permission to access these local devices has been granted to the page's origin, let filteredList be a copy of resultList, and all its elements, where the label member is the empty string.")

To paraphrase, this, to me, says the only way for an origin to persistently get labels is to persistently have access to gUM. If the user doesn't persist, then labels effectively appear only during active gUM calls.

This language was discussed between February and September of 2015 in #142. It's specific and implementable, and we implemented it in Firefox during that time period.

Having already implemented it, we're naturally sensitive to any changes, since it's more work to go and re-implement it, and would like to see that any subsequent behavioral changes are intentional, thoroughly discussed, beneficial, and not an accident.

@stefhak
Copy link
Contributor

stefhak commented Aug 17, 2016

Way back we discussed if labels survive a page reload (in the case no persistent permission exist). There were arguments that once the application has once gotten the labels there is no point in hiding them (I may have made that argument myself), but as it is recorded in https://www.w3.org/Bugs/Public/show_bug.cgi?id=22214 we decided that labels should not be available after a reload.

There is a lot more discussion on the list than what is captured in the bug itself.

@alvestrand
Copy link
Contributor

My recollection is that the discussion concluded that there was no reason to say that the sequence

getUserMedia() => permission granted, stream S
enumerateDevices() => with labels
S.allTracks.stop()
enumerateDevices() =>

should return a list of devices without labels - the cat is already out of the bag.
(In the same vein, having ondevicechange not fire after stopping the tracks seems silly too, but I forgot about that when discussing the other bug).

Once reload is done, we're on a new page.

@stefhak
Copy link
Contributor

stefhak commented Aug 17, 2016

My recollection was the same as yours @alvestrand (and it even went further: reload means we're at the same origin which could already have saved labels -> no point in hiding labels).

But looking at https://www.w3.org/Bugs/Public/show_bug.cgi?id=22214 we seem to have concluded that "all devices have stopped" means that labels should be hidden when doing enumerateDevices().

I don't know if we have discussed (and come to another conclusion) after closing 22214.

@jan-ivar
Copy link
Member Author

Thanks @stefhak for digging that up. https://www.w3.org/Bugs/Public/show_bug.cgi?id=22214 and #142 agree, so there seems to be no (written record of any) change in conclusion between September 2014 and September 2015. I liked the conclusion because it was implementable, so I think that's why it won.

I'll add that, that as with geolocation, where getting a user's location now, vs. getting it always, is different, since users move around, similarly users may add new USB devices over time, so having predictable revocation of this access seems desirable. Same with ondevicechange which can be used in social engineering attacks.

All that said, I think @alvestrand has a point that clearing the labels after .stop() even in the same session seems unnecessary (it just provokes people to write polyfills to counteract this behavior).

I also just this week discovered a problem with it, which is that it can be used to detect when garbage collection happens, something DOM APIs should avoid. So we should perhaps let labels survive .stop() at least in the current session (i.e. until navigation).

I see a couple of options for addressing when things are cleared:

  1. Let labels survive .stop() in the current session, and keep 2014-2015 behavior,
  2. list-devices permission (though I have questions about when it's cleared),
  3. Use the existence of a persisted device id (cleared with cookies).

@stefhak
Copy link
Contributor

stefhak commented Aug 18, 2016

Let labels survive .stop() in the current session, and keep 2014-2015 behavior,

That seems like a contradiction since the spec then said
If none of the local devices are attached to an active MediaStreamTrack in the current browsing context <snip> [make] label member is the empty string.

@jan-ivar
Copy link
Member Author

@stefhak sorry I forgot: 4. Leave as is and live with GC detection.

@jan-ivar
Copy link
Member Author

With 1. I meant, revert spec to 2014-2015 behavior _and_ modify it to let labels survive until the end of session once they're activated.

@alvestrand
Copy link
Contributor

Information that may matter:

At the moment, the permissions spec has mutated to say that the result of reading a permission is "the UA's current understanding of the user's intent" - there is no restriction on how the UA gathers information about the user's intent, and there is no guarantee for persistence - this is an UA matter.
The only guarantee of consistency is that if a query is re-tried, and the UA has no new information about the user's intent, the same value is returned.

The spec says "This is intentionally vague about the details of the permission UI and how the UA infers user intent. UAs should be able to explore lots of UI within this framework."

If we choose alternative 2, we relax the constraints on implementations here a bit.

Note: In a conversation with Jeffrey Yaskin this week, he suggested that we could describe the device access permissions as "stronger" than list-devices - that is, it's in the spec that the permission is granted whenever a device access is granted, but the spec will also permit list-devices when there's no device open if that's taken to be the user's intent.

This makes fewer guarantees about behavior than previously. It might make sense.

@jan-ivar
Copy link
Member Author

That might work. Sites already know if they see labels or not, but I suppose it makes sense for sites to want to query whether devicechange events may fire or not (even though they could infer that from whether they see labels).

@jan-ivar
Copy link
Member Author

If nothing is plugged in then there are no labels to check.

@jan-ivar jan-ivar changed the title Remove redundant list-devices permission. Define restrictions on device-info permission. Oct 11, 2016
@alvestrand
Copy link
Contributor

Is this OK once #422 is merged?

@jan-ivar
Copy link
Member Author

@alvestrand I think it also depends on w3c/permissions#131. What needs to happen there?

@alvestrand alvestrand assigned stefhak and unassigned alvestrand Apr 6, 2017
@jan-ivar
Copy link
Member Author

jan-ivar commented Aug 4, 2017

w3c/permissions#131 was merged, so closing this.

@jan-ivar jan-ivar closed this as completed Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants