Fall back API key names to auto-generated keys

This commit is contained in:
Alejandro Celaya 2024-11-05 11:26:39 +01:00
parent 819a535bfe
commit a094be2b9e
11 changed files with 139 additions and 111 deletions

View File

@ -102,19 +102,24 @@ class GenerateKeyCommand extends Command
{ {
$expirationDate = $input->getOption('expiration-date'); $expirationDate = $input->getOption('expiration-date');
$apiKey = $this->apiKeyService->create(ApiKeyMeta::fromParams( $apiKeyMeta = ApiKeyMeta::fromParams(
name: $input->getOption('name'), name: $input->getOption('name'),
expirationDate: isset($expirationDate) ? Chronos::parse($expirationDate) : null, expirationDate: isset($expirationDate) ? Chronos::parse($expirationDate) : null,
roleDefinitions: $this->roleResolver->determineRoles($input), roleDefinitions: $this->roleResolver->determineRoles($input),
)); );
$apiKey = $this->apiKeyService->create($apiKeyMeta);
$io = new SymfonyStyle($input, $output); $io = new SymfonyStyle($input, $output);
$io->success(sprintf('Generated API key: "%s"', $apiKey->key)); $io->success(sprintf('Generated API key: "%s"', $apiKeyMeta->key));
if ($input->isInteractive()) {
$io->warning('Save the key in a secure location. You will not be able to get it afterwards.');
}
if (! ApiKey::isAdmin($apiKey)) { if (! ApiKey::isAdmin($apiKey)) {
ShlinkTable::default($io)->render( ShlinkTable::default($io)->render(
['Role name', 'Role metadata'], ['Role name', 'Role metadata'],
$apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, 0)]), $apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, indentSize: 0)]),
null, null,
'Roles', 'Roles',
); );

View File

