w3c / permissions

Permissions API
https://www.w3.org/TR/permissions/
Other
106 stars 51 forks source link

Model temporary permissions better #86

Closed jyasskin closed 8 years ago

jyasskin commented 8 years ago

Right now, we have a single permission store that has basically one entry per permission per origin. This isn't rich enough to model UAs that can grant permission during the lifetime of a single tab. So, what model should we have instead?

I believe the basic cross-browser idea is that each environment settings object (tab visiting a particular page) has some grants and denials for various permissions ("local storage"). Each origin (roughly) also has some grants and denials stored in the browser as a whole ("global storage"). It may also be possible in some cases to continue using an object-capability (a MediaStream or series of watchPosition callbacks) even though no permission is stored even locally.

So the question is how to specify all of this flexibility.

alvestrand commented 8 years ago

I don't agree that this is the model .... the permissions store is keyed by origin, not by session. So the current spec is only the global store from your description. It would be interesting to investigate the concept of a session-keyed store in addition to the origin-keyed store. But I don't think it exists in the spec now.

jyasskin commented 8 years ago

Yeah, sorry for the lack of clarity. I'm trying to figure out the model we ought to put in the spec. The model that's currently in the spec is wrong.

jan-ivar commented 8 years ago

Unfortunately, the poor terminology used in this spec casts doubt over any consensus achieved so far.

My advice would be to shore up the existing language and terminology first, to salvage consensus, before attempting to add additional complexity to this.

Clear up the existing definitions in the document to reflect that they are persistent permissions as written, so that we have a minimum shared basis on what has been agreed to so far.

jyasskin commented 8 years ago

I see browsers doing 3 distinct things with a series of calls to navigator.geolocation.watchPosition().

  1. The user is prompted on the first call from an origin, and either by default (Chrome) or if they select an option in that prompt (Firefox and Safari), they aren't prompted again on subsequent calls from any tab visiting that origin. Each call produces a series of position callbacks that ends if the site drops the capability using clearWatch().
  2. The user is prompted on the first call from a tab, and they aren't prompted again on subsequent calls from that tab, but they are prompted again from other tabs visiting the same origin. Each call produces a series of position callbacks until the site drops the capability using clearWatch(). (Safari)
  3. The user is prompted on every single call to navigator.geolocation.watchPosition(), even within the same tab, but each accepted call produces an unbounded series of position callbacks while the site holds onto the resulting capability. (Firefox)

I'd like to specify algorithms in each capability for updating its permission storage, and have that work for all of the above behaviors.

Right now, the spec asks each capability to define a "permission request algorithm", which shows any necessary prompt and if the user accepts updates the stored permission data. You can see an example of this in "bluetooth". The surrounding "request a permission" algorithm handles actually reading and writing the permission data to storage. Having the surrounding algorithm handle reading and writing storage is good for generalizing to tab-specific storage, and there's actually a straightforward path to get geolocation generalized (just write the updated permission data to a UA-dependent list of storages), but this doesn't work for more complex permissions like "bluetooth" and probably "camera".

With "bluetooth", each prompt results in adding one or more devices to a list of allowed devices. If we have both tab-specific and origin-wide storages, a tab could wind up with a superset of the origin's devices in its storage, and a prompt that's set to "always allow" a device will need to result in adding that device to two different starting lists. Because the current permission request algorithm both shows the prompt and updates the list, we can't call it multiple times, once for each permission store. We need to separate it into "prompt" and "modify storage" steps.

My current guess at the right way to structure that is to define «update the permission storage storage for "permission name" using the following steps: 1. …» so that it runs the steps multiple times on a UA-dependent set of permission-stores. Then the permission request algorithm and permission revocation algorithm would call that to grant or revoke any permissions they need to, rather than modifying their input storage.


It's possible this is all over-complicated. I thought of having the storage for each capability just hold a set of "things", so that "add thing-1" could add it to multiple permission stores if the UA wants, and "remove all things matching this predicate" could do the same. However, I don't think that works well: if the user grants {name:"push", userVisibleOnly: false} locally to one tab, and then {name:"push", userVisibleOnly: true} globally from another one, we don't want to overwrite the first tab's invisible-push permission. (Don't object that we wouldn't write a global grant to other tab-local stores; then we just grant {name:"push", userVisibleOnly: false} globally and get the same problem on the global store.) We could treat the two userVisibleOnly values as separate "things", but that just pushes the complexity down into the capability specs: they start having to search for all the relevant "things" and merge overlapping ones.

jyasskin commented 8 years ago

