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

Describe the permission store using constraints instead of a full model. #96

Closed
wants to merge 4 commits into from

Conversation

jyasskin
Copy link
Member

Here's a third attempt at modeling the permission store, as an alternative to #95, based on @raymeskhoury's suggestions.

This says that each realm has a permission store that's a set of entries for each capability, or a flag saying that capability is denied. The UA gets to ignore writes if it wants and can post tasks to update a realm's store when it "receives new information about the user’s intent", but each particular capability can add constraints on these updates.

Preview at https://api.csswg.org/bikeshed/?url=https://raw.githubusercontent.com/jyasskin/permissions/set-permission-store/index.bs#permission-stores

@alvestrand, this removes the "request a permission" entry point, because it's now straightforward to read and write the current permission store directly. We might want to add a "prompt the user to choose between several options" to match what you're using in getUserMedia() step 7. The old algorithm didn't return a MediaStream anyway as step 8 expects. I think that should be a separate change.

@martinthomson @annevk

Fixes #84 and fixes #86.

@alvestrand
Copy link
Collaborator

@jyasskin your PermissionStorageEntry stores permission names. This won't do - it has to store permission descriptions. Otherwise, it's impossible to store "you have permission to access microphone A, but not microphone B".

I didn't read any further than this.

@alvestrand
Copy link
Collaborator

A "realm" is a concept I can't remember seeing referenced normatively in a W3C DOM-related spec before. I suspect that you are thinking of something completely specific that links in some way to top level browsing context, but it would be better to say which one you're thinking of.

@jyasskin
Copy link
Member Author

@alvestrand "Permission entries for a PermissionName are instances of that PermissionName's permission entry type, which will be PermissionStorageEntry [which is just a name] or one of its subtypes." It's up to the 'microphone' spec to define what data it needs in order to identify a microphone. You'll be able to look at the Bluetooth spec for how to do this, although it's not updated for this patch yet.

Realms are used in https://html.spec.whatwg.org/multipage/webappapis.html#realms-settings-objects-global-objects, and increasingly across the whole HTML spec. They're 1-1 with global objects and settings objects, which are per-frame rather than per-top-level-browsing-context. Because not all browsers and permissions work by top-level-browsing-context, I can't tie the base spec to those, but one of the extra constraints the media specs add can be that an iframe's realm should only get permission entries that were added by other realms with the same origin & top-level-origin.

The UA may initialize <a>realm</a>'s <a>permission store</a> almost
arbitrarily, subject to the constraints in <a
href="#permission-store-constraints"></a>. The UA may also <a>queue a
task</a> to update a <a>realm's</a> <a>permission store</a> almost
Copy link
Member Author

Choose a reason for hiding this comment

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

Making the UA queue tasks to update the realm's permission store gives a guarantee that it doesn't change within a single turn/microtask checkpoint. However, that's not so valuable since functions like query() and getCurrentLocation() operate in parallel and so could be affected by a task posted while they're executing.

@raymeskhoury has suggested that we fall back to having read and write return completely UA-determined results. I think that's plausible. It would make it harder to write things like "the permission store only changes if the UA receives new information about the user's intent" here, but I think (e.g. in the media capture spec) "the UA must not return cameras entries from 'read the permission store' unless the user has explicitly given permission for the origin to access them" still works.

What do folks think?

@annevk
Copy link
Member

annevk commented May 13, 2016

Instead of realm, I think it would be better to put this on either the settings object or the global object. Let's leave realms to JavaScript. We only use realms in HTML to define where the settings objects live and where the global object comes from.

@jyasskin
Copy link
Member Author

OK, that's easy. Thanks for the direction @annevk. So you like the rest of the structure? Do you have any other constraints UAs should follow?

<p>
The UA may store <a>permission entries</a> more persistently than a
<a>realm</a>, but this specification doesn't constrain how.
</p>
Copy link
Member

Choose a reason for hiding this comment

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

It should at least be constrained to origins. Cannot allow new kind of cookies. And ideally we do make it possible for certain permissions to require something more here. E.g., persistent storage isn't really useful without origin-wide permission.

@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 set-permission-store 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