Merge pull request #2497 from acelaya-forks/delete-api-key

Add new command to delete API keys

module/CLI/config/cli.config.php

@@ -26,6 +26,7 @@ return [
 
             Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class,
             Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class,
+            Command\Api\DeleteKeyCommand::NAME => Command\Api\DeleteKeyCommand::class,
             Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class,
             Command\Api\InitialApiKeyCommand::NAME => Command\Api\InitialApiKeyCommand::class,
             Command\Api\RenameApiKeyCommand::NAME => Command\Api\RenameApiKeyCommand::class,

module/CLI/config/dependencies.config.php

@@ -52,6 +52,7 @@ return [
 
             Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class,
             Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class,
+            Command\Api\DeleteKeyCommand::class => ConfigAbstractFactory::class,
             Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class,
             Command\Api\InitialApiKeyCommand::class => ConfigAbstractFactory::class,
             Command\Api\RenameApiKeyCommand::class => ConfigAbstractFactory::class,
@@ -108,6 +109,7 @@ return [
 
         Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class],
         Command\Api\DisableKeyCommand::class => [ApiKeyService::class],
+        Command\Api\DeleteKeyCommand::class => [ApiKeyService::class],
         Command\Api\ListKeysCommand::class => [ApiKeyService::class],
         Command\Api\InitialApiKeyCommand::class => [ApiKeyService::class],
         Command\Api\RenameApiKeyCommand::class => [ApiKeyService::class],

module/CLI/src/Command/Api/DeleteKeyCommand.php

@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\CLI\Command\Api;
+
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
+use Shlinkio\Shlink\Rest\Exception\ApiKeyNotFoundException;
+use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
+use Symfony\Component\Console\Attribute\Argument;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Style\SymfonyStyle;
+
+use function Shlinkio\Shlink\Core\ArrayUtils\map;
+use function sprintf;
+
+#[AsCommand(
+    name: DeleteKeyCommand::NAME,
+    description: 'Deletes an API key by name',
+    help: <<<HELP
+    The <info>%command.name%</info> command allows you to delete an existing API key via its name.
+
+    If no arguments are provided, you will be prompted to select one of the existing API keys.
+
+        <info>%command.full_name%</info>
+
+    You can optionally pass the API key name to be disabled:
+
+        <info>%command.full_name% the_key_name</info>
+
+    HELP,
+)]
+class DeleteKeyCommand extends Command
+{
+    public const string NAME = 'api-key:delete';
+
+    public function __construct(private readonly ApiKeyServiceInterface $apiKeyService)
+    {
+        parent::__construct();
+    }
+
+    protected function interact(InputInterface $input, OutputInterface $output): void
+    {
+        $apiKeyName = $input->getArgument('name');
+
+        if ($apiKeyName === null) {
+            $apiKeys = $this->apiKeyService->listKeys();
+            $name = (new SymfonyStyle($input, $output))->choice(
+                'What API key do you want to delete?',
+                map($apiKeys, static fn (ApiKey $apiKey) => $apiKey->name),
+            );
+
+            $input->setArgument('name', $name);
+        }
+    }
+
+    public function __invoke(
+        SymfonyStyle $io,
+        InputInterface $input,
+        #[Argument(description: 'The API key to delete.')]
+        string|null $name = null,
+    ): int {
+        if ($name === null) {
+            $io->warning('An API key name was not provided.');
+            return Command::INVALID;
+        }
+
+        if (! $this->shouldProceed($io, $input)) {
+            return Command::INVALID;
+        }
+
+        try {
+            $this->apiKeyService->deleteByName($name);
+            $io->success(sprintf('API key "%s" properly deleted', $name));
+            return Command::SUCCESS;
+        } catch (ApiKeyNotFoundException $e) {
+            $io->error($e->getMessage());
+            return Command::FAILURE;
+        }
+    }
+
+    private function shouldProceed(SymfonyStyle $io, InputInterface $input): bool
+    {
+        if (! $input->isInteractive()) {
+            return true;
+        }
+
+        $io->warning('You are about to delete an API key. This action cannot be undone.');
+        return $io->confirm('Are you sure you want to delete the API key?');
+    }
+}

