Converted AuthenticationpluginManager in a plain plugin manager and encasulated in new service adding extra behavior

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',

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.
+    protected $instanceOf = Plugin\AuthenticationPluginInterface::class;
-    // 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) ?? '';
-    }
 }

module/Rest/src/Authentication/AuthenticationPluginManagerInterface.php

@@ -3,15 +3,8 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Rest\Authentication;
 
-use Psr\Container;
+use Psr\Container\ContainerInterface;
-use Psr\Http\Message\ServerRequestInterface;
-use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException;
 
-interface AuthenticationPluginManagerInterface extends Container\ContainerInterface
+interface AuthenticationPluginManagerInterface extends ContainerInterface
 {
-    /**
-     * @throws Container\ContainerExceptionInterface
-     * @throws NoAuthenticationException
-     */
-    public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface;
 }

module/Rest/src/Authentication/RequestToHttpAuthPlugin.php

@@ -0,0 +1,64 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Rest\Authentication;
+
+use Psr\Container;
+use Psr\Http\Message\ServerRequestInterface;
+use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException;
+use function array_filter;
+use function array_reduce;
+use function array_shift;
+
+class RequestToHttpAuthPlugin implements RequestToHttpAuthPluginInterface
+{
+    // Headers here have to be defined in order of priority.
+    // When more than one is matched, the first one to be found will take precedence.
+    public const SUPPORTED_AUTH_HEADERS = [
+        Plugin\ApiKeyHeaderPlugin::HEADER_NAME,
+        Plugin\AuthorizationHeaderPlugin::HEADER_NAME,
+    ];
+
+    /**
+     * @var AuthenticationPluginManagerInterface
+     */
+    private $authPluginManager;
+
+    public function __construct(AuthenticationPluginManagerInterface $authPluginManager)
+    {
+        $this->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) ?? '';
+    }
+}

module/Rest/src/Authentication/RequestToHttpAuthPluginInterface.php

@@ -0,0 +1,17 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Rest\Authentication;
+
+use Psr\Container;
+use Psr\Http\Message\ServerRequestInterface;
+use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException;
+
+interface RequestToHttpAuthPluginInterface
+{
+    /**
+     * @throws Container\ContainerExceptionInterface
+     * @throws NoAuthenticationException
+     */
+    public function fromRequest(ServerRequestInterface $request): Plugin\AuthenticationPluginInterface;
+}

module/Rest/src/Middleware/AuthenticationMiddleware.php

@@ -12,8 +12,8 @@ use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
-use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager;
+use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin;
-use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerInterface;
+use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPluginInterface;
 use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException;
 use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException;
 use Shlinkio\Shlink\Rest\Util\RestUtils;
@@ -26,9 +26,6 @@ use function sprintf;
 
 class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface, RequestMethodInterface
 {
-    public const AUTHORIZATION_HEADER = 'Authorization';
-    public const API_KEY_HEADER = 'X-Api-Key';
-
     /**
      * @var TranslatorInterface
      */
@@ -42,12 +39,12 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
      */
     private $routesWhitelist;
     /**
-     * @var AuthenticationPluginManagerInterface
+     * @var RequestToHttpAuthPluginInterface
      */
-    private $authPluginManager;
+    private $requestToAuthPlugin;
 
     public function __construct(
-        AuthenticationPluginManagerInterface $authPluginManager,
+        RequestToHttpAuthPluginInterface $requestToAuthPlugin,
         TranslatorInterface $translator,
         array $routesWhitelist,
         LoggerInterface $logger = null
@@ -55,7 +52,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
         $this->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 {

module/Rest/test/Authentication/AuthenticationPluginManagerFactoryTest.php

@@ -0,0 +1,33 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Rest\Authentication;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManager;
+use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerFactory;
+use Zend\ServiceManager\ServiceManager;
+
+class AuthenticationPluginManagerFactoryTest extends TestCase
+{
+    /**
+     * @var AuthenticationPluginManagerFactory
+     */
+    private $factory;
+
+    public function setUp()
+    {
+        $this->factory = new AuthenticationPluginManagerFactory();
+    }
+
+    /**
+     * @test
+     */
+    public function serviceIsProperlyCreated()
+    {
+        $instance = $this->factory->__invoke(new ServiceManager(['services' => [
+            'config' => [],
+        ]]), '');
+        $this->assertInstanceOf(AuthenticationPluginManager::class, $instance);
+    }
+}

module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php

@@ -1,7 +1,7 @@
 <?php
 declare(strict_types=1);
 
-namespace ShlinkioTest\Shlink\Rest\Authentication;
+namespace ShlinkioTest\Shlink\Rest\Authentication\Plugin;
 
 use PHPUnit\Framework\TestCase;
 use Prophecy\Prophecy\ObjectProphecy;

module/Rest/test/Authentication/RequestToAuthPluginTest.php

@@ -0,0 +1,83 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Rest\Authentication;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\Prophecy\ObjectProphecy;
+use Shlinkio\Shlink\Rest\Authentication\AuthenticationPluginManagerInterface;
+use Shlinkio\Shlink\Rest\Authentication\Plugin\ApiKeyHeaderPlugin;
+use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthenticationPluginInterface;
+use Shlinkio\Shlink\Rest\Authentication\Plugin\AuthorizationHeaderPlugin;
+use Shlinkio\Shlink\Rest\Authentication\RequestToHttpAuthPlugin;
+use Shlinkio\Shlink\Rest\Exception\NoAuthenticationException;
+use Zend\Diactoros\ServerRequestFactory;
+
+class RequestToAuthPluginTest extends TestCase
+{
+    /**
+     * @var RequestToHttpAuthPlugin
+     */
+    private $requestToPlugin;
+    /**
+     * @var ObjectProphecy
+     */
+    private $pluginManager;
+
+    public function setUp()
+    {
+        $this->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],
+        ];
+    }
+}

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->requestToPlugin = $this->prophesize(RequestToHttpAuthPluginInterface::class);
-        $this->middleware = new AuthenticationMiddleware($this->pluginManager->reveal(), Translator::factory([]), [
+        $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());