From 637d8334f42e6432e2a0de8a17f38c5253bfa7c1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 21 Sep 2023 09:29:59 +0200 Subject: [PATCH] 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); }