usernameless flow demo

[bmd007]

Hi looking at the code in the demo folder, in the start authentication method, when username is null (indicating username less flow), the following happens:

AssertionRequestWrapper request =
          new AssertionRequestWrapper(
              generateRandom(32),
              rp.startAssertion(StartAssertionOptions.builder().username(username).build())) 

in which username is still empty (null) and userHandle is not defined (null again). Which in turn results in the following code being executed with null all over it:

            .allowCredentials(
                OptionalUtil.orElseOptional(
                        startAssertionOptions.getUsername(),
                        () ->
                            startAssertionOptions
                                .getUserHandle()
                                .flatMap(credentialRepository::getUsernameForUserHandle))
                    .map(
                        un ->
                            new ArrayList<>(credentialRepository.getCredentialIdsForUsername(un))))

startAssertionOptions.getUsername() is null, startAssertionOptions.getUserHandle() is empty ====> finally allowCredentials is null.

I think the demo doesn't really support the username less flow. I couldn't find enough documentations explaining how to implement the flow either.

[emlun]

That's what the "Authenticate without username" button in the demo is for. :slightly_smiling_face:

[bmd007]

that button in the demo eventually calls the code stack which I pointed to. So If I got it right, there is a problem with the demo code.

[emlun]

Oh! There is indeed an issue in the demo app, but it's not related to allowCredentials. That issue will be fixed by PR #301.

It is correct that allowCredentials should be empty or absent for usernameless authentication - that is what causes the client to ask available authenticators for any discoverable credential (passkey) instead of one of those listed in allowCredentials (since there are none). But for this you need to register the credential as a discoverable credential (passkey). The button for this in the demo is currently labeled "Register new account with resident credential", or "Add resident credential" if already logged in. This is what didn't work and will be fixed by PR #301.

We might also update the demo button labels to be more in line with the new terminology around passkeys.

[bmd007]

Thanks for the info @emlun

I realised there are still minor details ignored in the main code regarding username less flow. 1- You can't just leave the allowCredentials null. The javaScript side requires a list to be present even if empty. So an empty list is required instead of null. 2- Consequently, during finishAuthentication phase, step 5, the request has allowCredentials (an empty list). The current code tries to find a matching credential between the ones in the request and the ones in the response. Well, there is no credentials in the request (hence the username less ness). So checking the size of the list is also important.

Basically the code needs to be more defensive about lists inside optionals, and their (de-)serialization.

I will try to create a PR, let's see if I can make it.

[bmd007]

I created https://github.com/Yubico/java-webauthn-server/pull/305. Please have a look. The code base was too complicated for me to add tests, so please feel free to edit my PR if you find it any useful. Probably I'm already breaking some formatting rules.

[emlun]

There is no problem here as far as I can tell. The StartAssertionOptions.username(String) method wraps null values in Optional, and Optional.empty() values are handled correctly in FinishAssertionSteps. The changes suggested in #305 do not seem necessary.

[emlun]

With that, I'll consider this resolved. Please let us know if there's more to discuss here.

[bmd007]

With that, I'll consider this resolved. Please let us know if there's more to discuss here.

FinishAssertionStep does not handle the null. In username less flow the UI expects an empty collection for allowCredentials. Not null nor missing the allowCredentials field.

[bmd007]

I think you are not spending enough time looking at the code and actually debugging it. Nor understanding my PR.

[emlun]

Ah, my apologies, now I see what you mean: finishAssertion rejects the credential ID as "unrequested" if you explicitly set StartAssertionOptions.allowCredentials to a present but empty list. I was confused because the issue mostly mentions null values, which the library does handle correctly as far as I'm aware.

The updated test in 1576b3d0756f738e8c892855104fa4cb188faf59 confirms that FinishAssertionSteps incorrectly rejects the assertion when allowCredentials is explicitly set to an empty list. The same commit also fixes the issue. Thanks for the report!

[emlun]

The javaScript side requires a list to be present even if empty.

Just so you know: this is true only of precisely the value null. The JS API permits allowCredentials to be omitted or set to undefined; these are both equivalent to setting allowCredentials: []. See these examples in the Chrome console:

With allowCredentials: null:

> await navigator.credentials.get({
    publicKey: {
        challenge: crypto.getRandomValues(new Uint8Array(32)),
        allowCredentials: null,
    },
})

