Feature/name api keys

This commit is contained in:
KetchupBomb 2021-03-06 17:27:34 +00:00
parent 65f2ab6720
commit b93b14986e
10 changed files with 163 additions and 40 deletions

View file

@ -0,0 +1,45 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Types\Types;
use Doctrine\Migrations\AbstractMigration;
final class Version20210306165711 extends AbstractMigration
{
private const TABLE = 'api_keys';
private const COLUMN = 'name';
public function up(Schema $schema): void
{
$apiKeys = $schema->getTable(self::TABLE);
$this->skipIf($apiKeys->hasColumn(self::COLUMN));
$apiKeys->addColumn(
self::COLUMN,
Types::STRING,
[
'notnull' => false,
],
);
}
public function down(Schema $schema): void
{
$apiKeys = $schema->getTable(self::TABLE);
$this->skipIf(! $apiKeys->hasColumn(self::COLUMN));
$apiKeys->dropColumn(self::COLUMN);
}
/**
* @fixme Workaround for https://github.com/doctrine/migrations/issues/1104
*/
public function isTransactional(): bool
{
return false;
}
}

View file

@ -42,6 +42,10 @@ class GenerateKeyCommand extends BaseCommand
<info>%command.full_name%</info>
You can optionally set its name for tracking purposes with <comment>--name</comment> or <comment>-m</comment>:
<info>%command.full_name% --name Alice</info>
You can optionally set its expiration date with <comment>--expiration-date</comment> or <comment>-e</comment>:
<info>%command.full_name% --expiration-date 2020-01-01</info>
@ -56,6 +60,12 @@ class GenerateKeyCommand extends BaseCommand
$this
->setName(self::NAME)
->setDescription('Generates a new valid API key.')
->addOption(
'name',
'm',
InputOption::VALUE_REQUIRED,
'The name by which this API key will be known.',
)
->addOptionWithDeprecatedFallback(
'expiration-date',
'e',
@ -82,6 +92,7 @@ class GenerateKeyCommand extends BaseCommand
$expirationDate = $this->getOptionWithDeprecatedFallback($input, 'expiration-date');
$apiKey = $this->apiKeyService->create(
isset($expirationDate) ? Chronos::parse($expirationDate) : null,
$input->getOption('name'),
...$this->roleResolver->determineRoles($input),
);

View file

@ -58,6 +58,7 @@ class ListKeysCommand extends BaseCommand
// Set columns for this row
$rowData = [sprintf($messagePattern, $apiKey)];
$rowData[] = $apiKey->name() ?? '-';
if (! $enabledOnly) {
$rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
}
@ -74,10 +75,12 @@ class ListKeysCommand extends BaseCommand
ShlinkTable::fromOutput($output)->render(array_filter([
'Key',
'Name',
! $enabledOnly ? 'Is enabled' : null,
'Expiration date',
'Roles',
]), $rows);
return ExitCodes::EXIT_SUCCESS;
}

View file

@ -40,22 +40,43 @@ class GenerateKeyCommandTest extends TestCase
/** @test */
public function noExpirationDateIsDefinedIfNotProvided(): void
{
$create = $this->apiKeyService->create(null)->willReturn(new ApiKey());
$this->apiKeyService->create(
null, // Expiration date
null, // Name
)->shouldBeCalledOnce()
->willReturn(new ApiKey());
$this->commandTester->execute([]);
$output = $this->commandTester->getDisplay();
self::assertStringContainsString('Generated API key: ', $output);
$create->shouldHaveBeenCalledOnce();
}
/** @test */
public function expirationDateIsDefinedIfProvided(): void
{
$this->apiKeyService->create(Argument::type(Chronos::class))->shouldBeCalledOnce()
->willReturn(new ApiKey());
$this->apiKeyService->create(
Argument::type(Chronos::class), // Expiration date
null, // Name
)->shouldBeCalledOnce()
->willReturn(new ApiKey());
$this->commandTester->execute([
'--expiration-date' => '2016-01-01',
]);
}
/** @test */
public function nameIsDefinedIfProvided(): void
{
$this->apiKeyService->create(
null, // Expiration date
Argument::type('string'), // Name
)->shouldBeCalledOnce()
->willReturn(new ApiKey());
$this->commandTester->execute([
'--name' => 'Alice',
]);
}
}

View file

