fix(firewall): preserve tenant ownership for automatic blocks

Signed-off-by: Sebastian Krupinski <krupinski01@gmail.com>
2026-07-30 22:11:27 -04:00
parent 3b9a5cd5ab
commit 266b364c93
3 changed files with 254 additions and 8 deletions
@@ -0,0 +1,164 @@
+<?php
+
+declare(strict_types=1);
+
+namespace KTXT\Integration\Stores;
+
+use KTXC\Context\TenantContextInterface;
+use KTXC\Db\DataStore;
+use KTXC\Models\Firewall\FirewallRuleObject;
+use KTXC\Service\FirewallService;
+use KTXC\Stores\FirewallStore;
+use KTXF\Event\EventDispatcherInterface;
+use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations;
+use PHPUnit\Framework\Attributes\TestDox;
+use PHPUnit\Framework\TestCase;
+
+#[AllowMockObjectsWithoutExpectations]
+class FirewallStoreTest extends TestCase
+{
+    private DataStore $dataStore;
+    private FirewallStore $store;
+    private bool $databaseAvailable = false;
+
+    protected function setUp(): void
+    {
+        $system = require dirname(__DIR__, 4).'/config/system.php';
+        $database = $system['database'];
+        $database['database'] = sprintf('ktrix_firewall_test_%d', getmypid());
+
+        $this->dataStore = new DataStore($database);
+        try {
+            $this->dataStore->getDatabase()->drop();
+            $this->databaseAvailable = true;
+        } catch (\MongoDB\Driver\Exception\Exception $error) {
+            self::markTestSkipped('MongoDB is unavailable: '.$error->getMessage());
+        }
+        $this->store = new FirewallStore($this->dataStore);
+    }
+
+    protected function tearDown(): void
+    {
+        if ($this->databaseAvailable) {
+            $this->dataStore->getDatabase()->drop();
+        }
+    }
+
+    #[TestDox('Applicable rules contain system rules and only the requested tenant rules')]
+    public function testApplicableScopes(): void
+    {
+        $tenantA = $this->rule('tenant-a', FirewallRuleObject::SCOPE_TENANT, 'tenant-a');
+        $tenantB = $this->rule('tenant-b', FirewallRuleObject::SCOPE_TENANT, 'tenant-b');
+        $system = $this->rule('system', FirewallRuleObject::SCOPE_SYSTEM);
+        $expiredSystem = $this->rule('expired-system', FirewallRuleObject::SCOPE_SYSTEM)
+            ->setExpiresAt(new \DateTimeImmutable('-1 minute'));
+        $disabledSystem = $this->rule('disabled-system', FirewallRuleObject::SCOPE_SYSTEM)
+            ->setEnabled(false);
+
+        foreach ([$tenantA, $tenantB, $system, $expiredSystem, $disabledSystem] as $rule) {
+            $this->store->depositRule($rule);
+        }
+
+        $rules = $this->store->listApplicableRules('tenant-a');
+        $reasons = array_map(static fn(FirewallRuleObject $rule): ?string => $rule->getReason(), $rules);
+        sort($reasons);
+
+        self::assertSame(['system', 'tenant-a'], $reasons);
+    }
+
+    #[TestDox('Tenant rule listings cannot expose system or other tenant rules')]
+    public function testTenantListingIsolation(): void
+    {
+        $this->store->depositRule($this->rule('tenant-a', FirewallRuleObject::SCOPE_TENANT, 'tenant-a'));
+        $this->store->depositRule($this->rule('tenant-b', FirewallRuleObject::SCOPE_TENANT, 'tenant-b'));
+        $this->store->depositRule($this->rule('system', FirewallRuleObject::SCOPE_SYSTEM));
+
+        $rules = $this->store->listRules('tenant-a');
+
+        self::assertCount(1, $rules);
+        self::assertSame('tenant-a', $rules[0]->getReason());
+        self::assertSame(FirewallRuleObject::SCOPE_TENANT, $rules[0]->getScope());
+    }
+
+    #[TestDox('Expired exact IP rules do not suppress a replacement block')]
+    public function testExpiredBlockReplacement(): void
+    {
+        $expired = $this->rule('expired', FirewallRuleObject::SCOPE_TENANT, 'tenant-a')
+            ->setExpiresAt(new \DateTimeImmutable('-1 minute'));
+        $this->store->depositRule($expired);
+
+        self::assertNull($this->store->findExactIpRule(
+            'tenant-a',
+            '203.0.113.10',
+            FirewallRuleObject::ACTION_BLOCK
+        ));
+
+        $tenantContext = $this->createMock(TenantContextInterface::class);
+        $tenantContext->method('identifier')->willReturn('tenant-a');
+        $tenantContext->method('configuration')->willReturn(null);
+        $events = $this->createMock(EventDispatcherInterface::class);
+        $service = new FirewallService($this->store, $tenantContext, $events);
+
+        $replacement = $service->blockIp('203.0.113.10', durationSeconds: 300);
+
+        self::assertFalse($replacement->isExpired());
+        self::assertNotSame($expired->getId(), $replacement->getId());
+        self::assertSame(
+            $replacement->getId(),
+            $this->store->findExactIpRule(
+                'tenant-a',
+                '203.0.113.10',
+                FirewallRuleObject::ACTION_BLOCK
+            )?->getId()
+        );
+        self::assertCount(2, $this->store->listRules('tenant-a', false));
+    }
+
+    #[TestDox('Exact IP lookups respect tenant and system scope')]
+    public function testExactLookupScope(): void
+    {
+        $tenant = $this->rule('tenant', FirewallRuleObject::SCOPE_TENANT, 'tenant-a');
+        $system = $this->rule('system', FirewallRuleObject::SCOPE_SYSTEM);
+        $this->store->depositRule($tenant);
+        $this->store->depositRule($system);
+
+        self::assertSame(
+            $tenant->getId(),
+            $this->store->findExactIpRule(
+                'tenant-a',
+                '203.0.113.10',
+                FirewallRuleObject::ACTION_BLOCK
+            )?->getId()
+        );
+        self::assertSame(
+            $system->getId(),
+            $this->store->findExactIpRule(
+                null,
+                '203.0.113.10',
+                FirewallRuleObject::ACTION_BLOCK,
+                FirewallRuleObject::SCOPE_SYSTEM
+            )?->getId()
+        );
+        self::assertNull($this->store->findExactIpRule(
+            'tenant-b',
+            '203.0.113.10',
+            FirewallRuleObject::ACTION_BLOCK
+        ));
+    }
+
+    private function rule(
+        string $reason,
+        string $scope,
+        ?string $tenantId = null
+    ): FirewallRuleObject {
+        return (new FirewallRuleObject())
+            ->setScope($scope)
+            ->setTenantId($tenantId)
+            ->setType(FirewallRuleObject::TYPE_IP)
+            ->setAction(FirewallRuleObject::ACTION_BLOCK)
+            ->setValue('203.0.113.10')
+            ->setReason($reason)
+            ->setCreatedAt(new \DateTimeImmutable())
+            ->setEnabled(true);
+    }
+}
@@ -22,7 +22,7 @@ class FirewallServiceTest extends TestCase
    private TenantContextInterface&MockObject $tenantContext;
    private EventDispatcherInterface&MockObject $events;
    private FirewallService $service;
