Reimplement security-webauthn on top of webauthn4j by FroMage · Pull Request #44105 · quarkusio/quarkus

FroMage · 2024-10-25T21:33:20Z

This is following in the footsteps of the new vertx-auth-webauthn4j, which we're not using here because there was no point in converting our config to vertx config which converts to webauthn4j config.

I also took the opportunity to improve the docs and fix the API and endpoints to remove confusion.

As a result, this is not compatible with the previous API, here's the proposed migration guide, which I'll add in the migration notes as soon as we know where we merge this.

== Migration Guide

The quarkus-security-webauthn module has been reimplemented on top of WebAuthn4J. As a result, it is not binary or source compatible with the previous versions, and you must update the following code:

The Authenticator type
- The Authenticator class (from Vert.x) is no longer used, and has been replaced functionally with WebAuthnCredentialRecord. It holds similar data to the previous class, but as it is a subtype of a WebAuthn4J type, accessing its content is slightly different.
- Its replacing type WebAuthnCredentialRecord now has a getRequiredPersistedData() method which returns a RequiredPersistedData record holding all the data you need to persist to your data source (database, or anything persistent), to make it easier.
- The static method WebAuthnCredentialRecord.fromRequiredPersistedData(RequiredPersistedData) allows you to turn your stored data into a WebAuthnCredentialRecord for the opposite operation.
If you had any JPA entities or database tables storing the old Authenticator format of data, you will need to update them to the new set of data. Open an issue if you need help for that.
The WebAuthnUserProvider type that you had to implement has been restructured:
- findWebAuthnCredentialsByUserName() was renamed to findByUserName()
- findWebAuthnCredentialsByCredID() was renamed to findByCredentialId()
- updateOrStoreWebAuthnCredentials was split into the two methods it represented: update(String credentialId, long counter) and store(WebAuthnCredentialRecord credentialRecord)
Default endpoints:
- /q/webauthn/login was renamed to /q/webauthn/login-options-challenge
- /q/webauthn/register was renamed to /q/webauthn/register-options-challenge
- /q/webauthn/callback was split into the two operations it represented: /q/webauthn/login and /q/webauthn/register. These endpoints are now disabled by default (for security reasons) and can be enabled using the quarkus.webauthn.enable-registration-endpoint and quarkus.webauthn.enable-login-endpoint configuration properties.
- /q/webauthn/register now requires a username query parameter.
- /.well-known/webauthn was added to return the list of allowed related origins
- The user name for login and login-options-challenge is now optional
WebAuthnSecurity
- Two methods have been added to WebAuthnSecurity: getLoginOptionsChallenge() and getRegisterOptionsChallenge() to make it easier to call and customise them.
- The user name parameter for login() and loginOptionsChallenge() is now optional
- The register() method now takes an additional userName parameter (due to the removal of the username cookie)
Configuration:
- The quarkus.webauthn.require-resident-key (boolean defaulting to false) setting has been replaced by the quarkus.webauthn.resident-key (enum defaulting to REQUIRED)
- The quarkus.webauthn.challenge-username-cookie-name setting has been removed along with its related cookie.
- The quarkus.webauthn.load-metadata (boolean defaulting to false) setting has been added to control loading of FIDO metadata.
- The quarkus.webauthn.user-presence-required (boolean defaulting to true) setting has been added to control the requirement of user presence.
- The quarkus.webauthn.user-verification setting has changed default from DISCOURAGED to REQUIRED.
- The quarkus.webauthn.timeout default has increased from 1 minute to 5 minutes as specified in the WebAuthn standard
- The quarkus.webauthn.pub-key-cred-params setting has been renamed to quarkus.webauthn.public-key-credential-parameters
- The quarkus.webauthn.origin setting has been renamed to quarkus.webauthn.origins (plural) and now allows more than one origin (as required by spec)
- The new quarkus.webauthn.enable-registration-endpoint boolean configuration (defaults to false) enables the default registration endpoint.
- The new quarkus.webauthn.enable-login-endpoint boolean configuration (defaults to false) enables the default login endpoint.
The verification of WebAuthn credential attestations may differ slightly for settings of quarkus.security.webauthn.attestation other than NONE (the default).
In the quarkus-test-security-webauthn test module:
- The WebAuthnHardware contructor now requires a URL to represent the location of your endpoints, you can obtain in your test classes it using @TestHTTPResource URL url
- The WebAuthnEndpointHelper.invokeRegistration method was renamed to obtainRegistrationChallenge
- The WebAuthnEndpointHelper.invokeLogin method was renamed to obtainLoginChallenge
- The WebAuthnEndpointHelper.invokeCallback method was split into the two operations it represented: WebAuthnEndpointHelper.invokeRegistration and WebAuthnEndpointHelper.invokeLogin
- The WebAuthnEndpointHelper.invokeLogin and WebAuthnEndpointHelper.obtainLoginChallenge user name parameter is now optional
- The WebAuthnEndpointHelper.invokeRegistration method now takes a userName parameter
JavaScript library:
- The registerOnly method has been renamed to registerClientSteps
- The loginOnly method has been renamed to loginClientSteps
- The login and loginClientSteps method's user parameter is now optional
- The constructor's registerPath has been renamed to registerOptionsChallengePath and has the default value /q/webauthn/register-options-challenge
- The constructor's loginPath has been renamed to loginOptionsChallengePath and has the default value /q/webauthn/login-options-challenge
- The constructor's callbackPath has been split into its two components registerPath (default value /q/webauthn/register) and loginPath (default value /q/webauthn/login
- The constructor now accepts a csrf option of type JsonObject with two keys: header for the header name to use to add the CSRF token, and value for the CSRF token value. This is mostly useful when using quarkus-csrf and using custom endpoints that are protected by CSRF (unlike the default endpoints).

