From bc46e2f509ce58a7f88e6e39d7a6a12dfd6a4a3d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 20 Sep 2018 21:15:17 +0200 Subject: [PATCH 1/5] Defined API key authentication type in swagger docs --- docs/swagger/paths/v1_authenticate.json | 5 ++-- docs/swagger/paths/v1_short-urls.json | 6 +++++ .../paths/v1_short-urls_{shortCode}.json | 9 +++++++ .../paths/v1_short-urls_{shortCode}_tags.json | 3 +++ .../v1_short-urls_{shortCode}_visits.json | 3 +++ docs/swagger/paths/v1_tags.json | 12 ++++++++++ docs/swagger/swagger.json | 24 ++++++++++++------- 7 files changed, 51 insertions(+), 11 deletions(-) diff --git a/docs/swagger/paths/v1_authenticate.json b/docs/swagger/paths/v1_authenticate.json index 49a690fb..cffa9e05 100644 --- a/docs/swagger/paths/v1_authenticate.json +++ b/docs/swagger/paths/v1_authenticate.json @@ -1,11 +1,12 @@ { "post": { + "deprecated": true, "operationId": "authenticate", "tags": [ "Authentication" ], - "summary": "Perform authentication", - "description": "Performs an authentication", + "summary": "[Deprecated] Perform authentication", + "description": "**This endpoint is deprecated, since the authentication can be performed via API key now**. Performs an authentication.", "requestBody": { "description": "Request body.", "required": true, diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 17b5bc23..42cf2c27 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -54,6 +54,9 @@ } ], "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -150,6 +153,9 @@ "summary": "Create short URL", "description": "Creates a new short URL.

**Important note**: Before shlink v1.13, this endpoint used to use the `/short-codes` path instead of `/short-urls`. Both of them will keep working, while the first one is considered deprecated.", "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 3ff49443..778d0fb1 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -18,6 +18,9 @@ } ], "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -122,6 +125,9 @@ } }, "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -182,6 +188,9 @@ } ], "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json index 89bdd0fd..ab05d230 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json @@ -41,6 +41,9 @@ } }, "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json index 1383edfd..acae7155 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json @@ -36,6 +36,9 @@ } ], "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } diff --git a/docs/swagger/paths/v1_tags.json b/docs/swagger/paths/v1_tags.json index fb4df70d..5bf260bb 100644 --- a/docs/swagger/paths/v1_tags.json +++ b/docs/swagger/paths/v1_tags.json @@ -7,6 +7,9 @@ "summary": "List existing tags", "description": "Returns the list of all tags used in any short URL, ordered by name", "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -68,6 +71,9 @@ "summary": "Create tags", "description": "Provided a list of tags, creates all that do not yet exist", "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -152,6 +158,9 @@ "summary": "Rename tag", "description": "Renames one existing tag", "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } @@ -240,6 +249,9 @@ } ], "security": [ + { + "ApiKey": [] + }, { "Bearer": [] } diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index ba8da350..d3cf0656 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -23,8 +23,14 @@ "components": { "securitySchemes": { + "ApiKey": { + "description": "A valid shlink API key", + "type": "apiKey", + "in": "header", + "name": "X-API-KEY" + }, "Bearer": { - "description": "The JWT identifying a previously logged API key", + "description": "**[Deprecated]** The JWT identifying a previously authenticated API key", "type": "http", "scheme": "bearer", "bearerFormat": "JWT" @@ -33,10 +39,6 @@ }, "tags": [ - { - "name": "Authentication", - "description": "Authentication-related endpoints" - }, { "name": "Short URLs", "description": "Operations that can be performed on short URLs" @@ -48,14 +50,14 @@ { "name": "Visits", "description": "Operations to manage visits on short URLs" + }, + { + "name": "Authentication", + "description": "Authentication-related endpoints" } ], "paths": { - "/v1/authenticate": { - "$ref": "paths/v1_authenticate.json" - }, - "/v1/short-urls": { "$ref": "paths/v1_short-urls.json" }, @@ -75,6 +77,10 @@ "/v1/short-urls/{shortCode}/visits": { "$ref": "paths/v1_short-urls_{shortCode}_visits.json" + }, + + "/v1/authenticate": { + "$ref": "paths/v1_authenticate.json" } } } From e88468d86747fef0e6b13bc51547b7ad3a279e5e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 24 Sep 2018 19:24:23 +0200 Subject: [PATCH 2/5] Renamed CheckAuthenticationMiddleware to just AuthenticationMiddleware --- config/autoload/middleware-pipeline.global.php | 2 +- docs/swagger/swagger.json | 2 +- module/Rest/config/dependencies.config.php | 4 ++-- ...leware.php => AuthenticationMiddleware.php} | 7 ++++--- ...st.php => AuthenticationMiddlewareTest.php} | 18 +++++++++--------- 5 files changed, 17 insertions(+), 16 deletions(-) rename module/Rest/src/Middleware/{CheckAuthenticationMiddleware.php => AuthenticationMiddleware.php} (95%) rename module/Rest/test/Middleware/{CheckAuthenticationMiddlewareTest.php => AuthenticationMiddlewareTest.php} (88%) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 308315bb..4bd14e39 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -39,7 +39,7 @@ return [ Rest\Middleware\CrossDomainMiddleware::class, Expressive\Router\Middleware\ImplicitOptionsMiddleware::class, Rest\Middleware\BodyParserMiddleware::class, - Rest\Middleware\CheckAuthenticationMiddleware::class, + Rest\Middleware\AuthenticationMiddleware::class, ], 'priority' => 5, ], diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index d3cf0656..1ca741bb 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -27,7 +27,7 @@ "description": "A valid shlink API key", "type": "apiKey", "in": "header", - "name": "X-API-KEY" + "name": "X-Api-Key" }, "Bearer": { "description": "**[Deprecated]** The JWT identifying a previously authenticated API key", diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index c9a9da98..a8cca560 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -35,7 +35,7 @@ return [ Middleware\BodyParserMiddleware::class => InvokableFactory::class, Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\PathVersionMiddleware::class => InvokableFactory::class, - Middleware\CheckAuthenticationMiddleware::class => ConfigAbstractFactory::class, + Middleware\AuthenticationMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class, Middleware\ShortUrl\ShortCodePathMiddleware::class => InvokableFactory::class, ], @@ -92,7 +92,7 @@ return [ Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, Translator::class, LoggerInterface::class], - Middleware\CheckAuthenticationMiddleware::class => [ + Middleware\AuthenticationMiddleware::class => [ Authentication\JWTService::class, 'translator', 'config.auth.routes_whitelist', diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php similarity index 95% rename from module/Rest/src/Middleware/CheckAuthenticationMiddleware.php rename to module/Rest/src/Middleware/AuthenticationMiddleware.php index dd0adf3b..124b4f03 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Middleware; +use Fig\Http\Message\RequestMethodInterface; use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -18,9 +19,10 @@ use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\TranslatorInterface; use Zend\Stdlib\ErrorHandler; -class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface +class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface { public const AUTHORIZATION_HEADER = 'Authorization'; + public const API_KEY_HEADER = 'X-Api-Key'; /** * @var TranslatorInterface @@ -64,12 +66,11 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeIn */ public function process(Request $request, RequestHandlerInterface $handler): Response { - // If current route is the authenticate route or an OPTIONS request, continue to the next middleware /** @var RouteResult|null $routeResult */ $routeResult = $request->getAttribute(RouteResult::class); if ($routeResult === null || $routeResult->isFailure() - || $request->getMethod() === 'OPTIONS' + || $request->getMethod() === self::METHOD_OPTIONS || \in_array($routeResult->getMatchedRouteName(), $this->routesWhitelist, true) ) { return $handler->handle($request); diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php similarity index 88% rename from module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php rename to module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 7c74a6aa..ee64eaa6 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -9,7 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\JWTService; -use Shlinkio\Shlink\Rest\Middleware\CheckAuthenticationMiddleware; +use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; @@ -18,10 +18,10 @@ use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\Translator; use function Zend\Stratigility\middleware; -class CheckAuthenticationMiddlewareTest extends TestCase +class AuthenticationMiddlewareTest extends TestCase { /** - * @var CheckAuthenticationMiddleware + * @var AuthenticationMiddleware */ protected $middleware; /** @@ -37,7 +37,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase public function setUp() { $this->jwtService = $this->prophesize(JWTService::class); - $this->middleware = new CheckAuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([]), [ + $this->middleware = new AuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([]), [ AuthenticateAction::class, ]); $this->dummyMiddleware = middleware(function () { @@ -116,7 +116,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); @@ -133,7 +133,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); @@ -152,7 +152,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); @@ -168,7 +168,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'bearer ' . $authToken); + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); @@ -178,6 +178,6 @@ class CheckAuthenticationMiddlewareTest extends TestCase $resp = $this->middleware->process($request, $delegate->reveal()); $process->shouldHaveBeenCalledTimes(1); - $this->assertArrayHasKey(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $resp->getHeaders()); + $this->assertArrayHasKey(AuthenticationMiddleware::AUTHORIZATION_HEADER, $resp->getHeaders()); } } From 8e61639598c7e10f01bc2fb2eb7cb54fbd0071f3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 28 Sep 2018 22:08:01 +0200 Subject: [PATCH 3/5] Created system of authentication plugins --- module/Rest/config/auth.config.php | 35 +++ module/Rest/config/dependencies.config.php | 8 - .../SingleStepCreateShortUrlAction.php | 4 +- .../AuthenticationPluginManager.php | 57 ++++ .../AuthenticationPluginManagerFactory.php | 31 ++ .../AuthenticationPluginManagerInterface.php | 17 + module/Rest/src/Authentication/JWTService.php | 3 +- .../Plugin/ApiKeyHeaderPlugin.php | 26 ++ .../Plugin/AuthenticationPluginInterface.php | 18 ++ .../Plugin/AuthorizationHeaderPlugin.php | 95 ++++++ .../src/Exception/AuthenticationException.php | 7 +- .../Exception/NoAuthenticationException.php | 18 ++ .../VerifyAuthenticationException.php | 52 +++ .../Middleware/AuthenticationMiddleware.php | 100 ++---- module/Rest/src/Service/ApiKeyService.php | 1 + .../SingleStepCreateShortUrlActionTest.php | 18 +- .../AuthorizationHeaderPluginTest.php | 130 ++++++++ .../AuthenticationMiddlewareTest.php | 297 +++++++++--------- 18 files changed, 675 insertions(+), 242 deletions(-) create mode 100644 module/Rest/src/Authentication/AuthenticationPluginManager.php create mode 100644 module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php create mode 100644 module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php create mode 100644 module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php create mode 100644 module/Rest/src/Authentication/Plugin/AuthenticationPluginInterface.php create mode 100644 module/Rest/src/Authentication/Plugin/AuthorizationHeaderPlugin.php create mode 100644 module/Rest/src/Exception/NoAuthenticationException.php create mode 100644 module/Rest/src/Exception/VerifyAuthenticationException.php create mode 100644 module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index c60cc358..cd63447d 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; +use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; + return [ 'auth' => [ @@ -10,6 +12,39 @@ return [ Action\AuthenticateAction::class, Action\ShortUrl\SingleStepCreateShortUrlAction::class, ], + + 'plugins' => [ + 'factories' => [ + Authentication\Plugin\ApiKeyHeaderPlugin::class => ConfigAbstractFactory::class, + Authentication\Plugin\AuthorizationHeaderPlugin::class => ConfigAbstractFactory::class, + ], + 'aliases' => [ + Authentication\Plugin\ApiKeyHeaderPlugin::HEADER_NAME => + Authentication\Plugin\ApiKeyHeaderPlugin::class, + Authentication\Plugin\AuthorizationHeaderPlugin::HEADER_NAME => + Authentication\Plugin\AuthorizationHeaderPlugin::class, + ], + ], + ], + + 'dependencies' => [ + 'factories' => [ + Authentication\AuthenticationPluginManager::class => + Authentication\AuthenticationPluginManagerFactory::class, + + Middleware\AuthenticationMiddleware::class => ConfigAbstractFactory::class, + ], + ], + + ConfigAbstractFactory::class => [ + Authentication\Plugin\AuthorizationHeaderPlugin::class => [Authentication\JWTService::class, 'translator'], + + Middleware\AuthenticationMiddleware::class => [ + Authentication\AuthenticationPluginManager::class, + 'translator', + 'config.auth.routes_whitelist', + 'Logger_Shlink', + ], ], ]; diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index a8cca560..69f698bc 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -35,7 +35,6 @@ return [ Middleware\BodyParserMiddleware::class => InvokableFactory::class, Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\PathVersionMiddleware::class => InvokableFactory::class, - Middleware\AuthenticationMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class, Middleware\ShortUrl\ShortCodePathMiddleware::class => InvokableFactory::class, ], @@ -91,13 +90,6 @@ return [ Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, Translator::class, LoggerInterface::class], - - Middleware\AuthenticationMiddleware::class => [ - Authentication\JWTService::class, - 'translator', - 'config.auth.routes_whitelist', - 'Logger_Shlink', - ], ], ]; diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index 38057f5c..52bee6c1 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -43,9 +43,7 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction { $query = $request->getQueryParams(); - // Check provided API key - $apiKey = $this->apiKeyService->getByKey($query['apiKey'] ?? ''); - if ($apiKey === null || ! $apiKey->isValid()) { + if (! $this->apiKeyService->check($query['apiKey'] ?? '')) { throw new InvalidArgumentException( $this->translator->translate('No API key was provided or it is not valid') ); diff --git a/module/Rest/src/Authentication/AuthenticationPluginManager.php b/module/Rest/src/Authentication/AuthenticationPluginManager.php new file mode 100644 index 00000000..da8121e8 --- /dev/null +++ b/module/Rest/src/Authentication/AuthenticationPluginManager.php @@ -0,0 +1,57 @@ +hasAnySupportedHeader($request)) { + throw NoAuthenticationException::fromExpectedTypes([ + ApiKeyHeaderPlugin::HEADER_NAME, + AuthorizationHeaderPlugin::HEADER_NAME, + ]); + } + + return $this->get($this->getFirstAvailableHeader($request)); + } + + private function hasAnySupportedHeader(ServerRequestInterface $request): bool + { + return array_reduce( + self::SUPPORTED_AUTH_HEADERS, + function (bool $carry, string $header) use ($request) { + return $carry || $request->hasHeader($header); + }, + false + ); + } + + private function getFirstAvailableHeader(ServerRequestInterface $request): string + { + $foundHeaders = array_filter(self::SUPPORTED_AUTH_HEADERS, [$request, 'hasHeader']); + return array_shift($foundHeaders) ?? ''; + } +} diff --git a/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php b/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php new file mode 100644 index 00000000..9e64a75a --- /dev/null +++ b/module/Rest/src/Authentication/AuthenticationPluginManagerFactory.php @@ -0,0 +1,31 @@ +get('config') ?? []; + return new AuthenticationPluginManager($container, $config['auth']['plugins'] ?? []); + } +} diff --git a/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php b/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php new file mode 100644 index 00000000..11ae49ca --- /dev/null +++ b/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php @@ -0,0 +1,17 @@ +encode([ - 'iss' => $this->appOptions->__toString(), + 'iss' => (string) $this->appOptions, 'iat' => $currentTimestamp, 'exp' => $currentTimestamp + $lifetime, 'sub' => 'auth', diff --git a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php new file mode 100644 index 00000000..ccfa7eee --- /dev/null +++ b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php @@ -0,0 +1,26 @@ +jwtService = $jwtService; + $this->translator = $translator; + } + + /** + * @throws VerifyAuthenticationException + */ + public function verify(ServerRequestInterface $request): void + { + // Get token making sure the an authorization type is provided + $authToken = $request->getHeaderLine(self::HEADER_NAME); + $authTokenParts = explode(' ', $authToken); + if (count($authTokenParts) === 1) { + throw VerifyAuthenticationException::withError( + RestUtils::INVALID_AUTHORIZATION_ERROR, + sprintf( + $this->translator->translate('You need to provide the Bearer type in the %s header.'), + self::HEADER_NAME + ) + ); + } + + // Make sure the authorization type is Bearer + [$authType, $jwt] = $authTokenParts; + if (strtolower($authType) !== 'bearer') { + throw VerifyAuthenticationException::withError( + RestUtils::INVALID_AUTHORIZATION_ERROR, + sprintf($this->translator->translate( + 'Provided authorization type %s is not supported. Use Bearer instead.' + ), $authType) + ); + } + + try { + if (! $this->jwtService->verify($jwt)) { + throw $this->createInvalidTokenError(); + } + } catch (Throwable $e) { + throw $this->createInvalidTokenError($e); + } + } + + private function createInvalidTokenError(Throwable $prev = null): VerifyAuthenticationException + { + return VerifyAuthenticationException::withError( + RestUtils::INVALID_AUTH_TOKEN_ERROR, + sprintf($this->translator->translate( + 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' + . 'token on every new request on the %s header' + ), self::HEADER_NAME), + $prev + ); + } + + public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface + { + $authToken = $request->getHeaderLine(self::HEADER_NAME); + [, $jwt] = explode(' ', $authToken); + $jwt = $this->jwtService->refresh($jwt); + + return $response->withHeader(self::HEADER_NAME, sprintf('Bearer %s', $jwt)); + } +} diff --git a/module/Rest/src/Exception/AuthenticationException.php b/module/Rest/src/Exception/AuthenticationException.php index 63805483..9bb29123 100644 --- a/module/Rest/src/Exception/AuthenticationException.php +++ b/module/Rest/src/Exception/AuthenticationException.php @@ -5,12 +5,7 @@ namespace Shlinkio\Shlink\Rest\Exception; class AuthenticationException extends RuntimeException { - public static function fromCredentials($username, $password) - { - return new self(sprintf('Invalid credentials. Username -> "%s". Password -> "%s"', $username, $password)); - } - - public static function expiredJWT(\Exception $prev = null) + public static function expiredJWT(\Exception $prev = null): self { return new self('The token has expired.', -1, $prev); } diff --git a/module/Rest/src/Exception/NoAuthenticationException.php b/module/Rest/src/Exception/NoAuthenticationException.php new file mode 100644 index 00000000..4c750674 --- /dev/null +++ b/module/Rest/src/Exception/NoAuthenticationException.php @@ -0,0 +1,18 @@ +errorCode = $errorCode; + $this->publicMessage = $publicMessage; + } + + public static function withError(string $errorCode, string $publicMessage, Throwable $prev = null): self + { + return new self( + $errorCode, + $publicMessage, + sprintf('Authentication verification failed with the public message "%s"', $publicMessage), + 0, + $prev + ); + } + + public function getErrorCode(): string + { + return $this->errorCode; + } + + public function getPublicMessage(): string + { + return $this->publicMessage; + } +} diff --git a/module/Rest/src/Middleware/AuthenticationMiddleware.php b/module/Rest/src/Middleware/AuthenticationMiddleware.php index 124b4f03..8610db1d 100644 --- a/module/Rest/src/Middleware/AuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/AuthenticationMiddleware.php @@ -5,19 +5,24 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Fig\Http\Message\RequestMethodInterface; use Fig\Http\Message\StatusCodeInterface; +use Psr\Container\ContainerExceptionInterface; 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 Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; -use Shlinkio\Shlink\Rest\Exception\AuthenticationException; +use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager; +use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerInterface; +use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\TranslatorInterface; -use Zend\Stdlib\ErrorHandler; +use function implode; +use function in_array; +use function sprintf; class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface { @@ -28,10 +33,6 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa * @var TranslatorInterface */ private $translator; - /** - * @var JWTServiceInterface - */ - private $jwtService; /** * @var LoggerInterface */ @@ -40,17 +41,21 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa * @var array */ private $routesWhitelist; + /** + * @var AuthenticationPluginManagerInterface + */ + private $authPluginManager; public function __construct( - JWTServiceInterface $jwtService, + AuthenticationPluginManagerInterface $authPluginManager, TranslatorInterface $translator, array $routesWhitelist, LoggerInterface $logger = null ) { $this->translator = $translator; - $this->jwtService = $jwtService; $this->routesWhitelist = $routesWhitelist; $this->logger = $logger ?: new NullLogger(); + $this->authPluginManager = $authPluginManager; } /** @@ -62,7 +67,6 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa * * @return Response * @throws \InvalidArgumentException - * @throws \ErrorException */ public function process(Request $request, RequestHandlerInterface $handler): Response { @@ -71,75 +75,37 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa if ($routeResult === null || $routeResult->isFailure() || $request->getMethod() === self::METHOD_OPTIONS - || \in_array($routeResult->getMatchedRouteName(), $this->routesWhitelist, true) + || in_array($routeResult->getMatchedRouteName(), $this->routesWhitelist, true) ) { return $handler->handle($request); } - // Check that the auth header was provided, and that it belongs to a non-expired token - if (! $request->hasHeader(self::AUTHORIZATION_HEADER)) { - return $this->createTokenErrorResponse(); - } - - // Get token making sure the an authorization type is provided - $authToken = $request->getHeaderLine(self::AUTHORIZATION_HEADER); - $authTokenParts = \explode(' ', $authToken); - if (\count($authTokenParts) === 1) { - return new JsonResponse([ - 'error' => RestUtils::INVALID_AUTHORIZATION_ERROR, - 'message' => \sprintf($this->translator->translate( - 'You need to provide the Bearer type in the %s header.' - ), self::AUTHORIZATION_HEADER), - ], self::STATUS_UNAUTHORIZED); - } - - // Make sure the authorization type is Bearer - [$authType, $jwt] = $authTokenParts; - if (\strtolower($authType) !== 'bearer') { - return new JsonResponse([ - 'error' => RestUtils::INVALID_AUTHORIZATION_ERROR, - 'message' => \sprintf($this->translator->translate( - 'Provided authorization type %s is not supported. Use Bearer instead.' - ), $authType), - ], self::STATUS_UNAUTHORIZED); + try { + $plugin = $this->authPluginManager->fromRequest($request); + } catch (ContainerExceptionInterface | NoAuthenticationException $e) { + $this->logger->warning('Invalid or no authentication provided.' . PHP_EOL . $e); + return $this->createErrorResponse(sprintf($this->translator->translate( + 'Expected one of the following authentication headers, but none were provided, ["%s"]' + ), implode('", "', AuthenticationPluginManager::SUPPORTED_AUTH_HEADERS))); } try { - ErrorHandler::start(); - if (! $this->jwtService->verify($jwt)) { - return $this->createTokenErrorResponse(); - } - ErrorHandler::stop(true); - - // Update the token expiration and continue to next middleware - $jwt = $this->jwtService->refresh($jwt); + $plugin->verify($request); $response = $handler->handle($request); - - // Return the response with the updated token on it - return $response->withHeader(self::AUTHORIZATION_HEADER, 'Bearer ' . $jwt); - } catch (AuthenticationException $e) { - $this->logger->warning('Tried to access API with an invalid JWT.' . PHP_EOL . $e); - return $this->createTokenErrorResponse(); - } finally { - ErrorHandler::clean(); + return $plugin->update($request, $response); + } catch (VerifyAuthenticationException $e) { + $this->logger->warning('Authentication verification failed.' . PHP_EOL . $e); + return $this->createErrorResponse($e->getPublicMessage(), $e->getErrorCode()); } } - /** - * @return JsonResponse - * @throws \InvalidArgumentException - */ - private function createTokenErrorResponse(): JsonResponse - { + private function createErrorResponse( + string $message, + string $errorCode = RestUtils::INVALID_AUTHORIZATION_ERROR + ): JsonResponse { return new JsonResponse([ - 'error' => RestUtils::INVALID_AUTH_TOKEN_ERROR, - 'message' => \sprintf( - $this->translator->translate( - 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' - . 'token on every new request on the "%s" header' - ), - self::AUTHORIZATION_HEADER - ), + 'error' => $errorCode, + 'message' => $message, ], self::STATUS_UNAUTHORIZED); } } diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 815bbed3..ca8d80ec 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Rest\Service; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function sprintf; class ApiKeyService implements ApiKeyServiceInterface { diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index 438132cb..b145f9c7 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -11,7 +11,6 @@ use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; -use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequestFactory; @@ -50,12 +49,11 @@ class SingleStepCreateShortUrlActionTest extends TestCase /** * @test - * @dataProvider provideInvalidApiKeys */ - public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(?ApiKey $apiKey) + public function errorResponseIsReturnedIfInvalidApiKeyIsProvided() { $request = ServerRequestFactory::fromGlobals()->withQueryParams(['apiKey' => 'abc123']); - $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn($apiKey); + $findApiKey = $this->apiKeyService->check('abc123')->willReturn(false); /** @var JsonResponse $resp */ $resp = $this->action->handle($request); @@ -67,21 +65,13 @@ class SingleStepCreateShortUrlActionTest extends TestCase $findApiKey->shouldHaveBeenCalled(); } - public function provideInvalidApiKeys(): array - { - return [ - [null], - [(new ApiKey())->disable()], - ]; - } - /** * @test */ public function errorResponseIsReturnedIfNoUrlIsProvided() { $request = ServerRequestFactory::fromGlobals()->withQueryParams(['apiKey' => 'abc123']); - $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn(new ApiKey()); + $findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); /** @var JsonResponse $resp */ $resp = $this->action->handle($request); @@ -102,7 +92,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase 'apiKey' => 'abc123', 'longUrl' => 'http://foobar.com', ]); - $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn(new ApiKey()); + $findApiKey = $this->apiKeyService->check('abc123')->willReturn(true); $generateShortCode = $this->urlShortener->urlToShortCode( Argument::that(function (UriInterface $argument) { Assert::assertEquals('http://foobar.com', (string) $argument); diff --git a/module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php b/module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php new file mode 100644 index 00000000..2b85f989 --- /dev/null +++ b/module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php @@ -0,0 +1,130 @@ +jwtService = $this->prophesize(JWTServiceInterface::class); + $this->plugin = new AuthorizationHeaderPlugin($this->jwtService->reveal(), Translator::factory([])); + } + + /** + * @test + */ + public function verifyAnAuthorizationWithoutBearerTypeThrowsException() + { + $authToken = 'ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withHeader( + AuthorizationHeaderPlugin::HEADER_NAME, + $authToken + ); + + $this->expectException(VerifyAuthenticationException::class); + $this->expectExceptionMessage(sprintf( + 'You need to provide the Bearer type in the %s header.', + AuthorizationHeaderPlugin::HEADER_NAME + )); + + $this->plugin->verify($request); + } + + /** + * @test + */ + public function verifyAnAuthorizationWithWrongTypeThrowsException() + { + $authToken = 'Basic ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withHeader( + AuthorizationHeaderPlugin::HEADER_NAME, + $authToken + ); + + $this->expectException(VerifyAuthenticationException::class); + $this->expectExceptionMessage( + 'Provided authorization type Basic is not supported. Use Bearer instead.' + ); + + $this->plugin->verify($request); + } + + /** + * @test + */ + public function verifyAnExpiredTokenThrowsException() + { + $authToken = 'Bearer ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withHeader( + AuthorizationHeaderPlugin::HEADER_NAME, + $authToken + ); + $jwtVerify = $this->jwtService->verify('ABC-abc')->willReturn(false); + + $this->expectException(VerifyAuthenticationException::class); + $this->expectExceptionMessage(sprintf( + 'Missing or invalid auth token provided. Perform a new authentication request and send provided ' + . 'token on every new request on the %s header', + AuthorizationHeaderPlugin::HEADER_NAME + )); + + $this->plugin->verify($request); + + $jwtVerify->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function verifyValidTokenDoesNotThrowException() + { + $authToken = 'Bearer ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withHeader( + AuthorizationHeaderPlugin::HEADER_NAME, + $authToken + ); + $jwtVerify = $this->jwtService->verify('ABC-abc')->willReturn(true); + + $this->plugin->verify($request); + + $jwtVerify->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function updateReturnsAnUpdatedResponseWithNewJwt() + { + $authToken = 'Bearer ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withHeader( + AuthorizationHeaderPlugin::HEADER_NAME, + $authToken + ); + $jwtRefresh = $this->jwtService->refresh('ABC-abc')->willReturn('DEF-def'); + + $response = $this->plugin->update($request, new Response()); + + $this->assertTrue($response->hasHeader(AuthorizationHeaderPlugin::HEADER_NAME)); + $this->assertEquals('Bearer DEF-def', $response->getHeaderLine(AuthorizationHeaderPlugin::HEADER_NAME)); + $jwtRefresh->shouldHaveBeenCalledTimes(1); + } +} diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index ee64eaa6..acdbf9d9 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -3,19 +3,31 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Middleware; +use Exception; +use Fig\Http\Message\RequestMethodInterface; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\MethodProphecy; +use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Container\ContainerExceptionInterface; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; -use Shlinkio\Shlink\Rest\Authentication\JWTService; +use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager; +use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerInterface; +use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; +use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; -use ShlinkioTest\Shlink\Common\Util\TestUtils; +use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Router\Route; use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\Translator; +use function implode; +use function sprintf; use function Zend\Stratigility\middleware; class AuthenticationMiddlewareTest extends TestCase @@ -27,7 +39,7 @@ class AuthenticationMiddlewareTest extends TestCase /** * @var ObjectProphecy */ - protected $jwtService; + protected $pluginManager; /** * @var callable @@ -36,148 +48,147 @@ class AuthenticationMiddlewareTest extends TestCase public function setUp() { - $this->jwtService = $this->prophesize(JWTService::class); - $this->middleware = new AuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([]), [ + $this->pluginManager = $this->prophesize(AuthenticationPluginManagerInterface::class); + $this->middleware = new AuthenticationMiddleware($this->pluginManager->reveal(), Translator::factory([]), [ AuthenticateAction::class, ]); - $this->dummyMiddleware = middleware(function () { + } + + /** + * @test + * @dataProvider provideWhitelistedRequests + */ + public function someWhiteListedSituationsFallbackToNextMiddleware(ServerRequestInterface $request) + { + $handler = $this->prophesize(RequestHandlerInterface::class); + $handle = $handler->handle($request)->willReturn(new Response()); + $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn( + $this->prophesize(AuthenticationPluginInterface::class)->reveal() + ); + + $this->middleware->process($request, $handler->reveal()); + + $handle->shouldHaveBeenCalledTimes(1); + $fromRequest->shouldNotHaveBeenCalled(); + } + + public function provideWhitelistedRequests(): array + { + $dummyMiddleware = $this->getDummyMiddleware(); + + return [ + 'with no route result' => [ServerRequestFactory::fromGlobals()], + 'with failure route result' => [ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRouteFailure([RequestMethodInterface::METHOD_GET]) + )], + 'with whitelisted route' => [ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRoute( + new Route('foo', $dummyMiddleware, Route::HTTP_METHOD_ANY, AuthenticateAction::class) + ) + )], + 'with OPTIONS method' => [ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('bar', $dummyMiddleware), []) + )->withMethod(RequestMethodInterface::METHOD_OPTIONS)], + ]; + } + + /** + * @test + * @dataProvider provideExceptions + */ + public function errorIsReturnedWhenNoValidAuthIsProvided($e) + { + $authToken = 'ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willThrow($e); + + /** @var Response\JsonResponse $response */ + $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + $payload = $response->getPayload(); + + $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $payload['error']); + $this->assertEquals(sprintf( + 'Expected one of the following authentication headers, but none were provided, ["%s"]', + implode('", "', AuthenticationPluginManager::SUPPORTED_AUTH_HEADERS) + ), $payload['message']); + $fromRequest->shouldHaveBeenCalledTimes(1); + } + + public function provideExceptions(): array + { + return [ + [new class extends Exception implements ContainerExceptionInterface { + }], + [NoAuthenticationException::fromExpectedTypes([])], + ]; + } + + /** + * @test + */ + public function errorIsReturnedWhenVerificationFails() + { + $authToken = 'ABC-abc'; + $request = ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + $plugin = $this->prophesize(AuthenticationPluginInterface::class); + + $verify = $plugin->verify($request)->willThrow( + VerifyAuthenticationException::withError('the_error', 'the_message') + ); + $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn($plugin->reveal()); + + /** @var Response\JsonResponse $response */ + $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + $payload = $response->getPayload(); + + $this->assertEquals('the_error', $payload['error']); + $this->assertEquals('the_message', $payload['message']); + $verify->shouldHaveBeenCalledTimes(1); + $fromRequest->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function updatedResponseIsReturnedWhenVerificationPasses() + { + $authToken = 'ABC-abc'; + $newResponse = new Response(); + $request = ServerRequestFactory::fromGlobals()->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) + )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + $plugin = $this->prophesize(AuthenticationPluginInterface::class); + + $verify = $plugin->verify($request)->will(function () { + }); + $update = $plugin->update($request, Argument::type(ResponseInterface::class))->willReturn($newResponse); + $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn($plugin->reveal()); + + $handler = $this->prophesize(RequestHandlerInterface::class); + $handle = $handler->handle($request)->willReturn(new Response()); + $response = $this->middleware->process($request, $handler->reveal()); + + $this->assertSame($response, $newResponse); + $verify->shouldHaveBeenCalledTimes(1); + $update->shouldHaveBeenCalledTimes(1); + $handle->shouldHaveBeenCalledTimes(1); + $fromRequest->shouldHaveBeenCalledTimes(1); + } + + private function getDummyMiddleware(): MiddlewareInterface + { + return middleware(function () { return new Response\EmptyResponse(); }); } - - /** - * @test - */ - public function someWhiteListedSituationsFallbackToNextMiddleware() - { - $request = ServerRequestFactory::fromGlobals(); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - - $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); - - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRouteFailure(['GET']) - ); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); - - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route( - 'foo', - $this->dummyMiddleware, - Route::HTTP_METHOD_ANY, - AuthenticateAction::class - )) - ); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); - - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withMethod('OPTIONS'); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); - } - - /** - * @test - */ - public function noHeaderReturnsError() - { - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - ); - $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); - $this->assertEquals(401, $response->getStatusCode()); - } - - /** - * @test - */ - public function provideAnAuthorizationWithoutTypeReturnsError() - { - $authToken = 'ABC-abc'; - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); - - $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); - - $this->assertEquals(401, $response->getStatusCode()); - $this->assertTrue(strpos($response->getBody()->getContents(), 'You need to provide the Bearer type') > 0); - } - - /** - * @test - */ - public function provideAnAuthorizationWithWrongTypeReturnsError() - { - $authToken = 'ABC-abc'; - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); - - $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); - - $this->assertEquals(401, $response->getStatusCode()); - $this->assertTrue( - strpos($response->getBody()->getContents(), 'Provided authorization type Basic is not supported') > 0 - ); - } - - /** - * @test - */ - public function provideAnExpiredTokenReturnsError() - { - $authToken = 'ABC-abc'; - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); - $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); - - $response = $this->middleware->process($request, TestUtils::createReqHandlerMock()->reveal()); - $this->assertEquals(401, $response->getStatusCode()); - } - - /** - * @test - */ - public function provideCorrectTokenUpdatesExpirationAndFallsBackToNextMiddleware() - { - $authToken = 'ABC-abc'; - $request = ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRoute(new Route('bar', $this->dummyMiddleware), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, 'bearer ' . $authToken); - $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); - $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); - - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle($request)->willReturn(new Response()); - $resp = $this->middleware->process($request, $delegate->reveal()); - - $process->shouldHaveBeenCalledTimes(1); - $this->assertArrayHasKey(AuthenticationMiddleware::AUTHORIZATION_HEADER, $resp->getHeaders()); - } } From 0f5fb066d19f7b15ea447a1588362d5543c31edc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 29 Sep 2018 08:16:40 +0200 Subject: [PATCH 4/5] Converted AuthenticationpluginManager in a plain plugin manager and encasulated in new service adding extra behavior --- module/Rest/config/auth.config.php | 5 +- .../AuthenticationPluginManager.php | 48 +---------- .../AuthenticationPluginManagerInterface.php | 11 +-- .../RequestToHttpAuthPlugin.php | 64 ++++++++++++++ .../RequestToHttpAuthPluginInterface.php | 17 ++++ .../Middleware/AuthenticationMiddleware.php | 19 ++--- ...AuthenticationPluginManagerFactoryTest.php | 33 ++++++++ .../AuthorizationHeaderPluginTest.php | 2 +- .../RequestToAuthPluginTest.php | 83 +++++++++++++++++++ .../AuthenticationMiddlewareTest.php | 29 +++---- 10 files changed, 226 insertions(+), 85 deletions(-) create mode 100644 module/Rest/src/Authentication/RequestToHttpAuthPlugin.php create mode 100644 module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php create mode 100644 module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php rename module/Rest/test/Authentication/{ => Plugin}/AuthorizationHeaderPluginTest.php (98%) create mode 100644 module/Rest/test/Authentication/RequestToAuthPluginTest.php diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index cd63447d..94e853be 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -31,6 +31,7 @@ return [ 'factories' => [ Authentication\AuthenticationPluginManager::class => Authentication\AuthenticationPluginManagerFactory::class, + Authentication\RequestToHttpAuthPlugin::class => ConfigAbstractFactory::class, Middleware\AuthenticationMiddleware::class => ConfigAbstractFactory::class, ], @@ -39,8 +40,10 @@ return [ ConfigAbstractFactory::class => [ Authentication\Plugin\AuthorizationHeaderPlugin::class => [Authentication\JWTService::class, 'translator'], + Authentication\RequestToHttpAuthPlugin::class => [Authentication\AuthenticationPluginManager::class], + Middleware\AuthenticationMiddleware::class => [ - Authentication\AuthenticationPluginManager::class, + Authentication\RequestToHttpAuthPlugin::class, 'translator', 'config.auth.routes_whitelist', 'Logger_Shlink', diff --git a/module/Rest/src/Authentication/AuthenticationPluginManager.php b/module/Rest/src/Authentication/AuthenticationPluginManager.php index da8121e8..1837dcc6 100644 --- a/module/Rest/src/Authentication/AuthenticationPluginManager.php +++ b/module/Rest/src/Authentication/AuthenticationPluginManager.php @@ -3,55 +3,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Authentication; -use Psr\Container\ContainerExceptionInterface; -use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin; -use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthorizationHeaderPlugin; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; use Zend\ServiceManager\AbstractPluginManager; -use function array_filter; -use function array_reduce; -use function array_shift; class AuthenticationPluginManager extends AbstractPluginManager implements AuthenticationPluginManagerInterface { - // Headers here have to be defined in order of priority. - // When more than one is matched, the first one will take precedence - public const SUPPORTED_AUTH_HEADERS = [ - ApiKeyHeaderPlugin::HEADER_NAME, - AuthorizationHeaderPlugin::HEADER_NAME, - ]; - - /** - * @throws ContainerExceptionInterface - * @throws NoAuthenticationException - */ - public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface - { - if (! $this->hasAnySupportedHeader($request)) { - throw NoAuthenticationException::fromExpectedTypes([ - ApiKeyHeaderPlugin::HEADER_NAME, - AuthorizationHeaderPlugin::HEADER_NAME, - ]); - } - - return $this->get($this->getFirstAvailableHeader($request)); - } - - private function hasAnySupportedHeader(ServerRequestInterface $request): bool - { - return array_reduce( - self::SUPPORTED_AUTH_HEADERS, - function (bool $carry, string $header) use ($request) { - return $carry || $request->hasHeader($header); - }, - false - ); - } - - private function getFirstAvailableHeader(ServerRequestInterface $request): string - { - $foundHeaders = array_filter(self::SUPPORTED_AUTH_HEADERS, [$request, 'hasHeader']); - return array_shift($foundHeaders) ?? ''; - } + protected $instanceOf = Plugin\AuthenticationPluginInterface::class; } diff --git a/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php b/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php index 11ae49ca..ae8c1abf 100644 --- a/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php +++ b/module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php @@ -3,15 +3,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Authentication; -use Psr\Container; -use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; +use Psr\Container\ContainerInterface; -interface AuthenticationPluginManagerInterface extends Container\ContainerInterface +interface AuthenticationPluginManagerInterface extends ContainerInterface { - /** - * @throws Container\ContainerExceptionInterface - * @throws NoAuthenticationException - */ - public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface; } diff --git a/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php new file mode 100644 index 00000000..3be4b60a --- /dev/null +++ b/module/Rest/src/Authentication/RequestToHttpAuthPlugin.php @@ -0,0 +1,64 @@ +authPluginManager = $authPluginManager; + } + + /** + * @throws Container\ContainerExceptionInterface + * @throws NoAuthenticationException + */ + public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface + { + if (! $this->hasAnySupportedHeader($request)) { + throw NoAuthenticationException::fromExpectedTypes([ + Plugin\ApiKeyHeaderPlugin::HEADER_NAME, + Plugin\AuthorizationHeaderPlugin::HEADER_NAME, + ]); + } + + return $this->authPluginManager->get($this->getFirstAvailableHeader($request)); + } + + private function hasAnySupportedHeader(ServerRequestInterface $request): bool + { + return array_reduce( + self::SUPPORTED_AUTH_HEADERS, + function (bool $carry, string $header) use ($request) { + return $carry || $request->hasHeader($header); + }, + false + ); + } + + private function getFirstAvailableHeader(ServerRequestInterface $request): string + { + $foundHeaders = array_filter(self::SUPPORTED_AUTH_HEADERS, [$request, 'hasHeader']); + return array_shift($foundHeaders) ?? ''; + } +} diff --git a/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php b/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php new file mode 100644 index 00000000..a05f3274 --- /dev/null +++ b/module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php @@ -0,0 +1,17 @@ +translator = $translator; $this->routesWhitelist = $routesWhitelist; $this->logger = $logger ?: new NullLogger(); - $this->authPluginManager = $authPluginManager; + $this->requestToAuthPlugin = $requestToAuthPlugin; } /** @@ -81,12 +78,12 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa } try { - $plugin = $this->authPluginManager->fromRequest($request); + $plugin = $this->requestToAuthPlugin->fromRequest($request); } catch (ContainerExceptionInterface | NoAuthenticationException $e) { $this->logger->warning('Invalid or no authentication provided.' . PHP_EOL . $e); return $this->createErrorResponse(sprintf($this->translator->translate( 'Expected one of the following authentication headers, but none were provided, ["%s"]' - ), implode('", "', AuthenticationPluginManager::SUPPORTED_AUTH_HEADERS))); + ), implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS))); } try { diff --git a/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php new file mode 100644 index 00000000..0a1b3993 --- /dev/null +++ b/module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php @@ -0,0 +1,33 @@ +factory = new AuthenticationPluginManagerFactory(); + } + + /** + * @test + */ + public function serviceIsProperlyCreated() + { + $instance = $this->factory->__invoke(new ServiceManager(['services' => [ + 'config' => [], + ]]), ''); + $this->assertInstanceOf(AuthenticationPluginManager::class, $instance); + } +} diff --git a/module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php similarity index 98% rename from module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php rename to module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php index 2b85f989..31590c53 100644 --- a/module/Rest/test/Authentication/AuthorizationHeaderPluginTest.php +++ b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php @@ -1,7 +1,7 @@ pluginManager = $this->prophesize(AuthenticationPluginManagerInterface::class); + $this->requestToPlugin = new RequestToHttpAuthPlugin($this->pluginManager->reveal()); + } + + /** + * @test + */ + public function exceptionIsFoundWhenNoneOfTheSupportedMethodsIsFound() + { + $request = ServerRequestFactory::fromGlobals(); + + $this->expectException(NoAuthenticationException::class); + $this->expectExceptionMessage(sprintf( + 'None of the valid authentication mechanisms where provided. Expected one of ["%s"]', + implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) + )); + + $this->requestToPlugin->fromRequest($request); + } + + /** + * @test + * @dataProvider provideHeaders + */ + public function properPluginIsFetchedWhenAnyAuthTypeIsFound(array $headers, string $expectedHeader) + { + $request = ServerRequestFactory::fromGlobals(); + foreach ($headers as $header => $value) { + $request = $request->withHeader($header, $value); + } + + $plugin = $this->prophesize(AuthenticationPluginInterface::class); + $getPlugin = $this->pluginManager->get($expectedHeader)->willReturn($plugin->reveal()); + + $this->requestToPlugin->fromRequest($request); + + $getPlugin->shouldHaveBeenCalledTimes(1); + } + + public function provideHeaders(): array + { + return [ + 'API key header only' => [[ + ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', + ], ApiKeyHeaderPlugin::HEADER_NAME], + 'Authorization header only' => [[ + AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', + ], AuthorizationHeaderPlugin::HEADER_NAME], + 'Both headers' => [[ + AuthorizationHeaderPlugin::HEADER_NAME => 'foobar', + ApiKeyHeaderPlugin::HEADER_NAME => 'foobar', + ], ApiKeyHeaderPlugin::HEADER_NAME], + ]; + } +} diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index acdbf9d9..7886133a 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -14,9 +14,9 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Action\AuthenticateAction; -use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager; -use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerInterface; use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface; +use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin; +use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface; use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -39,7 +39,7 @@ class AuthenticationMiddlewareTest extends TestCase /** * @var ObjectProphecy */ - protected $pluginManager; + protected $requestToPlugin; /** * @var callable @@ -48,8 +48,8 @@ class AuthenticationMiddlewareTest extends TestCase public function setUp() { - $this->pluginManager = $this->prophesize(AuthenticationPluginManagerInterface::class); - $this->middleware = new AuthenticationMiddleware($this->pluginManager->reveal(), Translator::factory([]), [ + $this->requestToPlugin = $this->prophesize(RequestToHttpAuthPluginInterface::class); + $this->middleware = new AuthenticationMiddleware($this->requestToPlugin->reveal(), Translator::factory([]), [ AuthenticateAction::class, ]); } @@ -62,7 +62,7 @@ class AuthenticationMiddlewareTest extends TestCase { $handler = $this->prophesize(RequestHandlerInterface::class); $handle = $handler->handle($request)->willReturn(new Response()); - $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn( + $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn( $this->prophesize(AuthenticationPluginInterface::class)->reveal() ); @@ -101,12 +101,11 @@ class AuthenticationMiddlewareTest extends TestCase */ public function errorIsReturnedWhenNoValidAuthIsProvided($e) { - $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); - $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willThrow($e); + ); + $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willThrow($e); /** @var Response\JsonResponse $response */ $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); @@ -115,7 +114,7 @@ class AuthenticationMiddlewareTest extends TestCase $this->assertEquals(RestUtils::INVALID_AUTHORIZATION_ERROR, $payload['error']); $this->assertEquals(sprintf( 'Expected one of the following authentication headers, but none were provided, ["%s"]', - implode('", "', AuthenticationPluginManager::SUPPORTED_AUTH_HEADERS) + implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) ), $payload['message']); $fromRequest->shouldHaveBeenCalledTimes(1); } @@ -134,17 +133,16 @@ class AuthenticationMiddlewareTest extends TestCase */ public function errorIsReturnedWhenVerificationFails() { - $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + ); $plugin = $this->prophesize(AuthenticationPluginInterface::class); $verify = $plugin->verify($request)->willThrow( VerifyAuthenticationException::withError('the_error', 'the_message') ); - $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn($plugin->reveal()); + $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal()); /** @var Response\JsonResponse $response */ $response = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); @@ -161,18 +159,17 @@ class AuthenticationMiddlewareTest extends TestCase */ public function updatedResponseIsReturnedWhenVerificationPasses() { - $authToken = 'ABC-abc'; $newResponse = new Response(); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []) - )->withHeader(AuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); + ); $plugin = $this->prophesize(AuthenticationPluginInterface::class); $verify = $plugin->verify($request)->will(function () { }); $update = $plugin->update($request, Argument::type(ResponseInterface::class))->willReturn($newResponse); - $fromRequest = $this->pluginManager->fromRequest(Argument::any())->willReturn($plugin->reveal()); + $fromRequest = $this->requestToPlugin->fromRequest(Argument::any())->willReturn($plugin->reveal()); $handler = $this->prophesize(RequestHandlerInterface::class); $handle = $handler->handle($request)->willReturn(new Response()); From 5ecfe9f0f09ba3383951abfd55081ec5fa034c80 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 29 Sep 2018 08:34:47 +0200 Subject: [PATCH 5/5] Implemented ApiKeyHeaderPlugin --- module/Rest/config/auth.config.php | 1 + .../Plugin/ApiKeyHeaderPlugin.php | 28 ++++++- .../Plugin/ApiKeyHeaderPluginTest.php | 78 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index 94e853be..8786e202 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -39,6 +39,7 @@ return [ ConfigAbstractFactory::class => [ Authentication\Plugin\AuthorizationHeaderPlugin::class => [Authentication\JWTService::class, 'translator'], + Authentication\Plugin\ApiKeyHeaderPlugin::class => [Service\ApiKeyService::class, 'translator'], Authentication\RequestToHttpAuthPlugin::class => [Authentication\AuthenticationPluginManager::class], diff --git a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php index ccfa7eee..fcf41b96 100644 --- a/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php +++ b/module/Rest/src/Authentication/Plugin/ApiKeyHeaderPlugin.php @@ -6,17 +6,43 @@ namespace Shlinkio\Shlink\Rest\Authentication\Plugin; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; +use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; +use Shlinkio\Shlink\Rest\Util\RestUtils; +use Zend\I18n\Translator\TranslatorInterface; class ApiKeyHeaderPlugin implements AuthenticationPluginInterface { public const HEADER_NAME = 'X-Api-Key'; + /** + * @var ApiKeyServiceInterface + */ + private $apiKeyService; + /** + * @var TranslatorInterface + */ + private $translator; + + public function __construct(ApiKeyServiceInterface $apiKeyService, TranslatorInterface $translator) + { + $this->apiKeyService = $apiKeyService; + $this->translator = $translator; + } + /** * @throws VerifyAuthenticationException */ public function verify(ServerRequestInterface $request): void { - // TODO: Implement check() method. + $apiKey = $request->getHeaderLine(self::HEADER_NAME); + if ($this->apiKeyService->check($apiKey)) { + return; + } + + throw VerifyAuthenticationException::withError( + RestUtils::INVALID_API_KEY_ERROR, + $this->translator->translate('Provided API key does not exist or is invalid.') + ); } public function update(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface diff --git a/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php new file mode 100644 index 00000000..fef86e71 --- /dev/null +++ b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php @@ -0,0 +1,78 @@ +apiKeyService = $this->prophesize(ApiKeyServiceInterface::class); + $this->plugin = new ApiKeyHeaderPlugin($this->apiKeyService->reveal(), Translator::factory([])); + } + + /** + * @test + */ + public function verifyThrowsExceptionWhenApiKeyIsNotValid() + { + $apiKey = 'abc-ABC'; + $check = $this->apiKeyService->check($apiKey)->willReturn(false); + $check->shouldBeCalledTimes(1); + + $this->expectException(VerifyAuthenticationException::class); + $this->expectExceptionMessage('Provided API key does not exist or is invalid'); + + $this->plugin->verify($this->createRequest($apiKey)); + } + + /** + * @test + */ + public function verifyDoesNotThrowExceptionWhenApiKeyIsValid() + { + $apiKey = 'abc-ABC'; + $check = $this->apiKeyService->check($apiKey)->willReturn(true); + + $this->plugin->verify($this->createRequest($apiKey)); + + $check->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function updateReturnsResponseAsIs() + { + $apiKey = 'abc-ABC'; + $response = new Response(); + + $returnedResponse = $this->plugin->update($this->createRequest($apiKey), $response); + + $this->assertSame($response, $returnedResponse); + } + + private function createRequest(string $apiKey): ServerRequestInterface + { + return ServerRequestFactory::fromGlobals()->withHeader(ApiKeyHeaderPlugin::HEADER_NAME, $apiKey); + } +}