-    private string $currentTenant;
+    private ?string $currentTenant;
    private ?TenantConfiguration $currentConfiguration;

    protected function setUp(): void
@@ -33,7 +33,7 @@ class FirewallServiceTest extends TestCase
        $this->currentTenant = 'tenant-a';
        $this->currentConfiguration = null;
        $this->tenantContext->method('identifier')->willReturnCallback(
-            fn(): string => $this->currentTenant
+            fn(): ?string => $this->currentTenant
        );
        $this->tenantContext->method('configuration')->willReturnCallback(
            fn(): ?TenantConfiguration => $this->currentConfiguration
@@ -238,6 +238,70 @@ class FirewallServiceTest extends TestCase
        $this->service->handleAuthFailure($event);
    }

+    #[TestDox('Automatic blocks retain the tenant carried by the authentication event')]
+    public function testAutomaticBlockTenant(): void
+    {
+        $this->currentTenant = 'tenant-context';
+        $this->store->expects($this->once())
+            ->method('countRecentFailures')
+            ->with('tenant-event', '203.0.113.10', 300)
+            ->willReturn(4);
+        $this->store->expects($this->once())
+            ->method('findExactIpRule')
+            ->with(
+                'tenant-event',
+                '203.0.113.10',
+                FirewallRuleObject::ACTION_BLOCK
+            )
+            ->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-event'
+                    && $rule->getExpiresAt() !== null;
+            }))
+            ->willReturnArgument(0);
+
+        $publishedTenants = [];
+        $this->events->expects($this->exactly(2))
+            ->method('dispatch')
+            ->willReturnCallback(static function (\KTXF\Event\Event $event) use (&$publishedTenants): void {
+                $publishedTenants[] = $event->getTenantId();
+            });
+
+        $event = \KTXF\Event\SecurityEvent::authFailure('203.0.113.10');
+        $event->setTenantId('tenant-event');
+        $this->service->handleAuthFailure($event);
+
+        self::assertSame(['tenant-event', 'tenant-event'], $publishedTenants);
+    }
+
+    #[TestDox('Authentication events without a tenant use the current tenant')]
+    public function testAutomaticBlockTenantFallback(): void
+    {
+        $this->store->expects($this->once())
+            ->method('countRecentFailures')
+            ->with('tenant-a', '203.0.113.10', 300)
+            ->willReturn(0);
+
+        $this->service->handleAuthFailure(
+            \KTXF\Event\SecurityEvent::authFailure('203.0.113.10')
+        );
+    }
+
+    #[TestDox('Authentication failures are ignored when no tenant can be resolved')]
+    public function testAutomaticBlockWithoutTenant(): void
+    {
+        $this->currentTenant = null;
+        $this->store->expects($this->never())->method('countRecentFailures');
+        $this->store->expects($this->never())->method('depositRule');
+
+        $this->service->handleAuthFailure(
+            \KTXF\Event\SecurityEvent::authFailure('203.0.113.10')
+        );
+    }
+
    private function rule(
        string $id,
        string $scope,