boboko/core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Chore: Storing UserOtp in cache before he is registered

Browse Source
This commit is contained in:
arvanitakis
2026-09-25 20:10:29 +03:00
parent 985f53efa2
commit ccab9bbb8e
1 changed files with 113 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Auth/Services/UserOtpService.php
+107 -34
View File
@@ -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\Cache;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Mail;
@@ -29,17 +30,25 @@ use Modules\Core\Auth\Mail\UserOtpMail;
* here: doing our own on top would run a SECOND merge attempt with a
* hardcoded policy that ignores whatever the consumer configured.
*
* generateAndSend()'s find-or-create already triggers the full
* Customer/User pairing cascade for a genuinely new email — see
* Modules\Core\Auth\Events\UserCreated's own docblock and
* Modules\Core\Customer\Listeners\CreateCustomerForUser.
* generateAndSend() does NOT create a User row for an email it hasn't
* seen before — it used to (firstOrCreate() ran unconditionally), which
* meant this login FORM was effectively a registration form: anyone could
* create a real User (and, via UserCreated's own cascade, a paired
* Customer) for any email address they liked, whether or not a single
* correct code was ever entered. A genuinely new email's pending code now
* lives in the cache (see pendingKey()), keyed by email, with no DB row
* at all — firstOrCreate() and UserCreated only fire from validate(), and
* only once the code has actually been proven correct. An email that
* already has a User row is unaffected: its OTP state still lives on that
* row's own otp_code/otp_expires_at/otp_attempts columns exactly as
* before, so a returning shopper's login is unchanged.
*
* Two independent throttles, both configured under core.auth.otp — see
* config/core.php's own comment for why they're separate: max_attempts
* caps wrong guesses against ONE code; generation_limit caps how often a
* NEW code can be requested for the same email at all (closes both the
* "regenerate to reset my guess count" loophole and mail-bombing one
* inbox).
* inbox). Both apply identically whether or not a User row exists yet.
*
* validate() also records a UserSessionService entry for the new login —
* see that class's own docblock for the "logout everywhere" registry
@@ -74,28 +83,27 @@ class UserOtpService
RateLimiter::hit($limiterKey, (int) config('core.auth.otp.generation_decay_minutes', 10) * 60);
$model = config('auth.providers.users.model');
$user = $model::firstOrCreate(['email' => $email]);
// wasRecentlyCreated is Eloquent's own "did firstOrCreate() just
// INSERT, or did it find an existing row" flag — the only reliable
// way to tell them apart from firstOrCreate()'s return value alone.
// Without this check, a genuinely new signup never fired
// UserCreated at all (this class's own docblock claimed the
// Customer/User pairing cascade "already triggers" here, which was
// false as written — see Modules\Core\Customer\Listeners\
// CreateCustomerForUser, which depends entirely on this event).
if ($user->wasRecentlyCreated) {
Event::dispatch(new UserCreated($user));
}
$user = $model::where('email', $email)->first();
$code = str_pad((string) random_int(0, 999999), self::CODE_LENGTH, '0', STR_PAD_LEFT);
if ($user) {
$user->otp_code = $code;
$user->otp_expires_at = now()->addMinutes(self::EXPIRY_MINUTES);
$user->otp_attempts = 0;
$user->save();
} else {
// No row yet — deliberately not created here. See this
// class's own docblock for why: creating one on every
// generateAndSend() call let anyone mint real User/Customer
// rows for an email nobody proved they owned.
Cache::put($this->pendingKey($email), [
'code' => $code,
'expires_at' => now()->addMinutes(self::EXPIRY_MINUTES)->timestamp,
'attempts' => 0,
], now()->addMinutes(self::EXPIRY_MINUTES));
}
Mail::to($user->email)->send(new UserOtpMail($user->name ?? $user->email, $code));
Mail::to($email)->send(new UserOtpMail($user->name ?? $email, $code));
return true;
}
@@ -106,19 +114,44 @@ class UserOtpService
* a fresh one via generateAndSend() (itself throttled independently
* — see this class's own docblock) rather than being able to keep
* guessing against a still-live code for the rest of its 10-minute
* expiry window.
* expiry window. Applies identically to the cache-backed (no User row
* yet) and DB-backed (existing User row) paths.
*/
public function validate(string $email, string $code, ?Request $request = null): ?Authenticatable
{
$model = config('auth.providers.users.model');
$existing = $model::where('email', $email)->exists();
// 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) {
$result = $existing
? $this->validateExisting($model, $email, $code)
: $this->validatePending($model, $email, $code);
if (! $result) {
return null;
}
RateLimiter::clear($this->generationLimiterKey($email));
Auth::login($result);
$this->sessions->record($result, $request);
Event::dispatch(new UserAuthenticated($result));
return $result;
}
/**
* lockForUpdate() + a transaction make the read-check-increment-save
* 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.
*/
private function validateExisting(string $model, string $email, string $code): ?Authenticatable
{
return 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)) {
@@ -146,24 +179,64 @@ class UserOtpService
return $user;
});
}
if (! $result) {
/**
* No User row exists yet, so there's nothing to lockForUpdate() —
* Cache::lock() is the equivalent guard against two parallel guesses
* against the same pending signup both reading the same pre-increment
* attempts count. The User (and, via UserCreated, its paired Customer)
* is only ever created here, once the code has actually been proven
* correct — never from generateAndSend().
*/
private function validatePending(string $model, string $email, string $code): ?Authenticatable
{
$key = $this->pendingKey($email);
return Cache::lock("{$key}:lock", 10)->block(5, function () use ($model, $email, $code, $key) {
$pending = Cache::get($key);
if (! $pending || now()->timestamp > $pending['expires_at']) {
return null;
}
RateLimiter::clear($this->generationLimiterKey($email));
if (! hash_equals((string) $pending['code'], $code)) {
$pending['attempts']++;
Auth::login($result);
if ($pending['attempts'] >= (int) config('core.auth.otp.max_attempts', 5)) {
Cache::forget($key);
} else {
Cache::put($key, $pending, now()->addMinutes(self::EXPIRY_MINUTES));
}
$this->sessions->record($result, $request);
return null;
}
Event::dispatch(new UserAuthenticated($result));
Cache::forget($key);
return $result;
$user = $model::firstOrCreate(['email' => $email]);
// wasRecentlyCreated is Eloquent's own "did firstOrCreate()
// just INSERT, or did it find an existing row" flag. Always
// true here in practice (validatePending() only runs when no
// row existed moments ago), but checked anyway rather than
// assumed, in case of an extremely unlikely race with a
// signup completed through some other path in between.
if ($user->wasRecentlyCreated) {
Event::dispatch(new UserCreated($user));
}
return $user;
});
}
private function generationLimiterKey(string $email): string
{
return 'otp-generate:'.strtolower($email);
}
private function pendingKey(string $email): string
{
return 'otp-pending:'.strtolower($email);
}
}