From 8e61639598c7e10f01bc2fb2eb7cb54fbd0071f3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 28 Sep 2018 22:08:01 +0200 Subject: [PATCH] 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()); - } }