Ensure auto-generated name API keys do not throw duplicated name

This commit is contained in:
Alejandro Celaya 2024-11-09 12:07:07 +01:00
parent d228b88e51
commit 3c6f12aec6
5 changed files with 53 additions and 12 deletions

View File

@ -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,
);

View File

@ -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

View File

@ -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);

View File

@ -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;

View File

@ -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');