Restrict interaction with orphan visits when API key has that role

module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php

@@ -9,11 +9,15 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
 use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter
 {
-    public function __construct(private readonly VisitRepositoryInterface $repo, private readonly VisitsParams $params)
+    public function __construct(
-    {
+        private readonly VisitRepositoryInterface $repo,
+        private readonly VisitsParams $params,
+        private readonly ?ApiKey $apiKey,
+    ) {
     }
 
     protected function doCount(): int
@@ -21,6 +25,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte
         return $this->repo->countOrphanVisits(new VisitsCountFiltering(
             dateRange: $this->params->dateRange,
             excludeBots: $this->params->excludeBots,
+            apiKey: $this->apiKey,
         ));
     }
 
@@ -29,6 +34,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte
         return $this->repo->findOrphanVisits(new VisitsListFiltering(
             dateRange: $this->params->dateRange,
             excludeBots: $this->params->excludeBots,
+            apiKey: $this->apiKey,
             limit: $length,
             offset: $offset,
         ));

module/Core/src/Visit/Repository/VisitRepository.php

@@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
 use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits;
 use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits;
+use Shlinkio\Shlink\Rest\ApiKey\Role;
 
 use const PHP_INT_MAX;
 
@@ -139,6 +140,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
 
     public function findOrphanVisits(VisitsListFiltering $filtering): array
     {
+        if ($filtering->apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) {
+            return [];
+        }
+
         $qb = $this->createAllVisitsQueryBuilder($filtering);
         $qb->andWhere($qb->expr()->isNull('v.shortUrl'));
         return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset);
@@ -146,6 +151,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
 
     public function countOrphanVisits(VisitsCountFiltering $filtering): int
     {
+        if ($filtering->apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) {
+            return 0;
+        }
+
         return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering));
     }
 

module/Core/src/Visit/VisitsDeleter.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Visit;
 
 use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
 use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface;
+use Shlinkio\Shlink\Rest\ApiKey\Role;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class VisitsDeleter implements VisitsDeleterInterface
@@ -16,7 +17,7 @@ class VisitsDeleter implements VisitsDeleterInterface
 
     public function deleteOrphanVisits(?ApiKey $apiKey = null): BulkDeleteResult
     {
-        // TODO Check API key has permissions for orphan visits
+        $affectedItems = $apiKey?->hasRole(Role::NO_ORPHAN_VISITS) ? 0 : $this->repository->deleteOrphanVisits();
-        return new BulkDeleteResult($this->repository->deleteOrphanVisits());
+        return new BulkDeleteResult($affectedItems);
     }
 }

module/Core/src/Visit/VisitsStatsHelper.php

@@ -43,11 +43,13 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface
 
         return new VisitsStats(
             nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)),
-            orphanVisitsTotal: $visitsRepo->countOrphanVisits(new VisitsCountFiltering()),
+            orphanVisitsTotal: $visitsRepo->countOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)),
             nonOrphanVisitsNonBots: $visitsRepo->countNonOrphanVisits(
                 new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
             ),
-            orphanVisitsNonBots: $visitsRepo->countOrphanVisits(new VisitsCountFiltering(excludeBots: true)),
+            orphanVisitsNonBots: $visitsRepo->countOrphanVisits(
+                new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
+            ),
         );
     }
 
@@ -114,12 +116,12 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface
     /**
      * @return Visit[]|Paginator
      */
-    public function orphanVisits(VisitsParams $params): Paginator
+    public function orphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator
     {
         /** @var VisitRepositoryInterface $repo */
         $repo = $this->em->getRepository(Visit::class);
 
-        return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params), $params);
+        return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params);
     }
 
     public function nonOrphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator

module/Core/src/Visit/VisitsStatsHelperInterface.php

@@ -43,7 +43,7 @@ interface VisitsStatsHelperInterface
     /**
      * @return Visit[]|Paginator
      */
