Ensured delete/rename tags cannot be done with non-admin API keys

module/Core/src/Exception/ForbiddenTagOperationException.php

@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Exception;
+
+use Fig\Http\Message\StatusCodeInterface;
+use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait;
+use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface;
+
+class ForbiddenTagOperationException extends DomainException implements ProblemDetailsExceptionInterface
+{
+    use CommonProblemDetailsExceptionTrait;
+
+    private const TITLE = 'Forbidden tag operation';
+    private const TYPE = 'FORBIDDEN_OPERATION';
+
+    public static function forDeletion(): self
+    {
+        return self::createWithMessage('You are not allowed to delete tags');
+    }
+
+    public static function forRenaming(): self
+    {
+        return self::createWithMessage('You are not allowed to rename tags');
+    }
+
+    private static function createWithMessage(string $message): self
+    {
+        $e = new self($message);
+
+        $e->detail = $message;
+        $e->title = self::TITLE;
+        $e->type = self::TYPE;
+        $e->status = StatusCodeInterface::STATUS_FORBIDDEN;
+
+        return $e;
+    }
+}

module/Core/src/Tag/TagService.php

@@ -8,6 +8,7 @@ use Doctrine\Common\Collections\Collection;
 use Doctrine\ORM;
 use Happyr\DoctrineSpecification\Spec;
 use Shlinkio\Shlink\Core\Entity\Tag;
+use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
 use Shlinkio\Shlink\Core\Exception\TagConflictException;
 use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
@@ -56,9 +57,14 @@ class TagService implements TagServiceInterface
 
     /**
      * @param string[] $tagNames
+     * @throws ForbiddenTagOperationException
      */
-    public function deleteTags(array $tagNames): void
+    public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void
     {
+        if ($apiKey !== null && ! $apiKey->isAdmin()) {
+            throw ForbiddenTagOperationException::forDeletion();
+        }
+
         /** @var TagRepository $repo */
         $repo = $this->em->getRepository(Tag::class);
         $repo->deleteByName($tagNames);
@@ -82,9 +88,14 @@ class TagService implements TagServiceInterface
     /**
      * @throws TagNotFoundException
      * @throws TagConflictException
+     * @throws ForbiddenTagOperationException
      */
-    public function renameTag(TagRenaming $renaming): Tag
+    public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag
     {
+        if ($apiKey !== null && ! $apiKey->isAdmin()) {
+            throw ForbiddenTagOperationException::forRenaming();
+        }
+
         /** @var TagRepository $repo */
         $repo = $this->em->getRepository(Tag::class);
 

module/Core/src/Tag/TagServiceInterface.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Tag;
 
 use Doctrine\Common\Collections\Collection;
 use Shlinkio\Shlink\Core\Entity\Tag;
+use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
 use Shlinkio\Shlink\Core\Exception\TagConflictException;
 use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Tag\Model\TagInfo;
@@ -26,8 +27,9 @@ interface TagServiceInterface
 
     /**
      * @param string[] $tagNames
+     * @throws ForbiddenTagOperationException
      */
-    public function deleteTags(array $tagNames): void;
+    public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void;
 
     /**
      * @deprecated
@@ -39,6 +41,7 @@ interface TagServiceInterface
     /**
      * @throws TagNotFoundException
      * @throws TagConflictException
+     * @throws ForbiddenTagOperationException
      */
-    public function renameTag(TagRenaming $renaming): Tag;
+    public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag;
 }

module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Exception;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
+
+class ForbiddenTagOperationExceptionTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider provideExceptions
+     */
+    public function createsExpectedExceptionForDeletion(
+        ForbiddenTagOperationException $e,
+        string $expectedMessage
+    ): void {
+        $this->assertExceptionShape($e, $expectedMessage);
+    }
+
+    private function assertExceptionShape(ForbiddenTagOperationException $e, string $expectedMessage): void
+    {
+        self::assertEquals($expectedMessage, $e->getMessage());
+        self::assertEquals($expectedMessage, $e->getDetail());
+        self::assertEquals('Forbidden tag operation', $e->getTitle());
+        self::assertEquals('FORBIDDEN_OPERATION', $e->getType());
+        self::assertEquals(403, $e->getStatus());
+    }
+
+    public function provideExceptions(): iterable
+    {
+        yield 'deletion' => [ForbiddenTagOperationException::forDeletion(), 'You are not allowed to delete tags'];
+        yield 'renaming' => [ForbiddenTagOperationException::forRenaming(), 'You are not allowed to rename tags'];
+    }
+}

