diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 99bade7c..708984b5 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -59,15 +59,21 @@ class DomainService implements DomainServiceInterface return $domain; } - public function findByAuthority(string $authority): ?Domain + public function findByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain { $repo = $this->em->getRepository(Domain::class); - return $repo->findOneBy(['authority' => $authority]); + return $repo->findOneByAuthority($authority, $apiKey); } - public function getOrCreate(string $authority): Domain + public function getOrCreate(string $authority, ?ApiKey $apiKey = null): Domain { - $domain = $this->findByAuthority($authority) ?? Domain::withAuthority($authority); + $domain = $this->findByAuthority($authority, $apiKey); + if ($domain === null && $apiKey?->hasRole(Role::DOMAIN_SPECIFIC)) { + // This API key is restricted to one domain and a different one was tried to be fetched + throw DomainNotFoundException::fromAuthority($authority); + } + + $domain = $domain ?? Domain::withAuthority($authority); $this->em->persist($domain); $this->em->flush(); @@ -75,9 +81,12 @@ class DomainService implements DomainServiceInterface return $domain; } - public function configureNotFoundRedirects(string $authority, NotFoundRedirects $notFoundRedirects): Domain - { - $domain = $this->getOrCreate($authority); + public function configureNotFoundRedirects( + string $authority, + NotFoundRedirects $notFoundRedirects, + ?ApiKey $apiKey = null + ): Domain { + $domain = $this->getOrCreate($authority, $apiKey); $domain->configureNotFoundRedirects($notFoundRedirects); $this->em->flush(); diff --git a/module/Core/src/Domain/DomainServiceInterface.php b/module/Core/src/Domain/DomainServiceInterface.php index 802ecc7a..9ac48e69 100644 --- a/module/Core/src/Domain/DomainServiceInterface.php +++ b/module/Core/src/Domain/DomainServiceInterface.php @@ -22,9 +22,19 @@ interface DomainServiceInterface */ public function getDomain(string $domainId): Domain; - public function getOrCreate(string $authority): Domain; + /** + * @throws DomainNotFoundException If the API key is restricted to one domain and a different one is provided + */ + public function getOrCreate(string $authority, ?ApiKey $apiKey = null): Domain; - public function findByAuthority(string $authority): ?Domain; + public function findByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain; - public function configureNotFoundRedirects(string $authority, NotFoundRedirects $notFoundRedirects): Domain; + /** + * @throws DomainNotFoundException If the API key is restricted to one domain and a different one is provided + */ + public function configureNotFoundRedirects( + string $authority, + NotFoundRedirects $notFoundRedirects, + ?ApiKey $apiKey = null, + ): Domain; } diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 6d246924..50033604 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain\Repository; use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Domain\Spec\IsDomain; @@ -22,14 +23,8 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe */ public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array { - $qb = $this->createQueryBuilder('d'); - $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') - ->orderBy('d.authority', 'ASC') - ->groupBy('d') - ->having($qb->expr()->gt('COUNT(s.id)', '0')) - ->orHaving($qb->expr()->isNotNull('d.baseUrlRedirect')) - ->orHaving($qb->expr()->isNotNull('d.regular404Redirect')) - ->orHaving($qb->expr()->isNotNull('d.invalidShortUrlRedirect')); + $qb = $this->createPublicDomainsQueryBuilder(); + $qb->orderBy('d.authority', 'ASC'); $specs = $this->determineExtraSpecs($excludedAuthority, $apiKey); foreach ($specs as [$alias, $spec]) { @@ -39,6 +34,34 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe return $qb->getQuery()->getResult(); } + public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain + { + $qb = $this->createPublicDomainsQueryBuilder(); + $qb->where($qb->expr()->eq('d.authority', ':authority')) + ->setParameter('authority', $authority) + ->setMaxResults(1); + + $specs = $this->determineExtraSpecs(null, $apiKey); + foreach ($specs as [$alias, $spec]) { + $this->applySpecification($qb, $spec, $alias); + } + + return $qb->getQuery()->getOneOrNullResult(); + } + + private function createPublicDomainsQueryBuilder(): QueryBuilder + { + $qb = $this->createQueryBuilder('d'); + $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') + ->groupBy('d') + ->having($qb->expr()->gt('COUNT(s.id)', '0')) + ->orHaving($qb->expr()->isNotNull('d.baseUrlRedirect')) + ->orHaving($qb->expr()->isNotNull('d.regular404Redirect')) + ->orHaving($qb->expr()->isNotNull('d.invalidShortUrlRedirect')); + + return $qb; + } + private function determineExtraSpecs(?string $excludedAuthority, ?ApiKey $apiKey): iterable { if ($excludedAuthority !== null) { diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index 1d201520..123e349d 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -15,4 +15,6 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio * @return Domain[] */ public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array; + + public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain; } diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 9e3fe9be..f52ba54a 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -27,7 +27,7 @@ class DomainRepositoryTest extends DatabaseTestCase } /** @test */ - public function findDomainsReturnsExpectedResult(): void + public function expectedDomainsAreFoundWhenNoApiKeyIsInvolved(): void { $fooDomain = Domain::withAuthority('foo.com'); $this->getEntityManager()->persist($fooDomain); @@ -70,10 +70,15 @@ class DomainRepositoryTest extends DatabaseTestCase [$barDomain, $bazDomain, $fooDomain], $this->repo->findDomainsWithout('detached-with-redirects.com'), ); + + self::assertEquals($barDomain, $this->repo->findOneByAuthority('bar.com')); + self::assertEquals($detachedWithRedirects, $this->repo->findOneByAuthority('detached-with-redirects.com')); + self::assertNull($this->repo->findOneByAuthority('does-not-exist.com')); + self::assertNull($this->repo->findOneByAuthority('detached.com')); } /** @test */ - public function findDomainsReturnsJustThoseMatchingProvidedApiKey(): void + public function expectedDomainsAreFoundWhenApiKeyIsProvided(): void { $authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($authorApiKey); @@ -124,6 +129,15 @@ class DomainRepositoryTest extends DatabaseTestCase ); self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout(null, $authorApiKey)); self::assertEquals([], $this->repo->findDomainsWithout(null, $authorAndDomainApiKey)); + + self::assertEquals($fooDomain, $this->repo->findOneByAuthority('foo.com', $authorApiKey)); + self::assertNull($this->repo->findOneByAuthority('bar.com', $authorApiKey)); + self::assertEquals($barDomain, $this->repo->findOneByAuthority('bar.com', $barDomainApiKey)); + self::assertEquals( + $detachedWithRedirects, + $this->repo->findOneByAuthority('detached-with-redirects.com', $detachedWithRedirectsApiKey), + ); + self::assertNull($this->repo->findOneByAuthority('foo.com', $detachedWithRedirectsApiKey)); } private function createShortUrl(Domain $domain, ?ApiKey $apiKey = null): ShortUrl diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index b53b4a26..6e91b425 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -133,16 +133,16 @@ class DomainServiceTest extends TestCase * @test * @dataProvider provideFoundDomains */ - public function getOrCreateAlwaysPersistsDomain(?Domain $foundDomain): void + public function getOrCreateAlwaysPersistsDomain(?Domain $foundDomain, ?ApiKey $apiKey): void { $authority = 'example.com'; $repo = $this->prophesize(DomainRepositoryInterface::class); - $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); + $repo->findOneByAuthority($authority, $apiKey)->willReturn($foundDomain); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); $flush = $this->em->flush(); - $result = $this->domainService->getOrCreate($authority); + $result = $this->domainService->getOrCreate($authority, $apiKey); if ($foundDomain !== null) { self::assertSame($result, $foundDomain); @@ -152,15 +152,33 @@ class DomainServiceTest extends TestCase $flush->shouldHaveBeenCalledOnce(); } + /** @test */ + public function getOrCreateThrowsExceptionForApiKeysWithDomainRole(): void + { + $authority = 'example.com'; + $domain = Domain::withAuthority($authority)->setId('1'); + $apiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($domain))); + $repo = $this->prophesize(DomainRepositoryInterface::class); + $repo->findOneByAuthority($authority, $apiKey)->willReturn(null); + $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $this->expectException(DomainNotFoundException::class); + $getRepo->shouldBeCalledOnce(); + $this->em->persist(Argument::cetera())->shouldNotBeCalled(); + $this->em->flush()->shouldNotBeCalled(); + + $this->domainService->getOrCreate($authority, $apiKey); + } + /** * @test * @dataProvider provideFoundDomains */ - public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain): void + public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain, ?ApiKey $apiKey): void { $authority = 'example.com'; $repo = $this->prophesize(DomainRepositoryInterface::class); - $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); + $repo->findOneByAuthority($authority, $apiKey)->willReturn($foundDomain); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); $flush = $this->em->flush(); @@ -169,7 +187,7 @@ class DomainServiceTest extends TestCase 'foo.com', 'bar.com', 'baz.com', - )); + ), $apiKey); if ($foundDomain !== null) { self::assertSame($result, $foundDomain); @@ -184,7 +202,15 @@ class DomainServiceTest extends TestCase public function provideFoundDomains(): iterable { - yield 'domain not found' => [null]; - yield 'domain found' => [Domain::withAuthority('')]; + $domain = Domain::withAuthority(''); + $adminApiKey = ApiKey::create(); + $authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); + + yield 'domain not found and no API key' => [null, null]; + yield 'domain found and no API key' => [$domain, null]; + yield 'domain not found and admin API key' => [null, $adminApiKey]; + yield 'domain found and admin API key' => [$domain, $adminApiKey]; + yield 'domain not found and author API key' => [null, $authorApiKey]; + yield 'domain found and author API key' => [$domain, $authorApiKey]; } }