module/CLI/test/Command/Api/DeleteKeyCommandTest.php

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\CLI\Command\Api;
+
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\CLI\Command\Api\DeleteKeyCommand;
+use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
+use Shlinkio\Shlink\Rest\Exception\ApiKeyNotFoundException;
+use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
+use ShlinkioTest\Shlink\CLI\Util\CliTestUtils;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Tester\CommandTester;
+
+class DeleteKeyCommandTest extends TestCase
+{
+    private CommandTester $commandTester;
+    private MockObject & ApiKeyServiceInterface $apiKeyService;
+
+    protected function setUp(): void
+    {
+        $this->apiKeyService = $this->createMock(ApiKeyServiceInterface::class);
+        $this->commandTester = CliTestUtils::testerForCommand(new DeleteKeyCommand($this->apiKeyService));
+    }
+
+    #[Test]
+    public function warningIsReturnedIfNoArgumentIsProvidedInNonInteractiveMode(): void
+    {
+        $this->apiKeyService->expects($this->never())->method('deleteByName');
+        $this->apiKeyService->expects($this->never())->method('listKeys');
+
+        $exitCode = $this->commandTester->execute([], ['interactive' => false]);
+
+        self::assertEquals(Command::INVALID, $exitCode);
+    }
+
+    #[Test]
+    public function confirmationIsSkippedInNonInteractiveMode(): void
+    {
+        $this->apiKeyService->expects($this->once())->method('deleteByName');
+        $this->apiKeyService->expects($this->never())->method('listKeys');
+
+        $exitCode = $this->commandTester->execute(['name' => 'key to delete'], ['interactive' => false]);
+        $output = $this->commandTester->getDisplay();
+
+        self::assertEquals(Command::SUCCESS, $exitCode);
+        self::assertStringNotContainsString('Are you sure you want to delete the API key?', $output);
+    }
+
+    #[Test]
+    public function keyIsNotDeletedIfConfirmationIsCancelled(): void
+    {
+        $this->apiKeyService->expects($this->never())->method('deleteByName');
+        $this->apiKeyService->expects($this->never())->method('listKeys');
+
+        $this->commandTester->setInputs(['no']);
+        $exitCode = $this->commandTester->execute(['name' => 'key_to_delete']);
+
+        self::assertEquals(Command::INVALID, $exitCode);
+    }
+
+    #[Test]
+    public function existingApiKeyNamesAreListedIfNoArgumentIsProvidedInInteractiveMode(): void
+    {
+        $name = 'the key to delete';
+        $this->apiKeyService->expects($this->once())->method('deleteByName')->with($name);
+        $this->apiKeyService->expects($this->once())->method('listKeys')->willReturn([
+            ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'foo')),
+            ApiKey::fromMeta(ApiKeyMeta::fromParams(name: $name)),
+            ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'bar')),
+        ]);
+
+        $this->commandTester->setInputs([$name, 'y']);
+        $exitCode = $this->commandTester->execute([]);
+        $output = $this->commandTester->getDisplay();
+
+        self::assertStringContainsString('What API key do you want to delete?', $output);
+        self::assertStringContainsString('API key "the key to delete" properly deleted', $output);
+        self::assertEquals(Command::SUCCESS, $exitCode);
+    }
+
+    #[Test]
+    public function errorIsReturnedIfDisableByKeyThrowsException(): void
+    {
+        $apiKey = 'key to delete';
+        $e = ApiKeyNotFoundException::forName($apiKey);
+        $this->apiKeyService->expects($this->once())->method('deleteByName')->with($apiKey)->willThrowException($e);
+        $this->apiKeyService->expects($this->never())->method('listKeys');
+
+        $exitCode = $this->commandTester->execute(['name' => $apiKey]);
+        $output = $this->commandTester->getDisplay();
+
+        self::assertStringContainsString($e->getMessage(), $output);
+        self::assertEquals(Command::FAILURE, $exitCode);
+    }
+}

module/Rest/src/ApiKey/Repository/ApiKeyRepository.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Rest\ApiKey\Repository;
 
 use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\QueryBuilder;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
