From 75b4a4776860bbc36b80d891048c604b8ceac089 Mon Sep 17 00:00:00 2001 From: nexxo Date: Tue, 28 Jul 2026 23:19:20 +0200 Subject: [PATCH] Stop mailing the initial admin password, hold it in the panel until noted The password no longer travels through third-party mail servers or sits forever in an inbox: it moves onto the instance (encrypted, same as nc_admin_ref) and shows on the dashboard until the customer clicks "Notiert", which deletes it for good. --- app/Livewire/Dashboard.php | 55 +++++++++++++- app/Models/Instance.php | 10 ++- app/Notifications/CloudReady.php | 28 ++++---- .../Steps/Customer/CompleteProvisioning.php | 10 ++- ..._add_admin_password_to_instances_table.php | 29 ++++++++ lang/de/dashboard.php | 14 ++++ lang/de/provisioning.php | 8 +-- lang/en/dashboard.php | 14 ++++ lang/en/provisioning.php | 8 +-- resources/views/livewire/dashboard.blade.php | 63 ++++++++++++++++ tests/Feature/DashboardTest.php | 71 +++++++++++++++++++ tests/Feature/Mail/SenderAddressTest.php | 19 +---- .../CustomerProvisioningEndToEndTest.php | 8 ++- .../Provisioning/CustomerStepsTest.php | 32 ++++++++- 14 files changed, 324 insertions(+), 45 deletions(-) create mode 100644 database/migrations/2026_07_29_120000_add_admin_password_to_instances_table.php diff --git a/app/Livewire/Dashboard.php b/app/Livewire/Dashboard.php index a1f087e..7dec72a 100644 --- a/app/Livewire/Dashboard.php +++ b/app/Livewire/Dashboard.php @@ -42,12 +42,13 @@ class Dashboard extends Component $contract = $instance?->subscription; $maintenance = MaintenanceWindow::forInstance($instance)->first(); + $domain = $this->domain($instance); return view('livewire.dashboard', [ 'customer' => $customer, 'instance' => $instance, 'contract' => $contract, - 'domain' => $this->domain($instance), + 'domain' => $domain, 'location' => $this->location($instance), 'seats' => $this->seats($customer, $contract), 'traffic' => $instance !== null ? TrafficMeter::for($instance) : null, @@ -73,10 +74,62 @@ class Dashboard extends Component 'term' => (string) $contract->term, ], 'nextInvoice' => $this->nextInvoice($contract, $instance), + // The initial admin password, if it is still waiting to be + // acknowledged. Read straight off $instance (already scoped to + // THIS customer above) rather than a fresh unscoped lookup, and + // never stored as a public property — see acknowledgeCredentials(). + 'credentials' => $this->credentials($instance, $domain), 'asOf' => Carbon::now(), ]); } + /** + * Deletes the instance's stored admin password and stamps the + * acknowledgement — the customer has confirmed they noted it down. + * + * Re-resolves both the customer AND the instance from the authenticated + * session rather than trusting $uuid alone: this method is reachable by + * anyone who can post to /livewire/update, not only through this card's + * button, so ownership is checked here again rather than relying on the + * card simply not being shown to anyone else. + */ + public function acknowledgeCredentials(string $uuid): void + { + $customer = $this->requireCustomer(); + if ($customer === null) { + return; + } + + $instance = $customer->instances()->where('uuid', $uuid)->whereNotNull('admin_password')->first(); + if ($instance === null) { + return; + } + + $instance->update([ + 'admin_password' => null, + 'credentials_acknowledged_at' => now(), + ]); + + $this->dispatch('notify', message: __('dashboard.credentials.acknowledged')); + } + + /** + * @return array{uuid: string, url: string, user: string, password: string}|null + */ + private function credentials(?Instance $instance, ?string $domain): ?array + { + if ($instance === null || $instance->admin_password === null) { + return null; + } + + return [ + 'uuid' => $instance->uuid, + 'url' => 'https://'.($domain ?: $instance->subdomain), + 'user' => (string) $instance->nc_admin_ref, + 'password' => $instance->admin_password, + ]; + } + /** * Who holds the seats, so the card can say something true underneath the * figure rather than repeating it. diff --git a/app/Models/Instance.php b/app/Models/Instance.php index 91c14fd..032035c 100644 --- a/app/Models/Instance.php +++ b/app/Models/Instance.php @@ -16,16 +16,22 @@ class Instance extends Model protected $fillable = [ 'customer_id', 'order_id', 'host_id', 'vmid', 'guest_ip', 'plan', 'quota_gb', 'traffic_addons', 'disk_gb', - 'ram_mb', 'cores', 'subdomain', 'custom_domain', 'nc_admin_ref', + 'ram_mb', 'cores', 'subdomain', 'custom_domain', 'nc_admin_ref', 'admin_password', 'credentials_acknowledged_at', 'route_written', 'cert_ok', 'status', 'cancel_requested_at', 'service_ends_at', ]; - protected $hidden = ['nc_admin_ref']; + protected $hidden = ['nc_admin_ref', 'admin_password']; protected function casts(): array { return [ 'nc_admin_ref' => 'encrypted', + // The initial Nextcloud admin password, held only until the + // customer acknowledges it on the dashboard (App\Livewire\ + // Dashboard::acknowledgeCredentials). Same 'encrypted' cast as + // nc_admin_ref and Host::api_token_ref, for the same reason. + 'admin_password' => 'encrypted', + 'credentials_acknowledged_at' => 'datetime', 'route_written' => 'boolean', 'cert_ok' => 'boolean', 'vmid' => 'integer', diff --git a/app/Notifications/CloudReady.php b/app/Notifications/CloudReady.php index 8ab58c0..1c5fa2c 100644 --- a/app/Notifications/CloudReady.php +++ b/app/Notifications/CloudReady.php @@ -8,17 +8,23 @@ use App\Services\Mail\MailPurpose; use Illuminate\Bus\Queueable; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; -use Illuminate\Support\Facades\Crypt; /** - * Delivers the initial admin credentials once provisioning completes. + * Announces that provisioning has finished. Carries NO credential. + * + * The initial admin password used to travel here in clear text — crossing + * third-party mail servers and then sitting in an inbox forever, the weakest + * link in the chain. It now lives on the instance instead + * (Instance::$admin_password, encrypted — see CompleteProvisioning) until the + * customer acknowledges it on their dashboard, at which point it is deleted + * for good. This mail only says the cloud is ready and where to find them. * * Sent SYNCHRONOUSLY from CompleteProvisioning (not queued): the step only - * records the `credentials_sent` breadcrumb and scrubs the password AFTER the - * send returns, so a mail failure re-runs the step (password preserved) instead - * of losing the credential in a failed queue job. Delivery is at-least-once — a - * crash in the tiny window after send but before the breadcrumb can re-send a - * duplicate welcome email, which is preferable to a lost credential. + * records the `credentials_sent` breadcrumb AFTER the send returns, so a mail + * failure re-runs the step instead of losing the breadcrumb in a failed queue + * job. Delivery is at-least-once — a crash in the tiny window after send but + * before the breadcrumb can re-send a duplicate welcome email, which is + * preferable to the customer never being told their cloud is ready. */ class CloudReady extends Notification { @@ -26,8 +32,6 @@ class CloudReady extends Notification public function __construct( public Instance $instance, - public string $adminUser, - public string $adminPasswordEncrypted, // ciphertext — safe to serialize into the queue ) {} /** @return array */ @@ -56,8 +60,8 @@ class CloudReady extends Notification ->subject(__('provisioning.mail.ready_subject')) ->greeting(__('provisioning.mail.ready_greeting')) ->line(__('provisioning.mail.ready_line')) - ->line(__('provisioning.mail.ready_user', ['user' => $this->adminUser])) - ->line(__('provisioning.mail.ready_password', ['password' => Crypt::decryptString($this->adminPasswordEncrypted)])) - ->action(__('provisioning.mail.ready_action'), $url); + ->line(__('provisioning.mail.ready_address', ['url' => $url])) + ->line(__('provisioning.mail.ready_credentials')) + ->action(__('provisioning.mail.ready_action'), route('dashboard')); } } diff --git a/app/Provisioning/Steps/Customer/CompleteProvisioning.php b/app/Provisioning/Steps/Customer/CompleteProvisioning.php index 7b51ffb..f37e816 100644 --- a/app/Provisioning/Steps/Customer/CompleteProvisioning.php +++ b/app/Provisioning/Steps/Customer/CompleteProvisioning.php @@ -5,6 +5,7 @@ namespace App\Provisioning\Steps\Customer; use App\Models\ProvisioningRun; use App\Notifications\CloudReady; use App\Provisioning\StepResult; +use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Notification; class CompleteProvisioning extends CustomerStep @@ -32,9 +33,14 @@ class CompleteProvisioning extends CustomerStep $encrypted = $run->context('admin_password'); if ($encrypted !== null && ! $this->hasResource($run, 'credentials_sent')) { - // Pass the ENCRYPTED password (decrypted only when building the mail). + // Move the password onto the instance (encrypted cast — see + // Instance::casts()) so it survives until the customer + // acknowledges it on the dashboard. Decrypted here only to + // re-encrypt it under that cast; it never reaches the mail. + $instance->update(['admin_password' => Crypt::decryptString($encrypted)]); + Notification::route('mail', $order->customer->email)->notify( - new CloudReady($instance, (string) $instance->nc_admin_ref, $encrypted), + new CloudReady($instance), ); // Record delivery so a retry after a crash doesn't re-send. $this->recordResource($run, $instance->host, 'credentials_sent', (string) $instance->nc_admin_ref); diff --git a/database/migrations/2026_07_29_120000_add_admin_password_to_instances_table.php b/database/migrations/2026_07_29_120000_add_admin_password_to_instances_table.php new file mode 100644 index 0000000..ae02fbf --- /dev/null +++ b/database/migrations/2026_07_29_120000_add_admin_password_to_instances_table.php @@ -0,0 +1,29 @@ +text('admin_password')->nullable()->after('nc_admin_ref'); + $table->timestamp('credentials_acknowledged_at')->nullable()->after('admin_password'); + }); + } + + public function down(): void + { + Schema::table('instances', function (Blueprint $table) { + $table->dropColumn(['admin_password', 'credentials_acknowledged_at']); + }); + } +}; diff --git a/lang/de/dashboard.php b/lang/de/dashboard.php index c13fec7..5852824 100644 --- a/lang/de/dashboard.php +++ b/lang/de/dashboard.php @@ -28,6 +28,20 @@ return [ 'no_instance_body' => 'Für dieses Konto läuft derzeit keine Cloud. Sobald eine Bestellung bezahlt ist, richten wir sie ein — dieser Bereich füllt sich dann von selbst.', 'no_instance_cta' => 'Paket wählen', + // Das Erst-Passwort steht hier — statt in der Mail — bis der Kunde es + // bestätigt. Nach der Bestätigung ist es unwiderruflich gelöscht. + 'credentials' => [ + 'title' => 'Ihre Zugangsdaten', + 'body' => 'Notieren Sie sich Benutzername und Passwort an einem sicheren Ort. Nach der Bestätigung werden sie hier endgültig gelöscht.', + 'address' => 'Adresse', + 'user' => 'Benutzername', + 'password' => 'Passwort', + 'copy' => 'Kopieren', + 'copied' => 'Kopiert', + 'acknowledge' => 'Notiert', + 'acknowledged' => 'Zugangsdaten bestätigt — das Passwort ist jetzt gelöscht.', + ], + 'master' => [ 'label' => 'Stammblatt', 'package' => 'Paket', diff --git a/lang/de/provisioning.php b/lang/de/provisioning.php index 48d6790..a4e6def 100644 --- a/lang/de/provisioning.php +++ b/lang/de/provisioning.php @@ -22,9 +22,9 @@ return [ 'mail' => [ 'ready_subject' => 'Ihre CluPilot Cloud ist bereit', 'ready_greeting' => 'Willkommen bei CluPilot!', - 'ready_line' => 'Ihre Cloud ist eingerichtet und einsatzbereit. Hier sind Ihre Zugangsdaten:', - 'ready_user' => 'Benutzername: :user', - 'ready_password' => 'Initial-Passwort: :password (bitte nach dem ersten Login ändern)', - 'ready_action' => 'Zur Cloud', + 'ready_line' => 'Ihre Cloud ist eingerichtet und einsatzbereit.', + 'ready_address' => 'Sie erreichen sie unter :url.', + 'ready_credentials' => 'Ihre Zugangsdaten liegen in Ihrem Kundenportal für Sie bereit.', + 'ready_action' => 'Zum Kundenportal', ], ]; diff --git a/lang/en/dashboard.php b/lang/en/dashboard.php index 31b438c..9b5958d 100644 --- a/lang/en/dashboard.php +++ b/lang/en/dashboard.php @@ -28,6 +28,20 @@ return [ 'no_instance_body' => 'No cloud is running for this account at the moment. As soon as an order is paid we set one up — this area then fills itself.', 'no_instance_cta' => 'Choose a package', + // The initial password lives here — not in the mail — until the customer + // confirms it. Once confirmed it is deleted for good. + 'credentials' => [ + 'title' => 'Your credentials', + 'body' => 'Note down the username and password somewhere safe. Once you confirm, they are deleted here for good.', + 'address' => 'Address', + 'user' => 'Username', + 'password' => 'Password', + 'copy' => 'Copy', + 'copied' => 'Copied', + 'acknowledge' => 'Noted', + 'acknowledged' => 'Credentials confirmed — the password has now been deleted.', + ], + 'master' => [ 'label' => 'Master record', 'package' => 'Package', diff --git a/lang/en/provisioning.php b/lang/en/provisioning.php index b5cf271..c081a06 100644 --- a/lang/en/provisioning.php +++ b/lang/en/provisioning.php @@ -22,9 +22,9 @@ return [ 'mail' => [ 'ready_subject' => 'Your CluPilot cloud is ready', 'ready_greeting' => 'Welcome to CluPilot!', - 'ready_line' => 'Your cloud is set up and ready to use. Here are your credentials:', - 'ready_user' => 'Username: :user', - 'ready_password' => 'Initial password: :password (please change it after your first login)', - 'ready_action' => 'Open your cloud', + 'ready_line' => 'Your cloud is set up and ready to use.', + 'ready_address' => 'You can reach it at :url.', + 'ready_credentials' => 'Your credentials are waiting for you in your customer portal.', + 'ready_action' => 'Open customer portal', ], ]; diff --git a/resources/views/livewire/dashboard.blade.php b/resources/views/livewire/dashboard.blade.php index 67ec9c4..cd7eb39 100644 --- a/resources/views/livewire/dashboard.blade.php +++ b/resources/views/livewire/dashboard.blade.php @@ -39,6 +39,69 @@ {{-- Live provisioning progress (only while a run is in flight) --}} + @if ($credentials) + {{-- The initial admin password, shown here instead of mailed. Gone the + moment the customer confirms they noted it — acknowledgeCredentials() + deletes it outright, not just this card. --}} +
+
+ +
+

{{ __('dashboard.credentials.title') }}

+

{{ __('dashboard.credentials.body') }}

+ +
+
+
{{ __('dashboard.credentials.address') }}
+
+ {{ $credentials['url'] }} +
+
+
+
{{ __('dashboard.credentials.user') }}
+
+ {{ $credentials['user'] }} + +
+
+
+
{{ __('dashboard.credentials.password') }}
+
+ {{ $credentials['password'] }} + +
+
+
+ +
+ + {{ __('dashboard.credentials.acknowledge') }} + +

{{ __('dashboard.credentials.copied') }}

+
+
+
+
+ @endif + @if ($instance === null) {{-- Said plainly rather than papered over with an empty record: a page of dashes reads as broken, and "we are still setting it up" and diff --git a/tests/Feature/DashboardTest.php b/tests/Feature/DashboardTest.php index 826c047..7995d38 100644 --- a/tests/Feature/DashboardTest.php +++ b/tests/Feature/DashboardTest.php @@ -1,6 +1,11 @@ get('/dashboard')->assertRedirect('/login'); @@ -48,3 +53,69 @@ it('shows no next invoice once the customer has given notice', function () { ->assertViewHas('planPrice', fn (?array $p) => $p !== null && $p['cents'] === 17900) ->assertDontSee(__('dashboard.invoice.label')); }); + +it('shows the credentials card, and lets acknowledging delete the password rather than just hide it', function () { + $user = User::factory()->create(['email' => 'ack@cred.test']); + $customer = Customer::factory()->create(['email' => 'ack@cred.test', 'user_id' => $user->id]); + $order = Order::factory()->create(['customer_id' => $customer->id]); + $instance = Instance::factory()->create([ + 'customer_id' => $customer->id, + 'order_id' => $order->id, + 'status' => 'active', + 'nc_admin_ref' => 'admin', + 'admin_password' => 'my-secret-pw', + ]); + + Livewire::actingAs($user) + ->test(Dashboard::class) + ->assertSee(__('dashboard.credentials.title')) + ->assertSee('my-secret-pw') + ->call('acknowledgeCredentials', $instance->uuid) + ->assertDontSee('my-secret-pw') + ->assertDontSee(__('dashboard.credentials.title')); + + // Not merely hidden from the card — actually gone, at the raw stored + // column too, so a mutant that only toggled a "hide the card" flag + // without clearing admin_password is caught here. + expect($instance->fresh()->admin_password)->toBeNull() + ->and($instance->fresh()->credentials_acknowledged_at)->not->toBeNull(); + + $raw = DB::table('instances')->where('id', $instance->id)->value('admin_password'); + expect($raw)->toBeNull(); +}); + +it("refuses to acknowledge — or leak — another customer's credentials", function () { + $userA = User::factory()->create(['email' => 'a@cred.test']); + $customerA = Customer::factory()->create(['email' => 'a@cred.test', 'user_id' => $userA->id]); + $orderA = Order::factory()->create(['customer_id' => $customerA->id]); + $instanceA = Instance::factory()->create([ + 'customer_id' => $customerA->id, + 'order_id' => $orderA->id, + 'status' => 'active', + 'nc_admin_ref' => 'admin', + 'admin_password' => 'a-secret-pw', + ]); + + $userB = User::factory()->create(['email' => 'b@cred.test']); + $customerB = Customer::factory()->create(['email' => 'b@cred.test', 'user_id' => $userB->id]); + $orderB = Order::factory()->create(['customer_id' => $customerB->id]); + Instance::factory()->create([ + 'customer_id' => $customerB->id, + 'order_id' => $orderB->id, + 'status' => 'active', + ]); + + // B's own dashboard render must not show A's password — render() scopes + // $instance to $customer's own instances, so this should hold already. + Livewire::actingAs($userB) + ->test(Dashboard::class) + ->assertDontSee('a-secret-pw') + // B posting A's instance uuid directly — exactly what an unchecked + // Livewire action allows, since the card that would normally supply + // this uuid is never rendered for B. acknowledgeCredentials() must + // re-resolve the instance from B's own customer, not trust the uuid. + ->call('acknowledgeCredentials', $instanceA->uuid); + + expect($instanceA->fresh()->admin_password)->not->toBeNull() + ->and($instanceA->fresh()->credentials_acknowledged_at)->toBeNull(); +}); diff --git a/tests/Feature/Mail/SenderAddressTest.php b/tests/Feature/Mail/SenderAddressTest.php index 1a8f6ac..129b943 100644 --- a/tests/Feature/Mail/SenderAddressTest.php +++ b/tests/Feature/Mail/SenderAddressTest.php @@ -11,7 +11,6 @@ use App\Services\Mail\MailPurpose; use App\Services\Maintenance\MaintenanceNotifier; use App\Support\Settings; use Illuminate\Mail\SendQueuedMailable; -use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Queue; beforeEach(function () { @@ -129,11 +128,7 @@ it('sends the provisioning notification from the provisioning mailbox', function ]); Settings::set(MailPurpose::settingKey(MailPurpose::PROVISIONING), 'no-reply'); - $notification = new CloudReady( - Instance::factory()->create(), - 'admin', - Crypt::encryptString('geheim'), - ); + $notification = new CloudReady(Instance::factory()->create()); $message = $notification->toMail(new stdClass); @@ -152,11 +147,7 @@ it('sets Reply-To on the provisioning notification when its mailbox allows repli Mailbox::factory()->create(['key' => 'support', 'address' => 'support@clupilot.com', 'no_reply' => false]); Settings::set(MailPurpose::settingKey(MailPurpose::PROVISIONING), 'support'); - $notification = new CloudReady( - Instance::factory()->create(), - 'admin', - Crypt::encryptString('geheim'), - ); + $notification = new CloudReady(Instance::factory()->create()); $message = $notification->toMail(new stdClass); @@ -169,11 +160,7 @@ it("leaves the provisioning notification's sender fields at MailMessage's defaul // Settings mapping exists, so MailboxResolver::for() returns null. Must // not throw dereferencing a null mailbox, and must leave from/replyTo // exactly as `new MailMessage` starts them — [] and []. - $notification = new CloudReady( - Instance::factory()->create(), - 'admin', - Crypt::encryptString('geheim'), - ); + $notification = new CloudReady(Instance::factory()->create()); $message = $notification->toMail(new stdClass); diff --git a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php index d899641..404c755 100644 --- a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php +++ b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php @@ -53,9 +53,13 @@ it('provisions a paid order all the way to active (mocked)', function () { expect(RunResource::where('run_id', $run->id)->where('kind', $kind)->count())->toBe(1); } - // Secrets scrubbed from state; credentials delivered. + // Secrets scrubbed from run state; the admin password moved onto the + // instance instead — held there, not mailed, until the customer + // acknowledges it on the dashboard. expect($run->context('admin_password'))->toBeNull() - ->and($run->context('db_password'))->toBeNull(); + ->and($run->context('db_password'))->toBeNull() + ->and($instance->fresh()->admin_password)->not->toBeNull() + ->and($instance->fresh()->credentials_acknowledged_at)->toBeNull(); Notification::assertSentOnDemand(CloudReady::class); }); diff --git a/tests/Feature/Provisioning/CustomerStepsTest.php b/tests/Feature/Provisioning/CustomerStepsTest.php index d925782..7bb032b 100644 --- a/tests/Feature/Provisioning/CustomerStepsTest.php +++ b/tests/Feature/Provisioning/CustomerStepsTest.php @@ -358,11 +358,39 @@ it('completes: activates instance+order, seeds onboarding, delivers credentials' expect($instance->fresh()->status)->toBe('active') ->and($order->fresh()->status)->toBe('active') ->and($instance->fresh()->onboardingTasks()->count())->toBeGreaterThan(0) - ->and($run->fresh()->context('admin_password'))->toBeNull(); + ->and($run->fresh()->context('admin_password'))->toBeNull() + // The password moved onto the instance rather than the mail — held + // until the customer acknowledges it on the dashboard. + ->and($instance->fresh()->admin_password)->toBe('s3cret-pw') + ->and($instance->fresh()->credentials_acknowledged_at)->toBeNull(); - // Re-running must NOT deliver the credentials a second time. + // Re-running must NOT deliver the credentials a second time, and must not + // disturb the password already stored on the instance. app(CompleteProvisioning::class)->execute($run->fresh()); Notification::assertSentOnDemandTimes(CloudReady::class, 1); + expect($instance->fresh()->admin_password)->toBe('s3cret-pw'); +}); + +it('never puts the admin password in the CloudReady notification it sends', function () { + Notification::fake(); + fakeServices(); + ['run' => $run] = reservedRun([ + 'admin_password' => Crypt::encryptString('s3cret-pw'), + ]); + + app(CompleteProvisioning::class)->execute($run); + + Notification::assertSentOnDemand(CloudReady::class, function (CloudReady $notification) { + // Nothing about the password is even given to the notification — the + // constructor only takes the instance now — so there is nothing to + // render into the mail body or serialize into a queued job payload. + $mail = $notification->toMail(new stdClass); + $rendered = collect($mail->introLines)->implode(' '); + + expect($rendered)->not->toContain('s3cret-pw'); + + return true; + }); }); // 13b. Monitoring resilience — observability must not block delivery.