diff --git a/database/migrations/2026_09_30_000001_hash_otp_code_on_users_table.php b/database/migrations/2026_09_30_000001_hash_otp_code_on_users_table.php new file mode 100644 index 0000000..70dd2ac --- /dev/null +++ b/database/migrations/2026_09_30_000001_hash_otp_code_on_users_table.php @@ -0,0 +1,43 @@ +string('otp_code_hash')->nullable()->after('password'); + }); + + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('otp_code'); + }); + } + + public function down(): void + { + Schema::table('users', function (Blueprint $table) { + $table->string('otp_code', 6)->nullable()->after('password'); + }); + + Schema::table('users', function (Blueprint $table) { + $table->dropColumn('otp_code_hash'); + }); + } +}; diff --git a/database/migrations/2026_09_30_000002_hash_otp_code_on_lunar_staff_table.php b/database/migrations/2026_09_30_000002_hash_otp_code_on_lunar_staff_table.php new file mode 100644 index 0000000..a8f2efd --- /dev/null +++ b/database/migrations/2026_09_30_000002_hash_otp_code_on_lunar_staff_table.php @@ -0,0 +1,37 @@ +string('otp_code_hash')->nullable()->after('password'); + }); + + Schema::table('lunar_staff', function (Blueprint $table) { + $table->dropColumn('otp_code'); + }); + } + + public function down(): void + { + Schema::table('lunar_staff', function (Blueprint $table) { + $table->string('otp_code', 6)->nullable()->after('password'); + }); + + Schema::table('lunar_staff', function (Blueprint $table) { + $table->dropColumn('otp_code_hash'); + }); + } +}; diff --git a/docs/lunar.md b/docs/lunar.md index f21eb27..6db907a 100644 --- a/docs/lunar.md +++ b/docs/lunar.md @@ -393,7 +393,7 @@ Because Filament instantiates `Lunar\Admin\Models\Staff` directly (not a subclas ```php use Lunar\Admin\Models\Staff as LunarStaff; -LunarStaff::addActivitylogExcept(['otp_code', 'otp_expires_at', 'password']); +LunarStaff::addActivitylogExcept(['otp_code_hash', 'otp_expires_at', 'password']); ``` --- diff --git a/docs/otp-auth.md b/docs/otp-auth.md index 5dd8832..e07663a 100644 --- a/docs/otp-auth.md +++ b/docs/otp-auth.md @@ -30,11 +30,12 @@ Codes expire after **10 minutes**. After a successful validation the code is cle ### Database -Two columns on the `lunar_staff` table (added by `2026_05_06_000001_add_otp_to_lunar_staff_table`): +Two columns on the `lunar_staff` table (added by `2026_05_06_000001_add_otp_to_lunar_staff_table`, +`otp_code` replaced with a hashed column by `2026_09_30_000002_hash_otp_code_on_lunar_staff_table`): | Column | Type | Purpose | |---|---|---| -| `otp_code` | string, nullable | The generated code | +| `otp_code_hash` | string, nullable | Bcrypt hash of the generated code (`'hashed'` cast on `Staff`) | | `otp_expires_at` | timestamp, nullable | Expiry time | ### Login Page diff --git a/src/Auth/Models/Staff.php b/src/Auth/Models/Staff.php index 900d39e..5d2ba2a 100644 --- a/src/Auth/Models/Staff.php +++ b/src/Auth/Models/Staff.php @@ -11,7 +11,7 @@ class Staff extends ModelsStaff 'last_name', 'admin', 'email', - 'otp_code', + 'otp_code_hash', 'otp_expires_at', ]; @@ -19,6 +19,18 @@ class Staff extends ModelsStaff 'admin' => 'bool', 'email_verified_at' => 'datetime', 'password' => 'hashed', + 'otp_code_hash' => 'hashed', 'otp_expires_at' => 'datetime', ]; + + // Overrides (doesn't merge with) Lunar\Admin\Models\Staff's own + // $hidden — repeats its password/remember_token here so this class + // doesn't silently drop that protection while adding otp_code_hash/ + // otp_expires_at, which the base model has no reason to know about. + protected $hidden = [ + 'password', + 'remember_token', + 'otp_code_hash', + 'otp_expires_at', + ]; } diff --git a/src/Auth/Services/OtpService.php b/src/Auth/Services/OtpService.php index d5b813f..d524df3 100644 --- a/src/Auth/Services/OtpService.php +++ b/src/Auth/Services/OtpService.php @@ -2,6 +2,7 @@ namespace Modules\Core\Auth\Services; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Mail; use Modules\Core\Auth\Mail\OtpMail; use Modules\Core\Auth\Models\Staff; @@ -27,7 +28,10 @@ class OtpService $code = str_pad((string) random_int(0, 999999), self::CODE_LENGTH, '0', STR_PAD_LEFT); - $staff->otp_code = $code; + // otp_code_hash's 'hashed' cast (see Staff's own $casts) hashes + // this automatically on assignment, same as password — never + // stored or compared in plaintext. + $staff->otp_code_hash = $code; $staff->otp_expires_at = now()->addMinutes(self::EXPIRY_MINUTES); $staff->save(); @@ -44,11 +48,15 @@ class OtpService return null; } - if (! $staff->otp_expires_at || $staff->otp_code != $code || now()->isAfter($staff->otp_expires_at)) { + if (! $staff->otp_code_hash || ! $staff->otp_expires_at || now()->isAfter($staff->otp_expires_at)) { return null; } - $staff->otp_code = null; + if (! Hash::check($code, $staff->otp_code_hash)) { + return null; + } + + $staff->otp_code_hash = null; $staff->otp_expires_at = null; $staff->save(); diff --git a/src/Auth/Services/UserOtpService.php b/src/Auth/Services/UserOtpService.php index 8a43380..df5c4ef 100644 --- a/src/Auth/Services/UserOtpService.php +++ b/src/Auth/Services/UserOtpService.php @@ -8,6 +8,7 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\RateLimiter; use Modules\Core\Auth\Events\UserAuthenticated; @@ -40,8 +41,11 @@ use Modules\Core\Auth\Mail\UserOtpMail; * 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. + * row's own otp_code_hash/otp_expires_at/otp_attempts columns exactly as + * before, so a returning shopper's login is unchanged. otp_code_hash + * holds a bcrypt hash of the code (the 'otp_code_hash' => 'hashed' cast + * on App\Models\User hashes it automatically on assignment, same as + * password), not the code itself — compared via Hash::check(). * * Two independent throttles, both configured under core.auth.otp — see * config/core.php's own comment for why they're separate: max_attempts @@ -87,7 +91,7 @@ class UserOtpService $code = str_pad((string) random_int(0, 999999), self::CODE_LENGTH, '0', STR_PAD_LEFT); if ($user) { - $user->otp_code = $code; + $user->otp_code_hash = $code; $user->otp_expires_at = now()->addMinutes(self::EXPIRY_MINUTES); $user->otp_attempts = 0; $user->save(); @@ -96,8 +100,12 @@ class UserOtpService // 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. + // + // Hashed even in the cache (not just on the DB-backed path) + // — a code sitting in Cache::get()-able storage is the same + // exposure as a plaintext DB column if anything can read it. Cache::put($this->pendingKey($email), [ - 'code' => $code, + 'code_hash' => Hash::make($code), 'expires_at' => now()->addMinutes(self::EXPIRY_MINUTES)->timestamp, 'attempts' => 0, ], now()->addMinutes(self::EXPIRY_MINUTES)); @@ -154,15 +162,15 @@ class UserOtpService 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)) { + if (! $user || ! $user->otp_code_hash || ! $user->otp_expires_at || now()->isAfter($user->otp_expires_at)) { return null; } - if (! hash_equals((string) $user->otp_code, $code)) { + if (! Hash::check($code, $user->otp_code_hash)) { $user->otp_attempts++; if ($user->otp_attempts >= (int) config('core.auth.otp.max_attempts', 5)) { - $user->otp_code = null; + $user->otp_code_hash = null; $user->otp_expires_at = null; $user->otp_attempts = 0; } @@ -172,7 +180,7 @@ class UserOtpService return null; } - $user->otp_code = null; + $user->otp_code_hash = null; $user->otp_expires_at = null; $user->otp_attempts = 0; $user->save(); @@ -200,7 +208,7 @@ class UserOtpService return null; } - if (! hash_equals((string) $pending['code'], $code)) { + if (! Hash::check($code, $pending['code_hash'])) { $pending['attempts']++; if ($pending['attempts'] >= (int) config('core.auth.otp.max_attempts', 5)) { diff --git a/src/CorePlugin.php b/src/CorePlugin.php index afd9a47..1966a68 100644 --- a/src/CorePlugin.php +++ b/src/CorePlugin.php @@ -150,7 +150,7 @@ class CorePlugin implements Plugin }); LunarStaff::addActivitylogExcept([ - 'otp_code', + 'otp_code_hash', 'otp_expires_at', 'password', 'remember_token', diff --git a/src/Customer/Privacy/CustomerDataProvider.php b/src/Customer/Privacy/CustomerDataProvider.php index 7019be6..f4c12df 100644 --- a/src/Customer/Privacy/CustomerDataProvider.php +++ b/src/Customer/Privacy/CustomerDataProvider.php @@ -115,7 +115,7 @@ class CustomerDataProvider implements PersonalDataProvider // secret tied to an identity that no longer exists here — clear // it alongside name/email rather than leaving it to expire on // its own 10-minute window. - 'otp_code' => null, + 'otp_code_hash' => null, 'otp_expires_at' => null, 'otp_attempts' => 0, ]); diff --git a/src/Customer/Services/CustomerEmailChangeService.php b/src/Customer/Services/CustomerEmailChangeService.php index 54ceb5f..a5292c5 100644 --- a/src/Customer/Services/CustomerEmailChangeService.php +++ b/src/Customer/Services/CustomerEmailChangeService.php @@ -23,7 +23,7 @@ use Modules\Core\Customer\Exceptions\InvalidEmailChangeCodeException; * hash of the code, expiry, wrong-guess count) lives on the user's own * row (see the migration adding pending_email/pending_email_code_hash/ * pending_email_expires_at/pending_email_attempts) — the same convention - * Auth\Services\UserOtpService's otp_code/otp_expires_at/otp_attempts + * Auth\Services\UserOtpService's otp_code_hash/otp_expires_at/otp_attempts * already use — rather than the session, since a code arrives by email * and is often opened on a different device/session than the one that * requested it; a session-scoped pending change couldn't be confirmed