From 913d19d39c713b7a327fae7aff7ba46f98b9b37f Mon Sep 17 00:00:00 2001 From: nexxo Date: Tue, 28 Jul 2026 14:21:02 +0200 Subject: [PATCH] Give operators somewhere to actually set up two-factor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit console.require_2fa exempted admin.settings from the redirect it forces on an unconfirmed operator, but that page only ever configured the global policy — nobody could enrol behind it. A newly invited operator stayed permanently redirected to a page with no way to satisfy the requirement, the opposite of what the switch was for. Adds enable/confirm/regenerate/disable to Admin\Settings, reusing Fortify's own actions exactly as the portal's Settings already does — they only ever touch the model they're handed, no guard or auth() call inside them, so pointing ConfirmsPassword's guard at 'operator' is the only change needed. The new card is deliberately not gated behind the site.manage capability that hides the rest of this page's Owner-only sections: the compulsory switch applies to every operator, so enrolment has to be reachable by every operator too. Dropped the account card's stale hint pointing at a "separate security area" that never existed. --- app/Livewire/Admin/Settings.php | 124 +++++++++++++-- lang/de/admin_settings.php | 24 ++- lang/en/admin_settings.php | 24 ++- .../views/livewire/admin/settings.blade.php | 86 +++++++++- .../Admin/OperatorTwoFactorEnrollmentTest.php | 149 ++++++++++++++++++ .../Admin/OperatorTwoFactorPolicyTest.php | 41 +++++ 6 files changed, 434 insertions(+), 14 deletions(-) create mode 100644 tests/Feature/Admin/OperatorTwoFactorEnrollmentTest.php diff --git a/app/Livewire/Admin/Settings.php b/app/Livewire/Admin/Settings.php index 7636732..9951b90 100644 --- a/app/Livewire/Admin/Settings.php +++ b/app/Livewire/Admin/Settings.php @@ -2,20 +2,29 @@ namespace App\Livewire\Admin; +use App\Http\Middleware\RestrictConsoleNetwork; +use App\Livewire\Concerns\ChangesOwnPassword; +use App\Livewire\Concerns\ConfirmsPassword; use App\Models\Customer; use App\Models\Operator; use App\Models\VpnPeer; +use App\Provisioning\Jobs\ApplyVpnPeer; use App\Services\Deployment\UpdateChannel; use App\Support\Settings as AppSettings; -use App\Provisioning\Jobs\ApplyVpnPeer; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Str; -use Spatie\Permission\Models\Role; +use Illuminate\Validation\ValidationException; +use Laravel\Fortify\Actions\ConfirmTwoFactorAuthentication; +use Laravel\Fortify\Actions\DisableTwoFactorAuthentication; +use Laravel\Fortify\Actions\EnableTwoFactorAuthentication; +use Laravel\Fortify\Actions\GenerateNewRecoveryCodes; use Livewire\Attributes\Layout; use Livewire\Attributes\Validate; use Livewire\Component; +use Spatie\Permission\Models\Role; +use Symfony\Component\HttpFoundation\IpUtils; /** * Operator settings: the signed-in operator's own account plus (Owner-only) @@ -25,7 +34,8 @@ use Livewire\Component; #[Layout('layouts.admin')] class Settings extends Component { - use \App\Livewire\Concerns\ChangesOwnPassword; + use ChangesOwnPassword; + use ConfirmsPassword; /** This page changes the signed-in OPERATOR's password, never a portal one. */ protected function passwordAccountGuard(): string @@ -33,6 +43,87 @@ class Settings extends Component return 'operator'; } + /** Two-factor is confirmed against the operator's own record, not a portal one. */ + protected function confirmationGuard(): string + { + return 'operator'; + } + + /** Shown once, right after setting two-factor up. */ + public ?array $recoveryCodes = null; + + public string $twoFactorCode = ''; + + /** + * Start two-factor setup: generate the secret, then show the QR code. + * + * Not confirmed yet — Fortify keeps `two_factor_confirmed_at` null until a + * code from the app has been accepted, so someone who scans nothing and + * closes the tab is not left locked out of their own account. + */ + public function enableTwoFactor(): void + { + $this->requireConfirmedPassword(); + + app(EnableTwoFactorAuthentication::class)(Auth::guard('operator')->user()); + } + + /** Accept a code from the app, which is what actually turns it on. */ + public function confirmTwoFactor(): void + { + $this->requireConfirmedPassword(); + + $operator = Auth::guard('operator')->user(); + + try { + app(ConfirmTwoFactorAuthentication::class)( + $operator, + $this->twoFactorCode, + ); + } catch (ValidationException $e) { + $this->addError('twoFactorCode', $e->errors()['code'][0] ?? __('admin_settings.twofa_code_wrong')); + + return; + } + + $this->twoFactorCode = ''; + // Shown once, here, because this is the only moment they exist in a + // form anyone will read. They stay retrievable afterwards, but nobody + // writes down what they were not shown. + $this->recoveryCodes = json_decode(decrypt($operator->two_factor_recovery_codes), true); + $this->dispatch('notify', message: __('admin_settings.twofa_on')); + } + + public function regenerateRecoveryCodes(): void + { + $this->requireConfirmedPassword(); + + $operator = Auth::guard('operator')->user(); + app(GenerateNewRecoveryCodes::class)($operator); + $this->recoveryCodes = json_decode(decrypt($operator->refresh()->two_factor_recovery_codes), true); + } + + public function disableTwoFactor(): void + { + $this->requireConfirmedPassword(); + + app(DisableTwoFactorAuthentication::class)(Auth::guard('operator')->user()); + $this->recoveryCodes = null; + $this->dispatch('notify', message: __('admin_settings.twofa_off')); + } + + /** + * Every two-factor action goes through this. + * + * Server-side, on each call, and not merely by hiding the buttons: a + * Livewire action is reachable by anyone who can post to /livewire/update, + * and "the form was not on screen" has never stopped anybody. + */ + private function requireConfirmedPassword(): void + { + abort_unless($this->passwordRecentlyConfirmed(), 403); + } + // My account #[Validate('required|string|max:255')] public string $name = ''; @@ -262,7 +353,7 @@ class Settings extends Component $value = trim($this->consoleIp); - if (! \App\Http\Middleware\RestrictConsoleNetwork::isNetwork($value)) { + if (! RestrictConsoleNetwork::isNetwork($value)) { $this->addError('consoleIp', __('admin_settings.console_ip_invalid')); return; @@ -298,8 +389,8 @@ class Settings extends Component $ip = (string) request()->ip(); - if (\App\Http\Middleware\RestrictConsoleNetwork::isRestricted() - && ! \App\Http\Middleware\RestrictConsoleNetwork::wouldStillAllow($ip, $list)) { + if (RestrictConsoleNetwork::isRestricted() + && ! RestrictConsoleNetwork::wouldStillAllow($ip, $list)) { $this->dispatch('notify', message: __('admin_settings.console_ip_last', ['ip' => $ip])); return; @@ -320,9 +411,9 @@ class Settings extends Component { $this->authorize('site.manage'); - $on = ! \App\Http\Middleware\RestrictConsoleNetwork::isRestricted(); + $on = ! RestrictConsoleNetwork::isRestricted(); - if ($on && ! \App\Http\Middleware\RestrictConsoleNetwork::allows((string) request()->ip())) { + if ($on && ! RestrictConsoleNetwork::allows((string) request()->ip())) { $this->dispatch('notify', message: __('admin_settings.console_lock_refused', ['ip' => (string) request()->ip()])); return; @@ -355,7 +446,6 @@ class Settings extends Component $this->dispatch('notify', message: __('admin_settings.saved')); } - /** * Ask the host-side agent to update this installation. * @@ -377,6 +467,8 @@ class Settings extends Component public function render() { + $operator = Auth::guard('operator')->user(); + $staff = Operator::query() ->whereHas('roles', fn ($q) => $q->whereIn('name', Operator::OPERATOR_ROLES)) ->with('roles') @@ -396,13 +488,13 @@ class Settings extends Component 'sitePublic' => AppSettings::bool('site.public', true), 'canManageSite' => Auth::guard('operator')->user()?->can('site.manage') ?? false, // Shown so nobody has to guess why they still see the real site. - 'viewerOnVpn' => \Symfony\Component\HttpFoundation\IpUtils::checkIp( + 'viewerOnVpn' => IpUtils::checkIp( (string) request()->ip(), (array) config('admin_access.trusted_ranges', []), ), // Who may reach the console, and from where the viewer is asking — // shown so the consequence of a change is visible before it is made. - 'consoleRestricted' => \App\Http\Middleware\RestrictConsoleNetwork::isRestricted(), + 'consoleRestricted' => RestrictConsoleNetwork::isRestricted(), 'consoleIps' => (array) AppSettings::get('console.allowed_ips', []), 'consoleVpnRanges' => (array) config('admin_access.trusted_ranges', []), 'viewerIp' => (string) request()->ip(), @@ -410,6 +502,16 @@ class Settings extends Component 'roles' => Operator::OPERATOR_ROLES, 'canManageStaff' => Auth::guard('operator')->user()->can('staff.manage'), 'isOwner' => Auth::guard('operator')->user()->hasRole('Owner'), + // My own two-factor. Never the secret itself — only whether it + // exists, and the SVG Fortify renders from it. The secret in a + // Livewire property would travel to the browser and back in the + // component snapshot. + 'twoFactorPending' => $operator->two_factor_secret !== null && $operator->two_factor_confirmed_at === null, + 'twoFactorOn' => $operator->two_factor_confirmed_at !== null, + 'twoFactorQr' => $operator->two_factor_secret !== null && $operator->two_factor_confirmed_at === null + ? $operator->twoFactorQrCodeSvg() + : null, + 'passwordConfirmed' => $this->passwordRecentlyConfirmed(), ]); } } diff --git a/lang/de/admin_settings.php b/lang/de/admin_settings.php index dfa07ff..09e126b 100644 --- a/lang/de/admin_settings.php +++ b/lang/de/admin_settings.php @@ -20,7 +20,6 @@ return [ 'account_title' => 'Mein Konto', 'name' => 'Name', 'email' => 'E-Mail', - 'twofa_hint' => 'Passwort & Zwei-Faktor-Authentifizierung folgen in einem separaten Sicherheitsbereich.', 'account_saved' => 'Konto aktualisiert.', 'staff_title' => 'Team & Zugriff', @@ -85,6 +84,29 @@ return [ 'two_factor_required' => 'Zwei-Faktor ist für die Konsole verbindlich. Bitte richten Sie sie hier ein.', 'two_factor_self_first' => 'Richten Sie zuerst Ihre eigene Zwei-Faktor-Bestätigung ein — sonst sperren Sie sich mit diesem Schalter selbst aus.', + // My own two-factor enrolment — every operator, not only the Owner. This + // is the page RequireOperatorTwoFactor exempts from its redirect, so it + // has to actually work: the switch above is only fair if everyone it + // applies to has somewhere to satisfy it. + 'twofa_title' => 'Meine Zwei-Faktor-Anmeldung', + 'twofa_sub' => 'Zusätzlich zum Passwort ein Code aus einer App. Schützt Ihr Konto auch dann, wenn das Passwort irgendwo abhandenkommt.', + 'twofa_state_on' => 'Aktiv', + 'twofa_state_off' => 'Nicht eingerichtet', + 'twofa_confirm_first' => 'Bitte zuerst Ihr Passwort bestätigen — eine angemeldete Sitzung allein genügt hier nicht.', + 'twofa_confirm_button' => 'Bestätigen', + 'twofa_enable' => 'Einrichten', + 'twofa_scan' => 'Scannen Sie den Code mit Ihrer Authenticator-App und geben Sie danach die angezeigte Ziffernfolge ein.', + 'twofa_code' => 'Code aus der App', + 'twofa_activate' => 'Aktivieren', + 'twofa_code_wrong' => 'Dieser Code stimmt nicht.', + 'twofa_on' => 'Zwei-Faktor-Anmeldung ist aktiv.', + 'twofa_off' => 'Zwei-Faktor-Anmeldung wurde entfernt.', + 'twofa_off_confirm' => 'Zwei-Faktor-Anmeldung wirklich entfernen? Ihr Konto ist danach nur noch durch das Passwort geschützt.', + 'twofa_disable' => 'Entfernen', + 'twofa_new_codes' => 'Neue Wiederherstellungscodes', + 'twofa_codes_title' => 'Wiederherstellungscodes', + 'twofa_codes_hint' => 'Jetzt notieren und sicher verwahren. Sie sind der Weg zurück, wenn das Gerät verloren geht — jeder Code gilt einmal.', + 'update_title' => 'Version & Aktualisierung', 'update_version' => 'Läuft', 'update_source' => 'Quelle', diff --git a/lang/en/admin_settings.php b/lang/en/admin_settings.php index f00a7ce..c0bd6c4 100644 --- a/lang/en/admin_settings.php +++ b/lang/en/admin_settings.php @@ -20,7 +20,6 @@ return [ 'account_title' => 'My account', 'name' => 'Name', 'email' => 'Email', - 'twofa_hint' => 'Password & two-factor authentication follow in a separate security area.', 'account_saved' => 'Account updated.', 'staff_title' => 'Team & access', @@ -85,6 +84,29 @@ return [ 'two_factor_required' => 'Two-factor is compulsory for the console. Please set it up here.', 'two_factor_self_first' => 'Set up your own two-factor confirmation first — otherwise this switch locks you out yourself.', + // My own two-factor enrolment — every operator, not only the Owner. This + // is the page RequireOperatorTwoFactor exempts from its redirect, so it + // has to actually work: the switch above is only fair if everyone it + // applies to has somewhere to satisfy it. + 'twofa_title' => 'My two-factor login', + 'twofa_sub' => 'A code from an app, in addition to your password. Protects your account even if the password ends up somewhere it should not.', + 'twofa_state_on' => 'Active', + 'twofa_state_off' => 'Not set up', + 'twofa_confirm_first' => 'Please confirm your password first — a signed-in session alone is not enough here.', + 'twofa_confirm_button' => 'Confirm', + 'twofa_enable' => 'Set up', + 'twofa_scan' => 'Scan the code with your authenticator app, then enter the digits it shows.', + 'twofa_code' => 'Code from the app', + 'twofa_activate' => 'Activate', + 'twofa_code_wrong' => 'That code is not correct.', + 'twofa_on' => 'Two-factor login is active.', + 'twofa_off' => 'Two-factor login was removed.', + 'twofa_off_confirm' => 'Really remove two-factor login? Your account is then protected by the password alone.', + 'twofa_disable' => 'Remove', + 'twofa_new_codes' => 'New recovery codes', + 'twofa_codes_title' => 'Recovery codes', + 'twofa_codes_hint' => 'Write these down now and keep them safe. They are the way back in if the device is lost — each code works once.', + 'update_title' => 'Version & update', 'update_version' => 'Running', 'update_source' => 'Source', diff --git a/resources/views/livewire/admin/settings.blade.php b/resources/views/livewire/admin/settings.blade.php index 782e625..f00ea29 100644 --- a/resources/views/livewire/admin/settings.blade.php +++ b/resources/views/livewire/admin/settings.blade.php @@ -298,13 +298,97 @@ + {{-- My own two-factor. Reachable by EVERY operator, not only the Owner — + this is not gated behind $canManageSite, on purpose: the compulsory + switch below applies to every member of staff, so every operator + needs somewhere to satisfy it. This is also the page + RequireOperatorTwoFactor exempts from the redirect it otherwise + forces while the switch is on — without a working enrolment control + here, that exemption would lead nowhere. + + Every action is re-checked server-side against a recent password + confirmation — hiding the buttons stops nobody who can post to + /livewire/update. --}} +
+
+
+
+

{{ __('admin_settings.twofa_title') }}

+ + + {{ $twoFactorOn ? __('admin_settings.twofa_state_on') : __('admin_settings.twofa_state_off') }} + +
+

{{ __('admin_settings.twofa_sub') }}

+
+
+ + @if (! $passwordConfirmed) + {{-- The gate. A signed-in session is not enough to change how the + account is protected — the risk is an unlocked machine. --}} +
+

{{ __('admin_settings.twofa_confirm_first') }}

+
+
+ +
+ + {{ __('admin_settings.twofa_confirm_button') }} + +
+
+ @elseif ($twoFactorOn) +
+ + {{ __('admin_settings.twofa_new_codes') }} + + + {{ __('admin_settings.twofa_disable') }} + +
+ @elseif ($twoFactorPending) +
+
{!! $twoFactorQr !!}
+
+

{{ __('admin_settings.twofa_scan') }}

+
+ +
+ + {{ __('admin_settings.twofa_activate') }} + +
+
+ @else + + {{ __('admin_settings.twofa_enable') }} + + @endif + + @if ($recoveryCodes) + {{-- Shown once, on purpose: nobody writes down what they were never + shown, and these are the way back in when the phone is gone. --}} +
+

{{ __('admin_settings.twofa_codes_title') }}

+

{{ __('admin_settings.twofa_codes_hint') }}

+
    + @foreach ($recoveryCodes as $code)
  • {{ $code }}
    • @endforeach +
+
+ @endif +
+

{{ __('admin_settings.account_title') }}

-

{{ __('admin_settings.twofa_hint') }}

{{ __('admin_settings.save') }}
diff --git a/tests/Feature/Admin/OperatorTwoFactorEnrollmentTest.php b/tests/Feature/Admin/OperatorTwoFactorEnrollmentTest.php new file mode 100644 index 0000000..33b08bb --- /dev/null +++ b/tests/Feature/Admin/OperatorTwoFactorEnrollmentTest.php @@ -0,0 +1,149 @@ +test(Settings::class) + ->call($action) + ->assertForbidden(); + } + + expect($op->refresh()->two_factor_secret)->toBeNull(); +}); + +it('takes a password, then sets two-factor up and confirms it with a real code', function () { + $op = operator('Support'); + + $page = Livewire::actingAs($op, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->assertHasNoErrors(); + + $page->call('enableTwoFactor'); + expect($op->refresh()->two_factor_secret)->not->toBeNull() + // Not on until a code has been accepted: someone who scans nothing and + // closes the tab must not be locked out of their own account. + ->and($op->two_factor_confirmed_at)->toBeNull(); + + $code = app(Google2FA::class)->getCurrentOtp(decrypt($op->two_factor_secret)); + + $page->set('twoFactorCode', $code)->call('confirmTwoFactor')->assertHasNoErrors(); + + expect($op->refresh()->two_factor_confirmed_at)->not->toBeNull() + ->and($page->get('recoveryCodes'))->toBeArray()->not->toBeEmpty(); +}); + +it('rejects a wrong code instead of switching it on', function () { + $op = operator('Support'); + + Livewire::actingAs($op, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->call('enableTwoFactor') + ->set('twoFactorCode', '000000') + ->call('confirmTwoFactor') + ->assertHasErrors('twoFactorCode'); + + expect($op->refresh()->two_factor_confirmed_at)->toBeNull(); +}); + +it('does not accept a wrong password, and says so without hinting', function () { + $op = operator('Support'); + + Livewire::actingAs($op, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'falsch') + ->call('confirmPassword') + ->assertHasErrors('confirmablePassword') + ->call('enableTwoFactor') + ->assertForbidden(); +}); + +it('never puts the secret where the browser can see it', function () { + // A Livewire property travels to the browser and back in the component + // snapshot. The QR image is derived from the secret; the secret is not. + $op = operator('Support'); + + $page = Livewire::actingAs($op, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->call('enableTwoFactor'); + + $secret = decrypt($op->refresh()->two_factor_secret); + + expect(json_encode($page->snapshot))->not->toContain($secret); +}); + +it('actually renders the enrolment control on the page, not just in the component', function () { + // The original gap was exactly this: a component with the right methods + // but a blade that never called them. Assert on rendered HTML, not + // component state, so a forgotten wire:click regresses this test too. + $op = operator('Support'); + + // Nothing to enrol with yet — the password gate is what shows. + $page = Livewire::actingAs($op, 'operator')->test(Settings::class); + $page->assertSee(__('admin_settings.twofa_confirm_first')) + ->assertSee(__('admin_settings.twofa_state_off')); + + $page->set('confirmablePassword', 'password')->call('confirmPassword'); + $page->assertSee(__('admin_settings.twofa_enable')); + + $page->call('enableTwoFactor'); + $page->assertSee(__('admin_settings.twofa_activate')) + ->assertSee(__('admin_settings.twofa_code')); + + $code = app(Google2FA::class)->getCurrentOtp(decrypt($op->refresh()->two_factor_secret)); + $page->set('twoFactorCode', $code)->call('confirmTwoFactor'); + + $page->assertSee(__('admin_settings.twofa_state_on')) + ->assertSee(__('admin_settings.twofa_codes_title')) + ->assertSee(__('admin_settings.twofa_disable')); +}); + +it('lets an operator regenerate recovery codes and disable two-factor once confirmed', function () { + $op = operator('Support'); + + $page = Livewire::actingAs($op, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->call('enableTwoFactor'); + + $code = app(Google2FA::class)->getCurrentOtp(decrypt($op->refresh()->two_factor_secret)); + $page->set('twoFactorCode', $code)->call('confirmTwoFactor')->assertHasNoErrors(); + + $firstCodes = $op->refresh()->two_factor_recovery_codes; + + $page->call('regenerateRecoveryCodes'); + expect($op->refresh()->two_factor_recovery_codes)->not->toBe($firstCodes); + + $page->call('disableTwoFactor')->assertHasNoErrors(); + expect($op->refresh()->two_factor_confirmed_at)->toBeNull() + ->and($op->two_factor_secret)->toBeNull(); +}); diff --git a/tests/Feature/Admin/OperatorTwoFactorPolicyTest.php b/tests/Feature/Admin/OperatorTwoFactorPolicyTest.php index 2b25a4e..dbe6e22 100644 --- a/tests/Feature/Admin/OperatorTwoFactorPolicyTest.php +++ b/tests/Feature/Admin/OperatorTwoFactorPolicyTest.php @@ -4,6 +4,7 @@ use App\Livewire\Admin\Settings; use App\Models\Operator; use App\Support\Settings as AppSettings; use Livewire\Livewire; +use PragmaRX\Google2FA\Google2FA; it('is voluntary by default', function () { expect(AppSettings::bool('console.require_2fa', false))->toBeFalse(); @@ -47,6 +48,46 @@ it('refuses to switch it on while your own account has no two-factor', function expect(AppSettings::bool('console.require_2fa', false))->toBeFalse(); }); +it('lets an operator without two-factor genuinely enrol while the switch is on, then reach the console', function () { + // The trap this guards against: the exemption alone is not enough if + // there is nowhere to actually enrol behind it. Uses a non-Owner role — + // the requirement applies to every operator, not only the one who can + // flip the switch. + AppSettings::set('console.require_2fa', true); + + $operator = Operator::factory()->role('Support')->create(['password' => 'password']); + + // Bounced off an ordinary page… + $this->actingAs($operator, 'operator') + ->get(route('admin.overview')) + ->assertRedirect(route('admin.settings')); + + // …but the settings page itself is genuinely reachable, not another + // bounce in disguise. + $this->actingAs($operator, 'operator') + ->get(route('admin.settings')) + ->assertOk(); + + // Enrol for real: password confirmation, then a real TOTP code. + $page = Livewire::actingAs($operator, 'operator') + ->test(Settings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->assertHasNoErrors() + ->call('enableTwoFactor'); + + $code = app(Google2FA::class)->getCurrentOtp(decrypt($operator->refresh()->two_factor_secret)); + + $page->set('twoFactorCode', $code)->call('confirmTwoFactor')->assertHasNoErrors(); + + expect($operator->refresh()->two_factor_confirmed_at)->not->toBeNull(); + + // And now the rest of the console actually opens. + $this->actingAs($operator->fresh(), 'operator') + ->get(route('admin.overview')) + ->assertOk(); +}); + it('allows it once your own two-factor is confirmed', function () { $operator = Operator::factory()->role('Owner')->create([ 'two_factor_secret' => encrypt('JBSWY3DPEHPK3PXP'),