fix(auth): rotate remember_token on reset + atomic recovery-code use
Address Codex security review: - ForgotPassword + clusev:reset-admin now rotate remember_token, so a stolen remember-me cookie cannot survive a password reset (the email-token path already did this). - useRecoveryCode reads/checks/removes under a row lock (DB transaction + lockForUpdate), so two concurrent requests can't both spend the same one-time code (replay). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>feat/v1-foundation
parent
f6a65827fd
commit
3a67aa99ec
|
|
@ -35,7 +35,12 @@ class ResetAdmin extends Command
|
|||
}
|
||||
|
||||
$password = (string) ($this->option('password') ?: Str::password(20));
|
||||
$user->forceFill(['password' => Hash::make($password), 'must_change_password' => false]);
|
||||
// Rotate remember_token so any previously issued remember-me cookie is revoked.
|
||||
$user->forceFill([
|
||||
'password' => Hash::make($password),
|
||||
'must_change_password' => false,
|
||||
'remember_token' => Str::random(60),
|
||||
]);
|
||||
|
||||
if ($this->option('disable-2fa')) {
|
||||
$user->forceFill([
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ use App\Models\AuditEvent;
|
|||
use App\Models\User;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
use Illuminate\Support\Facades\RateLimiter;
|
||||
use Illuminate\Support\Str;
|
||||
use Illuminate\Validation\Rules\Password;
|
||||
use Illuminate\Validation\ValidationException;
|
||||
use Livewire\Attributes\Layout;
|
||||
|
|
@ -73,7 +74,12 @@ class ForgotPassword extends Component
|
|||
}
|
||||
|
||||
RateLimiter::clear($key);
|
||||
$user->forceFill(['password' => Hash::make($this->password), 'must_change_password' => false])->save();
|
||||
// Rotate remember_token too, so a stolen remember-me cookie can't survive the reset.
|
||||
$user->forceFill([
|
||||
'password' => Hash::make($this->password),
|
||||
'must_change_password' => false,
|
||||
'remember_token' => Str::random(60),
|
||||
])->save();
|
||||
|
||||
AuditEvent::create([
|
||||
'user_id' => $user->id,
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Attributes\Hidden;
|
|||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||
use Illuminate\Foundation\Auth\User as Authenticatable;
|
||||
use Illuminate\Notifications\Notifiable;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Str;
|
||||
|
||||
#[Fillable(['name', 'email', 'password', 'must_change_password', 'locale'])]
|
||||
|
|
@ -53,21 +54,31 @@ class User extends Authenticatable
|
|||
return count($this->recoveryCodes()) > 0;
|
||||
}
|
||||
|
||||
/** Consume one recovery code (one-time). Returns true if it was valid + removed. */
|
||||
/**
|
||||
* Consume one recovery code (one-time). Returns true if it was valid + removed.
|
||||
* Atomic: the read/check/remove runs under a row lock so two concurrent requests
|
||||
* can never both spend the same code (replay).
|
||||
*/
|
||||
public function useRecoveryCode(string $code): bool
|
||||
{
|
||||
$code = trim($code);
|
||||
$codes = $this->recoveryCodes();
|
||||
|
||||
if (! in_array($code, $codes, true)) {
|
||||
return false;
|
||||
}
|
||||
return DB::transaction(function () use ($code) {
|
||||
$locked = static::whereKey($this->getKey())->lockForUpdate()->first();
|
||||
$codes = $locked?->recoveryCodes() ?? [];
|
||||
|
||||
$this->forceFill([
|
||||
'two_factor_recovery_codes' => array_values(array_filter($codes, fn ($c) => $c !== $code)),
|
||||
])->save();
|
||||
if (! in_array($code, $codes, true)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
$remaining = array_values(array_filter($codes, fn ($c) => $c !== $code));
|
||||
$locked->forceFill(['two_factor_recovery_codes' => $remaining])->save();
|
||||
|
||||
// Keep the in-memory instance consistent for the rest of the request.
|
||||
$this->setRawAttributes($locked->getAttributes(), true);
|
||||
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
public function hasTwoFactorEnabled(): bool
|
||||
|
|
|
|||
|
|
@ -23,9 +23,10 @@ class ForgotPasswordTest extends TestCase
|
|||
]);
|
||||
}
|
||||
|
||||
public function test_totp_resets_password(): void
|
||||
public function test_totp_resets_password_and_rotates_remember_token(): void
|
||||
{
|
||||
$user = $this->user();
|
||||
$user->forceFill(['remember_token' => 'stale-cookie-token'])->save();
|
||||
$otp = (new Google2FA)->getCurrentOtp($user->two_factor_secret);
|
||||
|
||||
Livewire::test(ForgotPassword::class)
|
||||
|
|
@ -34,6 +35,8 @@ class ForgotPasswordTest extends TestCase
|
|||
->call('resetPassword')->assertHasNoErrors();
|
||||
|
||||
$this->assertTrue(Hash::check('NewPassw0rd123', $user->fresh()->password));
|
||||
// a stolen remember-me cookie must not survive the reset
|
||||
$this->assertNotSame('stale-cookie-token', $user->fresh()->remember_token);
|
||||
}
|
||||
|
||||
public function test_backup_code_resets_and_is_consumed(): void
|
||||
|
|
|
|||
Loading…
Reference in New Issue