From 72de7b9a222819ca6f7f6e1eaa8ea7275535f8e2 Mon Sep 17 00:00:00 2001 From: boban Date: Sun, 5 Jul 2026 02:28:13 +0200 Subject: [PATCH] fix(rbac,honeypot): gate audit-retention + email sendTest; honeytoken scans JSON body Re-review (completeness sweep + Codex) found: Audit\Index::saveRetention wrote the global audit_retention_days policy ungated (a non-admin could shrink retention and prune audit evidence) -> manage-panel guard + @can-hidden control; Settings\Email::sendTest lacked the role guard that mount/save already had -> manage-panel; DetectHoneytoken read only form-encoded bodies via post(), missing a JSON {"api_key":""} replay -> now scans json() too (still body-only, no query). --- app/Http/Middleware/DetectHoneytoken.php | 12 ++- app/Livewire/Audit/Index.php | 4 + app/Livewire/Settings/Email.php | 2 + .../views/livewire/audit/index.blade.php | 4 +- tests/Feature/RbacReviewFixTest.php | 102 ++++++++++++++++++ 5 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 tests/Feature/RbacReviewFixTest.php diff --git a/app/Http/Middleware/DetectHoneytoken.php b/app/Http/Middleware/DetectHoneytoken.php index ec0657b..ad633a2 100644 --- a/app/Http/Middleware/DetectHoneytoken.php +++ b/app/Http/Middleware/DetectHoneytoken.php @@ -45,13 +45,15 @@ class DetectHoneytoken return $next($request); } - // A small, bounded set of candidate values — request BODY only (post()), no query string, - // no recursion into nested arrays. + // A small, bounded set of candidate values — request BODY only (form-encoded via post() AND a + // JSON body via json()), never the query string (which a reflected link could carry to ban a + // victim), no recursion into nested arrays. $candidates = []; foreach (self::CANDIDATE_KEYS as $key) { - $value = $request->post($key); - if (is_string($value) && $value !== '') { - $candidates[] = $value; + foreach ([$request->post($key), $request->json($key)] as $value) { + if (is_string($value) && $value !== '') { + $candidates[] = $value; + } } } if (($bearer = $request->bearerToken()) !== null && $bearer !== '') { diff --git a/app/Livewire/Audit/Index.php b/app/Livewire/Audit/Index.php index 30c28e9..6ed213e 100644 --- a/app/Livewire/Audit/Index.php +++ b/app/Livewire/Audit/Index.php @@ -44,6 +44,10 @@ class Index extends Component /** Persist the retention policy, audit the change and notify the operator. */ public function saveRetention(): void { + // Panel-wide policy mutation (a short retention prunes audit history) — admin only. Viewing + // the audit log stays open to everyone; only changing the retention window is gated. + abort_unless(auth()->user()?->can('manage-panel'), 403); + $validated = $this->validate([ 'retentionDays' => ['nullable', 'integer', 'min:1', 'max:3650'], ]); diff --git a/app/Livewire/Settings/Email.php b/app/Livewire/Settings/Email.php index ba307bf..92c2f7d 100644 --- a/app/Livewire/Settings/Email.php +++ b/app/Livewire/Settings/Email.php @@ -106,6 +106,8 @@ class Email extends Component */ public function sendTest(): void { + abort_unless(auth()->user()?->can('manage-panel'), 403); + if (! $this->configured()) { $this->dispatch('notify', message: __('mail.notify_test_failed', ['error' => __('mail.not_configured')])); diff --git a/resources/views/livewire/audit/index.blade.php b/resources/views/livewire/audit/index.blade.php index 3314c9b..85a46fb 100644 --- a/resources/views/livewire/audit/index.blade.php +++ b/resources/views/livewire/audit/index.blade.php @@ -8,7 +8,8 @@ {{ __('audit.event_count', ['count' => $events->total()]) }} - {{-- Aufbewahrung (Retention) --}} + {{-- Aufbewahrung (Retention) — admin-only policy control --}} + @can('manage-panel')
{{-- Aktuelle Richtlinie --}} @@ -47,6 +48,7 @@
+ @endcan {{-- Ereignisliste --}} diff --git a/tests/Feature/RbacReviewFixTest.php b/tests/Feature/RbacReviewFixTest.php new file mode 100644 index 0000000..fb783e2 --- /dev/null +++ b/tests/Feature/RbacReviewFixTest.php @@ -0,0 +1,102 @@ +{$role}()->create()) + ->test(Index::class) + ->set('retentionDays', '1') + ->call('saveRetention') + ->assertForbidden(); + } + + $this->assertFalse(Setting::where('key', 'audit_retention_days')->exists()); + } + + public function test_admin_can_change_audit_retention(): void + { + Livewire::actingAs(User::factory()->create()) // default factory role = admin + ->test(Index::class) + ->set('retentionDays', '30') + ->call('saveRetention'); + + $this->assertSame('30', Setting::where('key', 'audit_retention_days')->value('value')); + } + + public function test_non_admin_cannot_send_test_email(): void + { + Mail::fake(); + + // Email::mount() is itself gated, so a non-admin cannot even mount the component; the sendTest + // guard is the defense-in-depth belt behind it. + foreach (['operator', 'viewer'] as $role) { + Livewire::actingAs(User::factory()->{$role}()->create()) + ->test(Email::class) + ->assertForbidden(); + } + } + + public function test_admin_passes_the_send_test_email_gate(): void + { + Mail::fake(); + + // Admin passes the role gate; unconfigured SMTP short-circuits with a notify (proves no 403). + Livewire::actingAs(User::factory()->create()) + ->test(Email::class) + ->call('sendTest') + ->assertDispatched('notify'); + } + + public function test_honeytoken_trips_on_a_json_body_from_a_non_exempt_ip(): void + { + config(['clusev.honeypot.enabled' => true, 'clusev.honeypot.canaries' => ['api_key' => 'C4n4ryJs0nV4lu3XYZ']]); + + $res = $this->call( + 'POST', + '/broadcasting/auth', + [], + [], + [], + ['REMOTE_ADDR' => '203.0.113.50', 'CONTENT_TYPE' => 'application/json', 'HTTP_ACCEPT' => 'application/json'], + (string) json_encode(['api_key' => 'C4n4ryJs0nV4lu3XYZ']) + ); + + $res->assertStatus(403); + $this->assertTrue(BannedIp::where('ip', '203.0.113.50')->exists()); + $this->assertTrue(AuditEvent::where('action', 'security.honeytoken_used')->exists()); + } + + public function test_honeytoken_does_not_trip_without_a_canary(): void + { + config(['clusev.honeypot.enabled' => true, 'clusev.honeypot.canaries' => ['api_key' => 'C4n4ryJs0nV4lu3XYZ']]); + + $this->call( + 'POST', + '/broadcasting/auth', + [], + [], + [], + ['REMOTE_ADDR' => '203.0.113.51', 'CONTENT_TYPE' => 'application/json', 'HTTP_ACCEPT' => 'application/json'], + (string) json_encode(['api_key' => 'just-a-normal-value']) + ); + + $this->assertFalse(BannedIp::where('ip', '203.0.113.51')->exists()); + } +}