From 0babc6a96da42797a5f3d7d42e9d11b1b8097a2a Mon Sep 17 00:00:00 2001
From: Konstantinos Arvanitakis <arvanitakis@radical-elements.com>
Date: Wed, 16 Sep 2026 00:05:53 +0300
Subject: [PATCH] Fix: Locking User on Login to manage otp attempts

---
 src/Auth/Services/UserOtpService.php | 56 ++++++++++++++++++----------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/src/Auth/Services/UserOtpService.php b/src/Auth/Services/UserOtpService.php
index 7bc15da..ce82abd 100644
--- a/src/Auth/Services/UserOtpService.php
+++ b/src/Auth/Services/UserOtpService.php
@@ -5,6 +5,7 @@ namespace Modules\Core\Auth\Services;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Auth;
+use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Facades\RateLimiter;
@@ -97,40 +98,55 @@ class UserOtpService
     public function validate(string $email, string $code, ?Request $request = null): ?Authenticatable
     {
         $model = config('auth.providers.users.model');
-        $user = $model::where('email', $email)->first();
 
-        if (! $user || ! $user->otp_expires_at || now()->isAfter($user->otp_expires_at)) {
-            return null;
-        }
+        // lockForUpdate() + a transaction make the read-check-increment-save
+        // below atomic across concurrent requests for the same user — without
+        // it, two guesses fired in parallel can each read the same
+        // pre-increment otp_attempts value and both save past
+        // max_attempts, letting an attacker exceed the lockout by
+        // parallelizing requests instead of sending them serially.
+        $result = DB::transaction(function () use ($model, $email, $code) {
+            $user = $model::where('email', $email)->lockForUpdate()->first();
 
-        if ($user->otp_code != $code) {
-            $user->otp_attempts++;
-
-            if ($user->otp_attempts >= (int) config('core.auth.otp.max_attempts', 5)) {
-                $user->otp_code = null;
-                $user->otp_expires_at = null;
-                $user->otp_attempts = 0;
+            if (! $user || ! $user->otp_expires_at || now()->isAfter($user->otp_expires_at)) {
+                return null;
             }
 
+            if (! hash_equals((string) $user->otp_code, $code)) {
+                $user->otp_attempts++;
+
+                if ($user->otp_attempts >= (int) config('core.auth.otp.max_attempts', 5)) {
+                    $user->otp_code = null;
+                    $user->otp_expires_at = null;
+                    $user->otp_attempts = 0;
+                }
+
+                $user->save();
+
+                return null;
+            }
+
+            $user->otp_code = null;
+            $user->otp_expires_at = null;
+            $user->otp_attempts = 0;
             $user->save();
 
+            return $user;
+        });
+
+        if (! $result) {
             return null;
         }
 
-        $user->otp_code = null;
-        $user->otp_expires_at = null;
-        $user->otp_attempts = 0;
-        $user->save();
-
         RateLimiter::clear($this->generationLimiterKey($email));
 
-        Auth::login($user);
+        Auth::login($result);
 
-        $this->sessions->record($user, $request);
+        $this->sessions->record($result, $request);
 
-        Event::dispatch(new CustomerLoggedIn($user));
+        Event::dispatch(new CustomerLoggedIn($result));
 
-        return $user;
+        return $result;
     }
 
     private function generationLimiterKey(string $email): string