@ -54,7 +54,7 @@ class ListKeysCommand extends Command
$messagePattern = $this->determineMessagePattern($apiKey); $messagePattern = $this->determineMessagePattern($apiKey);
// Set columns for this row // Set columns for this row
$rowData = [sprintf($messagePattern, $apiKey->key), sprintf($messagePattern, $apiKey->name ?? '-')]; $rowData = [sprintf($messagePattern, $apiKey->name ?? '-')];
if (! $enabledOnly) { if (! $enabledOnly) {
$rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey)); $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
} }
@ -67,7 +67,6 @@ class ListKeysCommand extends Command
}, $this->apiKeyService->listKeys($enabledOnly)); }, $this->apiKeyService->listKeys($enabledOnly));
ShlinkTable::withRowSeparators($output)->render(array_filter([ ShlinkTable::withRowSeparators($output)->render(array_filter([
'Key',
'Name', 'Name',
! $enabledOnly ? 'Is enabled' : null, ! $enabledOnly ? 'Is enabled' : null,
'Expiration date', 'Expiration date',

View File

@ -26,38 +26,38 @@ class ListApiKeysTest extends CliTestCase
{ {
$expiredApiKeyDate = Chronos::now()->subDays(1)->startOfDay()->toAtomString(); $expiredApiKeyDate = Chronos::now()->subDays(1)->startOfDay()->toAtomString();
$enabledOnlyOutput = <<<OUT $enabledOnlyOutput = <<<OUT
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| Key | Name | Expiration date | Roles | | Name | Expiration date | Roles |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| valid_api_key | - | - | Admin | | valid_api_key | - | Admin |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| expired_api_key | - | {$expiredApiKeyDate} | Admin | | expired_api_key | {$expiredApiKeyDate} | Admin |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| author_api_key | - | - | Author only | | author_api_key | - | Author only |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| domain_api_key | - | - | Domain only: example.com | | domain_api_key | - | Domain only: example.com |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
| no_orphans_api_key | - | - | No orphan visits | | no_orphans_api_key | - | No orphan visits |
+--------------------+------+---------------------------+--------------------------+ +--------------------+---------------------------+--------------------------+
OUT; OUT;
yield 'no flags' => [[], <<<OUT yield 'no flags' => [[], <<<OUT
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| Key | Name | Is enabled | Expiration date | Roles | | Name | Is enabled | Expiration date | Roles |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| valid_api_key | - | +++ | - | Admin | | valid_api_key | +++ | - | Admin |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| disabled_api_key | - | --- | - | Admin | | disabled_api_key | --- | - | Admin |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| expired_api_key | - | --- | {$expiredApiKeyDate} | Admin | | expired_api_key | --- | {$expiredApiKeyDate} | Admin |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| author_api_key | - | +++ | - | Author only | | author_api_key | +++ | - | Author only |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| domain_api_key | - | +++ | - | Domain only: example.com | | domain_api_key | +++ | - | Domain only: example.com |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
| no_orphans_api_key | - | +++ | - | No orphan visits | | no_orphans_api_key | +++ | - | No orphan visits |
+--------------------+------+------------+---------------------------+--------------------------+ +--------------------+------------+---------------------------+--------------------------+
OUT]; OUT];
yield '-e' => [['-e'], $enabledOnlyOutput]; yield '-e' => [['-e'], $enabledOnlyOutput];

View File

@ -36,7 +36,7 @@ class GenerateKeyCommandTest extends TestCase
public function noExpirationDateIsDefinedIfNotProvided(): void public function noExpirationDateIsDefinedIfNotProvided(): void
{ {
$this->apiKeyService->expects($this->once())->method('create')->with( $this->apiKeyService->expects($this->once())->method('create')->with(
$this->callback(fn (ApiKeyMeta $meta) => $meta->name === null && $meta->expirationDate === null), $this->callback(fn (ApiKeyMeta $meta) => $meta->expirationDate === null),
)->willReturn(ApiKey::create()); )->willReturn(ApiKey::create());
$this->commandTester->execute([]); $this->commandTester->execute([]);

View File

@ -52,15 +52,15 @@ class ListKeysCommandTest extends TestCase
], ],
false, false,
<<<OUTPUT <<<OUTPUT
+--------------------------------------+------+------------+---------------------------+-------+ +--------------------------------------+------------+---------------------------+-------+
| Key | Name | Is enabled | Expiration date | Roles | | Name | Is enabled | Expiration date | Roles |
+--------------------------------------+------+------------+---------------------------+-------+ +--------------------------------------+------------+---------------------------+-------+
| {$apiKey1->key} | - | --- | - | Admin | | {$apiKey1->name} | --- | - | Admin |
+--------------------------------------+------+------------+---------------------------+-------+ +--------------------------------------+------------+---------------------------+-------+
| {$apiKey2->key} | - | --- | 2020-01-01T00:00:00+00:00 | Admin | | {$apiKey2->name} | --- | 2020-01-01T00:00:00+00:00 | Admin |
+--------------------------------------+------+------------+---------------------------+-------+ +--------------------------------------+------------+---------------------------+-------+
| {$apiKey3->key} | - | +++ | - | Admin | | {$apiKey3->name} | +++ | - | Admin |
+--------------------------------------+------+------------+---------------------------+-------+ +--------------------------------------+------------+---------------------------+-------+
OUTPUT, OUTPUT,
]; ];
@ -68,13 +68,13 @@ class ListKeysCommandTest extends TestCase
[$apiKey1 = ApiKey::create()->disable(), $apiKey2 = ApiKey::create()], [$apiKey1 = ApiKey::create()->disable(), $apiKey2 = ApiKey::create()],
true, true,
<<<OUTPUT <<<OUTPUT
+--------------------------------------+------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| Key | Name | Expiration date | Roles | | Name | Expiration date | Roles |
+--------------------------------------+------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey1->key} | - | - | Admin | | {$apiKey1->name} | - | Admin |
+--------------------------------------+------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey2->key} | - | - | Admin | | {$apiKey2->name} | - | Admin |
+--------------------------------------+------+-----------------+-------+ +--------------------------------------+-----------------+-------+
OUTPUT, OUTPUT,
]; ];
@ -94,45 +94,45 @@ class ListKeysCommandTest extends TestCase
], ],
true, true,
<<<OUTPUT <<<OUTPUT
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| Key | Name | Expiration date | Roles | | Name | Expiration date | Roles |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey1->key} | - | - | Admin | | {$apiKey1->name} | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey2->key} | - | - | Author only | | {$apiKey2->name} | - | Author only |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey3->key} | - | - | Domain only: example.com | | {$apiKey3->name} | - | Domain only: example.com |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey4->key} | - | - | Admin | | {$apiKey4->name} | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey5->key} | - | - | Author only | | {$apiKey5->name} | - | Author only |
| | | | Domain only: example.com | | | | Domain only: example.com |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
| {$apiKey6->key} | - | - | Admin | | {$apiKey6->name} | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+ +--------------------------------------+-----------------+--------------------------+
OUTPUT, OUTPUT,
]; ];
yield 'with names' => [ yield 'with names' => [
[ [
$apiKey1 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice')), ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice')),
$apiKey2 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice and Bob')), ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice and Bob')),
$apiKey3 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: '')), $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: '')),
$apiKey4 = ApiKey::create(), $apiKey4 = ApiKey::create(),
], ],
true, true,
<<<OUTPUT <<<OUTPUT
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| Key | Name | Expiration date | Roles | | Name | Expiration date | Roles |
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey1->key} | Alice | - | Admin | | Alice | - | Admin |
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey2->key} | Alice and Bob | - | Admin | | Alice and Bob | - | Admin |
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey3->key} | | - | Admin | | {$apiKey3->name} | - | Admin |
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
| {$apiKey4->key} | - | - | Admin | | {$apiKey4->name} | - | Admin |
+--------------------------------------+---------------+-----------------+-------+ +--------------------------------------+-----------------+-------+
OUTPUT, OUTPUT,
]; ];

