From 74696bbeb37b38186ad71e2bff74ae34a0c89a4d Mon Sep 17 00:00:00 2001 From: Sebastian Krupinski Date: Mon, 3 Aug 2026 22:05:00 -0400 Subject: [PATCH] feat(firewall): add scoped rule administration reads Signed-off-by: Sebastian Krupinski --- core/lib/Controllers/FirewallController.php | 121 ++++++++++++++++++ core/lib/Service/FirewallRuleManager.php | 56 ++++++++ .../lib/Service/SystemFirewallRuleService.php | 17 +++ .../lib/Service/TenantFirewallRuleService.php | 23 ++++ core/lib/Stores/FirewallStore.php | 59 +++++++++ .../Integration/Stores/FirewallStoreTest.php | 36 ++++++ .../Controllers/FirewallControllerTest.php | 88 +++++++++++++ .../Unit/Service/FirewallRuleManagerTest.php | 53 ++++++++ .../Unit/Service/FirewallRuleServicesTest.php | 33 +++++ 9 files changed, 486 insertions(+) create mode 100644 core/lib/Controllers/FirewallController.php create mode 100644 tests/php/Unit/Controllers/FirewallControllerTest.php diff --git a/core/lib/Controllers/FirewallController.php b/core/lib/Controllers/FirewallController.php new file mode 100644 index 0000000..13832c6 --- /dev/null +++ b/core/lib/Controllers/FirewallController.php @@ -0,0 +1,121 @@ +queryResponse( + fn(int $parsedLimit, int $parsedOffset): array => $this->tenantRules->queryRules( + $status, + $type, + $action, + $parsedLimit, + $parsedOffset + ), + $limit, + $offset + ); + } + + #[AuthenticatedRoute( + '/firewall/rules/{ruleId}', + name: 'firewall.tenant.rules.fetch', + permissions: [TenantFirewallRuleService::PERMISSION_READ], + )] + public function tenantRule(string $ruleId): JsonResponse + { + return $this->ruleResponse($this->tenantRules->fetchRule($ruleId)); + } + + #[AuthenticatedRoute( + '/firewall/effective-policy', + name: 'firewall.tenant.policy.effective', + permissions: [TenantFirewallRuleService::PERMISSION_READ], + )] + public function effectivePolicy(): JsonResponse + { + return new JsonResponse($this->tenantRules->effectivePolicy()); + } + + #[AuthenticatedRoute( + '/firewall/system/rules', + name: 'firewall.system.rules.list', + permissions: [SystemFirewallRuleService::PERMISSION_READ], + )] + public function systemRules( + string $status = 'active', + ?string $type = null, + ?string $action = null, + string $limit = '50', + string $offset = '0' + ): JsonResponse { + return $this->queryResponse( + fn(int $parsedLimit, int $parsedOffset): array => $this->systemRules->queryRules( + $status, + $type, + $action, + $parsedLimit, + $parsedOffset + ), + $limit, + $offset + ); + } + + #[AuthenticatedRoute( + '/firewall/system/rules/{ruleId}', + name: 'firewall.system.rules.fetch', + permissions: [SystemFirewallRuleService::PERMISSION_READ], + )] + public function systemRule(string $ruleId): JsonResponse + { + return $this->ruleResponse($this->systemRules->fetchRule($ruleId)); + } + + private function queryResponse(callable $query, string $limit, string $offset): JsonResponse + { + try { + if (!ctype_digit($limit) || !ctype_digit($offset)) { + throw new \InvalidArgumentException('Pagination values must be non-negative integers.'); + } + return new JsonResponse($query((int)$limit, (int)$offset)); + } catch (\InvalidArgumentException $error) { + return new JsonResponse(['error' => $error->getMessage()], JsonResponse::HTTP_BAD_REQUEST); + } + } + + private function ruleResponse(?\JsonSerializable $rule): JsonResponse + { + if ($rule === null) { + return new JsonResponse(['error' => 'Firewall rule not found.'], JsonResponse::HTTP_NOT_FOUND); + } + + return new JsonResponse($rule); + } +} diff --git a/core/lib/Service/FirewallRuleManager.php b/core/lib/Service/FirewallRuleManager.php index edde77f..b7ae416 100644 --- a/core/lib/Service/FirewallRuleManager.php +++ b/core/lib/Service/FirewallRuleManager.php @@ -11,6 +11,8 @@ use KTXF\Event\SecurityEvent; final class FirewallRuleManager { + public const QUERY_STATUSES = ['active', 'disabled', 'expired', 'all']; + public const MAX_QUERY_LIMIT = 100; public const ORIGIN_MANUAL = 'manual'; public const ORIGIN_AUTOMATIC = 'automatic'; @@ -28,6 +30,60 @@ final class FirewallRuleManager : $this->store->listRules($scope->tenantId, $activeOnly); } + public function query( + FirewallRuleScope $scope, + string $status = 'active', + ?string $type = null, + ?string $action = null, + int $limit = 50, + int $offset = 0 + ): array { + if (!in_array($status, self::QUERY_STATUSES, true)) { + throw new \InvalidArgumentException('Invalid rule status filter.'); + } + if ($type !== null && !in_array($type, [ + FirewallRuleObject::TYPE_IP, + FirewallRuleObject::TYPE_IP_RANGE, + FirewallRuleObject::TYPE_DEVICE, + ], true)) { + throw new \InvalidArgumentException('Invalid rule type filter.'); + } + if ($action !== null && !in_array($action, [ + FirewallRuleObject::ACTION_ALLOW, + FirewallRuleObject::ACTION_BLOCK, + ], true)) { + throw new \InvalidArgumentException('Invalid rule action filter.'); + } + if ($limit < 1 || $limit > self::MAX_QUERY_LIMIT || $offset < 0) { + throw new \InvalidArgumentException('Pagination requires limit 1-100 and offset 0 or greater.'); + } + + return $this->store->queryRules( + $scope->scope, + $scope->tenantId, + $status, + $type, + $action, + $limit, + $offset + ); + } + + public function fetch(FirewallRuleScope $scope, string $ruleId): ?FirewallRuleObject + { + return $this->ownedRule($scope, $ruleId); + } + + /** @return array{precedence: string[], system: FirewallRuleObject[], tenant: FirewallRuleObject[]} */ + public function effectivePolicy(string $tenantId): array + { + return [ + 'precedence' => ['system_block', 'tenant_allow', 'tenant_block', 'system_allow', 'default_allow'], + 'system' => $this->store->listSystemRules(), + 'tenant' => $this->store->listRules($tenantId), + ]; + } + public function blockIp( FirewallRuleScope $scope, string $ipAddress, diff --git a/core/lib/Service/SystemFirewallRuleService.php b/core/lib/Service/SystemFirewallRuleService.php index 6c9cb66..4f49d3b 100644 --- a/core/lib/Service/SystemFirewallRuleService.php +++ b/core/lib/Service/SystemFirewallRuleService.php @@ -24,6 +24,23 @@ final class SystemFirewallRuleService return $this->rules->list(FirewallRuleScope::system(), $activeOnly); } + public function queryRules( + string $status = 'active', + ?string $type = null, + ?string $action = null, + int $limit = 50, + int $offset = 0 + ): array { + $this->requirePermission(self::PERMISSION_READ); + return $this->rules->query(FirewallRuleScope::system(), $status, $type, $action, $limit, $offset); + } + + public function fetchRule(string $ruleId): ?FirewallRuleObject + { + $this->requirePermission(self::PERMISSION_READ); + return $this->rules->fetch(FirewallRuleScope::system(), $ruleId); + } + public function blockIp(string $ip, ?string $reason = null, ?int $durationSeconds = null): FirewallRuleObject { $this->requirePermission(self::PERMISSION_MANAGE); diff --git a/core/lib/Service/TenantFirewallRuleService.php b/core/lib/Service/TenantFirewallRuleService.php index 1a08e0d..fa3d28d 100644 --- a/core/lib/Service/TenantFirewallRuleService.php +++ b/core/lib/Service/TenantFirewallRuleService.php @@ -26,6 +26,29 @@ final class TenantFirewallRuleService return $this->rules->list($this->scope(), $activeOnly); } + public function queryRules( + string $status = 'active', + ?string $type = null, + ?string $action = null, + int $limit = 50, + int $offset = 0 + ): array { + $this->requirePermission(self::PERMISSION_READ); + return $this->rules->query($this->scope(), $status, $type, $action, $limit, $offset); + } + + public function fetchRule(string $ruleId): ?FirewallRuleObject + { + $this->requirePermission(self::PERMISSION_READ); + return $this->rules->fetch($this->scope(), $ruleId); + } + + public function effectivePolicy(): array + { + $this->requirePermission(self::PERMISSION_READ); + return $this->rules->effectivePolicy($this->tenant->requireIdentifier()); + } + public function blockIp(string $ip, ?string $reason = null, ?int $durationSeconds = null): FirewallRuleObject { $this->requirePermission(self::PERMISSION_MANAGE); diff --git a/core/lib/Stores/FirewallStore.php b/core/lib/Stores/FirewallStore.php index eb13bd9..7cfdccf 100644 --- a/core/lib/Stores/FirewallStore.php +++ b/core/lib/Stores/FirewallStore.php @@ -46,6 +46,10 @@ class FirewallStore ['scope' => 1, 'tenantId' => 1, 'type' => 1, 'value' => 1, 'action' => 1, 'enabled' => 1, 'expiresAt' => 1], ['name' => 'rules_exact_lookup'] ), + $rules->createIndex( + ['scope' => 1, 'tenantId' => 1, 'createdAt' => -1], + ['name' => 'rules_browse'] + ), $logs->createIndex( ['tenantId' => 1, 'ipAddress' => 1, 'eventType' => 1, 'timestamp' => -1], ['name' => 'logs_auth_failures'] @@ -73,6 +77,61 @@ class FirewallStore // Rule Operations // ======================================== + /** + * Query rules within one ownership scope. + * + * @return array{items: FirewallRuleObject[], total: int, limit: int, offset: int} + */ + public function queryRules( + string $scope, + ?string $tenantId, + string $status, + ?string $type, + ?string $action, + int $limit, + int $offset + ): array { + $filter = [ + 'scope' => $scope, + 'tenantId' => $scope === FirewallRuleObject::SCOPE_SYSTEM ? null : $tenantId, + ]; + $now = self::bsonDate(new \DateTimeImmutable()); + if ($status === 'active') { + $filter['enabled'] = true; + $filter['$or'] = [ + ['expiresAt' => null], + ['expiresAt' => ['$gt' => $now]], + ]; + } elseif ($status === 'disabled') { + $filter['enabled'] = false; + } elseif ($status === 'expired') { + $filter['expiresAt'] = ['$ne' => null, '$lte' => $now]; + } + if ($type !== null) { + $filter['type'] = $type; + } + if ($action !== null) { + $filter['action'] = $action; + } + + $collection = $this->dataStore->selectCollection(self::RULES_COLLECTION); + $items = []; + foreach ($collection->find($filter, [ + 'sort' => ['createdAt' => -1, '_id' => -1], + 'limit' => $limit, + 'skip' => $offset, + ]) as $entry) { + $items[] = (new FirewallRuleObject())->jsonDeserialize((array)$entry); + } + + return [ + 'items' => $items, + 'total' => $collection->countDocuments($filter), + 'limit' => $limit, + 'offset' => $offset, + ]; + } + /** * List all rules for a tenant */ diff --git a/tests/php/Integration/Stores/FirewallStoreTest.php b/tests/php/Integration/Stores/FirewallStoreTest.php index 553d039..057ff15 100644 --- a/tests/php/Integration/Stores/FirewallStoreTest.php +++ b/tests/php/Integration/Stores/FirewallStoreTest.php @@ -452,6 +452,42 @@ class FirewallStoreTest extends TestCase self::assertContains('logs_auth_failures', self::indexNames($failurePlan)); } + #[TestDox('Administrative rule queries filter, paginate, and preserve scope')] + public function testAdministrativeRuleQuery(): void + { + $this->store->depositRule($this->rule('tenant-active', FirewallRuleObject::SCOPE_TENANT, 'tenant-a')); + $this->store->depositRule( + $this->rule('tenant-disabled', FirewallRuleObject::SCOPE_TENANT, 'tenant-a')->setEnabled(false) + ); + $this->store->depositRule($this->rule('other-tenant', FirewallRuleObject::SCOPE_TENANT, 'tenant-b')); + $this->store->depositRule($this->rule('system', FirewallRuleObject::SCOPE_SYSTEM)); + + $active = $this->store->queryRules( + FirewallRuleObject::SCOPE_TENANT, + 'tenant-a', + 'active', + FirewallRuleObject::TYPE_IP, + FirewallRuleObject::ACTION_BLOCK, + 1, + 0 + ); + $disabled = $this->store->queryRules( + FirewallRuleObject::SCOPE_TENANT, + 'tenant-a', + 'disabled', + null, + null, + 50, + 0 + ); + + self::assertSame(1, $active['total']); + self::assertCount(1, $active['items']); + self::assertSame('tenant-active', $active['items'][0]->getReason()); + self::assertSame(1, $disabled['total']); + self::assertSame('tenant-disabled', $disabled['items'][0]->getReason()); + } + private function rule( string $reason, string $scope, diff --git a/tests/php/Unit/Controllers/FirewallControllerTest.php b/tests/php/Unit/Controllers/FirewallControllerTest.php new file mode 100644 index 0000000..c3b2869 --- /dev/null +++ b/tests/php/Unit/Controllers/FirewallControllerTest.php @@ -0,0 +1,88 @@ +store = $this->createMock(FirewallStore::class); + $tenant = $this->createMock(TenantContextInterface::class); + $tenant->method('requireIdentifier')->willReturn('tenant-a'); + $identity = $this->createMock(IdentityContextInterface::class); + $identity->method('hasPermission')->willReturn(true); + $manager = new FirewallRuleManager( + $this->store, + new FirewallRuleCache($this->store), + $this->createStub(EventDispatcherInterface::class) + ); + $this->controller = new FirewallController( + new TenantFirewallRuleService($manager, $tenant, $identity), + new SystemFirewallRuleService($manager, $identity) + ); + } + + #[TestDox('Tenant rule endpoint returns bounded paginated results')] + public function testTenantRules(): void + { + $this->store->expects(self::once()) + ->method('queryRules') + ->with('tenant', 'tenant-a', 'active', null, null, 25, 10) + ->willReturn(['items' => [], 'total' => 0, 'limit' => 25, 'offset' => 10]); + + $response = $this->controller->tenantRules(limit: '25', offset: '10'); + $data = json_decode($response->getContent(), true, flags: JSON_THROW_ON_ERROR); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame(25, $data['limit']); + self::assertSame(10, $data['offset']); + } + + #[TestDox('Rule endpoints reject malformed and excessive pagination')] + public function testPaginationValidation(): void + { + $this->store->expects(self::never())->method('queryRules'); + + self::assertSame(400, $this->controller->tenantRules(limit: 'invalid')->getStatusCode()); + self::assertSame(400, $this->controller->systemRules(limit: '101')->getStatusCode()); + } + + #[TestDox('Every rule endpoint declares its scope-specific read permission')] + public function testRoutePermissions(): void + { + $expected = [ + 'tenantRules' => TenantFirewallRuleService::PERMISSION_READ, + 'tenantRule' => TenantFirewallRuleService::PERMISSION_READ, + 'effectivePolicy' => TenantFirewallRuleService::PERMISSION_READ, + 'systemRules' => SystemFirewallRuleService::PERMISSION_READ, + 'systemRule' => SystemFirewallRuleService::PERMISSION_READ, + ]; + + foreach ($expected as $method => $permission) { + $attributes = (new \ReflectionMethod(FirewallController::class, $method)) + ->getAttributes(AuthenticatedRoute::class); + self::assertCount(1, $attributes); + self::assertSame([$permission], $attributes[0]->newInstance()->permissions); + } + } +} diff --git a/tests/php/Unit/Service/FirewallRuleManagerTest.php b/tests/php/Unit/Service/FirewallRuleManagerTest.php index 6cd8347..8717780 100644 --- a/tests/php/Unit/Service/FirewallRuleManagerTest.php +++ b/tests/php/Unit/Service/FirewallRuleManagerTest.php @@ -91,6 +91,59 @@ class FirewallRuleManagerTest extends TestCase self::assertFalse($this->manager->remove(FirewallRuleScope::tenant('tenant-b'), 'tenant-rule')); } + #[TestDox('Rule queries retain scope, filters, and bounded pagination')] + public function testRuleQuery(): void + { + $result = ['items' => [], 'total' => 0, 'limit' => 25, 'offset' => 50]; + $this->store->expects(self::once()) + ->method('queryRules') + ->with( + FirewallRuleObject::SCOPE_TENANT, + 'tenant-a', + 'disabled', + FirewallRuleObject::TYPE_IP, + FirewallRuleObject::ACTION_BLOCK, + 25, + 50 + ) + ->willReturn($result); + + self::assertSame($result, $this->manager->query( + FirewallRuleScope::tenant('tenant-a'), + 'disabled', + FirewallRuleObject::TYPE_IP, + FirewallRuleObject::ACTION_BLOCK, + 25, + 50 + )); + } + + #[TestDox('Rule queries reject invalid filters and excessive pages')] + public function testRuleQueryValidation(): void + { + $this->store->expects(self::never())->method('queryRules'); + $this->expectException(\InvalidArgumentException::class); + + $this->manager->query(FirewallRuleScope::system(), 'active', null, null, 101); + } + + #[TestDox('Effective policy keeps system and tenant rule sets distinct')] + public function testEffectivePolicy(): void + { + $system = (new FirewallRuleObject())->setScope(FirewallRuleObject::SCOPE_SYSTEM); + $tenant = (new FirewallRuleObject()) + ->setScope(FirewallRuleObject::SCOPE_TENANT) + ->setTenantId('tenant-a'); + $this->store->method('listSystemRules')->willReturn([$system]); + $this->store->method('listRules')->with('tenant-a')->willReturn([$tenant]); + + $policy = $this->manager->effectivePolicy('tenant-a'); + + self::assertSame([$system], $policy['system']); + self::assertSame([$tenant], $policy['tenant']); + self::assertSame('system_block', $policy['precedence'][0]); + } + #[TestDox('Rule mutations invalidate the shared enforcement cache')] public function testCacheInvalidation(): void { diff --git a/tests/php/Unit/Service/FirewallRuleServicesTest.php b/tests/php/Unit/Service/FirewallRuleServicesTest.php index 5eca47a..2b9bfff 100644 --- a/tests/php/Unit/Service/FirewallRuleServicesTest.php +++ b/tests/php/Unit/Service/FirewallRuleServicesTest.php @@ -57,6 +57,39 @@ class FirewallRuleServicesTest extends TestCase $this->systemService()->blockIp('203.0.113.10'); } + #[TestDox('Read operations require their scope-specific permission')] + public function testReadPermissions(): void + { + $this->identity->method('hasPermission')->willReturn(false); + $this->store->expects(self::never())->method('queryRules'); + + try { + $this->tenantService()->queryRules(); + self::fail('Tenant read should have been rejected.'); + } catch (\RuntimeException $error) { + self::assertStringContainsString(TenantFirewallRuleService::PERMISSION_READ, $error->getMessage()); + } + + $this->expectExceptionMessage(SystemFirewallRuleService::PERMISSION_READ); + $this->systemService()->queryRules(); + } + + #[TestDox('Tenant reads derive ownership and effective policy from context')] + public function testTenantReads(): void + { + $this->allow(TenantFirewallRuleService::PERMISSION_READ); + $this->tenant->method('requireIdentifier')->willReturn('tenant-a'); + $this->store->expects(self::once()) + ->method('queryRules') + ->with(FirewallRuleObject::SCOPE_TENANT, 'tenant-a', 'active', null, null, 50, 0) + ->willReturn(['items' => [], 'total' => 0, 'limit' => 50, 'offset' => 0]); + $this->store->method('listSystemRules')->willReturn([]); + $this->store->method('listRules')->with('tenant-a')->willReturn([]); + + self::assertSame(0, $this->tenantService()->queryRules()['total']); + self::assertSame([], $this->tenantService()->effectivePolicy()['system']); + } + #[TestDox('Tenant management derives scope from tenant context')] public function testTenantScope(): void {