Origin-wide storage handles case (1), and realm/settings-object/tab-specific storage handles case (2), but I didn't address case (3). I think there we put an object-capability in each realm that controls whether it's allowed to access the capability. e.g. each call to watchPosition would result in a separate object-capability for geolocation. Then we write that the UA MAY neuter object-capabilities when the user says to or when a permission is revoked.

raymeskhoury commented 8 years ago

What do we think the benefits will be of defining storage behaviors in detail in the spec, especially if there are 3 different cases that vary across UA? I actually think that in the long run UAs will have more variations. As an alternative, what if the permission store was a UA-defined black box that returned whatever the UA wanted for any particular request?

Edit: One disadvantage of including this in the spec is that the size and the complexity of the spec are increased substantially.

jyasskin commented 8 years ago

The benefit in my mind is that page authors will know what to expect when they do certain things, instead of having to rely on outside-of-the-spec documentation. e.g. two consecutive calls to query(x) are likely to return the same value, and it's especially unlikely that the first would return "prompt" while the next returns "granted".

That said, it's true we're not guaranteeing anything. At most we're specifying likelihoods here, and that's not the usual domain of specifications. @annevk and @alvestrand, since you're trying to use this infrastructure in other specs, what do you think of @raymeskhoury' suggestion?

annevk commented 8 years ago

I don't think that works. While we should allow some freedom what becomes part of the key, we shouldn't allow all the things. E.g., if I have two tabs open for a given URL, those need to have access to the same permissions. There need to be some reasonable guarantees to developers, otherwise the whole system is meaningless.

And we need to make it clear that origin is at least part of that key, to preserve the same-origin policy. Furthermore, we need to define which origin that is and where it's obtained from, again, to preserve the same-origin policy and guarantee some consistency across user agents.

raymeskhoury commented 8 years ago

Thanks @annevk. I definitely agree with you that from a web developers perspective guarantees are good. However in this case, different browsers have different opinions, so it's hard to guarantee the way things should work in detail. I'm a bit worried that by defining in detail how persistent permissions work, temporary permissions work, etc. (which aren't used consistently across browsers) that the spec will become hard to follow and potentially overspecified in subtle ways.

if I have two tabs open for a given URL, those need to have access to the same permissions

But this isn't necessarily true for some browser with some permissions (e.g. camera/mic on FF) and this is the intended behavior.

There need to be some reasonable guarantees to developers, otherwise the whole system is meaningless

Maybe it's good to define and agree on what these guarantees are at a high level first? This might allow us to simplify what's put into the spec.

This is probably hitting on some of the same things as the discussion in #83.

annevk commented 8 years ago

But this isn't necessarily true for some browser with some permissions (e.g. camera/mic on FF) and this is the intended behavior.

If the permission is granted just once, it's not stored. It'll always be "prompt". I think that's consistent.

jan-ivar commented 8 years ago

Right. If persistent permission is 'granted' for the origin, then every tab from that origin benefits equally.

Introducing granularity below origin (except potentially double-keyed with iframe origin) seems like a mistake to me. I propose we expand our states instead. See https://github.com/w3c/permissions/issues/93#issuecomment-215431087.

jyasskin commented 8 years ago

We have 3 possible outcomes when a user grants permission to access a feature:

  1. Firefox: The site gets an object-capability providing access to that feature, but any future request would prompt again. I'd call this {state: 'prompt', access: obj}. (I've made up my mind compared to my comment earlier.) @jan-ivar appears to want to call this {state: 'allowed temporarily', access: obj}.
  2. Safari: The site gets an object-capability providing access to that feature, any future request in this tab would auto-grant, and any future request in another tab would prompt again. I'd call this {state: 'granted', access: obj}. @jan-ivar appears to want to call this {state: 'allowed temporarily', access: obj} also.
  3. Chrome: The site gets an object-capability providing access to that feature, and any future request in any tab would auto-grant. I'd call this {state: 'granted', access: obj}. @jan-ivar appears to want to call this {state: 'allowed', access: obj}.

Safari's the example where "two tabs open for a given URL" can have different granted permissions.

We do still have the guarantee that two newly-opened tabs on the same origin would start with the same granted permissions. Is that enough to be worth specifying?

jan-ivar commented 8 years ago

@jyasskin thanks for the summary. Two questions: Is state just the return value, or also the new value from subsequent query()ies? If the latter, and the store is per-origin as @annevk suggests, then with the values you suggest in your example, if I have two same-origin Safari tabs, wouldn't they both see 'granted' even though one of the tabs would still trigger prompts?

alvestrand commented 8 years ago

