Adding security fixes, rate limiting and more

Author: tnylea

app/Actions/TwoFactorAuth/CompleteTwoFactorAuthentication.php

@@ -15,10 +15,13 @@ class CompleteTwoFactorAuthentication
      */
     public function __invoke($user): void
     {
-        // Log the user in
-        Auth::login($user);
+        // Get the remember preference from the session (default to false if not set)
+        $remember = Session::get('login.remember', false);
+        
+        // Log the user in with the remember preference
+        Auth::login($user, $remember);
 
-        // Clear the session that is used to determine if the user can visit the 2fa challenge page
-        Session::forget('login.id');
+        // Clear the session variables used for the 2FA challenge
+        Session::forget(['login.id', 'login.remember']);
     }
 }

app/Actions/TwoFactorAuth/ProcessRecoveryCode.php

@@ -26,7 +26,7 @@ public function __invoke(array $recoveryCodes, string $submittedCode)
 
         // Remove the used recovery code from the list
         $updatedCodes = array_values(array_filter($recoveryCodes, function($code) use ($submittedCode) {
-            return $code !== $submittedCode;
+            return !hash_equals($code, $submittedCode);
         }));
 
         return $updatedCodes;

app/Http/Controllers/Auth/AuthenticatedSessionController.php

@@ -35,8 +35,12 @@ public function store(LoginRequest $request): RedirectResponse
 
         // If this user exists, password is correct, and 2FA is enabled, we want to redirect to the 2FA challenge
         if ($user && $user->two_factor_confirmed_at && Hash::check($request->password, $user->password)) {
-            $request->session()->put('login.id', $user->getKey());
-            // Optionally clear any previous errors
+            // Store the user ID and remember preference in the session
+            $request->session()->put([
+                'login.id' => $user->getKey(),
+                'login.remember' => $request->boolean('remember')
+            ]);
+            
             return redirect()->route('two-factor.challenge');
         }
 

app/Http/Controllers/Auth/TwoFactorAuthChallengeController.php

@@ -7,6 +7,9 @@
 use App\Http\Controllers\Controller;
 use App\Models\User;
 use Illuminate\Http\Request;
+use Illuminate\Support\Facades\RateLimiter;
+use Illuminate\Validation\ValidationException;
+use Illuminate\Support\Str;
 
 class TwoFactorAuthChallengeController extends Controller
 {
@@ -26,6 +29,9 @@ public function store(Request $request)
         // If we made it here, user is available via the EnsureTwoFactorChallengeSession middleware
         $user = $request->two_factor_auth_user;
 
+        // Ensure the 2FA challenge is not rate limited
+        $this->ensureIsNotRateLimited($user);
+
         // Handle one-time password (OTP) code
         if ($request->filled('code')) {
             return $this->authenticateUsingCode($request, $user);
@@ -53,9 +59,11 @@ protected function authenticateUsingCode(Request $request, User $user)
 
         if ($valid) {
             app(CompleteTwoFactorAuthentication::class)($user);
+            RateLimiter::clear($this->throttleKey($user));
             return redirect()->intended(route('dashboard', absolute: false));
         }
 
+        RateLimiter::hit($this->throttleKey($user));
         return back()->withErrors(['code' => __('The provided two factor authentication code was invalid.')]);
     }
 
@@ -75,6 +83,7 @@ protected function authenticateUsingRecoveryCode(Request $request, User $user)
 
         // If ProcessRecoveryCode returns false, the code was invalid
         if ($updatedCodes === false) {
+            RateLimiter::hit($this->throttleKey($user));
             return back()->withErrors(['recovery_code' => __('The provided two factor authentication recovery code was invalid.')]);
         }
 
@@ -84,8 +93,45 @@ protected function authenticateUsingRecoveryCode(Request $request, User $user)
         // Complete the authentication process
         app(CompleteTwoFactorAuthentication::class)($user);
 
+        // Clear rate limiter after successful authentication
+        RateLimiter::clear($this->throttleKey($user));
+        
         // Redirect to the intended page
         return redirect()->intended(route('dashboard', absolute: false));
     }
+    
+    /**
+     * Ensure the 2FA challenge is not rate limited.
+     *
+     * @param  \App\Models\User  $user
+     * @return void
+     *
+     * @throws \Illuminate\Validation\ValidationException
+     */
+    protected function ensureIsNotRateLimited(User $user): void
+    {
+        if (! RateLimiter::tooManyAttempts($this->throttleKey($user), 5)) {
+            return;
+        }
+
+        $seconds = RateLimiter::availableIn($this->throttleKey($user));
+
+        throw ValidationException::withMessages([
+            'code' => __('Too many two factor authentication attempts. Please try again in :seconds seconds.', [
+                'seconds' => $seconds,
+            ]),
+        ]);
+    }
+
+    /**
+     * Get the rate limiting throttle key for the given user.
+     *
+     * @param  \App\Models\User  $user
+     * @return string
+     */
+    protected function throttleKey(User $user): string
+    {
+        return Str::transliterate($user->id . '|2fa|' . request()->ip());
+    }
 }