From 5f28bfedd4d74c2eb54757c9bfab7f80339a8488 Mon Sep 17 00:00:00 2001 From: HolgerHatGarKeineNode <123783602+HolgerHatGarKeineNode@users.noreply.github.com> Date: Tue, 2 Jun 2026 19:23:51 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=80=20Enhance=20authorization=20and=20?= =?UTF-8?q?exception=20handling=20across=20Livewire=20components=20and=20S?= =?UTF-8?q?ecurityMonitor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **SecurityMonitor:** Added logic to record and prevent logging of locked-property exceptions, while ensuring non-security exceptions are properly forwarded. - **Livewire `Members/Admin`:** Centralized authorization logic in private methods, enforced access control on actions, and moved allowed pubkeys to class constant for maintainability. - **Livewire `News`:** Enforced authorization for editing and deleting news with guard methods and ensured unauthorized users can't access data. - **Bootstrap exceptions:** Implemented custom exception handling to record Livewire-related security issues while preventing redundant logs. - Updated tests with new behavior verification covering access control and exception responses. --- bootstrap/app.php | 20 +++- .../association/members/admin.blade.php | 97 +++++++++++++------ .../views/livewire/association/news.blade.php | 23 ++++- .../Association/Members/AdminTest.php | 59 +++++++++++ .../Feature/Livewire/Association/NewsTest.php | 52 ++++++++++ tests/Feature/SecurityMonitorTest.php | 34 +++++++ 6 files changed, 251 insertions(+), 34 deletions(-) diff --git a/bootstrap/app.php b/bootstrap/app.php index a9a3625..93b3267 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -20,10 +20,22 @@ return Application::configure(basePath: dirname(__DIR__)) ThrottleRequests::class.':api', ]); }) - ->withExceptions(function (Exceptions $exceptions) { - Integration::handles($exceptions); + ->withExceptions(function (Exceptions $exceptions): void { + // Record Livewire tampering exceptions, then return false to stop them + // reaching Sentry/Nightwatch/log. Must run before Integration::handles() + // (callbacks fire in order; false short-circuits the rest). dontReport() + // is unusable here — it short-circuits before the recording would run. + $exceptions->report(function (Throwable $e): bool { + $monitor = app(SecurityMonitor::class); - $exceptions->report(function (Throwable $e) { - app(SecurityMonitor::class)->recordFromException($e); + if ($monitor->shouldRecord($e)) { + $monitor->recordFromException($e); + + return false; + } + + return true; }); + + Integration::handles($exceptions); })->create(); diff --git a/resources/views/livewire/association/members/admin.blade.php b/resources/views/livewire/association/members/admin.blade.php index 15a3eb4..e96cf9d 100644 --- a/resources/views/livewire/association/members/admin.blade.php +++ b/resources/views/livewire/association/members/admin.blade.php @@ -12,6 +12,23 @@ use Symfony\Component\HttpFoundation\StreamedResponse; new class extends Component { + /** + * Pubkeys permitted to manage members. Authorization is re-checked + * server-side on every sensitive action — gating the view on $isAllowed + * is cosmetic only, because Livewire exposes every public method as a + * directly callable endpoint regardless of what the view renders. + * + * @var array + */ + private const ALLOWED_PUBKEYS = [ + '0adf67475ccc5ca456fd3022e46f5d526eb0af6284bf85494c0dd7847f3e5033', + '430169631f2f0682c60cebb4f902d68f0c71c498fd1711fd982f052cf1fd4279', + '7acf30cf60b85c62b8f654556cc21e4016df8f5604b3b6892794f88bb80d7a1d', + 'f240be2b684f85cc81566f2081386af81d7427ea86250c8bde6b7a8500c761ba', + '19e358b8011f5f4fc653c565c6d4c2f33f32661f4f90982c9eedc292a8774ec3', + 'acbcec475a1a4f9481939ecfbd1c3d111f5b5a474a39ae039bbc720fdd305bec', + ]; + #[Locked] public bool $isAllowed = false; @@ -25,10 +42,13 @@ new class extends Component public string $sortDirection = 'desc'; + #[Locked] public ?int $selectedPlebId = null; + #[Locked] public ?int $confirmAcceptId = null; + #[Locked] public ?int $confirmDeleteId = null; public string $search = ''; @@ -39,6 +59,8 @@ new class extends Component public function updatedSearch(): void { + $this->ensureAuthorized(); + $this->plebs = $this->loadPlebs(); } @@ -51,42 +73,18 @@ new class extends Component { if (NostrAuth::check()) { $this->currentPubkey = NostrAuth::pubkey(); - $this->currentPleb = \App\Models\EinundzwanzigPleb::query() - ->where('pubkey', $this->currentPubkey)->first(); - $allowedPubkeys = [ - '0adf67475ccc5ca456fd3022e46f5d526eb0af6284bf85494c0dd7847f3e5033', - '430169631f2f0682c60cebb4f902d68f0c71c498fd1711fd982f052cf1fd4279', - '7acf30cf60b85c62b8f654556cc21e4016df8f5604b3b6892794f88bb80d7a1d', - 'f240be2b684f85cc81566f2081386af81d7427ea86250c8bde6b7a8500c761ba', - '19e358b8011f5f4fc653c565c6d4c2f33f32661f4f90982c9eedc292a8774ec3', - 'acbcec475a1a4f9481939ecfbd1c3d111f5b5a474a39ae039bbc720fdd305bec', - ]; - if (in_array($this->currentPubkey, $allowedPubkeys, true)) { - $this->isAllowed = true; - } + $this->currentPleb = NostrAuth::user()?->getPleb(); } - $this->plebs = $this->loadPlebs(); + $this->refreshAccess(); } public function handleNostrLoggedIn($signedEvent = null): void { - $pubkey = NostrAuth::loginWithSignedEvent($signedEvent); + $this->currentPubkey = NostrAuth::loginWithSignedEvent($signedEvent); + $this->currentPleb = NostrAuth::user()?->getPleb(); - $this->currentPubkey = $pubkey; - $this->currentPleb = EinundzwanzigPleb::query() - ->where('pubkey', $pubkey)->first(); - - $allowedPubkeys = [ - '0adf67475ccc5ca456fd3022e46f5d526eb0af6284bf85494c0dd7847f3e5033', - '430169631f2f0682c60cebb4f902d68f0c71c498fd1711fd982f052cf1fd4279', - '7acf30cf60b85c62b8f654556cc21e4016df8f5604b3b6892794f88bb80d7a1d', - 'f240be2b684f85cc81566f2081386af81d7427ea86250c8bde6b7a8500c761ba', - '19e358b8011f5f4fc653c565c6d4c2f33f32661f4f90982c9eedc292a8774ec3', - 'acbcec475a1a4f9481939ecfbd1c3d111f5b5a474a39ae039bbc720fdd305bec', - ]; - - $this->isAllowed = in_array($pubkey, $allowedPubkeys, true); + $this->refreshAccess(); } public function handleNostrLoggedOut(): void @@ -94,6 +92,27 @@ new class extends Component $this->currentPubkey = null; $this->currentPleb = null; $this->isAllowed = false; + $this->plebs = []; + } + + private function isAuthorized(): bool + { + return NostrAuth::check() + && in_array(NostrAuth::pubkey(), self::ALLOWED_PUBKEYS, true); + } + + private function ensureAuthorized(): void + { + abort_unless($this->isAuthorized(), 403); + } + + private function refreshAccess(): void + { + $this->isAllowed = $this->isAuthorized(); + + if ($this->isAllowed) { + $this->plebs = $this->loadPlebs(); + } } private function loadPlebs() @@ -127,6 +146,8 @@ new class extends Component public function sort(string $column): void { + $this->ensureAuthorized(); + if ($this->sortBy === $column) { $this->sortDirection = $this->sortDirection === 'asc' ? 'desc' : 'asc'; } else { @@ -139,30 +160,40 @@ new class extends Component public function togglePaidFilter(): void { + $this->ensureAuthorized(); + $this->showPaidOnly = !$this->showPaidOnly; $this->plebs = $this->loadPlebs(); } public function openPaymentModal(int $plebId): void { + $this->ensureAuthorized(); + $this->selectedPlebId = $plebId; Flux::modal('payment-details')->show(); } public function accept($rowId): void { + $this->ensureAuthorized(); + $this->confirmAcceptId = $rowId; Flux::modal('confirm-accept-pleb')->show(); } public function delete($rowId): void { + $this->ensureAuthorized(); + $this->confirmDeleteId = $rowId; Flux::modal('confirm-delete-pleb')->show(); } public function acceptPleb(): void { + $this->ensureAuthorized(); + if ($this->confirmAcceptId) { $pleb = EinundzwanzigPleb::query()->findOrFail($this->confirmAcceptId); $for = $pleb->application_for; @@ -180,6 +211,8 @@ new class extends Component public function deletePleb(): void { + $this->ensureAuthorized(); + if ($this->confirmDeleteId) { $pleb = EinundzwanzigPleb::query()->findOrFail($this->confirmDeleteId); $pleb->application_for = null; @@ -199,6 +232,8 @@ new class extends Component public function exportCsv(): StreamedResponse { + $this->ensureAuthorized(); + $currentYear = (int) date('Y'); $years = PaymentEvent::query() ->where('year', '>=', 2025) @@ -257,6 +292,10 @@ new class extends Component #[Computed] public function selectedPleb(): ?EinundzwanzigPleb { + if (! $this->isAuthorized()) { + return null; + } + return EinundzwanzigPleb::with(['paymentEvents'])->find($this->selectedPlebId); } }; diff --git a/resources/views/livewire/association/news.blade.php b/resources/views/livewire/association/news.blade.php index fd03eb9..e59efe4 100644 --- a/resources/views/livewire/association/news.blade.php +++ b/resources/views/livewire/association/news.blade.php @@ -41,6 +41,7 @@ class extends Component { #[Locked] public ?\App\Models\EinundzwanzigPleb $currentPleb = null; + #[Locked] public ?int $confirmDeleteId = null; public function mount(): void @@ -58,7 +59,9 @@ class extends Component { $this->isAllowed = true; } - $this->loadNews(); + if ($this->isAllowed) { + $this->loadNews(); + } } #[Computed] @@ -93,6 +96,8 @@ class extends Component { public function save(): void { + $this->ensureCanEdit(); + $this->validate([ 'file' => 'required|file|mimes:pdf', 'form.category' => 'required|string|in:'.implode(',', NewsCategory::values()), @@ -119,11 +124,15 @@ class extends Component { public function confirmDelete(int $id): void { + $this->ensureCanEdit(); + $this->confirmDeleteId = $id; } public function delete(): void { + $this->ensureCanEdit(); + $news = Notification::query()->findOrFail($this->confirmDeleteId); $news->delete(); $this->loadNews(); @@ -131,9 +140,21 @@ class extends Component { public function removeFile(): void { + $this->ensureCanEdit(); + $this->file->delete(); $this->file = null; } + + private function canEditNews(): bool + { + return NostrAuth::user()?->isBoardMember() ?? false; + } + + private function ensureCanEdit(): void + { + abort_unless($this->canEditNews(), 403); + } }; ?> diff --git a/tests/Feature/Livewire/Association/Members/AdminTest.php b/tests/Feature/Livewire/Association/Members/AdminTest.php index 4a7e2b8..0dae943 100644 --- a/tests/Feature/Livewire/Association/Members/AdminTest.php +++ b/tests/Feature/Livewire/Association/Members/AdminTest.php @@ -4,6 +4,8 @@ use App\Models\EinundzwanzigPleb; use App\Support\NostrAuth; use Livewire\Livewire; +const ALLOWED_ADMIN_PUBKEY = '0adf67475ccc5ca456fd3022e46f5d526eb0af6284bf85494c0dd7847f3e5033'; + it('denies access to unauthorized users', function () { $pleb = EinundzwanzigPleb::factory()->create(); @@ -73,3 +75,60 @@ it('displays einundzwanzig pleb table when authorized', function () { ->assertSet('isAllowed', true) ->assertSee('einundzwanzig-pleb-table'); }); + +it('does not load the member list for unauthorized visitors', function () { + EinundzwanzigPleb::factory()->count(3)->create(); + + Livewire::test('association.members.admin') + ->assertSet('isAllowed', false) + ->assertSet('plebs', []); +}); + +it('forbids guests from exporting the member CSV', function () { + Livewire::test('association.members.admin') + ->call('exportCsv') + ->assertForbidden(); +}); + +it('forbids unauthorized members from exporting the member CSV', function () { + $pleb = EinundzwanzigPleb::factory()->create(); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.members.admin') + ->call('exportCsv') + ->assertForbidden(); +}); + +it('forbids unauthorized members from accepting an application', function () { + $pleb = EinundzwanzigPleb::factory()->create(); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.members.admin') + ->call('acceptPleb') + ->assertForbidden(); +}); + +it('forbids unauthorized members from rejecting an application', function () { + $pleb = EinundzwanzigPleb::factory()->create(); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.members.admin') + ->call('deletePleb') + ->assertForbidden(); +}); + +it('lets an authorized member pass the authorization guard', function () { + $pleb = EinundzwanzigPleb::factory()->create([ + 'pubkey' => ALLOWED_ADMIN_PUBKEY, + ]); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.members.admin') + ->call('acceptPleb') + ->assertStatus(200) + ->assertHasNoErrors(); +}); diff --git a/tests/Feature/Livewire/Association/NewsTest.php b/tests/Feature/Livewire/Association/NewsTest.php index 8545d60..942bbf8 100644 --- a/tests/Feature/Livewire/Association/NewsTest.php +++ b/tests/Feature/Livewire/Association/NewsTest.php @@ -112,6 +112,58 @@ it('can delete news entry', function () { expect(Notification::find($news->id))->toBeNull(); }); +it('forbids guests from deleting news', function () { + $author = EinundzwanzigPleb::factory()->create(); + $news = Notification::factory()->create([ + 'einundzwanzig_pleb_id' => $author->id, + ]); + + Livewire::test('association.news') + ->call('delete') + ->assertForbidden(); + + expect(Notification::find($news->id))->not->toBeNull(); +}); + +it('forbids non-board members from deleting news', function () { + $author = EinundzwanzigPleb::factory()->create(); + $news = Notification::factory()->create([ + 'einundzwanzig_pleb_id' => $author->id, + ]); + $pleb = EinundzwanzigPleb::factory()->active()->withPaidCurrentYear()->create(); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.news') + ->call('delete') + ->assertForbidden(); + + expect(Notification::find($news->id))->not->toBeNull(); +}); + +it('forbids non-board members from creating news', function () { + $pleb = EinundzwanzigPleb::factory()->active()->withPaidCurrentYear()->create(); + + NostrAuth::login($pleb->pubkey); + + Livewire::test('association.news') + ->call('save') + ->assertForbidden(); + + expect(Notification::count())->toBe(0); +}); + +it('does not load news for unauthorized visitors', function () { + $author = EinundzwanzigPleb::factory()->create(); + Notification::factory()->count(2)->create([ + 'einundzwanzig_pleb_id' => $author->id, + ]); + + Livewire::test('association.news') + ->assertSet('isAllowed', false) + ->assertSet('news', []); +}); + it('displays news list', function () { $pleb = EinundzwanzigPleb::factory()->active()->withPaidCurrentYear()->create(); $news1 = Notification::factory()->create(); diff --git a/tests/Feature/SecurityMonitorTest.php b/tests/Feature/SecurityMonitorTest.php index 1a45ead..727dd02 100644 --- a/tests/Feature/SecurityMonitorTest.php +++ b/tests/Feature/SecurityMonitorTest.php @@ -2,7 +2,9 @@ use App\Models\SecurityAttempt; use App\Services\SecurityMonitor; +use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Log; use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException; beforeEach(function () { @@ -145,6 +147,38 @@ it('truncates long values', function () { expect(strlen($attempt->user_agent))->toBeLessThanOrEqual(500); }); +it('records a security attempt when a locked-property exception is reported through the handler', function () { + $exception = new CannotUpdateLockedPropertyException('isLoggedIn'); + + app(ExceptionHandler::class)->report($exception); + + expect(SecurityAttempt::count())->toBe(1) + ->and(SecurityAttempt::first()->target_property)->toBe('isLoggedIn'); +}); + +it('does not forward locked-property exceptions to the default log stack', function () { + Log::spy(); + + app(ExceptionHandler::class)->report(new CannotUpdateLockedPropertyException('isLoggedIn')); + + expect(SecurityAttempt::count())->toBe(1); + + Log::shouldNotHaveReceived('log'); + Log::shouldNotHaveReceived('error'); + Log::shouldNotHaveReceived('critical'); + Log::shouldNotHaveReceived('warning'); +}); + +it('still forwards non-security exceptions to the default log stack', function () { + Log::spy(); + + app(ExceptionHandler::class)->report(new RuntimeException('boom')); + + expect(SecurityAttempt::count())->toBe(0); + + Log::shouldHaveReceived('error'); +}); + it('handles X-Forwarded-For header', function () { $exception = new CannotUpdateLockedPropertyException('test');