Added support to configure domain redirects but taking into consideration the permissions on an API key

This commit is contained in:
Alejandro Celaya 2021-07-29 19:08:29 +02:00 committed by Alejandro Celaya
parent 2ac7be4363
commit 5a1a4f5594
6 changed files with 112 additions and 28 deletions

View File

@ -59,15 +59,21 @@ class DomainService implements DomainServiceInterface
return $domain; return $domain;
} }
public function findByAuthority(string $authority): ?Domain public function findByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain
{ {
$repo = $this->em->getRepository(Domain::class); $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->persist($domain);
$this->em->flush(); $this->em->flush();
@ -75,9 +81,12 @@ class DomainService implements DomainServiceInterface
return $domain; return $domain;
} }
public function configureNotFoundRedirects(string $authority, NotFoundRedirects $notFoundRedirects): Domain public function configureNotFoundRedirects(
{ string $authority,
$domain = $this->getOrCreate($authority); NotFoundRedirects $notFoundRedirects,
?ApiKey $apiKey = null
): Domain {
$domain = $this->getOrCreate($authority, $apiKey);
$domain->configureNotFoundRedirects($notFoundRedirects); $domain->configureNotFoundRedirects($notFoundRedirects);
$this->em->flush(); $this->em->flush();

View File

@ -22,9 +22,19 @@ interface DomainServiceInterface
*/ */
public function getDomain(string $domainId): Domain; 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;
} }

View File

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Domain\Repository; namespace Shlinkio\Shlink\Core\Domain\Repository;
use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\Query\Expr\Join;
use Doctrine\ORM\QueryBuilder;
use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Spec;
use Shlinkio\Shlink\Core\Domain\Spec\IsDomain; 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 public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array
{ {
$qb = $this->createQueryBuilder('d'); $qb = $this->createPublicDomainsQueryBuilder();
$qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') $qb->orderBy('d.authority', 'ASC');
->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'));
$specs = $this->determineExtraSpecs($excludedAuthority, $apiKey); $specs = $this->determineExtraSpecs($excludedAuthority, $apiKey);
foreach ($specs as [$alias, $spec]) { foreach ($specs as [$alias, $spec]) {
@ -39,6 +34,34 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe
return $qb->getQuery()->getResult(); 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 private function determineExtraSpecs(?string $excludedAuthority, ?ApiKey $apiKey): iterable
{ {
if ($excludedAuthority !== null) { if ($excludedAuthority !== null) {

View File

@ -15,4 +15,6 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio
* @return Domain[] * @return Domain[]
*/ */
public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array; public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array;
public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain;
} }

View File

@ -27,7 +27,7 @@ class DomainRepositoryTest extends DatabaseTestCase
} }
/** @test */ /** @test */
public function findDomainsReturnsExpectedResult(): void public function expectedDomainsAreFoundWhenNoApiKeyIsInvolved(): void
{ {
$fooDomain = Domain::withAuthority('foo.com'); $fooDomain = Domain::withAuthority('foo.com');
$this->getEntityManager()->persist($fooDomain); $this->getEntityManager()->persist($fooDomain);
@ -70,10 +70,15 @@ class DomainRepositoryTest extends DatabaseTestCase
[$barDomain, $bazDomain, $fooDomain], [$barDomain, $bazDomain, $fooDomain],
$this->repo->findDomainsWithout('detached-with-redirects.com'), $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 */ /** @test */
public function findDomainsReturnsJustThoseMatchingProvidedApiKey(): void public function expectedDomainsAreFoundWhenApiKeyIsProvided(): void
{ {
$authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()));
$this->getEntityManager()->persist($authorApiKey); $this->getEntityManager()->persist($authorApiKey);
@ -124,6 +129,15 @@ class DomainRepositoryTest extends DatabaseTestCase
); );
self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout(null, $authorApiKey)); self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout(null, $authorApiKey));
self::assertEquals([], $this->repo->findDomainsWithout(null, $authorAndDomainApiKey)); 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 private function createShortUrl(Domain $domain, ?ApiKey $apiKey = null): ShortUrl

View File

@ -133,16 +133,16 @@ class DomainServiceTest extends TestCase
* @test * @test
* @dataProvider provideFoundDomains * @dataProvider provideFoundDomains
*/ */
public function getOrCreateAlwaysPersistsDomain(?Domain $foundDomain): void public function getOrCreateAlwaysPersistsDomain(?Domain $foundDomain, ?ApiKey $apiKey): void
{ {
$authority = 'example.com'; $authority = 'example.com';
$repo = $this->prophesize(DomainRepositoryInterface::class); $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()); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal());
$persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class));
$flush = $this->em->flush(); $flush = $this->em->flush();
$result = $this->domainService->getOrCreate($authority); $result = $this->domainService->getOrCreate($authority, $apiKey);
if ($foundDomain !== null) { if ($foundDomain !== null) {
self::assertSame($result, $foundDomain); self::assertSame($result, $foundDomain);
@ -152,15 +152,33 @@ class DomainServiceTest extends TestCase
$flush->shouldHaveBeenCalledOnce(); $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 * @test
* @dataProvider provideFoundDomains * @dataProvider provideFoundDomains
*/ */
public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain): void public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain, ?ApiKey $apiKey): void
{ {
$authority = 'example.com'; $authority = 'example.com';
$repo = $this->prophesize(DomainRepositoryInterface::class); $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()); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal());
$persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class));
$flush = $this->em->flush(); $flush = $this->em->flush();
@ -169,7 +187,7 @@ class DomainServiceTest extends TestCase
'foo.com', 'foo.com',
'bar.com', 'bar.com',
'baz.com', 'baz.com',
)); ), $apiKey);
if ($foundDomain !== null) { if ($foundDomain !== null) {
self::assertSame($result, $foundDomain); self::assertSame($result, $foundDomain);
@ -184,7 +202,15 @@ class DomainServiceTest extends TestCase
public function provideFoundDomains(): iterable public function provideFoundDomains(): iterable
{ {
yield 'domain not found' => [null]; $domain = Domain::withAuthority('');
yield 'domain found' => [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];
} }
} }