Skip to content

Commit

Permalink
Merge pull request #2248 from acelaya-forks/feature/api-key-simplific…
Browse files Browse the repository at this point in the history
…ation

Simplify ApiKey entity by exposing key as a readonly prop
  • Loading branch information
acelaya authored Nov 4, 2024
2 parents b5010e4 + 79c5418 commit e4fe7ad
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 46 deletions.
2 changes: 1 addition & 1 deletion module/CLI/src/Command/Api/GenerateKeyCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
));

$io = new SymfonyStyle($input, $output);
$io->success(sprintf('Generated API key: "%s"', $apiKey->toString()));
$io->success(sprintf('Generated API key: "%s"', $apiKey->key));

if (! ApiKey::isAdmin($apiKey)) {
ShlinkTable::default($io)->render(
Expand Down
2 changes: 1 addition & 1 deletion module/CLI/src/Command/Api/ListKeysCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$messagePattern = $this->determineMessagePattern($apiKey);

// Set columns for this row
$rowData = [sprintf($messagePattern, $apiKey), sprintf($messagePattern, $apiKey->name ?? '-')];
$rowData = [sprintf($messagePattern, $apiKey->key), sprintf($messagePattern, $apiKey->name ?? '-')];
if (! $enabledOnly) {
$rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
}
Expand Down
2 changes: 1 addition & 1 deletion module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private function resolveColumnsMap(InputInterface $input): array
}
if ($input->getOption('show-api-key')) {
$columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string =>
$shortUrl->authorApiKey?->__toString() ?? '';
$shortUrl->authorApiKey?->key ?? '';
}
if ($input->getOption('show-api-key-name')) {
$columnsMap['API Key Name'] = static fn (array $_, ShortUrl $shortUrl): string|null =>
Expand Down
30 changes: 15 additions & 15 deletions module/CLI/test/Command/Api/ListKeysCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ public static function provideKeysAndOutputs(): iterable
+--------------------------------------+------+------------+---------------------------+-------+
| Key | Name | Is enabled | Expiration date | Roles |
+--------------------------------------+------+------------+---------------------------+-------+
| {$apiKey1} | - | --- | - | Admin |
| {$apiKey1->key} | - | --- | - | Admin |
+--------------------------------------+------+------------+---------------------------+-------+
| {$apiKey2} | - | --- | 2020-01-01T00:00:00+00:00 | Admin |
| {$apiKey2->key} | - | --- | 2020-01-01T00:00:00+00:00 | Admin |
+--------------------------------------+------+------------+---------------------------+-------+
| {$apiKey3} | - | +++ | - | Admin |
| {$apiKey3->key} | - | +++ | - | Admin |
+--------------------------------------+------+------------+---------------------------+-------+
OUTPUT,
Expand All @@ -71,9 +71,9 @@ public static function provideKeysAndOutputs(): iterable
+--------------------------------------+------+-----------------+-------+
| Key | Name | Expiration date | Roles |
+--------------------------------------+------+-----------------+-------+
| {$apiKey1} | - | - | Admin |
| {$apiKey1->key} | - | - | Admin |
+--------------------------------------+------+-----------------+-------+
| {$apiKey2} | - | - | Admin |
| {$apiKey2->key} | - | - | Admin |
+--------------------------------------+------+-----------------+-------+
OUTPUT,
Expand All @@ -97,18 +97,18 @@ public static function provideKeysAndOutputs(): iterable
+--------------------------------------+------+-----------------+--------------------------+
| Key | Name | Expiration date | Roles |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey1} | - | - | Admin |
| {$apiKey1->key} | - | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey2} | - | - | Author only |
| {$apiKey2->key} | - | - | Author only |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey3} | - | - | Domain only: example.com |
| {$apiKey3->key} | - | - | Domain only: example.com |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey4} | - | - | Admin |
| {$apiKey4->key} | - | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey5} | - | - | Author only |
| {$apiKey5->key} | - | - | Author only |
| | | | Domain only: example.com |
+--------------------------------------+------+-----------------+--------------------------+
| {$apiKey6} | - | - | Admin |
| {$apiKey6->key} | - | - | Admin |
+--------------------------------------+------+-----------------+--------------------------+
OUTPUT,
Expand All @@ -125,13 +125,13 @@ public static function provideKeysAndOutputs(): iterable
+--------------------------------------+---------------+-----------------+-------+
| Key | Name | Expiration date | Roles |
+--------------------------------------+---------------+-----------------+-------+
| {$apiKey1} | Alice | - | Admin |
| {$apiKey1->key} | Alice | - | Admin |
+--------------------------------------+---------------+-----------------+-------+
| {$apiKey2} | Alice and Bob | - | Admin |
| {$apiKey2->key} | Alice and Bob | - | Admin |
+--------------------------------------+---------------+-----------------+-------+
| {$apiKey3} | | - | Admin |
| {$apiKey3->key} | | - | Admin |
+--------------------------------------+---------------+-----------------+-------+
| {$apiKey4} | - | - | Admin |
| {$apiKey4->key} | - | - | Admin |
+--------------------------------------+---------------+-----------------+-------+
OUTPUT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public function provideOptionalFlagsMakesNewColumnsToBeIncluded(
public static function provideOptionalFlags(): iterable
{
$apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'my api key'));
$key = $apiKey->toString();
$key = $apiKey->key;

