diff --git a/app/Livewire/Admin/Maintenance.php b/app/Livewire/Admin/Maintenance.php index afe3de8..8a9f27a 100644 --- a/app/Livewire/Admin/Maintenance.php +++ b/app/Livewire/Admin/Maintenance.php @@ -8,6 +8,7 @@ use App\Models\Host; use App\Models\MaintenanceNotification; use App\Models\MaintenanceWindow; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Mail; use Livewire\Attributes\Layout; use Livewire\Attributes\Validate; @@ -51,7 +52,15 @@ class Maintenance extends Component private function persist(string $state): ?MaintenanceWindow { - $data = $this->validate(); + $data = $this->validate([ + 'title' => 'required|string|max:255', + 'publicDescription' => 'nullable|string|max:2000', + 'internalNotes' => 'nullable|string|max:2000', + 'startsAt' => 'required|date', + 'endsAt' => 'required|date', + 'hostIds' => 'array', + 'hostIds.*' => 'integer|exists:hosts,id', + ]); $starts = Carbon::parse($data['startsAt']); $ends = Carbon::parse($data['endsAt']); @@ -74,17 +83,23 @@ class Maintenance extends Component } } - $window = MaintenanceWindow::create([ - 'title' => $data['title'], - 'public_description' => $data['publicDescription'] ?: null, - 'internal_notes' => $data['internalNotes'] ?: null, - 'starts_at' => $starts, - 'ends_at' => $ends, - 'state' => $state, - 'created_by' => auth()->id(), - 'published_at' => $state === 'scheduled' ? now() : null, - ]); - $window->hosts()->sync(array_map('intval', $this->hostIds)); + // Create the window and attach hosts atomically so a bad id can't leave + // an orphaned scheduled window behind. + $window = DB::transaction(function () use ($data, $starts, $ends, $state) { + $window = MaintenanceWindow::create([ + 'title' => $data['title'], + 'public_description' => $data['publicDescription'] ?: null, + 'internal_notes' => $data['internalNotes'] ?: null, + 'starts_at' => $starts, + 'ends_at' => $ends, + 'state' => $state, + 'created_by' => auth()->id(), + 'published_at' => $state === 'scheduled' ? now() : null, + ]); + $window->hosts()->sync(array_map('intval', $data['hostIds'] ?? [])); + + return $window; + }); $this->reset('title', 'publicDescription', 'internalNotes', 'startsAt', 'endsAt', 'hostIds'); $this->dispatch('notify', message: __($state === 'scheduled' ? 'maintenance.published' : 'maintenance.draft_saved')); @@ -129,13 +144,19 @@ class Maintenance extends Component ['email' => $customer->email], ); if ($delivery->wasRecentlyCreated) { - // The ledger row (created here) is the idempotency marker so we - // queue at most once per customer. sent_at stays null until the - // mail is actually delivered — a MessageSent listener could stamp - // it later; we never claim delivery merely on dispatch. - Mail::to($customer->email) - ->locale($customer->locale ?: config('app.locale')) - ->queue(new MaintenanceAnnouncementMail($window, $customer)); + // The ledger row is the idempotency marker (queue at most once). + // If dispatch itself fails, drop the row so a re-publish retries; + // sent_at stays null until real delivery (a MessageSent listener + // could stamp it) — we never claim delivery merely on dispatch. + try { + Mail::to($customer->email) + ->locale($customer->locale ?: config('app.locale')) + ->queue(new MaintenanceAnnouncementMail($window, $customer)); + } catch (\Throwable $e) { + $delivery->delete(); + + throw $e; + } } } } diff --git a/app/Livewire/Admin/Settings.php b/app/Livewire/Admin/Settings.php index df0865b..7f9835e 100644 --- a/app/Livewire/Admin/Settings.php +++ b/app/Livewire/Admin/Settings.php @@ -112,6 +112,11 @@ class Settings extends Component if ($target->id === auth()->id()) { return 'self'; } + // Only existing operators may be re-roled — never escalate a customer + // (or any non-staff) user into the console via a tampered id. + if (! $target->isOperator() || Customer::query()->where('email', $target->email)->exists()) { + return 'not_staff'; + } // Granting or revoking Owner is Owner-only. if (($role === 'Owner' || $target->hasRole('Owner')) && ! auth()->user()->hasRole('Owner')) { return 'owner_only'; @@ -134,7 +139,7 @@ class Settings extends Component $result = DB::transaction(function () use ($id) { $target = User::query()->whereKey($id)->lockForUpdate()->first(); - if ($target === null) { + if ($target === null || ! $target->isOperator()) { return 'gone'; } if ($target->id === auth()->id()) { @@ -160,6 +165,7 @@ class Settings extends Component 'self' => __('admin_settings.not_self'), 'owner_only' => __('admin_settings.owner_only'), 'last_owner' => __('admin_settings.last_owner'), + 'not_staff' => __('admin_settings.not_staff'), default => null, }; if ($msg !== null) { diff --git a/lang/de/admin_settings.php b/lang/de/admin_settings.php index 4592d43..d93a0e0 100644 --- a/lang/de/admin_settings.php +++ b/lang/de/admin_settings.php @@ -34,5 +34,6 @@ return [ 'not_self' => 'Sie können Ihre eigene Rolle nicht ändern.', 'owner_only' => 'Nur ein Inhaber darf die Inhaber-Rolle vergeben oder entziehen.', 'last_owner' => 'Der letzte Inhaber kann nicht geändert oder entfernt werden.', + 'not_staff' => 'Nur bestehende Mitarbeiter können bearbeitet werden.', 'is_customer' => 'Diese E-Mail gehört bereits zu einem Kundenkonto.', ]; diff --git a/lang/en/admin_settings.php b/lang/en/admin_settings.php index 783ecde..c788f68 100644 --- a/lang/en/admin_settings.php +++ b/lang/en/admin_settings.php @@ -34,5 +34,6 @@ return [ 'not_self' => 'You cannot change your own role.', 'owner_only' => 'Only an Owner may grant or revoke the Owner role.', 'last_owner' => 'The last Owner cannot be changed or removed.', + 'not_staff' => 'Only existing staff can be edited.', 'is_customer' => 'This email already belongs to a customer account.', ]; diff --git a/tests/Feature/Admin/AdminSettingsTest.php b/tests/Feature/Admin/AdminSettingsTest.php index e3edbd5..dec24e7 100644 --- a/tests/Feature/Admin/AdminSettingsTest.php +++ b/tests/Feature/Admin/AdminSettingsTest.php @@ -40,6 +40,14 @@ it('will not create an operator from a customer email', function () { ->call('inviteStaff')->assertHasErrors(['staffEmail']); }); +it('will not escalate a non-staff user via a tampered id', function () { + $victim = User::factory()->create(); // a plain (customer) user, no role + + Livewire::actingAs(operator('Owner'))->test(Settings::class)->call('setStaffRole', $victim->id, 'Admin'); + + expect($victim->fresh()->isOperator())->toBeFalse(); +}); + it('protects the last owner and self-role', function () { $owner = operator('Owner'); $support = operator('Support');