diff --git a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php index 66d7d889..ae1b189c 100644 --- a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php +++ b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php @@ -18,12 +18,13 @@ final readonly class ApiKeyMeta private function __construct( public string $key, public string $name, + public bool $isNameAutoGenerated, public Chronos|null $expirationDate, public iterable $roleDefinitions, ) { } - public static function empty(): self + public static function create(): self { return self::fromParams(); } @@ -38,9 +39,10 @@ final readonly class ApiKeyMeta iterable $roleDefinitions = [], ): self { $resolvedKey = $key ?? Uuid::uuid4()->toString(); + $isNameAutoGenerated = empty($name); // If a name was not provided, fall back to the key - if (empty($name)) { + if ($isNameAutoGenerated) { // If the key was auto-generated, fall back to a redacted version of the UUID, otherwise simply use the // plain key as fallback name $name = $key === null @@ -51,6 +53,7 @@ final readonly class ApiKeyMeta return new self( key: $resolvedKey, name: $name, + isNameAutoGenerated: $isNameAutoGenerated, expirationDate: $expirationDate, roleDefinitions: $roleDefinitions, ); diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index c9cdd3a6..63bb6fc9 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -33,7 +33,7 @@ class ApiKey extends AbstractEntity public static function create(): ApiKey { - return self::fromMeta(ApiKeyMeta::empty()); + return self::fromMeta(ApiKeyMeta::create()); } public static function fromMeta(ApiKeyMeta $meta): self diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 19140534..09d1bb76 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -19,17 +19,13 @@ readonly class ApiKeyService implements ApiKeyServiceInterface { } + /** + * @inheritDoc + */ public function create(ApiKeyMeta $apiKeyMeta): ApiKey { return $this->em->wrapInTransaction(function () use ($apiKeyMeta) { - $apiKey = ApiKey::fromMeta($apiKeyMeta); - // TODO If name is auto-generated, do not throw. Instead, re-generate a new key - if ($this->repo->nameExists($apiKey->name)) { - throw new InvalidArgumentException( - sprintf('Another API key with name "%s" already exists', $apiKeyMeta->name), - ); - } - + $apiKey = ApiKey::fromMeta($this->ensureUniqueName($apiKeyMeta)); $this->em->persist($apiKey); $this->em->flush(); @@ -37,6 +33,29 @@ readonly class ApiKeyService implements ApiKeyServiceInterface }); } + /** + * Given an ApiKeyMeta object, it returns another instance ensuring the name is unique. + * - If the name was auto-generated, it continues re-trying until a unique name is resolved. + * - If the name was explicitly provided, it throws in case of name conflict. + */ + private function ensureUniqueName(ApiKeyMeta $apiKeyMeta): ApiKeyMeta + { + if (! $this->repo->nameExists($apiKeyMeta->name)) { + return $apiKeyMeta; + } + + if (! $apiKeyMeta->isNameAutoGenerated) { + throw new InvalidArgumentException( + sprintf('Another API key with name "%s" already exists', $apiKeyMeta->name), + ); + } + + return $this->ensureUniqueName(ApiKeyMeta::fromParams( + expirationDate: $apiKeyMeta->expirationDate, + roleDefinitions: $apiKeyMeta->roleDefinitions, + )); + } + public function createInitial(string $key): ApiKey|null { return $this->repo->createInitialApiKey($key); diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index c42505b7..be7b9191 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -11,6 +11,9 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ApiKeyServiceInterface { + /** + * @throws InvalidArgumentException + */ public function create(ApiKeyMeta $apiKeyMeta): ApiKey; public function createInitial(string $key): ApiKey|null; diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 2b3d4f96..ee33e109 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -78,7 +78,23 @@ class ApiKeyServiceTest extends TestCase } #[Test] - public function exceptionIsThrownWhileCreatingIfNameIsInUse(): void + public function autoGeneratedNameIsRegeneratedIfAlreadyExists(): void + { + $callCount = 0; + $this->repo->expects($this->exactly(3))->method('nameExists')->with( + $this->isType('string'), + )->willReturnCallback(function () use (&$callCount): bool { + $callCount++; + return $callCount < 3; + }); + $this->em->expects($this->once())->method('flush'); + $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class)); + + $this->service->create(ApiKeyMeta::create()); + } + + #[Test] + public function exceptionIsThrownWhileCreatingIfExplicitlyProvidedNameIsInUse(): void { $this->repo->expects($this->once())->method('nameExists')->with('the_name')->willReturn(true); $this->em->expects($this->never())->method('flush');