From 3a4324fb6fa53bb27b98ae9f08af71ee8d7199b0 Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 16:50:29 +0200 Subject: [PATCH] Give people a way back in, and put the URL in English MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ── The hole ──────────────────────────────────────────────────────────────── There was no password reset. Fortify's feature was commented out, so there was no link on the sign-in form, no page and no route — a customer who forgot their password was locked out of their own cloud until somebody opened a shell. On a product whose selling point is that you can ring somebody, that is a support call a week and an embarrassing one. Two pages of ours under Fortify's route names (R1/R2), because with views off Fortify registers only the POST endpoints. The mail is a Mailable in this product's design rather than the framework's MailMessage: somebody who has just been locked out is exactly the person a phishing mail is aimed at, and a message that looks nothing like the rest of our post is one they cannot check — ours carries the footer that names our domains. Three decisions worth stating, each with a test: The answer is identical whether the address is known or not. "No account with that address" turns the form into a way of finding out who is a customer. The reset kills every other session. Whoever knew the old password may still be signed in somewhere, and a reset that leaves them there has fixed nothing. It does NOT sign the visitor in. The link arrived by email, and a mailbox somebody else can read would otherwise be a session somebody else gets. ── /sicherheit → /security ───────────────────────────────────────────────── R13: paths are English. Mine was not. The old path stays as a permanent redirect because it has already gone out in mail footers. ── One wordmark ──────────────────────────────────────────────────────────── "Sometimes it says Cloud and sometimes it does not" — correct. It was on the footer, the placeholder and the maintenance screen, and absent from the header, the sidebar and the sign-in plate. The product is called CluPilot Cloud; that is the name on the invoices and in every mail, so it is now the name everywhere. The console sidebar takes the small size so the lockup, the ADMIN badge and the close button still share one line (R18). Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- app/Livewire/Auth/ForgotPassword.php | 47 +++++++ app/Livewire/Auth/ResetPassword.php | 88 ++++++++++++ app/Mail/ResetPasswordMail.php | 47 +++++++ app/Models/User.php | 17 +++ config/fortify.php | 7 +- lang/de/auth.php | 16 +++ lang/de/reset_password.php | 13 ++ lang/en/auth.php | 16 +++ lang/en/reset_password.php | 13 ++ .../views/components/layouts/site.blade.php | 2 +- .../views/components/shell/nav.blade.php | 2 +- resources/views/components/ui/brand.blade.php | 9 +- .../livewire/auth/forgot-password.blade.php | 44 ++++++ resources/views/livewire/auth/login.blade.php | 5 +- .../livewire/auth/reset-password.blade.php | 41 ++++++ resources/views/mail/reset-password.blade.php | 39 ++++++ routes/web.php | 14 +- tests/Feature/Auth/PasswordResetTest.php | 131 ++++++++++++++++++ tests/Feature/OfficialDomainsTest.php | 7 +- 20 files changed, 551 insertions(+), 9 deletions(-) create mode 100644 app/Livewire/Auth/ForgotPassword.php create mode 100644 app/Livewire/Auth/ResetPassword.php create mode 100644 app/Mail/ResetPasswordMail.php create mode 100644 lang/de/reset_password.php create mode 100644 lang/en/reset_password.php create mode 100644 resources/views/livewire/auth/forgot-password.blade.php create mode 100644 resources/views/livewire/auth/reset-password.blade.php create mode 100644 resources/views/mail/reset-password.blade.php create mode 100644 tests/Feature/Auth/PasswordResetTest.php diff --git a/VERSION b/VERSION index 9181bf7..4fc01ac 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.18 +1.3.19 diff --git a/app/Livewire/Auth/ForgotPassword.php b/app/Livewire/Auth/ForgotPassword.php new file mode 100644 index 0000000..36b2a4b --- /dev/null +++ b/app/Livewire/Auth/ForgotPassword.php @@ -0,0 +1,47 @@ +validate(); + + // Throttled by Laravel's broker (once a minute per address). The + // response does not say which of the two happened: a slower answer for + // a known address would leak the same thing the message would. + Password::broker()->sendResetLink(['email' => $this->email]); + + $this->sent = true; + } + + public function render() + { + return view('livewire.auth.forgot-password'); + } +} diff --git a/app/Livewire/Auth/ResetPassword.php b/app/Livewire/Auth/ResetPassword.php new file mode 100644 index 0000000..84ba1cc --- /dev/null +++ b/app/Livewire/Auth/ResetPassword.php @@ -0,0 +1,88 @@ +token = $token; + $this->email = (string) request()->query('email', ''); + } + + /** + * NOT called reset(): Livewire\Component::reset() clears properties, and + * a component that redeclares it will not load at all. + */ + public function save(): void + { + $this->validate(); + + $status = Password::broker()->reset( + [ + 'email' => $this->email, + 'password' => $this->password, + 'password_confirmation' => $this->password_confirmation, + 'token' => $this->token, + ], + function (User $user, string $password) { + $user->forceFill([ + 'password' => Hash::make($password), + // Every existing session dies with it. Whoever knew the old + // password may still be signed in somewhere, and a reset + // that leaves them there has fixed nothing. + 'remember_token' => Str::random(60), + ])->save(); + + event(new PasswordReset($user)); + }, + ); + + if ($status !== Password::PASSWORD_RESET) { + $this->addError('email', __($status)); + + return; + } + + $this->done = true; + } + + public function render() + { + return view('livewire.auth.reset-password'); + } +} diff --git a/app/Mail/ResetPasswordMail.php b/app/Mail/ResetPasswordMail.php new file mode 100644 index 0000000..cdfe636 --- /dev/null +++ b/app/Mail/ResetPasswordMail.php @@ -0,0 +1,47 @@ +mailer('cp_'.MailPurpose::SYSTEM); + } + + public function envelope(): Envelope + { + return $this->mailboxEnvelope(MailPurpose::SYSTEM, __('reset_password.subject')); + } + + public function content(): Content + { + return new Content(view: 'mail.reset-password', with: [ + 'name' => $this->user->name, + 'url' => $this->url, + 'minutes' => $this->minutes, + ]); + } +} diff --git a/app/Models/User.php b/app/Models/User.php index a4d99bc..12c58a7 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -20,6 +20,23 @@ class User extends Authenticatable implements MustVerifyEmail /** @use HasFactory */ use HasFactory, Notifiable, TwoFactorAuthenticatable; + /** + * The reset mail in this product's design, not the framework's. + * + * Somebody who has just been locked out is exactly the person a phishing + * mail is aimed at. A message that looks nothing like the rest of our post + * is one they cannot check — and ours carries the footer naming our + * domains, which is the whole point. + */ + public function sendPasswordResetNotification(#[\SensitiveParameter] $token): void + { + \Illuminate\Support\Facades\Mail::to($this->email)->send(new \App\Mail\ResetPasswordMail( + $this, + route('password.reset', ['token' => $token, 'email' => $this->email]), + (int) config('auth.passwords.users.expire', 60), + )); + } + /** * The confirmation mail, in this product's design rather than Laravel's. * diff --git a/config/fortify.php b/config/fortify.php index cc7c717..184e21a 100644 --- a/config/fortify.php +++ b/config/fortify.php @@ -170,7 +170,12 @@ return [ // Registration is exposed via an app route (routes/web.php) with a // registration-scoped throttle instead of Fortify's unthrottled route. // Features::registration(), - // Features::resetPasswords(), + // Was off, which meant there was no forgot-password link, no page + // and no route: a customer who forgot their password was locked out + // of their own cloud until somebody opened a shell. The POST + // endpoints come from Fortify; the two GET pages are ours (R1/R2), + // registered in routes/web.php under Fortify's own route names. + Features::resetPasswords(), // Double opt-in. An account whose address was never confirmed is an // account that cannot be billed, cannot be told its server is down, // and may not be its owner's address at all — anybody can type diff --git a/lang/de/auth.php b/lang/de/auth.php index c964cb1..4546604 100644 --- a/lang/de/auth.php +++ b/lang/de/auth.php @@ -62,4 +62,20 @@ return [ 'phishing_note' => 'Sie sind gerade auf :host. Wir fragen Sie nie per E-Mail oder Telefon nach Ihrem Passwort.', 'phishing_link' => 'Echte Adressen erkennen', + + 'forgot_link' => "Passwort vergessen?", + 'forgot_title' => "Passwort vergessen", + 'forgot_subtitle' => "Wir schicken Ihnen einen Link, mit dem Sie ein neues vergeben.", + 'forgot_send' => "Link schicken", + 'forgot_sent_title' => "Schauen Sie in Ihr Postfach", + 'forgot_sent_body' => "Wenn es bei uns ein Konto zu :email gibt, ist der Link unterwegs. Er gilt eine Stunde und lässt sich einmal verwenden.", + 'forgot_sent_hint' => "Nichts angekommen? Sehen Sie im Spam-Ordner nach. Wir sagen aus Sicherheitsgründen nicht, ob es zu dieser Adresse ein Konto gibt.", + 'back_to_login' => "Zurück zur Anmeldung", + 'reset_title' => "Neues Passwort vergeben", + 'reset_subtitle' => "Danach melden Sie sich damit an.", + 'reset_new' => "Neues Passwort", + 'reset_repeat' => "Neues Passwort wiederholen", + 'reset_save' => "Passwort speichern", + 'reset_done_title' => "Passwort geändert", + 'reset_done_body' => "Alle anderen offenen Anmeldungen wurden beendet. Melden Sie sich jetzt mit dem neuen Passwort an — absichtlich nicht automatisch: der Link kam per E-Mail, und wer Ihr Postfach lesen kann, bekäme sonst gleich die Sitzung dazu.", ]; diff --git a/lang/de/reset_password.php b/lang/de/reset_password.php new file mode 100644 index 0000000..a9bca79 --- /dev/null +++ b/lang/de/reset_password.php @@ -0,0 +1,13 @@ + 'Neues Passwort für Ihren CluPilot-Zugang', + 'preheader' => 'Der Link gilt :minutes Minuten.', + 'heading' => 'Neues Passwort setzen', + 'greeting' => 'Guten Tag :name,', + 'intro' => 'Sie haben ein neues Passwort für Ihren CluPilot-Zugang angefordert. Über den Knopf unten vergeben Sie es.', + 'action' => 'Neues Passwort setzen', + 'expiry' => 'Der Link gilt :minutes und lässt sich nur einmal verwenden.', + 'fallback' => 'Falls der Knopf nicht funktioniert, kopieren Sie diese Adresse in Ihren Browser:', + 'not_you' => 'Sie haben das nicht angefordert? Dann ignorieren Sie diese Nachricht — ohne den Link ändert sich nichts an Ihrem Zugang. Wir fragen Sie nie per E-Mail nach Ihrem Passwort.', +]; diff --git a/lang/en/auth.php b/lang/en/auth.php index ae5445a..4f31b5d 100644 --- a/lang/en/auth.php +++ b/lang/en/auth.php @@ -62,4 +62,20 @@ return [ 'phishing_note' => 'You are currently on :host. We never ask for your password by email or on the phone.', 'phishing_link' => 'How to recognise our addresses', + + 'forgot_link' => "Forgotten your password?", + 'forgot_title' => "Forgotten password", + 'forgot_subtitle' => "We will send you a link to set a new one.", + 'forgot_send' => "Send the link", + 'forgot_sent_title' => "Check your inbox", + 'forgot_sent_body' => "If we have an account for :email, the link is on its way. It is valid for an hour and can be used once.", + 'forgot_sent_hint' => "Nothing arrived? Check your spam folder. For security we do not say whether an account exists for that address.", + 'back_to_login' => "Back to sign-in", + 'reset_title' => "Set a new password", + 'reset_subtitle' => "Then sign in with it.", + 'reset_new' => "New password", + 'reset_repeat' => "Repeat the new password", + 'reset_save' => "Save the password", + 'reset_done_title' => "Password changed", + 'reset_done_body' => "Every other open session has been ended. Sign in now with the new password — deliberately not automatically: the link arrived by email, and whoever can read your mailbox would otherwise get the session with it.", ]; diff --git a/lang/en/reset_password.php b/lang/en/reset_password.php new file mode 100644 index 0000000..afa4427 --- /dev/null +++ b/lang/en/reset_password.php @@ -0,0 +1,13 @@ + 'A new password for your CluPilot account', + 'preheader' => 'The link is valid for :minutes minutes.', + 'heading' => 'Set a new password', + 'greeting' => 'Hello :name,', + 'intro' => 'You asked for a new password for your CluPilot account. Use the button below to set one.', + 'action' => 'Set a new password', + 'expiry' => 'The link is valid for :minutes and can be used once.', + 'fallback' => 'If the button does not work, copy this address into your browser:', + 'not_you' => 'You did not ask for this? Then ignore this message — without the link nothing about your account changes. We never ask for your password by email.', +]; diff --git a/resources/views/components/layouts/site.blade.php b/resources/views/components/layouts/site.blade.php index fdd08e8..ec34e6c 100644 --- a/resources/views/components/layouts/site.blade.php +++ b/resources/views/components/layouts/site.blade.php @@ -73,7 +73,7 @@
- +