FroMage · 2024-10-25T21:35:29Z

Huh, I had completely forgotten I added WebAuthnHelper, I will document it in another PR, I'll open an issue.

FroMage · 2024-10-25T21:36:45Z

And quickstarts are at quarkusio/quarkus-quickstarts#1470

FroMage · 2024-10-25T21:39:10Z

WebAuthnHelper part is #44106

github-actions · 2024-10-25T22:22:07Z

🙈 The PR is closed and the preview is expired.

maxandersen · 2024-10-26T06:07:38Z

Sorry I'm noob in webauthn - does this break users or allow them to coexist ? And what kind of users are affected - Quarkus app devs or/and ext dev?

FroMage · 2024-10-28T14:03:34Z

The compatibility breakage is for users, but the module is in preview so we can break.

FroMage · 2024-10-28T14:04:24Z

Quickstarts break is handled by the PR. The native one I need to look at.

FroMage · 2024-10-28T18:25:05Z

Native should pass now.

michalvavrik · 2024-10-29T17:51:22Z

Will check tomorrow evening.

sberyozkin · 2024-10-30T09:24:03Z

Hi Steph, @FroMage, thanks for this major enhancement/refactoring, I have to do some homework to do a proper review, need to refresh some WebAuthn concepts and check webauthn4j docs, may take a bit of time, but I'm on it, cheers

ynojima · 2024-10-30T07:17:53Z

