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

Allow UAs to maintain zero or multiple permission stores. #95

Closed
wants to merge 4 commits into from

Conversation

jyasskin
Copy link
Member

@jyasskin jyasskin commented Apr 29, 2016

Each origin now gets its own store, and may have several to model
tab-specific or realm-specific grants.

Fixes #84 and fixes #86.

@raymeskhoury @jan-ivar @annevk, here's an attempt to handle temporary permissions by leaving the selection of where to read and write permission-grants up to the UA. What do you think?

https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/jyasskin/permissions/multiple-stores/index.bs

Each origin now gets its own store, and may have several to model
tab-specific or realm-specific grants.

Fixes w3c#84 and fixes w3c#86.
<li>
Let <var>store</var> be some <a>permission store</a> associated with the
<a>same origin</a> as <var>settings</var>. This store SHOULD be chosen
consistently for a given <a>environment settings object</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to simply state that the browser picks the permission store associated with the current realm on the understanding that this is the finest granularity we support?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say that on the presumption that there is only one permission store that is relevant to the request and that there is always one: either an immutable default, or one at any of the many different scopes that stores might appear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that works. I shied away from it because @annevk didn't want to have per-realm stores at all, but once we allow them, requiring them in the spec doesn't actually require implementations to store them separately.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wouldn't necessitate a per-realm store, just that every real has a single store that is authoritative for that realm.

Or is that really the case? Or is it that there is a hierarchy of stores for a given realm and that looking for a permission requires checking each in turn? If the realm has a store created by storing permissions for permission "A" and it queries for permission "B", which is not yet stored in the realm. It could have a permission for "B" stored in a per-origin store, though. This all suggests that you either have a chain of stores to check, or that you have a store for a realm that might be initially populated with information from other stores.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinthomson With an initially populated realm-store, wouldn't it need to update if the end-user grants persistent permission to its origin, and does so from, say, another tab or through some central UX?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. That implies to me that a synthesis is needed.

Wow, this got complex pretty fast.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current PR gives UAs enough flexibility to do this right:

The UA may initialize new permission stores as empty or as a copy of the mappings of any other permission store associated with the same origin.

and

Let stores be a UA-selected subset of the permission stores … Write a mapping … into each permission store in stores

The UA would initialize realm-store/"B" from origin-store/"B", and if "B" is updated globally from another realm, it would just select the subset as all realm-stores + the origin-store.

I can drop the requirement for a per-realm store and just replace the "SHOULD be chosen consistently" from my original text with MUST, which will probably simplify it a bit.

@martinthomson
Copy link
Member

The storage model is a little wonky here. The key to the store is the name, rather than the name and the full descriptor. That means I can't give a site access to one specific camera, I have to store permissions for ALL cameras. I'm sure that's just a mistake, or it's being addressed elsewhere, but it seems like a very important condition.

{{PermissionStorage}} instance or one of its subtypes.
A <dfn export>permission store</dfn> maps {{PermissionName}}s to instances
of {{PermissionStorage}} or one of its subtypes. A <a>permission store</a>
has one or more associated <a>origin</a>s, and SHOULD have only one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last sentence could use some elaboration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added an example of the domain-suffix grants. Is that what you meant?

@jyasskin
Copy link
Member Author

The storage model is a little wonky here. The key to the store is the name, rather than the name and the full descriptor. That means I can't give a site access to one specific camera, I have to store permissions for ALL cameras. I'm sure that's just a mistake, or it's being addressed elsewhere, but it seems like a very important condition.

@martinthomson It's being addressed in #91: we can't use the descriptor as the key because descriptors are information on searching for a particular capability, rather than the capability itself. For example, MediaTrackConstraints has all sorts of attributes of the camera or mic, but we'd only store the camera or mic id in the permission store. "push" and "midi" (#90), on the other hand, have a lattice structure on their permissions, where requesting {name:'push',userVisibleOnly:false} should imply getting {name:'push',userVisibleOnly:true}, and again storing the descriptor would do the wrong thing.

@jyasskin
Copy link
Member Author

jyasskin commented Jun 8, 2016

After discussion in http://www.w3.org/2016/06/08-webappsec-minutes.html, we're going with #97.

@jyasskin jyasskin closed this Jun 8, 2016
@jyasskin jyasskin deleted the multiple-stores branch July 13, 2016 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Model temporary permissions better Why is "retrieve the permission storage" not in "Permission store"?
4 participants