Allow API keys to be renamed

This commit is contained in:
Alejandro Celaya
2024-11-08 08:25:07 +01:00
parent 9e6f129de6
commit a661d05100
14 changed files with 132 additions and 31 deletions

View File

@@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Tag;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Shlinkio\Shlink\Core\Exception\TagConflictException;
use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
@@ -40,7 +40,7 @@ class RenameTagCommand extends Command
$newName = $input->getArgument('newName');
try {
$this->tagService->renameTag(TagRenaming::fromNames($oldName, $newName));
$this->tagService->renameTag(Renaming::fromNames($oldName, $newName));
$io->success('Tag properly renamed.');
return ExitCode::EXIT_SUCCESS;
} catch (TagNotFoundException | TagConflictException $e) {

View File

@@ -9,8 +9,8 @@ use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand;
use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
use ShlinkioTest\Shlink\CLI\Util\CliTestUtils;
use Symfony\Component\Console\Tester\CommandTester;
@@ -32,7 +32,7 @@ class RenameTagCommandTest extends TestCase
$oldName = 'foo';
$newName = 'bar';
$this->tagService->expects($this->once())->method('renameTag')->with(
TagRenaming::fromNames($oldName, $newName),
Renaming::fromNames($oldName, $newName),
)->willThrowException(TagNotFoundException::fromTag('foo'));
$this->commandTester->execute([
@@ -50,7 +50,7 @@ class RenameTagCommandTest extends TestCase
$oldName = 'foo';
$newName = 'bar';
$this->tagService->expects($this->once())->method('renameTag')->with(
TagRenaming::fromNames($oldName, $newName),
Renaming::fromNames($oldName, $newName),
)->willReturn(new Tag($newName));
$this->commandTester->execute([

View File

@@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Core\Exception;
use Fig\Http\Message\StatusCodeInterface;
use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait;
use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Model\Renaming;
use function Shlinkio\Shlink\Core\toProblemDetailsType;
use function sprintf;
@@ -19,7 +19,7 @@ class TagConflictException extends RuntimeException implements ProblemDetailsExc
private const TITLE = 'Tag conflict';
public const ERROR_CODE = 'tag-conflict';
public static function forExistingTag(TagRenaming $renaming): self
public static function forExistingTag(Renaming $renaming): self
{
$e = new self(sprintf('You cannot rename tag %s, because it already exists', $renaming->toString()));

View File

@@ -2,15 +2,15 @@
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Tag\Model;
namespace Shlinkio\Shlink\Core\Model;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use function sprintf;
final class TagRenaming
final readonly class Renaming
{
private function __construct(public readonly string $oldName, public readonly string $newName)
private function __construct(public string $oldName, public string $newName)
{
}

View File

@@ -10,8 +10,8 @@ use Shlinkio\Shlink\Common\Paginator\Paginator;
use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
use Shlinkio\Shlink\Core\Exception\TagConflictException;
use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Tag\Model\TagsParams;
use Shlinkio\Shlink\Core\Tag\Paginator\Adapter\TagsInfoPaginatorAdapter;
use Shlinkio\Shlink\Core\Tag\Paginator\Adapter\TagsPaginatorAdapter;
@@ -74,7 +74,7 @@ readonly class TagService implements TagServiceInterface
/**
* @inheritDoc
*/
public function renameTag(TagRenaming $renaming, ApiKey|null $apiKey = null): Tag
public function renameTag(Renaming $renaming, ApiKey|null $apiKey = null): Tag
{
if (ApiKey::isShortUrlRestricted($apiKey)) {
throw ForbiddenTagOperationException::forRenaming();

View File

@@ -8,9 +8,9 @@ use Shlinkio\Shlink\Common\Paginator\Paginator;
use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
use Shlinkio\Shlink\Core\Exception\TagConflictException;
use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use Shlinkio\Shlink\Core\Tag\Model\TagInfo;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Tag\Model\TagsParams;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
@@ -37,5 +37,5 @@ interface TagServiceInterface
* @throws TagConflictException
* @throws ForbiddenTagOperationException
*/
public function renameTag(TagRenaming $renaming, ApiKey|null $apiKey = null): Tag;
public function renameTag(Renaming $renaming, ApiKey|null $apiKey = null): Tag;
}

View File

@@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Core\Exception;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Exception\TagConflictException;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Model\Renaming;
use function sprintf;
@@ -19,7 +19,7 @@ class TagConflictExceptionTest extends TestCase
$oldName = 'foo';
$newName = 'bar';
$expectedMessage = sprintf('You cannot rename tag %s to %s, because it already exists', $oldName, $newName);
$e = TagConflictException::forExistingTag(TagRenaming::fromNames($oldName, $newName));
$e = TagConflictException::forExistingTag(Renaming::fromNames($oldName, $newName));
self::assertEquals($expectedMessage, $e->getMessage());
self::assertEquals($expectedMessage, $e->getDetail());

View File

@@ -13,9 +13,9 @@ use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException;
use Shlinkio\Shlink\Core\Exception\TagConflictException;
use Shlinkio\Shlink\Core\Exception\TagNotFoundException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use Shlinkio\Shlink\Core\Tag\Model\TagInfo;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering;
use Shlinkio\Shlink\Core\Tag\Model\TagsParams;
use Shlinkio\Shlink\Core\Tag\Repository\TagRepository;
@@ -127,7 +127,7 @@ class TagServiceTest extends TestCase
$this->repo->expects($this->once())->method('findOneBy')->willReturn(null);
$this->expectException(TagNotFoundException::class);
$this->service->renameTag(TagRenaming::fromNames('foo', 'bar'), $apiKey);
$this->service->renameTag(Renaming::fromNames('foo', 'bar'), $apiKey);
}
#[Test, DataProvider('provideValidRenames')]
@@ -139,7 +139,7 @@ class TagServiceTest extends TestCase
$this->repo->expects($this->exactly($count > 0 ? 0 : 1))->method('count')->willReturn($count);
$this->em->expects($this->once())->method('flush');
$tag = $this->service->renameTag(TagRenaming::fromNames($oldName, $newName));
$tag = $this->service->renameTag(Renaming::fromNames($oldName, $newName));
self::assertSame($expected, $tag);
self::assertEquals($newName, (string) $tag);
@@ -160,7 +160,7 @@ class TagServiceTest extends TestCase
$this->expectException(TagConflictException::class);
$this->service->renameTag(TagRenaming::fromNames('foo', 'bar'), $apiKey);
$this->service->renameTag(Renaming::fromNames('foo', 'bar'), $apiKey);
}
#[Test]
@@ -172,7 +172,7 @@ class TagServiceTest extends TestCase
$this->expectExceptionMessage('You are not allowed to rename tags');
$this->service->renameTag(
TagRenaming::fromNames('foo', 'bar'),
Renaming::fromNames('foo', 'bar'),
ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())),
);
}

View File

@@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Rest\Action\Tag;
use Laminas\Diactoros\Response\EmptyResponse;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Tag\Model\TagRenaming;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\TagServiceInterface;
use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
@@ -27,7 +27,7 @@ class UpdateTagAction extends AbstractRestAction
$body = $request->getParsedBody();
$apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
$this->tagService->renameTag(TagRenaming::fromArray($body), $apiKey);
$this->tagService->renameTag(Renaming::fromArray($body), $apiKey);
return new EmptyResponse();
}
}

View File

@@ -23,7 +23,8 @@ class ApiKey extends AbstractEntity
*/
private function __construct(
public readonly string $key,
public readonly string $name,
// TODO Use a property hook to allow public read but private write
public string $name,
public readonly Chronos|null $expirationDate = null,
private bool $enabled = true,
private Collection $roles = new ArrayCollection(),

View File

@@ -6,10 +6,13 @@ namespace Shlinkio\Shlink\Rest\Service;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use function sprintf;
readonly class ApiKeyService implements ApiKeyServiceInterface
{
public function __construct(private EntityManagerInterface $em, private ApiKeyRepositoryInterface $repo)
@@ -74,11 +77,6 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
return $this->repo->findBy($conditions);
}
private function findByKey(string $key): ApiKey|null
{
return $this->repo->findOneBy(['key' => ApiKey::hashKey($key)]);
}
/**
* @inheritDoc
*/
@@ -86,4 +84,37 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
{
return $this->repo->count(['name' => $apiKeyName]) > 0;
}
/**
* @inheritDoc
*/
public function renameApiKey(Renaming $apiKeyRenaming): ApiKey
{
$apiKey = $this->repo->findOneBy(['name' => $apiKeyRenaming->oldName]);
if ($apiKey === null) {
throw new InvalidArgumentException(
sprintf('API key with name "%s" could not be found', $apiKeyRenaming->oldName),
);
}
if (! $apiKeyRenaming->nameChanged()) {
return $apiKey;
}
if ($this->existsWithName($apiKeyRenaming->newName)) {
throw new InvalidArgumentException(
sprintf('Another API key with name "%s" already exists', $apiKeyRenaming->newName),
);
}
$apiKey->name = $apiKeyRenaming->newName;
$this->em->flush();
return $apiKey;
}
private function findByKey(string $key): ApiKey|null
{
return $this->repo->findOneBy(['key' => ApiKey::hashKey($key)]);
}
}

View File

@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Service;
use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
@@ -36,4 +37,9 @@ interface ApiKeyServiceInterface
* Check if an API key exists for provided name
*/
public function existsWithName(string $apiKeyName): bool;
/**
* @throws InvalidArgumentException If an API key with oldName does not exist, or newName is in use by another one
*/
public function renameApiKey(Renaming $apiKeyRenaming): ApiKey;
}