View File

@ -7,6 +7,9 @@ namespace Shlinkio\Shlink\Rest\ApiKey\Model;
use Cake\Chronos\Chronos; use Cake\Chronos\Chronos;
use Ramsey\Uuid\Uuid; use Ramsey\Uuid\Uuid;
use function sprintf;
use function substr;
final readonly class ApiKeyMeta final readonly class ApiKeyMeta
{ {
/** /**
@ -14,7 +17,7 @@ final readonly class ApiKeyMeta
*/ */
private function __construct( private function __construct(
public string $key, public string $key,
public string|null $name, public string $name,
public Chronos|null $expirationDate, public Chronos|null $expirationDate,
public iterable $roleDefinitions, public iterable $roleDefinitions,
) { ) {
@ -34,8 +37,19 @@ final readonly class ApiKeyMeta
Chronos|null $expirationDate = null, Chronos|null $expirationDate = null,
iterable $roleDefinitions = [], iterable $roleDefinitions = [],
): self { ): self {
$resolvedKey = $key ?? Uuid::uuid4()->toString();
// If a name was not provided, fall back to the key
if (empty($name)) {
// If the key was auto-generated, fall back to a "censored" version of the UUID, otherwise simply use the
// plain key as fallback name
$name = $key === null
? sprintf('%s-****-****-****-************', substr($resolvedKey, offset: 0, length: 8))
: $key;
}
return new self( return new self(
key: $key ?? Uuid::uuid4()->toString(), key: $resolvedKey,
name: $name, name: $name,
expirationDate: $expirationDate, expirationDate: $expirationDate,
roleDefinitions: $roleDefinitions, roleDefinitions: $roleDefinitions,

View File

@ -15,31 +15,30 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRepositoryInterface class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRepositoryInterface
{ {
/** /**
* Will create provided API key with admin permissions, only if there's no other API keys yet * Will create provided API key with admin permissions, only if no other API keys exist yet
*/ */
public function createInitialApiKey(string $apiKey): ApiKey|null public function createInitialApiKey(string $apiKey): ApiKey|null
{ {
$em = $this->getEntityManager(); $em = $this->getEntityManager();
return $em->wrapInTransaction(function () use ($apiKey, $em): ApiKey|null { return $em->wrapInTransaction(function () use ($apiKey, $em): ApiKey|null {
// Ideally this would be a SELECT COUNT(...), but MsSQL and Postgres do not allow locking on aggregates $firstResult = $em->createQueryBuilder()
// Because of that we check if at least one result exists ->select('a.id')
$firstResult = $em->createQueryBuilder()->select('a.id') ->from(ApiKey::class, 'a')
->from(ApiKey::class, 'a') ->setMaxResults(1)
->setMaxResults(1) ->getQuery()
->getQuery() ->setLockMode(LockMode::PESSIMISTIC_WRITE)
->setLockMode(LockMode::PESSIMISTIC_WRITE) ->getOneOrNullResult();
->getOneOrNullResult();
// Do not create an initial API key if other keys already exist // Do not create an initial API key if other keys already exist
if ($firstResult !== null) { if ($firstResult !== null) {
return null; return null;
} }
$new = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey)); $initialApiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey));
$em->persist($new); $em->persist($initialApiKey);
$em->flush(); $em->flush();
return $new; return $initialApiKey;
}); });
} }
} }

View File

@ -15,6 +15,8 @@ use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\ApiKey\Role;
use function hash;
class ApiKey extends AbstractEntity class ApiKey extends AbstractEntity
{ {
/** /**
@ -42,6 +44,7 @@ class ApiKey extends AbstractEntity
*/ */
public static function fromMeta(ApiKeyMeta $meta): self public static function fromMeta(ApiKeyMeta $meta): self
{ {
// $apiKey = new self(self::hashKey($meta->key), $meta->name, $meta->expirationDate);
$apiKey = new self($meta->key, $meta->name, $meta->expirationDate); $apiKey = new self($meta->key, $meta->name, $meta->expirationDate);
foreach ($meta->roleDefinitions as $roleDefinition) { foreach ($meta->roleDefinitions as $roleDefinition) {
$apiKey->registerRole($roleDefinition); $apiKey->registerRole($roleDefinition);
@ -50,6 +53,14 @@ class ApiKey extends AbstractEntity
return $apiKey; return $apiKey;
} }
/**
* Generates a hash for provided key, in the way Shlink expects API keys to be hashed
*/
public static function hashKey(string $key): string
{
return hash('sha256', $key);
}
public function isExpired(): bool public function isExpired(): bool
{ {
return $this->expirationDate !== null && $this->expirationDate->lessThan(Chronos::now()); return $this->expirationDate !== null && $this->expirationDate->lessThan(Chronos::now());

View File

@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Rest\Service;
use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Entity\ApiKey;
final class ApiKeyCheckResult final readonly class ApiKeyCheckResult
{ {
public function __construct(public readonly ApiKey|null $apiKey = null) public function __construct(public ApiKey|null $apiKey = null)
{ {
} }

View File

@ -10,11 +10,9 @@ use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface; use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Entity\ApiKey;
use function sprintf; readonly class ApiKeyService implements ApiKeyServiceInterface
class ApiKeyService implements ApiKeyServiceInterface
{ {
public function __construct(private readonly EntityManagerInterface $em) public function __construct(private EntityManagerInterface $em)
{ {
} }
@ -48,11 +46,12 @@ class ApiKeyService implements ApiKeyServiceInterface
{ {
$apiKey = $this->getByKey($key); $apiKey = $this->getByKey($key);
if ($apiKey === null) { if ($apiKey === null) {
throw new InvalidArgumentException(sprintf('API key "%s" does not exist and can\'t be disabled', $key)); throw new InvalidArgumentException('Provided API key does not exist and can\'t be disabled');
} }
$apiKey->disable(); $apiKey->disable();
$this->em->flush(); $this->em->flush();
return $apiKey; return $apiKey;
} }
@ -62,17 +61,14 @@ class ApiKeyService implements ApiKeyServiceInterface
public function listKeys(bool $enabledOnly = false): array public function listKeys(bool $enabledOnly = false): array
{ {
$conditions = $enabledOnly ? ['enabled' => true] : []; $conditions = $enabledOnly ? ['enabled' => true] : [];
/** @var ApiKey[] $apiKeys */ return $this->em->getRepository(ApiKey::class)->findBy($conditions);
$apiKeys = $this->em->getRepository(ApiKey::class)->findBy($conditions);
return $apiKeys;
} }
private function getByKey(string $key): ApiKey|null private function getByKey(string $key): ApiKey|null
{ {
/** @var ApiKey|null $apiKey */ return $this->em->getRepository(ApiKey::class)->findOneBy([
$apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ // 'key' => ApiKey::hashKey($key),
'key' => $key, 'key' => $key,
]); ]);
return $apiKey;
} }
} }

View File

@ -18,6 +18,8 @@ use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepository;
use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Service\ApiKeyService; use Shlinkio\Shlink\Rest\Service\ApiKeyService;
use function substr;
class ApiKeyServiceTest extends TestCase class ApiKeyServiceTest extends TestCase
{ {
private ApiKeyService $service; private ApiKeyService $service;
@ -40,12 +42,14 @@ class ApiKeyServiceTest extends TestCase
$this->em->expects($this->once())->method('flush'); $this->em->expects($this->once())->method('flush');
$this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class)); $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class));
$key = $this->service->create( $meta = ApiKeyMeta::fromParams(name: $name, expirationDate: $date, roleDefinitions: $roles);
ApiKeyMeta::fromParams(name: $name, expirationDate: $date, roleDefinitions: $roles), $key = $this->service->create($meta);
);
self::assertEquals($date, $key->expirationDate); self::assertEquals($date, $key->expirationDate);
self::assertEquals($name, $key->name); self::assertEquals(
empty($name) ? substr($meta->key, 0, 8) . '-****-****-****-************' : $name,
$key->name,
);
foreach ($roles as $roleDefinition) { foreach ($roles as $roleDefinition) {
self::assertTrue($key->hasRole($roleDefinition->role)); self::assertTrue($key->hasRole($roleDefinition->role));
} }