OilfieldToolsNet/shlink
4
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2024-12-22 15:13:59 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fixed CrossDomainMiddleware not inferring proper allowed methods

Browse Source
This commit is contained in:
Alejandro Celaya 2021-01-24 13:49:57 +01:00
parent ef54caab85
commit f585cfe02e
3 changed files with 32 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
module/Rest/src/Middleware/CrossDomainMiddleware.php
View File

@ -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;
@ -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,
]);
}
}

9
module/Rest/test-api/Middleware/CorsTest.php
View File

@ -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'];
}
}

26
module/Rest/test/Middleware/CrossDomainMiddlewareTest.php
View File

@ -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;
@ -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'];
}
/**