diff --git a/module/CLI/src/ApiKey/RoleResolver.php b/module/CLI/src/ApiKey/RoleResolver.php index 5a60120f..4df10bf4 100644 --- a/module/CLI/src/ApiKey/RoleResolver.php +++ b/module/CLI/src/ApiKey/RoleResolver.php @@ -32,7 +32,7 @@ class RoleResolver implements RoleResolverInterface $roleDefinitions[] = $this->resolveRoleForAuthority($domainAuthority); } if ($noOrphanVisits) { - $roleDefinitions[] = RoleDefinition::forOrphanVisitsExcluded(); + $roleDefinitions[] = RoleDefinition::forNoOrphanVisits(); } return $roleDefinitions; diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index d8e80e03..cca71a14 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -262,7 +262,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded())); + $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forNoOrphanVisits())); $this->getEntityManager()->persist($noOrphanVisitsApiKey); $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); @@ -330,7 +330,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); - $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded())); + $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forNoOrphanVisits())); $this->getEntityManager()->persist($noOrphanVisitsApiKey); $botsCount = 3; diff --git a/module/Core/test/Visit/VisitsDeleterTest.php b/module/Core/test/Visit/VisitsDeleterTest.php index eddc7767..e47706a9 100644 --- a/module/Core/test/Visit/VisitsDeleterTest.php +++ b/module/Core/test/Visit/VisitsDeleterTest.php @@ -48,7 +48,7 @@ class VisitsDeleterTest extends TestCase $this->repo->expects($this->never())->method('deleteOrphanVisits'); $result = $this->visitsDeleter->deleteOrphanVisits( - ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded())), + ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forNoOrphanVisits())), ); self::assertEquals(0, $result->affectedItems); diff --git a/module/Rest/src/ApiKey/Model/RoleDefinition.php b/module/Rest/src/ApiKey/Model/RoleDefinition.php index 819a224c..a35e904c 100644 --- a/module/Rest/src/ApiKey/Model/RoleDefinition.php +++ b/module/Rest/src/ApiKey/Model/RoleDefinition.php @@ -26,7 +26,7 @@ final class RoleDefinition ); } - public static function forOrphanVisitsExcluded(): self + public static function forNoOrphanVisits(): self { return new self(Role::NO_ORPHAN_VISITS, []); } diff --git a/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php b/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php index b7cf59b9..63a2f165 100644 --- a/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php +++ b/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php @@ -10,7 +10,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class DeleteOrphanVisitsTest extends ApiTestCase { #[Test] - public function deletesVisitsForShortUrlWithoutAffectingTheRest(): void + public function deletesOrphanVisitsWithoutAffectingTheRest(): void { self::assertEquals(7, $this->getTotalVisits()); self::assertEquals(3, $this->getOrphanVisits()); @@ -24,6 +24,21 @@ class DeleteOrphanVisitsTest extends ApiTestCase self::assertEquals(0, $this->getOrphanVisits()); } + #[Test] + public function doesNotDeleteOrphanVisitsForRestrictedApiKey(): void + { + self::assertEquals(7, $this->getTotalVisits()); + self::assertEquals(3, $this->getOrphanVisits()); + + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/visits/orphan', apiKey: 'no_orphans_api_key'); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals(0, $payload['deletedVisits']); + self::assertEquals(7, $this->getTotalVisits()); // This verifies that regular visits have not been affected + self::assertEquals(3, $this->getOrphanVisits()); // This verifies that all orphan visits still exist + } + private function getTotalVisits(): int { $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan'); diff --git a/module/Rest/test-api/Action/GlobalVisitsTest.php b/module/Rest/test-api/Action/GlobalVisitsTest.php index 50591a14..657f16a6 100644 --- a/module/Rest/test-api/Action/GlobalVisitsTest.php +++ b/module/Rest/test-api/Action/GlobalVisitsTest.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class GlobalVisitsTest extends ApiTestCase { #[Test, DataProvider('provideApiKeys')] - public function returnsExpectedVisitsStats(string $apiKey, int $expectedVisits): void + public function returnsExpectedVisitsStats(string $apiKey, int $expectedVisits, int $expectedOrphanVisits): void { $resp = $this->callApiWithKey(self::METHOD_GET, '/visits', [], $apiKey); $payload = $this->getJsonResponsePayload($resp); @@ -20,13 +20,14 @@ class GlobalVisitsTest extends ApiTestCase self::assertArrayHasKey('visitsCount', $payload['visits']); self::assertArrayHasKey('orphanVisitsCount', $payload['visits']); self::assertEquals($expectedVisits, $payload['visits']['visitsCount']); - self::assertEquals(3, $payload['visits']['orphanVisitsCount']); + self::assertEquals($expectedOrphanVisits, $payload['visits']['orphanVisitsCount']); } public static function provideApiKeys(): iterable { - yield 'admin API key' => ['valid_api_key', 7]; - yield 'domain API key' => ['domain_api_key', 0]; - yield 'author API key' => ['author_api_key', 5]; + yield 'admin API key' => ['valid_api_key', 7, 3]; + yield 'domain API key' => ['domain_api_key', 0, 3]; + yield 'author API key' => ['author_api_key', 5, 3]; + yield 'no orphans API key' => ['no_orphans_api_key', 7, 0]; } } diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index 2049d80d..2c8b2479 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -69,4 +69,16 @@ class OrphanVisitsTest extends ApiTestCase [self::REGULAR_NOT_FOUND], ]; } + + #[Test] + public function noVisitsAreReturnedForRestrictedApiKey(): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan', apiKey: 'no_orphans_api_key'); + $payload = $this->getJsonResponsePayload($resp); + $visits = $payload['visits']['data'] ?? null; + + self::assertIsArray($visits); + self::assertEmpty($visits); + self::assertEquals(0, $payload['visits']['pagination']['totalItems'] ?? Paginator::ALL_ITEMS); + } } diff --git a/module/Rest/test-api/Fixtures/ApiKeyFixture.php b/module/Rest/test-api/Fixtures/ApiKeyFixture.php index 5ac886ce..ef971d63 100644 --- a/module/Rest/test-api/Fixtures/ApiKeyFixture.php +++ b/module/Rest/test-api/Fixtures/ApiKeyFixture.php @@ -23,21 +23,29 @@ class ApiKeyFixture extends AbstractFixture implements DependentFixtureInterface public function load(ObjectManager $manager): void { - $manager->persist($this->buildApiKey('valid_api_key', true)); - $manager->persist($this->buildApiKey('disabled_api_key', false)); - $manager->persist($this->buildApiKey('expired_api_key', true, Chronos::now()->subDay()->startOfDay())); + $manager->persist($this->buildApiKey('valid_api_key', enabled: true)); + $manager->persist($this->buildApiKey('disabled_api_key', enabled: false)); + $manager->persist($this->buildApiKey( + 'expired_api_key', + enabled: true, + expiresAt: Chronos::now()->subDay()->startOfDay(), + )); - $authorApiKey = $this->buildApiKey('author_api_key', true); + $authorApiKey = $this->buildApiKey('author_api_key', enabled: true); $authorApiKey->registerRole(RoleDefinition::forAuthoredShortUrls()); $manager->persist($authorApiKey); $this->addReference('author_api_key', $authorApiKey); /** @var Domain $exampleDomain */ $exampleDomain = $this->getReference('example_domain'); - $domainApiKey = $this->buildApiKey('domain_api_key', true); + $domainApiKey = $this->buildApiKey('domain_api_key', enabled: true); $domainApiKey->registerRole(RoleDefinition::forDomain($exampleDomain)); $manager->persist($domainApiKey); + $authorApiKey = $this->buildApiKey('no_orphans_api_key', enabled: true); + $authorApiKey->registerRole(RoleDefinition::forNoOrphanVisits()); + $manager->persist($authorApiKey); + $manager->flush(); }