Fix: Locking User on Login to manage otp attempts
This commit is contained in:
@@ -5,6 +5,7 @@ namespace Modules\Core\Auth\Services;
|
|||||||
use Illuminate\Contracts\Auth\Authenticatable;
|
use Illuminate\Contracts\Auth\Authenticatable;
|
||||||
use Illuminate\Http\Request;
|
use Illuminate\Http\Request;
|
||||||
use Illuminate\Support\Facades\Auth;
|
use Illuminate\Support\Facades\Auth;
|
||||||
|
use Illuminate\Support\Facades\DB;
|
||||||
use Illuminate\Support\Facades\Event;
|
use Illuminate\Support\Facades\Event;
|
||||||
use Illuminate\Support\Facades\Mail;
|
use Illuminate\Support\Facades\Mail;
|
||||||
use Illuminate\Support\Facades\RateLimiter;
|
use Illuminate\Support\Facades\RateLimiter;
|
||||||
@@ -97,13 +98,21 @@ class UserOtpService
|
|||||||
public function validate(string $email, string $code, ?Request $request = null): ?Authenticatable
|
public function validate(string $email, string $code, ?Request $request = null): ?Authenticatable
|
||||||
{
|
{
|
||||||
$model = config('auth.providers.users.model');
|
$model = config('auth.providers.users.model');
|
||||||
$user = $model::where('email', $email)->first();
|
|
||||||
|
// 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 || ! $user->otp_expires_at || now()->isAfter($user->otp_expires_at)) {
|
if (! $user || ! $user->otp_expires_at || now()->isAfter($user->otp_expires_at)) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($user->otp_code != $code) {
|
if (! hash_equals((string) $user->otp_code, $code)) {
|
||||||
$user->otp_attempts++;
|
$user->otp_attempts++;
|
||||||
|
|
||||||
if ($user->otp_attempts >= (int) config('core.auth.otp.max_attempts', 5)) {
|
if ($user->otp_attempts >= (int) config('core.auth.otp.max_attempts', 5)) {
|
||||||
@@ -122,15 +131,22 @@ class UserOtpService
|
|||||||
$user->otp_attempts = 0;
|
$user->otp_attempts = 0;
|
||||||
$user->save();
|
$user->save();
|
||||||
|
|
||||||
|
return $user;
|
||||||
|
});
|
||||||
|
|
||||||
|
if (! $result) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
RateLimiter::clear($this->generationLimiterKey($email));
|
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
|
private function generationLimiterKey(string $email): string
|
||||||
|
|||||||
Reference in New Issue
Block a user