Fixed credential persistence and retrieval.

DarkGhostHunter · Jul 3, 2020 · 84f7422 · 84f7422
1 parent c83f063
commit 84f7422
Show file tree

Hide file tree

Showing 12 changed files with 171 additions and 134 deletions.
diff --git a/database/migrations/2020_04_02_000000_create_web_authn_credentials_table.php b/database/migrations/2020_04_02_000000_create_web_authn_credentials_table.php
@@ -15,7 +15,7 @@ class CreateWebAuthnCredentialsTable extends Migration
     public function up()
     {
         Schema::create('web_authn_credentials', function (Blueprint $table) {
-            $table->string('id');
+            $table->binary('id');
 
             // Change accordingly for your users table if you need to.
             $table->unsignedBigInteger('user_id');

diff --git a/src/Auth/EloquentWebAuthnProvider.php b/src/Auth/EloquentWebAuthnProvider.php
@@ -51,16 +51,26 @@ public function __construct(ConfigContract $config,
      */
     public function retrieveByCredentials(array $credentials)
     {
-        if ($this->isSignedChallenge($credentials)) {
-            // We will try to get the user for the given credential ID.
-            return call_user_func([$this->model, 'getFromCredentialId'], $credentials['id']);
+        if ($this->isSignedChallenge($credentials) && $id = $this->binaryID($credentials['rawId'])) {
+            return $this->model::getFromCredentialId($id);
         }
 
         if ($this->fallback) {
             return parent::retrieveByCredentials($credentials);
         }
     }
 
+    /**
+     * Transforms the raw ID string into a binary string.
+     *
+     * @param  string  $rawId
+     * @return null|string
+     */
+    protected function binaryID(string $rawId)
+    {
+        return base64_decode(strtr($rawId, '-_', '+/'), true);
+    }
+
     /**
      * Check if the credentials are for a public key signed challenge
      *
@@ -82,7 +92,7 @@ protected function isSignedChallenge(array $credentials)
     public function validateCredentials(UserContract $user, array $credentials)
     {
         if ($this->isSignedChallenge($credentials)) {
-            return (bool) $this->validator->validate($credentials);
+            return (bool)$this->validator->validate($credentials);
         }
 
         if ($this->fallback) {

diff --git a/src/Eloquent/WebAuthnCredential.php b/src/Eloquent/WebAuthnCredential.php
@@ -148,28 +148,6 @@ public function getAaguidAttribute($value)
         return $value;
     }
 
-    /**
-     * Sets the credential public key as binary form.
-     *
-     * @param  string  $value
-     * @return void
-     */
-    public function setPublicKeyAttribute($value)
-    {
-        $this->attributes['public_key'] = base64_decode($value);
-    }
-
-    /**
-     * Return the credential public key as a Base64 string.
-     *
-     * @param  string  $value
-     * @return string
-     */
-    public function getPublicKeyAttribute($value)
-    {
-        return base64_encode($value);
-    }
-
     /**
      * Filter the credentials for those explicitly enabled.
      *

diff --git a/src/Http/AssertsWebAuthn.php b/src/Http/AssertsWebAuthn.php
@@ -77,13 +77,31 @@ protected function userProvider()
      */
     public function login(Request $request)
     {
-        $credential = $request->only('rawId', 'id', 'response', 'type');
+        $credential = $request->validate($this->assertionRules());
 
         if ($authenticated = $this->attemptLogin($credential, $this->hasRemember($request))) {
             return $this->authenticated($request, $this->guard()->user()) ?? response()->noContent();
         }
 
-        return response()->noContent(401);
+        return response()->noContent(422);
+    }
+
+    /**
+     * The assertion rules to validate the incoming JSON payload.
+     *
+     * @return array|string[]
+     */
+    protected function assertionRules()
+    {
+        return [
+            'id'                         => 'required|string',
+            'rawId'                      => 'required|string',
+            'response.authenticatorData' => 'required|string',
+            'response.clientDataJSON'    => 'required|string',
+            'response.signature'         => 'required|string',
+            'response.userHandle'        => 'required|string',
+            'type'                       => 'required|string',
+        ];
     }
 
     /**

diff --git a/src/Http/AttestsWebAuthn.php b/src/Http/AttestsWebAuthn.php
@@ -33,7 +33,7 @@ public function register(Request $request, WebAuthnAuthenticatable $user)
         // save it into the credentials store. If the data is invalid we will bail
         // out and return a non-authorized response since we can't save the data.
         $validCredential = WebAuthn::validateAttestation(
-            $request->only('id', 'rawId', 'response', 'type'), $user
+            $request->validate($this->attestationRules()), $user
         );
 
         if ($validCredential) {
@@ -44,7 +44,23 @@ public function register(Request $request, WebAuthnAuthenticatable $user)
             return $this->credentialRegistered($user, $validCredential) ?? response()->noContent();
         }
 
-        return response()->noContent(400);
+        return response()->noContent(422);
+    }
+
+    /**
+     * The attestation rules to validate the incoming JSON payload.
+     *
+     * @return array|string[]
+     */
+    protected function attestationRules()
+    {
+        return [
+            'id'                         => 'required|string',
+            'rawId'                      => 'required|string',
+            'response.attestationObject' => 'required|string',
+            'response.clientDataJSON'    => 'required|string',
+            'type'                       => 'required|string',
+        ];
     }
 
     /**

diff --git a/src/WebAuthn/WebAuthnAssertValidator.php b/src/WebAuthn/WebAuthnAssertValidator.php
@@ -211,7 +211,7 @@ public function validate(array $data)
             }
 
             return $this->validator->check(
-                $credentials->getId(),
+                $credentials->getRawId(),
                 $response,
                 $this->retrieveAssertion(),
                 $this->request,

diff --git a/tests/Auth/EloquentWebAuthnProviderTest.php b/tests/Auth/EloquentWebAuthnProviderTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Auth;
 
+use Base64Url\Base64Url;
 use Tests\RegistersPackage;
 use Illuminate\Support\Str;
 use Tests\Stubs\TestWebAuthnUser;
@@ -46,27 +47,27 @@ public function test_retrieves_user_using_credential_id()
         $user->save();
 
         DB::table('web_authn_credentials')->insert([
-            'id'         => 'test_credential_id',
-            'user_id'               => 1,
-            'is_enabled'            => true,
-            'type'                  => 'public_key',
-            'transports'            => json_encode([]),
-            'attestation_type'      => 'none',
-            'trust_path'            => json_encode(['type' => EmptyTrustPath::class]),
-            'aaguid'                => Str::uuid()->toString(),
-            'public_key' => 'public_key',
-            'counter'               => 0,
-            'user_handle'           => 'test_user_handle',
-            'created_at'            => now()->toDateTimeString(),
-            'updated_at'            => now()->toDateTimeString(),
+            'id'               => 'test_credential_id',
+            'user_id'          => 1,
+            'is_enabled'       => true,
+            'type'             => 'public_key',
+            'transports'       => json_encode([]),
+            'attestation_type' => 'none',
+            'trust_path'       => json_encode(['type' => EmptyTrustPath::class]),
+            'aaguid'           => Str::uuid()->toString(),
+            'public_key'       => 'public_key',
+            'counter'          => 0,
+            'user_handle'      => 'test_user_handle',
+            'created_at'       => now()->toDateTimeString(),
+            'updated_at'       => now()->toDateTimeString(),
         ]);
 
         $retrieved = Auth::createUserProvider('users')
             ->retrieveByCredentials([
-                'id' => 'test_credential_id',
-                'rawId' => 'something',
+                'id'       => 'test_credential_id',
+                'rawId'    => Base64Url::encode('test_credential_id'),
                 'response' => ['something'],
-                'type' => 'public-key'
+                'type'     => 'public-key',
             ]);
 
         $this->assertTrue($user->is($retrieved));
@@ -84,7 +85,7 @@ public function test_retrieves_user_using_classic_credentials()
 
         $retrieved = Auth::createUserProvider('users')
             ->retrieveByCredentials([
-                'email' => '[email protected]'
+                'email' => '[email protected]',
             ]);
 
         $this->assertTrue($user->is($retrieved));
@@ -104,7 +105,7 @@ public function test_fails_retrieving_user_using_classic_credentials_without_fal
 
         $retrieved = Auth::createUserProvider('users')
             ->retrieveByCredentials([
-                'email' => '[email protected]'
+                'email' => '[email protected]',
             ]);
 
         $this->assertNull($retrieved);
@@ -122,8 +123,8 @@ public function test_validates_user_using_password_fallback()
 
         $result = Auth::createUserProvider('users')
             ->validateCredentials($user, [
-                'name' => 'john',
-                'password' => 'secret'
+                'name'     => 'john',
+                'password' => 'secret',
             ]);
 
         $this->assertTrue($result);
@@ -143,8 +144,8 @@ public function test_fails_using_password_and_fallback_disabled()
 
         $result = Auth::createUserProvider('users')
             ->validateCredentials($user, [
-                'name' => 'john',
-                'password' => 'secret'
+                'name'     => 'john',
+                'password' => 'secret',
             ]);
 
         $this->assertFalse($result);

diff --git a/tests/Eloquent/WebAuthnAuthenticationTest.php b/tests/Eloquent/WebAuthnAuthenticationTest.php
@@ -57,7 +57,7 @@ public function test_saves_credential_public_key_as_binary_string()
 
         $model->public_key = $key;
 
-        $this->assertSame($key, base64_encode($model->getAttributes()['public_key']));
+        $this->assertSame($key, $model->getAttributes()['public_key']);
     }
 
     public function test_finds_one_by_credential_id()

diff --git a/tests/Http/WebAuthnLoginTest.php b/tests/Http/WebAuthnLoginTest.php
@@ -98,19 +98,19 @@ public function test_receives_webauthn_options_by_credentials()
         ])->save();
 
         DB::table('web_authn_credentials')->insert([
-            'id'         => 'test_credential_id',
-            'user_id'               => 1,
-            'is_enabled'            => true,
-            'type'                  => 'public_key',
-            'transports'            => json_encode([]),
-            'attestation_type'      => 'none',
-            'trust_path'            => json_encode(['type' => EmptyTrustPath::class]),
-            'aaguid'                => $uuid->toString(),
-            'public_key' => 'public_key',
-            'counter'               => 0,
-            'user_handle'           => 'test_user_handle',
-            'created_at'            => now()->toDateTimeString(),
-            'updated_at'            => now()->toDateTimeString(),
+            'id'               => 'test_credential_id',
+            'user_id'          => 1,
+            'is_enabled'       => true,
+            'type'             => 'public_key',
+            'transports'       => json_encode([]),
+            'attestation_type' => 'none',
+            'trust_path'       => json_encode(['type' => EmptyTrustPath::class]),
+            'aaguid'           => $uuid->toString(),
+            'public_key'       => 'public_key',
+            'counter'          => 0,
+            'user_handle'      => 'test_user_handle',
+            'created_at'       => now()->toDateTimeString(),
+            'updated_at'       => now()->toDateTimeString(),
         ]);
 
         $this->post('webauthn/login/options', [
@@ -144,19 +144,19 @@ public function test_disabled_credential_doesnt_show()
         ])->save();
 
         DB::table('web_authn_credentials')->insert([
-            'id'         => 'test_credential_id',
-            'user_id'               => 1,
-            'is_enabled'            => false,
-            'type'                  => 'public_key',
-            'transports'            => json_encode([]),
-            'attestation_type'      => 'none',
-            'trust_path'            => json_encode(['type' => EmptyTrustPath::class]),
-            'aaguid'                => $uuid->toString(),
-            'public_key' => 'public_key',
-            'counter'               => 0,
-            'user_handle'           => 'test_user_handle',
-            'created_at'            => now()->toDateTimeString(),
-            'updated_at'            => now()->toDateTimeString(),
+            'id'               => 'test_credential_id',
+            'user_id'          => 1,
+            'is_enabled'       => false,
+            'type'             => 'public_key',
+            'transports'       => json_encode([]),
+            'attestation_type' => 'none',
+            'trust_path'       => json_encode(['type' => EmptyTrustPath::class]),
+            'aaguid'           => $uuid->toString(),
+            'public_key'       => 'public_key',
+            'counter'          => 0,
+            'user_handle'      => 'test_user_handle',
+            'created_at'       => now()->toDateTimeString(),
+            'updated_at'       => now()->toDateTimeString(),
         ]);
 
         $this->post('webauthn/login/options', [
@@ -181,7 +181,7 @@ public function test_unauthenticated_when_attest_response_is_invalid()
             ],
             'type'     => 'public-key',
         ])
-            ->assertStatus(401);
+            ->assertStatus(422);
     }
 
     public function test_user_authenticates_with_webauthn()
@@ -197,28 +197,33 @@ public function test_user_authenticates_with_webauthn()
         $user->save();
 
         DB::table('web_authn_credentials')->insert([
-            'id'         => 'test_credential_id',
-            'user_id'               => 1,
-            'is_enabled'            => true,
-            'type'                  => 'public_key',
-            'transports'            => json_encode([]),
-            'attestation_type'      => 'none',
-            'trust_path'            => json_encode(['type' => EmptyTrustPath::class]),
-            'aaguid'                => $uuid->toString(),
-            'public_key' => 'public_key',
-            'counter'               => 0,
-            'user_handle'           => 'test_user_handle',
-            'created_at'            => now()->toDateTimeString(),
-            'updated_at'            => now()->toDateTimeString(),
+            'id'               => 'test_credential_id',
+            'user_id'          => 1,
+            'is_enabled'       => true,
+            'type'             => 'public_key',
+            'transports'       => json_encode([]),
+            'attestation_type' => 'none',
+            'trust_path'       => json_encode(['type' => EmptyTrustPath::class]),
+            'aaguid'           => $uuid->toString(),
+            'public_key'       => 'public_key',
+            'counter'          => 0,
+            'user_handle'      => 'test_user_handle',
+            'created_at'       => now()->toDateTimeString(),
+            'updated_at'       => now()->toDateTimeString(),
         ]);
 
         $this->mock(WebAuthnAssertValidator::class)
             ->shouldReceive('validate')
             ->with($data = [
-                'id' => 'test_credential_id',
-                'rawId' => 'test_raw_id',
-                'type' => 'test_type',
-                'response' => 'test_response',
+                'id'       => 'test_credential_id',
+                'type'     => 'test_type',
+                'response' => [
+                    'authenticatorData' => 'test',
+                    'clientDataJSON' => 'test',
+                    'signature' => 'test',
+                    'userHandle' => 'test',
+                ],
+                'rawId'    => Base64Url::encode('test_credential_id'),
             ])
             ->andReturnUsing(function ($data) {
                 $credentials = WebAuthnCredential::whereKey($data['id'])->first();
@@ -233,8 +238,8 @@ public function test_user_authenticates_with_webauthn()
         $this->assertAuthenticatedAs($user);
 
         $this->assertDatabaseHas('web_authn_credentials', [
-            'id' => 'test_credential_id',
-            'counter'       => 1,
+            'id'      => 'test_credential_id',
+            'counter' => 1,
         ]);
     }