Fix missing login throttling on WebAuthn login controller

app/Http/Controllers/Auth/WebAuthnLoginController.php

@@ -3,19 +3,23 @@
 namespace App\Http\Controllers\Auth;
 
 use App\Http\Controllers\Controller;
+use App\Http\Requests\WebauthnAssertedRequest;
 use App\Models\User;
 use Carbon\Carbon;
 use Illuminate\Contracts\Support\Responsable;
+use Illuminate\Foundation\Auth\AuthenticatesUsers;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Response;
 use Illuminate\Support\Arr;
+use Illuminate\Support\Facades\Lang;
 use Illuminate\Support\Facades\Log;
-use Laragear\WebAuthn\Http\Requests\AssertedRequest;
 use Laragear\WebAuthn\Http\Requests\AssertionRequest;
 use Laragear\WebAuthn\WebAuthn;
 
 class WebAuthnLoginController extends Controller
 {
+    use AuthenticatesUsers;
+
     /*
     |--------------------------------------------------------------------------
     | WebAuthn Login Controller
@@ -56,12 +60,28 @@ class WebAuthnLoginController extends Controller
     /**
      * Log the user in.
      *
-     * @param  \Laragear\WebAuthn\Http\Requests\AssertedRequest  $request
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
      * @return \Illuminate\Http\Response|\Illuminate\Http\JsonResponse
      */
-    public function login(AssertedRequest $request)
+    public function login(WebauthnAssertedRequest $request)
     {
-        Log::info('User login via webauthn requested');
+        Log::info(sprintf('User login via webauthn requested by %s from %s', var_export($request['email'], true), $request->ip()));
+
+        // If the class is using the ThrottlesLogins trait, we can automatically throttle
+        // the login attempts for this application. We'll key this by the username and
+        // the IP address of the client making these requests into this application.
+        if (method_exists($this, 'hasTooManyLoginAttempts') &&
+            $this->hasTooManyLoginAttempts($request)) {
+            $this->fireLockoutEvent($request);
+
+            Log::notice(sprintf(
+                '%s from %s locked-out, too many failed login attempts (using webauthn)',
+                var_export($request['email'], true),
+                $request->ip()
+            ));
+
+            return $this->sendLockoutResponse($request);
+        }
 
         if ($request->has('response')) {
             $response = $request->response;
@@ -74,22 +94,110 @@ class WebAuthnLoginController extends Controller
             }
         }
 
+        if ($this->attemptLogin($request)) {
+            return $this->sendLoginResponse($request);
+        }
+
+        // If the login attempt was unsuccessful we will increment the number of attempts
+        // to login and redirect the user back to the login form. Of course, when this
+        // user surpasses their maximum number of attempts they will get locked out.
+        $this->incrementLoginAttempts($request);
+
+        Log::notice(sprintf(
+            'Failed login for %s from %s - Attemp %d/%d (using webauthn)',
+            var_export($request['email'], true),
+            $request->ip(),
+            $this->limiter()->attempts($this->throttleKey($request)),
+            $this->maxAttempts()
+        ));
+
+        return response()->json(['message' => 'unauthorized'], Response::HTTP_UNAUTHORIZED);
+    }
+
+    /**
+     * Attempt to log the user into the application.
+     *
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
+     * @return bool
+     */
+    protected function attemptLogin(WebauthnAssertedRequest $request)
+    {
+        return ! is_null($request->login());
+    }
+
+    /**
+     * Send the response after the user was authenticated.
+     *
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
+     * @return \Illuminate\Http\JsonResponse
+     */
+    protected function sendLoginResponse(WebauthnAssertedRequest $request)
+    {
+        $this->clearLoginAttempts($request);
+
         /**
          * @var \App\Models\User|null
          */
-        $user = $request->login();
+        $user = $this->guard()->user();
 
-        if ($user) {
-            $this->authenticated($user);
+        $this->authenticated($user);
 
-            return response()->json([
-                'message'     => 'authenticated',
-                'name'        => $user->name,
-                'preferences' => $user->preferences,
-            ], Response::HTTP_OK);
-        }
+        return response()->json([
+            'message'     => 'authenticated',
+            'name'        => $user->name,
+            'preferences' => $user->preferences,
+        ], Response::HTTP_OK);
+    }
+
+    /**
+     * Get the failed login response instance.
+     *
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
+     * @return \Illuminate\Http\JsonResponse
+     */
+    protected function sendFailedLoginResponse(WebauthnAssertedRequest $request)
+    {
+        return response()->json(['message' => 'unauthorized'], Response::HTTP_UNAUTHORIZED);
+    }
+
+    /**
+     * Redirect the user after determining they are locked out.
+     *
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
+     * @return \Illuminate\Http\JsonResponse
+     */
+    protected function sendLockoutResponse(WebauthnAssertedRequest $request)
+    {
+        $seconds = $this->limiter()->availableIn(
+            $this->throttleKey($request)
+        );
+
+        return response()->json(['message' => Lang::get('auth.throttle', ['seconds' => $seconds])], Response::HTTP_TOO_MANY_REQUESTS);
+    }
+
+    /**
+     * Get the login username to be used by the controller.
+     *
+     * @return string
+     */
+    public function username()
+    {
+        return 'email';
+    }
+
+    /**
+     * Get the needed authorization credentials from the request.
+     *
+     * @param  \App\Http\Requests\WebauthnAssertedRequest  $request
+     * @return array
+     */
+    protected function credentials(WebauthnAssertedRequest $request)
+    {
+        $credentials = [
+            $this->username() => strtolower($request->input($this->username())),
+        ];
 
-        return response()->noContent(422);
+        return $credentials;
     }
 
     /**
@@ -103,6 +211,6 @@ class WebAuthnLoginController extends Controller
         $user->last_seen_at = Carbon::now()->format('Y-m-d H:i:s');
         $user->save();
 
-        Log::info(sprintf('User ID #%s authenticated using webauthn', $user->id));
+        Log::info(sprintf('User ID #%s authenticated (using webauthn)', $user->id));
     }
 }

app/Http/Requests/WebauthnAssertedRequest.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace App\Http\Requests;
+
+use Laragear\WebAuthn\Http\Requests\AssertedRequest;
+
+class WebauthnAssertedRequest extends AssertedRequest
+{
+    /**
+     * Get the validation rules that apply to the request.
+     *
+     * @return array
+     */
+    public function rules(): array
+    {
+        return array_merge(
+            [
+                'email' => 'required|email',
+            ],
+            parent::rules()
+        );
+    }
+}

