- From: Jeffrey Yasskin <notifications@github.com>
- Date: Tue, 29 Nov 2022 12:10:32 -0800
- To: w3c/permissions <permissions@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/permissions/pull/390/review/1198215539@github.com>
@jyasskin commented on this pull request.
> + </dd>
+ <dt>
+ A <dfn data-dfn-for="powerful feature" class="export">permission key comparison algorithm</dfn>:
+ </dt>
+ <dd>
+ <p>
+ Takes two [=permission keys=] and returns a boolean that shows whether the two keys are equal.
+ If unspecified, this defaults to the <a>default permission key comparison algorithm</a>.
+ </p>
+ <p>
+ The <dfn class="export">default permission key comparison algorithm</dfn>,
+ given [=permission keys=] |key1| and |key2|, runs the following steps:
+ </p>
+ <ol class="algorithm">
+ <li>
+ If |key1| is not an [=origin=] or |key2| is not an [=origin=] return false.
These references are broken, but it might be webref's fault: https://github.com/w3c/webref/issues/807.
> + <p>
+ To <dfn class="export">get a permission store entry</dfn> given a {{PermissionDescriptor}} |descriptor| and [=permission key=] |key|, run these steps:
+ <ol class="algorithm">
+ <li>
+ If the user agent's [=permission store=] [=list/contains=] an [=entry=] whose [=permission store entry/descriptor=] is |descriptor|, and whose [=permission store entry/key=] [=permission key/is equal to=] |key| given |descriptor|, return that entry.
+ </li>
+ <li>
+ Return null.
+ </li>
+ </ol>
+ </p>
+ <p>
+ To <dfn class="export">set a permission store entry</dfn> given a {{PermissionDescriptor}} |descriptor|, a [=permission key=] |key|, and a [=permission/state=] |state|, run these steps:
+ <ol class="algorithm">
+ <li>
+ Let |newEntry| be a new [=permission store entry=] whose [=permission store entry/descriptor=] is |descriptor|, and whose [=permission store entry/key=] [=permission key/is equal to=] |key| given |descriptor|, and whose [=permission store entry/state=] is |state|.
I think this is just
```suggestion
Let |newEntry| be a new [=permission store entry=] whose [=permission store entry/descriptor=] is |descriptor|, and whose [=permission store entry/key=] is |key|, and whose [=permission store entry/state=] is |state|.
```
> + </dd>
+ <dt>
+ A <dfn data-dfn-for="powerful feature" class="export">permission key comparison algorithm</dfn>:
+ </dt>
+ <dd>
+ <p>
+ Takes two [=permission keys=] and returns a boolean that shows whether the two keys are equal.
+ If unspecified, this defaults to the <a>default permission key comparison algorithm</a>.
+ </p>
+ <p>
+ The <dfn class="export">default permission key comparison algorithm</dfn>,
+ given [=permission keys=] |key1| and |key2|, runs the following steps:
+ </p>
+ <ol class="algorithm">
+ <li>
+ If |key1| is not an [=origin=] or |key2| is not an [=origin=] return false.
I feel like if this ever returns false, we've mixed up the types in a bad way in this spec. Shouldn't we only be comparing permission keys that came from the same kind of descriptor?
> @@ -278,6 +278,71 @@ <h3>
reset because its [=permission/lifetime=] has expired.
</p>
</section>
+ <section>
+ <h3>
+ Permission Store
+ </h3>
+ <p>
+ The user agent maintains a single <dfn class="export">permission store</dfn> which is a [=/list=] of [=permission store entries=].
+ </p>
+ <p>
+ The user agent MAY remove [=entries=] from the [=permission store=] when their respective [=permission=]'s [=permission/lifetime=] has expired.
+ </p>
+ <p>
+ A <dfn class="export" data-local-lt="entry">permission store entry</dfn> is a [=tuple=] of {{PermissionDescriptor}} <dfn class="export" data-dfn-for="permission store entry">descriptor</dfn>, [=permission key=] <dfn class="export" data-dfn-for="permission store entry">key</dfn>, and [=permission/state=] <dfn class="export" data-dfn-for="permission store entry">state</dfn>.
It seems like [=key=] has to be an instance of the descriptor's permission key type. The definition of "permission key" waves at this, but it can't be precise without taking a parameter.
> @@ -278,6 +278,71 @@ <h3>
reset because its [=permission/lifetime=] has expired.
</p>
</section>
+ <section>
+ <h3>
+ Permission Store
+ </h3>
+ <p>
+ The user agent maintains a single <dfn class="export">permission store</dfn> which is a [=/list=] of [=permission store entries=].
It seems like you maintain the invariant that a particular (descriptor,key) pair can only appear in this list once. Does that mean it should be a map from those pairs to their states?
> + Takes an [=environment settings object=], and returns a new [=permission key=].
+ If unspecified, this defaults to the <a>default permission key generation algorithm</a>.
+ A feature that specifies a custom [=powerful feature/permission key generation algorithm=] MUST also specify a
+ [=powerful feature/permission key comparison algorithm=].
+ </p>
+ <p>
+ The <dfn class="export">default permission key generation algorithm</dfn>,
+ given an [=environment settings object=] |settings|, runs the following steps:
+ </p>
+ <ol class="algorithm">
+ <li>
+ Return |settings|'s [=environment/top-level origin=].
+ </li>
+ </ol>
+ <aside class="note" title="Permission Delegation">
+ Most powerful features grant permission to the top-level origin and delegate access to the requesting document via Permissions Policy.
```suggestion
Most powerful features grant permission to the top-level origin and delegate access to the requesting document via [[[webappsec-permissions-policy]]].
```
> @@ -628,9 +741,11 @@ <h3 id="reading-current-states">
</li>
</ol>
</li>
- <li>If there was a previous invocation of this algorithm with the same |descriptor| and
- |settings|, returning |previousResult|, and the user agent has not received <a>new
- information about the user's intent</a> since that invocation, return |previousResult|.
+ <li>Let |key| be the result of [=powerful feature/permission key generation algorithm|generating a permission key=] with |settings|.
```suggestion
<li>Let |key| be the result of [=powerful feature/permission key generation algorithm|generating a permission key=] for |descriptor| with |settings|.
```
I think?
> @@ -628,9 +741,11 @@ <h3 id="reading-current-states">
</li>
</ol>
</li>
- <li>If there was a previous invocation of this algorithm with the same |descriptor| and
- |settings|, returning |previousResult|, and the user agent has not received <a>new
- information about the user's intent</a> since that invocation, return |previousResult|.
+ <li>Let |key| be the result of [=powerful feature/permission key generation algorithm|generating a permission key=] with |settings|.
+ </li>
+ <li>Let |entry| be the result of [=get a permission store entry|getting a permission store entry=] with |descriptor| and |key|.
+ </li>
+ <li>If |entry| is not null, return a {{PermissionState}} enum value from |entry|'s [=permission store entry/state=] and |entry|'s [=permission store entry/descriptor=]'s {{PermissionDescriptor/name}}.
How's does the enum value's computation depend on the descriptor's name?
> </p>
<ol class="algorithm">
<li>
- <a>Queue a global task</a> on the [=user interaction task source=] to run that
- feature's [=powerful feature/permission revocation algorithm=].
+ <a>Queue a global task</a> on the [=user interaction task source=] to:.
You didn't introduce this, but I missed the fact that [global tasks need to take a global object as a parameter](https://html.spec.whatwg.org/multipage/webappapis.html#queue-a-global-task) to decide where to run. But there may not be a global when the user revokes permission: they can do it from the browser's settings with no pages open. And we probably don't need a task to remove permission store entries anyway.
> + </li>
+ <li>
+ [=list/Append=] |newEntry| to the user agent's [=permission store=].
+ </li>
+ </ol>
+ </p>
+ <p>
+ To <dfn class="export">remove a permission store entry</dfn> given a {{PermissionDescriptor}} |descriptor| and [=permission key=] |key|, run these steps:
+ <ol class="algorithm">
+ <li>
+ [=list/Remove=] the [=entry=] whose [=permission store entry/descriptor=] is |descriptor|, and whose [=permission store entry/key=] [=permission key/is equal to=] |key| given |descriptor|, from the user agent's [=permission store=].
+ </li>
+ </ol>
+ </p>
+ <p>
+ A <dfn class="export">permission key</dfn> has the type returned by the feature's [=powerful feature/permission key generation algorithm=].
Say something about what the permission key type means. I think it's the granularity at which users get to grant or deny permissions?
> + This is useful for features that want to restrict permissions based on additional context,
+ such as double-keying on both the embedded origin and the top-level origin.
This isn't really "restricting permissions", it's making finer- or coarser-grained choices about them.
--
Reply to this email directly or view it on GitHub:
https://github.com/w3c/permissions/pull/390#pullrequestreview-1198215539
You are receiving this because you are subscribed to this thread.
Message ID: <w3c/permissions/pull/390/review/1198215539@github.com>
Received on Tuesday, 29 November 2022 20:10:45 UTC