Merge pull request #2455 from acelaya-forks/feature/cors-customization

Add new CORS configuration options

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
     For BC, if this env vars is not present, we'll still consider the integration enabled if the `MERCURE_PUBLIC_HUB_URL` env var has a value. This is considered deprecated though, and next major version will rely only on `MERCURE_ENABLED`, so if you are using Mercure, make sure to set `MERCURE_ENABLED=true` to be ready.
 
 * [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `REAL_TIME_UPDATES_TOPICS` env var and corresponding config option, to granularly decide which real-time updates topics should be enabled.
+* [#2418](https://github.com/shlinkio/shlink/issues/2418) Add more granular control over how Shlink handles CORS. It is now possible to customize the `Access-Control-Allow-Origin`, `Access-Control-Max-Age` and `Access-Control-Allow-Credentials` headers via env vars or config options.
 
 ### Changed
 * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles.

composer.json

@@ -47,7 +47,7 @@
         "shlinkio/shlink-config": "^4.0",
         "shlinkio/shlink-event-dispatcher": "^4.2",
         "shlinkio/shlink-importer": "^5.6",
-        "shlinkio/shlink-installer": "dev-develop#c5a8523 as 9.6",
+        "shlinkio/shlink-installer": "dev-develop#9005232 as 9.6",
         "shlinkio/shlink-ip-geolocation": "^4.3",
         "shlinkio/shlink-json": "^1.2",
         "spiral/roadrunner": "^2025.1",

config/autoload/cors.global.php

@@ -1,11 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-return [
-
-    'cors' => [
-        'max_age' => 3600,
-    ],
-
-];

config/autoload/installer.global.php

@@ -77,6 +77,9 @@ return [
             Option\Matomo\MatomoSiteIdConfigOption::class,
             Option\Matomo\MatomoApiTokenConfigOption::class,
             Option\RealTimeUpdates\RealTimeUpdatesTopicsConfigOption::class,
+            Option\Cors\CorsAllowOriginConfigOption::class,
+            Option\Cors\CorsAllowCredentialsConfigOption::class,
+            Option\Cors\CorsMaxAgeConfigOption::class,
         ],
 
         'installation_commands' => [

module/Core/config/dependencies.config.php

@@ -37,6 +37,7 @@ return [
             Config\Options\RabbitMqOptions::class => [Config\Options\RabbitMqOptions::class, 'fromEnv'],
             Config\Options\RobotsOptions::class => [Config\Options\RobotsOptions::class, 'fromEnv'],
             Config\Options\RealTimeUpdatesOptions::class => [Config\Options\RealTimeUpdatesOptions::class, 'fromEnv'],
+            Config\Options\CorsOptions::class => [Config\Options\CorsOptions::class, 'fromEnv'],
 
             RedirectRule\ShortUrlRedirectRuleService::class => ConfigAbstractFactory::class,
             RedirectRule\ShortUrlRedirectionResolver::class => ConfigAbstractFactory::class,

module/Core/functions/array-utils.php

@@ -10,6 +10,11 @@ use function in_array;
 
 use const ARRAY_FILTER_USE_KEY;
 
+/**
+ * @template T
+ * @param T $value
+ * @param T[] $array
+ */
 function contains(mixed $value, array $array): bool
 {
     return in_array($value, $array, strict: true);

module/Core/src/Config/EnvVars.php

@@ -86,6 +86,9 @@ enum EnvVars: string
     case INITIAL_API_KEY = 'INITIAL_API_KEY';
     case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD';
     case REAL_TIME_UPDATES_TOPICS = 'REAL_TIME_UPDATES_TOPICS';
+    case CORS_ALLOW_ORIGIN = 'CORS_ALLOW_ORIGIN';
+    case CORS_ALLOW_CREDENTIALS = 'CORS_ALLOW_CREDENTIALS';
+    case CORS_MAX_AGE = 'CORS_MAX_AGE';
 
     /** @deprecated Use REDIRECT_EXTRA_PATH */
     case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH';
@@ -187,6 +190,10 @@ enum EnvVars: string
             self::DISABLE_REFERRER_TRACKING,
             self::DISABLE_UA_TRACKING => false,
 
+            self::CORS_ALLOW_ORIGIN => '*',
+            self::CORS_ALLOW_CREDENTIALS => false,
+            self::CORS_MAX_AGE => 3600,
+
             default => null,
         };
     }

module/Core/src/Config/Options/CorsOptions.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace Shlinkio\Shlink\Core\Config\Options;
+
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Shlinkio\Shlink\Core\Config\EnvVars;
+
+use function Shlinkio\Shlink\Core\ArrayUtils\contains;
+use function Shlinkio\Shlink\Core\splitByComma;
+use function strtolower;
+
+final readonly class CorsOptions
+{
+    private const string ORIGIN_PATTERN = '<origin>';
+
+    /** @var string[]|'*'|'<origin>' */
+    public string|array $allowOrigins;
+
+    public function __construct(
+        string $allowOrigins = '*',
+        public bool $allowCredentials = false,
+        public int $maxAge = 3600,
+    ) {
+        $lowerCaseAllowOrigins = strtolower($allowOrigins);
+        $this->allowOrigins = $lowerCaseAllowOrigins === '*' || $lowerCaseAllowOrigins === self::ORIGIN_PATTERN
+            ? $lowerCaseAllowOrigins
+            : splitByComma($lowerCaseAllowOrigins);
+    }
+
+    public static function fromEnv(): self
+    {
+        return new self(
+            allowOrigins: EnvVars::CORS_ALLOW_ORIGIN->loadFromEnv(),
+            allowCredentials: EnvVars::CORS_ALLOW_CREDENTIALS->loadFromEnv(),
+            maxAge: EnvVars::CORS_MAX_AGE->loadFromEnv(),
+        );
+    }
+
+    public function responseWithAllowOrigin(RequestInterface $request, ResponseInterface $response): ResponseInterface
+    {
+        if ($this->allowOrigins === '*') {
+            return $response->withHeader('Access-Control-Allow-Origin', '*');
+        }
+
+        $requestOrigin = $request->getHeaderLine('Origin');
+        if (
+            // The special <origin> value means we should allow requests from the origin set in the request's Origin
+            // header
+            $this->allowOrigins === self::ORIGIN_PATTERN
+            // If an array of allowed hosts was provided, set Access-Control-Allow-Origin header only if request's
+            // Origin header matches one of them
+            || contains($requestOrigin, $this->allowOrigins)
+        ) {
+            return $response->withHeader('Access-Control-Allow-Origin', $requestOrigin);
+        }
+
+        return $response;
+    }
+}

module/Core/test/Config/Options/CorsOptionsTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Config\Options;
+
+use Laminas\Diactoros\Response;
+use Laminas\Diactoros\ServerRequestFactory;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Config\Options\CorsOptions;
+
+class CorsOptionsTest extends TestCase
+{
+    #[Test]
+    #[TestWith(['*', '*', '*'])]
+    #[TestWith(['<origin>', '<origin>', 'https://example.com'])]
+    #[TestWith(['foo,bar, baz ', ['foo', 'bar', 'baz'], ''])]
+    #[TestWith(['foo,bar,https://example.com', ['foo', 'bar', 'https://example.com'], 'https://example.com'])]
+    public function expectedAccessControlAllowOriginIsSet(
+        string $allowOrigins,
+        string|array $expectedAllowOrigins,
+        string $expectedAllowOriginsHeader,
+    ): void {
+        $options = new CorsOptions($allowOrigins);
+
+        self::assertEquals($expectedAllowOrigins, $options->allowOrigins);
+        self::assertEquals(
+            $expectedAllowOriginsHeader,
+            $options->responseWithAllowOrigin(
+                ServerRequestFactory::fromGlobals()->withHeader('Origin', 'https://example.com'),
+                new Response(),
+            )->getHeaderLine('Access-Control-Allow-Origin'),
+        );
+    }
+}

module/Rest/config/dependencies.config.php

@@ -115,7 +115,7 @@ return [
             RedirectRule\ShortUrlRedirectRuleService::class,
         ],
 
-        Middleware\CrossDomainMiddleware::class => ['config.cors'],
+        Middleware\CrossDomainMiddleware::class => [Config\Options\CorsOptions::class],
         Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => [
             Config\Options\UrlShortenerOptions::class,
         ],

module/Rest/src/Middleware/CrossDomainMiddleware.php

@@ -10,12 +10,13 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Config\Options\CorsOptions;
 
 use function implode;
 
-class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface
+readonly class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface
 {
-    public function __construct(private array $config)
+    public function __construct(private CorsOptions $options)
     {
     }
 
@@ -27,7 +28,7 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
         }
 
         // Add Allow-Origin header
-        $response = $response->withHeader('Access-Control-Allow-Origin', '*');
+        $response = $this->options->responseWithAllowOrigin($request, $response);
         if ($request->getMethod() !== self::METHOD_OPTIONS) {
             return $response;
         }
@@ -40,9 +41,13 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa
         $corsHeaders = [
             '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'],
+            'Access-Control-Max-Age' => $this->options->maxAge,
         ];
 
+        if ($this->options->allowCredentials) {
+            $corsHeaders['Access-Control-Allow-Credentials'] = 'true';
+        }
+
         // Options requests should always be empty and have a 204 status code
         return EmptyResponse::withHeaders([...$response->getHeaders(), ...$corsHeaders]);
     }

module/Rest/test/Middleware/CrossDomainMiddlewareTest.php

@@ -8,19 +8,19 @@ use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Server\RequestHandlerInterface;
+use Shlinkio\Shlink\Core\Config\Options\CorsOptions;
 use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware;
 
 class CrossDomainMiddlewareTest extends TestCase
 {
-    private CrossDomainMiddleware $middleware;
     private MockObject & RequestHandlerInterface $handler;
 
     protected function setUp(): void
     {
-        $this->middleware = new CrossDomainMiddleware(['max_age' => 1000]);
         $this->handler = $this->createMock(RequestHandlerInterface::class);
     }
 
@@ -30,7 +30,7 @@ class CrossDomainMiddlewareTest extends TestCase
         $originalResponse = (new Response())->withStatus(404);
         $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse);
 
-        $response = $this->middleware->process(new ServerRequest(), $this->handler);
+        $response = $this->middleware()->process(new ServerRequest(), $this->handler);
         $headers = $response->getHeaders();
 
         self::assertSame($originalResponse, $response);
@@ -47,7 +47,7 @@ class CrossDomainMiddlewareTest extends TestCase
         $originalResponse = new Response();
         $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse);
 
