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

Add backup flags to virtual authenticator #1999

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

nsatragno
Copy link
Member

@nsatragno nsatragno commented Nov 20, 2023

Allow setting and changing the backup eligibility (BE) and backup state (BS) flags through the virtual authenticator API.

Fixed: #1987


Preview | Diff

Allow setting and changing the backup eligibility (BE) and backup state
(BS) flags through the virtual authenticator API.

Fixed: w3c#1987
@nsatragno nsatragno self-assigned this Nov 20, 2023
@nsatragno nsatragno marked this pull request as ready for review November 21, 2023 17:12
Copy link
Contributor

@MasterKale MasterKale left a comment

Choose a reason for hiding this comment

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

Thank you for taking this on, @nsatragno, this looks great!

The only thing I could think to suggest is that we call it defaultBackupEligible and backupEligible over ...Eligibility, but looking at the current state of the spec right now we seem to use "eligibility" more often than "eligible" so I don't think I have a strong case here:

Screenshot 2023-11-21 at 2 15 14 PM

Screenshot 2023-11-21 at 2 15 18 PM

Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

Should the new Set Credential Properties be able to set extensions and uvm too?

index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
nsatragno and others added 2 commits November 22, 2023 13:03
emlun's suggestion to change the `it` to be explicit.

Co-authored-by: Emil Lundberg <emil@emlun.se>
Copy link
Member

@emlun emlun left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nsatragno nsatragno assigned pascoej and unassigned pascoej Nov 27, 2023
@nsatragno
Copy link
Member Author

+@pascoej for review from webkit.

@timcappalli timcappalli self-requested a review November 29, 2023 20:14
aarongable pushed a commit to chromium/chromium that referenced this pull request Nov 30, 2023
Allow webauthn's virtual authenticator to to set default and per
credential backup eligibility (BE) and backup state (BS) flags. Expose
this through the devtools API.

Defaults can be set per authenticator using VirtualFidoDevice::State.
This sets the flags for both credentials created by executing the CTAP
make credential command, and credentials injected through State's
Inject* functions.

A future CL will expose this through webdriver.

See w3c/webauthn#1999

Bug: 1504858
Change-Id: Ic53570f9acb2e10396161472bbccc8c39fefa759
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5054633
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1231469}
@nsatragno nsatragno merged commit cf35363 into w3c:main Dec 13, 2023
2 checks passed
@nsatragno nsatragno deleted the virtual_authenticator_backup_flags branch December 13, 2023 20:14
github-actions bot added a commit that referenced this pull request Dec 13, 2023
SHA: cf35363
Reason: push, by nsatragno

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Virtual Authenticator API does not expose a way to set backup-eligible or backup-status flags
5 participants