diff --git a/core/lib/Security/Event/SecurityEvent.php b/core/lib/Security/Event/SecurityEvent.php index ccfaee3..fd9d150 100644 --- a/core/lib/Security/Event/SecurityEvent.php +++ b/core/lib/Security/Event/SecurityEvent.php @@ -9,7 +9,7 @@ use KTXF\Event\Event; /** * Security-specific event for authentication and access control events */ -class SecurityEvent extends Event +final class SecurityEvent extends Event { // Event names public const AUTH_SUCCESS = 'security.auth.success'; @@ -35,15 +35,6 @@ class SecurityEvent extends Event public const FIREWALL_RULE_REMOVED = 'security.firewall.rule.removed'; public const FIREWALL_SETTINGS_UPDATED = 'security.firewall.settings.updated'; - private ?string $ipAddress = null; - private ?string $deviceFingerprint = null; - private ?string $userAgent = null; - private ?string $requestPath = null; - private ?string $requestMethod = null; - private ?string $userId = null; - private ?string $reason = null; - private int $severity = self::SEVERITY_INFO; - // Severity levels public const SEVERITY_DEBUG = 0; public const SEVERITY_INFO = 1; @@ -51,11 +42,25 @@ class SecurityEvent extends Event public const SEVERITY_ERROR = 3; public const SEVERITY_CRITICAL = 4; - public function __construct(string $name, array $data = []) - { - parent::__construct($name, $data); + private readonly int $severity; - $this->severity = self::getSeverityForEvent($name); + public function __construct( + string $name, + array $data = [], + ?string $tenantId = null, + ?string $identityId = null, + private readonly ?string $ipAddress = null, + private readonly ?string $deviceFingerprint = null, + private readonly ?string $userAgent = null, + private readonly ?string $requestPath = null, + private readonly ?string $requestMethod = null, + private readonly ?string $userId = null, + private readonly ?string $reason = null, + ?int $severity = null, + ) { + parent::__construct($name, $data, $tenantId, $identityId); + + $this->severity = $severity ?? self::getSeverityForEvent($name); } /** @@ -65,13 +70,30 @@ class SecurityEvent extends Event string $name, ?string $ipAddress = null, ?string $deviceFingerprint = null, - array $data = [] + array $data = [], + ?string $tenantId = null, + ?string $identityId = null, + ?string $userAgent = null, + ?string $requestPath = null, + ?string $requestMethod = null, + ?string $userId = null, + ?string $reason = null, + ?int $severity = null, ): self { - $event = new self($name, $data); - $event->ipAddress = $ipAddress; - $event->deviceFingerprint = $deviceFingerprint; - - return $event; + return new self( + $name, + $data, + $tenantId, + $identityId, + $ipAddress, + $deviceFingerprint, + $userAgent, + $requestPath, + $requestMethod, + $userId, + $reason, + $severity, + ); } /** @@ -81,15 +103,26 @@ class SecurityEvent extends Event string $ipAddress, ?string $deviceFingerprint = null, ?string $userId = null, - ?string $reason = null + ?string $reason = null, + ?string $tenantId = null, + ?string $identityId = null, + ?string $userAgent = null, + ?string $requestPath = null, + ?string $requestMethod = null, ): self { - $event = self::create(self::AUTH_FAILURE, $ipAddress, $deviceFingerprint, [ - 'userId' => $userId, - 'reason' => $reason, - ]); - $event->userId = $userId; - $event->reason = $reason; - return $event; + return self::create( + self::AUTH_FAILURE, + $ipAddress, + $deviceFingerprint, + ['userId' => $userId, 'reason' => $reason], + $tenantId, + $identityId, + $userAgent, + $requestPath, + $requestMethod, + $userId, + $reason, + ); } /** @@ -98,13 +131,17 @@ class SecurityEvent extends Event public static function authSuccess( string $ipAddress, ?string $deviceFingerprint = null, - string $userId = null + ?string $userId = null, + ?string $tenantId = null, ): self { - $event = self::create(self::AUTH_SUCCESS, $ipAddress, $deviceFingerprint, [ - 'userId' => $userId, - ]); - $event->userId = $userId; - return $event; + return self::create( + self::AUTH_SUCCESS, + $ipAddress, + $deviceFingerprint, + ['userId' => $userId], + tenantId: $tenantId, + userId: $userId, + ); } /** @@ -113,18 +150,16 @@ class SecurityEvent extends Event public static function bruteForceDetected( string $ipAddress, int $failureCount, - int $windowSeconds + int $windowSeconds, + ?string $tenantId = null, ): self { - $event = self::create(self::BRUTE_FORCE_DETECTED, $ipAddress, null, [ - 'failureCount' => $failureCount, - 'windowSeconds' => $windowSeconds, - ]); - $event->reason = sprintf( - '%d failed attempts in %d seconds', - $failureCount, - $windowSeconds + return self::create( + self::BRUTE_FORCE_DETECTED, + $ipAddress, + data: ['failureCount' => $failureCount, 'windowSeconds' => $windowSeconds], + tenantId: $tenantId, + reason: sprintf('%d failed attempts in %d seconds', $failureCount, $windowSeconds), ); - return $event; } /** @@ -134,20 +169,21 @@ class SecurityEvent extends Event string $ipAddress, int $requestCount, int $windowSeconds, - ?string $endpoint = null + ?string $endpoint = null, + ?string $tenantId = null, ): self { - $event = self::create(self::RATE_LIMIT_EXCEEDED, $ipAddress, null, [ - 'requestCount' => $requestCount, - 'windowSeconds' => $windowSeconds, - 'endpoint' => $endpoint, - ]); - $event->requestPath = $endpoint; - $event->reason = sprintf( - '%d requests in %d seconds', - $requestCount, - $windowSeconds + return self::create( + self::RATE_LIMIT_EXCEEDED, + $ipAddress, + data: [ + 'requestCount' => $requestCount, + 'windowSeconds' => $windowSeconds, + 'endpoint' => $endpoint, + ], + tenantId: $tenantId, + requestPath: $endpoint, + reason: sprintf('%d requests in %d seconds', $requestCount, $windowSeconds), ); - return $event; } /** @@ -158,15 +194,19 @@ class SecurityEvent extends Event ?string $deviceFingerprint = null, ?string $ruleId = null, ?string $ruleScope = null, - ?string $reason = null + ?string $reason = null, + ?string $tenantId = null, + ?string $identityId = null, ): self { - $event = self::create(self::ACCESS_DENIED, $ipAddress, $deviceFingerprint, [ - 'ruleId' => $ruleId, - 'ruleScope' => $ruleScope, - 'reason' => $reason, - ]); - $event->reason = $reason; - return $event; + return self::create( + self::ACCESS_DENIED, + $ipAddress, + $deviceFingerprint, + ['ruleId' => $ruleId, 'ruleScope' => $ruleScope, 'reason' => $reason], + $tenantId, + $identityId, + reason: $reason, + ); } /** @@ -202,86 +242,39 @@ class SecurityEvent extends Event return $this->ipAddress; } - public function setIpAddress(?string $ipAddress): self - { - $this->ipAddress = $ipAddress; - return $this; - } - public function getDeviceFingerprint(): ?string { return $this->deviceFingerprint; } - public function setDeviceFingerprint(?string $deviceFingerprint): self - { - $this->deviceFingerprint = $deviceFingerprint; - return $this; - } - public function getUserAgent(): ?string { return $this->userAgent; } - public function setUserAgent(?string $userAgent): self - { - $this->userAgent = $userAgent; - return $this; - } - public function getRequestPath(): ?string { return $this->requestPath; } - public function setRequestPath(?string $requestPath): self - { - $this->requestPath = $requestPath; - return $this; - } - public function getRequestMethod(): ?string { return $this->requestMethod; } - public function setRequestMethod(?string $requestMethod): self - { - $this->requestMethod = $requestMethod; - return $this; - } - public function getUserId(): ?string { return $this->userId; } - public function setUserId(?string $userId): self - { - $this->userId = $userId; - return $this; - } - public function getReason(): ?string { return $this->reason; } - public function setReason(?string $reason): self - { - $this->reason = $reason; - return $this; - } - public function getSeverity(): int { return $this->severity; } - public function setSeverity(int $severity): self - { - $this->severity = $severity; - return $this; - } } diff --git a/core/lib/Service/FirewallRuleManager.php b/core/lib/Service/FirewallRuleManager.php index fc59a17..5bc03e0 100644 --- a/core/lib/Service/FirewallRuleManager.php +++ b/core/lib/Service/FirewallRuleManager.php @@ -254,8 +254,13 @@ final class FirewallRuleManager $origin ); - $event = new SecurityEvent(SecurityEvent::DEVICE_BLOCKED, ['device' => $fingerprint, 'reason' => $reason]); - $event->setDeviceFingerprint($fingerprint)->setReason($reason)->setTenantId($scope->tenantId); + $event = new SecurityEvent( + SecurityEvent::DEVICE_BLOCKED, + ['device' => $fingerprint, 'reason' => $reason], + tenantId: $scope->tenantId, + deviceFingerprint: $fingerprint, + reason: $reason, + ); $this->events->dispatch($event); return $rule; @@ -499,8 +504,13 @@ final class FirewallRuleManager string $ipAddress, ?string $reason ): void { - $event = new SecurityEvent($name, ['ip' => $ipAddress, 'reason' => $reason]); - $event->setIpAddress($ipAddress)->setReason($reason)->setTenantId($scope->tenantId); + $event = new SecurityEvent( + $name, + ['ip' => $ipAddress, 'reason' => $reason], + tenantId: $scope->tenantId, + ipAddress: $ipAddress, + reason: $reason, + ); $this->events->dispatch($event); } @@ -511,20 +521,23 @@ final class FirewallRuleManager array $change = [] ): void { - $event = new SecurityEvent($name, [ - 'ruleId' => $rule->getId(), - 'ruleScope' => $rule->getScope(), - 'ruleType' => $rule->getType(), - 'ruleAction' => $rule->getAction(), - 'ruleValue' => $rule->getValue(), - 'reason' => $rule->getReason(), - 'origin' => $rule->getMetadata()['origin'] ?? self::ORIGIN_MANUAL, - 'expiresAt' => $rule->getExpiresAt()?->format(\DateTimeInterface::ATOM), - ...($rule->getMetadata() ?? []), - ...$change, - ]); - $event->setTenantId($rule->getTenantId()) - ->setIdentityId($actorId ?? $rule->getCreatedBy()); + $event = new SecurityEvent( + $name, + [ + 'ruleId' => $rule->getId(), + 'ruleScope' => $rule->getScope(), + 'ruleType' => $rule->getType(), + 'ruleAction' => $rule->getAction(), + 'ruleValue' => $rule->getValue(), + 'reason' => $rule->getReason(), + 'origin' => $rule->getMetadata()['origin'] ?? self::ORIGIN_MANUAL, + 'expiresAt' => $rule->getExpiresAt()?->format(\DateTimeInterface::ATOM), + ...($rule->getMetadata() ?? []), + ...$change, + ], + tenantId: $rule->getTenantId(), + identityId: $actorId ?? $rule->getCreatedBy(), + ); $this->events->dispatch($event); } } diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index 94d4275..6f42dc7 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -139,7 +139,6 @@ class FirewallService return; } - $event->setTenantId($tenantId); $log = $this->securityLog($event); if ($log === null || !$this->store->createLogOnce($log)) { return; @@ -195,8 +194,12 @@ class FirewallService int $blockDuration ): void { // Publish brute force event - $event = SecurityEvent::bruteForceDetected($ipAddress, $failureCount, $windowSeconds); - $event->setTenantId($tenantId); + $event = SecurityEvent::bruteForceDetected( + $ipAddress, + $failureCount, + $windowSeconds, + $tenantId, + ); $this->events->dispatch($event); $this->rules->blockIp( @@ -308,9 +311,9 @@ class FirewallService $deviceFingerprint, $rule->getId(), $rule->getScope(), - $rule->getReason() + $rule->getReason(), + $this->tenantContext->identifier(), ); - $event->setTenantId($this->tenantContext->identifier()); $this->events->dispatch($event); } diff --git a/core/lib/Service/FirewallSettingsService.php b/core/lib/Service/FirewallSettingsService.php index c9d0b79..2fec583 100644 --- a/core/lib/Service/FirewallSettingsService.php +++ b/core/lib/Service/FirewallSettingsService.php @@ -52,13 +52,17 @@ final class FirewallSettingsService $tenant->setConfiguration($configuration); $this->tenants->deposit($tenant); - $event = new SecurityEvent(SecurityEvent::FIREWALL_SETTINGS_UPDATED, [ - 'changeReason' => $reason, - 'changeOrigin' => FirewallRuleManager::ORIGIN_MANUAL, - 'previous' => $previous, - 'current' => $current, - ]); - $event->setTenantId($tenantId)->setIdentityId($actorId); + $event = new SecurityEvent( + SecurityEvent::FIREWALL_SETTINGS_UPDATED, + [ + 'changeReason' => $reason, + 'changeOrigin' => FirewallRuleManager::ORIGIN_MANUAL, + 'previous' => $previous, + 'current' => $current, + ], + tenantId: $tenantId, + identityId: $actorId, + ); $this->events->dispatch($event); return $current; diff --git a/shared/lib/Event/Event.php b/shared/lib/Event/Event.php index 1ea6c96..daa34dd 100644 --- a/shared/lib/Event/Event.php +++ b/shared/lib/Event/Event.php @@ -10,16 +10,18 @@ namespace KTXF\Event; class Event { private bool $propagationStopped = false; - private array $data = []; - private float $timestamp; - private string $eventId; - private ?string $tenantId = null; - private ?string $identityId = null; + private readonly array $data; + private readonly float $timestamp; + private readonly string $eventId; public function __construct( private readonly string $name, - array $data = [] + array $data = [], + private readonly ?string $tenantId = null, + private readonly ?string $identityId = null, ) { + self::validateData($data); + $this->data = $data; $this->timestamp = microtime(true); $this->eventId = bin2hex(random_bytes(16)); @@ -41,15 +43,6 @@ class Event return $this->data[$key] ?? $default; } - /** - * Set a data value - */ - public function set(string $key, mixed $value): self - { - $this->data[$key] = $value; - return $this; - } - /** * Check if a data key exists */ @@ -111,15 +104,6 @@ class Event return $this->tenantId; } - /** - * Set tenant ID for multi-tenant context - */ - public function setTenantId(?string $tenantId): self - { - $this->tenantId = $tenantId; - return $this; - } - /** * Get identity ID (user who triggered the event) */ @@ -128,12 +112,18 @@ class Event return $this->identityId; } - /** - * Set identity ID - */ - public function setIdentityId(?string $identityId): self + private static function validateData(array $data): void { - $this->identityId = $identityId; - return $this; + foreach ($data as $value) { + if (is_array($value)) { + self::validateData($value); + continue; + } + if ($value !== null && !is_scalar($value)) { + throw new \InvalidArgumentException( + 'Event data must contain only scalar, null, or array values.', + ); + } + } } } diff --git a/tests/php/Unit/Event/EventTest.php b/tests/php/Unit/Event/EventTest.php new file mode 100644 index 0000000..6609c58 --- /dev/null +++ b/tests/php/Unit/Event/EventTest.php @@ -0,0 +1,41 @@ + ['value' => 'original']], + 'tenant-a', + 'identity-a', + ); + + $copy = $event->getData(); + $copy['nested']['value'] = 'changed'; + + self::assertSame('original', $event->get('nested')['value']); + self::assertSame('tenant-a', $event->getTenantId()); + self::assertSame('identity-a', $event->getIdentityId()); + } + + #[Test] + #[TestDox('Event data rejects mutable object references')] + public function rejectsMutablePayloadValues(): void + { + $this->expectException(\InvalidArgumentException::class); + + new Event('test.event', ['mutable' => new \stdClass()]); + } +} diff --git a/tests/php/Unit/Event/SecurityEventTest.php b/tests/php/Unit/Event/SecurityEventTest.php index ac04c8f..3d0b901 100644 --- a/tests/php/Unit/Event/SecurityEventTest.php +++ b/tests/php/Unit/Event/SecurityEventTest.php @@ -34,13 +34,35 @@ final class SecurityEventTest extends TestCase } #[Test] - #[TestDox('Explicit severity can override the event type default')] + #[TestDox('Construction can override the event type default severity')] public function allowsSeverityOverride(): void { - $event = new SecurityEvent(SecurityEvent::AUTH_FAILURE); - - $event->setSeverity(SecurityEvent::SEVERITY_CRITICAL); + $event = new SecurityEvent( + SecurityEvent::AUTH_FAILURE, + severity: SecurityEvent::SEVERITY_CRITICAL, + ); self::assertSame(SecurityEvent::SEVERITY_CRITICAL, $event->getSeverity()); } + + #[Test] + #[TestDox('Event state exposes no mutation methods')] + public function exposesNoMutationMethods(): void + { + foreach ([ + 'set', + 'setTenantId', + 'setIdentityId', + 'setIpAddress', + 'setDeviceFingerprint', + 'setUserAgent', + 'setRequestPath', + 'setRequestMethod', + 'setUserId', + 'setReason', + 'setSeverity', + ] as $method) { + self::assertFalse(method_exists(SecurityEvent::class, $method)); + } + } } diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php index 95c9f7b..28df6ef 100644 --- a/tests/php/Unit/Service/FirewallServiceTest.php +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -215,9 +215,9 @@ class FirewallServiceTest extends TestCase null, 'tenant-rule', FirewallRuleObject::SCOPE_TENANT, - 'Tenant block' + 'Tenant block', + 'tenant-a', ); - $event->setTenantId('tenant-a'); $this->service->logSecurityEvent($event); } @@ -275,9 +275,9 @@ class FirewallServiceTest extends TestCase '203.0.113.10', 101, 60, - '/login' + '/login', + 'tenant-a', ); - $event->setTenantId('tenant-a'); $this->service->logSecurityEvent($event); } @@ -300,11 +300,11 @@ class FirewallServiceTest extends TestCase \KTXC\Security\Event\SecurityEvent::SUSPICIOUS_ACTIVITY, '203.0.113.20', null, - ['detector' => 'payload-signature'] + ['detector' => 'payload-signature'], + tenantId: 'tenant-a', + requestPath: '/admin', + requestMethod: 'POST', ); - $event->setTenantId('tenant-a') - ->setRequestPath('/admin') - ->setRequestMethod('POST'); $this->service->logSecurityEvent($event); } @@ -329,9 +329,9 @@ class FirewallServiceTest extends TestCase 'ruleId' => 'rule-123', 'ruleScope' => FirewallRuleObject::SCOPE_SYSTEM, 'origin' => FirewallRuleManager::ORIGIN_MANUAL, - ] + ], + identityId: 'operator', ); - $event->setIdentityId('operator'); $this->service->logSecurityEvent($event); } @@ -351,9 +351,10 @@ class FirewallServiceTest extends TestCase ->willReturnArgument(0); $event = new \KTXC\Security\Event\SecurityEvent( \KTXC\Security\Event\SecurityEvent::FIREWALL_SETTINGS_UPDATED, - ['changeReason' => 'Tighten controls'] + ['changeReason' => 'Tighten controls'], + tenantId: 'tenant-a', + identityId: 'operator', ); - $event->setTenantId('tenant-a')->setIdentityId('operator'); $this->service->logSecurityEvent($event); } @@ -376,8 +377,10 @@ class FirewallServiceTest extends TestCase ->willReturn(4); $this->events->expects($this->never())->method('dispatch'); - $event = \KTXC\Security\Event\SecurityEvent::authFailure('203.0.113.10'); - $event->setTenantId('tenant-a'); + $event = \KTXC\Security\Event\SecurityEvent::authFailure( + '203.0.113.10', + tenantId: 'tenant-a', + ); $this->service->handleAuthFailure($event); self::assertSame(8, $this->currentConfiguration->firewall()->maxAuthFailures()); @@ -400,8 +403,10 @@ class FirewallServiceTest extends TestCase ->with('tenant-a', '203.0.113.10', 300) ->willReturn(0); - $event = \KTXC\Security\Event\SecurityEvent::authFailure('203.0.113.10'); - $event->setTenantId('tenant-a'); + $event = \KTXC\Security\Event\SecurityEvent::authFailure( + '203.0.113.10', + tenantId: 'tenant-a', + ); $this->service->handleAuthFailure($event); } @@ -454,8 +459,10 @@ class FirewallServiceTest extends TestCase } }); - $event = \KTXC\Security\Event\SecurityEvent::authFailure('203.0.113.10'); - $event->setTenantId('tenant-event'); + $event = \KTXC\Security\Event\SecurityEvent::authFailure( + '203.0.113.10', + tenantId: 'tenant-event', + ); $this->service->handleAuthFailure($event); self::assertSame(['tenant-event', 'tenant-event', 'tenant-event'], $publishedTenants);