Add support for creating and removing virtual authenticators#7760
Add support for creating and removing virtual authenticators#7760shs96c merged 6 commits intoSeleniumHQ:masterfrom
Conversation
|
This is ready to review @barancev |
|
|
shs96c
left a comment
shs96c
left a comment
There was a problem hiding this comment.
Hi! Thank you so much for this. It looks great! Thank you also for the nudge on the Slack channel :) I've been heads-down at work, so that was genuinely helpful.
I've added a few comments, and how we proceed is up to you. If you have the time and energy, I'd really appreciate you responding to them (particularly the one about moving the marker interface to the one class that implements it right now) If not, please let me know, and I'll land this when I have some space to address the points myself.
| public class VirtualAuthenticatorOptions { | ||
|
|
||
| enum Protocol { | ||
| CTAP2("ctap2"), U2F("ctap1/u2f"); |
This patch adds support for the Virtual Authenticators WebDriver API, allowing developers to write E2E tests that exercise the WebAuthn API. At the moment, the API is only supported on Chrome 79 onwards. Fixes SeleniumHQ#7753 [1] https://w3c.github.io/webauthn#sctn-automation
nsatragno
left a comment
nsatragno
left a comment
There was a problem hiding this comment.
Thank you for your review, rebased and addressed comments below:
Another thing I changed is instead of defining the test as a java_test_suite, I used +java_selenium_test_suite with browsers = ["chrome", "edge", "ie", "safari"], so we can run this in other browsers as they support it.
| public class VirtualAuthenticatorOptions { | ||
|
|
||
| enum Protocol { | ||
| CTAP2("ctap2"), U2F("ctap1/u2f"); |
Oh I'll be happy to take care of the comments and I'm planning a follow up to add the implementation for the other methods, too. I didn't want my first patch to the project to be too big though :) |
|
Thank you very much for the cleanups. This is great. Merging with pleasure and joy! Thank you! |
|
I want to note that a marker interface requires a driver to return a special capability. Should we report an issue to chromedriver to add a new read-only capability? |
|
Agreed, filed a bug in the spec. |
Ok, so w3c/webauthn#1351 and w3c/webauthn#1353 are wrt webauthn spec. How will the chromedriver implementers (and also those of other webdriver impls) learn about the new extension capability buried in the webauthn spec? Is there a canonical registry of WebDriver extensions that we can add VirtualAuthenticators to ? |
Presumably those implementing the WebAuthn WebDriver API would read it from the webauthn spec since that's where the API is defined in the first place. Since the capability is defined as an extension capability, matching it is optional for webdriver implementers [1], so it shouldn't be a problem if they don't. Not matching it would mean it's not supported, which is the correct behaviour. At least that's my understanding of the spec, it's very possible I got something wrong! :) |
|
that all makes sense. I'm just begging the question of how WebDriver implementors become aware of various webdriver extensions, including webauthn's. I suppose advertising it on [email protected] is one way to do so. |
5 participants
This patch adds support for the Virtual Authenticators WebDriver API,
allowing developers to write E2E tests that exercise the WebAuthn API.
At the moment, the API is only supported on Chrome 79 onwards.
Fixes #7753
Xin the preceding checkbox, I verify that I have signed the Contributor License Agreement -- Google already has a CLA on file with Selenium.