-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add permissions-policy check for publicKey credentials #182
Conversation
If the `CredentialRequestOptions` supplied to the Request a Credential algorithm contains a object named `publicKey` then check that the responsible document is allowed to use the publickey-credentials-get policy-controlled feature.
index.bs
Outdated
7. Run the following steps [=in parallel=]: | ||
7. Let |key| be the result of [=map/getting the key=] of |options|. | ||
|
||
8. If |key| is {{CredentialRequestOptions/publicKey}} then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add web OTP
https://wicg.github.io/web-otp/#sctn-permissions-policy
This mechanism is a bit of a pain to extend to other credential types and somewhat defeats the purpose of the credential types registry. Can we add the permission policy names to the registry, and then iterate those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, agreed. Although I suggest we do not attempt to do this in this PR, and we instead:
- submit an issue about this. ( done: see issue Add "policy controlled feature tokens" column to "Credential Types Registry" #184 )
- address issue #184 in a separate PR after both this PR and PR 181 (which creates the "Credential Types" registry) land.
I am thinking that we ought to land this PR before we land PR #181 so the latter PR can incorporate these changes (by rebasing onto main branch) because the changes in this PR are less invasive than those in PR 181. |
I resolved the conflicts with main branch using the github UI (which did a merge-from-main it seems (sorry Nina if that was the wrong thing to do)). I think this PR is ready to merge modulo the three outstanding questions (above) I have for @nina @jyasskin: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging into your feature branch won't cause any problems since the repo is configured to only allow squash-merges back to main
.
Co-authored-by: Jeffrey Yasskin <[email protected]>
Thanks @jyasskin for the corrections. I think this PR is now ready to land. Then, we need to complete PR #181, and then address issue #184 (as outlined in this #182 (comment) above). |
SHA: 307bd77 Reason: push, by @equalsJeffH Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
If the
CredentialRequestOptions
supplied to the Request a Credential algorithm contains a object namedpublicKey
then check that the responsible document is allowed to use the publickey-credentials-get policy-controlled feature.This PR supersedes PR #138.
Fixes #136
Improves #135
Preview | Diff