Added used API key to request

This commit is contained in:
Alejandro Celaya 2020-11-08 11:28:27 +01:00
parent 598f2d8622
commit 90551ff3bc
12 changed files with 87 additions and 44 deletions

View file

@ -58,7 +58,7 @@ final class Version20180913205455 extends AbstractMigration
} }
try { try {
return (string) IpAddress::fromString($addr)->getObfuscatedCopy(); return (string) IpAddress::fromString($addr)->getAnonymizedCopy();
} catch (InvalidArgumentException $e) { } catch (InvalidArgumentException $e) {
return null; return null;
} }

View file

@ -28,7 +28,7 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction
]); ]);
} }
$payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request); $payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request)->toString();
$meta = ShortUrlMeta::fromRawData($payload); $meta = ShortUrlMeta::fromRawData($payload);
return new CreateShortUrlData($payload['longUrl'], (array) ($payload['tags'] ?? []), $meta); return new CreateShortUrlData($payload['longUrl'], (array) ($payload['tags'] ?? []), $meta);

View file

@ -34,10 +34,10 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
protected function buildShortUrlData(Request $request): CreateShortUrlData protected function buildShortUrlData(Request $request): CreateShortUrlData
{ {
$query = $request->getQueryParams(); $query = $request->getQueryParams();
$apiKey = $query['apiKey'] ?? '';
$longUrl = $query['longUrl'] ?? null; $longUrl = $query['longUrl'] ?? null;
if (! $this->apiKeyService->check($apiKey)) { $apiKeyResult = $this->apiKeyService->check($query['apiKey'] ?? '');
if (! $apiKeyResult->isValid()) {
throw ValidationException::fromArray([ throw ValidationException::fromArray([
'apiKey' => 'No API key was provided or it is not valid', 'apiKey' => 'No API key was provided or it is not valid',
]); ]);
@ -50,7 +50,7 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
} }
return new CreateShortUrlData($longUrl, [], ShortUrlMeta::fromRawData([ return new CreateShortUrlData($longUrl, [], ShortUrlMeta::fromRawData([
ShortUrlMetaInputFilter::API_KEY => $apiKey, ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey()->toString(),
])); ]));
} }
} }

View file

@ -54,4 +54,9 @@ class ApiKey extends AbstractEntity
{ {
return $this->key; return $this->key;
} }
public function toString(): string
{
return $this->key;
}
} }

View file

@ -11,6 +11,7 @@ use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface; use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException;
use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
@ -43,20 +44,21 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
return $handler->handle($request); return $handler->handle($request);
} }
$apiKey = self::apiKeyFromRequest($request); $apiKey = $request->getHeaderLine(self::API_KEY_HEADER);
if (empty($apiKey)) { if (empty($apiKey)) {
throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]); throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]);
} }
if (! $this->apiKeyService->check($apiKey)) { $result = $this->apiKeyService->check($apiKey);
if (! $result->isValid()) {
throw VerifyAuthenticationException::forInvalidApiKey(); throw VerifyAuthenticationException::forInvalidApiKey();
} }
return $handler->handle($request); return $handler->handle($request->withAttribute(ApiKey::class, $result->apiKey()));
} }
public static function apiKeyFromRequest(Request $request): string public static function apiKeyFromRequest(Request $request): ApiKey
{ {
return $request->getHeaderLine(self::API_KEY_HEADER); return $request->getAttribute(ApiKey::class);
} }
} }

View file

@ -0,0 +1,27 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Service;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
final class ApiKeyCheckResult
{
private ?ApiKey $apiKey;
public function __construct(?ApiKey $apiKey = null)
{
$this->apiKey = $apiKey;
}
public function isValid(): bool
{
return $this->apiKey !== null && $this->apiKey->isValid();
}
public function apiKey(): ?ApiKey
{
return $this->apiKey;
}
}

View file

