diff --git a/app/Livewire/Users.php b/app/Livewire/Users.php index 62ad910..50f3371 100644 --- a/app/Livewire/Users.php +++ b/app/Livewire/Users.php @@ -61,16 +61,23 @@ class Users extends Component * * Bei JEDEM Besuch versucht, nicht nur beim allerersten: wer das Panel * oeffnet, bevor die Bereitstellung das Admin-Konto angelegt hat, - * behielte sonst fuer immer einen owner-Sitz auf `none`. Der Aufruf ist - * folgenlos, solange es kein Konto gibt, und die Bedingung davor haelt - * ihn von jedem Sitz fern, der schon verknuepft ist. + * behielte sonst fuer immer einen owner-Sitz ohne Verknuepfung. Der + * Aufruf ist folgenlos, solange es kein Konto gibt. + * + * Die Bedingung haengt an `nc_username`, NICHT an `nc_state`: `nc_state` + * sagt nur, wie der letzte Versuch ausging. Stuende hier + * `=== STATE_NONE`, waere ein Sitz nach einem einzigen fehlgeschlagenen + * Auftrag fuer immer unverknuepft — und ein spaeterer Versuch legte in + * der Nextcloud ein ZWEITES Konto in der Gruppe `admin` an, neben dem + * echten. `nc_username` ist die Angabe, die genau einmal gesetzt wird + * und danach steht. * * Die Wanderung aus Aufgabe 5 tut dasselbe fuer den Bestand — sie fuehrt * ihre eigene, eingefrorene Fassung. Hier gilt die laufende. */ $owner = $customer->seats()->where('role', 'owner')->first(); - if ($owner !== null && $owner->nc_state === Seat::STATE_NONE) { + if ($owner !== null && blank($owner->nc_username)) { $owner->linkToInstanceAdmin(); } } @@ -151,6 +158,27 @@ class Users extends Component return; } + // Entzogen bleibt entzogen. Ohne diese Zeile waere „Einladen" an einer + // entzogenen Zeile der Weg zurueck in einen Zugang, den der Inhaber + // beendet hat — und er ginge an der Platzgrenze vorbei, die nur in + // addSeat() steht. Der richtige Weg ist ein NEUER Sitz. + if ($seat->status === 'revoked') { + $this->dispatch('notify', message: __('users.revoked_is_final')); + + return; + } + + // Den Inhaber laedt niemand ein. Sein Konto legt die Bereitstellung an + // (CreateCustomerAdmin); ein Auftrag von hier aus wuerde entweder auf + // einen bestehenden Benutzer treffen oder — wenn die Bereitstellung + // noch nicht so weit ist — spaeter ein ZWEITES Konto in der Gruppe + // `admin` anlegen, neben dem echten. + if ($seat->role === 'owner') { + $this->dispatch('notify', message: __('users.owner_not_invitable')); + + return; + } + if (($warten = $this->rateLimited($customer, $seat)) !== null) { $this->dispatch('notify', message: __('users.too_many_invites', ['minutes' => $warten])); @@ -246,20 +274,24 @@ class Users extends Component } /** - * Ein Sitz, der nie in der Nextcloud war, braucht keinen Auftrag — es - * gaebe dort nichts zu aendern, und der Fehlschlag stellte die Zeile - * danach auf „fehlgeschlagen", also auf eine Fehlermeldung fuer etwas, - * das nie ein Fehler war. + * Ein Sitz, der noch nie erfolgreich in der Nextcloud angekommen ist, + * bekommt keinen Auftrag — dort gaebe es nichts zu aendern. * - * Geprueft werden BEIDE Angaben: ein Sitz, der schon einen Anmeldenamen - * traegt, kann dort ein Konto haben, auch wenn `nc_state` es (noch) nicht - * sagt. Von den beiden moeglichen Irrtuemern ist ein ueberfluessiger - * Auftrag der harmlose — der andere hiesse, dass ein entzogener Zugang - * offen bleibt. + * Massgeblich ist `nc_synced_at`, nicht `nc_state`: `nc_state` sagt nur, + * wie der LETZTE Versuch ausging. Eine Einladung, die an einem nicht + * erreichbaren Gast gescheitert ist, hinterlaesst einen Sitz mit + * Anmeldenamen und `failed` — in der Nextcloud aber nichts. Ein + * `user:disable` darauf muss scheitern und liesse die Zeile dauerhaft rot + * stehen, mit einem Knopf, der nur weiter scheitert: eine Fehlermeldung + * fuer etwas, das nie ein Fehler war. + * + * Das traegt nur, weil der Auftrag ein angelegtes Konto SOFORT vermerkt, + * auch wenn die Gruppe danach scheitert — siehe SyncSeatToNextcloud. Sonst + * bliebe genau der gefaehrliche Fall offen: Konto da, Sperre nie geschickt. */ private function queueSync(Seat $seat, string $action): void { - if ($seat->nc_state === Seat::STATE_NONE && blank($seat->nc_username)) { + if ($seat->nc_synced_at === null) { return; } @@ -330,6 +362,18 @@ class Users extends Component return; } + // Entzogen bleibt entzogen. Der Umschalter unten kennt nur zwei + // Zustaende: an einem entzogenen Sitz machte er beim ersten Klick + // `suspended` und beim zweiten `active` — samt `user:enable`. Der + // Mensch, dessen Zugang der Inhaber beendet hat, koennte sich wieder + // anmelden, und er zaehlte wieder gegen die Platzgrenze, ohne dass sie + // hier jemals geprueft wird. + if ($seat->status === 'revoked') { + $this->dispatch('notify', message: __('users.revoked_is_final')); + + return; + } + $seat->update(['status' => $seat->status === 'suspended' ? 'active' : 'suspended']); // Der Klick allein sperrt niemanden aus: bis der Auftrag durch ist, diff --git a/app/Provisioning/Jobs/SyncSeatToNextcloud.php b/app/Provisioning/Jobs/SyncSeatToNextcloud.php index 96ef576..c718f4f 100644 --- a/app/Provisioning/Jobs/SyncSeatToNextcloud.php +++ b/app/Provisioning/Jobs/SyncSeatToNextcloud.php @@ -2,6 +2,7 @@ namespace App\Provisioning\Jobs; +use App\Models\Instance; use App\Models\Seat; use App\Services\Nextcloud\NextcloudUsers; use Illuminate\Bus\Queueable; @@ -72,10 +73,7 @@ class SyncSeatToNextcloud implements ShouldQueue } $ok = match ($this->action) { - // Einladen setzt in einem Zug auch die Gruppe: ein Benutzer, der - // angelegt ist und in keiner Rolle steckt, sieht in seiner neuen - // Cloud nichts und meldet sich am ersten Tag beim Inhaber. - 'invite' => $users->invite($instance, $seat) && $users->applyRole($instance, $seat), + 'invite' => $this->invite($users, $instance, $seat), 'role' => $users->applyRole($instance, $seat), 'disable' => $users->disable($instance, $seat), 'enable' => $users->enable($instance, $seat), @@ -85,6 +83,33 @@ class SyncSeatToNextcloud implements ShouldQueue $this->record($seat, $ok, $ok ? null : 'guest_failed'); } + /** + * Einladen setzt in einem Zug auch die Gruppe: ein Benutzer, der angelegt + * ist und in keiner Rolle steckt, sieht in seiner neuen Cloud nichts und + * meldet sich am ersten Tag beim Inhaber. + * + * Das Anlegen wird dabei SOFORT vermerkt, noch bevor die Gruppe gesetzt + * wird. `nc_synced_at` beantwortet die Frage „gibt es dieses Konto dort + * ueberhaupt?", und ab dem geglueckten `user:add` lautet die Antwort ja — + * auch wenn der naechste Befehl scheitert. Die Seite entscheidet genau + * daran, ob ein spaeteres Sperren etwas zu sperren haette; ohne diese Zeile + * bliebe der gefaehrlichste Fall offen: Konto angelegt, Rolle gescheitert, + * Zugang entzogen — und nie eine Sperre losgeschickt. + * + * `nc_state` bleibt davon unberuehrt: der Versuch ist erst gelungen, wenn + * auch die Rolle sitzt. + */ + private function invite(NextcloudUsers $users, Instance $instance, Seat $seat): bool + { + if (! $users->invite($instance, $seat)) { + return false; + } + + $seat->forceFill(['nc_synced_at' => now()])->save(); + + return $users->applyRole($instance, $seat); + } + /** * `nc_synced_at` bleibt beim Fehlschlag stehen, statt geleert zu werden: * wann dieser Sitz zuletzt WIRKLICH stimmte, ist genau die Angabe, die diff --git a/lang/de/users.php b/lang/de/users.php index 8834822..284c069 100644 --- a/lang/de/users.php +++ b/lang/de/users.php @@ -78,6 +78,13 @@ return [ 'error_guest_failed' => 'Die Cloud hat die Änderung nicht angenommen.', 'error_unexpected' => 'Unerwarteter Fehler. Bitte noch einmal versuchen.', + // Entzogen ist endgültig. Vorher löschte revoke() die Zeile, und die Frage + // stellte sich nie; heute bleibt sie stehen, und ohne diese beiden Sätze + // sähe sie aus wie ein Zugang, den man wieder aufmachen kann. + 'revoked_final' => 'Endgültig entzogen', + 'revoked_is_final' => 'Dieser Zugang wurde entzogen und kann nicht wieder geöffnet werden. Legen Sie bei Bedarf einen neuen an.', + 'owner_not_invitable' => 'Der Zugang des Inhabers wird beim Aufbau der Cloud angelegt und kann hier nicht eingeladen werden.', + 'added' => 'Benutzer angelegt. Die Einladung verschicken Sie mit „Einladen".', 'invite_sent' => 'Einladung verschickt. Der Mitarbeiter bekommt einen Link, an dem er sein Passwort selbst setzt.', 'too_many_invites' => 'Zu viele Einladungen — in :minutes Minuten wieder möglich.', diff --git a/lang/en/users.php b/lang/en/users.php index 61cff87..547c832 100644 --- a/lang/en/users.php +++ b/lang/en/users.php @@ -77,6 +77,13 @@ return [ 'error_guest_failed' => 'The cloud did not accept the change.', 'error_unexpected' => 'Unexpected error. Please try again.', + // Revoked is final. revoke() used to delete the row and the question never + // came up; today it stays, and without these two sentences it would look + // like access somebody can open again. + 'revoked_final' => 'Revoked for good', + 'revoked_is_final' => 'This access was revoked and cannot be reopened. Add a new seat if you need one.', + 'owner_not_invitable' => 'The owner\'s account is created when the cloud is built and cannot be invited from here.', + 'added' => 'User added. Send the invitation with "Invite".', 'invite_sent' => 'Invitation sent. They get a link to set their own password.', 'too_many_invites' => 'Too many invitations — possible again in :minutes minutes.', diff --git a/resources/views/livewire/users.blade.php b/resources/views/livewire/users.blade.php index d243ede..2bd9efb 100644 --- a/resources/views/livewire/users.blade.php +++ b/resources/views/livewire/users.blade.php @@ -80,7 +80,16 @@ Inhaber wieder und wieder, weil nichts sichtbar geschieht. --}} - @if ($seat->nc_state === \App\Models\Seat::STATE_NONE) + @if ($seat->status === 'revoked') + {{-- Entzogen geht allem anderen vor und ist + endgueltig. Ein „wird eingerichtet …" + oder ein „Nochmal versuchen" an dieser + Zeile las sich wie ein Weg zurueck — + den es nicht gibt und nicht geben + soll. --}} + {{ __('users.status_revoked') }} +

