From 887531c44af40efe639c0a8c1884c4c47d466c4a Mon Sep 17 00:00:00 2001 From: Krystian Szymukowicz Date: Mon, 5 Oct 2026 19:24:08 +0200 Subject: [PATCH] [!!!] Refactor CORS handling Centralize actual and preflight response handling in CorsService. Apply CORS configuration consistently and match origin patterns against complete origins. Improve OPTIONS handling, normalize configuration lists, handle null and empty entries, and add Vary headers for HTTP caches. Preserve processor extension hooks and extend regression coverage. --- Classes/Configuration/CorsOptions.php | 14 +- .../OptionsOperationHandler.php | 65 +--- Classes/Processor/CorsProcessor.php | 36 +-- Classes/Service/CorsService.php | 123 +++++++- Documentation/Cors/Index.rst | 50 ++- .../Miscellaneous/Changelog/Index.rst | 1 + .../Dispatcher/CorsDispatcherTest.php | 156 ++++++++++ Tests/Unit/Processor/CorsProcessorTest.php | 62 ++++ Tests/Unit/Service/CorsServiceTest.php | 293 ++++++++++++++++++ 9 files changed, 700 insertions(+), 100 deletions(-) create mode 100644 Tests/Functional/Dispatcher/CorsDispatcherTest.php create mode 100644 Tests/Unit/Processor/CorsProcessorTest.php create mode 100644 Tests/Unit/Service/CorsServiceTest.php diff --git a/Classes/Configuration/CorsOptions.php b/Classes/Configuration/CorsOptions.php index 962a68ab..7b4908b6 100644 --- a/Classes/Configuration/CorsOptions.php +++ b/Classes/Configuration/CorsOptions.php @@ -29,11 +29,17 @@ public function __construct(array $options) $this->allowHeaders, (isset($options['simpleHeaders']) ? (array)$options['simpleHeaders'] : []) ); - $this->allowHeaders = array_map('strtolower', $this->allowHeaders); - $this->allowMethods = isset($options['allowMethods']) ? - array_map('strtoupper', (array)$options['allowMethods']) : $this->allowMethods; - $this->exposeHeaders = isset($options['exposeHeaders']) ? (array)$options['exposeHeaders'] : $this->exposeHeaders; + $this->allowHeaders = $this->normalizeList($this->allowHeaders, 'strtolower'); + $this->allowMethods = $this->normalizeList((array)($options['allowMethods'] ?? $this->allowMethods), 'strtoupper'); + $this->exposeHeaders = $this->normalizeList((array)($options['exposeHeaders'] ?? $this->exposeHeaders), 'strtolower'); $this->maxAge = isset($options['maxAge']) ? (int)$options['maxAge'] : $this->maxAge; $this->originRegex = isset($options['originRegex']) ? (bool)$options['originRegex'] : $this->originRegex; } + + private function normalizeList(array $values, callable $normalizeCase): array + { + $values = array_map(static fn($value): string => $normalizeCase(trim((string)$value)), $values); + + return array_values(array_unique(array_filter($values, static fn(string $value): bool => $value !== ''))); + } } diff --git a/Classes/OperationHandler/OptionsOperationHandler.php b/Classes/OperationHandler/OptionsOperationHandler.php index 845a181e..02c51dad 100644 --- a/Classes/OperationHandler/OptionsOperationHandler.php +++ b/Classes/OperationHandler/OptionsOperationHandler.php @@ -46,71 +46,8 @@ public static function supports(OperationInterface $operation, Request $request) */ public function handle(OperationInterface $operation, Request $request, array $route, ?ResponseInterface &$response) { - $options = $this->corsService->getOptions(); - $response = $response instanceof ResponseInterface ? $response : new Response(); + $response = $this->corsService->applyPreflightResponse($request, $response ?? new Response()); - if ($options->allowCredentials) { - $response = $response->withHeader('Access-Control-Allow-Credentials', 'true'); - } - - if ($options->allowMethods !== []) { - $response = $response->withHeader('Access-Control-Allow-Methods', implode(', ', $options->allowMethods)); - } - - if ($options->allowHeaders !== []) { - $allowHeaders = $this->corsService->isWildcard($options->allowHeaders) - ? $request->headers->get('Access-Control-Request-Headers') - : implode(', ', $options->allowHeaders); - - if ($allowHeaders !== null && $allowHeaders !== '') { - $response = $response->withHeader('Access-Control-Allow-Headers', $allowHeaders); - } - } - - if ($options->maxAge !== null) { - $response = $response->withHeader('Access-Control-Max-Age', (string)$options->maxAge); - } - - if (!$this->corsService->isAllowedOrigin($request->headers->get('Origin'), $options)) { - $response = $response->withoutHeader('Access-Control-Allow-Origin'); - - return; - } - - $response = $response->withHeader('Access-Control-Allow-Origin', $request->headers->get('Origin')); - - if (!in_array( - strtoupper($request->headers->get('Access-Control-Request-Method')), - $options->allowMethods, - true - )) { - $response = $response->withStatus(405); - - return; - } - - // Allow header in case-set received from client as some browsers may send it differently - if (!in_array($request->headers->get('Access-Control-Request-Method'), $options->allowMethods, true)) { - $allowMethods = array_merge( - $options->allowMethods, - [$request->headers->get('Access-Control-Request-Method')] - ); - $response = $response->withHeader('Access-Control-Allow-Methods', implode(', ', $allowMethods)); - } - - $headers = $request->headers->get('Access-Control-Request-Headers'); - if ($headers !== null) { - $headers = strtolower(trim($headers)); - if ($headers !== '' && !$this->corsService->isWildcard($options->allowHeaders)) { - foreach (preg_split('{, *}', $headers) as $header) { - if (!in_array($header, $options->allowHeaders, true)) { - $response = $response->withStatus(405); - - return 'Unauthorized header ' . $header; - } - } - } - } return null; } } diff --git a/Classes/Processor/CorsProcessor.php b/Classes/Processor/CorsProcessor.php index e285e972..0c55221d 100644 --- a/Classes/Processor/CorsProcessor.php +++ b/Classes/Processor/CorsProcessor.php @@ -14,47 +14,25 @@ public function __construct(private readonly ?CorsService $corsService) {} public function process(Request $request, ResponseInterface &$response): void { - if ( - !$this->isCorsRequest($request) - || $this->isPreflightRequest($request) - ) { + if ($this->corsService === null) { return; } - $options = $this->corsService->getOptions(); - - $requestOrigin = $request->headers->get('Origin'); - - if (!$this->corsService->isAllowedOrigin($requestOrigin, $options)) { - $response = $response->withoutHeader('Access-Control-Allow-Origin'); - } - - $response = $response->withHeader( - 'Access-Control-Allow-Origin', - $requestOrigin - ); - - if ($options->allowCredentials) { - $response = $response->withHeader('Access-Control-Allow-Credentials', 'true'); + $response = $this->corsService->addVary($response, ['Origin']); + if (!$this->isCorsRequest($request) || $this->isPreflightRequest($request)) { + return; } - if ($options->exposeHeaders !== []) { - $response = $response->withHeader( - 'Access-Control-Expose-Headers', - strtolower(implode(', ', $options->exposeHeaders)) - ); - } + $response = $this->corsService->applyActualResponse($request, $response); } protected function isCorsRequest(Request $request): bool { - return $request->headers->has('Origin') - && $request->headers->get('Origin') - !== $request->getSchemeAndHttpHost(); + return $this->corsService->isCorsRequest($request); } protected function isPreflightRequest(Request $request): bool { - return $request->getMethod() === Request::METHOD_OPTIONS; + return $this->corsService->isPreflightRequest($request); } } diff --git a/Classes/Service/CorsService.php b/Classes/Service/CorsService.php index 837eef3e..afea4d5b 100644 --- a/Classes/Service/CorsService.php +++ b/Classes/Service/CorsService.php @@ -4,8 +4,10 @@ namespace SourceBroker\T3api\Service; +use Psr\Http\Message\ResponseInterface; use SourceBroker\T3api\Configuration\Configuration; use SourceBroker\T3api\Configuration\CorsOptions; +use Symfony\Component\HttpFoundation\Request; use TYPO3\CMS\Core\Utility\GeneralUtility; class CorsService @@ -15,6 +17,84 @@ public function getOptions(): CorsOptions return GeneralUtility::makeInstance(CorsOptions::class, Configuration::getCors()); } + public function isCorsRequest(Request $request): bool + { + $origin = $request->headers->get('Origin'); + + return $origin !== null && $origin !== '' && $origin !== $request->getSchemeAndHttpHost(); + } + + public function isPreflightRequest(Request $request): bool + { + return $request->isMethod(Request::METHOD_OPTIONS) + && $request->headers->has('Origin') + && $request->headers->has('Access-Control-Request-Method'); + } + + public function applyActualResponse(Request $request, ResponseInterface $response): ResponseInterface + { + // Include requests without Origin: a shared cache must not reuse those for CORS requests. + $response = $this->addVary($this->removeCorsHeaders($response), ['Origin']); + $options = $this->getOptions(); + $origin = (string)$request->headers->get('Origin'); + if ($origin === '' || !$this->isAllowedOrigin($origin, $options)) { + return $response; + } + + $response = $this->allowOrigin($response, $origin, $options); + if ($options->exposeHeaders !== []) { + $response = $response->withHeader('Access-Control-Expose-Headers', implode(', ', $options->exposeHeaders)); + } + + return $response; + } + + public function applyPreflightResponse(Request $request, ResponseInterface $response): ResponseInterface + { + $response = $this->addVary($response, ['Origin', 'Access-Control-Request-Method', 'Access-Control-Request-Headers']); + if (!$this->isPreflightRequest($request)) { + return $response; + } + + $response = $this->removeCorsHeaders($response); + $options = $this->getOptions(); + $origin = (string)$request->headers->get('Origin'); + if ($origin === '' || !$this->isAllowedOrigin($origin, $options)) { + return $response->withStatus(403); + } + + $method = (string)$request->headers->get('Access-Control-Request-Method'); + if (!in_array(strtoupper($method), $options->allowMethods, true)) { + return $response->withStatus(405); + } + + $requestedHeaders = array_values(array_filter(array_map( + static fn(string $header): string => strtolower(trim($header)), + explode(',', (string)$request->headers->get('Access-Control-Request-Headers')) + ), static fn(string $header): bool => $header !== '')); + $wildcardHeaders = $this->isWildcard($options->allowHeaders); + if (!$wildcardHeaders && array_diff($requestedHeaders, $options->allowHeaders) !== []) { + return $response->withStatus(405); + } + + // Grant access only after the complete preflight has been validated. + $response = $this->allowOrigin($response, $origin, $options); + // Preserve legacy case-insensitive approval while also advertising the exact requested spelling. + $methods = array_unique([...$options->allowMethods, $method]); + $response = $response->withHeader('Access-Control-Allow-Methods', implode(', ', $methods)); + + // Echo concrete names for wildcard configuration, including credentialed requests. + $headers = $wildcardHeaders ? $requestedHeaders : $options->allowHeaders; + if ($headers !== []) { + $response = $response->withHeader('Access-Control-Allow-Headers', implode(', ', array_unique($headers))); + } + if ($options->maxAge !== null) { + $response = $response->withHeader('Access-Control-Max-Age', (string)$options->maxAge); + } + + return $response; + } + public function isAllowedOrigin(string $origin, CorsOptions $options): bool { if ($this->isWildcard($options->allowOrigin)) { @@ -23,7 +103,7 @@ public function isAllowedOrigin(string $origin, CorsOptions $options): bool if ($options->originRegex) { foreach ($options->allowOrigin as $originRegexp) { - if (preg_match('{' . $originRegexp . '}i', $origin)) { + if (preg_match('{\A(?:' . $originRegexp . ')\z}i', $origin) === 1) { return true; } } @@ -40,4 +120,45 @@ public function isWildcard($option): bool || (is_array($option) && in_array('*', $option, true)) || (is_string($option) && $option === '*'); } + + private function allowOrigin(ResponseInterface $response, string $origin, CorsOptions $options): ResponseInterface + { + $response = $response->withHeader('Access-Control-Allow-Origin', $origin); + if ($options->allowCredentials) { + $response = $response->withHeader('Access-Control-Allow-Credentials', 'true'); + } + + return $response; + } + + private function removeCorsHeaders(ResponseInterface $response): ResponseInterface + { + foreach ([ + 'Access-Control-Allow-Origin', + 'Access-Control-Allow-Credentials', + 'Access-Control-Allow-Methods', + 'Access-Control-Allow-Headers', + 'Access-Control-Expose-Headers', + 'Access-Control-Max-Age', + ] as $header) { + $response = $response->withoutHeader($header); + } + + return $response; + } + + public function addVary(ResponseInterface $response, array $headers): ResponseInterface + { + $vary = array_values(array_filter(array_map('trim', explode(',', $response->getHeaderLine('Vary'))))); + if (in_array('*', $vary, true)) { + return $response; + } + foreach ($headers as $header) { + if (!in_array(strtolower($header), array_map('strtolower', $vary), true)) { + $vary[] = $header; + } + } + + return $response->withHeader('Vary', implode(', ', $vary)); + } } diff --git a/Documentation/Cors/Index.rst b/Documentation/Cors/Index.rst index 4a3b02cb..fbf9b70d 100644 --- a/Documentation/Cors/Index.rst +++ b/Documentation/Cors/Index.rst @@ -28,7 +28,7 @@ In code below there is a list of all supported configuration options and their d - ``allowCredentials`` (``boolean``) - when set to ``true`` adds response header ``Access-Control-Allow-Credentials`` with value ``true``. -- ``allowOrigin`` (``array`` or ``string``) - can be set to ``*`` to accept any value. Can be an array with string values of allowed origins (e.g. ``['http://www.example.com', 'https://www.example.com']``) or regular expressions when ``originRegex`` is set to ``true`` (e.g. ``['http://.*\.example\.com', 'https://.*\.example\.com']``). +- ``allowOrigin`` (``array`` or ``string``) - can be set to ``*`` to accept any value. Can be an array with string values of allowed origins (e.g. ``['http://www.example.com', 'https://www.example.com']``) or regular expressions when ``originRegex`` is set to ``true`` (e.g. ``['https://[a-z0-9-]+\.example\.com']``). Patterns must match the complete origin, including its scheme and any non-default port: t3api wraps each pattern in ``\A(?:...)\z``. Existing patterns with ``^`` and ``$`` remain supported. - ``allowHeaders`` (``array`` or ``string``) - can be set to ``*`` to accept any value or an array of strings with allowed headers (e.g. ``['Content-Type']``). @@ -36,6 +36,52 @@ In code below there is a list of all supported configuration options and their d - ``exposeHeaders`` (``array``) - controls the value of ``Access-Control-Expose-Headers``. -- ``maxAge`` (``int``) - controls the value of ``Access-Control-Max-Age``. +- ``maxAge`` (``int`` or ``null``) - controls the value of ``Access-Control-Max-Age``. The default ``0`` disables preflight caching. Set a positive number of seconds to allow caching (e.g. ``600``), or ``null`` to omit the header and let the browser apply its default. - ``originRegex`` (``boolean``) - indicates if values from ``allowOrigin`` should be treated as regular expression. + +Request and response handling +============================= + +An allowed cross-origin request receives ``Access-Control-Allow-Origin`` containing its request origin. +This also applies when ``allowOrigin`` is ``*``; t3api reflects the concrete origin so that +``allowCredentials`` can work with the same configuration. Combining ``*`` with ``allowCredentials=true`` +allows every origin to read credentialed responses. Use an explicit list of trusted origins for private APIs. +An origin outside the configured policy receives no CORS permission headers. + +A preflight is an ``OPTIONS`` request containing both ``Origin`` and ``Access-Control-Request-Method``. +It succeeds only when the origin, requested method and requested headers are allowed. +A disallowed origin returns ``403``; a disallowed method or header returns ``405``. Rejected preflights have +an empty body and no CORS permission headers. Ordinary ``OPTIONS`` without preflight headers returns an empty +successful response for an existing endpoint. Cross-origin ``OPTIONS`` without ``Access-Control-Request-Method`` +uses the policy for an actual request. + +Header names are compared without regard to case, and surrounding whitespace in header lists is ignored. +With ``allowHeaders`` set to ``*``, t3api returns the concrete requested header names, including +``Authorization``, instead of a literal wildcard. The configured ``simpleHeaders`` list is merged into +``allowHeaders``; by default it includes ``Accept``, ``Accept-Language``, ``Content-Language``, ``Origin`` +and the configured language header. Add ``Content-Type`` to ``allowHeaders`` for JSON requests. +Method, allowed-header and exposed-header lists are trimmed and deduplicated; null and empty entries are ignored. +Methods are normalized to uppercase and header names to lowercase. + +Responses include ``Vary: Origin``, including requests without an origin and requests from disallowed origins. +Preflight responses also vary by ``Access-Control-Request-Method`` and ``Access-Control-Request-Headers``. +Existing ``Vary`` values are preserved. These headers tell HTTP caches which request headers affect the response, +as described in the `Fetch specification `__. +t3api's response cache stores the serialized content; CORS headers are evaluated for each request. + +CORS controls whether a browser can read a response. It does not authorize an operation or provide CSRF +protection for writes authenticated with cookies. Configure operation access rules and CSRF protection separately. + +Customizing CORS handling +========================= + +``CorsService`` provides the shared policy for ``CorsProcessor`` and ``OptionsOperationHandler``. +The protected ``CorsProcessor::isCorsRequest()`` and ``CorsProcessor::isPreflightRequest()`` hooks control only +whether that processor applies headers. They do not control the subsequent operation handler: +``OptionsOperationHandler`` independently uses ``CorsService::isPreflightRequest()``. + +To customize preflight detection consistently, supply the same custom ``CorsService`` implementation to both +components through dependency injection, and leave the processor hook delegating to that service. + +For release changes, see :ref:`changelog`. diff --git a/Documentation/Miscellaneous/Changelog/Index.rst b/Documentation/Miscellaneous/Changelog/Index.rst index 96fb2a1f..e498a46f 100644 --- a/Documentation/Miscellaneous/Changelog/Index.rst +++ b/Documentation/Miscellaneous/Changelog/Index.rst @@ -6,6 +6,7 @@ Changelog 5.0.0 ===== +- [!!!] CORS refactor: unify response handling, normalize configuration lists and improve preflight and cache behavior. Origin patterns now match complete origins. Review ``allowOrigin`` configuration and custom ``OPTIONS`` response handling when upgrading. See :ref:`cors`. - [!!!] Embed ``goldspecdigital/oooas`` library (PHP 8.5 compatibility). The package is no longer a dependency, namespace ``GoldSpecDigital\ObjectOrientedOAS`` is renamed to ``SourceBroker\T3api\OpenApi``. Update imports in custom filters implementing ``OpenApiSupportingFilterInterface`` and custom :ref:`collection response classes `. [`commit `__] - [!!!] ``@T3api\Serializer\Type\PasswordHash`` returns ``null`` on serialization instead of the stored password hash. Use ``@T3api\Serializer\Type\PasswordHash(serialize=true)`` to return the hash as before, see :ref:`serialization_handlers_password-hash`. [`commit `__] - [!!!] Refactor expression evaluation in access checkers. Protected method ``AbstractAccessChecker::getExpressionLanguageResolver()`` is removed, use ``evaluateExpression()`` instead. [`commit `__] diff --git a/Tests/Functional/Dispatcher/CorsDispatcherTest.php b/Tests/Functional/Dispatcher/CorsDispatcherTest.php new file mode 100644 index 00000000..130a9270 --- /dev/null +++ b/Tests/Functional/Dispatcher/CorsDispatcherTest.php @@ -0,0 +1,156 @@ + ['https://client.example', 'https://second.example'], + 'allowCredentials' => true, + 'allowMethods' => ['GET', 'POST'], + 'allowHeaders' => ['Content-Type'], + 'maxAge' => 600, + ]; + } + + #[Test] + #[DataProvider('optionsProvider')] + public function optionsRequestsDistinguishPreflightFromOrdinaryOptions( + array $headers, + int $status, + string $allowedOrigin, + bool $preflightAllowed + ): void { + $response = $this->dispatch('OPTIONS', '/_api/articles', $headers); + + self::assertSame($status, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame('', (string)$response->getBody()); + self::assertSame($allowedOrigin, $response->getHeaderLine('Access-Control-Allow-Origin')); + self::assertSame($allowedOrigin !== '' ? 'true' : '', $response->getHeaderLine('Access-Control-Allow-Credentials')); + self::assertSame($preflightAllowed ? 'GET, POST' : '', $response->getHeaderLine('Access-Control-Allow-Methods')); + self::assertSame($preflightAllowed ? 'content-type' : '', $response->getHeaderLine('Access-Control-Allow-Headers')); + self::assertSame('Origin, Access-Control-Request-Method, Access-Control-Request-Headers', $response->getHeaderLine('Vary')); + self::assertSame(0, $this->getConnectionPool()->getConnectionForTable('tx_functionaltest_domain_model_article') + ->count('*', 'tx_functionaltest_domain_model_article', [])); + } + + public static function optionsProvider(): array + { + $preflight = ['Origin' => 'https://client.example', 'Access-Control-Request-Method' => 'POST']; + + return [ + 'plain OPTIONS' => [[], 200, '', false], + 'OPTIONS with Origin only' => [['Origin' => 'https://client.example'], 200, 'https://client.example', false], + 'OPTIONS without Origin' => [['Access-Control-Request-Method' => 'POST'], 200, '', false], + 'valid preflight' => [$preflight + ['Access-Control-Request-Headers' => 'Content-Type'], 200, 'https://client.example', true], + 'denied origin' => [array_replace($preflight, ['Origin' => 'https://untrusted.example']), 403, '', false], + 'denied method' => [array_replace($preflight, ['Access-Control-Request-Method' => 'DELETE']), 405, '', false], + 'denied header' => [$preflight + ['Access-Control-Request-Headers' => 'X-Forbidden'], 405, '', false], + ]; + } + + #[Test] + public function cachedResponseUsesOriginOfCurrentRequest(): void + { + $this->importCSVDataSet(__DIR__ . '/../Fixtures/books.csv'); + $this->importCSVDataSet(__DIR__ . '/../Fixtures/authors.csv'); + $initial = $this->dispatch('GET', '/_api/books'); + self::assertSame(200, $initial->getStatusCode(), (string)$initial->getBody()); + self::assertStringContainsString('First book', (string)$initial->getBody()); + self::assertFalse($initial->hasHeader('Access-Control-Allow-Origin')); + self::assertSame('Origin', $initial->getHeaderLine('Vary')); + + // Prove later requests use cached content, while their CORS policy is evaluated afresh. + $this->getConnectionPool()->getConnectionForTable('tx_functionaltest_domain_model_book')->update( + 'tx_functionaltest_domain_model_book', + ['title' => 'Changed after warming cache'], + ['uid' => 1] + ); + $this->get(PersistenceManagerInterface::class)->clearState(); + + foreach (['https://client.example', 'https://second.example', 'https://untrusted.example'] as $origin) { + $response = $this->dispatch('GET', '/_api/books', ['Origin' => $origin]); + self::assertSame(200, $response->getStatusCode(), (string)$response->getBody()); + self::assertSame((string)$initial->getBody(), (string)$response->getBody()); + self::assertSame( + $origin === 'https://untrusted.example' ? '' : $origin, + $response->getHeaderLine('Access-Control-Allow-Origin') + ); + self::assertSame( + $origin === 'https://untrusted.example' ? '' : 'true', + $response->getHeaderLine('Access-Control-Allow-Credentials') + ); + self::assertSame('Origin', $response->getHeaderLine('Vary')); + } + } + + #[Test] + public function errorResponsesRemainReadableByAllowedOrigins(): void + { + $response = $this->dispatch('GET', '/_api/no-such-resource', ['Origin' => 'https://client.example']); + + self::assertSame(404, $response->getStatusCode()); + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + self::assertSame('true', $response->getHeaderLine('Access-Control-Allow-Credentials')); + self::assertSame('Origin', $response->getHeaderLine('Vary')); + } + + #[Test] + public function legacyOriginRegexDoesNotGrantPreflightToAppendedDomain(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['originRegex'] = true; + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowOrigin'] = ['http://.*\\.example\\.com']; + $allowed = $this->dispatch('OPTIONS', '/_api/articles', [ + 'Origin' => 'http://client.example.com', + 'Access-Control-Request-Method' => 'POST', + ]); + self::assertSame(200, $allowed->getStatusCode()); + self::assertSame('http://client.example.com', $allowed->getHeaderLine('Access-Control-Allow-Origin')); + self::assertSame('true', $allowed->getHeaderLine('Access-Control-Allow-Credentials')); + + $denied = $this->dispatch('OPTIONS', '/_api/articles', [ + 'Origin' => 'http://client.example.com.attacker.test', + 'Access-Control-Request-Method' => 'POST', + ]); + self::assertSame(403, $denied->getStatusCode()); + self::assertFalse($denied->hasHeader('Access-Control-Allow-Origin')); + self::assertFalse($denied->hasHeader('Access-Control-Allow-Credentials')); + } + + private function dispatch(string $method, string $path, array $headers = []): ResponseInterface + { + // Each call models a separate HTTP request, with a fresh response body. + $dispatcher = new Bootstrap( + $this->get(SerializerService::class), + $this->get(ApiResourceRepository::class), + $this->get(SerializationContextBuilder::class), + $this->get(DeserializationContextBuilder::class), + $this->get(EventDispatcherInterface::class), + $this->get(OperationResponseCache::class) + ); + $request = new ServerRequest('https://example.com' . $path, $method); + foreach ($headers as $name => $value) { + $request = $request->withHeader($name, $value); + } + + return $dispatcher->process($request); + } +} diff --git a/Tests/Unit/Processor/CorsProcessorTest.php b/Tests/Unit/Processor/CorsProcessorTest.php new file mode 100644 index 00000000..9d13f42e --- /dev/null +++ b/Tests/Unit/Processor/CorsProcessorTest.php @@ -0,0 +1,62 @@ + ['https://client.example']]; + $processor = new class (new CorsService()) extends CorsProcessor { + protected function isCorsRequest(Request $request): bool + { + return false; + } + }; + $request = Request::create('https://api.example/items'); + $request->headers->set('Origin', 'https://client.example'); + $response = new Response(); + $processor->process($request, $response); + + self::assertFalse($response->hasHeader('Access-Control-Allow-Origin')); + self::assertSame('Origin', $response->getHeaderLine('Vary')); + } + + #[Test] + public function subclassCanIncludeSameOriginRequestsInCorsProcessing(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors'] = ['allowOrigin' => ['https://api.example']]; + $processor = new class (new CorsService()) extends CorsProcessor { + protected function isCorsRequest(Request $request): bool + { + return true; + } + }; + $request = Request::create('https://api.example/items'); + $request->headers->set('Origin', 'https://api.example'); + $response = new Response(); + $processor->process($request, $response); + + self::assertSame('https://api.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + } + + #[Test] + public function missingServiceLeavesResponseUnchanged(): void + { + $response = new Response(); + $original = $response; + (new CorsProcessor(null))->process(Request::create('https://api.example/items'), $response); + + self::assertSame($original, $response); + } +} diff --git a/Tests/Unit/Service/CorsServiceTest.php b/Tests/Unit/Service/CorsServiceTest.php new file mode 100644 index 00000000..3aeab657 --- /dev/null +++ b/Tests/Unit/Service/CorsServiceTest.php @@ -0,0 +1,293 @@ + ['https://client.example'], + 'allowCredentials' => true, + 'allowMethods' => ['GET', 'POST'], + 'allowHeaders' => ['Content-Type', 'X-Token'], + 'exposeHeaders' => ['X-Total-Count'], + 'maxAge' => 600, + ]; + } + + #[Test] + #[DataProvider('actualRequestProvider')] + public function actualResponsesFollowOriginPolicy(array $configuration, ?string $origin, bool $allowed): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors'] = array_replace( + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors'], + $configuration + ); + $request = Request::create('https://api.example/items'); + if ($origin !== null) { + $request->headers->set('Origin', $origin); + } + $response = (new CorsService())->applyActualResponse($request, new Response()); + + self::assertSame($allowed ? $origin : '', $response->getHeaderLine('Access-Control-Allow-Origin')); + self::assertSame($allowed ? 'true' : '', $response->getHeaderLine('Access-Control-Allow-Credentials')); + self::assertSame($allowed ? 'x-total-count' : '', $response->getHeaderLine('Access-Control-Expose-Headers')); + self::assertSame('Origin', $response->getHeaderLine('Vary')); + self::assertSame(200, $response->getStatusCode()); + } + + public static function actualRequestProvider(): array + { + return [ + 'allowed origin' => [[], 'https://client.example', true], + 'disallowed origin' => [[], 'https://untrusted.example', false], + 'missing origin' => [[], null, false], + 'empty origin' => [[], '', false], + 'same origin' => [[], 'https://api.example', false], + 'opaque origin denied by default' => [[], 'null', false], + 'CORS disabled' => [['allowOrigin' => []], 'https://client.example', false], + 'wildcard string' => [['allowOrigin' => '*'], 'https://other.example', true], + 'wildcard array' => [['allowOrigin' => ['*']], 'https://other.example', true], + 'anchored regex match' => [ + ['originRegex' => true, 'allowOrigin' => ['^https://[a-z]+\\.example$']], + 'https://client.example', true, + ], + 'anchored regex mismatch' => [ + ['originRegex' => true, 'allowOrigin' => ['^https://[a-z]+\\.example$']], + 'https://client.example.attacker.test', false, + ], + 'legacy unanchored regex match' => [ + ['originRegex' => true, 'allowOrigin' => ['http://.*\\.example\\.com']], + 'http://client.example.com', true, + ], + 'legacy unanchored regex rejects appended domain' => [ + ['originRegex' => true, 'allowOrigin' => ['http://.*\\.example\\.com']], + 'http://client.example.com.attacker.test', false, + ], + 'regex alternative match' => [ + ['originRegex' => true, 'allowOrigin' => ['https://client\\.example|https://second\\.example']], + 'https://second.example', true, + ], + 'first regex alternative rejects appended domain' => [ + ['originRegex' => true, 'allowOrigin' => ['https://client\\.example|https://second\\.example']], + 'https://client.example.attacker.test', false, + ], + 'second regex alternative rejects appended domain' => [ + ['originRegex' => true, 'allowOrigin' => ['https://client\\.example|https://second\\.example']], + 'https://second.example.attacker.test', false, + ], + 'regex must match the complete origin including scheme' => [ + ['originRegex' => true, 'allowOrigin' => ['client\\.example']], + 'https://client.example', false, + ], + ]; + } + + #[Test] + public function disallowedOriginRemovesExistingCorsHeadersWithoutChangingResponse(): void + { + $request = Request::create('https://api.example/items'); + $request->headers->set('Origin', 'https://untrusted.example'); + $response = new Response('php://temp', 201, [ + 'Access-Control-Allow-Origin' => '*', + 'Access-Control-Allow-Credentials' => 'true', + 'Access-Control-Expose-Headers' => 'X-Secret', + 'Access-Control-Allow-Methods' => 'POST', + 'Access-Control-Allow-Headers' => '*', + 'Access-Control-Max-Age' => '600', + 'Content-Type' => 'application/json', + ]); + $response->getBody()->write('{"created":true}'); + $response = (new CorsService())->applyActualResponse($request, $response); + + self::assertSame(201, $response->getStatusCode()); + self::assertSame('{"created":true}', (string)$response->getBody()); + self::assertSame(['Content-Type' => ['application/json'], 'Vary' => ['Origin']], $response->getHeaders()); + } + + #[Test] + public function disabledCredentialsAreNotAdvertised(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowCredentials'] = false; + $request = Request::create('https://api.example/items'); + $request->headers->set('Origin', 'https://client.example'); + $response = (new CorsService())->applyActualResponse( + $request, + new Response('php://temp', 200, ['Access-Control-Allow-Credentials' => 'true']) + ); + + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + self::assertFalse($response->hasHeader('Access-Control-Allow-Credentials')); + } + + #[Test] + #[DataProvider('varyProvider')] + public function varyPreservesExistingValues(string $existing, string $expected): void + { + $response = (new CorsService())->applyActualResponse( + Request::create('https://api.example/items'), + new Response('php://temp', 200, ['Vary' => $existing]) + ); + + self::assertSame($expected, $response->getHeaderLine('Vary')); + } + + public static function varyProvider(): array + { + return [ + 'append' => ['Accept-Encoding', 'Accept-Encoding, Origin'], + 'already present in different case' => ['Accept-Encoding, origin', 'Accept-Encoding, origin'], + 'wildcard' => ['*', '*'], + ]; + } + + #[Test] + public function preflightValidatesAndNormalizesMethodAndHeaderLists(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowMethods'] = [' get ', 'post', 'POST']; + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowHeaders'] = [' Content-Type ', 'X-Token']; + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['simpleHeaders'] = ['X-Locale']; + $request = $this->preflight(); + $request->headers->set('Access-Control-Request-Method', 'post'); + $request->headers->set('Access-Control-Request-Headers', "Content-Type , X-TOKEN,\tX-Locale"); + $response = (new CorsService())->applyPreflightResponse($request, new Response()); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame('https://client.example', $response->getHeaderLine('Access-Control-Allow-Origin')); + self::assertSame('true', $response->getHeaderLine('Access-Control-Allow-Credentials')); + self::assertSame('GET, POST, post', $response->getHeaderLine('Access-Control-Allow-Methods')); + self::assertSame('content-type, x-token, x-locale', $response->getHeaderLine('Access-Control-Allow-Headers')); + self::assertSame('600', $response->getHeaderLine('Access-Control-Max-Age')); + self::assertSame('Origin, Access-Control-Request-Method, Access-Control-Request-Headers', $response->getHeaderLine('Vary')); + self::assertFalse($response->hasHeader('Access-Control-Expose-Headers')); + } + + #[Test] + #[DataProvider('deniedPreflightProvider')] + public function deniedPreflightsDoNotAdvertisePermission(string $header, string $value, int $status): void + { + $request = $this->preflight(); + $request->headers->set($header, $value); + $response = (new CorsService())->applyPreflightResponse( + $request, + new Response('php://temp', 200, ['Access-Control-Allow-Origin' => '*', 'Access-Control-Allow-Credentials' => 'true']) + ); + + self::assertSame($status, $response->getStatusCode()); + self::assertSame(['Vary'], array_keys($response->getHeaders())); + } + + public static function deniedPreflightProvider(): array + { + return [ + 'origin' => ['Origin', 'https://untrusted.example', 403], + 'empty origin' => ['Origin', '', 403], + 'method' => ['Access-Control-Request-Method', 'DELETE', 405], + 'empty method' => ['Access-Control-Request-Method', '', 405], + 'header' => ['Access-Control-Request-Headers', 'Content-Type, X-Forbidden', 405], + ]; + } + + #[Test] + public function wildcardHeadersAreEchoedAsConcreteNamesForCredentialedRequests(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowHeaders'] = '*'; + $request = $this->preflight(); + $request->headers->set('Access-Control-Request-Headers', 'Authorization, X-Custom, authorization'); + $response = (new CorsService())->applyPreflightResponse($request, new Response()); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame('authorization, x-custom', $response->getHeaderLine('Access-Control-Allow-Headers')); + self::assertSame('true', $response->getHeaderLine('Access-Control-Allow-Credentials')); + } + + #[Test] + public function wildcardWithoutRequestedHeadersDoesNotEmitEmptyHeader(): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['allowHeaders'] = '*'; + $response = (new CorsService())->applyPreflightResponse($this->preflight(), new Response()); + + self::assertSame(200, $response->getStatusCode()); + self::assertFalse($response->hasHeader('Access-Control-Allow-Headers')); + } + + #[Test] + #[DataProvider('optionalListEntriesProvider')] + public function optionalListEntriesDoNotBreakCorsResponses(string $option, array $values, string $header, string $expected): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors'][$option] = $values; + $service = new CorsService(); + $request = Request::create('https://api.example/items'); + $request->headers->set('Origin', 'https://client.example'); + $actual = $service->applyActualResponse($request, new Response()); + $preflight = $service->applyPreflightResponse($this->preflight(), new Response()); + + self::assertSame(200, $actual->getStatusCode()); + self::assertSame(200, $preflight->getStatusCode()); + $response = $header === 'Access-Control-Expose-Headers' ? $actual : $preflight; + self::assertSame($expected, $response->getHeaderLine($header)); + } + + public static function optionalListEntriesProvider(): array + { + return [ + 'nullable allowed headers' => [ + 'allowHeaders', [null, ' Content-Type ', 'content-type', '', ' '], + 'Access-Control-Allow-Headers', 'content-type', + ], + 'nullable language header in simpleHeaders' => [ + 'simpleHeaders', [null, ' X-Locale ', 'x-locale', '', ' '], + 'Access-Control-Allow-Headers', 'content-type, x-token, x-locale', + ], + 'nullable allowed methods' => [ + 'allowMethods', [null, ' post ', 'POST', '', ' '], + 'Access-Control-Allow-Methods', 'POST', + ], + 'nullable exposed headers' => [ + 'exposeHeaders', [null, ' X-Total-Count ', 'x-total-count', '', ' '], + 'Access-Control-Expose-Headers', 'x-total-count', + ], + ]; + } + + #[Test] + #[DataProvider('maxAgeProvider')] + public function maxAgeRemainsConfigurable(?int $maxAge, string $expected): void + { + $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['cors']['maxAge'] = $maxAge; + $response = (new CorsService())->applyPreflightResponse($this->preflight(), new Response()); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame($expected, $response->getHeaderLine('Access-Control-Max-Age')); + self::assertSame($maxAge !== null, $response->hasHeader('Access-Control-Max-Age')); + } + + public static function maxAgeProvider(): array + { + return [ + 'omit header' => [null, ''], + 'disable preflight caching' => [0, '0'], + 'explicit lifetime' => [600, '600'], + ]; + } + + private function preflight(): Request + { + $request = Request::create('https://api.example/items', 'OPTIONS'); + $request->headers->set('Origin', 'https://client.example'); + $request->headers->set('Access-Control-Request-Method', 'POST'); + + return $request; + } +}