diff --git a/data/migrations/Version20180913205455.php b/data/migrations/Version20180913205455.php index 8afa316b..c2bc2070 100644 --- a/data/migrations/Version20180913205455.php +++ b/data/migrations/Version20180913205455.php @@ -58,7 +58,7 @@ final class Version20180913205455 extends AbstractMigration } try { - return (string) IpAddress::fromString($addr)->getObfuscatedCopy(); + return (string) IpAddress::fromString($addr)->getAnonymizedCopy(); } catch (InvalidArgumentException $e) { return null; } diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index 28941579..b3db6460 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -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); return new CreateShortUrlData($payload['longUrl'], (array) ($payload['tags'] ?? []), $meta); diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index fe8c44aa..996e59a6 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -34,10 +34,10 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction protected function buildShortUrlData(Request $request): CreateShortUrlData { $query = $request->getQueryParams(); - $apiKey = $query['apiKey'] ?? ''; $longUrl = $query['longUrl'] ?? null; - if (! $this->apiKeyService->check($apiKey)) { + $apiKeyResult = $this->apiKeyService->check($query['apiKey'] ?? ''); + if (! $apiKeyResult->isValid()) { throw ValidationException::fromArray([ '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([ - ShortUrlMetaInputFilter::API_KEY => $apiKey, + ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey()->toString(), ])); } } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 1d372c9c..a800d530 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -54,4 +54,9 @@ class ApiKey extends AbstractEntity { return $this->key; } + + public function toString(): string + { + return $this->key; + } } diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index add9f513..1eff50d2 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -11,6 +11,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -43,20 +44,21 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa return $handler->handle($request); } - $apiKey = self::apiKeyFromRequest($request); + $apiKey = $request->getHeaderLine(self::API_KEY_HEADER); if (empty($apiKey)) { throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]); } - if (! $this->apiKeyService->check($apiKey)) { + $result = $this->apiKeyService->check($apiKey); + if (! $result->isValid()) { 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); } } diff --git a/module/Rest/src/Service/ApiKeyCheckResult.php b/module/Rest/src/Service/ApiKeyCheckResult.php new file mode 100644 index 00000000..8ec3f65e --- /dev/null +++ b/module/Rest/src/Service/ApiKeyCheckResult.php @@ -0,0 +1,27 @@ +apiKey = $apiKey; + } + + public function isValid(): bool + { + return $this->apiKey !== null && $this->apiKey->isValid(); + } + + public function apiKey(): ?ApiKey + { + return $this->apiKey; + } +} diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index baa545c0..6fb61be9 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -29,11 +29,11 @@ class ApiKeyService implements ApiKeyServiceInterface return $key; } - public function check(string $key): bool + public function check(string $key): ApiKeyCheckResult { /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); - return $apiKey !== null && $apiKey->isValid(); + return new ApiKeyCheckResult($apiKey); } /** @@ -63,7 +63,7 @@ class ApiKeyService implements ApiKeyServiceInterface return $apiKeys; } - public function getByKey(string $key): ?ApiKey + private function getByKey(string $key): ?ApiKey { /** @var ApiKey|null $apiKey */ $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index a08d8c60..e8c6d0ea 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -12,7 +12,7 @@ interface ApiKeyServiceInterface { public function create(?Chronos $expirationDate = null): ApiKey; - public function check(string $key): bool; + public function check(string $key): ApiKeyCheckResult; /** * @throws InvalidArgumentException @@ -23,6 +23,4 @@ interface ApiKeyServiceInterface * @return ApiKey[] */ public function listKeys(bool $enabledOnly = false): array; - - public function getByKey(string $key): ?ApiKey; } diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index 91e6014c..082b1783 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use function strpos; @@ -48,19 +49,19 @@ class CreateShortUrlActionTest extends TestCase * @test * @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(''); + $expectedMeta['apiKey'] = $apiKey->toString(); + $shorten = $this->urlShortener->shorten( Argument::type('string'), Argument::type('array'), - $expectedMeta, + ShortUrlMeta::fromRawData($expectedMeta), )->willReturn($shortUrl); - $request = ServerRequestFactory::fromGlobals()->withParsedBody($body); - if ($apiKey !== null) { - $request = $request->withHeader('X-Api-Key', $apiKey); - } + $request = ServerRequestFactory::fromGlobals()->withParsedBody($body)->withAttribute(ApiKey::class, $apiKey); $response = $this->action->handle($request); @@ -81,14 +82,8 @@ class CreateShortUrlActionTest extends TestCase 'domain' => 'my-domain.com', ]; - yield 'no data' => [['longUrl' => 'http://www.domain.com/foo/bar'], ShortUrlMeta::createEmpty(), null]; - yield 'all data' => [$fullMeta, ShortUrlMeta::fromRawData($fullMeta), null]; - yield 'all data and API key' => (static function (array $meta): array { - $apiKey = 'abc123'; - $meta['apiKey'] = $apiKey; - - return [$meta, ShortUrlMeta::fromRawData($meta), $apiKey]; - })($fullMeta); + yield 'no data' => [['longUrl' => 'http://www.domain.com/foo/bar'], []]; + yield 'all data' => [$fullMeta, $fullMeta]; } /** @@ -103,7 +98,7 @@ class CreateShortUrlActionTest extends TestCase $request = (new ServerRequest())->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', 'domain' => $domain, - ]); + ])->withAttribute(ApiKey::class, new ApiKey()); $this->expectException(ValidationException::class); $urlToShortCode->shouldNotBeCalled(); diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index 62005c8d..eb1d6cd2 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -15,6 +15,8 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; 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; class SingleStepCreateShortUrlActionTest extends TestCase @@ -44,7 +46,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void { $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); $findApiKey->shouldBeCalledOnce(); @@ -56,7 +58,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase public function errorResponseIsReturnedIfNoUrlIsProvided(): void { $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); $findApiKey->shouldBeCalledOnce(); @@ -67,18 +69,21 @@ class SingleStepCreateShortUrlActionTest extends TestCase /** @test */ public function properDataIsPassedWhenGeneratingShortCode(): void { + $apiKey = new ApiKey(); + $key = $apiKey->toString(); + $request = (new ServerRequest())->withQueryParams([ - 'apiKey' => 'abc123', + 'apiKey' => $key, 'longUrl' => 'http://foobar.com', ]); - $findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); + $findApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey)); $generateShortCode = $this->urlShortener->shorten( Argument::that(function (string $argument): string { Assert::assertEquals('http://foobar.com', $argument); return $argument; }), [], - ShortUrlMeta::fromRawData(['apiKey' => 'abc123']), + ShortUrlMeta::fromRawData(['apiKey' => $key]), )->willReturn(new ShortUrl('')); $resp = $this->action->handle($request); diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index db721780..39559f67 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -18,9 +18,11 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Action\HealthAction; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; +use Shlinkio\Shlink\Rest\Service\ApiKeyCheckResult; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use function Laminas\Stratigility\middleware; @@ -114,7 +116,7 @@ class AuthenticationMiddlewareTest extends TestCase ) ->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->expectException(VerifyAuthenticationException::class); $this->expectExceptionMessage('Provided API key does not exist or is invalid'); @@ -125,16 +127,17 @@ class AuthenticationMiddlewareTest extends TestCase /** @test */ public function validApiKeyFallsBackToNextMiddleware(): void { - $apiKey = 'abc123'; + $apiKey = new ApiKey(); + $key = $apiKey->toString(); $request = ServerRequestFactory::fromGlobals() ->withAttribute( RouteResult::class, 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()); - $checkApiKey = $this->apiKeyService->check($apiKey)->willReturn(true); + $handle = $this->handler->handle($request->withAttribute(ApiKey::class, $apiKey))->willReturn(new Response()); + $checkApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey)); $this->middleware->process($request, $this->handler->reveal()); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 656541f0..6d228661 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -59,7 +59,10 @@ class ApiKeyServiceTest extends TestCase ->shouldBeCalledOnce(); $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 @@ -72,12 +75,17 @@ class ApiKeyServiceTest extends TestCase /** @test */ public function checkReturnsTrueWhenConditionsAreFavorable(): void { + $apiKey = new ApiKey(); + $repo = $this->prophesize(EntityRepository::class); - $repo->findOneBy(['key' => '12345'])->willReturn(new ApiKey()) + $repo->findOneBy(['key' => '12345'])->willReturn($apiKey) ->shouldBeCalledOnce(); $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 */