module/Core/test/Service/Tag/TagServiceTest.php

@@ -10,12 +10,15 @@ use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\Tag;
+use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
 use Shlinkio\Shlink\Core\Exception\TagConflictException;
 use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
 use Shlinkio\Shlink\Core\Repository\TagRepository;
 use Shlinkio\Shlink\Core\Tag\Model\TagInfo;
 use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
 use Shlinkio\Shlink\Core\Tag\TagService;
+use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class TagServiceTest extends TestCase
 {
@@ -29,7 +32,7 @@ class TagServiceTest extends TestCase
     {
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $this->repo = $this->prophesize(TagRepository::class);
-        $this->em->getRepository(Tag::class)->willReturn($this->repo->reveal())->shouldBeCalled();
+        $this->em->getRepository(Tag::class)->willReturn($this->repo->reveal());
 
         $this->service = new TagService($this->em->reveal());
     }
@@ -60,16 +63,31 @@ class TagServiceTest extends TestCase
         $find->shouldHaveBeenCalled();
     }
 
-    /** @test */
+    /**
-    public function deleteTagsDelegatesOnRepository(): void
+     * @test
+     * @dataProvider provideAdminApiKeys
+     */
+    public function deleteTagsDelegatesOnRepository(?ApiKey $apiKey): void
     {
         $delete = $this->repo->deleteByName(['foo', 'bar'])->willReturn(4);
 
-        $this->service->deleteTags(['foo', 'bar']);
+        $this->service->deleteTags(['foo', 'bar'], $apiKey);
 
         $delete->shouldHaveBeenCalled();
     }
 
+    /** @test */
+    public function deleteTagsThrowsExceptionWhenProvidedApiKeyIsNotAdmin(): void
+    {
+        $delete = $this->repo->deleteByName(['foo', 'bar']);
+
+        $this->expectException(ForbiddenTagOperationException::class);
+        $this->expectExceptionMessage('You are not allowed to delete tags');
+        $delete->shouldNotBeCalled();
+
+        $this->service->deleteTags(['foo', 'bar'], new ApiKey(null, [RoleDefinition::forAuthoredShortUrls()]));
+    }
+
     /** @test */
     public function createTagsPersistsEntities(): void
     {
@@ -85,15 +103,18 @@ class TagServiceTest extends TestCase
         $flush->shouldHaveBeenCalled();
     }
 
-    /** @test */
+    /**
-    public function renameInvalidTagThrowsException(): void
+     * @test
+     * @dataProvider provideAdminApiKeys
+     */
+    public function renameInvalidTagThrowsException(?ApiKey $apiKey): void
     {
         $find = $this->repo->findOneBy(Argument::cetera())->willReturn(null);
 
         $find->shouldBeCalled();
         $this->expectException(TagNotFoundException::class);
 
-        $this->service->renameTag(TagRenaming::fromNames('foo', 'bar'));
+        $this->service->renameTag(TagRenaming::fromNames('foo', 'bar'), $apiKey);
     }
 
     /**
@@ -123,8 +144,11 @@ class TagServiceTest extends TestCase
         yield 'different names names' => ['foo', 'bar', 0];
     }
 
-    /** @test */
+    /**
-    public function renameTagToAnExistingNameThrowsException(): void
+     * @test
+     * @dataProvider provideAdminApiKeys
+     */
+    public function renameTagToAnExistingNameThrowsException(?ApiKey $apiKey): void
     {
         $find = $this->repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo'));
         $countTags = $this->repo->count(Argument::cetera())->willReturn(1);
@@ -135,6 +159,27 @@ class TagServiceTest extends TestCase
         $flush->shouldNotBeCalled();
         $this->expectException(TagConflictException::class);
 
-        $this->service->renameTag(TagRenaming::fromNames('foo', 'bar'));
+        $this->service->renameTag(TagRenaming::fromNames('foo', 'bar'), $apiKey);
+    }
+
+    public function provideAdminApiKeys(): iterable
+    {
+        yield 'no API key' => [null];
+        yield 'admin API key' => [new ApiKey()];
+    }
+
+    /** @test */
+    public function renamingTagThrowsExceptionWhenProvidedApiKeyIsNotAdmin(): void
+    {
+        $getRepo = $this->em->getRepository(Tag::class);
+
+        $this->expectExceptionMessage(ForbiddenTagOperationException::class);
+        $this->expectExceptionMessage('You are not allowed to rename tags');
+        $getRepo->shouldNotBeCalled();
+
+        $this->service->renameTag(
+            TagRenaming::fromNames('foo', 'bar'),
+            new ApiKey(null, [RoleDefinition::forAuthoredShortUrls()]),
+        );
     }
 }

