diff --git a/module/Core/src/Exception/ForbiddenTagOperationException.php b/module/Core/src/Exception/ForbiddenTagOperationException.php new file mode 100644 index 00000000..d4200c92 --- /dev/null +++ b/module/Core/src/Exception/ForbiddenTagOperationException.php @@ -0,0 +1,39 @@ +detail = $message; + $e->title = self::TITLE; + $e->type = self::TYPE; + $e->status = StatusCodeInterface::STATUS_FORBIDDEN; + + return $e; + } +} diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 5ce2be0f..ae46a312 100644 --- a/module/Core/src/Tag/TagService.php +++ b/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); diff --git a/module/Core/src/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index 7bcf2ace..34cf1871 100644 --- a/module/Core/src/Tag/TagServiceInterface.php +++ b/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; } diff --git a/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php b/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php new file mode 100644 index 00000000..c42f864a --- /dev/null +++ b/module/Core/test/Exception/ForbiddenTagOperationExceptionTest.php @@ -0,0 +1,37 @@ +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']; + } +} diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 842ce9a2..670944f1 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/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()]), + ); } } diff --git a/module/Rest/src/Action/Tag/DeleteTagsAction.php b/module/Rest/src/Action/Tag/DeleteTagsAction.php index aea11a41..b1be8af5 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/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(); } } diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index 77431798..d83d8b9a 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/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(); } } diff --git a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php index b167ee2c..957c01a5 100644 --- a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php +++ b/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]); - $deleteTags = $this->tagService->deleteTags($tags ?: []); + $request = (new ServerRequest()) + ->withQueryParams(['tags' => $tags]) + ->withAttribute(ApiKey::class, new ApiKey()); + $deleteTags = $this->tagService->deleteTags($tags ?: [], Argument::type(ApiKey::class)); $response = $this->action->handle($request); diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index 8546312f..681e68f6 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/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()); + } }