Merge pull request #1795 from acelaya-forks/feature/non-orphan-role

Feature/non orphan role
This commit is contained in:
Alejandro Celaya 2023-05-31 09:43:18 +02:00 committed by GitHub
commit cb4ba58b08
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
31 changed files with 232 additions and 84 deletions

View file

@ -6,7 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
## [Unreleased] ## [Unreleased]
### Added ### Added
* *Nothing* * [#1780](https://github.com/shlinkio/shlink/issues/1780) Add new `NO_ORPHAN_VISITS` API key role.
Keys with this role will always get `0` when fetching orphan visits.
When trying to delete orphan visits the result will also be `0` and no visits will actually get deleted.
### Changed ### Changed
* *Nothing* * *Nothing*

View file

@ -22,6 +22,7 @@ class RoleResolver implements RoleResolverInterface
{ {
$domainAuthority = $input->getOption(Role::DOMAIN_SPECIFIC->paramName()); $domainAuthority = $input->getOption(Role::DOMAIN_SPECIFIC->paramName());
$author = $input->getOption(Role::AUTHORED_SHORT_URLS->paramName()); $author = $input->getOption(Role::AUTHORED_SHORT_URLS->paramName());
$noOrphanVisits = $input->getOption(Role::NO_ORPHAN_VISITS->paramName());
$roleDefinitions = []; $roleDefinitions = [];
if ($author) { if ($author) {
@ -30,6 +31,9 @@ class RoleResolver implements RoleResolverInterface
if (is_string($domainAuthority)) { if (is_string($domainAuthority)) {
$roleDefinitions[] = $this->resolveRoleForAuthority($domainAuthority); $roleDefinitions[] = $this->resolveRoleForAuthority($domainAuthority);
} }
if ($noOrphanVisits) {
$roleDefinitions[] = RoleDefinition::forNoOrphanVisits();
}
return $roleDefinitions; return $roleDefinitions;
} }

View file

@ -25,8 +25,8 @@ class GenerateKeyCommand extends Command
public const NAME = 'api-key:generate'; public const NAME = 'api-key:generate';
public function __construct( public function __construct(
private ApiKeyServiceInterface $apiKeyService, private readonly ApiKeyServiceInterface $apiKeyService,
private RoleResolverInterface $roleResolver, private readonly RoleResolverInterface $roleResolver,
) { ) {
parent::__construct(); parent::__construct();
} }
@ -35,6 +35,8 @@ class GenerateKeyCommand extends Command
{ {
$authorOnly = Role::AUTHORED_SHORT_URLS->paramName(); $authorOnly = Role::AUTHORED_SHORT_URLS->paramName();
$domainOnly = Role::DOMAIN_SPECIFIC->paramName(); $domainOnly = Role::DOMAIN_SPECIFIC->paramName();
$noOrphanVisits = Role::NO_ORPHAN_VISITS->paramName();
$help = <<<HELP $help = <<<HELP
The <info>%command.name%</info> generates a new valid API key. The <info>%command.name%</info> generates a new valid API key.
@ -52,7 +54,8 @@ class GenerateKeyCommand extends Command
* Can interact with short URLs created with this API key: <info>%command.full_name% --{$authorOnly}</info> * Can interact with short URLs created with this API key: <info>%command.full_name% --{$authorOnly}</info>
* Can interact with short URLs for one domain: <info>%command.full_name% --{$domainOnly}=example.com</info> * Can interact with short URLs for one domain: <info>%command.full_name% --{$domainOnly}=example.com</info>
* Both: <info>%command.full_name% --{$authorOnly} --{$domainOnly}=example.com</info> * Cannot see orphan visits: <info>%command.full_name% --{$noOrphanVisits}</info>
* All: <info>%command.full_name% --{$authorOnly} --{$domainOnly}=example.com --{$noOrphanVisits}</info>
HELP; HELP;
$this $this
@ -85,6 +88,12 @@ class GenerateKeyCommand extends Command
Role::DOMAIN_SPECIFIC->value, Role::DOMAIN_SPECIFIC->value,
), ),
) )
->addOption(
$noOrphanVisits,
'o',
InputOption::VALUE_NONE,
sprintf('Adds the "%s" role to the new API key.', Role::NO_ORPHAN_VISITS->value),
)
->setHelp($help); ->setHelp($help);
} }