Eine eigene, isolierte Cloud für Ihr Unternehmen — eingerichtet, gesichert und überwacht von uns, mit Serverstandort in der EU. diff --git a/resources/views/components/shell/nav.blade.php b/resources/views/components/shell/nav.blade.php index c585863..7f143b1 100644 --- a/resources/views/components/shell/nav.blade.php +++ b/resources/views/components/shell/nav.blade.php @@ -22,7 +22,7 @@ >

- + @if ($console) {{ __('admin.badge') }} @endif diff --git a/resources/views/components/ui/brand.blade.php b/resources/views/components/ui/brand.blade.php index f36dd08..6f463a8 100644 --- a/resources/views/components/ui/brand.blade.php +++ b/resources/views/components/ui/brand.blade.php @@ -1,6 +1,5 @@ @props([ 'size' => 'md', // sm | md | lg - 'suffix' => false, // append "Cloud" in the muted tone 'tone' => 'ink', // ink | white — for the dark sign-in plate ]) {{-- @@ -16,6 +15,12 @@ of them, and because a two-tone wordmark makes the accent a permanent fixture rather than something used sparingly. + "Cloud" is no longer optional either. It was on the footer, the placeholder + and the maintenance screen and absent from the header, the sidebar and the + sign-in plate — "sometimes it says Cloud and sometimes it does not", which + is exactly right. The product is called CluPilot Cloud; that is the name on + the invoices and in every mail, so it is the name everywhere. + The mark and the word are the only two children of the flex row, and the word is a single element. `gap` applies between EVERY child of a flex container and a bare text node is a child — written the other way this @@ -33,5 +38,5 @@ view with a parse error. The newline also supplies the space before "Cloud". --}} CluPilot - @if ($suffix)Cloud@endif + Cloud diff --git a/resources/views/livewire/auth/forgot-password.blade.php b/resources/views/livewire/auth/forgot-password.blade.php new file mode 100644 index 0000000..b7d04e0 --- /dev/null +++ b/resources/views/livewire/auth/forgot-password.blade.php @@ -0,0 +1,44 @@ + diff --git a/resources/views/livewire/auth/login.blade.php b/resources/views/livewire/auth/login.blade.php index 4a8568d..9d90173 100644 --- a/resources/views/livewire/auth/login.blade.php +++ b/resources/views/livewire/auth/login.blade.php @@ -25,7 +25,10 @@ @csrf - + {{ __('auth.sign_in') }} diff --git a/resources/views/livewire/auth/reset-password.blade.php b/resources/views/livewire/auth/reset-password.blade.php new file mode 100644 index 0000000..c224ff8 --- /dev/null +++ b/resources/views/livewire/auth/reset-password.blade.php @@ -0,0 +1,41 @@ +
+ + +
+
+ + + @if ($done) +
+ +
+

{{ __('auth.reset_done_title') }}

+ {{-- Not signed in automatically: a reset link travels by email, + and a mailbox somebody else can read would otherwise be a + session somebody else gets. --}} +

{{ __('auth.reset_done_body') }}

+ + {{ __('auth.sign_in') }} + @else +

{{ __('auth.reset_title') }}

+

{{ __('auth.reset_subtitle') }}

+ +
+ + + + {{ __('auth.reset_save') }} + + @endif +
+
+
diff --git a/resources/views/mail/reset-password.blade.php b/resources/views/mail/reset-password.blade.php new file mode 100644 index 0000000..f12437f --- /dev/null +++ b/resources/views/mail/reset-password.blade.php @@ -0,0 +1,39 @@ + + + +

{{ __('reset_password.intro') }}

+ + + + + +
+ {{ __('reset_password.action') }} +
+

{!! __('reset_password.expiry', ['minutes' => ''.$minutes.' Minuten']) !!}

+ + +{{-- The link as text as well, for the same reason as on the verification mail: + corporate mail gateways rewrite button links and some of them fetch the + target first to scan it, which on a single-use link spends it before the + recipient ever clicks. --}} + + + +
+

{{ __('reset_password.fallback') }}

+

{{ $url }}

+
+ + +{{-- The sentence that matters most on this particular mail. Somebody who did + NOT ask for it has just learned that a stranger knows their address, and + the right advice is "do nothing" — not "contact us immediately", which is + what a phishing copy would say. --}} + +

{{ __('reset_password.not_you') }}

+ diff --git a/routes/web.php b/routes/web.php index 5545d03..381fdd5 100644 --- a/routes/web.php +++ b/routes/web.php @@ -183,7 +183,12 @@ $publicSite = function () { // Reachable WITHOUT an account, deliberately: somebody who has just typed // their password into a copy of our sign-in form is not signed in anywhere, // and the page they need cannot be behind the thing they lost. - Route::get('/sicherheit', fn () => view('security'))->name('security'); + Route::get('/security', fn () => view('security'))->name('security'); + + // The German path this shipped under for two releases. R13 says paths are + // English and this one was not — mine. Kept as a permanent redirect + // because it has already gone out in mail footers. + Route::get('/sicherheit', fn () => redirect()->route('security', status: 301)); Route::get('/robots.txt', function () { $body = App\Support\Settings::bool('site.public', true) @@ -220,6 +225,13 @@ $portal = function () { Route::post('/register', [\Laravel\Fortify\Http\Controllers\RegisteredUserController::class, 'store']) ->middleware('throttle:registration') ->name('register.store'); + // Fortify registers the two POST endpoints; with views off it + // registers no GET routes at all, so the pages are ours under its + // names — every framework redirect and the reset mail resolve + // `password.request` and `password.reset`. + Route::get('/forgot-password', \App\Livewire\Auth\ForgotPassword::class)->name('password.request'); + Route::get('/reset-password/{token}', \App\Livewire\Auth\ResetPassword::class)->name('password.reset'); + Route::get('/two-factor-challenge', TwoFactorChallenge::class)->name('two-factor.login'); }); diff --git a/tests/Feature/Auth/PasswordResetTest.php b/tests/Feature/Auth/PasswordResetTest.php new file mode 100644 index 0000000..e09670e --- /dev/null +++ b/tests/Feature/Auth/PasswordResetTest.php @@ -0,0 +1,131 @@ +get(route('login')) + ->assertOk() + ->assertSee(__('auth.forgot_link')) + ->assertSee(route('password.request'), false); +}); + +it('sends a link in this product’s own design', function () { + Mail::fake(); + + $user = User::factory()->create(['email' => 'kunde@example.test']); + + Livewire::test(ForgotPassword::class) + ->set('email', $user->email) + ->call('send') + ->assertHasNoErrors(); + + // Not the framework's default MailMessage: somebody who has just been + // locked out is exactly the person a phishing mail is aimed at, and a + // message that looks nothing like the rest of our post is one they cannot + // check against the others. + Mail::assertQueued(ResetPasswordMail::class, fn ($mail) => $mail->hasTo($user->email)); +}); + +it('answers the same whether the address is known or not', function () { + // Otherwise the form is a way of finding out who is a customer, which is + // the first thing anybody targeting a business does. + Mail::fake(); + + $known = Livewire::test(ForgotPassword::class)->set('email', User::factory()->create()->email)->call('send'); + $unknown = Livewire::test(ForgotPassword::class)->set('email', 'niemand@example.test')->call('send'); + + $known->assertSet('sent', true)->assertHasNoErrors(); + $unknown->assertSet('sent', true)->assertHasNoErrors(); + + Mail::assertQueuedCount(1); +}); + +it('sets the new password and kills every other session', function () { + $user = User::factory()->create(['password' => Hash::make('das-alte-passwort')]); + $before = $user->remember_token; + $token = Password::broker()->createToken($user); + + Livewire::test(ResetPassword::class, ['token' => $token]) + ->set('email', $user->email) + ->set('password', 'ein-langes-neues-passwort') + ->set('password_confirmation', 'ein-langes-neues-passwort') + ->call('save') + ->assertHasNoErrors() + ->assertSet('done', true); + + $user->refresh(); + + expect(Hash::check('ein-langes-neues-passwort', $user->password))->toBeTrue() + // Whoever knew the old password may still be signed in somewhere. A + // reset that leaves them there has fixed nothing. + ->and($user->remember_token)->not->toBe($before); +}); + +it('does not sign the visitor in', function () { + // The link arrived by email. A mailbox somebody else can read would + // otherwise be a session somebody else gets. + $user = User::factory()->create(); + $token = Password::broker()->createToken($user); + + Livewire::test(ResetPassword::class, ['token' => $token]) + ->set('email', $user->email) + ->set('password', 'ein-langes-neues-passwort') + ->set('password_confirmation', 'ein-langes-neues-passwort') + ->call('save'); + + expect(auth()->check())->toBeFalse(); +}); + +it('refuses a token that is not this user’s', function () { + $mine = User::factory()->create(); + $theirs = User::factory()->create(); + + Livewire::test(ResetPassword::class, ['token' => Password::broker()->createToken($theirs)]) + ->set('email', $mine->email) + ->set('password', 'ein-langes-neues-passwort') + ->set('password_confirmation', 'ein-langes-neues-passwort') + ->call('save') + ->assertHasErrors('email') + ->assertSet('done', false); +}); + +it('spends the token, so a forwarded link cannot be used twice', function () { + $user = User::factory()->create(); + $token = Password::broker()->createToken($user); + + $reset = fn () => Livewire::test(ResetPassword::class, ['token' => $token]) + ->set('email', $user->email) + ->set('password', 'ein-langes-neues-passwort') + ->set('password_confirmation', 'ein-langes-neues-passwort') + ->call('save'); + + $reset()->assertSet('done', true); + $reset()->assertSet('done', false)->assertHasErrors('email'); +}); + +it('insists on a password long enough to be worth resetting to', function () { + $user = User::factory()->create(); + + Livewire::test(ResetPassword::class, ['token' => Password::broker()->createToken($user)]) + ->set('email', $user->email) + ->set('password', 'kurz') + ->set('password_confirmation', 'kurz') + ->call('save') + ->assertHasErrors('password'); +}); diff --git a/tests/Feature/OfficialDomainsTest.php b/tests/Feature/OfficialDomainsTest.php index 07e75a3..0d6bebb 100644 --- a/tests/Feature/OfficialDomainsTest.php +++ b/tests/Feature/OfficialDomainsTest.php @@ -56,11 +56,16 @@ it('serves the page to somebody who is not signed in', function () { // Somebody who has just given their password to a copy of our sign-in form // is signed in nowhere. The page they need cannot be behind the thing they // lost. - $this->get('/sicherheit') + // The path is English (R13). /sicherheit shipped for two releases and + // still redirects, which is asserted below. + $this->get('/security') ->assertOk() ->assertSee('clupilot.com') ->assertSee('clupilot.cloud') ->assertSee(__('security.compromised_title')); + + // The German path went out in mail footers before the rule was applied. + $this->get('/sicherheit')->assertRedirect('/security'); }); it('carries the domains in every mail footer', function () {