-        $response = $this->middleware->process((new ServerRequest())->withHeader('Origin', 'local'), $this->handler);
+        $response = $this->middleware()->process((new ServerRequest())->withHeader('Origin', 'local'), $this->handler);
         self::assertNotSame($originalResponse, $response);
 
         $headers = $response->getHeaders();
@@ -68,7 +68,7 @@ class CrossDomainMiddlewareTest extends TestCase
             ->withHeader('Access-Control-Request-Headers', 'foo, bar, baz');
         $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse);
 
-        $response = $this->middleware->process($request, $this->handler);
+        $response = $this->middleware()->process($request, $this->handler);
         self::assertNotSame($originalResponse, $response);
 
         $headers = $response->getHeaders();
@@ -93,7 +93,7 @@ class CrossDomainMiddlewareTest extends TestCase
                                         ->withMethod('OPTIONS');
         $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse);
 
-        $response = $this->middleware->process($request, $this->handler);
+        $response = $this->middleware()->process($request, $this->handler);
 
         self::assertEquals($response->getHeaderLine('Access-Control-Allow-Methods'), $expectedAllowedMethods);
         self::assertEquals(204, $response->getStatusCode());
@@ -117,7 +117,7 @@ class CrossDomainMiddlewareTest extends TestCase
                                         ->withHeader('Origin', 'local');
         $this->handler->expects($this->once())->method('handle')->willReturn($originalResponse);
 
