Allow custom API keys to be created

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 "$*"

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,
 

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

module/CLI/src/ApiKey/RoleResolverInterface.php

@@ -10,7 +10,7 @@ use Symfony\Component\Console\Input\InputInterface;
 interface RoleResolverInterface
 {
     /**
-     * @return RoleDefinition[]
+     * @return iterable<RoleDefinition>
      */
-    public function determineRoles(InputInterface $input): array;
+    public function determineRoles(InputInterface $input): iterable;
 }

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 readonly ApiKeyServiceInterface $apiKeyService,
-        private RoleResolverInterface $roleResolver,
+        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,
+        $apiKey = $this->apiKeyService->create(ApiKeyMeta::fromParams(
-            $input->getOption('name'),
+            key: $input->getArgument('key'),
-            ...$this->roleResolver->determineRoles($input),
+            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()));

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

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->callback(fn (ApiKeyMeta $meta) => $meta->name === null && $meta->expirationDate === null),
-            $this->isNull(),
         )->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->callback(fn (ApiKeyMeta $meta) => $meta->expirationDate instanceof Chronos),
-            $this->isNull(),
         )->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->callback(fn (ApiKeyMeta $meta) => is_string($meta->name)),
-            $this->isType('string'),
         )->willReturn(ApiKey::create());
 
         $this->commandTester->execute([

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')),
+                $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice')),
-                $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withName('Alice and Bob')),
+                $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'Alice and Bob')),
-                $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::withName('')),
+                $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: '')),
                 $apiKey4 = ApiKey::create(),
             ],
             true,

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' => [

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<RoleDefinition> $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
+    /**
-    {
+     * @param iterable<RoleDefinition> $roleDefinitions
-        return new self(null, $expirationDate, []);
+     */
-    }
+    public static function fromParams(
-
+        ?string $key = null,
-    public static function withNameAndExpirationDate(string $name, Chronos $expirationDate): self
+        ?string $name = null,
-    {
+        ?Chronos $expirationDate = null,
-        return new self($name, $expirationDate, []);
+        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);
     }
 }

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();
             }
         });

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;

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(
+    public function create(ApiKeyMeta $apiKeyMeta): ApiKey
-        ?Chronos $expirationDate = null,
+    {
-        ?string $name = null,
+        $apiKey = ApiKey::fromMeta($apiKeyMeta);
-        RoleDefinition ...$roleDefinitions,
-    ): ApiKey {
-        $key = $this->buildApiKeyWithParams($expirationDate, $name);
-        foreach ($roleDefinitions as $definition) {
-            $key->registerRole($definition);
-        }
 
-        $this->em->persist($key);
+        $this->em->persist($apiKey);
         $this->em->flush();
 
-        return $key;
+        return $apiKey;
-    }
-
-    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(),
-        };
     }
 
     public function check(string $key): ApiKeyCheckResult

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(
+    public function create(ApiKeyMeta $apiKeyMeta): ApiKey;
-        ?Chronos $expirationDate = null,
-        ?string $name = null,
-        RoleDefinition ...$roleDefinitions,
-    ): ApiKey;
 
     public function check(string $key): ApiKeyCheckResult;
 

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);

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);
     }