VM340:2 Uncaught TypeError: Failed to execute 'get' on 'CredentialsContainer': Failed to read the 'publicKey' property from 'CredentialRequestOptions': Failed to read the 'allowCredentials' property from 'PublicKeyCredentialRequestOptions': The provided value cannot be converted to a sequence.
    at <anonymous>:1:29
(anonymous) @ VM340:1

With allowCredentials: undefined:

> await navigator.credentials.get({
    publicKey: {
        challenge: crypto.getRandomValues(new Uint8Array(32)),
        allowCredentials: undefined,
    },
})

PublicKeyCredential {rawId: ArrayBuffer(32), response: AuthenticatorAssertionResponse, authenticatorAttachment: 'platform', id: '1zBfDVjoA_QGE4kKKDNTEdOZryCYF9PsOtsCcm8_5bs', type: 'public-key'}

With allowCredentials omitted:

> await navigator.credentials.get({
    publicKey: {
        challenge: crypto.getRandomValues(new Uint8Array(32)),
    },
})

PublicKeyCredential {rawId: ArrayBuffer(32), response: AuthenticatorAssertionResponse, authenticatorAttachment: 'platform', id: '1zBfDVjoA_QGE4kKKDNTEdOZryCYF9PsOtsCcm8_5bs', type: 'public-key'}

[bmd007]

No worries.

https://github.com/Yubico/java-webauthn-server/commit/1576b3d0756f738e8c892855104fa4cb188faf59 solves one of the issues I mentioned. The read side so to say. The write side still produces a null instead of an empty list.

The current code on master, when username is null, results in allow Credentials becoming null. Thus results in an error in the JS side (as you described yourself).

The PR that I provided before, solves this issue by making sure there will always be a allow Credentials.

[bmd007]

@emlun

The screenshots below show what happens in the username less flow:

You can see that when both username and userHandle are empty optionals (on the java side), the resulting json that reaches frontend (JS) contains a allowCredentials with null as value. Which as you mentioned, results in an error. My PR was making sure that allowCredentials is never an empty optional, but an empty list.

[bmd007]

you probably have some config somewhere in your serialiser which ignores empty optional fields during serialisation. But that's not the norm, simply assuming that is problematic.

---

    public AssertionRequestWrapper startAuthentication() {
        var assertionExtensionInputs = AssertionExtensionInputs.builder()
                .uvm()
                .build();
        var startAssertionOptions = StartAssertionOptions.builder()
                .userVerification(UserVerificationRequirement.PREFERRED)
                .extensions(assertionExtensionInputs)
                .build();
        AssertionRequest assertionRequest = relyingParty.startAssertion(startAssertionOptions);
        var assertionRequestWrapper = new AssertionRequestWrapper(randomUUIDByteArray(), assertionRequest);
        assertRequestStorage.put(assertionRequestWrapper.getRequestId(), assertionRequestWrapper);
        return assertionRequestWrapper;
    }

The code above represents the username less flow. Serialising assertionRequestWrapper results in a json with allowCredentials filed present in it but with value null .

[emlun]

Ah, I see. Using an external serializer is an unsupported use case, and this issue is one of the reasons for that. The supported use, as instructed in both the "Getting started" README and the package JavaDoc, is to use the [toCredentialsCreateJson()](https://developers.yubico.com/java-webauthn-server/JavaDoc/webauthn-server-core/latest/com/yubico/webauthn/data/PublicKeyCredentialCreationOptions.html#toCredentialsCreateJson()) and [toCredentialsGetJson()](https://developers.yubico.com/java-webauthn-server/JavaDoc/webauthn-server-core/latest/com/yubico/webauthn/AssertionRequest.html#toCredentialsGetJson()) methods to convert the request objects to JSON.

[bmd007]

Ah, I see. Using an external serializer is an unsupported use case, and this issue is one of the reasons for that. The supported use, as instructed in both the "Getting started" README and the package JavaDoc, is to use the [toCredentialsCreateJson()](https://developers.yubico.com/java-webauthn-server/JavaDoc/webauthn-server-core/latest/com/yubico/webauthn/data/PublicKeyCredentialCreationOptions.html#toCredentialsCreateJson()) and [toCredentialsGetJson()](https://developers.yubico.com/java-webauthn-server/JavaDoc/webauthn-server-core/latest/com/yubico/webauthn/AssertionRequest.html#toCredentialsGetJson()) methods to convert the request objects to JSON.

Mixing (de)serialuzation logic with domain/dto classes to avoid "such" cases instead of writing a more defensive code, not gonna fly much. No need for it either.