View file

@ -27,7 +27,7 @@ class ListKeysCommand extends Command
public const NAME = 'api-key:list'; public const NAME = 'api-key:list';
public function __construct(private ApiKeyServiceInterface $apiKeyService) public function __construct(private readonly ApiKeyServiceInterface $apiKeyService)
{ {
parent::__construct(); parent::__construct();
} }
@ -60,10 +60,7 @@ class ListKeysCommand extends Command
} }
$rowData[] = $expiration?->toAtomString() ?? '-'; $rowData[] = $expiration?->toAtomString() ?? '-';
$rowData[] = ApiKey::isAdmin($apiKey) ? 'Admin' : implode("\n", $apiKey->mapRoles( $rowData[] = ApiKey::isAdmin($apiKey) ? 'Admin' : implode("\n", $apiKey->mapRoles(
fn (Role $role, array $meta) => fn (Role $role, array $meta) => $role->toFriendlyName($meta),
empty($meta)
? $role->toFriendlyName()
: sprintf('%s: %s', $role->toFriendlyName(), Role::domainAuthorityFromMeta($meta)),
)); ));
return $rowData; return $rowData;

View file

@ -26,34 +26,38 @@ class ListApiKeysTest extends CliTestCase
{ {
$expiredApiKeyDate = Chronos::now()->subDay()->startOfDay()->toAtomString(); $expiredApiKeyDate = Chronos::now()->subDay()->startOfDay()->toAtomString();
$enabledOnlyOutput = <<<OUT $enabledOnlyOutput = <<<OUT
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| Key | Name | Expiration date | Roles | | Key | Name | Expiration date | Roles |
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| valid_api_key | - | - | Admin | | valid_api_key | - | - | Admin |
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| expired_api_key | - | {$expiredApiKeyDate} | Admin | | expired_api_key | - | {$expiredApiKeyDate} | Admin |
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| author_api_key | - | - | Author only | | author_api_key | - | - | Author only |
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| domain_api_key | - | - | Domain only: example.com | | domain_api_key | - | - | Domain only: example.com |
+-----------------+------+---------------------------+--------------------------+ +--------------------+------+---------------------------+--------------------------+
| no_orphans_api_key | - | - | No orphan visits |
+--------------------+------+---------------------------+--------------------------+
OUT; OUT;
yield 'no flags' => [[], <<<OUT yield 'no flags' => [[], <<<OUT
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| Key | Name | Is enabled | Expiration date | Roles | | Key | Name | Is enabled | Expiration date | Roles |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| valid_api_key | - | +++ | - | Admin | | valid_api_key | - | +++ | - | Admin |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| disabled_api_key | - | --- | - | Admin | | disabled_api_key | - | --- | - | Admin |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| expired_api_key | - | --- | {$expiredApiKeyDate} | Admin | | expired_api_key | - | --- | {$expiredApiKeyDate} | Admin |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| author_api_key | - | +++ | - | Author only | | author_api_key | - | +++ | - | Author only |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| domain_api_key | - | +++ | - | Domain only: example.com | | domain_api_key | - | +++ | - | Domain only: example.com |
+------------------+------+------------+---------------------------+--------------------------+ +--------------------+------+------------+---------------------------+--------------------------+
| no_orphans_api_key | - | +++ | - | No orphan visits |
+--------------------+------+------------+---------------------------+--------------------------+
OUT]; OUT];
yield '-e' => [['-e'], $enabledOnlyOutput]; yield '-e' => [['-e'], $enabledOnlyOutput];

View file

@ -79,6 +79,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe
yield from $apiKey?->mapRoles(fn (Role $role, array $meta) => match ($role) { yield from $apiKey?->mapRoles(fn (Role $role, array $meta) => match ($role) {
Role::DOMAIN_SPECIFIC => ['d', new IsDomain(Role::domainIdFromMeta($meta))], Role::DOMAIN_SPECIFIC => ['d', new IsDomain(Role::domainIdFromMeta($meta))],
Role::AUTHORED_SHORT_URLS => ['s', new BelongsToApiKey($apiKey)], Role::AUTHORED_SHORT_URLS => ['s', new BelongsToApiKey($apiKey)],
default => null,
}) ?? []; }) ?? [];
} }
} }

