Files
projectsend/tests/Feature/Identity/TwoFactorTest.php
denkfabrik-li 674781e57a Claim a TOTP code atomically instead of checking then writing
verify() asked Cache::has(), verified, then Cache::put(). Between the read
and the write the key is free, so two requests carrying the same code
could both be told yes -- which is exactly what the replay guard exists to
prevent, and the window an intercepted code has is the whole of its
validity either side.

The claim is now the answer: Cache::add() writes only if the key is
absent, so of two requests carrying the same valid code exactly one gets
true back. That is the same mechanism, for the same reason, as the
preview log's debounce -- "Cache::add is the whole mechanism: it writes
only if the key is absent ... without a read-then-write race between two
of them".

Verification still happens first, so a wrong code never touches the cache
and cannot burn the window for the code the person is about to type
correctly.

One test, modelling the interleaving it is about: the winner's claim has
landed, and the loser's has() answers from before that write. Without the
fix the loser is signed in.
2026-08-28 06:40:52 +02:00

222 lines
7.9 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Identity\TwoFactor\TwoFactorService;
use Illuminate\Support\Facades\Cache;
use PragmaRX\Google2FA\Google2FA;
// confirmPassword() and enableTwoFactor() live in tests/Helpers.php —
// more than one file needs them, and a helper used by more than one file
// has to be loaded from there or a --filter run cannot find it.
test('a user can enroll in two-factor authentication', function () {
$user = User::factory()->create();
confirmPassword($user);
$this->actingAs($user)->post('/settings/two-factor')->assertRedirect();
$user->refresh();
expect($user->two_factor_secret)->not->toBeNull()
->and($user->hasTwoFactorEnabled())->toBeFalse();
$code = app(Google2FA::class)->getCurrentOtp((string) $user->two_factor_secret);
$this->actingAs($user)
->post('/settings/two-factor/confirm', ['code' => $code])
->assertRedirect()
->assertSessionHas('two_factor_recovery_codes');
$user->refresh();
expect($user->hasTwoFactorEnabled())->toBeTrue()
->and($user->two_factor_recovery_codes)->toHaveCount(8);
});
test('confirming with a wrong code fails and leaves 2fa disabled', function () {
$user = User::factory()->create();
confirmPassword($user);
$this->actingAs($user)->post('/settings/two-factor');
$this->actingAs($user)
->post('/settings/two-factor/confirm', ['code' => '000000'])
->assertSessionHasErrors('code');
expect($user->refresh()->hasTwoFactorEnabled())->toBeFalse();
});
test('login with 2fa enabled requires the challenge instead of creating a session', function () {
$user = User::factory()->create();
enableTwoFactor($user);
Auth::logout();
$this->flushSession();
$this->post('/login', ['email' => $user->email, 'password' => 'password'])
->assertRedirect(route('two-factor.challenge'));
$this->assertGuest();
$code = app(Google2FA::class)->getCurrentOtp((string) $user->refresh()->two_factor_secret);
$this->post('/two-factor-challenge', ['code' => $code])
->assertRedirect(route('dashboard', absolute: false));
$this->assertAuthenticatedAs($user);
});
test('a totp code cannot be replayed', function () {
$user = User::factory()->create();
$secret = enableTwoFactor($user);
Auth::logout();
$this->flushSession();
$code = app(Google2FA::class)->getCurrentOtp($secret);
$this->post('/login', ['email' => $user->email, 'password' => 'password']);
$this->post('/two-factor-challenge', ['code' => $code]);
$this->assertAuthenticatedAs($user);
Auth::logout();
$this->flushSession();
$this->post('/login', ['email' => $user->email, 'password' => 'password']);
$this->post('/two-factor-challenge', ['code' => $code])
->assertSessionHasErrors('code');
$this->assertGuest();
});
// The replay guard used to read, verify, then write. Two requests carrying
// the same code could both read "unused" before either wrote, and both be
// told yes -- which is the whole window an intercepted code has. This is
// that interleaving: the second request's read lands before the winner's
// write, so the key looks free and is not.
test('a code already claimed by another request in flight is refused', function () {
$user = User::factory()->create();
$secret = enableTwoFactor($user);
$code = app(Google2FA::class)->getCurrentOtp($secret);
// The winner of the race has claimed the code.
Cache::put('two-factor.used.'.$user->id.'.'.hash('sha256', $code), true, now()->addSeconds(90));
// The loser read before that write landed, so its has() still reports
// the key as free. Only that one answer is stale — everything else is
// the real store — which leaves the claim itself as the deciding call.
$stale = Mockery::mock(Cache::store())->makePartial();
$stale->shouldReceive('has')->andReturnFalse();
config()->set('cache.stores.stale-read', ['driver' => 'stale-read']);
Cache::extend('stale-read', fn () => $stale);
Cache::setDefaultDriver('stale-read');
expect(app(TwoFactorService::class)->verify($user->refresh(), $code))->toBeFalse();
});
test('a recovery code logs in and is consumed', function () {
$user = User::factory()->create();
enableTwoFactor($user);
/** @var list<string> $codes */
$codes = $user->refresh()->two_factor_recovery_codes;
$recovery = $codes[0];
Auth::logout();
$this->flushSession();
$this->post('/login', ['email' => $user->email, 'password' => 'password']);
$this->post('/two-factor-challenge', ['recovery_code' => $recovery])
->assertRedirect(route('dashboard', absolute: false));
$this->assertAuthenticatedAs($user);
expect($user->refresh()->two_factor_recovery_codes)->toHaveCount(7)
->and($user->two_factor_recovery_codes)->not->toContain($recovery);
// The same code again is rejected.
Auth::logout();
$this->flushSession();
$this->post('/login', ['email' => $user->email, 'password' => 'password']);
$this->post('/two-factor-challenge', ['recovery_code' => $recovery])
->assertSessionHasErrors('code');
$this->assertGuest();
});
test('the challenge is not reachable without a pending login', function () {
User::factory()->create();
$this->get('/two-factor-challenge')->assertRedirect(route('login'));
$this->post('/two-factor-challenge', ['code' => '123456'])->assertRedirect(route('login'));
});
test('wrong credentials never reach the challenge even with 2fa enabled', function () {
$user = User::factory()->create();
enableTwoFactor($user);
Auth::logout();
$this->flushSession();
$this->post('/login', ['email' => $user->email, 'password' => 'wrong-password'])
->assertSessionHasErrors('email');
$this->assertGuest();
expect(session('two_factor.login_id'))->toBeNull();
});
test('disabling 2fa clears all two-factor state', function () {
$user = User::factory()->create();
enableTwoFactor($user);
confirmPassword($user);
$this->actingAs($user)->delete('/settings/two-factor')->assertRedirect();
$user->refresh();
expect($user->hasTwoFactorEnabled())->toBeFalse()
->and($user->two_factor_secret)->toBeNull()
->and($user->two_factor_recovery_codes)->toBeNull();
});
test('regenerating recovery codes replaces the set', function () {
$user = User::factory()->create();
enableTwoFactor($user);
/** @var list<string> $before */
$before = $user->refresh()->two_factor_recovery_codes;
confirmPassword($user);
$this->actingAs($user)->post('/settings/two-factor/recovery-codes')->assertRedirect();
/** @var list<string> $after */
$after = $user->refresh()->two_factor_recovery_codes;
expect($after)->toHaveCount(8)->and(array_intersect($before, $after))->toBeEmpty();
});
// A stolen session is exactly the situation 2FA exists to survive, so the
// second factor must not be removable with nothing but that session.
test('the two-factor mutation routes require a fresh password confirmation', function () {
$user = User::factory()->create();
enableTwoFactor($user);
// enableTwoFactor() confirmed the password; drop that back out of the
// session to model a session that never proved the first factor.
$this->withSession(['auth.password_confirmed_at' => null]);
$this->actingAs($user)->post('/settings/two-factor')->assertRedirect(route('password.confirm'));
$this->actingAs($user)->post('/settings/two-factor/recovery-codes')->assertRedirect(route('password.confirm'));
$this->actingAs($user)->delete('/settings/two-factor')->assertRedirect(route('password.confirm'));
expect($user->refresh()->hasTwoFactorEnabled())->toBeTrue()
->and($user->two_factor_secret)->not->toBeNull();
// With the password proved, the same request goes through.
confirmPassword($user);
$this->actingAs($user)->delete('/settings/two-factor')->assertRedirect();
expect($user->refresh()->hasTwoFactorEnabled())->toBeFalse();
});