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.

Sign up for GitHub

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

Jump to bottom

Use credential type registry for permissions + digital credentials #242

Merged
merged 20 commits into from
Jul 24, 2024
Merged

Use credential type registry for permissions + digital credentials #242

Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3d119ce
Use credential type registry for permissions
marcoscaceres Jul 1, 2024
480c51f
Distiguish get and create
marcoscaceres Jul 1, 2024
ab2f574
Make it a must, will null
marcoscaceres Jul 1, 2024
7a328ad
Update index.bs
marcoscaceres Jul 1, 2024
6f91620
fixup and remove note
marcoscaceres Jul 1, 2024
d6fa875
Update index.bs
marcoscaceres Jul 1, 2024
9b7fc39
Update index.bs
marcoscaceres Jul 1, 2024
ab2c6ea
fixup
marcoscaceres Jul 1, 2024
8f1303b
Update index.bs
marcoscaceres Jul 1, 2024
0e05b94
Update index.bs
marcoscaceres Jul 1, 2024
55bd2b4
Update index.bs
marcoscaceres Jul 1, 2024
7fecb45
Update index.bs
marcoscaceres Jul 1, 2024
121c423
Update index.bs
marcoscaceres Jul 1, 2024
39c66a2
fix whitespace
marcoscaceres Jul 1, 2024
3eb3781
Update index.bs
marcoscaceres Jul 1, 2024
b0f097c
Update index.bs
marcoscaceres Jul 4, 2024
89ad755
Update index.bs
marcoscaceres Jul 22, 2024
0ec907a
fix linking of otp-credentials
marcoscaceres Jul 22, 2024
8aa2974
review feedback
marcoscaceres Jul 22, 2024
a4d1981
review feedback
marcoscaceres Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 48 additions & 36 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,15 @@ spec:css-syntax-3;
</pre>
<pre class="biblio">
{
"FEDCM": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Already in Specref... don't need this... or WEB-OTP

"authors": [ "Sam Goto" ],
"href": "https://fedidcg.github.io/FedCM/",
"title": "FedCM API"
},
"WEB-LOGIN": {
"authors": [ "Jason Denizac", "Robin Berjon", "Anne van Kesteren" ],
"href": "https://github.com/jden/web-login",
"title": "web-login"
},
"WEB-OTP": {
"authors": [ "Sam Goto" ],
"href": "https://wicg.github.io/web-otp/",
"title": "WebOTP API"
"DIGITAL-CREDENTIALS": {
"authors": [ "Marcos Cáceres", "Sam Goto" ],
"href": "https://wicg.github.io/digital-credentials/",
"title": "Digital Credentials"
}
}
</pre>
Expand Down Expand Up @@ -325,42 +320,62 @@ spec:css-syntax-3;
<small>(in alphabetical order)</small></th>
<th><dfn for="credential type registry">Options Member Identifier</dfn></th>
<th><dfn for="credential type registry">Appropriate Interface Object</dfn></th>
<th><dfn for="credential type registry">Get Permissions Policy</dfn></th>
<th><dfn for="credential type registry">Create Permissions Policy</dfn></th>
<th>Specification</th>
<th>Requestor Contact</th>
</tr>
</thead>
<tr>
<td>digital-credential</td>
<td>digital</td>
<td>{{DigitalCredential}}</td>
<td>digital-credentials-get</td>
<td>null</td>
<td>[[DIGITAL-CREDENTIALS]]</td>
<td><a href="https://wicg.io/">WICG</a></td>
</tr>
<tr>
<td>federated</td>
<td>federated</td>
<td>{{FederatedCredential}}</td>
<td>null</td>
<td>null</td>
<td>This specification: [[#federated]]</td>
<td><a href="https://www.w3.org/2011/webappsec/">W3C</a></td>
</tr>
<tr>
<td>identity</td>
<td>identity</td>
<td>{{IdentityCredential}}</td>
<td>[=identity-credentials-get=]</td>
<td>null</td>
<td>[[FEDCM]]</td>
<td><a href="https://www.w3.org/community/fed-id/">W3C</a></td>
</tr>
<tr>
<td>otp</td>
<td>otp</td>
<td>{{OTPCredential}}</td>
<td>otp-credentials</td>
Copy link
Member

Choose a reason for hiding this comment

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

looks like you're missing a <td>null</td> here.

marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
<td>[[WEB-OTP]]</td>
<td><a href="https://wicg.io/">WICG</a></td>
</tr>
<tr>
<td>password</td>
<td>password</td>
<td>{{PasswordCredential}}</td>
<td>null</td>
<td>null</td>
<td>This specification: [[#passwords]]</td>
<td><a href="https://www.w3.org/2011/webappsec/">W3C</a></td>
</tr>
<tr>
<td>public-key</td>
<td>publicKey</td>
<td>{{PublicKeyCredential}}</td>
<td>[=publickey-credentials-get-feature|publickey-credentials-get=]</td>
<td>[=publickey-credentials-create-feature|publickey-credentials-create=]</td>
<td>[[WEBAUTHN]]</td>
<td><a href="https://www.w3.org/blog/webauthn/">W3C</a></td>
</tr>
Expand All @@ -382,6 +397,12 @@ spec:css-syntax-3;
* Each registry entry must state the [=credential type registry/Appropriate Interface Object=] [=identifier=] for the
[=credential type registry/credential type=].

* Each registry entry must state the [=credential type registry/Get Permissions Policy=] [=permission=] used when executing <a abstract-op>Request a `Credential`</a>
for a [=credential type registry/credential type=], or null if no [=Document/permissions policy=] specified.

* Each registry entry must state the [=credential type registry/Create Permissions Policy=] [=permission=] used when executing <a abstract-op>Create a `Credential`</a>
for a [=credential type registry/credential type=], or null if no [=Document/permissions policy=] specified.
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved

* Each registry entry must include a link that references a publicly available specification
defining the [=credential type registry/credential type=] and the [=dictionary member=] [=identifier=].

Expand Down Expand Up @@ -950,8 +971,10 @@ spec:css-syntax-3;

1. Assert: |settings| is a [=secure context=].

1. If |settings|'s [=relevant global object=]'s [=associated Document=] is not [=Document/fully active=],
then return [=a promise rejected with=] "{{NotAllowedError}}" {{DOMException}}.
1. Let |document| be |settings|'s [=relevant global object=]'s [=associated Document=].

1. If |document| is not [=Document/fully active=], then return [=a promise rejected with=]
"{{NotAllowedError}}" {{DOMException}}.

1. If <code>|options|.{{CredentialRequestOptions/signal}}</code> is [=AbortSignal/aborted=],
then return [=a promise rejected with=]
Expand All @@ -965,8 +988,6 @@ spec:css-syntax-3;
1. If |interface| does not support {{CredentialMediationRequirement/conditional}}
[=user mediation=], return [=a promise rejected with=] a "{{TypeError}}" {{DOMException}}.

1. Let |p| be [=a new promise=].
Copy link
Member Author

Choose a reason for hiding this comment

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

Was defined before it was used! oops!


1. For each |interface| in |options|' <a>relevant credential interface objects</a>:

1. If |settings|' [=active credential types=] [=set/contains=] |interface|'s
Expand All @@ -983,21 +1004,14 @@ spec:css-syntax-3;
1. Let |sameOriginWithAncestors| be `true` if |settings| is [=same-origin with its
ancestors=], and `false` otherwise.

1. If |options|[{{CredentialRequestOptions/identity}}] [=map/exists=] and
if |settings|' [=relevant global object=]'s [=associated Document=] is **not**
[=allowed to use=] the [=identity-credentials-get=]
[=policy-controlled feature=] return [=a promise rejected with=] a "{{NotAllowedError}}"
{{DOMException}}.
1. For each |permission| in |options|' [=credential type registry/Get Permissions Policy=]:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be something like:

1. For each |interface| in |options|' <a>relevant credential interface objects</a>:
  1. Let |permission| be the |interface|'s {{Credential/[[type]]}} [=credential type registry/Get Permissions Policy=].
  2. If |permission| is null, continue.
  3. If |document| is **not** [=allowed to use=] |permission|, return 
           [=a promise rejected with=] a "{{NotAllowedError}}" {{DOMException}}.

Otherwise, we're looping through all the permissions regardless of what's in our options object.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh yeah.. oops... 🙈


1. If |options|[{{CredentialRequestOptions/publicKey}}] [=map/exists=] and
if |settings|' [=relevant global object=]'s [=associated Document=] is **not**
[=allowed to use=] the [=publickey-credentials-get-feature|publickey-credentials-get=]
[=policy-controlled feature=] return [=a promise rejected with=] a "{{NotAllowedError}}"
{{DOMException}}.
1. If |permission| is null, continue.

Note: <a const>`password`</a> and <a const>`federated`</a>
[=credential type registry/credential types=] are not presently treated as
[=policy-controlled features=], although this may change in the future.
1. If |document| is **not** [=allowed to use=] the |permission| [=policy-controlled feature=],
return [=a promise rejected with=] a "{{NotAllowedError}}" {{DOMException}}.

marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
1. Let |p| be [=a new promise=].

1. Run the following steps [=in parallel=]:

Expand Down Expand Up @@ -1164,21 +1178,19 @@ spec:css-syntax-3;

1. Let |global| be |settings|' [=environment settings object/global object=].

1. If |settings|'s [=relevant global object=]'s [=associated Document=] is not [=Document/fully active=],
then return [=a promise rejected with=] "{{NotAllowedError}}" {{DOMException}}.
1. Let |document| be the [=relevant global object=]'s [=associated Document=].

1. If |document| is not [=Document/fully active=], then return
[=a promise rejected with=] "{{NotAllowedError}}" {{DOMException}}.

1. Let |sameOriginWithAncestors| be `true` if the [=current settings object=] is [=same-origin
with its ancestors=], and `false` otherwise.

1. If |options|[{{CredentialCreationOptions/publicKey}}] [=map/exists=] and
if |settings|' [=relevant global object=]'s [=associated Document=] is **not**
[=allowed to use=] the [=publickey-credentials-create-feature|publickey-credentials-create=]
[=policy-controlled feature=] return [=a promise rejected with=] a "{{NotAllowedError}}"
{{DOMException}}.
1. Let |permission| be |options|'s [=credential type registry/Create Permissions Policy=].
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Also, at this stage we still have a list of credential types to loop through. It's not until L1203 that we assert there's only one credential.

(Ideally, we'd make the permissions policy check after verifying there's only one credential, but we already shipped this and it's not that big of a deal).

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, yeah... I'm also unsure as to why there's a document check below... it seems unnecessary with the fully active check above.


Note: <a const>`password`</a> and <a const>`federated`</a>
[=credential type registry/credential types=] are not presently treated as
[=policy-controlled features=], although this may change in the future.
1. If |permission| is not null, and |document| is **not** [=allowed to use=]
the |permission| [=policy-controlled feature=], return [=a promise rejected with=]
a "{{NotAllowedError}}" {{DOMException}}.
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved

1. Let |interfaces| be the [=set=] of |options|' <a>relevant credential interface objects</a>.

Expand Down
Loading