@ -29,11 +29,11 @@ class ApiKeyService implements ApiKeyServiceInterface
return $key; return $key;
} }
public function check(string $key): bool public function check(string $key): ApiKeyCheckResult
{ {
/** @var ApiKey|null $apiKey */ /** @var ApiKey|null $apiKey */
$apiKey = $this->getByKey($key); $apiKey = $this->getByKey($key);
return $apiKey !== null && $apiKey->isValid(); return new ApiKeyCheckResult($apiKey);
} }
/** /**
@ -63,7 +63,7 @@ class ApiKeyService implements ApiKeyServiceInterface
return $apiKeys; return $apiKeys;
} }
public function getByKey(string $key): ?ApiKey private function getByKey(string $key): ?ApiKey
{ {
/** @var ApiKey|null $apiKey */ /** @var ApiKey|null $apiKey */
$apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([

View file

@ -12,7 +12,7 @@ interface ApiKeyServiceInterface
{ {
public function create(?Chronos $expirationDate = null): ApiKey; public function create(?Chronos $expirationDate = null): ApiKey;
public function check(string $key): bool; public function check(string $key): ApiKeyCheckResult;
/** /**
* @throws InvalidArgumentException * @throws InvalidArgumentException
@ -23,6 +23,4 @@ interface ApiKeyServiceInterface
* @return ApiKey[] * @return ApiKey[]
*/ */
public function listKeys(bool $enabledOnly = false): array; public function listKeys(bool $enabledOnly = false): array;
public function getByKey(string $key): ?ApiKey;
} }

View file

@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortener;
use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use function strpos; use function strpos;
@ -48,19 +49,19 @@ class CreateShortUrlActionTest extends TestCase
* @test * @test
* @dataProvider provideRequestBodies * @dataProvider provideRequestBodies
*/ */
public function properShortcodeConversionReturnsData(array $body, ShortUrlMeta $expectedMeta, ?string $apiKey): void public function properShortcodeConversionReturnsData(array $body, array $expectedMeta): void
{ {
$apiKey = new ApiKey();
$shortUrl = new ShortUrl(''); $shortUrl = new ShortUrl('');
$expectedMeta['apiKey'] = $apiKey->toString();
$shorten = $this->urlShortener->shorten( $shorten = $this->urlShortener->shorten(
Argument::type('string'), Argument::type('string'),
Argument::type('array'), Argument::type('array'),
$expectedMeta, ShortUrlMeta::fromRawData($expectedMeta),
)->willReturn($shortUrl); )->willReturn($shortUrl);
$request = ServerRequestFactory::fromGlobals()->withParsedBody($body); $request = ServerRequestFactory::fromGlobals()->withParsedBody($body)->withAttribute(ApiKey::class, $apiKey);
if ($apiKey !== null) {
$request = $request->withHeader('X-Api-Key', $apiKey);
}
$response = $this->action->handle($request); $response = $this->action->handle($request);
@ -81,14 +82,8 @@ class CreateShortUrlActionTest extends TestCase
'domain' => 'my-domain.com', 'domain' => 'my-domain.com',
]; ];
yield 'no data' => [['longUrl' => 'http://www.domain.com/foo/bar'], ShortUrlMeta::createEmpty(), null]; yield 'no data' => [['longUrl' => 'http://www.domain.com/foo/bar'], []];
yield 'all data' => [$fullMeta, ShortUrlMeta::fromRawData($fullMeta), null]; yield 'all data' => [$fullMeta, $fullMeta];
yield 'all data and API key' => (static function (array $meta): array {
$apiKey = 'abc123';
$meta['apiKey'] = $apiKey;
return [$meta, ShortUrlMeta::fromRawData($meta), $apiKey];
})($fullMeta);
} }
/** /**
@ -103,7 +98,7 @@ class CreateShortUrlActionTest extends TestCase
$request = (new ServerRequest())->withParsedBody([ $request = (new ServerRequest())->withParsedBody([
'longUrl' => 'http://www.domain.com/foo/bar', 'longUrl' => 'http://www.domain.com/foo/bar',
'domain' => $domain, 'domain' => $domain,
]); ])->withAttribute(ApiKey::class, new ApiKey());
$this->expectException(ValidationException::class); $this->expectException(ValidationException::class);
$urlToShortCode->shouldNotBeCalled(); $urlToShortCode->shouldNotBeCalled();

View file

@ -15,6 +15,8 @@ use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Service\ApiKeyCheckResult;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
class SingleStepCreateShortUrlActionTest extends TestCase class SingleStepCreateShortUrlActionTest extends TestCase
@ -44,7 +46,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void
{ {
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(false); $findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult());
$this->expectException(ValidationException::class); $this->expectException(ValidationException::class);
$findApiKey->shouldBeCalledOnce(); $findApiKey->shouldBeCalledOnce();
@ -56,7 +58,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase
public function errorResponseIsReturnedIfNoUrlIsProvided(): void public function errorResponseIsReturnedIfNoUrlIsProvided(): void
{ {
$request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']); $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); $findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult(new ApiKey()));
$this->expectException(ValidationException::class); $this->expectException(ValidationException::class);
$findApiKey->shouldBeCalledOnce(); $findApiKey->shouldBeCalledOnce();
@ -67,18 +69,21 @@ class SingleStepCreateShortUrlActionTest extends TestCase
/** @test */ /** @test */
public function properDataIsPassedWhenGeneratingShortCode(): void public function properDataIsPassedWhenGeneratingShortCode(): void
{ {
$apiKey = new ApiKey();
$key = $apiKey->toString();
$request = (new ServerRequest())->withQueryParams([ $request = (new ServerRequest())->withQueryParams([
'apiKey' => 'abc123', 'apiKey' => $key,
'longUrl' => 'http://foobar.com', 'longUrl' => 'http://foobar.com',
]); ]);
$findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); $findApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey));
$generateShortCode = $this->urlShortener->shorten( $generateShortCode = $this->urlShortener->shorten(
Argument::that(function (string $argument): string { Argument::that(function (string $argument): string {
Assert::assertEquals('http://foobar.com', $argument); Assert::assertEquals('http://foobar.com', $argument);
return $argument; return $argument;
}), }),
[], [],
ShortUrlMeta::fromRawData(['apiKey' => 'abc123']), ShortUrlMeta::fromRawData(['apiKey' => $key]),
)->willReturn(new ShortUrl('')); )->willReturn(new ShortUrl(''));
$resp = $this->action->handle($request); $resp = $this->action->handle($request);