yield 'tags only' => [
['--show-tags' => true],
Expand Down
10 changes: 5 additions & 5 deletions module/Rest/src/ApiKey/Model/ApiKeyMeta.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
use Cake\Chronos\Chronos;
use Ramsey\Uuid\Uuid;

final class ApiKeyMeta
final readonly class ApiKeyMeta
{
/**
* @param iterable<RoleDefinition> $roleDefinitions
*/
private function __construct(
public readonly string $key,
public readonly string|null $name,
public readonly Chronos|null $expirationDate,
public readonly iterable $roleDefinitions,
public string $key,
public string|null $name,
public Chronos|null $expirationDate,
public iterable $roleDefinitions,
) {
}

Expand Down
13 changes: 1 addition & 12 deletions module/Rest/src/Entity/ApiKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ class ApiKey extends AbstractEntity
{
/**
* @param Collection<string, ApiKeyRole> $roles
* @throws Exception
*/
private function __construct(
private string $key,
public readonly string $key,
public readonly string|null $name = null,
public readonly Chronos|null $expirationDate = null,
private bool $enabled = true,
Expand Down Expand Up @@ -75,16 +74,6 @@ public function isValid(): bool
return $this->isEnabled() && ! $this->isExpired();
}

public function __toString(): string
{
return $this->key;
}

public function toString(): string
{
return $this->key;
}

public function spec(string|null $context = null): Specification
{
$specs = $this->roles->map(fn (ApiKeyRole $role) => Role::toSpec($role, $context))->getValues();
Expand Down
8 changes: 1 addition & 7 deletions module/Rest/test-api/Fixtures/ApiKeyFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Doctrine\Common\DataFixtures\AbstractFixture;
use Doctrine\Common\DataFixtures\DependentFixtureInterface;
use Doctrine\Persistence\ObjectManager;
use ReflectionObject;
use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
Expand Down Expand Up @@ -51,12 +50,7 @@ public function load(ObjectManager $manager): void

private function buildApiKey(string $key, bool $enabled, Chronos|null $expiresAt = null): ApiKey
{
$apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(expirationDate: $expiresAt));
$ref = new ReflectionObject($apiKey);
$keyProp = $ref->getProperty('key');
$keyProp->setAccessible(true);
$keyProp->setValue($apiKey, $key);

$apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $key, expirationDate: $expiresAt));
if (! $enabled) {
$apiKey->disable();
}
Expand Down
5 changes: 2 additions & 3 deletions module/Rest/test/Middleware/AuthenticationMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,17 @@ public function throwsExceptionWhenProvidedApiKeyIsInvalid(): void
public function validApiKeyFallsBackToNextMiddleware(): void
{
$apiKey = ApiKey::create();
$key = $apiKey->toString();
$request = ServerRequestFactory::fromGlobals()
->withAttribute(
RouteResult::class,
RouteResult::fromRoute(new Route('bar', self::getDummyMiddleware()), []),
)
->withHeader('X-Api-Key', $key);
->withHeader('X-Api-Key', $apiKey->key);

$this->handler->expects($this->once())->method('handle')->with(
$request->withAttribute(ApiKey::class, $apiKey),
)->willReturn(new Response());
$this->apiKeyService->expects($this->once())->method('check')->with($key)->willReturn(
$this->apiKeyService->expects($this->once())->method('check')->with($apiKey->key)->willReturn(
new ApiKeyCheckResult($apiKey),
);

Expand Down

0 comments on commit e4fe7ad

Please sign in to comment.