KEYCLOAK-13319 Use newest WebDriver/Selenium for the WebAuthn testing by mabartos · Pull Request #8534 · keycloak/keycloak

mabartos · 2021-10-06T09:22:05Z

This is a prototype for the WebAuthn testing, which use features from Selenium 4. Virtual Authenticators work as expected in the WebAuthn tests.

@pskopek @Pepo48 Could you please take a look at it?

mposolda

@mabartos Thanks for the work! I did not review in details, but just added a comment to WebAuthnPasswordlessFactory class

mabartos · 2021-10-08T12:04:50Z

I need to fix some additional tests.

mabartos · 2021-10-11T13:16:20Z

I edited test cases and this PR is ready to review.

mposolda

@mabartos This is great work! Thanks for it! I have few more concerns and added some additional comments:

Does this PR work in the pipeline already?
Any update for HOW-TO-RUN.md ? Or is it expected that this does not work yet in the community? HOW-TO-RUN.md has some notes related to WebAuthn, which are surely not valid anymore with these changes, so some update to this document will be nice in this PR.
It seems WebAuthnRegisterAndLoginTest tests only single configuration of WebAuthn authenticator? Is it a plan to extend this to run with multiple various WebAuthn configurations (EG. BLE, NFC and others...). Or is it a plan to do it as a follow-up?

mabartos · 2021-10-15T07:16:47Z

@mposolda Thank you very much for your comments!

I'm trying to integrate those tests, at least temporarily, to the pipeline, but I'm quite struggling with those custom packages. I haven't used the GH packages before and I need to investigate it more. Did you try to build the module in testsuite? Were you able to fetch those packages?
I'll update the HOW-TO-RUN.md file for WebAuthn tests.
There are missing a lot of test cases for WebAuthn to be considered as properly verified feature. I've created a few follow-up tasks for it and I resolved some, but I need to wait for this PR to be merged. Please, see epic WebAuthn Automated testing.

mposolda · 2021-10-18T18:11:11Z

@mposolda Thank you very much for your comments!

* I'm trying to integrate those tests, at least temporarily, to the pipeline, but I'm quite struggling with those custom packages. I haven't used the GH packages before and I need to investigate it more. Did you try to build the module in testsuite? Were you able to fetch those packages?

No, I did not yet tried it. I guess that if it runs in pipeline, it can be considered that it works?

One thing, which I am not sure, is using these credentials to download your privately build packages. I don't have much experience with this approach and not 100% sure whether it is good practice. However if @miquelsi or @pdrozd are fine with this approach and approves this PR, I am fine as well :-)

* I'll update the `HOW-TO-RUN.md` file for WebAuthn tests.

That will be nice, Thanks!

* There are missing a lot of test cases for WebAuthn to be considered as properly verified feature. I've created a few follow-up tasks for it and I resolved some, but I need to wait for this PR to be merged. Please, see epic [WebAuthn Automated testing](https://issues.redhat.com/browse/KEYCLOAK-13131).

Will it be useful for you to have this PR merged to be able to continue with the follow-up work? From my point of view, this PR is ready to go once:

Minor typo in BTE is fixed (See inline comment)
Assume is removed (See inline comment)
Pavel or Miquel approve this PR. Especially I am not sure about the part with the custom packages. I am adding them as reviewers to confirm whether they are ok with this approach.

I consider that other things (HOW-TO-RUN update, commented SigningInTest, follow-up test tasks etc) can be done as a follow-up as long as there are JIRA for them in the "WebAuthn testing epic" .

mabartos · 2021-10-19T07:56:39Z

@mposolda Thanks! I'm trying to properly execute those tests in pipeline, which ensures us that everything works correctly. I'm still investigate it and then I'll notify you about it.

vanrar68 · 2021-11-23T10:39:13Z

Selenium 4 is now officially available. Apparently this version is required if we want to test WebAuthn LoginLess authentication scenarios (resident key and user verification set to 'required'). I'll add the tests for #7860 as soon as this is merged

mabartos · 2021-11-23T13:05:37Z

@vanrar68 I'm working on a WebAuthn testsuite, but it seems, there won't be any other major changes in this PR, so you can follow up on this PR. Moreover, probably we'll be able to properly execute those tests in a pipeline soon, therefore this PR will be merged soon.

mabartos · 2021-12-03T16:32:55Z

@mposolda Pipeline tests passed: https://keycloak-jenkins.com/job/universal-test-pipeline-server/1033/console

mabartos · 2021-12-04T12:08:44Z

The GH actions failures are not related to this PR.

vanrar68 · 2021-12-05T14:35:23Z

