Refuse a two-factor completion for a revoked operator

completeLogin() re-checks isActive()/isOperator() immediately before signing
in, not only at the password step. The two-factor challenge previously only
confirmed the operator row still existed (Operator::find() + a null check),
so disabling an operator or revoking every console role in the window between
the password step and the pending code did not stop the login from
completing — the realistic case is an owner locking someone out mid-login,
not a contrived one.

One check, in the static method both OperatorLogin's direct exit and
OperatorTwoFactorChallenge::verify() already share, rather than a second copy
in the challenge. completeLogin() now returns bool instead of void: false
means refused, and neither the guard nor the session was touched. Each caller
turns that into a ValidationException in its own vocabulary — 'email' /
auth.failed for the direct exit (already unreachable in practice, since
authenticate() just checked the same two conditions synchronously; kept for
defense in depth), 'code' / auth.not_an_operator for the challenge, since the
code entered may be genuinely correct and reusing "wrong code" would mislead.
feat/operator-identity
nexxo 2026-07-28 15:47:39 +02:00
parent 58835a1051
commit 92dd6cc2d8
4 changed files with 91 additions and 7 deletions

View File

@ -82,17 +82,41 @@ class OperatorLogin extends Component
$this->completeLoginAndRedirect($operator, $this->remember);
}
/** Shared with the two-factor challenge, so both exits behave identically. */
public static function completeLogin(Operator $operator, bool $remember): void
/**
* Shared with the two-factor challenge, so both exits behave identically.
*
* Re-checks isActive()/isOperator() here, not only at the password step:
* an operator can be disabled, or lose every console role, in the window
* between entering a password and completing a PENDING two-factor
* challenge an owner revoking access while someone is mid-login is the
* realistic case, not a contrived one, and the challenge on its own only
* ever confirmed the row still existed. One check, in the one place both
* entry points already share, rather than a second copy in the challenge
* that the next change forgets to keep in sync.
*
* @return bool False when the operator no longer qualifies: the login is
* refused, not completed neither the guard nor the
* session is touched, and the caller decides how to say so.
*/
public static function completeLogin(Operator $operator, bool $remember): bool
{
if (! $operator->isActive() || ! $operator->isOperator()) {
return false;
}
Auth::guard('operator')->login($operator, $remember);
session()->regenerate();
$operator->forceFill(['last_login_at' => now()])->save();
return true;
}
private function completeLoginAndRedirect(Operator $operator, bool $remember): void
{
self::completeLogin($operator, $remember);
if (! self::completeLogin($operator, $remember)) {
throw ValidationException::withMessages(['email' => __('auth.failed')]);
}
$this->redirect(AdminArea::home(), navigate: false);
}

View File

@ -58,7 +58,16 @@ class OperatorTwoFactorChallenge extends Component
$remember = (bool) session('operator.2fa.remember', false);
session()->forget(['operator.2fa.id', 'operator.2fa.remember']);
OperatorLogin::completeLogin($operator, $remember);
if (! OperatorLogin::completeLogin($operator, $remember)) {
// Disabled, or lost every console role, in the window between
// the password step and this one. The code just entered may be
// perfectly correct — the account behind it no longer is, so
// reusing "wrong code" would mislead. The pending challenge is
// spent either way (already forgotten above): no retry loop
// against a session that can never complete now.
throw ValidationException::withMessages(['code' => __('auth.not_an_operator')]);
}
$this->redirect(AdminArea::home(), navigate: false);
}

View File

@ -3,6 +3,7 @@
use App\Livewire\Auth\OperatorLogin;
use App\Models\Operator;
use App\Models\User;
use App\Support\AdminArea;
use Illuminate\Support\Facades\Auth;
use Livewire\Livewire;
@ -19,7 +20,7 @@ it('signs an operator in on the operator guard', function () {
->set('email', $operator->email)
->set('password', 'richtig-langes-pw')
->call('authenticate')
->assertRedirect(App\Support\AdminArea::home());
->assertRedirect(AdminArea::home());
expect(Auth::guard('operator')->check())->toBeTrue()
->and(Auth::guard('web')->check())->toBeFalse();
@ -81,6 +82,22 @@ it('throttles repeated failures', function () {
expect(session()->has('login.id'))->toBeFalse();
});
it('refuses to complete a login for an operator who no longer qualifies', function () {
// completeLogin() is the one place both the direct exit and the
// two-factor challenge complete a sign-in — re-checked here so a
// revocation in the window between the password step and a pending
// two-factor code cannot slip through the challenge, which only ever
// confirmed the row still existed.
$disabled = Operator::factory()->role('Owner')->create(['disabled_at' => now()]);
$roleless = Operator::factory()->create(); // no role synced at all
expect(OperatorLogin::completeLogin($disabled, false))->toBeFalse()
->and(Auth::guard('operator')->check())->toBeFalse();
expect(OperatorLogin::completeLogin($roleless, false))->toBeFalse()
->and(Auth::guard('operator')->check())->toBeFalse();
});
it('records when the operator last signed in', function () {
$operator = Operator::factory()->role('Owner')->create(['password' => 'richtig-langes-pw']);

View File

@ -2,6 +2,7 @@
use App\Livewire\Auth\OperatorTwoFactorChallenge;
use App\Models\Operator;
use App\Support\AdminArea;
use Illuminate\Support\Facades\Auth;
use Livewire\Livewire;
use PragmaRX\Google2FA\Google2FA;
@ -41,7 +42,7 @@ it('signs the operator in once the code actually checks out', function () {
Livewire::test(OperatorTwoFactorChallenge::class)
->set('code', $code)
->call('verify')
->assertRedirect(App\Support\AdminArea::home());
->assertRedirect(AdminArea::home());
expect(Auth::guard('operator')->check())->toBeTrue()
->and(session()->has('operator.2fa.id'))->toBeFalse();
@ -66,7 +67,7 @@ it('accepts a recovery code exactly once', function () {
->set('usingRecovery', true)
->set('recoveryCode', 'aaaa-1111')
->call('verify')
->assertRedirect(App\Support\AdminArea::home());
->assertRedirect(AdminArea::home());
expect(Auth::guard('operator')->check())->toBeTrue();
@ -87,6 +88,39 @@ it('accepts a recovery code exactly once', function () {
expect(Auth::guard('operator')->check())->toBeFalse();
});
it('refuses to complete the login when the operator was disabled after the password step', function () {
// The realistic case: an owner revokes access WHILE someone is mid-login,
// not before. The code entered here is genuinely correct — the account
// it belongs to no longer qualifies.
$operator = pendingOperator();
$code = app(Google2FA::class)->getCurrentOtp(decrypt($operator->two_factor_secret));
$operator->forceFill(['disabled_at' => now()])->save();
Livewire::test(OperatorTwoFactorChallenge::class)
->set('code', $code)
->call('verify')
->assertHasErrors('code');
expect(Auth::guard('operator')->check())->toBeFalse()
->and($operator->fresh()->last_login_at)->toBeNull();
});
it('refuses to complete the login when every console role was revoked after the password step', function () {
$operator = pendingOperator();
$code = app(Google2FA::class)->getCurrentOtp(decrypt($operator->two_factor_secret));
$operator->syncRoles([]);
Livewire::test(OperatorTwoFactorChallenge::class)
->set('code', $code)
->call('verify')
->assertHasErrors('code');
expect(Auth::guard('operator')->check())->toBeFalse()
->and($operator->fresh()->last_login_at)->toBeNull();
});
it('throttles repeated wrong codes', function () {
pendingOperator();