View File

@@ -11,8 +11,8 @@ use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Core\Tag\Entity\Tag;
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;
@@ -53,7 +53,7 @@ class UpdateTagActionTest extends TestCase
'newName' => 'bar',
]);
$this->tagService->expects($this->once())->method('renameTag')->with(
TagRenaming::fromNames('foo', 'bar'),
Renaming::fromNames('foo', 'bar'),
$this->isInstanceOf(ApiKey::class),
)->willReturn(new Tag('bar'));

View File

@@ -13,6 +13,7 @@ use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Model\Renaming;
use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface;
@@ -188,4 +189,66 @@ class ApiKeyServiceTest extends TestCase
$this->repo->expects($this->once())->method('count')->with(['name' => $name])->willReturn($count);
self::assertEquals($this->service->existsWithName($name), $expected);
}
#[Test]
public function renameApiKeyThrowsExceptionIfApiKeyIsNotFound(): void
{
$renaming = Renaming::fromNames(oldName: 'old', newName: 'new');
$this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn(null);
$this->repo->expects($this->never())->method('count');
$this->em->expects($this->never())->method('flush');
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('API key with name "old" could not be found');
$this->service->renameApiKey($renaming);
}
#[Test]
public function renameApiKeyReturnsApiKeyVerbatimIfBothNamesAreEqual(): void
{
$renaming = Renaming::fromNames(oldName: 'same_value', newName: 'same_value');
$apiKey = ApiKey::create();
$this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'same_value'])->willReturn($apiKey);
$this->repo->expects($this->never())->method('count');
$this->em->expects($this->never())->method('flush');
$result = $this->service->renameApiKey($renaming);
self::assertSame($apiKey, $result);
}
#[Test]
public function renameApiKeyThrowsExceptionIfNewNameIsInUse(): void
{
$renaming = Renaming::fromNames(oldName: 'old', newName: 'new');
$apiKey = ApiKey::create();
$this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn($apiKey);
$this->repo->expects($this->once())->method('count')->with(['name' => 'new'])->willReturn(1);
$this->em->expects($this->never())->method('flush');
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Another API key with name "new" already exists');
$this->service->renameApiKey($renaming);
}
#[Test]
public function renameApiKeyReturnsApiKeyWithNewName(): void
{
$renaming = Renaming::fromNames(oldName: 'old', newName: 'new');
$apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'old'));
$this->repo->expects($this->once())->method('findOneBy')->with(['name' => 'old'])->willReturn($apiKey);
$this->repo->expects($this->once())->method('count')->with(['name' => 'new'])->willReturn(0);
$this->em->expects($this->once())->method('flush');
$result = $this->service->renameApiKey($renaming);
self::assertSame($apiKey, $result);
self::assertEquals('new', $apiKey->name);
}
}