diff --git a/database/migrations/2026_08_03_190000_ein_versandkonto_fuer_die_kundeninstanzen.php b/database/migrations/2026_08_03_190000_ein_versandkonto_fuer_die_kundeninstanzen.php new file mode 100644 index 0000000..526f78f --- /dev/null +++ b/database/migrations/2026_08_03_190000_ein_versandkonto_fuer_die_kundeninstanzen.php @@ -0,0 +1,96 @@ + self::KEY]); + + // Ein bestehender Datensatz wird NICHT überschrieben. Läuft die + // Wanderung ein zweites Mal — ein Wiederholungslauf nach einem + // Abbruch, ein `migrate` auf einem Server, der schon eines hatte — + // hätte der Betreiber sonst Adresse, Anmeldung und Passwort neu + // einzutragen, und bis er es merkt, verschickt keine Kundencloud + // mehr eine Einladung. Der stillste denkbare Ausfall. + if ($box->exists) { + return; + } + + $box->fill([ + 'address' => 'noreply@clupilot.cloud', + 'display_name' => 'CluPilot', + // Null heisst „Anmeldename gleich Adresse" (siehe + // Mailbox::smtpUsername()). Die Adresse selbst steht schon + // oben; sie hier zu wiederholen wäre eine Kopie, die nur + // einmal auseinanderlaufen muss, um zum Fehler zu werden. + 'username' => null, + 'password' => null, + // Auf diese Adresse antwortet niemand. Ein „noreply", dem man + // schreiben kann, ist eine Lüge im Absender. + 'no_reply' => true, + // Der Kern dieser Wanderung: die Zeile ist da, der Versand + // nicht. Erst wer Adresse und Passwort einträgt und dieses + // Häkchen setzt, hat den Mailversand der Kundeninstanzen + // wirklich in Betrieb genommen. + 'active' => false, + // Ein Relay ohne Anmeldung ist erlaubt (siehe Mailbox:: + // isConfigured()) — aber es ist die Ausnahme, und ein + // vorbelegtes „braucht kein Passwort" läse sich wie eine + // Aussage über einen Server, den hier noch niemand kennt. + 'authenticates' => true, + ])->save(); + }); + } + + public function down(): void + { + DB::table('mailboxes')->where('key', self::KEY)->delete(); + } +}; diff --git a/tests/Feature/Mail/GuestMailConfigTest.php b/tests/Feature/Mail/GuestMailConfigTest.php index 44ece29..963a498 100644 --- a/tests/Feature/Mail/GuestMailConfigTest.php +++ b/tests/Feature/Mail/GuestMailConfigTest.php @@ -5,14 +5,31 @@ use App\Models\Mailbox; use App\Services\Mail\GuestMailConfig; use App\Support\Settings; +/** + * Das gemeinsame Versandkonto ausfüllen — so, wie der Betreiber es in der + * Konsole täte. + * + * Die ZEILE legt eine eigene Wanderung an, inaktiv und ohne Zugangsdaten + * (siehe InstanceRelaySeedMigrationTest). Ein `Mailbox::factory()->create()` + * mit demselben Schlüssel liefe deshalb in die Eindeutigkeit der Spalte — + * und das ist auch der ehrlichere Ablauf: ausgefüllt wird hier, angelegt + * wurde dort. + */ +function versandkonto(array $werte): Mailbox +{ + $box = Mailbox::findByKey(GuestMailConfig::RELAY_KEY); + $box->update($werte); + + return $box; +} + function versandbereit(): void { Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); Settings::set('mail.encryption', 'tls'); - Mailbox::factory()->create([ - 'key' => 'instance-relay', + versandkonto([ 'address' => 'noreply@clupilot.cloud', 'username' => 'noreply@clupilot.cloud', 'password' => 'geheim', @@ -43,17 +60,16 @@ it('baut die Werte aus Server UND Postfach zusammen', function () { ->and($config->password())->toBe('geheim'); }); -it('schreibt mail_smtpauth false fuer ein Postfach ohne Anmeldung', function () { - // Ein Relay ohne Anmeldung ist bei Mailbox::isConfigured() ausdruecklich - // erlaubt und verlangt kein Passwort. Ein fest verdrahtetes 'true' wuerde +it('schreibt mail_smtpauth false für ein Postfach ohne Anmeldung', function () { + // Ein Relay ohne Anmeldung ist bei Mailbox::isConfigured() ausdrücklich + // erlaubt und verlangt kein Passwort. Ein fest verdrahtetes 'true' würde // Nextcloud trotzdem zur Anmeldung mit leerem Passwort zwingen und jeden // Versand still scheitern lassen — genau das darf hier nicht passieren. Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); Settings::set('mail.encryption', 'tls'); - Mailbox::factory()->create([ - 'key' => 'instance-relay', + versandkonto([ 'address' => 'noreply@clupilot.cloud', 'password' => null, 'active' => true, @@ -66,22 +82,22 @@ it('schreibt mail_smtpauth false fuer ein Postfach ohne Anmeldung', function () ->and($config->values())->toMatchArray(['mail_smtpauth' => 'false']); }); -it('traegt das Passwort NICHT unter values()', function () { +it('trägt das Passwort NICHT unter values()', function () { // values() wandert in Befehle. Das Passwort geht einen eigenen Weg, damit - // niemand es versehentlich mit den uebrigen Werten mitschleift. + // niemand es versehentlich mit den übrigen Werten mitschleift. versandbereit(); $values = GuestMailConfig::for(Instance::factory()->create())->values(); expect($values)->not->toHaveKey('mail_smtppassword') - // Der Schluessel allein beweist nichts: ein Leck unter einem anderen - // Namen faende der Test oben nicht. Deshalb zusaetzlich nach dem WERT - // suchen, egal unter welchem Schluessel er sich versteckt. + // Der Schlüssel allein beweist nichts: ein Leck unter einem anderen + // Namen fände der Test oben nicht. Deshalb zusätzlich nach dem WERT + // suchen, egal unter welchem Schlüssel er sich versteckt. ->and($values)->not->toContain('geheim'); }); it('sagt ohne Mailserver, dass nichts geschrieben werden darf', function () { - Mailbox::factory()->create(['key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', 'password' => 'geheim', 'active' => true]); + versandkonto(['address' => 'noreply@clupilot.cloud', 'password' => 'geheim', 'active' => true]); Settings::set('mail.host', ''); $config = GuestMailConfig::for(Instance::factory()->create()); @@ -91,6 +107,9 @@ it('sagt ohne Mailserver, dass nichts geschrieben werden darf', function () { }); it('sagt ohne Absenderpostfach, dass nichts geschrieben werden darf', function () { + // Die Zeile GIBT es seit der eigenen Wanderung — inaktiv und ohne + // Zugangsdaten. Genau das muss hier immer noch „kein Postfach" heissen: + // ein Datensatz allein ist kein eingerichteter Versand. Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); @@ -103,7 +122,7 @@ it('sagt ohne Absenderpostfach, dass nichts geschrieben werden darf', function ( it('weist ein Postfach ohne Zugangsdaten ab', function () { Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); - Mailbox::factory()->create(['key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', 'password' => null, 'active' => true, 'authenticates' => true]); + versandkonto(['address' => 'noreply@clupilot.cloud', 'password' => null, 'active' => true, 'authenticates' => true]); expect(GuestMailConfig::for(Instance::factory()->create())->problem())->toBe('no_mailbox'); }); diff --git a/tests/Feature/Mail/InstanceRelaySeedMigrationTest.php b/tests/Feature/Mail/InstanceRelaySeedMigrationTest.php new file mode 100644 index 0000000..91ed941 --- /dev/null +++ b/tests/Feature/Mail/InstanceRelaySeedMigrationTest.php @@ -0,0 +1,96 @@ +not->toBeNull() + ->and($box->address)->toBe('noreply@clupilot.cloud'); +}); + +it('legt es inaktiv und ohne Zugangsdaten an', function () { + // Ein Postfach, das ohne Zutun des Betreibers als einsatzbereit dastünde, + // wäre das nächste stille Versprechen: ConfigureInstanceMail schriebe + // dann Werte in die Kunden-Nextcloud, die niemand je eingetragen hat. + $box = Mailbox::findByKey(GuestMailConfig::RELAY_KEY); + + expect($box->active)->toBeFalse() + ->and($box->getRawOriginal('password'))->toBeNull() + ->and($box->isConfigured())->toBeFalse(); +}); + +it('hält GuestMailConfig davon ab, halb eingetragene Werte zu liefern', function () { + // Die Probe aufs Exempel: solange niemand das Postfach ausgefüllt hat, + // muss `no_mailbox` herauskommen — auch jetzt, wo es die Zeile GIBT. + Settings::set('mail.host', 'mail.clupilot.cloud'); + Settings::set('mail.port', 587); + + $config = GuestMailConfig::for(Instance::factory()->create()); + + expect($config->available())->toBeFalse() + ->and($config->problem())->toBe('no_mailbox'); +}); + +it('ist zweimal zu laufen unbedenklich', function () { + // `firstOrNew`, nicht `new Mailbox`: eine Wanderung, die auf halbem Weg + // scheitert, steht nicht in der migrations-Tabelle — der zweite Lauf muss + // die Zeile aktualisieren, nicht an der Eindeutigkeit des Schlüssels + // abprallen. + $threw = null; + try { + loadInstanceRelaySeedMigration()->up(); + } catch (Throwable $e) { + $threw = $e; + } + + expect($threw)->toBeNull() + ->and(Mailbox::query()->where('key', GuestMailConfig::RELAY_KEY)->count())->toBe(1); +}); + +it('lässt ein bereits ausgefülltes Postfach beim zweiten Lauf in Ruhe', function () { + // Der Fall, der wirklich weh täte: der Betreiber hat Adresse und Passwort + // eingetragen und das Konto scharfgeschaltet, dann läuft die Wanderung + // aus irgendeinem Grund erneut. Sie darf ihm den Versand nicht wieder + // abschalten. + Mailbox::findByKey(GuestMailConfig::RELAY_KEY)->update([ + 'address' => 'versand@kunde.example', + 'password' => 'geheim', + 'active' => true, + ]); + + loadInstanceRelaySeedMigration()->up(); + + $box = Mailbox::findByKey(GuestMailConfig::RELAY_KEY); + + expect($box->address)->toBe('versand@kunde.example') + ->and($box->active)->toBeTrue() + ->and($box->password)->toBe('geheim'); +}); + +it('nimmt beim Zurücknehmen nur das eigene Postfach mit', function () { + loadInstanceRelaySeedMigration()->down(); + + expect(Mailbox::findByKey(GuestMailConfig::RELAY_KEY))->toBeNull() + // Die fünf Postfächer der früheren Wanderung gehören ihr, nicht + // dieser hier. + ->and(Mailbox::findByKey('no-reply'))->not->toBeNull(); +}); diff --git a/tests/Feature/Mail/MailboxSeedMigrationTest.php b/tests/Feature/Mail/MailboxSeedMigrationTest.php index fb0912f..f150b1d 100644 --- a/tests/Feature/Mail/MailboxSeedMigrationTest.php +++ b/tests/Feature/Mail/MailboxSeedMigrationTest.php @@ -623,8 +623,12 @@ it('is safe to run twice — a migrations-table drift must not crash on the uniq } expect($threw)->toBeNull(); + // 'instance-relay' comes from a migration of its own (the shared sending + // account the CUSTOMER instances use — see InstanceRelaySeedMigrationTest), + // and is listed here because nothing cleared the real baseline: this + // migration must leave it exactly where it found it. expect(Mailbox::query()->pluck('key')->sort()->values()->all()) - ->toBe(['billing', 'info', 'no-reply', 'office', 'support']); + ->toBe(['billing', 'info', 'instance-relay', 'no-reply', 'office', 'support']); }); it('un-seeds on rollback, and does not leave the settings cache holding what it just deleted', function () { @@ -636,7 +640,11 @@ it('un-seeds on rollback, and does not leave the settings cache holding what it loadMailboxSeedMigration()->down(); - expect(Mailbox::query()->count())->toBe(0) + // Its own five rows, and only those: 'instance-relay' belongs to another + // migration and must survive this one's rollback — tearing down a + // neighbour's row would leave the customer instances' sending account + // gone with no migration recording it. + expect(Mailbox::query()->pluck('key')->all())->toBe(['instance-relay']) ->and(Settings::get('mail.purpose.system'))->toBeNull() ->and(Settings::get('mail.host'))->toBeNull(); }); diff --git a/tests/Feature/Mail/MailboxTakeoverTest.php b/tests/Feature/Mail/MailboxTakeoverTest.php index e5381c7..96fe50c 100644 --- a/tests/Feature/Mail/MailboxTakeoverTest.php +++ b/tests/Feature/Mail/MailboxTakeoverTest.php @@ -6,8 +6,11 @@ use App\Services\Secrets\SecretVault; use App\Support\Settings; it('seeds the five mailboxes so the page shows what is expected', function () { + // `instance-relay` steht daneben, aus einer eigenen Wanderung: das + // gemeinsame Versandkonto der KUNDENINSTANZEN, nicht eines der fuenf + // Absender des Betreibers. Siehe InstanceRelaySeedMigrationTest. expect(Mailbox::query()->pluck('key')->sort()->values()->all()) - ->toBe(['billing', 'info', 'no-reply', 'office', 'support']); + ->toBe(['billing', 'info', 'instance-relay', 'no-reply', 'office', 'support']); }); it('marks no-reply as unanswerable and the rest as answerable', function () { diff --git a/tests/Feature/Provisioning/ConfigureInstanceMailTest.php b/tests/Feature/Provisioning/ConfigureInstanceMailTest.php index cf2f9a4..271f424 100644 --- a/tests/Feature/Provisioning/ConfigureInstanceMailTest.php +++ b/tests/Feature/Provisioning/ConfigureInstanceMailTest.php @@ -19,14 +19,14 @@ function laufMitInstanz(Instance $instance): ProvisioningRun } /** - * Eine Instanz, die tatsaechlich auf einem Host steht. + * Eine Instanz, die tatsächlich auf einem Host steht. * * Der Schritt ruft ProxmoxClient::forHost($instance->host) auf — genau wie * ApplyStorageQuota, dessen Testaufbau (ApplyStorageQuotasTest::unquotedInstance()) * aus demselben Grund einen Host anlegt. Eine Instanz ohne host_id gibt es in - * diesem Bestand fuer eine Maschine, die tatsaechlich Gastbefehle bekommt, + * diesem Bestand für eine Maschine, die tatsächlich Gastbefehle bekommt, * nicht — FakeProxmoxClient::forHost() verlangt ein echtes Host-Objekt, weil - * der reale Client genauso wenig ohne einen Host wuesste, wohin er soll. + * der reale Client genauso wenig ohne einen Host wüsste, wohin er soll. */ function instanzAufHost(): Instance { @@ -38,14 +38,18 @@ function versandbereitFuerSchritt(): void Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); Settings::set('mail.encryption', 'tls'); - Mailbox::factory()->create([ - 'key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', + // Die Zeile legt eine eigene Wanderung an — inaktiv und ohne + // Zugangsdaten. Ausgefüllt wird sie hier, so wie der Betreiber es in der + // Konsole täte; ein factory()->create() mit demselben Schlüssel liefe in + // die Eindeutigkeit der Spalte. + Mailbox::findByKey('instance-relay')->update([ + 'address' => 'noreply@clupilot.cloud', 'username' => 'noreply@clupilot.cloud', 'password' => 'geheim', 'active' => true, 'authenticates' => true, ]); } -it('traegt jeden Wert einzeln in den Gast', function () { +it('trägt jeden Wert einzeln in den Gast', function () { versandbereitFuerSchritt(); $pve = new FakeProxmoxClient; app()->instance(ProxmoxClient::class, $pve); @@ -56,7 +60,7 @@ it('traegt jeden Wert einzeln in den Gast', function () { $befehle = implode("\n", $pve->guestCommands); // Alle acht Werte aus GuestMailConfig::values(), nicht nur zwei — ein - // vergessener Schluessel soll hier auffallen, nicht erst beim Kunden. + // vergessener Schlüssel soll hier auffallen, nicht erst beim Kunden. expect($befehle)->toContain('config:system:set mail_smtpmode --value='.escapeshellarg('smtp')) ->and($befehle)->toContain('config:system:set mail_smtphost --value='.escapeshellarg('mail.clupilot.cloud')) ->and($befehle)->toContain('config:system:set mail_smtpport --value='.escapeshellarg('587')) @@ -68,15 +72,15 @@ it('traegt jeden Wert einzeln in den Gast', function () { }); it('maskiert das Passwort, sodass es keinen zweiten Befehl starten kann', function () { - // Verstecken laesst sich der Wert auf dieser Maschine nicht (siehe - // Kopfkommentar des Schrittes). Was sehr wohl gilt und geprueft gehoert: - // er darf aus seiner Klammerung nicht ausbrechen. Der Befehl laeuft als - // root auf einer Kundenmaschine — ein Semikolon im Passwort waere dort + // Verstecken lässt sich der Wert auf dieser Maschine nicht (siehe + // Kopfkommentar des Schrittes). Was sehr wohl gilt und geprüft gehört: + // er darf aus seiner Klammerung nicht ausbrechen. Der Befehl läuft als + // root auf einer Kundenmaschine — ein Semikolon im Passwort wäre dort // ein zweiter Befehl. Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); - Mailbox::factory()->create([ - 'key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', + Mailbox::findByKey('instance-relay')->update([ + 'address' => 'noreply@clupilot.cloud', 'username' => 'noreply@clupilot.cloud', 'password' => "boes'; touch /tmp/PWNED; echo '", 'active' => true, 'authenticates' => true, @@ -91,7 +95,7 @@ it('maskiert das Passwort, sodass es keinen zweiten Befehl starten kann', functi ->first(fn ($b) => str_contains($b, 'mail_smtppassword')); // Der ganze Wert steht in EINEM maskierten Argument: das Semikolon darf - // nicht ausserhalb der Anfuehrungszeichen stehen. + // nicht ausserhalb der Anführungszeichen stehen. expect($passwortbefehl)->toContain(escapeshellarg("boes'; touch /tmp/PWNED; echo '")); }); @@ -101,7 +105,7 @@ it('schreibt GAR NICHTS, wenn der Mailserver fehlt', function () { // Cloud auch dann, wenn der Mailversand noch nicht steht — derselbe // Grundsatz wie bei RegisterMonitoring. Der Lauf geht deshalb WEITER // (advance), nicht in einen Fehlschlag; nur in den Gast wird nichts - // geschrieben, und der Grund landet im Protokoll fuer den Betreiber. + // geschrieben, und der Grund landet im Protokoll für den Betreiber. Log::spy(); Settings::set('mail.host', ''); $pve = new FakeProxmoxClient; @@ -117,11 +121,11 @@ it('schreibt GAR NICHTS, wenn der Mailserver fehlt', function () { ); }); -it('haengt ein Lauf-Ereignis an, das der Betreiber in der Konsole sieht', function () { +it('hängt ein Lauf-Ereignis an, das der Betreiber in der Konsole sieht', function () { // Log::warning allein reicht hier nicht: bis der Mailserver steht, trifft - // dieser Zweig auf JEDE Bestellung zu, und die einzige Rueckmeldung darf + // dieser Zweig auf JEDE Bestellung zu, und die einzige Rückmeldung darf // nicht in einer Logdatei verschwinden, in die niemand schaut — - // RegisterMonitoring macht sein Ueberspringen genauso am Lauf sichtbar. + // RegisterMonitoring macht sein Überspringen genauso am Lauf sichtbar. Settings::set('mail.host', ''); $pve = new FakeProxmoxClient; app()->instance(ProxmoxClient::class, $pve); @@ -149,7 +153,7 @@ it('schreibt beim zweiten Lauf erneut, statt sich mit einem Merker zu sperren', // config:system:set ist von sich aus wiederholbar — derselbe Wert zweimal // geschrieben ist derselbe Wert. Der Schritt darf deshalb ohne Merker - // erneut laufen; das ist bei einer Nachruestung ueber den Bestand der + // erneut laufen; das ist bei einer Nachrüstung über den Bestand der // Normalfall, nicht die Ausnahme. expect(count($pve->guestCommands))->toBe($ersteRunde * 2); }); diff --git a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php index e32aa83..4e58b66 100644 --- a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php +++ b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php @@ -24,14 +24,17 @@ it('provisions a paid order all the way to active (mocked)', function () { // ConfigureInstanceMail steht jetzt in dieser Pipeline. Mailversand ist // eingerichtet, damit DIESER Lauf den Schreib-Pfad beweist (siehe - // Zusicherung weiter unten, dass der Gast die Werte tatsaechlich bekam). + // Zusicherung weiter unten, dass der Gast die Werte tatsächlich bekam). // Der Fall OHNE Einrichtung — ein bezahlter Kunde bekommt seine Cloud // trotzdem — hat einen eigenen Test weiter unten, weil er das genaue // Gegenteil dieser Fixture braucht. Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); - Mailbox::factory()->create([ - 'key' => 'instance-relay', 'address' => 'noreply@clupilot.cloud', + // Ausgefüllt, nicht angelegt: die Zeile bringt eine eigene Wanderung mit + // — inaktiv und ohne Zugangsdaten, damit sie in der Konsole erscheint und + // dort ausgefüllt werden kann. + Mailbox::findByKey('instance-relay')->update([ + 'address' => 'noreply@clupilot.cloud', 'username' => 'noreply@clupilot.cloud', 'password' => 'geheim', 'active' => true, 'authenticates' => true, ]); @@ -74,8 +77,8 @@ it('provisions a paid order all the way to active (mocked)', function () { ->and($instance->quota_applied_gb)->toBeGreaterThan(0) ->and($s['pve']->guestRan('config:app:set files default_quota'))->toBeTrue() // Mail war eingerichtet (siehe Fixture oben) — der Schreib-Pfad von - // ConfigureInstanceMail lief also tatsaechlich, nicht nur der - // Ueberspringen-Pfad, den der Test weiter unten prueft. + // ConfigureInstanceMail lief also tatsächlich, nicht nur der + // Überspringen-Pfad, den der Test weiter unten prüft. ->and($s['pve']->guestRan('config:system:set mail_smtphost'))->toBeTrue(); // Every external resource created exactly once. @@ -99,7 +102,7 @@ it('completes a paid order even when the mail server does not exist yet', functi // seine Cloud trotzdem: ConfigureInstanceMail ist eine Nebenfunktion in // einer PFLICHT-Pipeline und darf sie nicht aufhalten, genau wie // RegisterMonitoring die Bereitstellung nie an einer nicht erreichbaren - // Ueberwachung scheitern laesst. Bewusst KEINE mail.host/mail.port- oder + // Überwachung scheitern lässt. Bewusst KEINE mail.host/mail.port- oder // Mailbox-Fixture hier — das ist der ganze Punkt dieses Tests. Notification::fake(); Queue::fake(); @@ -133,7 +136,7 @@ it('completes a paid order even when the mail server does not exist yet', functi ->and($run->error)->toBeNull() ->and($order->fresh()->status)->toBe('active') ->and($instance->status)->toBe('active') - // Das Gegenstueck zur Zusicherung im ersten Test: ohne Einrichtung + // Das Gegenstück zur Zusicherung im ersten Test: ohne Einrichtung // ging kein einziger Mailbefehl an den Gast, weder die Werte noch das // Passwort. ->and($s['pve']->guestRan('config:system:set mail_smtphost'))->toBeFalse()