Hi! it seems webauthn tests are failing on Windows platform because chrome 64bits webdriver are not available on remote storage (no matter which version of chrome is installed on the machine performing the tests). Google seems to publish 32bits driver exclusively for Windows platform. It used to work nicely with old webauthn test framework though:

Caused by: org.jboss.arquillian.drone.webdriver.binary.downloading.source.MissingBinaryException: There wasn't found any binary with the key matching regex \Q96.0.4664.45/chromedriver_win64.zip\E in the storage: https://chromedriver.storage.googleapis.com/ at org.jboss.arquillian.drone.webdriver.binary.downloading.source.XmlStorageSource.getReleaseForVersion(XmlStorageSource.java:157) at org.jboss.arquillian.drone.webdriver.binary.downloading.source.XmlStorageSource.getLatestRelease(XmlStorageSource.java:82) at org.jboss.arquillian.drone.webdriver.binary.downloading.source.XmlStorageSource.getLatestRelease(XmlStorageSource.java:75) at org.jboss.arquillian.drone.webdriver.binary.handler.AbstractBinaryHandler.downloadAndPrepare(AbstractBinaryHandler.java:175) at org.jboss.arquillian.drone.webdriver.binary.handler.AbstractBinaryHandler.checkAndSetBinary(AbstractBinaryHandler.java:61) ... 12 more

vanrar68 · 2021-12-05T18:19:44Z

Hi! it seems webauthn tests are failing on Windows platform because chrome 64bits webdriver are not available on remote storage (no matter which version of chrome is installed on the machine performing the tests). Google seems to publish 32bits driver exclusively for Windows platform. It used to work nicely with old webauthn test framework though:

I managed to make the webauthn tests work on Windows platform by adding the -Dwebdriver.chrome.driver=C:/path/to/chromedriver.exe option to the maven command line. Could be interesting to add to the HOW-TO-RUN.md

mabartos · 2021-12-06T07:43:18Z

@vanrar68 Thanks for your information. I'll create a new issue for Arquillian Drone and note the workaround to the HOW-TO-RUN.md

mposolda

@mabartos Thanks for all your work on this PR!

@vanrar68 Thanks for the additional testing and verifications!

mabartos · 2021-12-08T11:58:06Z

@vanrar68 The issue for that was created. See arquillian/arquillian-extension-drone#300

mposolda requested changes Oct 6, 2021

View reviewed changes

Comment thread ...keycloak/authentication/authenticators/browser/WebAuthnPasswordlessAuthenticatorFactory.java Outdated

mabartos force-pushed the KEYCLOAK-13319_WEB_AUTHN branch 2 times, most recently from 1151967 to dc56d09 Compare October 6, 2021 15:47

stianst assigned mposolda Oct 7, 2021

mabartos force-pushed the KEYCLOAK-13319_WEB_AUTHN branch from dc56d09 to 0df34a4 Compare October 11, 2021 12:19

mposolda requested changes Oct 14, 2021

View reviewed changes

mposolda requested review from miquelsi and pdrozd October 18, 2021 18:13

mposolda mentioned this pull request Nov 16, 2021

KEYCLOAK-16134 Allow webauthn idless login flow #7860

Merged

mabartos added 2 commits December 3, 2021 13:31

KEYCLOAK-13319 Use newest WebDriver/Selenium for the WebAuthn testing

50b6e29

Resolve some issues with dependencies

a3f8e7d

mabartos force-pushed the KEYCLOAK-13319_WEB_AUTHN branch from 0df34a4 to a3f8e7d Compare December 3, 2021 12:34

Update WebAuthn section in HOW-TO-RUN.md

9646ca1

Documentation and code polishing

ab710bb

mposolda approved these changes Dec 6, 2021

View reviewed changes

mposolda merged commit 12fe5e0 into keycloak:main Dec 6, 2021

Conversation

mabartos commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mabartos commented Oct 8, 2021

Uh oh!

mabartos commented Oct 11, 2021

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mabartos commented Oct 15, 2021

Uh oh!

mposolda commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mabartos commented Oct 19, 2021

Uh oh!

vanrar68 commented Nov 23, 2021

Uh oh!

mabartos commented Nov 23, 2021

Uh oh!

mabartos commented Dec 3, 2021

Uh oh!

mabartos commented Dec 4, 2021

Uh oh!

vanrar68 commented Dec 5, 2021

Uh oh!

vanrar68 commented Dec 5, 2021

Uh oh!

mabartos commented Dec 6, 2021

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

mabartos commented Dec 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mabartos commented Oct 6, 2021 •

edited

Loading

mposolda commented Oct 18, 2021 •

edited

Loading