resources/js/views/auth/Login.vue

@@ -138,7 +138,8 @@
 
                 if (!credentials) return false
 
-                const publicKeyCredential = this.webauthn.parseOutgoingCredentials(credentials)
+                let publicKeyCredential = this.webauthn.parseOutgoingCredentials(credentials)
+                publicKeyCredential.email = this.form.email
 
                 this.axios.post('/webauthn/login', publicKeyCredential, {returnError: true}).then(response => {
                     this.applyPreferences(response.data.preferences);

tests/Feature/Http/Auth/LoginTest.php

@@ -44,9 +44,14 @@ class LoginTest extends FeatureTestCase
             'password' => self::PASSWORD,
         ])
             ->assertOk()
-            ->assertExactJson([
+            ->assertJsonFragment([
                 'message' => 'authenticated',
                 'name'    => $this->user->name,
+            ])
+            ->assertJsonStructure([
+                'message',
+                'name',
+                'preferences',
             ]);
     }
 
@@ -62,9 +67,14 @@ class LoginTest extends FeatureTestCase
             'password' => self::PASSWORD,
         ])
             ->assertOk()
-            ->assertExactJson([
+            ->assertJsonFragment([
                 'message' => 'authenticated',
                 'name'    => $this->user->name,
+            ])
+            ->assertJsonStructure([
+                'message',
+                'name',
+                'preferences',
             ]);
     }
 
@@ -73,7 +83,7 @@ class LoginTest extends FeatureTestCase
      *
      * @covers  \App\Http\Middleware\SkipIfAuthenticated
      */
-    public function test_user_login_already_authenticated_returns_bad_request()
+    public function test_user_login_already_authenticated_returns_success()
     {
         $response = $this->json('POST', '/user/login', [
             'email'    => $this->user->email,
@@ -113,7 +123,7 @@ class LoginTest extends FeatureTestCase
      *
      * @covers  \App\Exceptions\Handler
      */
-    public function test_user_login_with_invalid_credentials_returns_authentication_error()
+    public function test_user_login_with_invalid_credentials_returns_unauthorized()
     {
         $response = $this->json('POST', '/user/login', [
             'email'    => $this->user->email,
@@ -121,7 +131,7 @@ class LoginTest extends FeatureTestCase
         ])
             ->assertStatus(401)
             ->assertJson([
-                'message' => 'unauthorised',
+                'message' => 'unauthorized',
             ]);
     }
 
@@ -130,35 +140,17 @@ class LoginTest extends FeatureTestCase
      */
     public function test_too_many_login_attempts_with_invalid_credentials_returns_too_many_request_error()
     {
-        $response = $this->json('POST', '/user/login', [
-            'email'    => $this->user->email,
-            'password' => self::WRONG_PASSWORD,
-        ]);
-
-        $response = $this->json('POST', '/user/login', [
-            'email'    => $this->user->email,
-            'password' => self::WRONG_PASSWORD,
-        ]);
-
-        $response = $this->json('POST', '/user/login', [
+        $post = [
             'email'    => $this->user->email,
             'password' => self::WRONG_PASSWORD,
-        ]);
-
-        $response = $this->json('POST', '/user/login', [
-            'email'    => $this->user->email,
-            'password' => self::WRONG_PASSWORD,
-        ]);
-
-        $response = $this->json('POST', '/user/login', [
-            'email'    => $this->user->email,
-            'password' => self::WRONG_PASSWORD,
-        ]);
-
-        $response = $this->json('POST', '/user/login', [
-            'email'    => $this->user->email,
-            'password' => self::WRONG_PASSWORD,
-        ]);
+        ];
+
+        $this->json('POST', '/user/login', $post);
+        $this->json('POST', '/user/login', $post);
+        $this->json('POST', '/user/login', $post);
+        $this->json('POST', '/user/login', $post);
+        $this->json('POST', '/user/login', $post);
+        $response = $this->json('POST', '/user/login', $post);
 
         $response->assertStatus(429);
     }

tests/Feature/Http/Auth/WebAuthnLoginControllerTest.php

@@ -6,7 +6,6 @@ use App\Models\User;
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\DB;
 use Laragear\WebAuthn\Assertion\Validator\AssertionValidator;
-use Laragear\WebAuthn\Http\Requests\AssertedRequest;
 use Laragear\WebAuthn\WebAuthn;
 use Tests\FeatureTestCase;
 
@@ -33,6 +32,8 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
 
     const USER_ID_ALT = 'e8af6f703f8042aa91c30cf72289aa07';
 
+    const EMAIL = 'john.doe@example.com';
+
     const ASSERTION_RESPONSE = [
         'id'       => self::CREDENTIAL_ID_ALT,
         'rawId'    => self::CREDENTIAL_ID_ALT_RAW,
@@ -43,6 +44,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
             'signature'         => 'ca4IJ9h8bZnjMbEFuHX1zfX5LcbiPyDVz6sD1/ppR4t8++1DxKa5EdBIrfNlo8FSOv/JSzMrGGUCQvc/Ngj1KnZpO3s9OdTb54/gMDewH/K8EG4wSvxzHdL6sMbP7UUc5Wq1pcdu9MgXY8V+1gftXpzcoaae0X+mLEETgU7eB8jG0mZhVWvE4yQKuDnZA1i9r8oQhqsvG4nUw1BxvR8wAGiRR+R287LaL41k+xum5mS8zEojUmuLSH50miyVxZ4Y+/oyfxG7i+wSYGNSXlW5iNPB+2WupGS7ce4TuOgaFeMmP2a9rzP4m2IBSQoJ2FyrdzR7HwBEewqqrUVbGQw3Aw==',
             'userHandle'        => self::USER_ID_ALT,
         ],
+        'email' => self::EMAIL,
     ];
 
     const ASSERTION_RESPONSE_NO_HANDLE = [
@@ -55,6 +57,20 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
             'signature'         => 'ca4IJ9h8bZnjMbEFuHX1zfX5LcbiPyDVz6sD1/ppR4t8++1DxKa5EdBIrfNlo8FSOv/JSzMrGGUCQvc/Ngj1KnZpO3s9OdTb54/gMDewH/K8EG4wSvxzHdL6sMbP7UUc5Wq1pcdu9MgXY8V+1gftXpzcoaae0X+mLEETgU7eB8jG0mZhVWvE4yQKuDnZA1i9r8oQhqsvG4nUw1BxvR8wAGiRR+R287LaL41k+xum5mS8zEojUmuLSH50miyVxZ4Y+/oyfxG7i+wSYGNSXlW5iNPB+2WupGS7ce4TuOgaFeMmP2a9rzP4m2IBSQoJ2FyrdzR7HwBEewqqrUVbGQw3Aw==',
             'userHandle'        => null,
         ],
+        'email' => self::EMAIL,
+    ];
+
+    const ASSERTION_RESPONSE_INVALID = [
+        'id'       => self::CREDENTIAL_ID_ALT,
+        'rawId'    => self::CREDENTIAL_ID_ALT_RAW,
+        'type'     => 'public-key',
+        'response' => [
+            'clientDataJSON'    => 'eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiaVhvem15bktpLVlEMmlSdktOYlNQQSIsIm9yaWdpbiI6Imh0dHA6Ly9sb2NhbGhvc3QiLCJjcm9zc09yaWdpbiI6ZmFsc2V9',
+            'authenticatorData' => 'SZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2MFAAAAAQ==',
+            'signature'         => 'ca4IJ9h8bZnjMbEFuHX1zfX5LcbiPyDVz6sD1/ppR4t8++1DxKa5EdBIrfNlo8FSOv/JSzMrGGUCQvc/Ngj1KnZpO3s9OdTb54/gMDewH/K8EG4wSvxzHdL6sMbP7UUc5Wq1pcdu9MgXY8V+1gftXpzcoaae0X+mLEETgU7eB8jG0mZhVWvE4yQKuDnZA1i9r8oQhqsvG4nUw1BxvR8wAGiRR+R287LaL41k+xum5mS8zEojUmuLSH50miyVxZ4Y+/oyfxG7i+wSYGNSXlW5iNPB+2WupGS7ce4TuOgaFeMmP2a9rzP4m2IBSQoJ2FyrdzR7HwBEewqqrUVbGQw3Aw==',
+            'userHandle'        => self::USER_ID_ALT,
+        ],
+        'email' => self::EMAIL,
     ];
 
     const ASSERTION_CHALLENGE = 'iXozmynKi+YD2iRvKNbSPA==';
@@ -72,18 +88,46 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_webauthn_login_uses_login_and_returns_no_content()
+    public function test_webauthn_login_returns_success()
     {
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
-        $mock = $this->mock(AssertedRequest::class)->makePartial()->shouldIgnoreMissing();
-        $mock->shouldReceive([
-            'has'   => false,
-            'login' => $this->user,
+        DB::table('webauthn_credentials')->insert([
+            'id'                   => self::CREDENTIAL_ID_ALT,
+            'authenticatable_type' => \App\Models\User::class,
+            'authenticatable_id'   => $this->user->id,
+            'user_id'              => self::USER_ID_ALT,
+            'counter'              => 0,
+            'rp_id'                => 'http://localhost',
+            'origin'               => 'http://localhost',
+            'aaguid'               => '00000000-0000-0000-0000-000000000000',
+            'attestation_format'   => 'none',
+            'public_key'           => self::PUBLIC_KEY,
+            'updated_at'           => now(),
+            'created_at'           => now(),
         ]);
 
-        $this->json('POST', '/webauthn/login')
-            ->assertNoContent();
+        $this->session(['_webauthn' => new \Laragear\WebAuthn\Challenge(
+            new \Laragear\WebAuthn\ByteBuffer(base64_decode(self::ASSERTION_CHALLENGE)),
+            60,
+            false,
+        )]);
+
+        $this->mock(AssertionValidator::class)
+            ->expects('send->thenReturn')
+            ->andReturn();
+
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE)
+            ->assertOk()
+            ->assertJsonFragment([
+                'message'     => 'authenticated',
+                'name'        => $this->user->name,
+            ])
+            ->assertJsonStructure([
+                'message',
+                'name',
+                'preferences',
+            ]);
     }
 
     /**
@@ -91,7 +135,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
      */
     public function test_webauthn_login_merge_handle_if_missing()
     {
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
         DB::table('webauthn_credentials')->insert([
             'id'                   => self::CREDENTIAL_ID_ALT,
@@ -119,24 +163,66 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
             ->andReturn();
 
         $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_NO_HANDLE)
-            ->assertNoContent();
+            ->assertOk()
+            ->assertJsonFragment([
+                'message' => 'authenticated',
+                'name'    => $this->user->name,
+            ])
+            ->assertJsonStructure([
+                'message',
+                'name',
+                'preferences',
+            ]);
     }
 
     /**
      * @test
+     *
+     * @covers  \App\Http\Middleware\SkipIfAuthenticated
      */
-    public function test_webauthn_invalid_login_returns_error()
+    public function test_webauthn_login_already_authenticated_returns_success()
     {
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
-        $mock = $this->mock(AssertedRequest::class)->makePartial()->shouldIgnoreMissing();
-        $mock->shouldReceive([
-            'has'   => false,
-            'login' => null,
+        DB::table('webauthn_credentials')->insert([
+            'id'                   => self::CREDENTIAL_ID_ALT,
+            'authenticatable_type' => \App\Models\User::class,
+            'authenticatable_id'   => $this->user->id,
+            'user_id'              => self::USER_ID_ALT,
+            'counter'              => 0,
+            'rp_id'                => 'http://localhost',
+            'origin'               => 'http://localhost',
+            'aaguid'               => '00000000-0000-0000-0000-000000000000',
+            'attestation_format'   => 'none',
+            'public_key'           => self::PUBLIC_KEY,
+            'updated_at'           => now(),
+            'created_at'           => now(),
         ]);
 
-        $this->json('POST', '/webauthn/login')
-            ->assertNoContent(422);
+        $this->session(['_webauthn' => new \Laragear\WebAuthn\Challenge(
+            new \Laragear\WebAuthn\ByteBuffer(base64_decode(self::ASSERTION_CHALLENGE)),
+            60,
+            false,
+        )]);
+
+        $this->mock(AssertionValidator::class)
+            ->expects('send->thenReturn')
+            ->andReturn();
+
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE)
+            ->assertOk();
+
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE)
+            ->assertOk()
+            ->assertJsonFragment([
+                'message'     => 'authenticated',
+                'name'        => $this->user->name,
+            ])
+            ->assertJsonStructure([
+                'message',
+                'name',
+                'preferences',
+            ]);
     }
 
     /**
@@ -144,7 +230,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
      */
     public function test_webauthn_login_with_missing_data_returns_validation_error()
     {
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
         $data = [
             'id'       => '',
@@ -170,6 +256,46 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
             ]);
     }
 
+    /**
+     * @test
+     */
+    public function test_webauthn_invalid_login_returns_unauthorized()
+    {
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
+
+        $this->session(['_webauthn' => new \Laragear\WebAuthn\Challenge(
+            new \Laragear\WebAuthn\ByteBuffer(base64_decode(self::ASSERTION_CHALLENGE)),
+            60,
+            false,
+        )]);
+
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID)
+            ->assertUnauthorized();
+    }
+
+    /**
+     * @test
+     */
+    public function test_too_many_invalid_login_attempts_returns_too_many_request_error()
+    {
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
+
+        $this->session(['_webauthn' => new \Laragear\WebAuthn\Challenge(
+            new \Laragear\WebAuthn\ByteBuffer(base64_decode(self::ASSERTION_CHALLENGE)),
+            60,
+            false,
+        )]);
+
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+        $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+        $response = $this->json('POST', '/webauthn/login', self::ASSERTION_RESPONSE_INVALID);
+
+        $response->assertStatus(429);
+    }
+
     /**
      * @test
      */
@@ -177,7 +303,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
     {
         Config::set('webauthn.user_verification', WebAuthn::USER_VERIFICATION_REQUIRED);
 
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
         DB::table('webauthn_credentials')->insert([
             'id'                   => self::CREDENTIAL_ID,
@@ -219,7 +345,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
     {
         Config::set('webauthn.user_verification', WebAuthn::USER_VERIFICATION_DISCOURAGED);
 
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
         DB::table('webauthn_credentials')->insert([
             'id'                   => self::CREDENTIAL_ID,
@@ -259,7 +385,7 @@ class WebAuthnLoginControllerTest extends FeatureTestCase
      */
     public function test_get_options_with_capitalized_email_returns_success()
     {
-        $this->user = User::factory()->create();
+        $this->user = User::factory()->create(['email' => self::EMAIL]);
 
         $this->json('POST', '/webauthn/login/options', [
             'email' => strtoupper($this->user->email),