From f6b9181ed8017fa1a4bb7dbdeb6189f2f59c7255 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 08:48:34 +0200 Subject: [PATCH] Give customers two-factor, and stop every button on the settings page reacting at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking Save made every button on the console's settings page appear to fire. Seven `wire:loading.attr="disabled"` had no `wire:target`, and without one Livewire applies the loading state to ANY request on the component — so one save put all of them into their disabled state simultaneously. Each names its own action now. Two-factor for customers. Fortify's endpoints already existed; only the screen was missing. Setting up requires re-entering the password first, and every action re-checks that server-side rather than relying on the button not being on screen — a Livewire action is reachable by anyone who can post to /livewire/update. The confirmation marker is Laravel's own session key, so it and the framework's password.confirm middleware mean the same thing rather than drifting apart. The secret never enters a Livewire property. Component state travels to the browser and back in the snapshot; the QR image is derived from the secret, the secret is not in it — and there is a test that says so. The status page moves to the ROOT of its hostname: status.clupilot.com/status says the same word twice. That needed the domain-bound `/` registered BEFORE the landing page, because Laravel takes the first match and the landing route is host-agnostic — so the status host would have served the marketing site. Co-Authored-By: Claude Opus 5 --- app/Livewire/Concerns/ConfirmsPassword.php | 94 ++++++++++++++++ app/Livewire/Settings.php | 84 ++++++++++++++ lang/de/settings.php | 19 ++++ lang/en/settings.php | 19 ++++ .../views/livewire/admin/settings.blade.php | 10 +- resources/views/livewire/settings.blade.php | 78 +++++++++++++ routes/web.php | 40 ++++--- tests/Feature/CustomerTwoFactorTest.php | 103 ++++++++++++++++++ tests/Feature/WelcomeTest.php | 19 ++++ 9 files changed, 445 insertions(+), 21 deletions(-) create mode 100644 app/Livewire/Concerns/ConfirmsPassword.php create mode 100644 tests/Feature/CustomerTwoFactorTest.php diff --git a/app/Livewire/Concerns/ConfirmsPassword.php b/app/Livewire/Concerns/ConfirmsPassword.php new file mode 100644 index 0000000..fc31c06 --- /dev/null +++ b/app/Livewire/Concerns/ConfirmsPassword.php @@ -0,0 +1,94 @@ + 0 && (time() - $at) < $this->confirmationWindow(); + } + + public function confirmPassword(): void + { + $key = 'confirm-password:'.auth()->id().'|'.request()->ip(); + + if (RateLimiter::tooManyAttempts($key, 5)) { + $this->addError('confirmablePassword', __('auth.throttle', [ + 'seconds' => RateLimiter::availableIn($key), + ])); + + return; + } + + if (! Hash::check($this->confirmablePassword, auth()->user()->password)) { + RateLimiter::hit($key, 60); + $this->addError('confirmablePassword', __('admin_settings.password_wrong')); + $this->reset('confirmablePassword'); + + return; + } + + RateLimiter::clear($key); + $this->reset('confirmablePassword'); + + // Regenerated first: a confirmation raises what this session can do, and + // a session id that was already known to someone else must not be the + // one that gets the extra authority. + session()->regenerate(); + session(['auth.password_confirmed_at' => time()]); + } + + /** Give up the confirmation early — leaving a page should not keep it open. */ + public function forgetPasswordConfirmation(): void + { + session()->forget('auth.password_confirmed_at'); + } + + /** + * A value shown only in outline: the last four characters, and nothing else. + * + * Enough to tell two keys apart, useless to anyone reading over a shoulder + * or scrolling back through a screen recording. + */ + protected function outline(?string $value, int $keep = 4): ?string + { + if ($value === null || $value === '') { + return null; + } + + return Str::length($value) <= $keep + ? str_repeat('•', Str::length($value)) + : str_repeat('•', 8).Str::substr($value, -$keep); + } +} diff --git a/app/Livewire/Settings.php b/app/Livewire/Settings.php index e131dcc..01e0a02 100644 --- a/app/Livewire/Settings.php +++ b/app/Livewire/Settings.php @@ -12,6 +12,79 @@ use Livewire\WithFileUploads; class Settings extends Component { use \App\Livewire\Concerns\ChangesOwnPassword; + use \App\Livewire\Concerns\ConfirmsPassword; + + /** 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(\Laravel\Fortify\Actions\EnableTwoFactorAuthentication::class)(auth()->user()); + } + + /** Accept a code from the app, which is what actually turns it on. */ + public function confirmTwoFactor(): void + { + $this->requireConfirmedPassword(); + + try { + app(\Laravel\Fortify\Actions\ConfirmTwoFactorAuthentication::class)( + auth()->user(), + $this->twoFactorCode, + ); + } catch (\Illuminate\Validation\ValidationException $e) { + $this->addError('twoFactorCode', $e->errors()['code'][0] ?? __('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(auth()->user()->two_factor_recovery_codes), true); + $this->dispatch('notify', message: __('settings.twofa_on')); + } + + public function regenerateRecoveryCodes(): void + { + $this->requireConfirmedPassword(); + + app(\Laravel\Fortify\Actions\GenerateNewRecoveryCodes::class)(auth()->user()); + $this->recoveryCodes = json_decode(decrypt(auth()->user()->refresh()->two_factor_recovery_codes), true); + } + + public function disableTwoFactor(): void + { + $this->requireConfirmedPassword(); + + app(\Laravel\Fortify\Actions\DisableTwoFactorAuthentication::class)(auth()->user()); + $this->recoveryCodes = null; + $this->dispatch('notify', message: __('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); + } use ResolvesCustomer, WithFileUploads; @@ -152,7 +225,18 @@ class Settings extends Component $scheduled = $c?->instances()->where('status', 'cancellation_scheduled')->latest('id')->first(); $instance = $active ?? $scheduled ?? $c?->instances()->latest('id')->first(); + $user = auth()->user(); + return view('livewire.settings', [ + // 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' => $user->two_factor_secret !== null && $user->two_factor_confirmed_at === null, + 'twoFactorOn' => $user->two_factor_confirmed_at !== null, + 'twoFactorQr' => $user->two_factor_secret !== null && $user->two_factor_confirmed_at === null + ? $user->twoFactorQrCodeSvg() + : null, + 'passwordConfirmed' => $this->passwordRecentlyConfirmed(), 'customer' => $c, 'instance' => $instance, 'branding' => $c?->brandingResolved(), diff --git a/lang/de/settings.php b/lang/de/settings.php index 59d74e9..cbcdfd7 100644 --- a/lang/de/settings.php +++ b/lang/de/settings.php @@ -53,4 +53,23 @@ return [ 'close_mismatch' => 'Bitte das Bestätigungswort korrekt eingeben.', 'close_blocked_title' => 'Noch nicht möglich', 'close_blocked' => 'Bitte kündigen Sie zuerst Ihr aktives Paket, bevor Sie das Konto schließen.', + + 'twofa_title' => 'Zwei-Faktor-Anmeldung', + 'twofa_sub' => 'Zusätzlich zum Passwort ein Code aus einer App. Schützt das 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? Das 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.', ]; diff --git a/lang/en/settings.php b/lang/en/settings.php index 8d16e99..2782d09 100644 --- a/lang/en/settings.php +++ b/lang/en/settings.php @@ -53,4 +53,23 @@ return [ 'close_mismatch' => 'Please type the confirmation word correctly.', 'close_blocked_title' => 'Not possible yet', 'close_blocked' => 'Please cancel your active package before closing the account.', + + 'twofa_title' => 'Two-factor sign-in', + 'twofa_sub' => 'A code from an app in addition to the password. Protects the account even if the password gets out.', + 'twofa_state_on' => 'Active', + 'twofa_state_off' => 'Not set up', + 'twofa_confirm_first' => '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 right.', + 'twofa_on' => 'Two-factor sign-in is active.', + 'twofa_off' => 'Two-factor sign-in has been removed.', + 'twofa_off_confirm' => 'Really remove two-factor sign-in? The account will then be protected by its 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 works once.' ]; diff --git a/resources/views/livewire/admin/settings.blade.php b/resources/views/livewire/admin/settings.blade.php index a24a26f..907f99d 100644 --- a/resources/views/livewire/admin/settings.blade.php +++ b/resources/views/livewire/admin/settings.blade.php @@ -64,7 +64,7 @@ {{ __('admin_settings.update_running') }} @elseif ($update['available']) - + {{ __('admin_settings.update_now') }} @else @@ -124,7 +124,7 @@

- + {{ $sitePublic ? __('admin_settings.site_hide') : __('admin_settings.site_show') }} @@ -153,7 +153,7 @@

+ wire:click="toggleConsoleRestriction" wire:loading.attr="disabled" wire:target="toggleConsoleRestriction"> {{ $consoleRestricted ? __('admin_settings.console_unlock') : __('admin_settings.console_lock') }} @@ -188,7 +188,7 @@
- {{ __('admin_settings.console_add') }} + {{ __('admin_settings.console_add') }}

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

@@ -215,7 +215,7 @@
- {{ __('admin_settings.password_save') }} + {{ __('admin_settings.password_save') }}
diff --git a/resources/views/livewire/settings.blade.php b/resources/views/livewire/settings.blade.php index f87366a..32f6d8d 100644 --- a/resources/views/livewire/settings.blade.php +++ b/resources/views/livewire/settings.blade.php @@ -48,6 +48,84 @@ + + {{-- Two-factor. Every action is re-checked server-side against a recent + password confirmation — hiding the buttons stops nobody who can post + to /livewire/update. --}} +
+
+
+
+

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

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

{{ __('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. --}} +
+

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

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

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

+
+ +
+ + {{ __('settings.twofa_activate') }} + +
+
+ @else + + {{ __('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. --}} +
+

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

+

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

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

{{ __('settings.branding_title') }}

diff --git a/routes/web.php b/routes/web.php index c34034c..a6119d4 100644 --- a/routes/web.php +++ b/routes/web.php @@ -75,6 +75,30 @@ if (AdminArea::isExclusive()) { ->name('admin.legacy'); } +// The service status page. Its own address: it used to sit under /legal beside +// the imprint and the terms, and nothing about the current health of the +// platform is a legal document. +// +// Bound to its own hostname when one is configured, and every other host +// redirects there rather than 404ing — a status page is the one address people +// keep in a bookmark and reach for when something is already wrong. +$statusHost = (string) config('admin_access.status_host'); + +if ($statusHost !== '') { + // At the ROOT of its own hostname. "status.clupilot.com/status" says the + // same word twice and reads like a mistake, because it is one. + // + // Registered BEFORE the landing page below: `/` exists on both, Laravel + // takes the first match, and a host-agnostic route registered earlier wins + // over a domain-bound one registered later. + Route::domain($statusHost)->get('/', StatusController::class)->name('status'); + // The old path on that host keeps working rather than 404ing. + Route::domain($statusHost)->get('/status', fn () => redirect()->to('/', 301)); + Route::get('/status', fn () => redirect()->away('https://'.$statusHost, 301))->name('status.elsewhere'); +} else { + Route::get('/status', StatusController::class)->name('status'); +} + Route::get('/', LandingController::class)->name('home'); // Generated, not a static file: while the site is hidden this has to say so, @@ -90,22 +114,6 @@ Route::get('/robots.txt', function () { // Stripe webhook — paid order → customer provisioning run (CSRF-exempt, signed). Route::post('/webhooks/stripe', StripeWebhookController::class)->name('webhooks.stripe'); -// The service status page. Its own address: it used to sit under /legal beside -// the imprint and the terms, and nothing about the current health of the -// platform is a legal document. -// -// Bound to its own hostname when one is configured, and every other host -// redirects there rather than 404ing — a status page is the one address people -// keep in a bookmark and reach for when something is already wrong. -$statusHost = (string) config('admin_access.status_host'); - -if ($statusHost !== '') { - Route::domain($statusHost)->get('/status', StatusController::class)->name('status'); - Route::get('/status', fn () => redirect()->away('https://'.$statusHost.'/status', 301))->name('status.elsewhere'); -} else { - Route::get('/status', StatusController::class)->name('status'); -} - // Public legal pages (placeholders — replace with real content before launch). Route::prefix('legal')->name('legal.')->group(function () { Route::get('/impressum', fn () => view('legal', ['title' => 'Impressum']))->name('impressum'); diff --git a/tests/Feature/CustomerTwoFactorTest.php b/tests/Feature/CustomerTwoFactorTest.php new file mode 100644 index 0000000..cf77fb2 --- /dev/null +++ b/tests/Feature/CustomerTwoFactorTest.php @@ -0,0 +1,103 @@ +create(['status' => 'active']); + $user = $customer->ensureUser(); + $user->forceFill(['password' => bcrypt('password')])->save(); + + return $user; +} + +it('refuses every two-factor action without a recent password confirmation', function () { + $user = portalUser(); + + foreach (['enableTwoFactor', 'confirmTwoFactor', 'disableTwoFactor', 'regenerateRecoveryCodes'] as $action) { + Livewire::actingAs($user) + ->test(PortalSettings::class) + ->call($action) + ->assertForbidden(); + } + + expect($user->refresh()->two_factor_secret)->toBeNull(); +}); + +it('takes a password, then sets two-factor up and confirms it with a real code', function () { + $user = portalUser(); + + $page = Livewire::actingAs($user) + ->test(PortalSettings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->assertHasNoErrors(); + + $page->call('enableTwoFactor'); + expect($user->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($user->two_factor_confirmed_at)->toBeNull(); + + $code = app(PragmaRX\Google2FA\Google2FA::class)->getCurrentOtp(decrypt($user->two_factor_secret)); + + $page->set('twoFactorCode', $code)->call('confirmTwoFactor')->assertHasNoErrors(); + + expect($user->refresh()->two_factor_confirmed_at)->not->toBeNull() + // Shown once, here, because this is the only moment anyone reads them. + ->and($page->get('recoveryCodes'))->toBeArray()->not->toBeEmpty(); +}); + +it('rejects a wrong code instead of switching it on', function () { + $user = portalUser(); + + Livewire::actingAs($user) + ->test(PortalSettings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->call('enableTwoFactor') + ->set('twoFactorCode', '000000') + ->call('confirmTwoFactor') + ->assertHasErrors('twoFactorCode'); + + expect($user->refresh()->two_factor_confirmed_at)->toBeNull(); +}); + +it('does not accept a wrong password, and says so without hinting', function () { + $user = portalUser(); + + Livewire::actingAs($user) + ->test(PortalSettings::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. + $user = portalUser(); + + $page = Livewire::actingAs($user) + ->test(PortalSettings::class) + ->set('confirmablePassword', 'password') + ->call('confirmPassword') + ->call('enableTwoFactor'); + + $secret = decrypt($user->refresh()->two_factor_secret); + + expect(json_encode($page->snapshot))->not->toContain($secret); +}); diff --git a/tests/Feature/WelcomeTest.php b/tests/Feature/WelcomeTest.php index 49f5528..3ff750e 100644 --- a/tests/Feature/WelcomeTest.php +++ b/tests/Feature/WelcomeTest.php @@ -49,3 +49,22 @@ it('does not call the estate protected when an instance has no backup at all', f ->assertOk() ->assertSee(__('status.detail.backups', ['stale' => 1, 'total' => 2])); }); + +it('serves the status page at the root of its own hostname, and the landing page everywhere else', function () { + // "status.clupilot.com/status" says the same word twice. And the generic + // `/` route is registered host-agnostically, so a domain-bound `/` has to + // come FIRST or Laravel matches the landing page on the status host. + config()->set('admin_access.status_host', 'status.example.test'); + + $router = new Illuminate\Routing\Router(app('events'), app()); + Illuminate\Support\Facades\Route::swap($router); + require base_path('routes/web.php'); + $router->getRoutes()->refreshNameLookups(); + + $matches = fn (string $host, string $path) => $router->getRoutes() + ->match(Illuminate\Http\Request::create("http://{$host}{$path}", 'GET')) + ->getActionName(); + + expect($matches('status.example.test', '/'))->toContain('StatusController') + ->and($matches('www.example.test', '/'))->toContain('LandingController'); +});