@@ -48,10 +49,9 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe
     {
         $qb = $this->getEntityManager()->createQueryBuilder();
         $qb->select('a.id')
-           ->from(ApiKey::class, 'a')
-           ->where($qb->expr()->eq('a.name', ':name'))
-           ->setParameter('name', $name)
-           ->setMaxResults(1);
+           ->from(ApiKey::class, 'a');
+
+        $this->queryBuilderByName($qb, $name);
 
         // Lock for update, to avoid a race condition that inserts a duplicate name after we have checked if one existed
         $query = $qb->getQuery();
@@ -59,4 +59,27 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe
 
         return $query->getOneOrNullResult() !== null;
     }
+
+    /**
+     * @inheritDoc
+     */
+    public function deleteByName(string $name): int
+    {
+        $qb = $this->getEntityManager()->createQueryBuilder();
+        $qb->delete(ApiKey::class, 'a');
+
+        $this->queryBuilderByName($qb, $name);
+
+        return $qb->getQuery()->execute();
+    }
+
+    /**
+     * Apply a condition by name to a query builder, and ensure only one result is returned
+     */
+    private function queryBuilderByName(QueryBuilder $qb, string $name): void
+    {
+        $qb->where($qb->expr()->eq('a.name', ':name'))
+           ->setParameter('name', $name)
+           ->setMaxResults(1);
+    }
 }

module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php

@@ -22,4 +22,10 @@ interface ApiKeyRepositoryInterface extends EntityRepositoryInterface, EntitySpe
      * Checks whether an API key with provided name exists or not
      */
     public function nameExists(string $name): bool;
+
+    /**
+     * Delete an API key by name
+     * @return positive-int|0 Number of affected results
+     */
+    public function deleteByName(string $name): int;
 }

module/Rest/src/Service/ApiKeyService.php

@@ -68,6 +68,17 @@ readonly class ApiKeyService implements ApiKeyServiceInterface
         return new ApiKeyCheckResult($apiKey);
     }
 
+    /**
+     * @inheritDoc
+     */
+    public function deleteByName(string $apiKeyName): void
+    {
+        $affectedResults = $this->repo->deleteByName($apiKeyName);
+        if ($affectedResults === 0) {
+            throw ApiKeyNotFoundException::forName($apiKeyName);
+        }
+    }
+
     /**
      * @inheritDoc
      */

module/Rest/src/Service/ApiKeyServiceInterface.php

@@ -22,6 +22,11 @@ interface ApiKeyServiceInterface
 
     public function check(string $key): ApiKeyCheckResult;
 
+    /**
+     * @throws ApiKeyNotFoundException
+     */
+    public function deleteByName(string $apiKeyName): void;
+
     /**
      * @throws ApiKeyNotFoundException
      */

module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php

@@ -40,4 +40,18 @@ class ApiKeyRepositoryTest extends DatabaseTestCase
         self::assertTrue($this->repo->nameExists('foo'));
         self::assertFalse($this->repo->nameExists('bar'));
     }
+
+    #[Test]
+    public function deleteByNameReturnsExpectedValue(): void
+    {
+        $this->getEntityManager()->persist(ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'foo')));
+        $this->getEntityManager()->flush();
+        $this->getEntityManager()->clear();
+
+        self::assertEquals(0, $this->repo->deleteByName('invalid'));
+        self::assertEquals(1, $this->repo->deleteByName('foo'));
+
+        // Verify the API key has been deleted
+        self::assertNull($this->repo->findOneBy(['name' => 'foo']));
+    }
 }

module/Rest/test/Service/ApiKeyServiceTest.php

@@ -8,6 +8,7 @@ use Cake\Chronos\Chronos;
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
@@ -268,4 +269,19 @@ class ApiKeyServiceTest extends TestCase
         self::assertSame($apiKey, $result);
         self::assertEquals('new', $apiKey->name);
     }
+
+    #[Test]
+    #[TestWith([0, true])]
+    #[TestWith([1, false])]
+    public function deleteByNameThrowsIfNoResultsAreAffected(int $affectedResults, bool $shouldThrow): void
+    {
+        $name = 'some_name';
+        $this->repo->expects($this->once())->method('deleteByName')->with($name)->willReturn($affectedResults);
+
+        if ($shouldThrow) {
+            $this->expectException(ApiKeyNotFoundException::class);
+        }
+
+        $this->service->deleteByName($name);
+    }
 }