@jyasskin seems to me that your tuple is consistent with the current spec as long as we say that the current spec specifies the "state:" part of the tuple only - except that you have the 'state: granted' as a new value of the state variable. This is the same thing I tried in #63, but later decided not to ask for. I think.

jyasskin commented 8 years ago

@jan-ivar

  1. The {state: 'prompt', access: obj} strings I showed would be the return value from permissions.request() or any other request function in the platform, like a revamped getCurrentLocation(). The access: obj part would vary by permission. I think subsequent query()s within the same Realm would tend to return the same state value, although the UA is free to change that based on new information it receives.
  2. The Safari store is not just per-origin, regardless of what we might want it to be. It has per-Realm state.

@alvestrand Could you elaborate a bit? I don't understand "'state: granted' as a new value of the state variable": 'granted' is an existing value for PermissionState, which is the type of the state attribute. The current editors' draft does only define the state: field in the dictionary (not tuple), and leaves any other fields up to the individual permission.

alvestrand commented 8 years ago

@jyasskin I meant a new value of the https://w3c.github.io/permissions/#enumdef-permissionstate enum, which is part of the dictionary. I regard the tuple as the underlying abstraction and the dictionary as the WebIDL implementation; sorry if I sounded confused.

jan-ivar commented 8 years ago

The Safari store is not just per-origin, regardless of what we might want it to be. It has per-Realm state.

@jyasskin I think you're reading too much into what looks to me like Safari merely caching this state in-session (Hitting Run in a jsfiddle is enough to erase it). More revealing, I think, is that if I check "Remember this choice for a day", then it applies to all tabs from that origin. To me, this points to origin-level storage, which I think we should standardize around.

Safari's only exposure of site-specific settings that I could find were under Preferences/Security/Plug-in Settings..., where I can control things like Flash per website (origin, I suspect). Seems fairly regular.

jyasskin commented 8 years ago

Hitting 'Run' in a jsfiddle must create a new Realm. A cache is storage, and this one has user-visible effects, so we can't just ignore it in the spec.

"Remember this choice for a day" is persistent storage (with a scheduled revocation, which is just as allowed as manual revocation), matching both Chrome's and Firefox's persistent models, which are both origin-level storage. That doesn't make Safari's temporary model the same as Firefox's.

raymeskhoury commented 8 years ago

I think another way to frame this discussion is: "what are the permissible ways to scope a permission decision?". There are many dimensions to scoping a decision, e.g. 1) The lifetime of the reference to the object-capability returned (e.g. a single call to getUserMedia). 2) The lifetime of the tab to which the grant occurred. 3) A URL scope (e.g. domain, origin, path etc.). 4) Whether the grant applies in embedded contexts as well as top level contexts.

Each browser scopes the decision differently in most of these dimensions. To me there's not an approach that is clearly best or has consensus. Part of which approach is best depends on the UI for permission management in the browser.

The only restriction that is clear and has consensus in my mind is that a decision shouldn't be scoped more broadly than the origin of the requesting document. It seemed complicated to try to model all the other reasonable possibilities in the spec which is one reason I felt hesitant about it.

jan-ivar commented 8 years ago

I see @jyasskin 's point now, which is that temporary settings (what I'd like to call 'allowed temporarily' and 'blocked temporarily') will typically come from the current browsing session (tab), rather than the origin where persistent settings come from. Not just in Safari, but in Firefox too. It's harder to see in Firefox, if you're just going by whether you get prompted or not - because Firefox prompts relentlessly - but if you look from where I borrowed the terms, the Firefox-user-facing page-info mockups, it seems clear that from a browser user's POV at least, there is interest in seeing both what's been granted temporarily in a given tab, overlaid with what's been granted permanently to the origin. Two tabs of the same origin therefore, may differ in what's shown in their respective page-info dropdowns. Like in the Safari example - hypothetically if Safari had a page-info dropdown - one tab might show 'allowed temporarily' (because of it's local state) while the other would show nothing (aka 'prompt' in that UX design), which is reflective of default the shared origin setting.

My apologies if mixing in browser-user-facing UX is not helpful. I understand that site developers may have different needs from browser-users, and might not even care whether access is permanent or temporary, but I'm thinking it can't hurt, and it seems to solve the "inconsistency bug" @mounirlamouri brought up at least, by giving us unambiguous return values.

jyasskin commented 8 years ago

https://github.com/w3c/permissions/pull/95 is an attempt to improve the storage system to handle temporary permissions. It doesn't say anything at all about what request() should return, but I think it gives us the infrastructure to specify that in a subsequent PR. What do you think?

jyasskin commented 8 years ago

I think #97 got this. Please reopen if you disagree.