@ -52,13 +52,13 @@ class ListKeysCommandTest extends TestCase
[ApiKey::withKey('foo'), ApiKey::withKey('bar'), ApiKey::withKey('baz')],
false,
<<<OUTPUT
+-----+------------+-----------------+-------+
| Key | Is enabled | Expiration date | Roles |
+-----+------------+-----------------+-------+
| foo | +++ | - | Admin |
| bar | +++ | - | Admin |
| baz | +++ | - | Admin |
+-----+------------+-----------------+-------+
+-----+------+------------+-----------------+-------+
| Key | Name | Is enabled | Expiration date | Roles |
+-----+------+------------+-----------------+-------+
| foo | - | +++ | - | Admin |
| bar | - | +++ | - | Admin |
| baz | - | +++ | - | Admin |
+-----+------+------------+-----------------+-------+
OUTPUT,
];
@ -66,12 +66,12 @@ class ListKeysCommandTest extends TestCase
[ApiKey::withKey('foo')->disable(), ApiKey::withKey('bar')],
true,
<<<OUTPUT
+-----+-----------------+-------+
| Key | Expiration date | Roles |
+-----+-----------------+-------+
| foo | - | Admin |
| bar | - | Admin |
+-----+-----------------+-------+
+-----+------+-----------------+-------+
| Key | Name | Expiration date | Roles |
+-----+------+-----------------+-------+
| foo | - | - | Admin |
| bar | - | - | Admin |
+-----+------+-----------------+-------+
OUTPUT,
];
@ -89,17 +89,37 @@ class ListKeysCommandTest extends TestCase
],
true,
<<<OUTPUT
+------+-----------------+--------------------------+
| Key | Expiration date | Roles |
+------+-----------------+--------------------------+
| foo | - | Admin |
| bar | - | Author only |
| baz | - | Domain only: example.com |
| foo2 | - | Admin |
| baz2 | - | Author only |
| | | Domain only: example.com |
| foo3 | - | Admin |
+------+-----------------+--------------------------+
+------+------+-----------------+--------------------------+
| Key | Name | Expiration date | Roles |
+------+------+-----------------+--------------------------+
| foo | - | - | Admin |
| bar | - | - | Author only |
| baz | - | - | Domain only: example.com |
| foo2 | - | - | Admin |
| baz2 | - | - | Author only |
| | | | Domain only: example.com |
| foo3 | - | - | Admin |
+------+------+-----------------+--------------------------+
OUTPUT,
];
yield 'with names' => [
[
ApiKey::withKey('abc', null, 'Alice'),
ApiKey::withKey('def', null, 'Alice and Bob'),
ApiKey::withKey('ghi', null, ''),
ApiKey::withKey('jkl', null, null),
],
true,
<<<OUTPUT
+-----+---------------+-----------------+-------+
| Key | Name | Expiration date | Roles |
+-----+---------------+-----------------+-------+
| abc | Alice | - | Admin |
| def | Alice and Bob | - | Admin |
| ghi | | - | Admin |
| jkl | - | - | Admin |
+-----+---------------+-----------------+-------+
OUTPUT,
];

View file

@ -28,6 +28,11 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
->unique()
->build();
$builder->createField('name', Types::STRING)
->columnName('`name`')
->nullable()
->build();
$builder->createField('expirationDate', ChronosDateTimeType::CHRONOS_DATETIME)
->columnName('expiration_date')
->nullable()

View file

@ -22,14 +22,16 @@ class ApiKey extends AbstractEntity
private bool $enabled;
/** @var Collection|ApiKeyRole[] */
private Collection $roles;
private ?string $name;
/**
* @throws Exception
*/
public function __construct(?Chronos $expirationDate = null)
public function __construct(?Chronos $expirationDate = null, ?string $name = null)
{
$this->key = Uuid::uuid4()->toString();
$this->expirationDate = $expirationDate;
$this->name = $name;
$this->enabled = true;
$this->roles = new ArrayCollection();
}
@ -45,9 +47,9 @@ class ApiKey extends AbstractEntity
return $apiKey;
}
public static function withKey(string $key, ?Chronos $expirationDate = null): self
public static function withKey(string $key, ?Chronos $expirationDate = null, ?string $name = null): self
{
$apiKey = new self($expirationDate);
$apiKey = new self($expirationDate, $name);
$apiKey->key = $key;
return $apiKey;
@ -63,6 +65,11 @@ class ApiKey extends AbstractEntity
return $this->expirationDate !== null && $this->expirationDate->lt(Chronos::now());
}
public function name(): ?string
{
return $this->name;
}
public function isEnabled(): bool
{
return $this->enabled;

View file

@ -21,9 +21,12 @@ class ApiKeyService implements ApiKeyServiceInterface
$this->em = $em;
}
public function create(?Chronos $expirationDate = null, RoleDefinition ...$roleDefinitions): ApiKey
{
$key = new ApiKey($expirationDate);
public function create(
?Chronos $expirationDate = null,
?string $name = null,
RoleDefinition ...$roleDefinitions
): ApiKey {
$key = new ApiKey($expirationDate, $name);
foreach ($roleDefinitions as $definition) {
$key->registerRole($definition);
}

View file

@ -11,7 +11,11 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
interface ApiKeyServiceInterface
{
public function create(?Chronos $expirationDate = null, RoleDefinition ...$roleDefinitions): ApiKey;
public function create(
?Chronos $expirationDate = null,
?string $name = null,
RoleDefinition ...$roleDefinitions
): ApiKey;
public function check(string $key): ApiKeyCheckResult;

View file

@ -35,14 +35,15 @@ class ApiKeyServiceTest extends TestCase
* @dataProvider provideCreationDate
* @param RoleDefinition[] $roles
*/
public function apiKeyIsProperlyCreated(?Chronos $date, array $roles): void
public function apiKeyIsProperlyCreated(?Chronos $date, ?string $name, array $roles): void
{
$this->em->flush()->shouldBeCalledOnce();
$this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledOnce();
$key = $this->service->create($date, ...$roles);
$key = $this->service->create($date, $name, ...$roles);
self::assertEquals($date, $key->getExpirationDate());
self::assertEquals($name, $key->name());
foreach ($roles as $roleDefinition) {
self::assertTrue($key->hasRole($roleDefinition->roleName()));
}
@ -50,12 +51,15 @@ class ApiKeyServiceTest extends TestCase
public function provideCreationDate(): iterable
{
yield 'no expiration date' => [null, []];
yield 'expiration date' => [Chronos::parse('2030-01-01'), []];
yield 'roles' => [null, [
yield 'no expiration date or name' => [null, null, []];
yield 'expiration date' => [Chronos::parse('2030-01-01'), null, []];
yield 'roles' => [null, null, [
RoleDefinition::forDomain((new Domain(''))->setId('123')),
RoleDefinition::forAuthoredShortUrls(),
]];
yield 'single name' => [null, 'Alice', []];
yield 'multi-word name' => [null, 'Alice and Bob', []];
yield 'empty name' => [null, '', []];
}
/**