From caa1ae0de8c08399a0c6b4c30e6ae912ef1b3cc2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Jan 2021 12:38:06 +0100 Subject: [PATCH] Added all missing unit tests covering API key permissions --- module/Core/test/Domain/DomainServiceTest.php | 45 ++++++++++-- .../VisitsForTagPaginatorAdapterTest.php | 27 ++++--- .../Adapter/VisitsPaginatorAdapterTest.php | 27 ++++--- .../Service/ShortUrl/ShortUrlResolverTest.php | 31 +++++--- .../Core/test/Service/ShortUrlServiceTest.php | 29 +++++--- .../Core/test/Service/Tag/TagServiceTest.php | 11 +-- .../Core/test/Service/VisitsTrackerTest.php | 40 +++++++---- module/Rest/test/ApiKey/RoleTest.php | 72 +++++++++++++++++++ 8 files changed, 222 insertions(+), 60 deletions(-) create mode 100644 module/Rest/test/ApiKey/RoleTest.php diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 0201d7d9..5fc4aaaa 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -13,6 +13,8 @@ use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; +use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class DomainServiceTest extends TestCase { @@ -31,13 +33,13 @@ class DomainServiceTest extends TestCase * @test * @dataProvider provideExcludedDomains */ - public function listDomainsDelegatesIntoRepository(array $domains, array $expectedResult): void + public function listDomainsDelegatesIntoRepository(array $domains, array $expectedResult, ?ApiKey $apiKey): void { $repo = $this->prophesize(DomainRepositoryInterface::class); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $findDomains = $repo->findDomainsWithout('default.com', null)->willReturn($domains); + $findDomains = $repo->findDomainsWithout('default.com', $apiKey)->willReturn($domains); - $result = $this->domainService->listDomains(); + $result = $this->domainService->listDomains($apiKey); self::assertEquals($expectedResult, $result); $getRepo->shouldHaveBeenCalledOnce(); @@ -47,12 +49,43 @@ class DomainServiceTest extends TestCase public function provideExcludedDomains(): iterable { $default = new DomainItem('default.com', true); + $adminApiKey = new ApiKey(); + $domainSpecificApiKey = new ApiKey(null, [RoleDefinition::forDomain('123')]); - yield 'empty list' => [[], [$default]]; - yield 'one item' => [[new Domain('bar.com')], [$default, new DomainItem('bar.com', false)]]; - yield 'multiple items' => [ + yield 'empty list without API key' => [[], [$default], null]; + yield 'one item without API key' => [ + [new Domain('bar.com')], + [$default, new DomainItem('bar.com', false)], + null, + ]; + yield 'multiple items without API key' => [ [new Domain('foo.com'), new Domain('bar.com')], [$default, new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + null, + ]; + + yield 'empty list with admin API key' => [[], [$default], $adminApiKey]; + yield 'one item with admin API key' => [ + [new Domain('bar.com')], + [$default, new DomainItem('bar.com', false)], + $adminApiKey, + ]; + yield 'multiple items with admin API key' => [ + [new Domain('foo.com'), new Domain('bar.com')], + [$default, new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + $adminApiKey, + ]; + + yield 'empty list with domain-specific API key' => [[], [], $domainSpecificApiKey]; + yield 'one item with domain-specific API key' => [ + [new Domain('bar.com')], + [new DomainItem('bar.com', false)], + $domainSpecificApiKey, + ]; + yield 'multiple items with domain-specific API key' => [ + [new Domain('foo.com'), new Domain('bar.com')], + [new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + $domainSpecificApiKey, ]; } diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index 8d577b91..a0bc6405 100644 --- a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -11,23 +11,17 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsForTagPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsForTagPaginatorAdapterTest extends TestCase { use ProphecyTrait; - private VisitsForTagPaginatorAdapter $adapter; private ObjectProphecy $repo; protected function setUp(): void { $this->repo = $this->prophesize(VisitRepositoryInterface::class); - $this->adapter = new VisitsForTagPaginatorAdapter( - $this->repo->reveal(), - 'foo', - VisitsParams::fromRawData([]), - null, - ); } /** @test */ @@ -36,10 +30,11 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $count = 3; $limit = 1; $offset = 5; + $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByTag('foo', new DateRange(), $limit, $offset, null)->willReturn([]); for ($i = 0; $i < $count; $i++) { - $this->adapter->getItems($offset, $limit); + $adapter->getItems($offset, $limit); } $findVisits->shouldHaveBeenCalledTimes($count); @@ -49,12 +44,24 @@ class VisitsForTagPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $countVisits = $this->repo->countVisitsByTag('foo', new DateRange(), null)->willReturn(3); + $apiKey = new ApiKey(); + $adapter = $this->createAdapter($apiKey); + $countVisits = $this->repo->countVisitsByTag('foo', new DateRange(), $apiKey->spec())->willReturn(3); for ($i = 0; $i < $count; $i++) { - $this->adapter->count(); + $adapter->count(); } $countVisits->shouldHaveBeenCalledOnce(); } + + private function createAdapter(?ApiKey $apiKey): VisitsForTagPaginatorAdapter + { + return new VisitsForTagPaginatorAdapter( + $this->repo->reveal(), + 'foo', + VisitsParams::fromRawData([]), + $apiKey, + ); + } } diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php index ca0c5806..76ccc220 100644 --- a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -12,23 +12,17 @@ use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsPaginatorAdapterTest extends TestCase { use ProphecyTrait; - private VisitsPaginatorAdapter $adapter; private ObjectProphecy $repo; protected function setUp(): void { $this->repo = $this->prophesize(VisitRepositoryInterface::class); - $this->adapter = new VisitsPaginatorAdapter( - $this->repo->reveal(), - new ShortUrlIdentifier(''), - VisitsParams::fromRawData([]), - null, - ); } /** @test */ @@ -37,12 +31,13 @@ class VisitsPaginatorAdapterTest extends TestCase $count = 3; $limit = 1; $offset = 5; + $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByShortCode('', null, new DateRange(), $limit, $offset, null)->willReturn( [], ); for ($i = 0; $i < $count; $i++) { - $this->adapter->getItems($offset, $limit); + $adapter->getItems($offset, $limit); } $findVisits->shouldHaveBeenCalledTimes($count); @@ -52,12 +47,24 @@ class VisitsPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange(), null)->willReturn(3); + $apiKey = new ApiKey(); + $adapter = $this->createAdapter($apiKey); + $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange(), $apiKey->spec())->willReturn(3); for ($i = 0; $i < $count; $i++) { - $this->adapter->count(); + $adapter->count(); } $countVisits->shouldHaveBeenCalledOnce(); } + + private function createAdapter(?ApiKey $apiKey): VisitsPaginatorAdapter + { + return new VisitsPaginatorAdapter( + $this->repo->reveal(), + new ShortUrlIdentifier(''), + VisitsParams::fromRawData([]), + $apiKey !== null ? $apiKey->spec() : null, + ); + } } diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php index e9ff7a51..54d2eeda 100644 --- a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolver; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use function Functional\map; use function range; @@ -35,37 +36,49 @@ class ShortUrlResolverTest extends TestCase $this->urlResolver = new ShortUrlResolver($this->em->reveal()); } - /** @test */ - public function shortCodeIsProperlyParsed(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function shortCodeIsProperlyParsed(?ApiKey $apiKey): void { $shortUrl = new ShortUrl('expected_url'); $shortCode = $shortUrl->getShortCode(); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOne = $repo->findOne($shortCode, null, null)->willReturn($shortUrl); + $findOne = $repo->findOne($shortCode, null, $apiKey !== null ? $apiKey->spec() : null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); + $result = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey); self::assertSame($shortUrl, $result); $findOne->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } - /** @test */ - public function exceptionIsThrownIfShortcodeIsNotFound(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function exceptionIsThrownIfShortcodeIsNotFound(?ApiKey $apiKey): void { $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOne = $repo->findOne($shortCode, null, null)->willReturn(null); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $findOne = $repo->findOne($shortCode, null, $apiKey !== null ? $apiKey->spec() : null)->willReturn(null); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal(), $apiKey); $this->expectException(ShortUrlNotFoundException::class); $findOne->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey); + } + + public function provideApiKeys(): iterable + { + yield 'no API key' => [null]; + yield 'API key' => [new ApiKey()]; } /** @test */ diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 19c92b6f..bb2f685a 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -49,8 +49,11 @@ class ShortUrlServiceTest extends TestCase ); } - /** @test */ - public function listedUrlsAreReturnedFromEntityManager(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function listedUrlsAreReturnedFromEntityManager(?ApiKey $apiKey): void { $list = [ new ShortUrl(''), @@ -64,25 +67,35 @@ class ShortUrlServiceTest extends TestCase $repo->countList(Argument::cetera())->willReturn(count($list))->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $list = $this->service->listShortUrls(ShortUrlsParams::emptyInstance()); + $list = $this->service->listShortUrls(ShortUrlsParams::emptyInstance(), $apiKey); self::assertEquals(4, $list->getCurrentItemCount()); } - /** @test */ - public function providedTagsAreGetFromRepoAndSetToTheShortUrl(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function providedTagsAreGetFromRepoAndSetToTheShortUrl(?ApiKey $apiKey): void { $shortUrl = $this->prophesize(ShortUrl::class); $shortUrl->setTags(Argument::any())->shouldBeCalledOnce(); $shortCode = 'abc123'; - $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), null)->willReturn($shortUrl->reveal()) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey) + ->willReturn($shortUrl->reveal()) + ->shouldBeCalledOnce(); $tagRepo = $this->prophesize(EntityRepository::class); $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldBeCalledOnce(); $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldBeCalledOnce(); $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); - $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar']); + $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar'], $apiKey); + } + + public function provideApiKeys(): iterable + { + yield 'no API key' => [null]; + yield 'API key' => [new ApiKey()]; } /** diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 670944f1..c0ef8760 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -50,14 +50,17 @@ class TagServiceTest extends TestCase $match->shouldHaveBeenCalled(); } - /** @test */ - public function tagsInfoDelegatesOnRepository(): void + /** + * @test + * @dataProvider provideAdminApiKeys + */ + public function tagsInfoDelegatesOnRepository(?ApiKey $apiKey): void { $expected = [new TagInfo(new Tag('foo'), 1, 1), new TagInfo(new Tag('bar'), 3, 10)]; - $find = $this->repo->findTagsWithInfo(null)->willReturn($expected); + $find = $this->repo->findTagsWithInfo($apiKey === null ? null : $apiKey->spec())->willReturn($expected); - $result = $this->service->tagsInfo(); + $result = $this->service->tagsInfo($apiKey); self::assertEquals($expected, $result); $find->shouldHaveBeenCalled(); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 56478966..38facd24 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -43,7 +43,7 @@ class VisitsTrackerTest extends TestCase $this->em = $this->prophesize(EntityManager::class); $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), true); + $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), true); } /** @test */ @@ -59,23 +59,27 @@ class VisitsTrackerTest extends TestCase $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); } - /** @test */ - public function infoReturnsVisitsForCertainShortCode(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function infoReturnsVisitsForCertainShortCode(?ApiKey $apiKey): void { $shortCode = '123ABC'; + $spec = $apiKey === null ? null : $apiKey->spec(); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(true); + $count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, null)->willReturn( + $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, $spec)->willReturn( $list, ); - $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), null)->willReturn(1); + $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), $spec)->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - $paginator = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); + $paginator = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentItems())); $count->shouldHaveBeenCalledOnce(); @@ -111,24 +115,34 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker->visitsForTag($tag, new VisitsParams(), $apiKey); } - /** @test */ - public function visitsForTagAreReturnedAsExpected(): void + /** + * @test + * @dataProvider provideApiKeys + */ + public function visitsForTagAreReturnedAsExpected(?ApiKey $apiKey): void { $tag = 'foo'; $repo = $this->prophesize(TagRepository::class); - $tagExists = $repo->tagExists($tag, null)->willReturn(true); + $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(true); $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $spec = $apiKey === null ? null : $apiKey->spec(); $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, null)->willReturn($list); - $repo2->countVisitsByTag($tag, Argument::type(DateRange::class), null)->willReturn(1); + $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, $spec)->willReturn($list); + $repo2->countVisitsByTag($tag, Argument::type(DateRange::class), $spec)->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - $paginator = $this->visitsTracker->visitsForTag($tag, new VisitsParams()); + $paginator = $this->visitsTracker->visitsForTag($tag, new VisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentItems())); $tagExists->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } + + public function provideApiKeys(): iterable + { + yield 'no API key' => [null]; + yield 'API key' => [new ApiKey()]; + } } diff --git a/module/Rest/test/ApiKey/RoleTest.php b/module/Rest/test/ApiKey/RoleTest.php new file mode 100644 index 00000000..b2dead47 --- /dev/null +++ b/module/Rest/test/ApiKey/RoleTest.php @@ -0,0 +1,72 @@ + [new ApiKeyRole('invalid', [], $apiKey), true, Spec::andX()]; + yield 'not inline invalid role' => [new ApiKeyRole('invalid', [], $apiKey), false, Spec::andX()]; + yield 'inline author role' => [ + new ApiKeyRole(Role::AUTHORED_SHORT_URLS, [], $apiKey), + true, + new BelongsToApiKeyInlined($apiKey), + ]; + yield 'not inline author role' => [ + new ApiKeyRole(Role::AUTHORED_SHORT_URLS, [], $apiKey), + false, + new BelongsToApiKey($apiKey), + ]; + yield 'inline domain role' => [ + new ApiKeyRole(Role::DOMAIN_SPECIFIC, ['domain_id' => '123'], $apiKey), + true, + new BelongsToDomainInlined('123'), + ]; + yield 'not inline domain role' => [ + new ApiKeyRole(Role::DOMAIN_SPECIFIC, ['domain_id' => '456'], $apiKey), + false, + new BelongsToDomain('456'), + ]; + } + + /** + * @test + * @dataProvider provideMetas + */ + public function getsExpectedDomainIdFromMeta(array $meta, string $expectedDomainId): void + { + self::assertEquals($expectedDomainId, Role::domainIdFromMeta($meta)); + } + + public function provideMetas(): iterable + { + yield 'empty meta' => [[], '-1']; + yield 'meta without domain_id' => [['foo' => 'bar'], '-1']; + yield 'meta with domain_id' => [['domain_id' => '123'], '123']; + } +}