[3.x] Stateless behavior

[illambo]

Please check these requirements

• [X] This feature helps everyone using this package
• [X] It's feasible and maintainable
• [X] It's non breaking
• [ ] I issued a PR with the implementation (optional)

Description

Hi, first of all thanks for super power :rocket: package,

I think it can be very useful and important to give the ability of using a stateless mode (e.g. for token based communications like rest/grapqhl/..).

In this sense I identify two areas for improvement:

• ability to use the cache as an alternative driver to the session
• pipelines, ability of receiving an alternative contract to the request as input from which to obtain the exchange data

I'll try to clarify this last point for a moment: at several points pipes expect precise data present in the input of incoming request but this is quite limiting if the communication where the payload of the data exchanged is different (e.g. graphql/rest/..) and this cause complications in use of manual management, (as a work around I currently use request()->merge([]) but I think we can find a better strategy.

What do you think about this? Did I miss something, maybe some security-related steps?

Thanks.

Code sample

// I'm trying to see if I can produce a draft PR on this.

[DarkGhostHunter]

Detaching the Request from the pipelines is already done, but since it breaks everything, it will be pushed onto v3.0.

About the challenge, I purposely used the session to avoid completing the attestation and assertion outside the same request lifecycle.

Using a cache would make that possible, like using requesting the assertion options in one server and completing it on another, both having access to the same cache.

Because they become stateless, now you have to make them identifiable. I believe that userless assertion options have no way to be uniquely identifiable, the reason why I decided to map it into the request and call it a day, AFAIK.

[illambo]

Great! Sorry, I didn't notice the 3.x branch.

About:

Because they become stateless, now you have to make them identifiable. I believe that userless assertion options have no way to be uniquely identifiable, the reason why I decided to map it into the request and call it a day, AFAIK.

I'll take a moment to get into the matter, it seems to me that other implementations (just as you thought) also require the user to generate the identifier for the cache.

I'll think about it and update you, thanks in the meantime!

[DarkGhostHunter]

I'll take a moment to get into the matter, it seems to me that other implementations (just as you thought) also require the user to generate the identifier for the cache.

Yea, these take the request IP. I would rather use the session instead.

[illambo]

I was rechecking the flow at a high level (therefore not the current implementation in detail), since the challenge is generated on the server side (and then only subsequently verified that it corresponds) it would not be enough to use something like an md5 or hash of this object (maybe the ByteBuffer perhaps concatenating useful information such as ip if necessary) to generate the identifier ? In this way you wouldn't need to pass it back to the client.

[DarkGhostHunter]

I would rather let the user use a custom ChallengeRepository to put and pull challenges rather saving the challenge outside the session.

[illambo]

Ok so if I understood correctly everyone could optionally implement their own ChallengeRepository (possibly alternative to the session) to manage the challenge, correct? It seems like a good compromise and solution to me.

[DarkGhostHunter]

Yep. You would have to register it to a manager, tho.

public function boot(ChallengeRepositoryManager $manager)
{
    $manager->extend('my-repository', function () {
        return new MyRepository();
    });
}

And then use the config to set the repository:

return [
    // ...

    'challenge' => [
        'repository' => 'session'
    ]
]

[illambo]

About:

About the challenge, I purposely used the session to avoid completing the attestation and assertion outside the same request lifecycle. Using a cache would make that possible, like using requesting the assertion options in one server and completing it on another, both having access to the same cache.

Both attestation and assertion makes two distinct http calls (one for option and the other for completion), in cloud scenario with multi instance application (where each instance can have an ephemeral life) the session is commonly shared so in my opinion the situation would be the same. Don't you think so?

[illambo]

However, I agree that using the session is the most convenient and probably "confidential" way, but I think leaving the way open to other "scenarios" is super!

[DarkGhostHunter]

Done in https://github.com/Laragear/WebAuthn/commit/9e1b6a2ad7f7c7e56320a014d68fa5ad91bf8f70

[illambo]

Great! I contribute with a sponsorship. I left you a couple of comments on the commit (small correction on the readme $fingerprint should be $this->getFingerprint()). Do you have an eta and roadmap to share for v3 ? Thanks :pray:

[illambo]

Hei @DarkGhostHunter sorry for the quick replay, I'm trying the branch, I think there's a problem in Attestation/Validator/Pipes/MakeWebAuthnCredential, unless I did something wrong, this throw Laragear\WebAuthn\JsonTransport::get(): Return value must be of type string|int|null, array returned, the JsonTransport shouldn't be able to return array too? (e.g "transports" => ["internal"], I did a test and it seems to work with this change).

[illambo]

Other issue in AssertionValidator with pipe FireCredentialAssertedEvent,

$credential = app()->make(AssertionValidator::class)
  ->send(new AssertionValidation(new JsonTransport($input)))
 ->thenReturn()
 ->credential;

//ray($credential?->authenticatable);

The line CredentialAsserted::dispatch($validation->user, $validation->credential); i think should be CredentialAsserted::dispatch($validation->credential->authenticatable, $validation->credential); and perhaps consider that this event can receive null values.