View file

@ -56,10 +56,11 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
Role::AUTHORED_SHORT_URLS => $qb->andWhere( Role::AUTHORED_SHORT_URLS => $qb->andWhere(
$qb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())), $qb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())),
), ),
default => $qb,
}); });
// For admins and when no API key is present, we'll return tags which are not linked to any short URL // For non-restricted API keys, we'll return tags which are not linked to any short URL
$joiningMethod = ApiKey::isAdmin($apiKey) ? 'leftJoin' : 'join'; $joiningMethod = ! ApiKey::isShortUrlRestricted($apiKey) ? 'leftJoin' : 'join';
$tagsSubQb = $conn->createQueryBuilder(); $tagsSubQb = $conn->createQueryBuilder();
$tagsSubQb $tagsSubQb
->select('t.id AS tag_id', 't.name AS tag', 'COUNT(DISTINCT s.id) AS short_urls_count') ->select('t.id AS tag_id', 't.name AS tag', 'COUNT(DISTINCT s.id) AS short_urls_count')

View file

@ -59,7 +59,7 @@ class TagService implements TagServiceInterface
*/ */
public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void
{ {
if (! ApiKey::isAdmin($apiKey)) { if (ApiKey::isShortUrlRestricted($apiKey)) {
throw ForbiddenTagOperationException::forDeletion(); throw ForbiddenTagOperationException::forDeletion();
} }
@ -75,7 +75,7 @@ class TagService implements TagServiceInterface
*/ */
public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag
{ {
if (! ApiKey::isAdmin($apiKey)) { if (ApiKey::isShortUrlRestricted($apiKey)) {
throw ForbiddenTagOperationException::forRenaming(); throw ForbiddenTagOperationException::forRenaming();
} }

View file

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

View file

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

View file

@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Visit;
use Shlinkio\Shlink\Core\Model\BulkDeleteResult; use Shlinkio\Shlink\Core\Model\BulkDeleteResult;
use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface; use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface;
use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Entity\ApiKey;
class VisitsDeleter implements VisitsDeleterInterface class VisitsDeleter implements VisitsDeleterInterface
@ -16,7 +17,7 @@ class VisitsDeleter implements VisitsDeleterInterface
public function deleteOrphanVisits(?ApiKey $apiKey = null): BulkDeleteResult 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);
} }
} }

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -50,13 +50,14 @@ class VisitsStatsHelperTest extends TestCase
} }
#[Test, DataProvider('provideCounts')] #[Test, DataProvider('provideCounts')]
public function returnsExpectedVisitsStats(int $expectedCount): void public function returnsExpectedVisitsStats(int $expectedCount, ?ApiKey $apiKey): void
{ {
$repo = $this->createMock(VisitRepository::class); $repo = $this->createMock(VisitRepository::class);
$callCount = 0; $callCount = 0;
$repo->expects($this->exactly(2))->method('countNonOrphanVisits')->willReturnCallback( $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($callCount !== 0, $options->excludeBots);
Assert::assertEquals($apiKey, $options->apiKey);
$callCount++; $callCount++;
return $expectedCount * 3; return $expectedCount * 3;
@ -67,14 +68,17 @@ class VisitsStatsHelperTest extends TestCase
)->willReturn($expectedCount); )->willReturn($expectedCount);
$this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo); $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); self::assertEquals(new VisitsStats($expectedCount * 3, $expectedCount), $stats);
} }
public static function provideCounts(): iterable 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')] #[Test, DataProvider('provideAdminApiKeys')]