+                && config.attestation().get() == WebAuthnRunTimeConfig.Attestation.ENTERPRISE) {
+            TrustAnchorAsyncRepository something;
+            // FIXME: make config name configurable?
+            Optional<TlsConfiguration> webauthnTlsConfiguration = certificates.get("webauthn");


WebAuthn attestation certificate is not related to TLS.
Is it appropriate to use TlsConfigurationRegistry for webauthn attestation certificate?

It's definitely not related to TLS, but for some reason the Quarkus certificate registry is called the TLS registry https://quarkus.io/guides/tls-registry-reference
This is just how every certificate list (and trust stores, private, public keys) are configured in Quarkus.
I do agree that the name implies a single purpose that is limiting, @cescoffier

Ha ha, @sberyozkin stopped me from using it from anything that is not TLS when I integrated this registry to OIDC. I agree, it would be nice to have Quarkus certificate registry, but there are things that doesn't make sense when you use it this way. Like handshakeTimeout, alpn and other stuff. I suppose it is fine for your use case because I think users does not configure this, right?

Well, @cescoffier himself told me to use this, to centralise everything 🤷

@FroMage TLS registry entries have properties related to TLS like handshake timeout, host verification options, reload related one, etc. If the attestation is not happening on the TLS path, it can be confusing for Quarkus WebAuthn users having to refer to TLS registry...
TLS registry is a great concept, but perhaps it can be enhanced over time to extend say TrustStoreRegistry instead of having to refer to it on the non-TLS paths

michalvavrik · 2024-10-31T17:24:45Z

Sorry, it has been hectic week I must look later. But since @ynojima and @sberyozkin are going to have a look, I think it's not an issue. Great PR.

…security/webauthn/WebAuthnUserProvider.java Co-authored-by: Sergey Beryozkin <sberyozkin@gmail.com>

…tore

quarkus-bot · 2024-12-05T15:23:53Z

Status for workflow `Quarkus Documentation CI`

This is the status report for running Quarkus Documentation CI on commit db12e82.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

quarkus-bot · 2024-12-05T20:41:20Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit db12e82.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✖	Quickstarts Compilation - JDK 17	`Compile Quickstarts`	Failures	Logs	Raw logs	🚧
✖	Native Tests - Virtual Thread - Messaging	`Build`	⚠️ Check →	Logs	Raw logs	🔍

You can consult the Develocity build scans.

Failures

⚙️ Quickstarts Compilation - JDK 17 #

- Failing: security-webauthn-quickstart security-webauthn-reactive-quickstart

📦 security-webauthn-quickstart

✖ Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project security-webauthn-quickstart: Compilation failure

📦 security-webauthn-reactive-quickstart

✖ Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project security-webauthn-reactive-quickstart: Compilation failure

Flaky tests - Develocity

⚙️ Native Tests - Messaging1

📦 integration-tests/reactive-messaging-kafka

✖ io.quarkus.it.kafka.KafkaConnectorIT.testRequestReply - History

iterable contents differ at index [2], expected: <reply-3> but was: <{"details":"Error id edb8cb3a-bc93-4331-ae9f-aa3fb9ba0561-1","stack":""}> - org.opentest4j.AssertionFailedError

Details

org.opentest4j.AssertionFailedError: iterable contents differ at index [2], expected: <reply-3> but was: <{"details":"Error id edb8cb3a-bc93-4331-ae9f-aa3fb9ba0561-1","stack":""}>
	at io.quarkus.it.kafka.KafkaConnectorTest.testRequestReply(KafkaConnectorTest.java:112)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:788)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

FroMage · 2024-12-06T09:36:29Z

Thanks.

gsmet · 2024-12-06T17:08:05Z

I was going to ask for integrating something in the migration guide but it's already done! Thanks!

These default value was introduced in quarkusio#44105, but the doc was not aligned.

These default values were introduced in quarkusio#44105, but the doc was not aligned.

quarkus-bot Bot added area/dependencies Pull requests that update a dependency file area/documentation area/testing labels Oct 25, 2024

FroMage mentioned this pull request Oct 25, 2024

Updated webauthn code quarkusio/quarkus-quickstarts#1470

Merged

This comment has been minimized.

Sign in to view

FroMage force-pushed the webauthn4j branch from 9a475d0 to 91421c6 Compare October 28, 2024 18:24

This comment has been minimized.

Sign in to view

FroMage requested review from michalvavrik, sberyozkin and ynojima October 29, 2024 16:20

ynojima reviewed Oct 30, 2024

View reviewed changes

ynojima reviewed Oct 31, 2024

View reviewed changes

Comment thread ...s/security-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnSecurity.java Outdated

ynojima reviewed Oct 31, 2024

View reviewed changes

Comment thread ...security-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnController.java Outdated

FroMage mentioned this pull request Nov 4, 2024

WebAuthn: support registration and login when already logged in #44292

Open

ynojima reviewed Nov 4, 2024

View reviewed changes

Comment thread ...s/security-webauthn/runtime/src/main/java/io/quarkus/security/webauthn/WebAuthnSecurity.java Outdated

FroMage force-pushed the webauthn4j branch from 91421c6 to 370c508 Compare November 4, 2024 14:50

This comment has been minimized.

Sign in to view

FroMage and others added 9 commits December 5, 2024 15:52

Reimplement security-webauthn on top of webauthn4j

3575e50

Update extensions/security-webauthn/runtime/src/main/java/io/quarkus/…

f410e54

…security/webauthn/WebAuthnUserProvider.java Co-authored-by: Sergey Beryozkin <sberyozkin@gmail.com>

Update extensions/security-webauthn/runtime/src/main/java/io/quarkus/…

