diff --git a/data/migrations/Version20210306165711.php b/data/migrations/Version20210306165711.php new file mode 100644 index 00000000..5a7b9fe7 --- /dev/null +++ b/data/migrations/Version20210306165711.php @@ -0,0 +1,45 @@ +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; + } +} diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index 119fa020..31df82a1 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -42,6 +42,10 @@ class GenerateKeyCommand extends BaseCommand %command.full_name% + You can optionally set its name for tracking purposes with --name or -m: + + %command.full_name% --name Alice + You can optionally set its expiration date with --expiration-date or -e: %command.full_name% --expiration-date 2020-01-01 @@ -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), ); diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index 9243779b..ac5fc73d 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -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; } diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 00548f17..ee822bee 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -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', + ]); + } } diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index e0cada5d..54ce19bc 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -52,13 +52,13 @@ class ListKeysCommandTest extends TestCase [ApiKey::withKey('foo'), ApiKey::withKey('bar'), ApiKey::withKey('baz')], false, <<disable(), ApiKey::withKey('bar')], true, << [ + [ + ApiKey::withKey('abc', null, 'Alice'), + ApiKey::withKey('def', null, 'Alice and Bob'), + ApiKey::withKey('ghi', null, ''), + ApiKey::withKey('jkl', null, null), + ], + true, + <<unique() ->build(); + $builder->createField('name', Types::STRING) + ->columnName('`name`') + ->nullable() + ->build(); + $builder->createField('expirationDate', ChronosDateTimeType::CHRONOS_DATETIME) ->columnName('expiration_date') ->nullable() diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index d153b7f1..b475a69d 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -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; diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 917cf048..ef60ee08 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -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); } diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 562f106b..982bdf4f 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -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; diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 6879d492..af50be23 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -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, '', []]; } /**