View file

@ -44,7 +44,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
$builder->createOneToMany('roles', ApiKeyRole::class) $builder->createOneToMany('roles', ApiKeyRole::class)
->mappedBy('apiKey') ->mappedBy('apiKey')
->setIndexBy('roleName') ->setIndexBy('role')
->cascadePersist() ->cascadePersist()
->orphanRemoval() ->orphanRemoval()
->build(); ->build();

View file

@ -25,7 +25,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
->build(); ->build();
(new FieldBuilder($builder, [ (new FieldBuilder($builder, [
'fieldName' => 'roleName', 'fieldName' => 'role',
'type' => Types::STRING, 'type' => Types::STRING,
'enumType' => Role::class, 'enumType' => Role::class,
]))->columnName('role_name') ]))->columnName('role_name')

View file

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

View file

@ -25,4 +25,9 @@ final class RoleDefinition
['domain_id' => $domain->getId(), 'authority' => $domain->authority], ['domain_id' => $domain->getId(), 'authority' => $domain->authority],
); );
} }
public static function forNoOrphanVisits(): self
{
return new self(Role::NO_ORPHAN_VISITS, []);
}
} }

View file

@ -12,16 +12,20 @@ use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomain;
use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomainInlined; use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomainInlined;
use Shlinkio\Shlink\Rest\Entity\ApiKeyRole; use Shlinkio\Shlink\Rest\Entity\ApiKeyRole;
use function sprintf;
enum Role: string enum Role: string
{ {
case AUTHORED_SHORT_URLS = 'AUTHORED_SHORT_URLS'; case AUTHORED_SHORT_URLS = 'AUTHORED_SHORT_URLS';
case DOMAIN_SPECIFIC = 'DOMAIN_SPECIFIC'; case DOMAIN_SPECIFIC = 'DOMAIN_SPECIFIC';
case NO_ORPHAN_VISITS = 'NO_ORPHAN_VISITS';
public function toFriendlyName(): string public function toFriendlyName(array $meta): string
{ {
return match ($this) { return match ($this) {
self::AUTHORED_SHORT_URLS => 'Author only', self::AUTHORED_SHORT_URLS => 'Author only',
self::DOMAIN_SPECIFIC => 'Domain only', self::DOMAIN_SPECIFIC => sprintf('Domain only: %s', Role::domainAuthorityFromMeta($meta)),
self::NO_ORPHAN_VISITS => 'No orphan visits',
}; };
} }
@ -30,6 +34,7 @@ enum Role: string
return match ($this) { return match ($this) {
self::AUTHORED_SHORT_URLS => 'author-only', self::AUTHORED_SHORT_URLS => 'author-only',
self::DOMAIN_SPECIFIC => 'domain-only', self::DOMAIN_SPECIFIC => 'domain-only',
self::NO_ORPHAN_VISITS => 'no-orphan-visits',
}; };
} }
@ -38,6 +43,7 @@ enum Role: string
return match ($role->role()) { return match ($role->role()) {
self::AUTHORED_SHORT_URLS => new BelongsToApiKey($role->apiKey(), $context), self::AUTHORED_SHORT_URLS => new BelongsToApiKey($role->apiKey(), $context),
self::DOMAIN_SPECIFIC => new BelongsToDomain(self::domainIdFromMeta($role->meta()), $context), self::DOMAIN_SPECIFIC => new BelongsToDomain(self::domainIdFromMeta($role->meta()), $context),
default => Spec::andX(),
}; };
} }
@ -46,6 +52,7 @@ enum Role: string
return match ($role->role()) { return match ($role->role()) {
self::AUTHORED_SHORT_URLS => Spec::andX(new BelongsToApiKeyInlined($role->apiKey())), self::AUTHORED_SHORT_URLS => Spec::andX(new BelongsToApiKeyInlined($role->apiKey())),
self::DOMAIN_SPECIFIC => Spec::andX(new BelongsToDomainInlined(self::domainIdFromMeta($role->meta()))), self::DOMAIN_SPECIFIC => Spec::andX(new BelongsToDomainInlined(self::domainIdFromMeta($role->meta()))),
default => Spec::andX(),
}; };
} }