What do you think?

[illambo]

Another request, for WebAuthnChallengeRepository can u change the signature

• from public function store(Challenge $challenge): void;to public function store(Challenge $challenge, AttestationCreation|AssertionCreation|null): void;
• from public function pull(): ?Challenge; to public function pull(AttestationValidation|AssertionValidation|null): ?Challenge;

and push the related data in pipes?

This opens up the possibility of what I mentioned previously regarding the hash/md5/.. of the incoming payload.

[DarkGhostHunter]

Oh, great idea. Didn't thought of that.

[DarkGhostHunter]

Other issue in AssertionValidator with pipe FireCredentialAssertedEvent,

$credential = app()->make(AssertionValidator::class)
  ->send(new AssertionValidation(new JsonTransport($input)))
 ->thenReturn()
 ->credential;

//ray($credential?->authenticatable);

The line CredentialAsserted::dispatch($validation->user, $validation->credential); i think should be CredentialAsserted::dispatch($validation->credential->authenticatable, $validation->credential); and perhaps consider that this event can receive null values.

What do you think?

It's better to put the user as nullable, in case the user wasn't put in the assertion pipeline. The $validation->credential->authenticatable line adds a new uncontrollable query to find the user, and I've seen apps with full bios on that table. Hard pass.

[DarkGhostHunter]

Checkout the newest push to the branch. I added:

• New events for validating attestation & assertion
• Modified the Challenge Repository contract to include the ceremony data.
• Fixed JsonTransport get() to return anything instead of only a set of primitives.

[illambo]

Hi, I did a quick test locally and everything seems to work both with session and stateless with custom WebAuthnChallengeRepository.

The only note I have for now is that in the readme I bind the contract in register method with $this->app->bind(\Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository::class, fn () => new \App\WebAuthn\WebAuthnChallengeRepository); because the one noted generate exception for me ("Cannot instantiate interface Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository").

I'll try to do some more in-depth tests and if necessary I'll update you (in my opinion you can close the issue anyway). Thank you, I appreciate it!

[illambo]

I would have something else (don't hate me).

class WebAuthnChallengeRepository implements \Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository
{
    /**
     * Puts a ceremony challenge into the repository.
     */
    public function store(AttestationCreation|AssertionCreation $ceremony, Challenge $challenge): void
    {
        $fingerprint = implode('|', [
            'webauthn_challenge', Request::ip(), (string) $challenge->data,
        ]);

        ray(__METHOD__.': '.$fingerprint);

        Cache::put($fingerprint, $challenge, $challenge->expiresAt());
    }

    /**
     * Pulls a ceremony challenge out from the repository, if it exists.
     */
    public function pull(AttestationValidation|AssertionValidation $ceremony): ?Challenge
    {
        $fingerprint = implode('|', [
            'webauthn_challenge', Request::ip(), (string) $ceremony->clientDataJson->challenge,
        ]);

        ray(__METHOD__.': '.$fingerprint);

        return Cache::pull($fingerprint);
    }
}

I can't access the challenge (pull method from $cerimony), maybe we need to rearrange the pipes to access the challenge. What do u think?

[DarkGhostHunter]

I would have something else (don't hate me).

class WebAuthnChallengeRepository implements \Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository
{
    /**
     * Puts a ceremony challenge into the repository.
     */
    public function store(AttestationCreation|AssertionCreation $ceremony, Challenge $challenge): void
    {
        $fingerprint = implode('|', [
            'webauthn_challenge', Request::ip(), (string) $challenge,
        ]);

        ray(__METHOD__.': '.$fingerprint);

        Cache::put($fingerprint, $challenge, $challenge->expiresAt());
    }

    /**
     * Pulls a ceremony challenge out from the repository, if it exists.
     */
    public function pull(AttestationValidation|AssertionValidation $ceremony): ?Challenge
    {
        $fingerprint = implode('|', [
            'webauthn_challenge', Request::ip(), (string) $ceremony->clientDataJson->challenge,
        ]);

        ray(__METHOD__.': '.$fingerprint);

        return Cache::pull($fingerprint);
    }
}

I can't access the challenge (pull method from $cerimony), maybe we need to rearrange the pipes to access the challenge. What do u think?

Test the new commit. I re-arranged them to be the last as possible.

[DarkGhostHunter]

...the one noted generate exception for me ("Cannot instantiate interface Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository").

Where it happens?

[illambo]

Perfect now it works! :muscle:

About the exception "Cannot instantiate interface Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository" happens when i put $this->app->register(\Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository::class, fn () => new \App\WebAuthn\WebAuthnChallengeRepository()); in boot or register method of AppServiceProvider (in local poc app laravel 10.x) so i used $this->app->bind(\Laragear\WebAuthn\Contracts\WebAuthnChallengeRepository::class, fn () => new \App\WebAuthn\WebAuthnChallengeRepository); (or public $bindings array property).

[DarkGhostHunter]

3.x will be published at the start of next month.