-    public function orphanVisits(VisitsParams $params): Paginator;
+    public function orphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator;
 
     /**
      * @return Visit[]|Paginator

module/Core/test-db/Visit/Repository/VisitRepositoryTest.php

@@ -262,6 +262,9 @@ class VisitRepositoryTest extends DatabaseTestCase
 
         $this->getEntityManager()->flush();
 
+        $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded()));
+        $this->getEntityManager()->persist($noOrphanVisitsApiKey);
+
         $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()));
         $this->getEntityManager()->persist($apiKey1);
         $shortUrl = ShortUrl::create(
@@ -305,6 +308,7 @@ class VisitRepositoryTest extends DatabaseTestCase
         self::assertEquals(4, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey1)));
         self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey2)));
         self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($domainApiKey)));
+        self::assertEquals(0, $this->repo->countOrphanVisits(VisitsCountFiltering::withApiKey($noOrphanVisitsApiKey)));
         self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since(
             Chronos::parse('2016-01-05')->startOfDay(),
         ))));
@@ -326,6 +330,9 @@ class VisitRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($shortUrl);
         $this->createVisitsForShortUrl($shortUrl, 7);
 
+        $noOrphanVisitsApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded()));
+        $this->getEntityManager()->persist($noOrphanVisitsApiKey);
+
         $botsCount = 3;
         for ($i = 0; $i < 6; $i++) {
             $this->getEntityManager()->persist($this->setDateOnVisit(
@@ -346,6 +353,7 @@ class VisitRepositoryTest extends DatabaseTestCase
 
         $this->getEntityManager()->flush();
 
+        self::assertCount(0, $this->repo->findOrphanVisits(new VisitsListFiltering(apiKey: $noOrphanVisitsApiKey)));
         self::assertCount(18, $this->repo->findOrphanVisits(new VisitsListFiltering()));
         self::assertCount(15, $this->repo->findOrphanVisits(new VisitsListFiltering(null, true)));
         self::assertCount(5, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 5)));

module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php

@@ -15,18 +15,22 @@ use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
 use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class OrphanVisitsPaginatorAdapterTest extends TestCase
 {
     private OrphanVisitsPaginatorAdapter $adapter;
     private MockObject & VisitRepositoryInterface $repo;
     private VisitsParams $params;
+    private ApiKey $apiKey;
 
     protected function setUp(): void
     {
         $this->repo = $this->createMock(VisitRepositoryInterface::class);
         $this->params = VisitsParams::fromRawData([]);
-        $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo, $this->params);
+        $this->apiKey = ApiKey::create();
+
+        $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey);
     }
 
     #[Test]
@@ -34,7 +38,7 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase
     {
         $expectedCount = 5;
         $this->repo->expects($this->once())->method('countOrphanVisits')->with(
-            new VisitsCountFiltering($this->params->dateRange),
+            new VisitsCountFiltering($this->params->dateRange, apiKey: $this->apiKey),
         )->willReturn($expectedCount);
 
         $result = $this->adapter->getNbResults();
@@ -51,9 +55,13 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase
     {
         $visitor = Visitor::emptyInstance();
         $list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)];
-        $this->repo->expects($this->once())->method('findOrphanVisits')->with(
+        $this->repo->expects($this->once())->method('findOrphanVisits')->with(new VisitsListFiltering(
-            new VisitsListFiltering($this->params->dateRange, $this->params->excludeBots, null, $limit, $offset),
+            $this->params->dateRange,
-        )->willReturn($list);
+            $this->params->excludeBots,
+            $this->apiKey,
+            $limit,
+            $offset,
+        ))->willReturn($list);
 
         $result = $this->adapter->getSlice($offset, $limit);
 

module/Core/test/Visit/VisitsDeleterTest.php

@@ -10,6 +10,9 @@ use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface;
 use Shlinkio\Shlink\Core\Visit\VisitsDeleter;
+use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
+use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class VisitsDeleterTest extends TestCase
 {
@@ -38,4 +41,16 @@ class VisitsDeleterTest extends TestCase
         yield '5000' => [5000];
         yield '0' => [0];
     }
+
+    #[Test]
+    public function returnsNoDeletedVisitsForApiKeyWithNoPermission(): void
+    {
+        $this->repo->expects($this->never())->method('deleteOrphanVisits');
+
+        $result = $this->visitsDeleter->deleteOrphanVisits(
+            ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forOrphanVisitsExcluded())),
+        );
+
+        self::assertEquals(0, $result->affectedItems);
+    }
 }

module/Core/test/Visit/VisitsStatsHelperTest.php

@@ -50,13 +50,14 @@ class VisitsStatsHelperTest extends TestCase
     }
 
     #[Test, DataProvider('provideCounts')]
-    public function returnsExpectedVisitsStats(int $expectedCount): void
+    public function returnsExpectedVisitsStats(int $expectedCount, ?ApiKey $apiKey): void
     {
         $repo = $this->createMock(VisitRepository::class);
         $callCount = 0;
         $repo->expects($this->exactly(2))->method('countNonOrphanVisits')->willReturnCallback(
-            function (VisitsCountFiltering $options) use ($expectedCount, &$callCount) {
+            function (VisitsCountFiltering $options) use ($expectedCount, $apiKey, &$callCount) {
                 Assert::assertEquals($callCount !== 0, $options->excludeBots);
+                Assert::assertEquals($apiKey, $options->apiKey);
                 $callCount++;
 
                 return $expectedCount * 3;
@@ -67,14 +68,17 @@ class VisitsStatsHelperTest extends TestCase
         )->willReturn($expectedCount);
         $this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo);
 
-        $stats = $this->helper->getVisitsStats();
+        $stats = $this->helper->getVisitsStats($apiKey);
 
         self::assertEquals(new VisitsStats($expectedCount * 3, $expectedCount), $stats);
     }
 
     public static function provideCounts(): iterable
     {
-        return map(range(0, 50, 5), fn (int $value) => [$value]);
+        return [
+            ...map(range(0, 50, 5), fn (int $value) => [$value, null]),
+            ...map(range(0, 18, 3), fn (int $value) => [$value, ApiKey::create()]),
+        ];
     }
 
     #[Test, DataProvider('provideAdminApiKeys')]

module/Rest/src/Action/Visit/OrphanVisitsAction.php

@@ -12,6 +12,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
 use Shlinkio\Shlink\Core\Visit\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
+use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
 
 class OrphanVisitsAction extends AbstractRestAction
 {
@@ -29,7 +30,8 @@ class OrphanVisitsAction extends AbstractRestAction
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
         $params = VisitsParams::fromRawData($request->getQueryParams());
-        $visits = $this->visitsHelper->orphanVisits($params);
+        $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
+        $visits = $this->visitsHelper->orphanVisits($params, $apiKey);
 
         return new JsonResponse([
             'visits' => $this->serializePaginator($visits, $this->orphanVisitTransformer),

module/Rest/test/Action/Visit/OrphanVisitsActionTest.php

@@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor;
 use Shlinkio\Shlink\Core\Visit\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface;
 use Shlinkio\Shlink\Rest\Action\Visit\OrphanVisitsAction;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 use function count;
 
@@ -48,7 +49,9 @@ class OrphanVisitsActionTest extends TestCase
         )->willReturn([]);
 
         /** @var JsonResponse $response */
-        $response = $this->action->handle(ServerRequestFactory::fromGlobals());
+        $response = $this->action->handle(
+            ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()),
+        );
         $payload = $response->getPayload();
 
         self::assertCount($visitsAmount, $payload['visits']['data']);