From b17c576a307ed3884be27a8b95a7029e3db33e1d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 15 Jun 2023 18:53:42 +0200 Subject: [PATCH 01/12] Fix incorrect timeout in init commands --- CHANGELOG.md | 17 +++++++++++++++++ composer.json | 2 +- module/CLI/src/Util/ProcessRunner.php | 4 ++-- module/Core/src/Model/DeviceType.php | 2 +- module/Rest/src/Action/Tag/ListTagsAction.php | 2 +- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44125533..6539166a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1819](https://github.com/shlinkio/shlink/issues/1819) Fix incorrect timeout when running DB commands during Shlink start-up. + + ## [3.6.3] - 2023-06-14 ### Added * *Nothing* diff --git a/composer.json b/composer.json index 98ce47eb..8f76670b 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "^3.0", "shlinkio/shlink-importer": "^5.1", - "shlinkio/shlink-installer": "^8.4.1", + "shlinkio/shlink-installer": "^8.4.2", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", diff --git a/module/CLI/src/Util/ProcessRunner.php b/module/CLI/src/Util/ProcessRunner.php index 1a5471e5..5a568dbe 100644 --- a/module/CLI/src/Util/ProcessRunner.php +++ b/module/CLI/src/Util/ProcessRunner.php @@ -23,8 +23,8 @@ class ProcessRunner implements ProcessRunnerInterface public function __construct(private ProcessHelper $helper, ?callable $createProcess = null) { $this->createProcess = $createProcess !== null - ? Closure::fromCallable($createProcess) - : static fn (array $cmd) => new Process($cmd, null, null, null, LockedCommandConfig::DEFAULT_TTL); + ? $createProcess(...) + : static fn (array $cmd) => new Process($cmd, timeout: LockedCommandConfig::DEFAULT_TTL); } public function run(OutputInterface $output, array $cmd): void diff --git a/module/Core/src/Model/DeviceType.php b/module/Core/src/Model/DeviceType.php index df4a1838..e394716a 100644 --- a/module/Core/src/Model/DeviceType.php +++ b/module/Core/src/Model/DeviceType.php @@ -12,7 +12,7 @@ enum DeviceType: string public static function matchFromUserAgent(string $userAgent): ?self { - $detect = new MobileDetect(null, $userAgent); // @phpstan-ignore-line + $detect = new MobileDetect(userAgent: $userAgent); // @phpstan-ignore-line return match (true) { // $detect->is('iOS') && $detect->isTablet() => self::IOS, // TODO To detect iPad only diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index d52436d2..34f44475 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -40,7 +40,7 @@ class ListTagsAction extends AbstractRestAction // This part is deprecated. To get tags with stats, the /tags/stats endpoint should be used instead $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); - $rawTags = $this->serializePaginator($tagsInfo, null, 'stats'); + $rawTags = $this->serializePaginator($tagsInfo, dataProp: 'stats'); $rawTags['data'] = map($tagsInfo, static fn (TagInfo $info) => $info->tag); return new JsonResponse(['tags' => $rawTags]); From a3b2f943390f09d752c305516c12e21436de9bda Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 12 Sep 2023 08:21:34 +0200 Subject: [PATCH 02/12] Make sure local config is not loaded in tests --- composer.json | 2 +- config/config.php | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 8f76670b..ab187f55 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "ext-json": "*", "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.1", - "cakephp/chronos": "^2.3", + "cakephp/chronos": "~2.3.3", "doctrine/migrations": "^3.5", "doctrine/orm": "^2.14", "endroid/qr-code": "^4.7", diff --git a/config/config.php b/config/config.php index e0ec6c23..9df29138 100644 --- a/config/config.php +++ b/config/config.php @@ -42,10 +42,9 @@ return (new ConfigAggregator\ConfigAggregator([ Core\ConfigProvider::class, CLI\ConfigProvider::class, Rest\ConfigProvider::class, - new ConfigAggregator\PhpFileProvider('config/autoload/{{,*.}global,{,*.}local}.php'), - $isTestEnv - ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php') - : new ConfigAggregator\ArrayProvider([]), + new ConfigAggregator\PhpFileProvider('config/autoload/{,*.}global.php'), + // Local config should not be loaded during tests, whereas test config should be loaded ONLY during tests + new ConfigAggregator\PhpFileProvider($isTestEnv ? 'config/test/*.global.php' : 'config/autoload/{,*.}local.php'), // Routes have to be loaded last new ConfigAggregator\PhpFileProvider('config/autoload/routes.config.php'), ], 'data/cache/app_config.php', [ From 074f2135f61367b43d0df8fc6abf37a929bd2558 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 12 Sep 2023 21:20:38 +0200 Subject: [PATCH 03/12] Make sure locks include the same cache namespace when sent to Redis --- CHANGELOG.md | 1 + composer.json | 2 +- config/autoload/locks.global.php | 6 +++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6539166a..bb9f5f0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1819](https://github.com/shlinkio/shlink/issues/1819) Fix incorrect timeout when running DB commands during Shlink start-up. +* [#1870](https://github.com/shlinkio/shlink/issues/1870) Make sure shared locks include the cache prefix when using Redis. ## [3.6.3] - 2023-06-14 diff --git a/composer.json b/composer.json index ab187f55..4361797e 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "^5.5", + "shlinkio/shlink-common": "dev-main#5a8bd5a as 5.6", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "^3.0", "shlinkio/shlink-importer": "^5.1", diff --git a/config/autoload/locks.global.php b/config/autoload/locks.global.php index 5e37e770..28426c6a 100644 --- a/config/autoload/locks.global.php +++ b/config/autoload/locks.global.php @@ -4,6 +4,7 @@ declare(strict_types=1); use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Shlinkio\Shlink\Common\Cache\RedisFactory; +use Shlinkio\Shlink\Common\Lock\NamespacedStore; use Shlinkio\Shlink\Common\Logger\LoggerAwareDelegatorFactory; use Shlinkio\Shlink\Core\Config\EnvVars; use Symfony\Component\Lock; @@ -22,11 +23,12 @@ return [ Lock\Store\RedisStore::class => ConfigAbstractFactory::class, Lock\LockFactory::class => ConfigAbstractFactory::class, LOCAL_LOCK_FACTORY => ConfigAbstractFactory::class, + NamespacedStore::class => ConfigAbstractFactory::class, ], 'aliases' => [ 'lock_store' => EnvVars::REDIS_SERVERS->existsInEnv() ? 'redis_lock_store' : 'local_lock_store', - 'redis_lock_store' => Lock\Store\RedisStore::class, + 'redis_lock_store' => NamespacedStore::class, 'local_lock_store' => Lock\Store\FlockStore::class, ], 'delegators' => [ @@ -39,6 +41,8 @@ return [ ConfigAbstractFactory::class => [ Lock\Store\FlockStore::class => ['config.locks.locks_dir'], Lock\Store\RedisStore::class => [RedisFactory::SERVICE_NAME], + NamespacedStore::class => [Lock\Store\RedisStore::class, 'config.cache.namespace'], + Lock\LockFactory::class => ['lock_store'], LOCAL_LOCK_FACTORY => ['local_lock_store'], ], From 65a0a90a51748b8497f8495ec91d2afd4ff9d848 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 19 Sep 2023 09:10:17 +0200 Subject: [PATCH 04/12] Allow custom API keys to be created --- indocker | 2 +- module/CLI/config/cli.config.php | 1 + module/CLI/src/ApiKey/RoleResolver.php | 15 ++++---- .../CLI/src/ApiKey/RoleResolverInterface.php | 4 +-- .../src/Command/Api/GenerateKeyCommand.php | 30 +++++++++++----- module/CLI/test/ApiKey/RoleResolverTest.php | 4 +-- .../Command/Api/GenerateKeyCommandTest.php | 12 +++---- .../test/Command/Api/ListKeysCommandTest.php | 8 ++--- .../ShortUrl/ListShortUrlsCommandTest.php | 2 +- module/Rest/src/ApiKey/Model/ApiKeyMeta.php | 35 ++++++++++++------- .../ApiKey/Repository/ApiKeyRepository.php | 3 +- module/Rest/src/Entity/ApiKey.php | 20 +++++------ module/Rest/src/Service/ApiKeyService.php | 32 ++++------------- .../src/Service/ApiKeyServiceInterface.php | 9 ++--- .../Rest/test-api/Fixtures/ApiKeyFixture.php | 2 +- .../Rest/test/Service/ApiKeyServiceTest.php | 8 +++-- 16 files changed, 93 insertions(+), 94 deletions(-) diff --git a/indocker b/indocker index 789386ac..7cfbe2c3 100755 --- a/indocker +++ b/indocker @@ -2,7 +2,7 @@ # Run docker containers if they are not up yet if ! [[ $(docker ps | grep shlink_swoole) ]]; then - docker-compose up -d + docker compose up -d fi docker exec -it shlink_swoole /bin/sh -c "$*" diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 9feeee7b..2a1bc5e8 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -22,6 +22,7 @@ return [ Command\Visit\GetNonOrphanVisitsCommand::NAME => Command\Visit\GetNonOrphanVisitsCommand::class, Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, + Command\Api\GenerateKeyCommand::ALIAS => Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class, Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class, diff --git a/module/CLI/src/ApiKey/RoleResolver.php b/module/CLI/src/ApiKey/RoleResolver.php index c1ae8f05..ad98bde4 100644 --- a/module/CLI/src/ApiKey/RoleResolver.php +++ b/module/CLI/src/ApiKey/RoleResolver.php @@ -14,24 +14,23 @@ use function is_string; class RoleResolver implements RoleResolverInterface { - public function __construct(private DomainServiceInterface $domainService, private string $defaultDomain) - { + public function __construct( + private readonly DomainServiceInterface $domainService, + private readonly string $defaultDomain, + ) { } - public function determineRoles(InputInterface $input): array + public function determineRoles(InputInterface $input): iterable { $domainAuthority = $input->getOption(Role::DOMAIN_SPECIFIC->paramName()); $author = $input->getOption(Role::AUTHORED_SHORT_URLS->paramName()); - $roleDefinitions = []; if ($author) { - $roleDefinitions[] = RoleDefinition::forAuthoredShortUrls(); + yield RoleDefinition::forAuthoredShortUrls(); } if (is_string($domainAuthority)) { - $roleDefinitions[] = $this->resolveRoleForAuthority($domainAuthority); + yield $this->resolveRoleForAuthority($domainAuthority); } - - return $roleDefinitions; } private function resolveRoleForAuthority(string $domainAuthority): RoleDefinition diff --git a/module/CLI/src/ApiKey/RoleResolverInterface.php b/module/CLI/src/ApiKey/RoleResolverInterface.php index 92a04594..e849ad13 100644 --- a/module/CLI/src/ApiKey/RoleResolverInterface.php +++ b/module/CLI/src/ApiKey/RoleResolverInterface.php @@ -10,7 +10,7 @@ use Symfony\Component\Console\Input\InputInterface; interface RoleResolverInterface { /** - * @return RoleDefinition[] + * @return iterable */ - public function determineRoles(InputInterface $input): array; + public function determineRoles(InputInterface $input): iterable; } diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index c2d6cf10..ec7b5cb2 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -8,10 +8,12 @@ use Cake\Chronos\Chronos; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -22,11 +24,13 @@ use function sprintf; class GenerateKeyCommand extends Command { - public const NAME = 'api-key:generate'; + public const NAME = 'api-key:create'; + /** @deprecated */ + public const ALIAS = 'api-key:generate'; public function __construct( - private ApiKeyServiceInterface $apiKeyService, - private RoleResolverInterface $roleResolver, + private readonly ApiKeyServiceInterface $apiKeyService, + private readonly RoleResolverInterface $roleResolver, ) { parent::__construct(); } @@ -57,7 +61,13 @@ class GenerateKeyCommand extends Command $this ->setName(self::NAME) - ->setDescription('Generates a new valid API key.') + ->setDescription('Creates a new valid API key.') + ->setAliases([self::ALIAS]) + ->addArgument( + 'key', + InputArgument::OPTIONAL, + 'The API key to create. A random one will be generated if not provided', + ) ->addOption( 'name', 'm', @@ -91,11 +101,13 @@ class GenerateKeyCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { $expirationDate = $input->getOption('expiration-date'); - $apiKey = $this->apiKeyService->create( - isset($expirationDate) ? Chronos::parse($expirationDate) : null, - $input->getOption('name'), - ...$this->roleResolver->determineRoles($input), - ); + + $apiKey = $this->apiKeyService->create(ApiKeyMeta::fromParams( + key: $input->getArgument('key'), + name: $input->getOption('name'), + expirationDate: isset($expirationDate) ? Chronos::parse($expirationDate) : null, + roleDefinitions: $this->roleResolver->determineRoles($input), + )); $io = new SymfonyStyle($input, $output); $io->success(sprintf('Generated API key: "%s"', $apiKey->toString())); diff --git a/module/CLI/test/ApiKey/RoleResolverTest.php b/module/CLI/test/ApiKey/RoleResolverTest.php index bc151c5e..7aecda6d 100644 --- a/module/CLI/test/ApiKey/RoleResolverTest.php +++ b/module/CLI/test/ApiKey/RoleResolverTest.php @@ -40,7 +40,7 @@ class RoleResolverTest extends TestCase 'example.com', )->willReturn(self::domainWithId(Domain::withAuthority('example.com'))); - $result = $this->resolver->determineRoles($input); + $result = [...$this->resolver->determineRoles($input)]; self::assertEquals($expectedRoles, $result); } @@ -111,7 +111,7 @@ class RoleResolverTest extends TestCase $this->expectException(InvalidRoleConfigException::class); - $this->resolver->determineRoles($input); + [...$this->resolver->determineRoles($input)]; } private static function domainWithId(Domain $domain): Domain diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index b5dbe513..0b306357 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -10,12 +10,15 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Tester\CommandTester; +use function is_string; + class GenerateKeyCommandTest extends TestCase { use CliTestUtilsTrait; @@ -37,8 +40,7 @@ class GenerateKeyCommandTest extends TestCase public function noExpirationDateIsDefinedIfNotProvided(): void { $this->apiKeyService->expects($this->once())->method('create')->with( - $this->isNull(), - $this->isNull(), + $this->callback(fn (ApiKeyMeta $meta) => $meta->name === null && $meta->expirationDate === null), )->willReturn(ApiKey::create()); $this->commandTester->execute([]); @@ -51,8 +53,7 @@ class GenerateKeyCommandTest extends TestCase public function expirationDateIsDefinedIfProvided(): void { $this->apiKeyService->expects($this->once())->method('create')->with( - $this->isInstanceOf(Chronos::class), - $this->isNull(), + $this->callback(fn (ApiKeyMeta $meta) => $meta->expirationDate instanceof Chronos), )->willReturn(ApiKey::create()); $this->commandTester->execute([ @@ -64,8 +65,7 @@ class GenerateKeyCommandTest extends TestCase public function nameIsDefinedIfProvided(): void { $this->apiKeyService->expects($this->once())->method('create')->with( - $this->isNull(), - $this->isType('string'), + $this->callback(fn (ApiKeyMeta $meta) => is_string($meta->name)), )->willReturn(ApiKey::create()); $this->commandTester->execute([ diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index e4cdb438..5e246fc7 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -49,7 +49,7 @@ class ListKeysCommandTest extends TestCase yield 'all keys' => [ [ $apiKey1 = ApiKey::create()->disable(), - $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withExpirationDate($dateInThePast)), + $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::fromParams(expirationDate: $dateInThePast)), $apiKey3 = ApiKey::create(), ], false, @@ -117,9 +117,9 @@ class ListKeysCommandTest extends TestCase ]; yield 'with names' => [ [ - $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withName('Alice')), - $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withName('Alice and Bob')), - $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::withName('')), + $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice')), + $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice and Bob')), + $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: '')), $apiKey4 = ApiKey::create(), ], true, diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index d81172ed..aff1ebdb 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -138,7 +138,7 @@ class ListShortUrlsCommandTest extends TestCase public static function provideOptionalFlags(): iterable { - $apiKey = ApiKey::fromMeta(ApiKeyMeta::withName('my api key')); + $apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'my api key')); $key = $apiKey->toString(); yield 'tags only' => [ diff --git a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php index 430221a2..e28a9ec3 100644 --- a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php +++ b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php @@ -5,36 +5,45 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\ApiKey\Model; use Cake\Chronos\Chronos; +use Ramsey\Uuid\Uuid; final class ApiKeyMeta { /** - * @param RoleDefinition[] $roleDefinitions + * @param iterable $roleDefinitions */ private function __construct( + public readonly string $key, public readonly ?string $name, public readonly ?Chronos $expirationDate, - public readonly array $roleDefinitions, + public readonly iterable $roleDefinitions, ) { } - public static function withName(string $name): self + public static function empty(): self { - return new self($name, null, []); + return self::fromParams(); } - public static function withExpirationDate(Chronos $expirationDate): self - { - return new self(null, $expirationDate, []); - } - - public static function withNameAndExpirationDate(string $name, Chronos $expirationDate): self - { - return new self($name, $expirationDate, []); + /** + * @param iterable $roleDefinitions + */ + public static function fromParams( + ?string $key = null, + ?string $name = null, + ?Chronos $expirationDate = null, + iterable $roleDefinitions = [], + ): self { + return new self( + key: $key ?? Uuid::uuid4()->toString(), + name: $name, + expirationDate: $expirationDate, + roleDefinitions: $roleDefinitions, + ); } public static function withRoles(RoleDefinition ...$roleDefinitions): self { - return new self(null, null, $roleDefinitions); + return self::fromParams(roleDefinitions: $roleDefinitions); } } diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php index ec49145e..eb11fc0d 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Rest\ApiKey\Repository; use Doctrine\DBAL\LockMode; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRepositoryInterface @@ -24,7 +25,7 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe ->getOneOrNullResult(); if ($firstResult === null) { - $em->persist(ApiKey::fromKey($apiKey)); + $em->persist(ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey))); $em->flush(); } }); diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 88cfa27e..07d05a03 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -10,7 +10,6 @@ use Doctrine\Common\Collections\Collection; use Exception; use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; -use Ramsey\Uuid\Uuid; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; @@ -28,21 +27,27 @@ class ApiKey extends AbstractEntity /** * @throws Exception */ - private function __construct(?string $key = null) + private function __construct(string $key) { - $this->key = $key ?? Uuid::uuid4()->toString(); + $this->key = $key; $this->enabled = true; $this->roles = new ArrayCollection(); } + /** + * @throws Exception + */ public static function create(): ApiKey { - return new self(); + return self::fromMeta(ApiKeyMeta::empty()); } + /** + * @throws Exception + */ public static function fromMeta(ApiKeyMeta $meta): self { - $apiKey = self::create(); + $apiKey = new self($meta->key); $apiKey->name = $meta->name; $apiKey->expirationDate = $meta->expirationDate; @@ -53,11 +58,6 @@ class ApiKey extends AbstractEntity return $apiKey; } - public static function fromKey(string $key): self - { - return new self($key); - } - public function getExpirationDate(): ?Chronos { return $this->expirationDate; diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 7d7e0710..b819c22d 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -4,47 +4,27 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Service; -use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; -use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use function sprintf; class ApiKeyService implements ApiKeyServiceInterface { - public function __construct(private EntityManagerInterface $em) + public function __construct(private readonly EntityManagerInterface $em) { } - public function create( - ?Chronos $expirationDate = null, - ?string $name = null, - RoleDefinition ...$roleDefinitions, - ): ApiKey { - $key = $this->buildApiKeyWithParams($expirationDate, $name); - foreach ($roleDefinitions as $definition) { - $key->registerRole($definition); - } + public function create(ApiKeyMeta $apiKeyMeta): ApiKey + { + $apiKey = ApiKey::fromMeta($apiKeyMeta); - $this->em->persist($key); + $this->em->persist($apiKey); $this->em->flush(); - return $key; - } - - private function buildApiKeyWithParams(?Chronos $expirationDate, ?string $name): ApiKey - { - return match (true) { - $expirationDate !== null && $name !== null => ApiKey::fromMeta( - ApiKeyMeta::withNameAndExpirationDate($name, $expirationDate), - ), - $expirationDate !== null => ApiKey::fromMeta(ApiKeyMeta::withExpirationDate($expirationDate)), - $name !== null => ApiKey::fromMeta(ApiKeyMeta::withName($name)), - default => ApiKey::create(), - }; + return $apiKey; } public function check(string $key): ApiKeyCheckResult diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 85b726df..826931a3 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -4,18 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Service; -use Cake\Chronos\Chronos; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ApiKeyServiceInterface { - public function create( - ?Chronos $expirationDate = null, - ?string $name = null, - RoleDefinition ...$roleDefinitions, - ): ApiKey; + public function create(ApiKeyMeta $apiKeyMeta): ApiKey; public function check(string $key): ApiKeyCheckResult; diff --git a/module/Rest/test-api/Fixtures/ApiKeyFixture.php b/module/Rest/test-api/Fixtures/ApiKeyFixture.php index 5ac886ce..bc33c678 100644 --- a/module/Rest/test-api/Fixtures/ApiKeyFixture.php +++ b/module/Rest/test-api/Fixtures/ApiKeyFixture.php @@ -43,7 +43,7 @@ class ApiKeyFixture extends AbstractFixture implements DependentFixtureInterface private function buildApiKey(string $key, bool $enabled, ?Chronos $expiresAt = null): ApiKey { - $apiKey = $expiresAt !== null ? ApiKey::fromMeta(ApiKeyMeta::withExpirationDate($expiresAt)) : ApiKey::create(); + $apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(expirationDate: $expiresAt)); $ref = new ReflectionObject($apiKey); $keyProp = $ref->getProperty('key'); $keyProp->setAccessible(true); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 325713be..952887a5 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -40,7 +40,9 @@ class ApiKeyServiceTest extends TestCase $this->em->expects($this->once())->method('flush'); $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class)); - $key = $this->service->create($date, $name, ...$roles); + $key = $this->service->create( + ApiKeyMeta::fromParams(name: $name, expirationDate: $date, roleDefinitions: $roles), + ); self::assertEquals($date, $key->getExpirationDate()); self::assertEquals($name, $key->name()); @@ -81,7 +83,7 @@ class ApiKeyServiceTest extends TestCase { yield 'non-existent api key' => [null]; yield 'disabled api key' => [ApiKey::create()->disable()]; - yield 'expired api key' => [ApiKey::fromMeta(ApiKeyMeta::withExpirationDate(Chronos::now()->subDay()))]; + yield 'expired api key' => [ApiKey::fromMeta(ApiKeyMeta::fromParams(expirationDate: Chronos::now()->subDay()))]; } #[Test] @@ -144,7 +146,7 @@ class ApiKeyServiceTest extends TestCase $this->repo->expects($this->once())->method('findBy')->with(['enabled' => true])->willReturn($expectedApiKeys); $this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo); - $result = $this->service->listKeys(true); + $result = $this->service->listKeys(enabledOnly: true); self::assertEquals($expectedApiKeys, $result); } From f6b1cc75563ca3e7baea545d3e14e34a1df6f671 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 19 Sep 2023 10:14:04 +0200 Subject: [PATCH 05/12] Test API key creation with custom key --- config/autoload/installer.global.php | 2 +- module/CLI/config/cli.config.php | 4 +-- module/CLI/config/dependencies.config.php | 4 +-- ...ateKeyCommand.php => CreateKeyCommand.php} | 3 +- .../CLI/test-cli/Command/CreateApiKeyTest.php | 31 +++++++++++++++++++ .../test-cli/Command/GenerateApiKeyTest.php | 22 ------------- ...mmandTest.php => CreateKeyCommandTest.php} | 20 ++++++++---- 7 files changed, 51 insertions(+), 35 deletions(-) rename module/CLI/src/Command/Api/{GenerateKeyCommand.php => CreateKeyCommand.php} (98%) create mode 100644 module/CLI/test-cli/Command/CreateApiKeyTest.php delete mode 100644 module/CLI/test-cli/Command/GenerateApiKeyTest.php rename module/CLI/test/Command/Api/{GenerateKeyCommandTest.php => CreateKeyCommandTest.php} (78%) diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 029a50d6..de6a69a3 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -84,7 +84,7 @@ return [ 'command' => 'bin/cli ' . Command\Visit\DownloadGeoLiteDbCommand::NAME, ], InstallationCommand::API_KEY_GENERATE->value => [ - 'command' => 'bin/cli ' . Command\Api\GenerateKeyCommand::NAME, + 'command' => 'bin/cli ' . Command\Api\CreateKeyCommand::NAME, ], ], ], diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 2a1bc5e8..1857bec7 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -21,8 +21,8 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::NAME => Command\Visit\DeleteOrphanVisitsCommand::class, Command\Visit\GetNonOrphanVisitsCommand::NAME => Command\Visit\GetNonOrphanVisitsCommand::class, - Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, - Command\Api\GenerateKeyCommand::ALIAS => Command\Api\GenerateKeyCommand::class, + Command\Api\CreateKeyCommand::NAME => Command\Api\CreateKeyCommand::class, + Command\Api\CreateKeyCommand::ALIAS => Command\Api\CreateKeyCommand::class, Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class, Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 6b7fc552..c96b13c1 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -50,7 +50,7 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\GetNonOrphanVisitsCommand::class => ConfigAbstractFactory::class, - Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, + Command\Api\CreateKeyCommand::class => ConfigAbstractFactory::class, Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class, Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class, @@ -102,7 +102,7 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::class => [Visit\VisitsDeleter::class], Command\Visit\GetNonOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class, ShortUrlStringifier::class], - Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], + Command\Api\CreateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], Command\Api\DisableKeyCommand::class => [ApiKeyService::class], Command\Api\ListKeysCommand::class => [ApiKeyService::class], diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/CreateKeyCommand.php similarity index 98% rename from module/CLI/src/Command/Api/GenerateKeyCommand.php rename to module/CLI/src/Command/Api/CreateKeyCommand.php index ec7b5cb2..30d4ca58 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/CreateKeyCommand.php @@ -22,10 +22,9 @@ use Symfony\Component\Console\Style\SymfonyStyle; use function Shlinkio\Shlink\Core\arrayToString; use function sprintf; -class GenerateKeyCommand extends Command +class CreateKeyCommand extends Command { public const NAME = 'api-key:create'; - /** @deprecated */ public const ALIAS = 'api-key:generate'; public function __construct( diff --git a/module/CLI/test-cli/Command/CreateApiKeyTest.php b/module/CLI/test-cli/Command/CreateApiKeyTest.php new file mode 100644 index 00000000..1b2b9c0d --- /dev/null +++ b/module/CLI/test-cli/Command/CreateApiKeyTest.php @@ -0,0 +1,31 @@ +exec([CreateKeyCommand::NAME]); + + self::assertStringContainsString('[OK] Generated API key', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + } + + #[Test] + public function allowsCustomKeyToBeProvided(): void + { + [$output, $exitCode] = $this->exec([CreateKeyCommand::NAME, 'custom_api_key']); + + self::assertStringContainsString('[OK] Generated API key: "custom_api_key"', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + } +} diff --git a/module/CLI/test-cli/Command/GenerateApiKeyTest.php b/module/CLI/test-cli/Command/GenerateApiKeyTest.php deleted file mode 100644 index 7d90c336..00000000 --- a/module/CLI/test-cli/Command/GenerateApiKeyTest.php +++ /dev/null @@ -1,22 +0,0 @@ -exec([GenerateKeyCommand::NAME]); - - self::assertStringContainsString('[OK] Generated API key', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); - } -} diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/CreateKeyCommandTest.php similarity index 78% rename from module/CLI/test/Command/Api/GenerateKeyCommandTest.php rename to module/CLI/test/Command/Api/CreateKeyCommandTest.php index 0b306357..44481114 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/CreateKeyCommandTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; -use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; +use Shlinkio\Shlink\CLI\Command\Api\CreateKeyCommand; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -17,9 +17,7 @@ use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Tester\CommandTester; -use function is_string; - -class GenerateKeyCommandTest extends TestCase +class CreateKeyCommandTest extends TestCase { use CliTestUtilsTrait; @@ -32,7 +30,7 @@ class GenerateKeyCommandTest extends TestCase $roleResolver = $this->createMock(RoleResolverInterface::class); $roleResolver->method('determineRoles')->with($this->isInstanceOf(InputInterface::class))->willReturn([]); - $command = new GenerateKeyCommand($this->apiKeyService, $roleResolver); + $command = new CreateKeyCommand($this->apiKeyService, $roleResolver); $this->commandTester = $this->testerForCommand($command); } @@ -65,11 +63,21 @@ class GenerateKeyCommandTest extends TestCase public function nameIsDefinedIfProvided(): void { $this->apiKeyService->expects($this->once())->method('create')->with( - $this->callback(fn (ApiKeyMeta $meta) => is_string($meta->name)), + $this->callback(fn (ApiKeyMeta $meta) => $meta->name === 'Alice'), )->willReturn(ApiKey::create()); $this->commandTester->execute([ '--name' => 'Alice', ]); } + + #[Test] + public function createsCustomApiKeyWhenProvided(): void + { + $this->apiKeyService->expects($this->once())->method('create')->with( + $this->callback(fn (ApiKeyMeta $meta) => $meta->key === 'my_custom_key'), + )->willReturn(ApiKey::create()); + + $this->commandTester->execute(['key' => 'my_custom_key']); + } } From 6db46b50e98d3489a6e5bed5490bb6b27704a42f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 21 Sep 2023 08:58:05 +0200 Subject: [PATCH 06/12] Roll back change to allow creating API keys with custom value --- config/autoload/installer.global.php | 2 +- docker/docker-entrypoint.sh | 7 ++++- module/CLI/config/cli.config.php | 3 +- module/CLI/config/dependencies.config.php | 4 +-- ...eKeyCommand.php => GenerateKeyCommand.php} | 15 ++------- .../CLI/test-cli/Command/CreateApiKeyTest.php | 31 ------------------- .../test-cli/Command/GenerateApiKeyTest.php | 22 +++++++++++++ ...andTest.php => GenerateKeyCommandTest.php} | 16 ++-------- 8 files changed, 38 insertions(+), 62 deletions(-) rename module/CLI/src/Command/Api/{CreateKeyCommand.php => GenerateKeyCommand.php} (88%) delete mode 100644 module/CLI/test-cli/Command/CreateApiKeyTest.php create mode 100644 module/CLI/test-cli/Command/GenerateApiKeyTest.php rename module/CLI/test/Command/Api/{CreateKeyCommandTest.php => GenerateKeyCommandTest.php} (81%) diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index de6a69a3..029a50d6 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -84,7 +84,7 @@ return [ 'command' => 'bin/cli ' . Command\Visit\DownloadGeoLiteDbCommand::NAME, ], InstallationCommand::API_KEY_GENERATE->value => [ - 'command' => 'bin/cli ' . Command\Api\CreateKeyCommand::NAME, + 'command' => 'bin/cli ' . Command\Api\GenerateKeyCommand::NAME, ], ], ], diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 2058c44c..1fcdfa99 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -12,8 +12,13 @@ fi php vendor/bin/shlink-installer init ${flags} +# TODO If INIT_API_KEY was provided, create an initial API key +#if [ -n "${INIT_API_KEY}" ]; then +# php bin/cli api-key:initial "${INIT_API_KEY}" +#fi + # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root -# ENABLE_PERIODIC_VISIT_LOCATE is deprecated. Remove cron support in Shlink 4.0.0 +# FIXME: ENABLE_PERIODIC_VISIT_LOCATE is deprecated. Remove cron support in Shlink 4.0.0 if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "root" ]; then echo "Configuring periodic visit location..." echo "0 * * * * php /etc/shlink/bin/cli visit:locate -q" > /etc/crontabs/root diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 1857bec7..9feeee7b 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -21,8 +21,7 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::NAME => Command\Visit\DeleteOrphanVisitsCommand::class, Command\Visit\GetNonOrphanVisitsCommand::NAME => Command\Visit\GetNonOrphanVisitsCommand::class, - Command\Api\CreateKeyCommand::NAME => Command\Api\CreateKeyCommand::class, - Command\Api\CreateKeyCommand::ALIAS => Command\Api\CreateKeyCommand::class, + Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class, Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index c96b13c1..6b7fc552 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -50,7 +50,7 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\GetNonOrphanVisitsCommand::class => ConfigAbstractFactory::class, - Command\Api\CreateKeyCommand::class => ConfigAbstractFactory::class, + Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class, Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class, @@ -102,7 +102,7 @@ return [ Command\Visit\DeleteOrphanVisitsCommand::class => [Visit\VisitsDeleter::class], Command\Visit\GetNonOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class, ShortUrlStringifier::class], - Command\Api\CreateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], + Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], Command\Api\DisableKeyCommand::class => [ApiKeyService::class], Command\Api\ListKeysCommand::class => [ApiKeyService::class], diff --git a/module/CLI/src/Command/Api/CreateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php similarity index 88% rename from module/CLI/src/Command/Api/CreateKeyCommand.php rename to module/CLI/src/Command/Api/GenerateKeyCommand.php index 30d4ca58..85f709f8 100644 --- a/module/CLI/src/Command/Api/CreateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -13,7 +13,6 @@ use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -22,10 +21,9 @@ use Symfony\Component\Console\Style\SymfonyStyle; use function Shlinkio\Shlink\Core\arrayToString; use function sprintf; -class CreateKeyCommand extends Command +class GenerateKeyCommand extends Command { - public const NAME = 'api-key:create'; - public const ALIAS = 'api-key:generate'; + public const NAME = 'api-key:generate'; public function __construct( private readonly ApiKeyServiceInterface $apiKeyService, @@ -60,13 +58,7 @@ class CreateKeyCommand extends Command $this ->setName(self::NAME) - ->setDescription('Creates a new valid API key.') - ->setAliases([self::ALIAS]) - ->addArgument( - 'key', - InputArgument::OPTIONAL, - 'The API key to create. A random one will be generated if not provided', - ) + ->setDescription('Generate a new valid API key.') ->addOption( 'name', 'm', @@ -102,7 +94,6 @@ class CreateKeyCommand extends Command $expirationDate = $input->getOption('expiration-date'); $apiKey = $this->apiKeyService->create(ApiKeyMeta::fromParams( - key: $input->getArgument('key'), name: $input->getOption('name'), expirationDate: isset($expirationDate) ? Chronos::parse($expirationDate) : null, roleDefinitions: $this->roleResolver->determineRoles($input), diff --git a/module/CLI/test-cli/Command/CreateApiKeyTest.php b/module/CLI/test-cli/Command/CreateApiKeyTest.php deleted file mode 100644 index 1b2b9c0d..00000000 --- a/module/CLI/test-cli/Command/CreateApiKeyTest.php +++ /dev/null @@ -1,31 +0,0 @@ -exec([CreateKeyCommand::NAME]); - - self::assertStringContainsString('[OK] Generated API key', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); - } - - #[Test] - public function allowsCustomKeyToBeProvided(): void - { - [$output, $exitCode] = $this->exec([CreateKeyCommand::NAME, 'custom_api_key']); - - self::assertStringContainsString('[OK] Generated API key: "custom_api_key"', $output); - self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); - } -} diff --git a/module/CLI/test-cli/Command/GenerateApiKeyTest.php b/module/CLI/test-cli/Command/GenerateApiKeyTest.php new file mode 100644 index 00000000..7d90c336 --- /dev/null +++ b/module/CLI/test-cli/Command/GenerateApiKeyTest.php @@ -0,0 +1,22 @@ +exec([GenerateKeyCommand::NAME]); + + self::assertStringContainsString('[OK] Generated API key', $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + } +} diff --git a/module/CLI/test/Command/Api/CreateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php similarity index 81% rename from module/CLI/test/Command/Api/CreateKeyCommandTest.php rename to module/CLI/test/Command/Api/GenerateKeyCommandTest.php index 44481114..aedda4ca 100644 --- a/module/CLI/test/Command/Api/CreateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; -use Shlinkio\Shlink\CLI\Command\Api\CreateKeyCommand; +use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -17,7 +17,7 @@ use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Tester\CommandTester; -class CreateKeyCommandTest extends TestCase +class GenerateKeyCommandTest extends TestCase { use CliTestUtilsTrait; @@ -30,7 +30,7 @@ class CreateKeyCommandTest extends TestCase $roleResolver = $this->createMock(RoleResolverInterface::class); $roleResolver->method('determineRoles')->with($this->isInstanceOf(InputInterface::class))->willReturn([]); - $command = new CreateKeyCommand($this->apiKeyService, $roleResolver); + $command = new GenerateKeyCommand($this->apiKeyService, $roleResolver); $this->commandTester = $this->testerForCommand($command); } @@ -70,14 +70,4 @@ class CreateKeyCommandTest extends TestCase '--name' => 'Alice', ]); } - - #[Test] - public function createsCustomApiKeyWhenProvided(): void - { - $this->apiKeyService->expects($this->once())->method('create')->with( - $this->callback(fn (ApiKeyMeta $meta) => $meta->key === 'my_custom_key'), - )->willReturn(ApiKey::create()); - - $this->commandTester->execute(['key' => 'my_custom_key']); - } } From 637d8334f42e6432e2a0de8a17f38c5253bfa7c1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 21 Sep 2023 09:29:59 +0200 Subject: [PATCH 07/12] New CLI command to create the initial API key idempotently --- docker/docker-entrypoint.sh | 13 +++-- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 2 + .../src/Command/Api/InitialApiKeyCommand.php | 43 ++++++++++++++ module/Core/src/Config/EnvVars.php | 1 - module/Rest/config/initial-api-key.config.php | 26 --------- .../src/ApiKey/InitialApiKeyDelegator.php | 31 ---------- .../ApiKey/Repository/ApiKeyRepository.php | 19 +++++-- .../Repository/ApiKeyRepositoryInterface.php | 3 +- module/Rest/src/Service/ApiKeyService.php | 8 +++ .../src/Service/ApiKeyServiceInterface.php | 2 + .../Repository/ApiKeyRepositoryTest.php | 4 +- .../ApiKey/InitialApiKeyDelegatorTest.php | 57 ------------------- module/Rest/test/ConfigProviderTest.php | 3 +- 14 files changed, 84 insertions(+), 129 deletions(-) create mode 100644 module/CLI/src/Command/Api/InitialApiKeyCommand.php delete mode 100644 module/Rest/config/initial-api-key.config.php delete mode 100644 module/Rest/src/ApiKey/InitialApiKeyDelegator.php delete mode 100644 module/Rest/test/ApiKey/InitialApiKeyDelegatorTest.php diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 1fcdfa99..1a9e44e3 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -10,12 +10,17 @@ if [ -z "${GEOLITE_LICENSE_KEY}" ] || [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" == "t flags="${flags} --skip-download-geolite" fi +# TODO If INITIAL_API_KEY was provided, create an initial API key +#if [ -n "${INITIAL_API_KEY}" ]; then +# flags="${flags} --initial-api-key=${INITIAL_API_KEY}" +#fi + php vendor/bin/shlink-installer init ${flags} -# TODO If INIT_API_KEY was provided, create an initial API key -#if [ -n "${INIT_API_KEY}" ]; then -# php bin/cli api-key:initial "${INIT_API_KEY}" -#fi +# If INITIAL_API_KEY was provided, create an initial API key +if [ -n "${INITIAL_API_KEY}" ]; then + php bin/cli api-key:initial "${INITIAL_API_KEY}" +fi # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root # FIXME: ENABLE_PERIODIC_VISIT_LOCATE is deprecated. Remove cron support in Shlink 4.0.0 diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 9feeee7b..bcd4fd3c 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -24,6 +24,7 @@ return [ Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class, Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class, + Command\Api\InitialApiKeyCommand::NAME => Command\Api\InitialApiKeyCommand::class, Command\Tag\ListTagsCommand::NAME => Command\Tag\ListTagsCommand::class, Command\Tag\RenameTagCommand::NAME => Command\Tag\RenameTagCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 6b7fc552..2736a21e 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -53,6 +53,7 @@ return [ Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class, Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class, + Command\Api\InitialApiKeyCommand::class => ConfigAbstractFactory::class, Command\Tag\ListTagsCommand::class => ConfigAbstractFactory::class, Command\Tag\RenameTagCommand::class => ConfigAbstractFactory::class, @@ -105,6 +106,7 @@ return [ Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], Command\Api\DisableKeyCommand::class => [ApiKeyService::class], Command\Api\ListKeysCommand::class => [ApiKeyService::class], + Command\Api\InitialApiKeyCommand::class => [ApiKeyService::class], Command\Tag\ListTagsCommand::class => [TagService::class], Command\Tag\RenameTagCommand::class => [TagService::class], diff --git a/module/CLI/src/Command/Api/InitialApiKeyCommand.php b/module/CLI/src/Command/Api/InitialApiKeyCommand.php new file mode 100644 index 00000000..1f5a1794 --- /dev/null +++ b/module/CLI/src/Command/Api/InitialApiKeyCommand.php @@ -0,0 +1,43 @@ +setHidden() + ->setName(self::NAME) + ->setDescription('Tries to create initial API key') + ->addArgument('apiKey', InputArgument::REQUIRED, 'The initial API to create'); + } + + protected function execute(InputInterface $input, OutputInterface $output): ?int + { + $key = $input->getArgument('apiKey'); + $result = $this->apiKeyService->createInitial($key); + + if ($result === null && $output->isVerbose()) { + $output->writeln('Other API keys already exist. Initial API key creation skipped.'); + } + + return ExitCode::EXIT_SUCCESS; + } +} diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 44919415..ec7d384c 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -47,7 +47,6 @@ enum EnvVars: string case PORT = 'PORT'; case TASK_WORKER_NUM = 'TASK_WORKER_NUM'; case WEB_WORKER_NUM = 'WEB_WORKER_NUM'; - case INITIAL_API_KEY = 'INITIAL_API_KEY'; case ANONYMIZE_REMOTE_ADDR = 'ANONYMIZE_REMOTE_ADDR'; case TRACK_ORPHAN_VISITS = 'TRACK_ORPHAN_VISITS'; case DISABLE_TRACK_PARAM = 'DISABLE_TRACK_PARAM'; diff --git a/module/Rest/config/initial-api-key.config.php b/module/Rest/config/initial-api-key.config.php deleted file mode 100644 index a44f877f..00000000 --- a/module/Rest/config/initial-api-key.config.php +++ /dev/null @@ -1,26 +0,0 @@ - PHP_SAPI !== 'cli' ? null : EnvVars::INITIAL_API_KEY->loadFromEnv(), - - 'dependencies' => [ - 'delegators' => [ - Application::class => [ - ApiKey\InitialApiKeyDelegator::class, - ], - ], - ], - -]; diff --git a/module/Rest/src/ApiKey/InitialApiKeyDelegator.php b/module/Rest/src/ApiKey/InitialApiKeyDelegator.php deleted file mode 100644 index a5aa9d33..00000000 --- a/module/Rest/src/ApiKey/InitialApiKeyDelegator.php +++ /dev/null @@ -1,31 +0,0 @@ -get('config')['initial_api_key'] ?? null; - if (! empty($initialApiKey)) { - $this->createInitialApiKey($initialApiKey, $container); - } - - return $callback(); - } - - private function createInitialApiKey(string $initialApiKey, ContainerInterface $container): void - { - /** @var ApiKeyRepositoryInterface $repo */ - $repo = $container->get(EntityManager::class)->getRepository(ApiKey::class); - $repo->createInitialApiKey($initialApiKey); - } -} diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php index eb11fc0d..ad09b22d 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php @@ -11,10 +11,13 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRepositoryInterface { - public function createInitialApiKey(string $apiKey): void + /** + * Will create provided API key with admin permissions, only if there's no other API keys yet + */ + public function createInitialApiKey(string $apiKey): ?ApiKey { $em = $this->getEntityManager(); - $em->wrapInTransaction(function () use ($apiKey, $em): void { + return $em->wrapInTransaction(function () use ($apiKey, $em): ?ApiKey { // Ideally this would be a SELECT COUNT(...), but MsSQL and Postgres do not allow locking on aggregates // Because of that we check if at least one result exists $firstResult = $em->createQueryBuilder()->select('a.id') @@ -24,10 +27,16 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe ->setLockMode(LockMode::PESSIMISTIC_WRITE) ->getOneOrNullResult(); - if ($firstResult === null) { - $em->persist(ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey))); - $em->flush(); + // Do not create an initial API key if other keys already exist + if ($firstResult !== null) { + return null; } + + $new = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $apiKey)); + $em->persist($new); + $em->flush(); + + return $new; }); } } diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php index f5beb3e9..a557e603 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php @@ -6,11 +6,12 @@ namespace Shlinkio\Shlink\Rest\ApiKey\Repository; use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ApiKeyRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { /** * Will create provided API key only if there's no API keys yet */ - public function createInitialApiKey(string $apiKey): void; + public function createInitialApiKey(string $apiKey): ?ApiKey; } diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index b819c22d..21f69f90 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Rest\Service; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; +use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; use function sprintf; @@ -27,6 +28,13 @@ class ApiKeyService implements ApiKeyServiceInterface return $apiKey; } + public function createInitial(string $key): ?ApiKey + { + /** @var ApiKeyRepositoryInterface $repo */ + $repo = $this->em->getRepository(ApiKey::class); + return $repo->createInitialApiKey($key); + } + public function check(string $key): ApiKeyCheckResult { $apiKey = $this->getByKey($key); diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 826931a3..b82d7760 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -12,6 +12,8 @@ interface ApiKeyServiceInterface { public function create(ApiKeyMeta $apiKeyMeta): ApiKey; + public function createInitial(string $key): ?ApiKey; + public function check(string $key): ApiKeyCheckResult; /** diff --git a/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php b/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php index 86db9176..c19d8512 100644 --- a/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php +++ b/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php @@ -22,10 +22,10 @@ class ApiKeyRepositoryTest extends DatabaseTestCase public function initialApiKeyIsCreatedOnlyOfNoApiKeysExistYet(): void { self::assertCount(0, $this->repo->findAll()); - $this->repo->createInitialApiKey('initial_value'); + self::assertNotNull($this->repo->createInitialApiKey('initial_value')); self::assertCount(1, $this->repo->findAll()); self::assertCount(1, $this->repo->findBy(['key' => 'initial_value'])); - $this->repo->createInitialApiKey('another_one'); + self::assertNull($this->repo->createInitialApiKey('another_one')); self::assertCount(1, $this->repo->findAll()); self::assertCount(0, $this->repo->findBy(['key' => 'another_one'])); } diff --git a/module/Rest/test/ApiKey/InitialApiKeyDelegatorTest.php b/module/Rest/test/ApiKey/InitialApiKeyDelegatorTest.php deleted file mode 100644 index 9b6edff6..00000000 --- a/module/Rest/test/ApiKey/InitialApiKeyDelegatorTest.php +++ /dev/null @@ -1,57 +0,0 @@ -delegator = new InitialApiKeyDelegator(); - $this->container = $this->createMock(ContainerInterface::class); - } - - #[Test, DataProvider('provideConfigs')] - public function apiKeyIsInitializedWhenAppropriate(array $config, int $expectedCalls): void - { - $app = $this->createMock(Application::class); - $apiKeyRepo = $this->createMock(ApiKeyRepositoryInterface::class); - $apiKeyRepo->expects($this->exactly($expectedCalls))->method('createInitialApiKey'); - $em = $this->createMock(EntityManagerInterface::class); - $em->expects($this->exactly($expectedCalls))->method('getRepository')->with(ApiKey::class)->willReturn( - $apiKeyRepo, - ); - $this->container->expects($this->exactly($expectedCalls + 1))->method('get')->willReturnMap([ - ['config', $config], - [EntityManager::class, $em], - ]); - - $result = ($this->delegator)($this->container, '', fn () => $app); - - self::assertSame($result, $app); - } - - public static function provideConfigs(): iterable - { - yield 'no api key' => [[], 0]; - yield 'null api key' => [['initial_api_key' => null], 0]; - yield 'empty api key' => [['initial_api_key' => ''], 0]; - yield 'valid api key' => [['initial_api_key' => 'the_initial_key'], 1]; - } -} diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index ee729eb5..72063a72 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -24,11 +24,10 @@ class ConfigProviderTest extends TestCase { $config = ($this->configProvider)(); - self::assertCount(5, $config); + self::assertCount(4, $config); self::assertArrayHasKey('dependencies', $config); self::assertArrayHasKey('auth', $config); self::assertArrayHasKey('entity_manager', $config); - self::assertArrayHasKey('initial_api_key', $config); self::assertArrayHasKey(ConfigAbstractFactory::class, $config); } From b0ec0601c16e9b5d6631e7f659655b7a5dcdfe60 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 22 Sep 2023 10:00:19 +0200 Subject: [PATCH 08/12] Update to latest shlink-installer --- CHANGELOG.md | 7 +++++-- composer.json | 4 ++-- config/autoload/installer.global.php | 3 +++ docker/docker-entrypoint.sh | 11 +++-------- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb9f5f0c..683bcf7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [3.6.4] - 2023-09-22 ### Added * *Nothing* ### Changed -* *Nothing* +* [#1866](https://github.com/shlinkio/shlink/issues/1866) The `INITIAL_API_KEY` env var is now only relevant for the official docker image. + + Going forward, new non-docker Shlink installations provisioned with env vars that also wish to provide an initial API key, should do it by using the `vendor/bin/shlink-installer init --initial-api-key=%SOME_KEY%` command, instead of using `INITIAL_API_KEY`. ### Deprecated * *Nothing* @@ -20,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1819](https://github.com/shlinkio/shlink/issues/1819) Fix incorrect timeout when running DB commands during Shlink start-up. * [#1870](https://github.com/shlinkio/shlink/issues/1870) Make sure shared locks include the cache prefix when using Redis. +* [#1866](https://github.com/shlinkio/shlink/issues/1866) Fix error when starting docker image with `INITIAL_API_KEY` env var. ## [3.6.3] - 2023-06-14 diff --git a/composer.json b/composer.json index 4361797e..9c784a06 100644 --- a/composer.json +++ b/composer.json @@ -45,11 +45,11 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "dev-main#5a8bd5a as 5.6", + "shlinkio/shlink-common": "^5.6", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "^3.0", "shlinkio/shlink-importer": "^5.1", - "shlinkio/shlink-installer": "^8.4.2", + "shlinkio/shlink-installer": "^8.5", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 029a50d6..966e36ee 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -86,6 +86,9 @@ return [ InstallationCommand::API_KEY_GENERATE->value => [ 'command' => 'bin/cli ' . Command\Api\GenerateKeyCommand::NAME, ], + InstallationCommand::API_KEY_CREATE->value => [ + 'command' => 'bin/cli ' . Command\Api\InitialApiKeyCommand::NAME, + ], ], ], diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 1a9e44e3..06cb8ec4 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -10,18 +10,13 @@ if [ -z "${GEOLITE_LICENSE_KEY}" ] || [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" == "t flags="${flags} --skip-download-geolite" fi -# TODO If INITIAL_API_KEY was provided, create an initial API key -#if [ -n "${INITIAL_API_KEY}" ]; then -# flags="${flags} --initial-api-key=${INITIAL_API_KEY}" -#fi - -php vendor/bin/shlink-installer init ${flags} - # If INITIAL_API_KEY was provided, create an initial API key if [ -n "${INITIAL_API_KEY}" ]; then - php bin/cli api-key:initial "${INITIAL_API_KEY}" + flags="${flags} --initial-api-key=${INITIAL_API_KEY}" fi +php vendor/bin/shlink-installer init ${flags} + # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root # FIXME: ENABLE_PERIODIC_VISIT_LOCATE is deprecated. Remove cron support in Shlink 4.0.0 if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "root" ]; then From ec839183e82e6a1d4b175069a607b56efeb0c854 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Sep 2023 08:01:10 +0200 Subject: [PATCH 09/12] Add unit test for ApiKeyService::createInitial --- .../Rest/test/Service/ApiKeyServiceTest.php | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 952887a5..5c9d28db 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; +use Shlinkio\Shlink\Rest\ApiKey\Repository\ApiKeyRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -22,12 +23,12 @@ class ApiKeyServiceTest extends TestCase { private ApiKeyService $service; private MockObject & EntityManager $em; - private MockObject & EntityRepository $repo; + private MockObject & ApiKeyRepositoryInterface $repo; protected function setUp(): void { $this->em = $this->createMock(EntityManager::class); - $this->repo = $this->createMock(EntityRepository::class); + $this->repo = $this->createMock(ApiKeyRepositoryInterface::class); $this->service = new ApiKeyService($this->em); } @@ -150,4 +151,21 @@ class ApiKeyServiceTest extends TestCase self::assertEquals($expectedApiKeys, $result); } + + #[Test, DataProvider('provideInitialApiKeys')] + public function createInitialDelegatesToRepository(?ApiKey $apiKey): void + { + $this->repo->expects($this->once())->method('createInitialApiKey')->with('the_key')->willReturn($apiKey); + $this->em->method('getRepository')->with(ApiKey::class)->willReturn($this->repo); + + $result = $this->service->createInitial('the_key'); + + self::assertSame($result, $apiKey); + } + + public static function provideInitialApiKeys(): iterable + { + yield 'first api key' => [ApiKey::create()]; + yield 'existing api keys' => [null]; + } } From ba4a66f772e740e6c5fcf73fc61d2b56d5e9d6f8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Sep 2023 08:16:00 +0200 Subject: [PATCH 10/12] Add InitialApiKeyCommand unit test --- .../Command/Api/InitialApiKeyCommandTest.php | 56 +++++++++++++++++++ .../Rest/test/Service/ApiKeyServiceTest.php | 1 - 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 module/CLI/test/Command/Api/InitialApiKeyCommandTest.php diff --git a/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php b/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php new file mode 100644 index 00000000..e0732aab --- /dev/null +++ b/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php @@ -0,0 +1,56 @@ +apiKeyService = $this->createMock(ApiKeyServiceInterface::class); + $this->commandTester = $this->testerForCommand(new InitialApiKeyCommand($this->apiKeyService)); + } + + #[Test, DataProvider('provideParams')] + public function initialKeyIsCreatedWithProvidedValue(?ApiKey $result, bool $verbose, string $expectedOutput): void + { + $this->apiKeyService->expects($this->once())->method('createInitial')->with('the_key')->willReturn($result); + + $this->commandTester->execute( + ['apiKey' => 'the_key'], + ['verbosity' => $verbose ? OutputInterface::VERBOSITY_VERBOSE : OutputInterface::VERBOSITY_NORMAL], + ); + $output = $this->commandTester->getDisplay(); + + self::assertEquals($expectedOutput, $output); + } + + public static function provideParams(): iterable + { + yield 'api key created, no verbose' => [ApiKey::create(), false, '']; + yield 'api key created, verbose' => [ApiKey::create(), true, '']; + yield 'no api key created, no verbose' => [null, false, '']; + yield 'no api key created, verbose' => [null, true, << Date: Sat, 23 Sep 2023 08:28:57 +0200 Subject: [PATCH 11/12] Create InitialApiKeyCommand cli test --- .../test-cli/Command/InitialApiKeyTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 module/CLI/test-cli/Command/InitialApiKeyTest.php diff --git a/module/CLI/test-cli/Command/InitialApiKeyTest.php b/module/CLI/test-cli/Command/InitialApiKeyTest.php new file mode 100644 index 00000000..4f4f1922 --- /dev/null +++ b/module/CLI/test-cli/Command/InitialApiKeyTest.php @@ -0,0 +1,26 @@ +exec([InitialApiKeyCommand::NAME, 'new_api_key', '-v']); + + self::assertEquals( + << Date: Sat, 23 Sep 2023 08:41:57 +0200 Subject: [PATCH 12/12] Fix date in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 683bcf7f..0dc26f69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [3.6.4] - 2023-09-22 +## [3.6.4] - 2023-09-23 ### Added * *Nothing*