{{ __('users.revoked_is_final') }}

+ @elseif ($seat->nc_state === \App\Models\Seat::STATE_NONE) {{ __('users.state_none') }} @elseif ($seat->nc_state === \App\Models\Seat::STATE_PENDING) {{ __('users.state_pending') }} @@ -101,6 +110,17 @@
+ @if ($seat->status === 'revoked') + {{-- Kein Knopf. Kein Wiedereinsetzen: der + richtige Weg zurueck ist ein NEUER + Sitz, und der geht durch die + Platzpruefung. Gesagt statt leer + gelassen, wie bei der Inhaber-Zeile — + eine leere Zelle liesse jemanden nach + einem Knopf suchen, der absichtlich + fehlt. --}} + {{ __('users.revoked_final') }} + @else {{-- Einladen ist der zweite Vorgang und hat deshalb seinen eigenen Knopf. Er steht nur an einer Zeile, die noch @@ -108,12 +128,14 @@ Auftrag unterwegs, gibt es nichts zu druecken, und bei einem Fehlschlag steht „Nochmal versuchen" schon in der - Zustandsspalte. --}} - @if ($seat->nc_state === \App\Models\Seat::STATE_NONE) + Zustandsspalte. Nie an der + Inhaber-Zeile: dieses Konto legt die + Bereitstellung an, nicht das Panel. --}} + @if ($seat->role !== 'owner' && $seat->nc_state === \App\Models\Seat::STATE_NONE) - @elseif ($seat->nc_state === \App\Models\Seat::STATE_SYNCED && $seat->status === 'invited') + @elseif ($seat->role !== 'owner' && $seat->nc_state === \App\Models\Seat::STATE_SYNCED && $seat->status === 'invited') @@ -156,6 +178,7 @@ @endif + @endif
diff --git a/tests/Feature/Seats/SyncSeatToNextcloudTest.php b/tests/Feature/Seats/SyncSeatToNextcloudTest.php index 635105c..cb7987c 100644 --- a/tests/Feature/Seats/SyncSeatToNextcloudTest.php +++ b/tests/Feature/Seats/SyncSeatToNextcloudTest.php @@ -169,7 +169,12 @@ it('entzieht, ohne die Zeile zu loeschen', function () { $customer = Customer::factory()->create(); $user = $customer->ensureUser(); Instance::factory()->for($customer)->create(['status' => 'active']); - $sitz = Seat::factory()->for($customer)->create(['role' => 'member', 'nc_username' => 'anna@firma.tld']); + // Ein Sitz, der wirklich in der Nextcloud steht: nc_synced_at ist die + // Angabe, an der die Seite entscheidet, ob es dort etwas zu sperren gibt. + $sitz = Seat::factory()->for($customer)->create([ + 'role' => 'member', 'nc_username' => 'anna@firma.tld', + 'nc_state' => Seat::STATE_SYNCED, 'nc_synced_at' => now(), + ]); Livewire::actingAs($user)->test(Users::class)->call('revoke', $sitz->uuid); @@ -198,13 +203,52 @@ it('schickt fuer einen Sitz, der nie in der Nextcloud war, keinen Auftrag', func ->and($sitz->fresh()->nc_state)->toBe(Seat::STATE_NONE); }); +it('sperrt nichts, was in der Nextcloud nie angekommen ist', function () { + // Einladung losgeschickt, Gast war unerreichbar: der Sitz traegt schon + // einen Anmeldenamen, in der Nextcloud existiert aber nichts. Ein + // `user:disable` darauf muesste scheitern und liesse die Zeile dauerhaft + // rot stehen — eine Fehlermeldung fuer etwas, das nie ein Fehler war. + Queue::fake(); + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + $sitz = Seat::factory()->for($customer)->create([ + 'role' => 'member', 'nc_username' => 'anna@firma.tld', + 'nc_state' => Seat::STATE_FAILED, 'nc_error' => 'no_instance', 'nc_synced_at' => null, + ]); + + Livewire::actingAs($user)->test(Users::class)->call('revoke', $sitz->uuid); + + Queue::assertNothingPushed(); + expect($sitz->fresh()->status)->toBe('revoked'); +}); + +it('vermerkt ein angelegtes Konto, auch wenn die Rolle danach scheitert', function () { + // Der gefaehrlichste Halbschritt: `user:add` ist durch, `group:adduser` + // nicht. Das Konto GIBT es ab jetzt — wuerde der Auftrag das verschweigen, + // liesse ein spaeteres Entziehen die Sperre weg, weil die Seite den Sitz + // fuer nie angekommen hielte. Der Zustand bleibt trotzdem 'failed'. + $pve = new FakeProxmoxClient; + $pve->guestScript('group:adduser', 1); + app()->instance(ProxmoxClient::class, $pve); + $customer = Customer::factory()->create(); + Instance::factory()->for($customer)->create(['status' => 'active', 'vmid' => 201, 'host_id' => Host::factory()]); + $sitz = Seat::factory()->for($customer)->create(['nc_username' => 'anna@firma.tld']); + + (new SyncSeatToNextcloud($sitz->uuid, 'invite'))->handle(app(NextcloudUsers::class)); + + expect($sitz->fresh()->nc_state)->toBe(Seat::STATE_FAILED) + ->and($sitz->fresh()->nc_synced_at)->not->toBeNull(); +}); + it('spiegelt eine Rollenaenderung in die Nextcloud', function () { Queue::fake(); $customer = Customer::factory()->create(); $user = $customer->ensureUser(); Instance::factory()->for($customer)->create(['status' => 'active']); $sitz = Seat::factory()->for($customer)->create([ - 'role' => 'member', 'nc_username' => 'anna@firma.tld', 'nc_state' => Seat::STATE_SYNCED, + 'role' => 'member', 'nc_username' => 'anna@firma.tld', + 'nc_state' => Seat::STATE_SYNCED, 'nc_synced_at' => now(), ]); Livewire::actingAs($user)->test(Users::class)->call('setRole', $sitz->uuid, 'readonly'); @@ -232,6 +276,117 @@ it('gibt dem Inhaber nach einem Fehlschlag einen zweiten Versuch', function () { ->and($sitz->fresh()->nc_error)->toBeNull(); }); +it('macht einen entzogenen Sitz nicht ueber das Sperren wieder aktiv', function () { + // Der Umschalter in suspend() kannte nur zwei Zustaende: an einer + // entzogenen Zeile machte er beim ersten Klick 'suspended' und beim + // zweiten 'active', samt `user:enable`. Der Mensch, dessen Zugang der + // Inhaber beendet hat, koennte sich wieder anmelden — und er zaehlte + // wieder gegen die Platzgrenze, die nur in addSeat() geprueft wird. + Queue::fake(); + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + $sitz = Seat::factory()->for($customer)->create([ + 'role' => 'member', 'status' => 'revoked', 'nc_username' => 'anna@firma.tld', + 'nc_state' => Seat::STATE_SYNCED, 'nc_synced_at' => now(), + ]); + + $seite = Livewire::actingAs($user)->test(Users::class); + $seite->call('suspend', $sitz->uuid); + $seite->call('suspend', $sitz->uuid); + + expect($sitz->fresh()->status)->toBe('revoked'); + Queue::assertNothingPushed(); +}); + +it('laedt einen entzogenen Sitz nicht erneut ein', function () { + // Derselbe Weg zurueck, nur ueber die andere Tuer — und ebenfalls an der + // Platzgrenze vorbei. Der richtige Weg ist ein NEUER Sitz. + Queue::fake(); + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + $sitz = Seat::factory()->for($customer)->create(['role' => 'member', 'status' => 'revoked']); + + Livewire::actingAs($user)->test(Users::class)->call('sendInvite', $sitz->uuid); + + Queue::assertNothingPushed(); + expect($sitz->fresh()->nc_state)->toBe(Seat::STATE_NONE) + ->and($sitz->fresh()->nc_username)->toBeNull() + ->and($sitz->fresh()->status)->toBe('revoked'); +}); + +it('zeigt an einer entzogenen Zeile keinen Handlungsknopf mehr', function () { + // Ein Knopf, den der Server ohnehin abweist, ist eine Einladung zum + // Draufdruecken. Die Zeile sagt stattdessen, dass sie endgueltig ist. + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + $sitz = Seat::factory()->for($customer)->create(['role' => 'member', 'status' => 'revoked']); + + Livewire::actingAs($user)->test(Users::class) + ->assertSee(__('users.revoked_final')) + ->assertDontSee("sendInvite('{$sitz->uuid}')", escape: false) + ->assertDontSee("suspend('{$sitz->uuid}')", escape: false) + ->assertDontSee("retry('{$sitz->uuid}')", escape: false) + ->assertDontSee('confirm-revoke-seat', escape: false) + ->assertDontSee('edit-seat', escape: false); +}); + +it('sagt an einer entzogenen Zeile „entzogen", nicht „noch nicht eingeladen"', function () { + // Die Zustandsspalte muss „entzogen" VOR jeden nc_state stellen. Dieser + // Sitz steht auf `none` — ohne den Vorrang schriebe die Spalte „angelegt — + // noch nicht eingeladen" an einen Zugang, den der Inhaber beendet hat, und + // das laese sich wie ein Weg zurueck. + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + Seat::factory()->for($customer)->create(['role' => 'member', 'status' => 'revoked']); + + Livewire::actingAs($user)->test(Users::class) + ->assertSee(__('users.status_revoked')) + ->assertSee(__('users.revoked_is_final')) + ->assertDontSee(__('users.state_none')); +}); + +it('laedt den Inhaber nicht zu seiner eigenen Cloud ein', function () { + // Dieses Konto legt die Bereitstellung an (CreateCustomerAdmin). Ein + // Auftrag von hier aus traefe entweder auf einen bestehenden Benutzer — + // oder legte, solange die Bereitstellung noch nicht so weit ist, ein + // ZWEITES Konto in der Gruppe `admin` an, neben dem echten. + Queue::fake(); + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + Instance::factory()->for($customer)->create(['status' => 'active']); + $sitz = Seat::factory()->for($customer)->owner()->create(); + + Livewire::actingAs($user)->test(Users::class)->call('sendInvite', $sitz->uuid); + + Queue::assertNothingPushed(); + expect($sitz->fresh()->nc_username)->toBeNull() + ->and($sitz->fresh()->nc_state)->toBe(Seat::STATE_NONE); +}); + +it('verknuepft den Inhaber-Sitz beim Besuch, auch wenn die Instanz spaeter dazukommt', function () { + // Wer /users oeffnet, bevor die Bereitstellung durch ist, hatte danach + // einen owner-Sitz ohne Verknuepfung. Haenge die Bedingung an nc_state, + // reicht EIN fehlgeschlagener Auftrag, damit sie nie wieder greift — der + // Sitz bliebe fuer immer unverknuepft. Deshalb steht hier ein Sitz auf + // 'failed' und wird trotzdem geheilt. + $customer = Customer::factory()->create(); + $user = $customer->ensureUser(); + $sitz = Seat::factory()->for($customer)->owner()->create([ + 'nc_state' => Seat::STATE_FAILED, 'nc_error' => 'no_instance', + ]); + + Instance::factory()->for($customer)->create(['status' => 'active', 'nc_admin_ref' => 'admin']); + + Livewire::actingAs($user)->test(Users::class)->assertOk(); + + expect($sitz->fresh()->nc_username)->toBe('admin') + ->and($sitz->fresh()->nc_state)->toBe(Seat::STATE_SYNCED); +}); + it('nennt in keiner Datei unter app/ das Loeschen eines Benutzers', function () { // Testerzwungene Regel: kein Nextcloud-Benutzer wird je geloescht, und // keine Datei. Wer das aendern will, muss diese Pruefung anfassen und