diff --git a/composer.json b/composer.json index b3de8371..e503edcb 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,6 @@ "doctrine/migrations": "^3.0.2", "doctrine/orm": "^2.8", "endroid/qr-code": "^3.6", - "friendsofphp/proxy-manager-lts": "^1.0", "geoip2/geoip2": "^2.9", "guzzlehttp/guzzle": "^7.0", "laminas/laminas-config": "^3.3", @@ -43,6 +42,7 @@ "mezzio/mezzio-swoole": "^2.6.4", "monolog/monolog": "^2.0", "nikolaposa/monolog-factory": "^3.1", + "ocramius/proxy-manager": "^2.11", "php-middleware/request-id": "^4.1", "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", diff --git a/config/autoload/cors.global.php b/config/autoload/cors.global.php new file mode 100644 index 00000000..58ad9428 --- /dev/null +++ b/config/autoload/cors.global.php @@ -0,0 +1,11 @@ + [ + 'max_age' => 3600, + ], + +]; diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index bdd9d3a9..8c1cdb8e 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -41,7 +41,7 @@ return [ ImplicitOptionsMiddleware::class => Middleware\EmptyResponseImplicitOptionsMiddlewareFactory::class, Middleware\BodyParserMiddleware::class => InvokableFactory::class, - Middleware\CrossDomainMiddleware::class => InvokableFactory::class, + Middleware\CrossDomainMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class, Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => ConfigAbstractFactory::class, @@ -76,6 +76,7 @@ return [ Action\Tag\UpdateTagAction::class => [TagService::class], Action\Domain\ListDomainsAction::class => [DomainService::class, 'config.url_shortener.domain.hostname'], + Middleware\CrossDomainMiddleware::class => ['config.cors'], Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'], Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => [ 'config.url_shortener.default_short_codes_length', diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index 171142a1..88d62904 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -17,6 +17,13 @@ use function implode; class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterface { + private array $config; + + public function __construct(array $config) + { + $this->config = $config; + } + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $response = $handler->handle($request); @@ -48,7 +55,7 @@ class CrossDomainMiddleware implements MiddlewareInterface, RequestMethodInterfa ]; $corsHeaders = [ 'Access-Control-Allow-Methods' => implode(',', $matchedMethods), - 'Access-Control-Max-Age' => '1000', + 'Access-Control-Max-Age' => $this->config['max_age'], 'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'), ]; diff --git a/module/Rest/test-api/Middleware/CorsTest.php b/module/Rest/test-api/Middleware/CorsTest.php new file mode 100644 index 00000000..4e060352 --- /dev/null +++ b/module/Rest/test-api/Middleware/CorsTest.php @@ -0,0 +1,83 @@ +callApiWithKey(self::METHOD_GET, '/short-urls'); + + self::assertEquals(200, $resp->getStatusCode()); + self::assertFalse($resp->hasHeader('Access-Control-Allow-Origin')); + self::assertFalse($resp->hasHeader('Access-Control-Expose-Headers')); + self::assertFalse($resp->hasHeader('Access-Control-Allow-Methods')); + self::assertFalse($resp->hasHeader('Access-Control-Max-Age')); + self::assertFalse($resp->hasHeader('Access-Control-Allow-Headers')); + } + + /** + * @test + * @dataProvider provideOrigins + */ + public function responseIncludesCorsHeadersIfOriginIsSent( + string $origin, + string $endpoint, + int $expectedStatusCode + ): void { + $resp = $this->callApiWithKey(self::METHOD_GET, $endpoint, [ + RequestOptions::HEADERS => ['Origin' => $origin], + ]); + + self::assertEquals($expectedStatusCode, $resp->getStatusCode()); + self::assertEquals($origin, $resp->getHeaderLine('Access-Control-Allow-Origin')); + self::assertEquals('X-Api-Key', $resp->getHeaderLine('Access-Control-Expose-Headers')); + self::assertFalse($resp->hasHeader('Access-Control-Allow-Methods')); + self::assertFalse($resp->hasHeader('Access-Control-Max-Age')); + self::assertFalse($resp->hasHeader('Access-Control-Allow-Headers')); + } + + public function provideOrigins(): iterable + { + yield 'foo.com' => ['foo.com', '/short-urls', 200]; + yield 'bar.io' => ['bar.io', '/foo/bar', 404]; + yield 'baz.dev' => ['baz.dev', '/short-urls', 200]; + } + + /** + * @test + * @dataProvider providePreflightEndpoints + */ + public function preflightRequestsIncludeExtraCorsHeaders(string $endpoint, string $expectedAllowedMethods): void + { + $allowedHeaders = 'Authorization'; + $resp = $this->callApiWithKey(self::METHOD_OPTIONS, $endpoint, [ + RequestOptions::HEADERS => [ + 'Origin' => 'foo.com', + 'Access-Control-Request-Headers' => $allowedHeaders, + ], + ]); + + self::assertEquals(204, $resp->getStatusCode()); + self::assertTrue($resp->hasHeader('Access-Control-Allow-Origin')); + self::assertTrue($resp->hasHeader('Access-Control-Expose-Headers')); + self::assertTrue($resp->hasHeader('Access-Control-Max-Age')); + self::assertEquals($expectedAllowedMethods, $resp->getHeaderLine('Access-Control-Allow-Methods')); + self::assertEquals($allowedHeaders, $resp->getHeaderLine('Access-Control-Allow-Headers')); + } + + 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 + } +} diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 03675fce..72e95a36 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -26,7 +26,7 @@ class CrossDomainMiddlewareTest extends TestCase public function setUp(): void { - $this->middleware = new CrossDomainMiddleware(); + $this->middleware = new CrossDomainMiddleware(['max_age' => 1000]); $this->handler = $this->prophesize(RequestHandlerInterface::class); }