-        $response = $this->middleware->process($request, $this->handler);
+        $response = $this->middleware()->process($request, $this->handler);
 
         self::assertEquals($expectedStatus, $response->getStatusCode());
     }
@@ -140,4 +140,30 @@ class CrossDomainMiddlewareTest extends TestCase
         yield 'OPTIONS 400' => ['OPTIONS', 400, 204];
         yield 'OPTIONS 500' => ['OPTIONS', 500, 204];
     }
+
+    #[Test]
+    #[TestWith([true])]
+    #[TestWith([false])]
+    public function credentialsAreAllowedIfConfiguredSo(bool $allowCredentials): void
+    {
+        $originalResponse = new Response();
+        $request = (new ServerRequest())
+            ->withMethod('OPTIONS')
+            ->withHeader('Origin', 'local');
+        $this->handler->method('handle')->willReturn($originalResponse);
+
+        $response = $this->middleware(allowCredentials: $allowCredentials)->process($request, $this->handler);
+        $headers = $response->getHeaders();
+
+        if ($allowCredentials) {
+            self::assertArrayHasKey('Access-Control-Allow-Credentials', $headers);
+        } else {
+            self::assertArrayNotHasKey('Access-Control-Allow-Credentials', $headers);
+        }
+    }
+
+    private function middleware(bool $allowCredentials = false): CrossDomainMiddleware
+    {
+        return new CrossDomainMiddleware(new CorsOptions(allowCredentials: $allowCredentials, maxAge: 1000));
+    }
 }