diff --git a/core/lib/Models/Firewall/FirewallLogObject.php b/core/lib/Models/Firewall/FirewallLogObject.php index 375301a..8204b4b 100644 --- a/core/lib/Models/Firewall/FirewallLogObject.php +++ b/core/lib/Models/Firewall/FirewallLogObject.php @@ -26,6 +26,7 @@ class FirewallLogObject implements \JsonSerializable, JsonDeserializable public const EVENT_RULE_REMOVED = 'rule_removed'; private ?string $id = null; + private ?string $eventId = null; private ?string $tenantId = null; private ?string $ipAddress = null; private ?string $deviceFingerprint = null; @@ -55,6 +56,9 @@ class FirewallLogObject implements \JsonSerializable, JsonDeserializable if (array_key_exists('tenantId', $data)) { $this->tenantId = $data['tenantId'] !== null ? (string)$data['tenantId'] : null; } + if (array_key_exists('eventId', $data)) { + $this->eventId = $data['eventId'] !== null ? (string)$data['eventId'] : null; + } if (array_key_exists('ipAddress', $data)) { $this->ipAddress = $data['ipAddress'] !== null ? (string)$data['ipAddress'] : null; } @@ -101,6 +105,7 @@ class FirewallLogObject implements \JsonSerializable, JsonDeserializable { return [ 'id' => $this->id, + 'eventId' => $this->eventId, 'tenantId' => $this->tenantId, 'ipAddress' => $this->ipAddress, 'deviceFingerprint' => $this->deviceFingerprint, @@ -130,6 +135,17 @@ class FirewallLogObject implements \JsonSerializable, JsonDeserializable return $this; } + public function getEventId(): ?string + { + return $this->eventId; + } + + public function setEventId(?string $eventId): self + { + $this->eventId = $eventId; + return $this; + } + public function getTenantId(): ?string { return $this->tenantId; diff --git a/core/lib/Module/Module.php b/core/lib/Module/Module.php index 8862b0b..c2d599a 100644 --- a/core/lib/Module/Module.php +++ b/core/lib/Module/Module.php @@ -35,7 +35,6 @@ class Module extends ModuleInstanceAbstract implements ModuleConsoleInterface, M ); foreach ([ - SecurityEvent::AUTH_FAILURE, SecurityEvent::AUTH_SUCCESS, SecurityEvent::ACCESS_DENIED, SecurityEvent::BRUTE_FORCE_DETECTED, diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index 24d381e..cd06315 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -139,6 +139,12 @@ class FirewallService return; } + $event->setTenantId($tenantId); + $log = $this->securityLog($event); + if ($log === null || !$this->store->createLogOnce($log)) { + return; + } + // Check for brute force $windowSeconds = $this->getBoundedIntegerConfig( self::CONFIG_FAILURE_WINDOW, @@ -157,9 +163,6 @@ class FirewallService $windowSeconds ); - // Include current failure in count - $failureCount++; - if ($failureCount >= $maxFailures) { $this->handleBruteForce($tenantId, $ipAddress, $failureCount, $windowSeconds); } @@ -200,15 +203,24 @@ class FirewallService * Log security event to firewall logs */ public function logSecurityEvent(SecurityEvent $event): void + { + $log = $this->securityLog($event); + if ($log !== null) { + $this->store->createLog($log); + } + } + + private function securityLog(SecurityEvent $event): ?FirewallLogObject { $tenantId = $event->getTenantId() ?? $this->tenantContext->identifier(); $ruleScope = $event->get('ruleScope'); if (!$tenantId && $ruleScope !== FirewallRuleObject::SCOPE_SYSTEM) { - return; + return null; } $log = new FirewallLogObject(); - $log->setTenantId($tenantId) + return $log->setEventId($event->getEventId()) + ->setTenantId($tenantId) ->setIpAddress($event->getIpAddress()) ->setDeviceFingerprint($event->getDeviceFingerprint()) ->setUserAgent($event->getUserAgent()) @@ -221,8 +233,6 @@ class FirewallService ->setIdentityId($event->getUserId() ?? $event->getIdentityId()) ->setTimestamp(new \DateTimeImmutable()) ->setMetadata($event->getData()); - - $this->store->createLog($log); } /** diff --git a/core/lib/Stores/FirewallStore.php b/core/lib/Stores/FirewallStore.php index a40e238..e8a2a02 100644 --- a/core/lib/Stores/FirewallStore.php +++ b/core/lib/Stores/FirewallStore.php @@ -260,6 +260,30 @@ class FirewallStore return $log; } + /** + * Insert an event-backed log once, using the event ID as MongoDB's unique key. + */ + public function createLogOnce(FirewallLogObject $log): bool + { + $eventId = $log->getEventId(); + if ($eventId === null || $eventId === '') { + throw new \InvalidArgumentException('Idempotent firewall logs require an event ID.'); + } + + $data = $log->jsonSerialize(); + unset($data['id']); + $data['_id'] = $eventId; + + $result = $this->dataStore->selectCollection(self::LOGS_COLLECTION)->updateOne( + ['_id' => $eventId], + ['$setOnInsert' => $data], + ['upsert' => true] + ); + $log->setId($eventId); + + return $result->getUpsertedCount() === 1; + } + /** * Get logs for a tenant with optional filters */ diff --git a/shared/lib/Event/Event.php b/shared/lib/Event/Event.php index a6454e8..16c65d2 100644 --- a/shared/lib/Event/Event.php +++ b/shared/lib/Event/Event.php @@ -12,6 +12,7 @@ class Event private bool $propagationStopped = false; private array $data = []; private float $timestamp; + private string $eventId; private ?string $tenantId = null; private ?string $identityId = null; @@ -21,6 +22,7 @@ class Event ) { $this->data = $data; $this->timestamp = microtime(true); + $this->eventId = bin2hex(random_bytes(16)); } /** @@ -80,6 +82,11 @@ class Event return $this->timestamp; } + public function getEventId(): string + { + return $this->eventId; + } + /** * Stop event propagation to subsequent listeners */ @@ -143,4 +150,4 @@ class Event 'identityId' => $this->identityId, ]; } -} \ No newline at end of file +} diff --git a/tests/php/Integration/Stores/FirewallStoreTest.php b/tests/php/Integration/Stores/FirewallStoreTest.php index d436a99..62fd600 100644 --- a/tests/php/Integration/Stores/FirewallStoreTest.php +++ b/tests/php/Integration/Stores/FirewallStoreTest.php @@ -219,6 +219,25 @@ class FirewallStoreTest extends TestCase self::assertSame(FirewallRuleManager::ORIGIN_MANUAL, $logs[0]->getMetadata()['origin']); } + #[TestDox('Event-backed firewall logs are inserted exactly once')] + public function testIdempotentLogPersistence(): void + { + $log = (new FirewallLogObject()) + ->setEventId('event-123') + ->setTenantId('tenant-a') + ->setIpAddress('203.0.113.10') + ->setEventType(FirewallLogObject::EVENT_AUTH_FAILURE) + ->setResult(FirewallLogObject::RESULT_BLOCKED) + ->setTimestamp(new \DateTimeImmutable()); + + self::assertTrue($this->store->createLogOnce($log)); + self::assertFalse($this->store->createLogOnce($log)); + + $logs = $this->store->listLogs('tenant-a'); + self::assertCount(1, $logs); + self::assertSame('event-123', $logs[0]->getEventId()); + } + private function rule( string $reason, string $scope, diff --git a/tests/php/Unit/Models/Firewall/FirewallLogObjectTest.php b/tests/php/Unit/Models/Firewall/FirewallLogObjectTest.php index 2f1191f..6518612 100644 --- a/tests/php/Unit/Models/Firewall/FirewallLogObjectTest.php +++ b/tests/php/Unit/Models/Firewall/FirewallLogObjectTest.php @@ -15,12 +15,14 @@ class FirewallLogObjectTest extends TestCase public function testRuleContextSerialization(): void { $log = (new FirewallLogObject()) + ->setEventId('event-123') ->setRuleId('rule-123') ->setRuleScope(FirewallRuleObject::SCOPE_SYSTEM); $restored = (new FirewallLogObject())->jsonDeserialize($log->jsonSerialize()); self::assertSame('rule-123', $restored->getRuleId()); + self::assertSame('event-123', $restored->getEventId()); self::assertSame(FirewallRuleObject::SCOPE_SYSTEM, $restored->getRuleScope()); } diff --git a/tests/php/Unit/Module/CoreModuleTest.php b/tests/php/Unit/Module/CoreModuleTest.php index 45317d5..74daceb 100644 --- a/tests/php/Unit/Module/CoreModuleTest.php +++ b/tests/php/Unit/Module/CoreModuleTest.php @@ -28,12 +28,13 @@ final class CoreModuleTest extends TestCase $module->boot(); $definitions = $registry->definitions(); - self::assertCount(10, $definitions); + self::assertCount(9, $definitions); self::assertSame(['core'], array_values(array_unique(array_column($definitions, 'module')))); self::assertSame( FirewallService::class, $registry->listeners(SecurityEvent::AUTH_FAILURE, DeliveryMode::Immediate)[0]->service, ); + self::assertSame([], $registry->listeners(SecurityEvent::AUTH_FAILURE, DeliveryMode::Deferred)); foreach ([ SecurityEvent::RATE_LIMIT_EXCEEDED, SecurityEvent::SUSPICIOUS_ACTIVITY, diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php index 326298e..4ca1e42 100644 --- a/tests/php/Unit/Service/FirewallServiceTest.php +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -339,6 +339,7 @@ class FirewallServiceTest extends TestCase #[TestDox('Typed tenant firewall settings drive brute-force thresholds')] public function testFirewallConfiguration(): void { + $this->store->method('createLogOnce')->willReturn(true); $this->currentConfiguration = (new TenantConfiguration())->jsonDeserialize([ 'firewall' => [ 'enabled' => true, @@ -364,6 +365,7 @@ class FirewallServiceTest extends TestCase #[TestDox('Unsafe numeric firewall settings fall back to safe defaults')] public function testConfigurationBounds(): void { + $this->store->method('createLogOnce')->willReturn(true); $this->currentConfiguration = (new TenantConfiguration())->jsonDeserialize([ 'firewall' => [ 'maxAuthFailures' => 0, @@ -384,11 +386,12 @@ class FirewallServiceTest extends TestCase #[TestDox('Automatic blocks retain the tenant carried by the authentication event')] public function testAutomaticBlockTenant(): void { + $this->store->method('createLogOnce')->willReturn(true); $this->currentTenant = 'tenant-context'; $this->store->expects($this->once()) ->method('countRecentFailures') ->with('tenant-event', '203.0.113.10', 300) - ->willReturn(4); + ->willReturn(5); $this->store->expects($this->once()) ->method('findExactIpRule') ->with( @@ -431,6 +434,7 @@ class FirewallServiceTest extends TestCase #[TestDox('Authentication events without a tenant use the current tenant')] public function testAutomaticBlockTenantFallback(): void { + $this->store->method('createLogOnce')->willReturn(true); $this->store->expects($this->once()) ->method('countRecentFailures') ->with('tenant-a', '203.0.113.10', 300) @@ -453,6 +457,25 @@ class FirewallServiceTest extends TestCase ); } + #[TestDox('Repeated delivery of one authentication event is counted once')] + public function testAuthenticationFailureIdempotency(): void + { + $this->store->expects($this->exactly(2)) + ->method('createLogOnce') + ->willReturnOnConsecutiveCalls(true, false); + $this->store->expects($this->once()) + ->method('countRecentFailures') + ->with('tenant-a', '203.0.113.10', 300) + ->willReturn(1); + $event = \KTXF\Event\SecurityEvent::authFailure('203.0.113.10'); + $eventId = $event->getEventId(); + + $this->service->handleAuthFailure($event); + $this->service->handleAuthFailure($event); + + self::assertSame($eventId, $event->getEventId()); + } + private function rule( string $id, string $scope,