From 92d7a44cee7b664d83a3b93f0ea58bf875af9197 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 5 Jul 2025 10:34:50 +0200 Subject: [PATCH 1/6] Add new CORS configuration options --- config/autoload/cors.global.php | 11 ---- module/Core/config/dependencies.config.php | 1 + module/Core/src/Config/EnvVars.php | 7 +++ .../Core/src/Config/Options/CorsOptions.php | 58 +++++++++++++++++++ module/Rest/config/dependencies.config.php | 2 +- .../src/Middleware/CrossDomainMiddleware.php | 9 +-- .../Middleware/CrossDomainMiddlewareTest.php | 3 +- 7 files changed, 74 insertions(+), 17 deletions(-) delete mode 100644 config/autoload/cors.global.php create mode 100644 module/Core/src/Config/Options/CorsOptions.php diff --git a/config/autoload/cors.global.php b/config/autoload/cors.global.php deleted file mode 100644 index 58ad9428..00000000 --- a/config/autoload/cors.global.php +++ /dev/null @@ -1,11 +0,0 @@ - [ - 'max_age' => 3600, - ], - -]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 75c70594..0ad943e9 100644 --- a/module/Core/config/dependencies.config.php +++ b/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, diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 7fa1a95b..4f57a721 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/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, }; } diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php new file mode 100644 index 00000000..041503b6 --- /dev/null +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -0,0 +1,58 @@ +'; + + /** @var string[]|'*'|'' */ + public string|array $allowOrigins; + + public function __construct( + string $allowOrigins = '*', + public bool $allowCredentials = false, + public int $maxAge = 3600, + ) { + $this->allowOrigins = $allowOrigins !== '*' && $allowOrigins !== self::ORIGIN_PATTERN + ? splitByComma($allowOrigins) + : $allowOrigins; + } + + 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->getHeader('Origin'); + if ( + // The special 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; + } +} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index df482a46..058a860d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/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, ], diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index d6a51a0c..4e3409d2 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/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,7 +41,7 @@ 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, ]; // Options requests should always be empty and have a 204 status code diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index b74a435f..615b0132 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -11,6 +11,7 @@ use PHPUnit\Framework\Attributes\Test; 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 @@ -20,7 +21,7 @@ class CrossDomainMiddlewareTest extends TestCase protected function setUp(): void { - $this->middleware = new CrossDomainMiddleware(['max_age' => 1000]); + $this->middleware = new CrossDomainMiddleware(new CorsOptions(maxAge: 1000)); $this->handler = $this->createMock(RequestHandlerInterface::class); } From 834bc4ae20a2803dc503bbfc248398cede9748e4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 10:36:12 +0200 Subject: [PATCH 2/6] Allow credentials to be enabled in CORS --- module/Core/src/Config/Options/CorsOptions.php | 8 +++++--- module/Rest/src/Middleware/CrossDomainMiddleware.php | 4 ++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index 041503b6..efc6ae1c 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -8,6 +8,7 @@ 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 { @@ -21,9 +22,10 @@ final readonly class CorsOptions public bool $allowCredentials = false, public int $maxAge = 3600, ) { - $this->allowOrigins = $allowOrigins !== '*' && $allowOrigins !== self::ORIGIN_PATTERN - ? splitByComma($allowOrigins) - : $allowOrigins; + $lowerCaseAllowOrigins = strtolower($allowOrigins); + $this->allowOrigins = contains($lowerCaseAllowOrigins, ['*', self::ORIGIN_PATTERN]) + ? $lowerCaseAllowOrigins + : splitByComma($lowerCaseAllowOrigins); } public static function fromEnv(): self diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index 4e3409d2..37360e2e 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -44,6 +44,10 @@ readonly class CrossDomainMiddleware implements MiddlewareInterface, RequestMeth '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]); } From cd4fcc9b0a8a04f5e34d753bd9775c3c37d7ad40 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 13:06:16 +0200 Subject: [PATCH 3/6] Update shlink-installer --- composer.json | 2 +- config/autoload/installer.global.php | 3 +++ module/Core/src/Config/Options/CorsOptions.php | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index f391842b..26031b7c 100644 --- a/composer.json +++ b/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", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 063ff7d4..e9270eb6 100644 --- a/config/autoload/installer.global.php +++ b/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' => [ diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index efc6ae1c..fbf191ee 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -23,7 +23,7 @@ final readonly class CorsOptions public int $maxAge = 3600, ) { $lowerCaseAllowOrigins = strtolower($allowOrigins); - $this->allowOrigins = contains($lowerCaseAllowOrigins, ['*', self::ORIGIN_PATTERN]) + $this->allowOrigins = $lowerCaseAllowOrigins === '*' || $lowerCaseAllowOrigins === self::ORIGIN_PATTERN ? $lowerCaseAllowOrigins : splitByComma($lowerCaseAllowOrigins); } From 1d96cc0279f9bcbe0013bff92f520da326ae9540 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 8 Jul 2025 13:17:46 +0200 Subject: [PATCH 4/6] Update CrossDomainMiddleware test --- .../Middleware/CrossDomainMiddlewareTest.php | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 615b0132..5893ca8c 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -8,6 +8,7 @@ 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; @@ -16,12 +17,10 @@ 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(new CorsOptions(maxAge: 1000)); $this->handler = $this->createMock(RequestHandlerInterface::class); } @@ -31,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); @@ -48,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(); @@ -69,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(); @@ -94,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()); @@ -118,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()); } @@ -141,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)); + } } From 3369afe22cdaf7983e41b91e90572a4ec1b90aef Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 16 Jul 2025 08:29:57 +0200 Subject: [PATCH 5/6] Add CorsOptions test --- module/Core/functions/array-utils.php | 5 +++ .../Core/src/Config/Options/CorsOptions.php | 2 +- .../test/Config/Options/CorsOptionsTest.php | 37 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 module/Core/test/Config/Options/CorsOptionsTest.php diff --git a/module/Core/functions/array-utils.php b/module/Core/functions/array-utils.php index d68851d8..3e0010a2 100644 --- a/module/Core/functions/array-utils.php +++ b/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); diff --git a/module/Core/src/Config/Options/CorsOptions.php b/module/Core/src/Config/Options/CorsOptions.php index fbf191ee..f4a23139 100644 --- a/module/Core/src/Config/Options/CorsOptions.php +++ b/module/Core/src/Config/Options/CorsOptions.php @@ -43,7 +43,7 @@ final readonly class CorsOptions return $response->withHeader('Access-Control-Allow-Origin', '*'); } - $requestOrigin = $request->getHeader('Origin'); + $requestOrigin = $request->getHeaderLine('Origin'); if ( // The special value means we should allow requests from the origin set in the request's Origin // header diff --git a/module/Core/test/Config/Options/CorsOptionsTest.php b/module/Core/test/Config/Options/CorsOptionsTest.php new file mode 100644 index 00000000..cfed36d7 --- /dev/null +++ b/module/Core/test/Config/Options/CorsOptionsTest.php @@ -0,0 +1,37 @@ +', '', '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'), + ); + } +} From f5c6bc8204c9e2a9fcd20bd984cf713f6798b7c0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 16 Jul 2025 08:37:38 +0200 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 1 + module/Core/test/Config/Options/CorsOptionsTest.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7b4c4ba..a22b5be9 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/module/Core/test/Config/Options/CorsOptionsTest.php b/module/Core/test/Config/Options/CorsOptionsTest.php index cfed36d7..b02799b0 100644 --- a/module/Core/test/Config/Options/CorsOptionsTest.php +++ b/module/Core/test/Config/Options/CorsOptionsTest.php @@ -30,7 +30,7 @@ class CorsOptionsTest extends TestCase $expectedAllowOriginsHeader, $options->responseWithAllowOrigin( ServerRequestFactory::fromGlobals()->withHeader('Origin', 'https://example.com'), - new Response() + new Response(), )->getHeaderLine('Access-Control-Allow-Origin'), ); }