a69385e

…security/webauthn/WebAuthnUserProvider.java Co-authored-by: Sergey Beryozkin <sberyozkin@gmail.com>

Reuse challenge when it is restorable

39b5a78

Take username from query parameter on register endpoint

f7d857f

Remove username cookie

f791c2d

WebAuthn: username cookie removal documentation and JS

b824867

WebAuthn: document the security constraints of WebAuthnUserProvider.s…

34b96fb

…tore

Updated webauthn extension status to experimental

db12e82

FroMage force-pushed the webauthn4j branch from 79cac55 to db12e82 Compare December 5, 2024 14:52

sberyozkin self-requested a review December 5, 2024 16:03

sberyozkin approved these changes Dec 5, 2024

View reviewed changes

sberyozkin added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 5, 2024

sberyozkin merged commit fab189a into quarkusio:main Dec 5, 2024

quarkus-bot Bot added this to the 3.18 - main milestone Dec 5, 2024

quarkus-bot Bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 5, 2024

FroMage deleted the webauthn4j branch December 6, 2024 09:36

michalvavrik mentioned this pull request Dec 9, 2024

Fix daily build by taking Quarkus QE Test Framework dependencies from Sonatype snapshot repository and stop building Quarkus main quarkus-qe/quarkus-test-suite#2243

Merged

9 tasks

This was referenced Dec 13, 2024

WebAuthn: review username query parameter change #45091

Closed

Revisit options challenge endpoints #45132

Merged

Rename options-challenge endpoints to options endpoints #45133

Closed

ynojima added a commit to ynojima/quarkus that referenced this pull request Jan 1, 2025

Update default value in spring-webauthn.adoc

ad6fb3d

These default value was introduced in quarkusio#44105, but the doc was not aligned.

ynojima added a commit to ynojima/quarkus that referenced this pull request Jan 1, 2025

Update default values in spring-webauthn.adoc

1236ccd

These default values were introduced in quarkusio#44105, but the doc was not aligned.

ynojima added a commit to ynojima/quarkus that referenced this pull request Jan 1, 2025

Update default values in spring-webauthn.adoc

8d7a35b

These default values were introduced in quarkusio#44105, but the doc was not aligned.

ynojima mentioned this pull request Jan 1, 2025

Update default values in security-webauthn.adoc #45326

Merged

carlesarnal pushed a commit to carlesarnal/quarkus that referenced this pull request Feb 6, 2025

Update default values in spring-webauthn.adoc

c2d74d1

These default values were introduced in quarkusio#44105, but the doc was not aligned.

Conversation

FroMage commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FroMage commented Oct 25, 2024

Uh oh!

FroMage commented Oct 25, 2024

Uh oh!

FroMage commented Oct 25, 2024

Uh oh!

github-actions Bot commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

maxandersen commented Oct 26, 2024

Uh oh!

FroMage commented Oct 28, 2024

Uh oh!

FroMage commented Oct 28, 2024

Uh oh!

FroMage commented Oct 28, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

michalvavrik commented Oct 29, 2024

Uh oh!

sberyozkin commented Oct 30, 2024

Uh oh!

Uh oh!

Uh oh!

ynojima Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

FroMage Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

michalvavrik Nov 15, 2024

Choose a reason for hiding this comment

Uh oh!

FroMage Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

sberyozkin Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michalvavrik commented Oct 31, 2024

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

quarkus-bot Bot commented Dec 5, 2024

Status for workflow Quarkus Documentation CI

Uh oh!

quarkus-bot Bot commented Dec 5, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status for workflow Quarkus CI

Failing Jobs

Failures

⚙️ Quickstarts Compilation - JDK 17 #

📦 security-webauthn-quickstart

📦 security-webauthn-reactive-quickstart

Flaky tests - Develocity

⚙️ Native Tests - Messaging1

📦 integration-tests/reactive-messaging-kafka

Uh oh!

FroMage commented Dec 6, 2024

Uh oh!

gsmet commented Dec 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

FroMage commented Oct 25, 2024 •

edited

Loading

github-actions Bot commented Oct 25, 2024 •

edited

Loading

Status for workflow `Quarkus Documentation CI`

quarkus-bot Bot commented Dec 5, 2024 •

edited by github-actions Bot

Loading

Status for workflow `Quarkus CI`