View file

@ -18,7 +18,7 @@ class WithApiKeySpecsEnsuringJoin extends BaseSpecification
protected function getSpec(): Specification protected function getSpec(): Specification
{ {
return $this->apiKey === null || ApiKey::isAdmin($this->apiKey) ? Spec::andX() : Spec::andX( return $this->apiKey === null || ! ApiKey::isShortUrlRestricted($this->apiKey) ? Spec::andX() : Spec::andX(
Spec::join($this->fieldToJoin, 's'), Spec::join($this->fieldToJoin, 's'),
$this->apiKey->spec($this->fieldToJoin), $this->apiKey->spec($this->fieldToJoin),
); );

View file

@ -122,6 +122,21 @@ class ApiKey extends AbstractEntity
return $apiKey === null || $apiKey->roles->isEmpty(); return $apiKey === null || $apiKey->roles->isEmpty();
} }
/**
* Tells if provided API key has any of the roles restricting at the short URL level
*/
public static function isShortUrlRestricted(?ApiKey $apiKey): bool
{
if ($apiKey === null) {
return false;
}
return (
$apiKey->roles->containsKey(Role::AUTHORED_SHORT_URLS->value)
|| $apiKey->roles->containsKey(Role::DOMAIN_SPECIFIC->value)
);
}
public function hasRole(Role $role): bool public function hasRole(Role $role): bool
{ {
return $this->roles->containsKey($role->value); return $this->roles->containsKey($role->value);

View file

@ -9,13 +9,24 @@ use Shlinkio\Shlink\Rest\ApiKey\Role;
class ApiKeyRole extends AbstractEntity class ApiKeyRole extends AbstractEntity
{ {
public function __construct(private Role $roleName, private array $meta, private ApiKey $apiKey) public function __construct(public readonly Role $role, private array $meta, public readonly ApiKey $apiKey)
{ {
} }
/**
* @deprecated Use property access directly
*/
public function role(): Role public function role(): Role
{ {
return $this->roleName; return $this->role;
}
/**
* @deprecated Use property access directly
*/
public function apiKey(): ApiKey
{
return $this->apiKey;
} }
public function meta(): array public function meta(): array
@ -27,9 +38,4 @@ class ApiKeyRole extends AbstractEntity
{ {
$this->meta = $newMeta; $this->meta = $newMeta;
} }
public function apiKey(): ApiKey
{
return $this->apiKey;
}
} }

View file

@ -10,7 +10,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
class DeleteOrphanVisitsTest extends ApiTestCase class DeleteOrphanVisitsTest extends ApiTestCase
{ {
#[Test] #[Test]
public function deletesVisitsForShortUrlWithoutAffectingTheRest(): void public function deletesOrphanVisitsWithoutAffectingTheRest(): void
{ {
self::assertEquals(7, $this->getTotalVisits()); self::assertEquals(7, $this->getTotalVisits());
self::assertEquals(3, $this->getOrphanVisits()); self::assertEquals(3, $this->getOrphanVisits());
@ -24,6 +24,21 @@ class DeleteOrphanVisitsTest extends ApiTestCase
self::assertEquals(0, $this->getOrphanVisits()); 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 private function getTotalVisits(): int
{ {
$resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan'); $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan');

View file

@ -11,7 +11,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
class GlobalVisitsTest extends ApiTestCase class GlobalVisitsTest extends ApiTestCase
{ {
#[Test, DataProvider('provideApiKeys')] #[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); $resp = $this->callApiWithKey(self::METHOD_GET, '/visits', [], $apiKey);
$payload = $this->getJsonResponsePayload($resp); $payload = $this->getJsonResponsePayload($resp);
@ -20,13 +20,14 @@ class GlobalVisitsTest extends ApiTestCase
self::assertArrayHasKey('visitsCount', $payload['visits']); self::assertArrayHasKey('visitsCount', $payload['visits']);
self::assertArrayHasKey('orphanVisitsCount', $payload['visits']); self::assertArrayHasKey('orphanVisitsCount', $payload['visits']);
self::assertEquals($expectedVisits, $payload['visits']['visitsCount']); self::assertEquals($expectedVisits, $payload['visits']['visitsCount']);
self::assertEquals(3, $payload['visits']['orphanVisitsCount']); self::assertEquals($expectedOrphanVisits, $payload['visits']['orphanVisitsCount']);
} }
public static function provideApiKeys(): iterable public static function provideApiKeys(): iterable
{ {
yield 'admin API key' => ['valid_api_key', 7]; yield 'admin API key' => ['valid_api_key', 7, 3];
yield 'domain API key' => ['domain_api_key', 0]; yield 'domain API key' => ['domain_api_key', 0, 3];
yield 'author API key' => ['author_api_key', 5]; yield 'author API key' => ['author_api_key', 5, 3];
yield 'no orphans API key' => ['no_orphans_api_key', 7, 0];
} }
} }

View file

@ -69,4 +69,16 @@ class OrphanVisitsTest extends ApiTestCase
[self::REGULAR_NOT_FOUND], [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);
}
} }

View file

@ -23,21 +23,29 @@ class ApiKeyFixture extends AbstractFixture implements DependentFixtureInterface
public function load(ObjectManager $manager): void public function load(ObjectManager $manager): void
{ {
$manager->persist($this->buildApiKey('valid_api_key', true)); $manager->persist($this->buildApiKey('valid_api_key', enabled: true));
$manager->persist($this->buildApiKey('disabled_api_key', false)); $manager->persist($this->buildApiKey('disabled_api_key', enabled: false));
$manager->persist($this->buildApiKey('expired_api_key', true, Chronos::now()->subDay()->startOfDay())); $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()); $authorApiKey->registerRole(RoleDefinition::forAuthoredShortUrls());
$manager->persist($authorApiKey); $manager->persist($authorApiKey);
$this->addReference('author_api_key', $authorApiKey); $this->addReference('author_api_key', $authorApiKey);
/** @var Domain $exampleDomain */ /** @var Domain $exampleDomain */
$exampleDomain = $this->getReference('example_domain'); $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)); $domainApiKey->registerRole(RoleDefinition::forDomain($exampleDomain));
$manager->persist($domainApiKey); $manager->persist($domainApiKey);
$authorApiKey = $this->buildApiKey('no_orphans_api_key', enabled: true);
$authorApiKey->registerRole(RoleDefinition::forNoOrphanVisits());
$manager->persist($authorApiKey);
$manager->flush(); $manager->flush();
} }

