Added support for vertx-auth-webauth4j

[FroMage]

Requires https://github.com/eclipse-vertx/vertx-auth/pull/692 and https://github.com/vert-x3/vertx-dependencies/pull/197

[FroMage]

This won't pass CI before the two other PRs are merged.

[vietj]

thanks @FroMage

[vietj]

@FroMage you should rebase this PR onto latest master

[vietj]

@FroMage I think we are missing a require static for examples to pass, I know examples should not require a require and I will try to find a solution to remove those later that is global to all projects

[FroMage]

Yeah, I should have compiled before rebasing and pushing :)

[FroMage]

That's not enough, now I get:

[ERROR] Error occurred during initialization of boot layer
[ERROR] java.lang.module.FindException: Module com.fasterxml.jackson.databind not found, required by com.fasterxml.jackson.datatype.jsr310

Perhaps vertx-auth-webauthn4j should declare this module, even though it's actually used by webauthn4j, it probably doesn't have the right dependencies declared since it's an automatic module?

[FroMage]

Oh, probably vertx-auth-webauthn4j should export the webauthn4j module since it's used in the API?

[FroMage]

That's not it either

[FroMage]

Oh, it's the tests, there's a classpath exclusion for jackson-databind when running the tests.

[FroMage]

So, it's due to https://github.com/vert-x3/vertx-web/blob/master/vertx-web/pom.xml#L132-L134 If I comment it out, the tests run. But I suppose that's there for a reason. Any advise @vietj ?

[vietj]

@FroMage I think the databind exclusion comes from https://github.com/vert-x3/vertx-web/commit/494f006722b0e68c5d068dab3576e1a87940fd11

[vietj]

why is jackson databind needed @FroMage ?

[vietj]

I see that webauthn4j has dependency on databind, is that something the project can avoid ?

[FroMage]

I see that webauthn4j has dependency on databind, is that something the project can avoid ?

I suppose not, it's using its JSON and CBOR mapping internally.

[FroMage]

Any news?

[vietj]

it's on my radar :-)

[vietj]

I believe also that webauthn4j now supports explicit java modules

[FroMage]

So, I've added one test, which was not trivial to find out how, but at least it passes :)

I've noticed something annoying though:

WebAuthnImpl.authenticate() (and same for WebAuthn4J) creates User like this:

User user = User.create(authenticator.toJson());

With:

public class Authenticator {

  /**
   * The username linked to this authenticator
   */
  private String userName;
[…]

Note the casing of userName.

The problem is in User:

  /**
   * The user subject. Usually a human representation that identifies this user.
   * <p>
   * The lookup for this information will take place in several places in the following order:
   *
   * <ol>
   *   <li>{@code principal.username} - Usually for username/password or webauthn authentication</li>
   *   <li>{@code principal.userHandle} - Optional field for webauthn</li>
   *   <li>{@code attributes.idToken.sub} - For OpenID Connect ID Tokens</li>
   *   <li>{@code attributes.[rootClaim?]accessToken.sub} - For OpenID Connect/OAuth2 Access Tokens</li>
   * </ol>
   *
   * @return the subject for this user or {@code null}.
   */
  default @Nullable String subject() {
    if (principal().containsKey("username")) {
      return principal().getString("username");
    }
[…]

Where the casing is username vs userName. So… that's likely a bug, but not sure how to solve it, because Authenticator.getUserName() is API, so it'd break users. I can fix the WebAuthn4J module, since that's new, but that won't fix WebAuthn. Or we also support userName in User 🤷

OTOH, apparently, nobody noticed…

[FroMage]

Ah, apparently I broke some other tests by adding a dependency on the webauthn4j test module :-/

Also, other question is related to needing the webauthn4j version in the pom.xml, because I need to depend on the webauthn4j test module, but I don't like that we're duplicating that version between here and vertx-auth but I'm not sure where to put it to avoid that?

[FroMage]

I updated the code for the username case change, and made the webauthn4j test modules static.

When I run the entire test suite, graphql fails with:

[ERROR] Error occurred during initialization of boot layer

But when I launch it with mvn -f vertx-graphql test it passes. Let's see what happens in CI.

[vietj]

I'll have a look, thank you

On Thu, Dec 5, 2024 at 4:17 PM Stéphane Épardaud @.***> wrote:

I updated the code for the username case change, and made the webauthn4j test modules static.

When I run the entire test suite, graphql fails with:

[ERROR] Error occurred during initialization of boot layer

But when I launch it with mvn -f vertx-graphql test it passes. Let's see what happens in CI.

— Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-web/pull/2655#issuecomment-2520602200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCSRYAM2IO44FURQDCD2EBVBLAVCNFSM6AAAAABPWJWII2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRQGYYDEMRQGA . You are receiving this because you were mentioned.Message ID: @.***>

[FroMage]

Well, CI passed :)

[vietj]

yes it did, so we are good to merge ?

[FroMage]

Yup

[vietj]

thanks for the contribution @FroMage this is great!