View file

@ -18,9 +18,11 @@ use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface; use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Rest\Action\HealthAction; use Shlinkio\Shlink\Rest\Action\HealthAction;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException;
use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
use Shlinkio\Shlink\Rest\Service\ApiKeyCheckResult;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
use function Laminas\Stratigility\middleware; use function Laminas\Stratigility\middleware;
@ -114,7 +116,7 @@ class AuthenticationMiddlewareTest extends TestCase
) )
->withHeader('X-Api-Key', $apiKey); ->withHeader('X-Api-Key', $apiKey);
$this->apiKeyService->check($apiKey)->willReturn(false)->shouldBeCalledOnce(); $this->apiKeyService->check($apiKey)->willReturn(new ApiKeyCheckResult())->shouldBeCalledOnce();
$this->handler->handle($request)->shouldNotBeCalled(); $this->handler->handle($request)->shouldNotBeCalled();
$this->expectException(VerifyAuthenticationException::class); $this->expectException(VerifyAuthenticationException::class);
$this->expectExceptionMessage('Provided API key does not exist or is invalid'); $this->expectExceptionMessage('Provided API key does not exist or is invalid');
@ -125,16 +127,17 @@ class AuthenticationMiddlewareTest extends TestCase
/** @test */ /** @test */
public function validApiKeyFallsBackToNextMiddleware(): void public function validApiKeyFallsBackToNextMiddleware(): void
{ {
$apiKey = 'abc123'; $apiKey = new ApiKey();
$key = $apiKey->toString();
$request = ServerRequestFactory::fromGlobals() $request = ServerRequestFactory::fromGlobals()
->withAttribute( ->withAttribute(
RouteResult::class, RouteResult::class,
RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []), RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []),
) )
->withHeader('X-Api-Key', $apiKey); ->withHeader('X-Api-Key', $key);
$handle = $this->handler->handle($request)->willReturn(new Response()); $handle = $this->handler->handle($request->withAttribute(ApiKey::class, $apiKey))->willReturn(new Response());
$checkApiKey = $this->apiKeyService->check($apiKey)->willReturn(true); $checkApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey));
$this->middleware->process($request, $this->handler->reveal()); $this->middleware->process($request, $this->handler->reveal());

View file

@ -59,7 +59,10 @@ class ApiKeyServiceTest extends TestCase
->shouldBeCalledOnce(); ->shouldBeCalledOnce();
$this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal());
self::assertFalse($this->service->check('12345')); $result = $this->service->check('12345');
self::assertFalse($result->isValid());
self::assertSame($invalidKey, $result->apiKey());
} }
public function provideInvalidApiKeys(): iterable public function provideInvalidApiKeys(): iterable
@ -72,12 +75,17 @@ class ApiKeyServiceTest extends TestCase
/** @test */ /** @test */
public function checkReturnsTrueWhenConditionsAreFavorable(): void public function checkReturnsTrueWhenConditionsAreFavorable(): void
{ {
$apiKey = new ApiKey();
$repo = $this->prophesize(EntityRepository::class); $repo = $this->prophesize(EntityRepository::class);
$repo->findOneBy(['key' => '12345'])->willReturn(new ApiKey()) $repo->findOneBy(['key' => '12345'])->willReturn($apiKey)
->shouldBeCalledOnce(); ->shouldBeCalledOnce();
$this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal());
self::assertTrue($this->service->check('12345')); $result = $this->service->check('12345');
self::assertTrue($result->isValid());
self::assertSame($apiKey, $result->apiKey());
} }
/** @test */ /** @test */