module/Rest/src/Action/Tag/DeleteTagsAction.php

@@ -9,6 +9,7 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
+use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
 
 class DeleteTagsAction extends AbstractRestAction
 {
@@ -26,8 +27,9 @@ class DeleteTagsAction extends AbstractRestAction
     {
         $query = $request->getQueryParams();
         $tags = $query['tags'] ?? [];
+        $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
 
-        $this->tagService->deleteTags($tags);
+        $this->tagService->deleteTags($tags, $apiKey);
         return new EmptyResponse();
     }
 }

module/Rest/src/Action/Tag/UpdateTagAction.php

@@ -10,6 +10,7 @@ use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
 use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
+use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
 
 class UpdateTagAction extends AbstractRestAction
 {
@@ -23,17 +24,12 @@ class UpdateTagAction extends AbstractRestAction
         $this->tagService = $tagService;
     }
 
-    /**
-     * Process an incoming server request and return a response, optionally delegating
-     * to the next middleware component to create the response.
-     *
-     *
-     * @throws \InvalidArgumentException
-     */
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
         $body = $request->getParsedBody();
-        $this->tagService->renameTag(TagRenaming::fromArray($body));
+        $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
+
+        $this->tagService->renameTag(TagRenaming::fromArray($body), $apiKey);
         return new EmptyResponse();
     }
 }

module/Rest/test/Action/Tag/DeleteTagsActionTest.php

@@ -6,10 +6,12 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag;
 
 use Laminas\Diactoros\ServerRequest;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
 use Shlinkio\Shlink\Rest\Action\Tag\DeleteTagsAction;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class DeleteTagsActionTest extends TestCase
 {
@@ -30,8 +32,10 @@ class DeleteTagsActionTest extends TestCase
      */
     public function processDelegatesIntoService(?array $tags): void
     {
-        $request = (new ServerRequest())->withQueryParams(['tags' => $tags]);
+        $request = (new ServerRequest())
-        $deleteTags = $this->tagService->deleteTags($tags ?: []);
+            ->withQueryParams(['tags' => $tags])
+            ->withAttribute(ApiKey::class, new ApiKey());
+        $deleteTags = $this->tagService->deleteTags($tags ?: [], Argument::type(ApiKey::class));
 
         $response = $this->action->handle($request);
 

module/Rest/test/Action/Tag/UpdateTagActionTest.php

@@ -4,15 +4,18 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Rest\Action\Tag;
 
-use Laminas\Diactoros\ServerRequest;
+use Laminas\Diactoros\ServerRequestFactory;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
 use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
 use Shlinkio\Shlink\Rest\Action\Tag\UpdateTagAction;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 class UpdateTagActionTest extends TestCase
 {
@@ -33,7 +36,7 @@ class UpdateTagActionTest extends TestCase
      */
     public function whenInvalidParamsAreProvidedAnErrorIsReturned(array $bodyParams): void
     {
-        $request = (new ServerRequest())->withParsedBody($bodyParams);
+        $request = $this->requestWithApiKey()->withParsedBody($bodyParams);
 
         $this->expectException(ValidationException::class);
 
@@ -50,15 +53,23 @@ class UpdateTagActionTest extends TestCase
     /** @test */
     public function correctInvocationRenamesTag(): void
     {
-        $request = (new ServerRequest())->withParsedBody([
+        $request = $this->requestWithApiKey()->withParsedBody([
             'oldName' => 'foo',
             'newName' => 'bar',
         ]);
-        $rename = $this->tagService->renameTag(TagRenaming::fromNames('foo', 'bar'))->willReturn(new Tag('bar'));
+        $rename = $this->tagService->renameTag(
+            TagRenaming::fromNames('foo', 'bar'),
+            Argument::type(ApiKey::class),
+        )->willReturn(new Tag('bar'));
 
         $resp = $this->action->handle($request);
 
         self::assertEquals(204, $resp->getStatusCode());
         $rename->shouldHaveBeenCalled();
     }
+
+    private function requestWithApiKey(): ServerRequestInterface
+    {
+        return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey());
+    }
 }