View file

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

View file

@ -86,14 +86,15 @@ class RoleTest extends TestCase
} }
#[Test, DataProvider('provideRoleNames')] #[Test, DataProvider('provideRoleNames')]
public function getsExpectedRoleFriendlyName(Role $role, string $expectedFriendlyName): void public function getsExpectedRoleFriendlyName(Role $role, array $meta, string $expectedFriendlyName): void
{ {
self::assertEquals($expectedFriendlyName, $role->toFriendlyName()); self::assertEquals($expectedFriendlyName, $role->toFriendlyName($meta));
} }
public static function provideRoleNames(): iterable public static function provideRoleNames(): iterable
{ {
yield Role::AUTHORED_SHORT_URLS->value => [Role::AUTHORED_SHORT_URLS, 'Author only']; yield Role::AUTHORED_SHORT_URLS->value => [Role::AUTHORED_SHORT_URLS, [], 'Author only'];
yield Role::DOMAIN_SPECIFIC->value => [Role::DOMAIN_SPECIFIC, 'Domain only']; yield Role::DOMAIN_SPECIFIC->value => [Role::DOMAIN_SPECIFIC, ['authority' => 's.test'], 'Domain only: s.test'];
yield Role::NO_ORPHAN_VISITS->value => [Role::NO_ORPHAN_VISITS, [], 'No orphan visits'];
} }
} }