From abc954aa47780eb8b28c1768e173b4eee04ec690 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Thu, 30 Sep 2021 22:57:24 +0200
Subject: [PATCH 1/2] Added extra DB tests ensuring proper short URL visits are
 resolved from an API key

---
 .../Repository/VisitRepositoryTest.php        | 53 ++++++++++++++++---
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php
index 9f7859ff..c78583af 100644
--- a/module/Core/test-db/Repository/VisitRepositoryTest.php
+++ b/module/Core/test-db/Repository/VisitRepositoryTest.php
@@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver;
+use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
 use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
@@ -25,6 +26,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
 use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase;
 
 use function Functional\map;
+use function is_string;
 use function range;
 use function sprintf;
 
@@ -171,6 +173,38 @@ class VisitRepositoryTest extends DatabaseTestCase
         ));
     }
 
+    /** @test */
+    public function findVisitsByShortCodeReturnsProperDataWhenUsingAPiKeys(): void
+    {
+        $adminApiKey = ApiKey::create();
+        $this->getEntityManager()->persist($adminApiKey);
+
+        $restrictedApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()));
+        $this->getEntityManager()->persist($restrictedApiKey);
+
+        $this->getEntityManager()->flush();
+
+        [$shortCode1] = $this->createShortUrlsAndVisits(true, [], $adminApiKey);
+        [$shortCode2] = $this->createShortUrlsAndVisits('bar.com', [], $restrictedApiKey);
+
+        self::assertNotEmpty($this->repo->findVisitsByShortCode(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1),
+            new VisitsListFiltering(null, false, $adminApiKey->spec()),
+        ));
+        self::assertNotEmpty($this->repo->findVisitsByShortCode(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2),
+            new VisitsListFiltering(null, false, $adminApiKey->spec()),
+        ));
+        self::assertEmpty($this->repo->findVisitsByShortCode(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1),
+            new VisitsListFiltering(null, false, $restrictedApiKey->spec()),
+        ));
+        self::assertNotEmpty($this->repo->findVisitsByShortCode(
+            ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2),
+            new VisitsListFiltering(null, false, $restrictedApiKey->spec()),
+        ));
+    }
+
     /** @test */
     public function findVisitsByTagReturnsProperData(): void
     {
@@ -354,19 +388,26 @@ class VisitRepositoryTest extends DatabaseTestCase
         ));
     }
 
-    private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array
-    {
+    /**
+     * @return array{string, string, ShortUrl}
+     */
+    private function createShortUrlsAndVisits(
+        bool|string $withDomain = true,
+        array $tags = [],
+        ?ApiKey $apiKey = null,
+    ): array {
         $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
-            'longUrl' => '',
-            'tags' => $tags,
+            ShortUrlInputFilter::LONG_URL => '',
+            ShortUrlInputFilter::TAGS => $tags,
+            ShortUrlInputFilter::API_KEY => $apiKey,
         ]), $this->relationResolver);
-        $domain = 'example.com';
+        $domain = is_string($withDomain) ? $withDomain : 'example.com';
         $shortCode = $shortUrl->getShortCode();
         $this->getEntityManager()->persist($shortUrl);
 
         $this->createVisitsForShortUrl($shortUrl);
 
-        if ($withDomain) {
+        if ($withDomain !== false) {
             $shortUrlWithDomain = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
                 'customSlug' => $shortCode,
                 'domain' => $domain,

From 5e627641eac275a85294d754de006f9710645d61 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 1 Oct 2021 19:32:34 +0200
Subject: [PATCH 2/2] Added more tests ensuring any short URL can be fetched by
 using an admin API key

---
 .../Repository/ShortUrlRepositoryTest.php     | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
index a614844f..d4ff42b8 100644
--- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
+++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
@@ -355,6 +355,8 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($wrongDomainApiKey);
         $rightDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($rightDomain)));
         $this->getEntityManager()->persist($rightDomainApiKey);
+        $adminApiKey = ApiKey::create();
+        $this->getEntityManager()->persist($adminApiKey);
 
         $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
             'validSince' => $start,
@@ -365,6 +367,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         ]), $this->relationResolver);
         $this->getEntityManager()->persist($shortUrl);
 
+        $nonDomainShortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([
+            'apiKey' => $apiKey,
+            'longUrl' => 'non-domain',
+        ]), $this->relationResolver);
+        $this->getEntityManager()->persist($nonDomainShortUrl);
+
         $this->getEntityManager()->flush();
 
         self::assertSame(
@@ -379,6 +387,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
             'longUrl' => 'foo',
             'tags' => ['foo', 'bar'],
         ])));
+        self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlMeta::fromRawData([
+            'validSince' => $start,
+            'apiKey' => $adminApiKey,
+            'longUrl' => 'foo',
+            'tags' => ['foo', 'bar'],
+        ])));
         self::assertNull($this->repo->findOneMatching(ShortUrlMeta::fromRawData([
             'validSince' => $start,
             'apiKey' => $otherApiKey,
@@ -424,6 +438,27 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
                 'tags' => ['foo', 'bar'],
             ])),
         );
+
+        self::assertSame(
+            $nonDomainShortUrl,
+            $this->repo->findOneMatching(ShortUrlMeta::fromRawData([
+                'apiKey' => $apiKey,
+                'longUrl' => 'non-domain',
+            ])),
+        );
+        self::assertSame(
+            $nonDomainShortUrl,
+            $this->repo->findOneMatching(ShortUrlMeta::fromRawData([
+                'apiKey' => $adminApiKey,
+                'longUrl' => 'non-domain',
+            ])),
+        );
+        self::assertNull(
+            $this->repo->findOneMatching(ShortUrlMeta::fromRawData([
+                'apiKey' => $otherApiKey,
+                'longUrl' => 'non-domain',
+            ])),
+        );
     }
 
     /** @test */