Merge pull request #984 from shlinkio/release/2.5.2

Release/2.5.2

CHANGELOG.md

@@ -4,6 +4,43 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [2.5.2] - 2021-01-24
+### Added
+* [#965](https://github.com/shlinkio/shlink/issues/965) Added docs section for Architectural Decision Records, including the one for API key roles.
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#979](https://github.com/shlinkio/shlink/issues/979) Added missing `itemsPerPage` query param to swagger docs for short RULs list.
+* [#980](https://github.com/shlinkio/shlink/issues/980) Fixed value used for `Access-Control-Allow-Origin`, that could not work as expected when including an IP address.
+* [#947](https://github.com/shlinkio/shlink/issues/947) Fixed incorrect value returned in `Access-Control-Allow-Methods` header, which always contained all methods.
+
+
+## [2.5.1] - 2021-01-21
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#968](https://github.com/shlinkio/shlink/issues/968) Fixed index error in MariaDB while updating to v2.5.0.
+* [#972](https://github.com/shlinkio/shlink/issues/972) Fixed 500 error when calling single-step short URL creation endpoint.
+
+
 ## [2.5.0] - 2021-01-17
 ### Added
 * [#795](https://github.com/shlinkio/shlink/issues/795) and [#882](https://github.com/shlinkio/shlink/issues/882) Added new roles system to API keys.

CONTRIBUTING.md

@@ -46,27 +46,28 @@ This is a simplified version of the project structure:
 ```
 shlink
 ├── bin
-│   ├── cli
-│   ├── install
-│   └── update
+│   ├── cli
+│   ├── install
+│   └── update
 ├── config
-│   ├── autoload
-│   ├── params
-│   ├── config.php
-│   └── container.php
+│   ├── autoload
+│   ├── params
+│   ├── config.php
+│   └── container.php
 ├── data
-│   ├── cache
-│   ├── locks
-│   ├── log
-│   ├── migrations
-│   └── proxies
+│   ├── cache
+│   ├── locks
+│   ├── log
+│   ├── migrations
+│   └── proxies
 ├── docs
-│   ├── async-api
-│   └── swagger
+│   ├── adr
+│   ├── async-api
+│   └── swagger
 ├── module
-│   ├── CLI
-│   ├── Core
-│   └── Rest
+│   ├── CLI
+│   ├── Core
+│   └── Rest
 ├── public
 ├── composer.json
 └── README.md
@@ -77,7 +78,7 @@ The purposes of every folder are:
 * `bin`: It contains the CLI tools. The `cli` one is the main entry point to run shlink from the command line, while `install` and `update` are helper tools used to install and update shlink when not using the docker image.
 * `config`: Contains application-wide configurations, which are later merged with the ones provided by every module.
 * `data`: Common runtime-generated git-ignored assets, like logs, caches, etc.
-* `docs`: Any project documentation is stored here, like API spec definitions.
+* `docs`: Any project documentation is stored here, like API spec definitions or architectural decision records.
 * `module`: Contains a subfolder for every module in the project. Modules contain the source code, tests and configurations for every context in the project.
 * `public`: Few assets (like `favicon.ico` or `robots.txt`) and the web entry point are stored here. This web entry point is not used when serving the app with swoole.
 
@@ -134,3 +135,9 @@ In order to provide pull requests to this project, you should always start by cr
 The base branch should always be `develop`, and the target branch for the pull request should also be `develop`.
 
 Before your branch can be merged, all the checks described in [Running code checks](#running-code-checks) have to be passing. You can verify that manually by running `./indocker composer ci`, or wait for the build to be run automatically after the pull request is created.
+
+## Architectural Decision Records
+
+The project includes logs for some architectural decisions, using the [adr](https://adr.github.io/) proposal.
+
+If you are curious or want to understand why something has been built in some specific way, [take a look at them](docs/adr).

README.md

@@ -34,6 +34,8 @@ The idea is that you can just generate a container using the image and provide t
 First, make sure the host where you are going to run shlink fulfills these requirements:
 
 * PHP 7.4 with JSON, curl, PDO, intl and gd extensions enabled (PHP 8.0 support is coming).
+    * apcu extension is recommended if you don't plan to use swoole.
+    * xml extension is required if you want to generate QR codes in svg format.
 * MySQL, MariaDB, PostgreSQL, Microsoft SQL Server or SQLite.
 * The web server of your choice with PHP integration (Apache or Nginx recommended).
 

data/migrations/Version20210102174433.php

@@ -25,7 +25,7 @@ final class Version20210102174433 extends AbstractMigration
         $table->setPrimaryKey(['id']);
 
         $table->addColumn('role_name', Types::STRING, [
-            'length' => 256,
+            'length' => 255,
             'notnull' => true,
         ]);
         $table->addColumn('meta', Types::JSON, [

data/migrations/Version20210118153932.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20210118153932 extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        // Prev migration used to set the length to 256, which made some set-ups crash
+        // It has been updated to 255, and this migration ensures whoever managed to run the prev one, gets the value
+        // also updated to 255
+
+        $rolesTable = $schema->getTable('api_key_roles');
+        $nameColumn = $rolesTable->getColumn('role_name');
+        $nameColumn->setLength(255);
+    }
+
+    public function down(Schema $schema): void
+    {
+    }
+}

docs/adr/2021-01-17-support-restrictions-and-permissions-in-api-keys.md

@@ -0,0 +1,50 @@
+# Support restrictions and permissions in API keys
+
+* Status: Accepted
+* Date: 2021-01-17
+
+## Context and problem statement
+
+Historically, every API key generated for Shlink granted you access to all existing resources.
+
+The intention is to be able to apply some form of restriction to API keys, so that only a subset of "resources" can be accessed with it, naming:
+
+* Allowing interactions only with short URLs and related resources, that have been created with the same API key.
+* Allowing interactions only with short URLs and related resources, that have been attached to a specific domain.
+
+The intention is to implement a system that allows adding to API keys as many of these restrictions as wanted.
+
+Supporting more restrictions in the future is also desirable.
+
+## Considered option
+
+* Using an ACL/RBAC library, and checking roles in a middleware.
+* Using a service that, provided an API key, tells if certain resource is reachable while it also allows building queries dynamically.
+* Using some library implementing the specification pattern, to dynamically build queries transparently for outer layers.
+
+## Decision outcome
+
+The main difficulty on implementing this is that the entity conditioning the behavior (the API key) comes in the request in some form, but it can potentially affect database queries performed in the persistence layer.
+
+Because of this, it has to traverse all the application layers from top to bottom, in most of the cases.
+
+This motivated selecting the third option, as we can propagate the API key and delay its handling to the last step, without changing the behavior of the rest of the layers that much (except in some individual use cases).
+
+The domain term used to refer these "restrictions" is finally **roles**.
+
+It can be combined in the future with an ACL/RBAC library, if we want to restrict access to certain resources, but it didn't fulfil the initial requirements.
+
+## Pros and Cons of the Options
+
+### An ACL/RBAC library
+
+* Good, because there are many good libraries out there.
+* Bad, because when you need to filter resources lists this kind of libraries doesn't really work.
+
+### A service with the logic
+
+* Bad, because it would need to be used in many layers of the application, mixing unrelated concerns.
+
+### A library implementing the specification pattern
+
+* Good, because allows centralizing the generation of dynamic specs by the entity itself, that are later translated automatically into database queries.

docs/adr/README.md

@@ -0,0 +1,5 @@
+# Architectural Decision Records
+
+Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome.
+
+* [2021-01-17 Support restrictions and permissions in API keys](2021-01-17-support-restrictions-and-permissions-in-api-keys.md)

docs/swagger/paths/v1_short-urls.json

@@ -19,6 +19,15 @@
                     "type": "integer"
                 }
             },
+            {
+                "name": "itemsPerPage",
+                "in": "query",
+                "description": "The amount of items to return on every page. Defaults to 10",
+                "required": false,
+                "schema": {
+                    "type": "number"
+                }
+            },
             {
                 "name": "searchTerm",
                 "in": "query",

module/Rest/config/auth.config.php

@@ -11,9 +11,12 @@ return [
     'auth' => [
         'routes_whitelist' => [
             Action\HealthAction::class,
-            Action\ShortUrl\SingleStepCreateShortUrlAction::class,
             ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME,
         ],
+
+        'routes_with_query_api_key' => [
+            Action\ShortUrl\SingleStepCreateShortUrlAction::class,
+        ],
     ],
 
     'dependencies' => [
@@ -23,7 +26,11 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        Middleware\AuthenticationMiddleware::class => [Service\ApiKeyService::class, 'config.auth.routes_whitelist'],
+        Middleware\AuthenticationMiddleware::class => [
+            Service\ApiKeyService::class,
+            'config.auth.routes_whitelist',
+            'config.auth.routes_with_query_api_key',
+        ],
     ],
 
 ];

module/Rest/config/dependencies.config.php

@@ -57,7 +57,6 @@ return [
         Action\ShortUrl\CreateShortUrlAction::class => [Service\UrlShortener::class, 'config.url_shortener.domain'],
         Action\ShortUrl\SingleStepCreateShortUrlAction::class => [
             Service\UrlShortener::class,
-            ApiKeyService::class,
             'config.url_shortener.domain',
         ],
         Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class],

module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKeyRole.php

@@ -24,7 +24,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
 
     $builder->createField('roleName', Types::STRING)
             ->columnName('role_name')
-            ->length(256)
+            ->length(255)
             ->nullable(false)
             ->build();
 

module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php

@@ -8,49 +8,28 @@ use Psr\Http\Message\ServerRequestInterface as Request;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Model\CreateShortUrlData;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
-use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
-use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
+use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware;
 
 class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction
 {
     protected const ROUTE_PATH = '/short-urls/shorten';
     protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET];
 
-    private ApiKeyServiceInterface $apiKeyService;
-
-    public function __construct(
-        UrlShortenerInterface $urlShortener,
-        ApiKeyServiceInterface $apiKeyService,
-        array $domainConfig
-    ) {
-        parent::__construct($urlShortener, $domainConfig);
-        $this->apiKeyService = $apiKeyService;
-    }
-
-    /**
-     * @throws ValidationException
-     */
     protected function buildShortUrlData(Request $request): CreateShortUrlData
     {
         $query = $request->getQueryParams();
         $longUrl = $query['longUrl'] ?? null;
 
-        $apiKeyResult = $this->apiKeyService->check($query['apiKey'] ?? '');
-        if (! $apiKeyResult->isValid()) {
-            throw ValidationException::fromArray([
-                'apiKey' => 'No API key was provided or it is not valid',
-            ]);
-        }
-
         if ($longUrl === null) {
             throw ValidationException::fromArray([
                 'longUrl' => 'A URL was not provided',
             ]);
         }
 
+        $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request);
         return new CreateShortUrlData($longUrl, [], ShortUrlMeta::fromRawData([
-            ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey(),
+            ShortUrlMetaInputFilter::API_KEY => $apiKey,
             // This will usually be null, unless this API key enforces one specific domain
             ShortUrlMetaInputFilter::DOMAIN => $request->getAttribute(ShortUrlMetaInputFilter::DOMAIN),
         ]));

module/Rest/src/Exception/MissingAuthenticationException.php

@@ -18,18 +18,36 @@ class MissingAuthenticationException extends RuntimeException implements Problem
     private const TITLE = 'Invalid authorization';
     private const TYPE = 'INVALID_AUTHORIZATION';
 
-    public static function fromExpectedTypes(array $expectedTypes): self
+    public static function forHeaders(array $expectedHeaders): self
     {
-        $e = new self(sprintf(
+        $e = self::withMessage(sprintf(
             'Expected one of the following authentication headers, ["%s"], but none were provided',
-            implode('", "', $expectedTypes),
+            implode('", "', $expectedHeaders),
         ));
+        $e->additional = [
+            'expectedTypes' => $expectedHeaders, // Deprecated
+            'expectedHeaders' => $expectedHeaders,
+        ];
 
-        $e->detail = $e->getMessage();
+        return $e;
+    }
+
+    public static function forQueryParam(string $param): self
+    {
+        $e = self::withMessage(sprintf('Expected authentication to be provided in "%s" query param', $param));
+        $e->additional = ['param' => $param];
+
+        return $e;
+    }
+
+    private static function withMessage(string $message): self
+    {
+        $e = new self($message);
+
+        $e->detail = $message;
         $e->title = self::TITLE;
         $e->type = self::TYPE;
         $e->status = StatusCodeInterface::STATUS_UNAUTHORIZED;
-        $e->additional = ['expectedTypes' => $expectedTypes];
 
         return $e;
     }

module/Rest/src/Middleware/AuthenticationMiddleware.php

@@ -8,6 +8,7 @@ use Fig\Http\Message\RequestMethodInterface;
 use Fig\Http\Message\StatusCodeInterface;
 use Mezzio\Router\RouteResult;
 use Psr\Http\Message\ResponseInterface as Response;
+use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
@@ -24,11 +25,16 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
 
     private ApiKeyServiceInterface $apiKeyService;
     private array $routesWhitelist;
+    private array $routesWithQueryApiKey;
 
-    public function __construct(ApiKeyServiceInterface $apiKeyService, array $routesWhitelist)
-    {
+    public function __construct(
+        ApiKeyServiceInterface $apiKeyService,
+        array $routesWhitelist,
+        array $routesWithQueryApiKey
+    ) {
         $this->apiKeyService = $apiKeyService;
         $this->routesWhitelist = $routesWhitelist;
+        $this->routesWithQueryApiKey = $routesWithQueryApiKey;
     }
 
     public function process(Request $request, RequestHandlerInterface $handler): Response
@@ -44,11 +50,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
             return $handler->handle($request);
         }
 
-        $apiKey = $request->getHeaderLine(self::API_KEY_HEADER);
-        if (empty($apiKey)) {
-            throw MissingAuthenticationException::fromExpectedTypes([self::API_KEY_HEADER]);
-        }
-
+        $apiKey = $this->getApiKeyFromRequest($request, $routeResult);
         $result = $this->apiKeyService->check($apiKey);
         if (! $result->isValid()) {
             throw VerifyAuthenticationException::forInvalidApiKey();
@@ -61,4 +63,20 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
     {
         return $request->getAttribute(ApiKey::class);
     }
+
+    private function getApiKeyFromRequest(ServerRequestInterface $request, RouteResult $routeResult): string
+    {
+        $routeName = $routeResult->getMatchedRouteName();
+        $query = $request->getQueryParams();
+        $isRouteWithApiKeyInQuery = contains($this->routesWithQueryApiKey, $routeName);
+        $apiKey = $isRouteWithApiKeyInQuery ? ($query['apiKey'] ?? '') : $request->getHeaderLine(self::API_KEY_HEADER);
+
+        if (empty($apiKey)) {
+            throw $isRouteWithApiKeyInQuery
+                ? MissingAuthenticationException::forQueryParam('apiKey')
+                : MissingAuthenticationException::forHeaders([self::API_KEY_HEADER]);
+        }
+
+        return $apiKey;
+    }
 }

module/Rest/src/Middleware/CrossDomainMiddleware.php

@@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Rest\Middleware;
 
 use Fig\Http\Message\RequestMethodInterface;
 use Laminas\Diactoros\Response\EmptyResponse;
-use Mezzio\Router\RouteResult;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
@@ -32,7 +31,7 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
         }
 
         // Add Allow-Origin header
-        $response = $response->withHeader('Access-Control-Allow-Origin', $request->getHeader('Origin'));
+        $response = $response->withHeader('Access-Control-Allow-Origin', '*');
         if ($request->getMethod() !== self::METHOD_OPTIONS) {
             return $response;
         }
@@ -42,20 +41,8 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
 
     private function addOptionsHeaders(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
     {
-        // TODO This won't work. The route has to be matched from the router as this middleware needs to be executed
-        //      before trying to match the route
-        /** @var RouteResult|null $matchedRoute */
-        $matchedRoute = $request->getAttribute(RouteResult::class);
-        $matchedMethods = $matchedRoute !== null ? $matchedRoute->getAllowedMethods() : [
-            self::METHOD_GET,
-            self::METHOD_POST,
-            self::METHOD_PUT,
-            self::METHOD_PATCH,
-            self::METHOD_DELETE,
-            self::METHOD_OPTIONS,
-        ];
         $corsHeaders = [
-            'Access-Control-Allow-Methods' => implode(',', $matchedMethods),
+            'Access-Control-Allow-Methods' => $this->resolveCorsAllowedMethods($response),
             'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'),
             'Access-Control-Max-Age' => $this->config['max_age'],
         ];
@@ -63,4 +50,22 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
         // Options requests should always be empty and have a 204 status code
         return EmptyResponse::withHeaders(array_merge($response->getHeaders(), $corsHeaders));
     }
+
+    private function resolveCorsAllowedMethods(ResponseInterface $response): string
+    {
+        // ImplicitOptionsMiddleware resolves allowed methods using the RouteResult request's attribute and sets them
+        // in the "Allow" header.
+        // If the header is there, we can re-use the value as it is.
+        if ($response->hasHeader('Allow')) {
+            return $response->getHeaderLine('Allow');
+        }
+
+        return implode(',', [
+            self::METHOD_GET,
+            self::METHOD_POST,
+            self::METHOD_PUT,
+            self::METHOD_PATCH,
+            self::METHOD_DELETE,
+        ]);
+    }
 }

module/Rest/test-api/Action/CreateShortUrlTest.php

@@ -12,7 +12,7 @@ use function Functional\map;
 use function range;
 use function sprintf;
 
-class CreateShortUrlActionTest extends ApiTestCase
+class CreateShortUrlTest extends ApiTestCase
 {
     /** @test */
     public function createsNewShortUrlWhenOnlyLongUrlIsProvided(): void

module/Rest/test-api/Action/DeleteShortUrlTest.php

@@ -7,7 +7,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action;
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait;
 
-class DeleteShortUrlActionTest extends ApiTestCase
+class DeleteShortUrlTest extends ApiTestCase
 {
     use NotFoundUrlHelpersTrait;
 

module/Rest/test-api/Action/EditShortUrlTagsTest.php

@@ -8,7 +8,7 @@ use GuzzleHttp\RequestOptions;
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait;
 
-class EditShortUrlTagsActionTest extends ApiTestCase
+class EditShortUrlTagsTest extends ApiTestCase
 {
     use NotFoundUrlHelpersTrait;
 

module/Rest/test-api/Action/EditShortUrlTest.php

@@ -14,7 +14,7 @@ use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait;
 use function GuzzleHttp\Psr7\build_query;
 use function sprintf;
 
-class EditShortUrlActionTest extends ApiTestCase
+class EditShortUrlTest extends ApiTestCase
 {
     use ArraySubsetAsserts;
     use NotFoundUrlHelpersTrait;

module/Rest/test-api/Action/GlobalVisitsTest.php

@@ -6,7 +6,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action;
 
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 
-class GlobalVisitsActionTest extends ApiTestCase
+class GlobalVisitsTest extends ApiTestCase
 {
     /**
      * @test

module/Rest/test-api/Action/ListTagsTest.php

@@ -7,7 +7,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action;
 use GuzzleHttp\RequestOptions;
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 
-class ListTagsActionTest extends ApiTestCase
+class ListTagsTest extends ApiTestCase
 {
     /**
      * @test

module/Rest/test-api/Action/ResolveShortUrlTest.php

@@ -11,7 +11,7 @@ use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait;
 
 use function sprintf;
 
-class ResolveShortUrlActionTest extends ApiTestCase
+class ResolveShortUrlTest extends ApiTestCase
 {
     use NotFoundUrlHelpersTrait;
 

module/Rest/test-api/Action/ShortUrlVisitsTest.php

@@ -11,7 +11,7 @@ use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait;
 use function GuzzleHttp\Psr7\build_query;
 use function sprintf;
 
-class ShortUrlVisitsActionTest extends ApiTestCase
+class ShortUrlVisitsTest extends ApiTestCase
 {
     use NotFoundUrlHelpersTrait;
 

module/Rest/test-api/Action/SingleStepCreateShortUrlTest.php

@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioApiTest\Shlink\Rest\Action;
+
+use GuzzleHttp\RequestOptions;
+use Psr\Http\Message\ResponseInterface;
+use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
+
+class SingleStepCreateShortUrlTest extends ApiTestCase
+{
+    /**
+     * @test
+     * @dataProvider provideFormats
+     */
+    public function createsNewShortUrlWithExpectedResponse(?string $format, string $expectedContentType): void
+    {
+        $resp = $this->createShortUrl($format, 'valid_api_key');
+
+        self::assertEquals(self::STATUS_OK, $resp->getStatusCode());
+        self::assertEquals($expectedContentType, $resp->getHeaderLine('Content-Type'));
+    }
+
+    public function provideFormats(): iterable
+    {
+        yield 'txt format' => ['txt', 'text/plain'];
+        yield 'json format' => ['json', 'application/json'];
+        yield '<empty> format' => [null, 'application/json'];
+    }
+
+    /** @test */
+    public function authorizationErrorIsReturnedIfNoApiKeyIsSent(): void
+    {
+        $expectedDetail = 'Expected authentication to be provided in "apiKey" query param';
+
+        $resp = $this->createShortUrl();
+        $payload = $this->getJsonResponsePayload($resp);
+
+        self::assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode());
+        self::assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']);
+        self::assertEquals('INVALID_AUTHORIZATION', $payload['type']);
+        self::assertEquals($expectedDetail, $payload['detail']);
+        self::assertEquals('Invalid authorization', $payload['title']);
+    }
+
+    private function createShortUrl(?string $format = 'json', ?string $apiKey = null): ResponseInterface
+    {
+        $query = [
+            'longUrl' => 'https://app.shlink.io',
+            'apiKey' => $apiKey,
+            'format' => $format,
+        ];
+        return $this->callApi(self::METHOD_GET, '/short-urls/shorten', [RequestOptions::QUERY => $query]);
+    }
+}

module/Rest/test-api/Action/TagVisitsTest.php

@@ -8,7 +8,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 
 use function sprintf;
 
-class TagVisitsActionTest extends ApiTestCase
+class TagVisitsTest extends ApiTestCase
 {
     /**
      * @test

module/Rest/test-api/Action/UpdateTagTest.php

@@ -7,7 +7,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action;
 use GuzzleHttp\RequestOptions;
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 
-class UpdateTagActionTest extends ApiTestCase
+class UpdateTagTest extends ApiTestCase
 {
     /**
      * @test

module/Rest/test-api/Middleware/CorsTest.php

@@ -35,7 +35,7 @@ class CorsTest extends ApiTestCase
         ]);
 
         self::assertEquals($expectedStatusCode, $resp->getStatusCode());
-        self::assertEquals($origin, $resp->getHeaderLine('Access-Control-Allow-Origin'));
+        self::assertEquals('*', $resp->getHeaderLine('Access-Control-Allow-Origin'));
         self::assertFalse($resp->hasHeader('Access-Control-Allow-Methods'));
         self::assertFalse($resp->hasHeader('Access-Control-Max-Age'));
         self::assertFalse($resp->hasHeader('Access-Control-Allow-Headers'));
@@ -71,10 +71,9 @@ class CorsTest extends ApiTestCase
 
     public function providePreflightEndpoints(): iterable
     {
-        yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE,OPTIONS'];
-        yield 'short URLs routes' => ['/short-urls', 'GET,POST,PUT,PATCH,DELETE,OPTIONS'];
-//        yield 'short URLs routes' => ['/short-urls', 'GET,POST']; // TODO This should be the good one
-        yield 'tags routes' => ['/tags', 'GET,POST,PUT,PATCH,DELETE,OPTIONS'];
-//        yield 'tags routes' => ['/short-urls', 'GET,POST,PUT,DELETE']; // TODO This should be the good one
+        yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE'];
+        yield 'short URLs route' => ['/short-urls', 'GET,POST'];
+        yield 'tags route' => ['/tags', 'GET,POST,PUT,DELETE'];
+        yield 'health route' => ['/health', 'GET'];
     }
 }

module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php

@@ -16,8 +16,6 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
 use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
-use Shlinkio\Shlink\Rest\Service\ApiKeyCheckResult;
-use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
 
 class SingleStepCreateShortUrlActionTest extends TestCase
 {
@@ -30,11 +28,9 @@ class SingleStepCreateShortUrlActionTest extends TestCase
     public function setUp(): void
     {
         $this->urlShortener = $this->prophesize(UrlShortenerInterface::class);
-        $this->apiKeyService = $this->prophesize(ApiKeyServiceInterface::class);
 
         $this->action = new SingleStepCreateShortUrlAction(
             $this->urlShortener->reveal(),
-            $this->apiKeyService->reveal(),
             [
                 'schema' => 'http',
                 'hostname' => 'foo.com',
@@ -42,26 +38,12 @@ class SingleStepCreateShortUrlActionTest extends TestCase
         );
     }
 
-    /** @test */
-    public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(): void
-    {
-        $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
-        $findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult());
-
-        $this->expectException(ValidationException::class);
-        $findApiKey->shouldBeCalledOnce();
-
-        $this->action->handle($request);
-    }
-
     /** @test */
     public function errorResponseIsReturnedIfNoUrlIsProvided(): void
     {
-        $request = (new ServerRequest())->withQueryParams(['apiKey' => 'abc123']);
-        $findApiKey = $this->apiKeyService->check('abc123')->willReturn(new ApiKeyCheckResult(new ApiKey()));
+        $request = new ServerRequest();
 
         $this->expectException(ValidationException::class);
-        $findApiKey->shouldBeCalledOnce();
 
         $this->action->handle($request);
     }
@@ -70,13 +52,10 @@ class SingleStepCreateShortUrlActionTest extends TestCase
     public function properDataIsPassedWhenGeneratingShortCode(): void
     {
         $apiKey = new ApiKey();
-        $key = $apiKey->toString();
 
         $request = (new ServerRequest())->withQueryParams([
-            'apiKey' => $key,
             'longUrl' => 'http://foobar.com',
-        ]);
-        $findApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey));
+        ])->withAttribute(ApiKey::class, $apiKey);
         $generateShortCode = $this->urlShortener->shorten(
             Argument::that(function (string $argument): bool {
                 Assert::assertEquals('http://foobar.com', $argument);
@@ -89,7 +68,6 @@ class SingleStepCreateShortUrlActionTest extends TestCase
         $resp = $this->action->handle($request);
 
         self::assertEquals(200, $resp->getStatusCode());
-        $findApiKey->shouldHaveBeenCalled();
         $generateShortCode->shouldHaveBeenCalled();
     }
 }

module/Rest/test/Exception/MissingAuthenticationExceptionTest.php

@@ -16,21 +16,22 @@ class MissingAuthenticationExceptionTest extends TestCase
      * @test
      * @dataProvider provideExpectedTypes
      */
-    public function exceptionIsProperlyCreatedFromExpectedTypes(array $expectedTypes): void
+    public function exceptionIsProperlyCreatedFromExpectedHeaders(array $expectedHeaders): void
     {
         $expectedMessage = sprintf(
             'Expected one of the following authentication headers, ["%s"], but none were provided',
-            implode('", "', $expectedTypes),
+            implode('", "', $expectedHeaders),
         );
 
-        $e = MissingAuthenticationException::fromExpectedTypes($expectedTypes);
+        $e = MissingAuthenticationException::forHeaders($expectedHeaders);
 
+        $this->assertCommonExceptionShape($e);
         self::assertEquals($expectedMessage, $e->getMessage());
         self::assertEquals($expectedMessage, $e->getDetail());
-        self::assertEquals('Invalid authorization', $e->getTitle());
-        self::assertEquals('INVALID_AUTHORIZATION', $e->getType());
-        self::assertEquals(401, $e->getStatus());
-        self::assertEquals(['expectedTypes' => $expectedTypes], $e->getAdditionalData());
+        self::assertEquals([
+            'expectedTypes' => $expectedHeaders,
+            'expectedHeaders' => $expectedHeaders,
+        ], $e->getAdditionalData());
     }
 
     public function provideExpectedTypes(): iterable
@@ -40,4 +41,34 @@ class MissingAuthenticationExceptionTest extends TestCase
         yield [[]];
         yield [['foo', 'bar', 'baz']];
     }
+
+    /**
+     * @test
+     * @dataProvider provideExpectedParam
+     */
+    public function exceptionIsProperlyCreatedFromExpectedQueryParam(string $param): void
+    {
+        $expectedMessage = sprintf('Expected authentication to be provided in "%s" query param', $param);
+
+        $e = MissingAuthenticationException::forQueryParam($param);
+
+        $this->assertCommonExceptionShape($e);
+        self::assertEquals($expectedMessage, $e->getMessage());
+        self::assertEquals($expectedMessage, $e->getDetail());
+        self::assertEquals(['param' => $param], $e->getAdditionalData());
+    }
+
+    public function provideExpectedParam(): iterable
+    {
+        yield ['foo'];
+        yield ['bar'];
+        yield ['something'];
+    }
+
+    private function assertCommonExceptionShape(MissingAuthenticationException $e): void
+    {
+        self::assertEquals('Invalid authorization', $e->getTitle());
+        self::assertEquals('INVALID_AUTHORIZATION', $e->getType());
+        self::assertEquals(401, $e->getStatus());
+    }
 }

module/Rest/test/Middleware/AuthenticationMiddlewareTest.php

@@ -38,7 +38,11 @@ class AuthenticationMiddlewareTest extends TestCase
     public function setUp(): void
     {
         $this->apiKeyService = $this->prophesize(ApiKeyServiceInterface::class);
-        $this->middleware = new AuthenticationMiddleware($this->apiKeyService->reveal(), [HealthAction::class]);
+        $this->middleware = new AuthenticationMiddleware(
+            $this->apiKeyService->reveal(),
+            [HealthAction::class],
+            ['with_query_api_key'],
+        );
         $this->handler = $this->prophesize(RequestHandlerInterface::class);
     }
 
@@ -82,27 +86,34 @@ class AuthenticationMiddlewareTest extends TestCase
      * @test
      * @dataProvider provideRequestsWithoutApiKey
      */
-    public function throwsExceptionWhenNoApiKeyIsProvided(ServerRequestInterface $request): void
-    {
+    public function throwsExceptionWhenNoApiKeyIsProvided(
+        ServerRequestInterface $request,
+        string $expectedMessage
+    ): void {
         $this->apiKeyService->check(Argument::any())->shouldNotBeCalled();
         $this->handler->handle($request)->shouldNotBeCalled();
         $this->expectException(MissingAuthenticationException::class);
-        $this->expectExceptionMessage(
-            'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided',
-        );
+        $this->expectExceptionMessage($expectedMessage);
 
         $this->middleware->process($request, $this->handler->reveal());
     }
 
     public function provideRequestsWithoutApiKey(): iterable
     {
-        $baseRequest = ServerRequestFactory::fromGlobals()->withAttribute(
+        $baseRequest = fn (string $routeName) => ServerRequestFactory::fromGlobals()->withAttribute(
             RouteResult::class,
-            RouteResult::fromRoute(new Route('bar', $this->getDummyMiddleware()), []),
+            RouteResult::fromRoute(new Route($routeName, $this->getDummyMiddleware()), []),
         );
+        $apiKeyMessage = 'Expected one of the following authentication headers, ["X-Api-Key"], but none were provided';
+        $queryMessage = 'Expected authentication to be provided in "apiKey" query param';
 
-        yield 'no api key' => [$baseRequest];
-        yield 'empty api key' => [$baseRequest->withHeader('X-Api-Key', '')];
+        yield 'no api key in header' => [$baseRequest('bar'), $apiKeyMessage];
+        yield 'empty api key in header' => [$baseRequest('bar')->withHeader('X-Api-Key', ''), $apiKeyMessage];
+        yield 'no api key in query' => [$baseRequest('with_query_api_key'), $queryMessage];
+        yield 'empty api key in query' => [
+            $baseRequest('with_query_api_key')->withQueryParams(['apiKey' => '']),
+            $queryMessage,
+        ];
     }
 
     /** @test */

module/Rest/test/Middleware/CrossDomainMiddlewareTest.php

@@ -6,8 +6,6 @@ namespace ShlinkioTest\Shlink\Rest\Middleware;
 
 use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
-use Mezzio\Router\Route;
-use Mezzio\Router\RouteResult;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
@@ -15,8 +13,6 @@ use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware;
 
-use function Laminas\Stratigility\middleware;
-
 class CrossDomainMiddlewareTest extends TestCase
 {
     use ProphecyTrait;
@@ -61,7 +57,7 @@ class CrossDomainMiddlewareTest extends TestCase
 
         $headers = $response->getHeaders();
 
-        self::assertEquals('local', $response->getHeaderLine('Access-Control-Allow-Origin'));
+        self::assertEquals('*', $response->getHeaderLine('Access-Control-Allow-Origin'));
         self::assertArrayNotHasKey('Access-Control-Allow-Methods', $headers);
         self::assertArrayNotHasKey('Access-Control-Max-Age', $headers);
         self::assertArrayNotHasKey('Access-Control-Allow-Headers', $headers);
@@ -82,7 +78,7 @@ class CrossDomainMiddlewareTest extends TestCase
 
         $headers = $response->getHeaders();
 
-        self::assertEquals('local', $response->getHeaderLine('Access-Control-Allow-Origin'));
+        self::assertEquals('*', $response->getHeaderLine('Access-Control-Allow-Origin'));
         self::assertArrayHasKey('Access-Control-Allow-Methods', $headers);
         self::assertEquals('1000', $response->getHeaderLine('Access-Control-Max-Age'));
         self::assertEquals('foo, bar, baz', $response->getHeaderLine('Access-Control-Allow-Headers'));
@@ -94,13 +90,15 @@ class CrossDomainMiddlewareTest extends TestCase
      * @dataProvider provideRouteResults
      */
     public function optionsRequestParsesRouteMatchToDetermineAllowedMethods(
-        ?RouteResult $result,
+        ?string $allowHeader,
         string $expectedAllowedMethods
     ): void {
         $originalResponse = new Response();
-        $request = (new ServerRequest())->withAttribute(RouteResult::class, $result)
-                                        ->withMethod('OPTIONS')
-                                        ->withHeader('Origin', 'local');
+        if ($allowHeader !== null) {
+            $originalResponse = $originalResponse->withHeader('Allow', $allowHeader);
+        }
+        $request = (new ServerRequest())->withHeader('Origin', 'local')
+                                        ->withMethod('OPTIONS');
         $this->handler->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce();
 
         $response = $this->middleware->process($request, $this->handler->reveal());
@@ -111,15 +109,9 @@ class CrossDomainMiddlewareTest extends TestCase
 
     public function provideRouteResults(): iterable
     {
-        yield 'with no route result' => [null, 'GET,POST,PUT,PATCH,DELETE,OPTIONS'];
-        yield 'with failed route result' => [RouteResult::fromRouteFailure(['POST', 'GET']), 'POST,GET'];
-        yield 'with success route result' => [
-            RouteResult::fromRoute(
-                new Route('/', middleware(function (): void {
-                }), ['DELETE', 'PATCH', 'PUT']),
-            ),
-            'DELETE,PATCH,PUT',
-        ];
+        yield 'no allow header in response' => [null, 'GET,POST,PUT,PATCH,DELETE'];
+        yield 'allow header in response' => ['POST,GET', 'POST,GET'];
+        yield 'also allow header in response' => ['DELETE,PATCH,PUT', 'DELETE,PATCH,PUT'];
     }
 
     /**