diff --git a/app/Livewire/Users.php b/app/Livewire/Users.php index a45c7f0..1463497 100644 --- a/app/Livewire/Users.php +++ b/app/Livewire/Users.php @@ -343,6 +343,10 @@ class Users extends Component $action = match (true) { in_array($seat->status, ['revoked', 'suspended'], true) => 'disable', $seat->nc_synced_at === null => 'invite', + // Fünfte Aktion statt gemerkter letzter Auftrag: woran ein fälliges + // `enable` zu erkennen wäre, steht nicht am Sitz — ein Feld dafür + // wäre die nächste Behauptung des Portals über eine Cloud, in die + // es nicht sehen kann. Siehe den Absatz oben. default => 'restore', }; @@ -496,7 +500,7 @@ class Users extends Component } if ($seat->role === 'owner') { - $this->dispatch('notify', message: __('users.owner_locked')); + $this->dispatch('notify', message: __('users.owner_role_locked')); return; } @@ -528,7 +532,7 @@ class Users extends Component $seat = $customer->seats()->where('uuid', $uuid)->first(); if ($seat === null || $seat->role === 'owner') { - // The owner cannot lock themselves out of their own cloud. + // Der Inhaber sperrt sich nicht selbst aus seiner eigenen Cloud aus. $this->dispatch('notify', message: __('users.owner_locked')); return; @@ -571,12 +575,13 @@ class Users extends Component return; } - // Wortgleich zu suspend() und setRole(), und aus demselben Grund: der - // Inhaber sperrt sich nicht selbst aus seiner eigenen Cloud aus. Die - // frühere Wache zählte owner-Sitze — und diese Zählung liess sich - // über setRole() vorbereiten. Siehe den Kopf von setRole(). + // Aus demselben Grund wie suspend() und setRole(): der Inhaber sperrt + // sich nicht selbst aus seiner eigenen Cloud aus. Die frühere Wache + // zählte owner-Sitze — und diese Zählung liess sich über setRole() + // vorbereiten. Siehe den Kopf von setRole(). Eigene Meldung statt der + // wortgleichen von suspend(): „entfernen" ist nicht „sperren". if ($seat->role === 'owner') { - $this->dispatch('notify', message: __('users.owner_locked')); + $this->dispatch('notify', message: __('users.owner_revoke_locked')); return; } diff --git a/app/Provisioning/Jobs/SyncSeatToNextcloud.php b/app/Provisioning/Jobs/SyncSeatToNextcloud.php index 182fffc..2962226 100644 --- a/app/Provisioning/Jobs/SyncSeatToNextcloud.php +++ b/app/Provisioning/Jobs/SyncSeatToNextcloud.php @@ -72,6 +72,34 @@ class SyncSeatToNextcloud implements ShouldQueue return; } + /* + * Die Restnaht des K1-Fixes (queueSync()'s Ausnahme bei `pending`, + * Users.php): ein nachgeschicktes `disable` — dispatcht, WÄHREND ein + * `invite`-Auftrag für denselben Sitz noch in der Warteschlange + * steht — geht davon aus, dass DIESER `invite`-Auftrag das Konto bis + * dahin angelegt hat. + * + * Scheitert der `invite`-Auftrag stattdessen schon oben an + * `no_instance` (kein seltener Fall — siehe den retry()-Kommentar in + * Users.php: eine Instanz, die beim Absenden noch nicht aktiv war, + * ist ein dokumentierter Weg dorthin), entsteht in der Nextcloud gar + * nichts. Wird die Instanz genau im Fenster zwischen den beiden + * Läufen erreichbar, kommt das nachgeschickte `disable` trotzdem bis + * hierher und schickt `user:disable` gegen einen Benutzer, den + * niemand angelegt hat. + * + * Bewusst nicht gehärtet: occ scheitert daran folgenlos (Exitcode + * ungleich 0, in der Nextcloud ändert sich nichts), und + * `nc_synced_at` bleibt `null` — genau daran hängt der + * Wiederholen-Knopf im Portal (dieselbe Bedingung an zwei Stellen: + * Users::retry() und resources/views/livewire/users.blade.php). Er + * bleibt an dieser Zeile verborgen, weil eine entzogene oder + * gesperrte Zeile ohne `nc_synced_at` dort nichts zu wiederholen + * hat; der Weg zurück bleibt die Vordertür (dieselbe Adresse über + * das Anlegen-Formular). Ein eigener „wurde je angelegt"-Zwischenstand + * nur für dieses enge Zeitfenster wäre mehr Feld für einen Randfall, + * der sich am Ende von selbst schliesst. + */ $ok = match ($this->action) { 'invite' => $this->invite($users, $instance, $seat), 'role' => $users->applyRole($instance, $seat), diff --git a/app/Services/Nextcloud/NextcloudUsers.php b/app/Services/Nextcloud/NextcloudUsers.php index 98b7fa5..c577226 100644 --- a/app/Services/Nextcloud/NextcloudUsers.php +++ b/app/Services/Nextcloud/NextcloudUsers.php @@ -93,6 +93,21 @@ class NextcloudUsers // Nur fürs Entfernen. Ein gescheitertes `group:adduser` bleibt // ein Fehlschlag: wer in keiner Gruppe landet, sieht in seiner // neuen Cloud nichts. + // + // Genauer als es oben klingt: `&&` und `||` sind in der Shell + // linksassoziativ, und NextcloudOcc::command() setzt VOR den + // occ-Aufruf ein eigenes `cd … &&` (siehe dort). Das fertige + // Kommando ist damit EIN einziger Ausdruck von diesem `cd` bis + // zum `|| true` am Ende, nicht zwei getrennte. Das Netz fängt + // damit auch ein gescheitertes `cd` auf, nicht nur die fehlende + // Gruppe. + // + // Folgenlos: `group:adduser` und die Speicherplatzzeile laufen + // in derselben Runde OHNE eigenes Netz und ziehen den Lauf beim + // selben `cd`-Fehlschlag ohnehin auf Fehlschlag (run() verundet + // alle Exitcodes). Dieses Netz rettet also nichts, was den Lauf + // sonst überstünde — es beschreibt nur mehr, als sein Kommentar + // eben behauptet. foreach (array_unique(array_values(Seat::GROUPS)) as $gruppe) { if ($gruppe !== $ziel) { $befehle[] = 'group:removeuser '.escapeshellarg($gruppe).' ' diff --git a/app/Services/Proxmox/FakeProxmoxClient.php b/app/Services/Proxmox/FakeProxmoxClient.php index 826fede..7054676 100644 --- a/app/Services/Proxmox/FakeProxmoxClient.php +++ b/app/Services/Proxmox/FakeProxmoxClient.php @@ -4,6 +4,18 @@ namespace App\Services\Proxmox; use App\Models\Host; +/** + * Test double fuer den Proxmox-Client. Gastbefehle (`guestExec()`) werden + * nicht ausgefuehrt, sondern nur verskriptet und aufgezeichnet. + * + * `guestExec()` beachtet ein angehaengtes `|| true` und liefert dann immer + * Exitcode 0 — siehe die Begruendung dort. `App\Services\Ssh\FakeRemoteShell`, + * das Gegenstueck fuer SSH-Befehle, tut das ausdruecklich NICHT: es fuehrt + * keine echte Shell aus und kann `|| true` daher gar nicht abbilden. Die + * beiden Fakes behandeln dieselbe Shell-Redewendung also unterschiedlich, und + * wer nur den einen kennt, nimmt sein Verhalten sonst irrtuemlich vom anderen + * an — deshalb steht dieser Hinweis an beiden Stellen. + */ class FakeProxmoxClient implements ProxmoxClient { public ?Host $host = null; diff --git a/app/Services/Ssh/FakeRemoteShell.php b/app/Services/Ssh/FakeRemoteShell.php index 4c3b795..2ea5cb6 100644 --- a/app/Services/Ssh/FakeRemoteShell.php +++ b/app/Services/Ssh/FakeRemoteShell.php @@ -4,6 +4,14 @@ namespace App\Services\Ssh; /** * Test double: script command output by substring, record everything. + * + * Anders als `App\Services\Proxmox\FakeProxmoxClient::guestExec()` beachtet + * `run()` hier ein angehaengtes `|| true` NICHT: dieser Fake fuehrt keine + * echte Shell aus, sondern liefert nur das verskriptete oder das + * Standardergebnis zurueck — `|| true` ist eine Aussage der SHELL, und ohne + * Shell gibt es nichts, das sie treffen koennte (siehe auch der Kommentar in + * HostFirewallTest). Wer das Verhalten des anderen Fakes von hier aus + * vermutet, irrt: die beiden behandeln dieselbe Redewendung unterschiedlich. */ class FakeRemoteShell implements RemoteShell { diff --git a/lang/de/users.php b/lang/de/users.php index 473aa0f..5b45600 100644 --- a/lang/de/users.php +++ b/lang/de/users.php @@ -41,7 +41,12 @@ return [ 'reactivate' => 'Entsperren', 'suspended' => 'Zugang gesperrt.', 'reactivated' => 'Zugang wieder freigegeben.', - 'owner_locked' => 'Der Inhaber kann sich nicht selbst sperren.', + // Drei Handlungen, drei eigene Sätze: der Inhaber-Sitz ist der Zugang, + // mit dem der Kunde seine eigene Cloud verwaltet, und jeder Satz sagt, + // was HIER gerade nicht geht — nicht nur, dass etwas gesperrt ist. + 'owner_locked' => 'Der Inhaber kann sich nicht selbst sperren — der Inhaber-Sitz ist der Zugang, mit dem der Kunde seine eigene Cloud verwaltet.', + 'owner_role_locked' => 'Die Rolle des Inhabers kann nicht geändert werden — der Inhaber-Sitz ist der Zugang, mit dem der Kunde seine eigene Cloud verwaltet.', + 'owner_revoke_locked' => 'Der Zugang des Inhabers kann nicht entfernt werden — der Inhaber-Sitz ist der Zugang, mit dem der Kunde seine eigene Cloud verwaltet.', 'status_suspended' => 'Gesperrt', 'status_active' => 'Aktiv', 'status_invited' => 'Eingeladen', diff --git a/lang/en/users.php b/lang/en/users.php index 8dc5ee1..fbcb651 100644 --- a/lang/en/users.php +++ b/lang/en/users.php @@ -40,7 +40,12 @@ return [ 'reactivate' => 'Reactivate', 'suspended' => 'Access suspended.', 'reactivated' => 'Access restored.', - 'owner_locked' => 'The owner cannot lock themselves out.', + // Three actions, three separate sentences: the owner seat is the access + // the customer uses to manage their own cloud, and each sentence says + // what is blocked HERE — not just that something is locked. + 'owner_locked' => 'The owner cannot lock themselves out — the owner seat is the access the customer uses to manage their own cloud.', + 'owner_role_locked' => 'The owner\'s role cannot be changed — the owner seat is the access the customer uses to manage their own cloud.', + 'owner_revoke_locked' => 'The owner\'s access cannot be removed — the owner seat is the access the customer uses to manage their own cloud.', 'status_suspended' => 'Suspended', 'status_active' => 'Active', 'status_invited' => 'Invited', diff --git a/tests/Feature/SeatsTest.php b/tests/Feature/SeatsTest.php index 2765a66..9558607 100644 --- a/tests/Feature/SeatsTest.php +++ b/tests/Feature/SeatsTest.php @@ -64,7 +64,11 @@ it('will not remove or demote the last owner', function () { Livewire::actingAs($user)->test(Users::class); // creates owner $owner = $customer->seats()->where('role', 'owner')->first(); - Livewire::actingAs($user)->test(Users::class)->call('revoke', $owner->uuid); + // Drei Handlungen, drei eigene Meldungen (users.owner_locked trug bis vor + // kurzem alle drei) — je Knopf ein eigener Satz, der sagt, was HIER + // gerade nicht geht. + Livewire::actingAs($user)->test(Users::class)->call('revoke', $owner->uuid) + ->assertDispatched('notify', message: __('users.owner_revoke_locked')); // Auf den STATUS geprüft, nicht mehr nur auf die Zeile: seit revoke() // grundsätzlich nicht mehr löscht, bewiese eine noch vorhandene Zeile // gar nichts — sie bliebe auch dann stehen, wenn die Inhaber-Sperre @@ -72,8 +76,13 @@ it('will not remove or demote the last owner', function () { expect($customer->seats()->whereKey($owner->id)->exists())->toBeTrue() ->and($owner->fresh()->status)->toBe('active'); - Livewire::actingAs($user)->test(Users::class)->call('setRole', $owner->uuid, 'member'); + Livewire::actingAs($user)->test(Users::class)->call('setRole', $owner->uuid, 'member') + ->assertDispatched('notify', message: __('users.owner_role_locked')); expect($owner->fresh()->role)->toBe('owner'); + + Livewire::actingAs($user)->test(Users::class)->call('suspend', $owner->uuid) + ->assertDispatched('notify', message: __('users.owner_locked')); + expect($owner->fresh()->status)->toBe('active'); }); it('revokes a non-owner seat without deleting the row', function () {