Merge branch 'feature/set_reset_password_expiration' into dev
This commit is contained in:
@@ -22,10 +22,18 @@ class ValidateResetPasswordAttemptController extends Controller
|
||||
{
|
||||
$data = $request->validated();
|
||||
|
||||
if (! $this->resetPasswordAttemptService->validateCode(
|
||||
$result = $this->resetPasswordAttemptService->validateCode(
|
||||
$data['email'],
|
||||
$data['codigo'],
|
||||
)) {
|
||||
);
|
||||
|
||||
if ($result === ResetPasswordAttemptService::CODE_EXPIRED) {
|
||||
throw ValidationException::withMessages([
|
||||
'codigo' => __('api.auth.reset_code_expired'),
|
||||
]);
|
||||
}
|
||||
|
||||
if ($result !== ResetPasswordAttemptService::CODE_VALID) {
|
||||
throw ValidationException::withMessages([
|
||||
'codigo' => __('api.auth.reset_code_invalid'),
|
||||
]);
|
||||
|
||||
@@ -7,7 +7,7 @@ use Illuminate\Database\Eloquent\Attributes\Hidden;
|
||||
use Illuminate\Database\Eloquent\Model;
|
||||
use Illuminate\Database\Eloquent\Relations\BelongsTo;
|
||||
|
||||
#[Fillable(['user_id', 'codigo', 'reason', 'status'])]
|
||||
#[Fillable(['user_id', 'codigo', 'reason', 'status', 'expires_at'])]
|
||||
#[Hidden(['codigo'])]
|
||||
class ResetPasswordAttempt extends Model
|
||||
{
|
||||
@@ -31,6 +31,7 @@ class ResetPasswordAttempt extends Model
|
||||
{
|
||||
return [
|
||||
'user_id' => 'integer',
|
||||
'expires_at' => 'datetime',
|
||||
];
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,12 @@ use Throwable;
|
||||
|
||||
class ResetPasswordAttemptService
|
||||
{
|
||||
public const CODE_VALID = 'valid';
|
||||
|
||||
public const CODE_INVALID = 'invalid';
|
||||
|
||||
public const CODE_EXPIRED = 'expired';
|
||||
|
||||
public function createForEmail(
|
||||
string $email,
|
||||
string $tenantCode,
|
||||
@@ -146,12 +152,12 @@ class ResetPasswordAttemptService
|
||||
);
|
||||
}
|
||||
|
||||
public function validateCode(string $email, string $code): bool
|
||||
public function validateCode(string $email, string $code): string
|
||||
{
|
||||
$emailFingerprint = $this->emailFingerprint($email);
|
||||
|
||||
try {
|
||||
return DB::transaction(function () use ($email, $code, $emailFingerprint): bool {
|
||||
return DB::transaction(function () use ($email, $code, $emailFingerprint): string {
|
||||
$user = User::query()
|
||||
->where('email', $email)
|
||||
->lockForUpdate()
|
||||
@@ -169,14 +175,27 @@ class ResetPasswordAttemptService
|
||||
'email_fingerprint' => $emailFingerprint,
|
||||
]);
|
||||
|
||||
return false;
|
||||
return self::CODE_INVALID;
|
||||
}
|
||||
|
||||
if ($attempt->expires_at?->isPast()) {
|
||||
$attempt->update([
|
||||
'status' => ResetPasswordAttempt::STATUS_EXPIRED,
|
||||
]);
|
||||
|
||||
Log::info('Password reset code validation failed: attempt expired.', [
|
||||
'email_fingerprint' => $emailFingerprint,
|
||||
'attempt_id' => $attempt->getKey(),
|
||||
]);
|
||||
|
||||
return self::CODE_EXPIRED;
|
||||
}
|
||||
|
||||
$attempt->update([
|
||||
'status' => ResetPasswordAttempt::STATUS_VALIDATED,
|
||||
]);
|
||||
|
||||
return true;
|
||||
return self::CODE_VALID;
|
||||
});
|
||||
} catch (Throwable $exception) {
|
||||
Log::error('Failed to validate password reset code.', [
|
||||
@@ -214,6 +233,19 @@ class ResetPasswordAttemptService
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($attempt->expires_at?->isPast()) {
|
||||
$attempt->update([
|
||||
'status' => ResetPasswordAttempt::STATUS_EXPIRED,
|
||||
]);
|
||||
|
||||
Log::info('Password reset failed: attempt expired.', [
|
||||
'email_fingerprint' => $emailFingerprint,
|
||||
'attempt_id' => $attempt->getKey(),
|
||||
]);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
$user->password = $password;
|
||||
$user->failed_login_attempts = 0;
|
||||
$user->last_failed_login_at = null;
|
||||
@@ -270,6 +302,7 @@ class ResetPasswordAttemptService
|
||||
'codigo' => $this->generateCode(),
|
||||
'reason' => $reason,
|
||||
'status' => ResetPasswordAttempt::STATUS_PENDING,
|
||||
'expires_at' => now()->addMinutes((int) config('auth.passwords.users.expire')),
|
||||
]);
|
||||
|
||||
return $attempt->getKey();
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
<?php
|
||||
|
||||
use App\Domains\Auth\Models\ResetPasswordAttempt;
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
public function up(): void
|
||||
{
|
||||
Schema::table('reset_password_attempts', function (Blueprint $table): void {
|
||||
$table->timestamp('expires_at')->nullable()->after('status');
|
||||
});
|
||||
|
||||
// Attempts created before this migration did not have an expiration instant.
|
||||
DB::table('reset_password_attempts')
|
||||
->whereIn('status', [
|
||||
ResetPasswordAttempt::STATUS_PENDING,
|
||||
ResetPasswordAttempt::STATUS_VALIDATED,
|
||||
])
|
||||
->update(['status' => ResetPasswordAttempt::STATUS_EXPIRED]);
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
{
|
||||
Schema::table('reset_password_attempts', function (Blueprint $table): void {
|
||||
$table->dropColumn('expires_at');
|
||||
});
|
||||
}
|
||||
};
|
||||
@@ -12,6 +12,7 @@ return [
|
||||
'password_reset_invalid' => 'The password recovery request is invalid or has already been used.',
|
||||
'password_updated' => 'Password updated successfully.',
|
||||
'reset_code_invalid' => 'The code you entered is invalid.',
|
||||
'reset_code_expired' => 'The password recovery code expired. Request a new one.',
|
||||
'reset_code_valid' => 'Code validated successfully.',
|
||||
'invalid_tenant_or_return_url' => 'The tenant or return URL is invalid.',
|
||||
'request_expired' => 'The authentication request expired. Please try again.',
|
||||
|
||||
@@ -12,6 +12,7 @@ return [
|
||||
'password_reset_invalid' => 'La solicitud de recuperación es inválida o ya fue utilizada.',
|
||||
'password_updated' => 'Contraseña modificada correctamente.',
|
||||
'reset_code_invalid' => 'El código ingresado es inválido.',
|
||||
'reset_code_expired' => 'El código de recuperación expiró. Solicitá uno nuevo.',
|
||||
'reset_code_valid' => 'Código validado correctamente.',
|
||||
'invalid_tenant_or_return_url' => 'El tenant o la URL de retorno no son válidos.',
|
||||
'request_expired' => 'La solicitud de autenticación expiró. Intenta nuevamente.',
|
||||
|
||||
@@ -69,6 +69,10 @@ class CreateResetPasswordAttemptControllerTest extends TestCase
|
||||
$this->assertTrue($attempt->user->is($user));
|
||||
$this->assertMatchesRegularExpression('/^\d{4}$/', $attempt->codigo);
|
||||
$this->assertSame(ResetPasswordAttempt::STATUS_PENDING, $attempt->status);
|
||||
$this->assertTrue($attempt->expires_at->between(
|
||||
now()->addMinutes(59),
|
||||
now()->addMinutes(60),
|
||||
));
|
||||
Event::assertDispatched(
|
||||
PasswordResetRequested::class,
|
||||
fn (PasswordResetRequested $event): bool => $event->attemptId === $attempt->id
|
||||
|
||||
@@ -18,7 +18,9 @@ class ResetPasswordAttemptTest extends TestCase
|
||||
'id',
|
||||
'user_id',
|
||||
'codigo',
|
||||
'reason',
|
||||
'status',
|
||||
'expires_at',
|
||||
], Schema::getColumnListing('reset_password_attempts'));
|
||||
}
|
||||
|
||||
@@ -28,12 +30,14 @@ class ResetPasswordAttemptTest extends TestCase
|
||||
|
||||
$attempt = $user->resetPasswordAttempts()->create([
|
||||
'codigo' => '123456',
|
||||
'expires_at' => now()->addHour(),
|
||||
]);
|
||||
|
||||
$this->assertSame(ResetPasswordAttempt::STATUS_PENDING, $attempt->status);
|
||||
$this->assertTrue($attempt->user->is($user));
|
||||
$this->assertTrue($user->resetPasswordAttempts->contains($attempt));
|
||||
$this->assertFalse($attempt->usesTimestamps());
|
||||
$this->assertTrue($attempt->expires_at->isFuture());
|
||||
$this->assertArrayNotHasKey('codigo', $attempt->toArray());
|
||||
}
|
||||
|
||||
|
||||
@@ -99,6 +99,30 @@ class ResetPasswordControllerTest extends TestCase
|
||||
$this->assertSame(ResetPasswordAttempt::STATUS_USED, $attempt->fresh()->status);
|
||||
}
|
||||
|
||||
public function test_an_expired_validated_attempt_cannot_reset_the_password(): void
|
||||
{
|
||||
$user = User::factory()->create([
|
||||
'email' => 'ada@example.com',
|
||||
'password' => 'OldSecret!123',
|
||||
]);
|
||||
$attempt = $user->resetPasswordAttempts()->create([
|
||||
'codigo' => '1234',
|
||||
'status' => ResetPasswordAttempt::STATUS_VALIDATED,
|
||||
'expires_at' => now()->subSecond(),
|
||||
]);
|
||||
|
||||
$this->postJson('/api/password/reset', [
|
||||
'email' => 'ada@example.com',
|
||||
'codigo' => '1234',
|
||||
'password' => 'NewSecret!456',
|
||||
'password_confirmation' => 'NewSecret!456',
|
||||
])->assertUnprocessable()
|
||||
->assertJsonValidationErrors('codigo');
|
||||
|
||||
$this->assertTrue(Hash::check('OldSecret!123', $user->fresh()->password));
|
||||
$this->assertSame(ResetPasswordAttempt::STATUS_EXPIRED, $attempt->fresh()->status);
|
||||
}
|
||||
|
||||
public function test_it_validates_password_confirmation_and_strength(): void
|
||||
{
|
||||
$this->postJson('/api/password/reset', [
|
||||
|
||||
@@ -49,6 +49,27 @@ class ValidateResetPasswordAttemptControllerTest extends TestCase
|
||||
);
|
||||
}
|
||||
|
||||
public function test_it_expires_an_attempt_and_returns_the_expired_code_message(): void
|
||||
{
|
||||
$user = User::factory()->create(['email' => 'ada@example.com']);
|
||||
$attempt = $user->resetPasswordAttempts()->create([
|
||||
'codigo' => '1234',
|
||||
'expires_at' => now()->subSecond(),
|
||||
]);
|
||||
|
||||
$this->postJson('/api/password/reset-attempts/validate', [
|
||||
'email' => 'ada@example.com',
|
||||
'codigo' => '1234',
|
||||
])->assertUnprocessable()
|
||||
->assertJsonValidationErrors('codigo')
|
||||
->assertJsonPath('errors.codigo.0', __('api.auth.reset_code_expired'));
|
||||
|
||||
$this->assertSame(
|
||||
ResetPasswordAttempt::STATUS_EXPIRED,
|
||||
$attempt->fresh()->status,
|
||||
);
|
||||
}
|
||||
|
||||
public function test_it_rejects_an_expired_or_already_validated_attempt(): void
|
||||
{
|
||||
$user = User::factory()->create(['email' => 'ada@example.com']);
|
||||
|
||||
Reference in New Issue
Block a user