fix(admin): block role escalation of non-staff; validate maintenance host ids atomically; rollback ledger on dispatch failure

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 16:01:45 +02:00
parent e5c74c6bdd
commit bc2e95ba18
5 changed files with 57 additions and 20 deletions

View File

@ -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;
}
}
}
}

View File

@ -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) {

View File

@ -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.',
];

View File

@ -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.',
];

View File

@ -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');