From 63d91ca7fafc38a6a16d3c1c8af8017cb92c228b Mon Sep 17 00:00:00 2001 From: Sebastian Krupinski Date: Thu, 30 Jul 2026 21:38:19 -0400 Subject: [PATCH] feat(firewall): add tenant and system rule scopes Signed-off-by: Sebastian Krupinski --- .../Models/Firewall/FirewallRuleObject.php | 45 ++++++ core/lib/Service/FirewallService.php | 68 ++++---- core/lib/Stores/FirewallStore.php | 76 ++++++++- .../Firewall/FirewallRuleObjectTest.php | 55 +++++++ .../php/Unit/Service/FirewallServiceTest.php | 145 ++++++++++++++++++ 5 files changed, 352 insertions(+), 37 deletions(-) create mode 100644 tests/php/Unit/Models/Firewall/FirewallRuleObjectTest.php create mode 100644 tests/php/Unit/Service/FirewallServiceTest.php diff --git a/core/lib/Models/Firewall/FirewallRuleObject.php b/core/lib/Models/Firewall/FirewallRuleObject.php index 3636242..c7a41e4 100644 --- a/core/lib/Models/Firewall/FirewallRuleObject.php +++ b/core/lib/Models/Firewall/FirewallRuleObject.php @@ -11,6 +11,9 @@ use KTXF\Json\JsonDeserializable; */ class FirewallRuleObject implements \JsonSerializable, JsonDeserializable { + public const SCOPE_TENANT = 'tenant'; + public const SCOPE_SYSTEM = 'system'; + public const TYPE_IP = 'ip'; public const TYPE_IP_RANGE = 'ip_range'; public const TYPE_DEVICE = 'device'; @@ -19,6 +22,7 @@ class FirewallRuleObject implements \JsonSerializable, JsonDeserializable public const ACTION_BLOCK = 'block'; private ?string $id = null; + private string $scope = self::SCOPE_TENANT; private ?string $tenantId = null; private ?string $type = null; // ip, ip_range, device private ?string $action = null; // allow, block @@ -42,6 +46,10 @@ class FirewallRuleObject implements \JsonSerializable, JsonDeserializable $this->id = $data['id'] !== null ? (string)$data['id'] : null; } + if (!array_key_exists('scope', $data)) { + throw new \InvalidArgumentException('Firewall rules require an explicit scope.'); + } + $this->setScope((string)$data['scope']); if (array_key_exists('tenantId', $data)) { $this->tenantId = $data['tenantId'] !== null ? (string)$data['tenantId'] : null; } @@ -84,6 +92,7 @@ class FirewallRuleObject implements \JsonSerializable, JsonDeserializable { return [ 'id' => $this->id, + 'scope' => $this->scope, 'tenantId' => $this->tenantId, 'type' => $this->type, 'action' => $this->action, @@ -134,6 +143,42 @@ class FirewallRuleObject implements \JsonSerializable, JsonDeserializable return $this->tenantId; } + public function getScope(): string + { + return $this->scope; + } + + public function setScope(string $scope): self + { + if (!in_array($scope, [self::SCOPE_TENANT, self::SCOPE_SYSTEM], true)) { + throw new \InvalidArgumentException("Invalid firewall rule scope: {$scope}"); + } + + $this->scope = $scope; + return $this; + } + + public function isTenantScoped(): bool + { + return $this->scope === self::SCOPE_TENANT; + } + + public function isSystemScoped(): bool + { + return $this->scope === self::SCOPE_SYSTEM; + } + + public function assertValidScopeOwnership(): void + { + if ($this->isTenantScoped() && ($this->tenantId === null || $this->tenantId === '')) { + throw new \InvalidArgumentException('Tenant firewall rules require a tenant ID.'); + } + + if ($this->isSystemScoped() && $this->tenantId !== null) { + throw new \InvalidArgumentException('System firewall rules cannot have a tenant ID.'); + } + } + public function setTenantId(?string $tenantId): self { $this->tenantId = $tenantId; diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index 0cae13b..8d56d2b 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -36,8 +36,8 @@ class FirewallService private const CONFIG_AUTO_BLOCK_DURATION = 'firewall.autoBlockDuration'; private const CONFIG_ENABLED = 'firewall.enabled'; - /** @var FirewallRuleObject[]|null */ - private ?array $rulesCache = null; + /** @var array */ + private array $rulesCache = []; public function __construct( private readonly FirewallStore $store, @@ -83,24 +83,25 @@ class FirewallService $rules = $this->getActiveRules(); - // First check for explicit allow rules (whitelist takes precedence) - foreach ($rules as $rule) { - if ($rule->getAction() !== FirewallRuleObject::ACTION_ALLOW) { - continue; - } - - if ($this->ruleMatchesRequest($rule, $ipAddress, $deviceFingerprint)) { - return new FirewallAnalyzeResult(true, $rule->getId(), 'Explicitly allowed'); - } - } + foreach ([ + [FirewallRuleObject::SCOPE_SYSTEM, FirewallRuleObject::ACTION_BLOCK], + [FirewallRuleObject::SCOPE_TENANT, FirewallRuleObject::ACTION_ALLOW], + [FirewallRuleObject::SCOPE_TENANT, FirewallRuleObject::ACTION_BLOCK], + [FirewallRuleObject::SCOPE_SYSTEM, FirewallRuleObject::ACTION_ALLOW], + ] as [$scope, $action]) { + foreach ($rules as $rule) { + if ($rule->getScope() !== $scope || $rule->getAction() !== $action) { + continue; + } + + if (!$this->ruleMatchesRequest($rule, $ipAddress, $deviceFingerprint)) { + continue; + } + + if ($action === FirewallRuleObject::ACTION_ALLOW) { + return new FirewallAnalyzeResult(true, $rule->getId(), 'Explicitly allowed'); + } - // Then check for block rules - foreach ($rules as $rule) { - if ($rule->getAction() !== FirewallRuleObject::ACTION_BLOCK) { - continue; - } - - if ($this->ruleMatchesRequest($rule, $ipAddress, $deviceFingerprint)) { $this->publishAccessDenied($ipAddress, $deviceFingerprint, $rule); return new FirewallAnalyzeResult(false, $rule->getId(), $rule->getReason()); } @@ -293,7 +294,8 @@ class FirewallService } $rule = new FirewallRuleObject(); - $rule->setTenantId($tenantId) + $rule->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId($tenantId) ->setType(FirewallRuleObject::TYPE_IP) ->setAction(FirewallRuleObject::ACTION_BLOCK) ->setValue($ipAddress) @@ -335,7 +337,8 @@ class FirewallService } $rule = new FirewallRuleObject(); - $rule->setTenantId($tenantId) + $rule->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId($tenantId) ->setType(FirewallRuleObject::TYPE_IP) ->setAction(FirewallRuleObject::ACTION_ALLOW) ->setValue($ipAddress) @@ -371,7 +374,8 @@ class FirewallService } $rule = new FirewallRuleObject(); - $rule->setTenantId($tenantId) + $rule->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId($tenantId) ->setType(FirewallRuleObject::TYPE_IP_RANGE) ->setAction(FirewallRuleObject::ACTION_BLOCK) ->setValue($cidr) @@ -401,7 +405,8 @@ class FirewallService } $rule = new FirewallRuleObject(); - $rule->setTenantId($tenantId) + $rule->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId($tenantId) ->setType(FirewallRuleObject::TYPE_DEVICE) ->setAction(FirewallRuleObject::ACTION_BLOCK) ->setValue($fingerprint) @@ -551,13 +556,16 @@ class FirewallService */ private function getActiveRules(): array { - if ($this->rulesCache === null) { - $tenantId = $this->tenantContext->identifier(); - $this->rulesCache = $tenantId - ? $this->store->listRules($tenantId, true) - : []; + $tenantId = $this->tenantContext->identifier(); + if (!$tenantId) { + return []; } - return $this->rulesCache; + + if (!array_key_exists($tenantId, $this->rulesCache)) { + $this->rulesCache[$tenantId] = $this->store->listApplicableRules($tenantId); + } + + return $this->rulesCache[$tenantId]; } /** @@ -565,7 +573,7 @@ class FirewallService */ private function clearRulesCache(): void { - $this->rulesCache = null; + $this->rulesCache = []; } /** diff --git a/core/lib/Stores/FirewallStore.php b/core/lib/Stores/FirewallStore.php index 728bea8..723eaad 100644 --- a/core/lib/Stores/FirewallStore.php +++ b/core/lib/Stores/FirewallStore.php @@ -29,14 +29,19 @@ class FirewallStore */ public function listRules(string $tenantId, bool $activeOnly = true): array { - $filter = ['tenantId' => $tenantId]; + $filter = [ + 'tenantId' => $tenantId, + 'scope' => FirewallRuleObject::SCOPE_TENANT, + ]; if ($activeOnly) { $filter['enabled'] = true; - $filter['$or'] = [ + $filter['$and'] = [[ + '$or' => [ ['expiresAt' => null], ['expiresAt' => ['$gt' => (new \DateTimeImmutable())->format(\DateTimeInterface::ATOM)]] - ]; + ], + ]]; } $cursor = $this->dataStore->selectCollection(self::RULES_COLLECTION)->find($filter); @@ -50,6 +55,43 @@ class FirewallStore return $list; } + /** + * List active system-wide rules and active rules owned by a tenant. + */ + public function listApplicableRules(string $tenantId): array + { + $now = (new \DateTimeImmutable())->format(\DateTimeInterface::ATOM); + $filter = [ + 'enabled' => true, + '$and' => [ + [ + '$or' => [ + ['scope' => FirewallRuleObject::SCOPE_SYSTEM], + [ + 'tenantId' => $tenantId, + 'scope' => FirewallRuleObject::SCOPE_TENANT, + ], + ], + ], + [ + '$or' => [ + ['expiresAt' => null], + ['expiresAt' => ['$gt' => $now]], + ], + ], + ], + ]; + + $cursor = $this->dataStore->selectCollection(self::RULES_COLLECTION)->find($filter); + $list = []; + + foreach ($cursor as $entry) { + $list[] = (new FirewallRuleObject())->jsonDeserialize((array)$entry); + } + + return $list; + } + /** * Find rules by IP address */ @@ -118,15 +160,33 @@ class FirewallStore /** * Check if exact IP rule exists */ - public function findExactIpRule(string $tenantId, string $ipAddress, string $action): ?FirewallRuleObject + public function findExactIpRule( + ?string $tenantId, + string $ipAddress, + string $action, + string $scope = FirewallRuleObject::SCOPE_TENANT + ): ?FirewallRuleObject { - $entry = $this->dataStore->selectCollection(self::RULES_COLLECTION)->findOne([ - 'tenantId' => $tenantId, + $filter = [ 'type' => FirewallRuleObject::TYPE_IP, 'value' => $ipAddress, 'action' => $action, 'enabled' => true, - ]); + '$or' => [ + ['expiresAt' => null], + ['expiresAt' => ['$gt' => (new \DateTimeImmutable())->format(\DateTimeInterface::ATOM)]], + ], + ]; + + if ($scope === FirewallRuleObject::SCOPE_SYSTEM) { + $filter['scope'] = FirewallRuleObject::SCOPE_SYSTEM; + $filter['tenantId'] = null; + } else { + $filter['tenantId'] = $tenantId; + $filter['scope'] = FirewallRuleObject::SCOPE_TENANT; + } + + $entry = $this->dataStore->selectCollection(self::RULES_COLLECTION)->findOne($filter); if (!$entry) { return null; @@ -139,6 +199,8 @@ class FirewallStore */ public function depositRule(FirewallRuleObject $rule): ?FirewallRuleObject { + $rule->assertValidScopeOwnership(); + if ($rule->getId()) { return $this->updateRule($rule); } else { diff --git a/tests/php/Unit/Models/Firewall/FirewallRuleObjectTest.php b/tests/php/Unit/Models/Firewall/FirewallRuleObjectTest.php new file mode 100644 index 0000000..40ab786 --- /dev/null +++ b/tests/php/Unit/Models/Firewall/FirewallRuleObjectTest.php @@ -0,0 +1,55 @@ +expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('explicit scope'); + + (new FirewallRuleObject())->jsonDeserialize([ + 'tenantId' => 'tenant-a', + 'type' => FirewallRuleObject::TYPE_IP, + 'action' => FirewallRuleObject::ACTION_BLOCK, + 'value' => '203.0.113.10', + ]); + } + + #[TestDox('Unknown rule scopes are rejected')] + public function testUnknownScope(): void + { + $this->expectException(\InvalidArgumentException::class); + + (new FirewallRuleObject())->setScope('unknown'); + } + + #[TestDox('Tenant rules require a tenant ID')] + public function testTenantOwnership(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('require a tenant ID'); + + (new FirewallRuleObject())->assertValidScopeOwnership(); + } + + #[TestDox('System rules cannot have a tenant ID')] + public function testSystemOwnership(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('cannot have a tenant ID'); + + (new FirewallRuleObject()) + ->setScope(FirewallRuleObject::SCOPE_SYSTEM) + ->setTenantId('tenant-a') + ->assertValidScopeOwnership(); + } +} diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php new file mode 100644 index 0000000..14d28c3 --- /dev/null +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -0,0 +1,145 @@ +store = $this->createMock(FirewallStore::class); + $this->tenantContext = $this->createMock(TenantContextInterface::class); + $this->events = $this->createMock(EventDispatcherInterface::class); + $this->currentTenant = 'tenant-a'; + $this->tenantContext->method('identifier')->willReturnCallback( + fn(): string => $this->currentTenant + ); + $this->tenantContext->method('configuration')->willReturn(null); + $this->service = new FirewallService($this->store, $this->tenantContext, $this->events); + } + + #[TestDox('System blocks cannot be overridden by tenant allows')] + public function testSystemBlockPrecedence(): void + { + $systemBlock = $this->rule( + 'system-block', + FirewallRuleObject::SCOPE_SYSTEM, + FirewallRuleObject::ACTION_BLOCK, + null + ); + $tenantAllow = $this->rule( + 'tenant-allow', + FirewallRuleObject::SCOPE_TENANT, + FirewallRuleObject::ACTION_ALLOW, + 'tenant-a' + ); + + $this->store->expects($this->once()) + ->method('listApplicableRules') + ->with('tenant-a') + ->willReturn([$tenantAllow, $systemBlock]); + $this->events->expects($this->once())->method('dispatch'); + + $result = $this->service->analyze('203.0.113.10'); + + self::assertTrue($result->isBlocked()); + self::assertSame('system-block', $result->ruleId); + } + + #[TestDox('Tenant blocks override system allows')] + public function testTenantBlockPrecedence(): void + { + $systemAllow = $this->rule( + 'system-allow', + FirewallRuleObject::SCOPE_SYSTEM, + FirewallRuleObject::ACTION_ALLOW, + null + ); + $tenantBlock = $this->rule( + 'tenant-block', + FirewallRuleObject::SCOPE_TENANT, + FirewallRuleObject::ACTION_BLOCK, + 'tenant-a' + ); + + $this->store->method('listApplicableRules')->willReturn([$systemAllow, $tenantBlock]); + $this->events->expects($this->once())->method('dispatch'); + + $result = $this->service->analyze('203.0.113.10'); + + self::assertTrue($result->isBlocked()); + self::assertSame('tenant-block', $result->ruleId); + } + + #[TestDox('Rule caches are isolated by tenant')] + public function testTenantCacheIsolation(): void + { + $this->store->expects($this->exactly(2)) + ->method('listApplicableRules') + ->willReturnCallback(static fn(string $tenantId): array => [ + (new FirewallRuleObject()) + ->setId($tenantId) + ->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId($tenantId) + ->setType(FirewallRuleObject::TYPE_IP) + ->setAction(FirewallRuleObject::ACTION_BLOCK) + ->setValue('203.0.113.10'), + ]); + + self::assertSame('tenant-a', $this->service->analyze('203.0.113.10')->ruleId); + $this->currentTenant = 'tenant-b'; + self::assertSame('tenant-b', $this->service->analyze('203.0.113.10')->ruleId); + } + + #[TestDox('New IP blocks are explicitly tenant-scoped')] + public function testIpBlockScope(): void + { + $this->store->method('findExactIpRule')->willReturn(null); + $this->store->expects($this->once()) + ->method('depositRule') + ->with(self::callback(static function (FirewallRuleObject $rule): bool { + return $rule->getScope() === FirewallRuleObject::SCOPE_TENANT + && $rule->getTenantId() === 'tenant-a'; + })) + ->willReturnArgument(0); + + $rule = $this->service->blockIp('203.0.113.10'); + + self::assertSame(FirewallRuleObject::SCOPE_TENANT, $rule->getScope()); + self::assertSame('tenant-a', $rule->getTenantId()); + } + + private function rule( + string $id, + string $scope, + string $action, + ?string $tenantId + ): FirewallRuleObject { + return (new FirewallRuleObject()) + ->setId($id) + ->setScope($scope) + ->setTenantId($tenantId) + ->setType(FirewallRuleObject::TYPE_IP) + ->setAction($action) + ->setValue('203.0.113.10') + ->setReason($id); + } +}