diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index 4e6e4daf..e871d125 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/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, )); diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index 7021e70b..dc54057c 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/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)); } diff --git a/module/Core/src/Visit/VisitsDeleter.php b/module/Core/src/Visit/VisitsDeleter.php index 5a8adca5..2b925e17 100644 --- a/module/Core/src/Visit/VisitsDeleter.php +++ b/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 - return new BulkDeleteResult($this->repository->deleteOrphanVisits()); + $affectedItems = $apiKey?->hasRole(Role::NO_ORPHAN_VISITS) ? 0 : $this->repository->deleteOrphanVisits(); + return new BulkDeleteResult($affectedItems); } } diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 25f44921..bdd2fd3b 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/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 diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index af6cb77c..71173553 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/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 diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 6eb2fe4e..d8e80e03 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/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))); diff --git a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php index 6a367ed7..04e3f84c 100644 --- a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php +++ b/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( - new VisitsListFiltering($this->params->dateRange, $this->params->excludeBots, null, $limit, $offset), - )->willReturn($list); + $this->repo->expects($this->once())->method('findOrphanVisits')->with(new VisitsListFiltering( + $this->params->dateRange, + $this->params->excludeBots, + $this->apiKey, + $limit, + $offset, + ))->willReturn($list); $result = $this->adapter->getSlice($offset, $limit); diff --git a/module/Core/test/Visit/VisitsDeleterTest.php b/module/Core/test/Visit/VisitsDeleterTest.php index 155d0725..eddc7767 100644 --- a/module/Core/test/Visit/VisitsDeleterTest.php +++ b/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); + } } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 18954a22..3fc024df 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/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')] diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index af5292a2..c7adf3a1 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/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), diff --git a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php index f4a22caa..da660d0e 100644 --- a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php +++ b/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']);