Merge pull request #30 from deibertf/1.x

Update webauthn.js to prevent wrong request urls

README.md

@@ -258,6 +258,8 @@ const webAuthn = new WebAuthn({}, {
 
 Attestation is the _ceremony_ to create WebAuthn Credentials. To create an Attestable Response that the user device can understand, use the `AttestationRequest::toCreate()` form request.
 
+For example, we can create our own `AttestationController` to create it.
+
 ```php
 // app\Http\Controllers\WebAuthn\AttestationController.php
 use Laragear\WebAuthn\Http\Requests\AttestationRequest;
@@ -354,6 +356,8 @@ The Assertion procedure also follows a two-step procedure: the user will input i
 
 First, use the `AssertionRequest::toVerify()` form request. It will automatically create an assertion for the user that matches the credentials, or a blank one in case you're using [userless login](#userlessone-touchtypeless-login). Otherwise, you may set stricter validation rules to always ask for credentials.
 
+For example, we can use our own `AssertionController` to handle it.
+
 ```php
 // app\Http\Controllers\WebAuthn\AssertionController.php
 use Laragear\WebAuthn\Http\Requests\AssertionRequest;
@@ -573,10 +577,17 @@ return [
 The _Relying Party_ is just a way to uniquely identify your application in the user device:
 
 * `name`: The name of the application. Defaults to the application name.
-* `id`: An unique ID the application, like the site domain. If `null`, the device may fill it internally, usually as the full domain.
+* `id`: An unique ID the application, like the site URL. If `null`, the device _may_ fill it internally, usually as the full domain.
 
 > WebAuthn authentication only work on the top domain it was registered. 
 
+Instead of modifying the config file, you should use the environment variables to set the name and ID for WebAuthn.
+
+```dotenv
+WEBAUTHN_NAME=SecureBank
+WEBAUTHN_ID=https://auth.securebank.com
+```
+
 ### Challenge configuration
 
 ```php

resources/js/webauthn.js

@@ -156,7 +156,9 @@ class WebAuthn {
      * @returns {Promise<Response>}
      */
     #fetch(data, route, headers = {}) {
-        return fetch(route, {
+        const url = new URL(route, window.location.origin).href;
+        
+        return fetch(url, {
             method: "POST",
             credentials: this.#includeCredentials ? "include" : "same-origin",
             redirect: "error",
@@ -313,6 +315,7 @@ class WebAuthn {
         const publicKeyCredential = this.#parseOutgoingCredentials(credentials);
 
         Object.assign(publicKeyCredential, response);
+        Object.assign(publicKeyCredential, request);
 
         return await this.#fetch(publicKeyCredential, this.#routes.register).then(WebAuthn.#handleResponse);
     }

src/Models/WebAuthnCredential.php

@@ -95,7 +95,7 @@ class WebAuthnCredential extends Model
      *
      * @var array<int, string>
      */
-    protected $visible = ['id', 'origin', 'alias', 'aaguid', 'attestation_format', 'disabled_at', 'is_enabled'];
+    protected $visible = ['id', 'origin', 'alias', 'aaguid', 'attestation_format', 'disabled_at'];
 
     /**
      * @phpstan-ignore-next-line

src/WebAuthnAuthentication.php

@@ -4,9 +4,9 @@ namespace Laragear\WebAuthn;
 
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Relations\MorphMany;
+use Illuminate\Support\Facades\Date;
 use JetBrains\PhpStorm\ArrayShape;
 use Laragear\WebAuthn\Models\WebAuthnCredential;
-use function in_array;
 
 /**
  * @property-read \Illuminate\Database\Eloquent\Collection<int, \Laragear\WebAuthn\Models\WebAuthnCredential> $webAuthnCredentials
@@ -38,20 +38,17 @@ trait WebAuthnAuthentication
      */
     public function flushCredentials(string ...$except): void
     {
-        if ($this->relationLoaded('webAuthnCredentials') && $this->webAuthnCredentials instanceof Collection) {
-            $partitioned = $this->webAuthnCredentials
-                ->partition(static function (WebAuthnCredential $credential) use ($except): bool {
-                    return in_array($credential->getKey(), $except, true);
-                });
-
-            $partitioned->first()->each->delete();
-
-            $this->setRelation('webAuthnCredentials', $partitioned->last());
+        if (! $this->relationLoaded('webAuthnCredentials')) {
+            $this->webAuthnCredentials()->whereKeyNot($except)->delete();
 
             return;
         }
 
-        $this->webAuthnCredentials()->whereKeyNot($except)->delete();
+        if ($this->webAuthnCredentials instanceof Collection && $this->webAuthnCredentials->isNotEmpty()) {
+            $this->webAuthnCredentials->whereNotIn('id', $except)->each->delete();
+
+            $this->setRelation('webAuthnCredentials', $this->webAuthnCredentials->whereIn('id', $except));
+        }
     }
 
     /**
@@ -64,13 +61,14 @@ trait WebAuthnAuthentication
     {
         if ($this->relationLoaded('webAuthnCredentials') && $this->webAuthnCredentials instanceof Collection) {
             $this->webAuthnCredentials
-                ->each(static function (WebAuthnCredential $credential) use ($except): bool {
-                    if ($credential->isEnabled() && in_array($credential->getKey(), $except, true)) {
+                ->when($except)->whereNotIn('id', $except)
+                ->each(static function (WebAuthnCredential $credential): void {
+                    if ($credential->isEnabled()) {
                         $credential->disable();
                     }
                 });
         } else {
-            $this->webAuthnCredentials()->whereKeyNot($except)->update(['is_enabled' => false]);
+            $this->webAuthnCredentials()->whereKeyNot($except)->whereEnabled()->update(['disabled_at' => Date::now()]);
         }
     }
 

tests/WebAuthnAuthenticationTest.php

@@ -0,0 +1,260 @@
+<?php
+
+namespace Tests;
+
+use Illuminate\Support\Carbon;
+use Laragear\WebAuthn\Models\WebAuthnCredential;
+use Ramsey\Uuid\Uuid;
+use function now;
+
+class WebAuthnAuthenticationTest extends TestCase
+{
+    protected Stubs\WebAuthnAuthenticatableUser $user;
+
+    protected function afterRefreshingDatabase(): void
+    {
+        $this->user = Stubs\WebAuthnAuthenticatableUser::forceCreate([
+            'name' => FakeAuthenticator::ATTESTATION_USER['displayName'],
+            'email' => FakeAuthenticator::ATTESTATION_USER['name'],
+            'password' => 'test_password',
+        ]);
+
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id',
+            'user_id' => Uuid::NIL,
+            'counter' => 0,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+        ])->save();
+    }
+
+    public function test_shows_webauthn_data(): void
+    {
+        static::assertSame([
+            'name' => FakeAuthenticator::ATTESTATION_USER['name'],
+            'displayName' => FakeAuthenticator::ATTESTATION_USER['displayName'],
+        ], $this->user->webAuthnData());
+    }
+
+    public function test_flushes_all_credentials(): void
+    {
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()
+        ])->save();
+
+        $this->user->flushCredentials();
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 0);
+    }
+
+    public function test_flushes_all_credentials_using_loaded_relation(): void
+    {
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()
+        ])->save();
+
+        $this->user->load('webAuthnCredentials');
+
+        static::assertCount(2, $this->user->webAuthnCredentials);
+
+        $this->user->flushCredentials();
+
+        static::assertEmpty($this->user->webAuthnCredentials);
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 0);
+    }
+
+    public function test_flushes_all_credentials_except_given_id(): void
+    {
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()
+        ])->save();
+
+        $this->user->flushCredentials('test_id_2');
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 1);
+        $this->assertDatabaseMissing(WebAuthnCredential::class, [
+            'id' => 'test_id'
+        ]);
+    }
+
+    public function test_flushes_all_credentials_using_loaded_relation_except_given_id(): void
+    {
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()
+        ])->save();
+
+        $this->user->load('webAuthnCredentials');
+
+        static::assertCount(2, $this->user->webAuthnCredentials);
+
+        $this->user->flushCredentials('test_id_2');
+
+        static::assertCount(1, $this->user->webAuthnCredentials);
+        static::assertTrue($this->user->webAuthnCredentials->contains('id', 'test_id_2'));
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 1);
+        $this->assertDatabaseMissing(WebAuthnCredential::class, [
+            'id' => 'test_id'
+        ]);
+    }
+
+    public function test_disables_all_credentials(): void
+    {
+        $this->travelTo(Carbon::now()->startOfSecond());
+
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()->subMinute()
+        ])->save();
+
+        $this->user->disableAllCredentials();
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 2);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id',
+            'disabled_at' => now()->toDateTimeString(),
+        ]);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id_2',
+            'disabled_at' => now()->subMinute()->toDateTimeString(),
+        ]);
+    }
+
+    public function test_disables_all_credentials_with_loaded_relation(): void
+    {
+        $this->travelTo(Carbon::now()->startOfSecond());
+
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+            'disabled_at' => now()->subMinute()
+        ])->save();
+
+        $this->user->load('webAuthnCredentials');
+
+        $this->user->disableAllCredentials();
+
+        static::assertTrue($this->user->webAuthnCredentials->firstWhere('id', 'test_id')->isDisabled());
+        static::assertTrue($this->user->webAuthnCredentials->firstWhere('id', 'test_id_2')->isDisabled());
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 2);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id',
+            'disabled_at' => now()->toDateTimeString(),
+        ]);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id_2',
+            'disabled_at' => now()->subMinute()->toDateTimeString(),
+        ]);
+    }
+
+    public function test_disables_all_credentials_except_one(): void
+    {
+        $this->travelTo(Carbon::now()->startOfSecond());
+
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+        ])->save();
+
+        $this->user->disableAllCredentials('test_id');
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 2);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id',
+            'disabled_at' => null,
+        ]);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id_2',
+            'disabled_at' => now()->toDateTimeString(),
+        ]);
+    }
+
+    public function test_disables_all_credentials_with_loaded_relation_except_one(): void
+    {
+        $this->travelTo(Carbon::now()->startOfSecond());
+
+        $this->user->webAuthnCredentials()->make()->forceFill([
+            'id' => 'test_id_2',
+            'user_id' => Uuid::NIL,
+            'counter' => 10,
+            'rp_id' => 'http://localhost',
+            'origin' => 'http://localhost:8000',
+            'aaguid' => Uuid::NIL,
+            'public_key' => 'test_key',
+            'attestation_format' => 'none',
+        ])->save();
+
+        $this->user->load('webAuthnCredentials');
+
+        $this->user->disableAllCredentials('test_id_2');
+
+        static::assertTrue($this->user->webAuthnCredentials->firstWhere('id', 'test_id')->isDisabled());
+        static::assertFalse($this->user->webAuthnCredentials->firstWhere('id', 'test_id_2')->isDisabled());
+
+        $this->assertDatabaseCount(WebAuthnCredential::class, 2);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id',
+            'disabled_at' => now()->toDateTimeString(),
+        ]);
+        $this->assertDatabaseHas(WebAuthnCredential::class, [
+            'id' => 'test_id_2',
+            'disabled_at' => null,
+        ]);
+    }
+}