Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Private Network Access #190

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support Private Network Access #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Expand Up @@ -44,6 +44,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->append($this->getAllowOrigin())
->append($this->getAllowHeaders())
->append($this->getAllowMethods())
->append($this->getAllowPrivateNetwork())
->append($this->getExposeHeaders())
->append($this->getMaxAge())
->append($this->getHosts())
Expand All @@ -60,6 +61,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->append($this->getAllowOrigin())
->append($this->getAllowHeaders())
->append($this->getAllowMethods())
->append($this->getAllowPrivateNetwork())
->append($this->getExposeHeaders())
->append($this->getMaxAge())
->append($this->getHosts())
Expand Down Expand Up @@ -137,6 +139,14 @@ private function getAllowMethods(): ArrayNodeDefinition
return $node;
}

private function getAllowPrivateNetwork(): BooleanNodeDefinition
{
$node = new BooleanNodeDefinition('allow_private_network');
$node->defaultFalse();

return $node;
}

private function getExposeHeaders(): ArrayNodeDefinition
{
$node = new ArrayNodeDefinition('expose_headers');
Expand Down
1 change: 1 addition & 0 deletions DependencyInjection/NelmioCorsExtension.php
View file
Open in desktop
Expand Up @@ -34,6 +34,7 @@ public function load(array $configs, ContainerBuilder $container): void
'allow_origin' => [],
'allow_credentials' => false,
'allow_headers' => [],
'allow_private_network' => false,
'expose_headers' => [],
'allow_methods' => [],
'max_age' => 0,
Expand Down
19 changes: 18 additions & 1 deletion EventListener/CorsListener.php
View file
Open in desktop
Expand Up @@ -95,7 +95,10 @@ public function onKernelRequest(RequestEvent $event): void
}

// perform preflight checks
if ('OPTIONS' === $request->getMethod() && $request->headers->has('Access-Control-Request-Method')) {
if ('OPTIONS' === $request->getMethod() &&
($request->headers->has('Access-Control-Request-Method') ||
$request->headers->has('Access-Control-Request-Private-Network'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it allowed to have the Access-Control-Request-Private-Network header without Access-Control-Request-Method?

) {
$this->logger->debug("Request is a preflight check, setting event response now.");

$event->setResponse($this->getPreflightResponse($request, $options));
Expand Down Expand Up @@ -217,6 +220,20 @@ protected function getPreflightResponse(Request $request, array $options): Respo

$response->headers->set('Access-Control-Allow-Origin', $origin);

// check private network access
if ($request->headers->has('Access-Control-Request-Private-Network')
&& strtolower($request->headers->get('Access-Control-Request-Private-Network')) === 'true'
Comment on lines +224 to +225
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($request->headers->has('Access-Control-Request-Private-Network')
&& strtolower($request->headers->get('Access-Control-Request-Private-Network')) === 'true'
if (strtolower($request->headers->get('Access-Control-Request-Private-Network')) === 'true'

and, I'm not sure non lowercase true is allowed

) {
if ($options['allow_private_network']) {
$this->logger->debug("Setting 'Access-Control-Allow-Private-Network' response header to 'true'.");

$response->headers->set('Access-Control-Allow-Private-Network', 'true');
} else {
$response->setStatusCode(400);
$response->setContent('Private Network Access is not allowed.');
}
}

// check request method
$method = strtoupper($request->headers->get('Access-Control-Request-Method'));
if (!in_array($method, $options['allow_methods'], true)) {
Expand Down
1 change: 1 addition & 0 deletions Options/ProviderInterface.php
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@ interface ProviderInterface
* - bool allow_credentials
* - bool allow_origin
* - bool allow_headers
* - bool allow_private_network
* - bool origin_regex
* - array allow_methods
* - array expose_headers
Expand Down
3 changes: 2 additions & 1 deletion Resources/doc/index.rst
View file
Open in desktop
Expand Up @@ -77,6 +77,7 @@ seconds.
allow_origin: []
allow_headers: []
allow_methods: []
allow_private_network: false
expose_headers: []
max_age: 0
hosts: []
Expand All @@ -102,7 +103,7 @@ the allowed methods however have to be explicitly listed. ``paths`` must
contain at least one item.

``expose_headers`` can be set to ``*`` to accept any value as long as
``allow_credentials`` is ``false`` `as per the specification`_.
``allow_credentials`` and ``allow_private_network`` are ``false`` `as per the specification`_.

If ``origin_regex`` is set, ``allow_origin`` must be a list of regular
expressions matching allowed origins. Remember to use ``^`` and ``$`` to
Expand Down
76 changes: 76 additions & 0 deletions Tests/CorsListenerTest.php
View file
Open in desktop
Expand Up @@ -36,6 +36,7 @@ public function getListener(array $options = []): CorsListener
'allow_headers' => [],
'expose_headers' => [],
'allow_methods' => [],
'allow_private_network' => false,
'max_age' => 0,
'hosts' => [],
'origin_regex' => false,
Expand Down Expand Up @@ -259,4 +260,79 @@ public function testRequestWithForcedAllowOriginValue(): void
$this->assertEquals(200, $resp->getStatusCode());
$this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin'));
}

/**
* @param bool $option
* @param string|null $header
* @param string|null $expectedHeader
* @param int $expectedStatus
*/
private function testPreflightedRequestWithPrivateNetworkAccess($option, $header, $expectedHeader, $expectedStatus): void
{
$options = [
'allow_origin' => [true],
'allow_headers' => ['foo', 'bar'],
'allow_methods' => ['POST', 'PUT'],
'allow_private_network' => $option,
];

// preflight
$req = Request::create('/foo', 'OPTIONS');
$req->headers->set('Origin', 'http://example.com');
$req->headers->set('Access-Control-Request-Method', 'POST');
$req->headers->set('Access-Control-Request-Headers', 'Foo, BAR');
if ($header) {
$req->headers->set('Access-Control-Request-Private-Network', $header);
}

$dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
$event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
$this->getListener($options)->onKernelRequest($event);
$resp = $event->getResponse();
$this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
$this->assertEquals($expectedStatus, $resp->getStatusCode());
$this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin'));
$this->assertEquals('POST, PUT', $resp->headers->get('Access-Control-Allow-Methods'));
$this->assertEquals('foo, bar', $resp->headers->get('Access-Control-Allow-Headers'));
$this->assertEquals($expectedHeader, $resp->headers->get('Access-Control-Allow-Private-Network'));
$this->assertEquals(['Origin'], $resp->getVary());

// actual request
$req = Request::create('/foo', 'POST');
$req->headers->set('Origin', 'http://example.com');
$req->headers->set('Foo', 'huh');
$req->headers->set('BAR', 'lala');

$event = new RequestEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
$this->getListener($options)->onKernelRequest($event);
$event = new ResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, new Response());
$this->getListener($options)->onKernelResponse($event);
$resp = $event->getResponse();
$this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
$this->assertEquals(200, $resp->getStatusCode());
$this->assertEquals('http://example.com', $resp->headers->get('Access-Control-Allow-Origin'));
$this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Methods'));
$this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Headers'));
$this->assertEquals(null, $resp->headers->get('Access-Control-Allow-Private-Network'));
}

public function testPreflightedRequestWithPrivateNetworkAccessAllowedAndProvided(): void
{
$this->testPreflightedRequestWithPrivateNetworkAccess(true, 'true', 'true', 200);
}

public function testPreflightedRequestWithPrivateNetworkAccessAllowedButNotProvided(): void
{
$this->testPreflightedRequestWithPrivateNetworkAccess(true, null, null, 200);
}

public function testPreflightedRequestWithPrivateNetworkAccessForbiddenButProvided(): void
{
$this->testPreflightedRequestWithPrivateNetworkAccess(false, 'true', null, 400);
}

public function testPreflightedRequestWithPrivateNetworkAccessForbiddenAndNotProvided(): void
{
$this->testPreflightedRequestWithPrivateNetworkAccess(false, null, null, 200);
}
}
6 changes: 6 additions & 0 deletions Tests/Options/ConfigProviderTest.php
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ class ConfigProviderTest extends TestCase
'allow_credentials' => false,
'allow_origin' => ['http://one.example.com'],
'allow_headers' => false,
'allow_private_network' => false,
'allow_methods' => ['GET'],
'expose_headers' => [],
'max_age' => 0,
Expand All @@ -29,6 +30,7 @@ class ConfigProviderTest extends TestCase
'allow_credentials' => true,
'allow_origin' => ['http://two.example.com'],
'allow_headers' => true,
'allow_private_network' => false,
'allow_methods' => ['PUT', 'POST'],
'expose_headers' => ['X-CorsTest'],
'max_age' => 120,
Expand All @@ -39,6 +41,7 @@ class ConfigProviderTest extends TestCase
'allow_credentials' => true,
'allow_origin' => ['http://domainmatch.example.com'],
'allow_headers' => true,
'allow_private_network' => false,
'allow_methods' => ['PUT', 'POST'],
'expose_headers' => [],
'max_age' => 160,
Expand All @@ -49,6 +52,7 @@ class ConfigProviderTest extends TestCase
'allow_credentials' => true,
'allow_origin' => ['http://nomatch.example.com'],
'allow_headers' => true,
'allow_private_network' => false,
'allow_methods' => ['PUT', 'POST'],
'expose_headers' => ['X-CorsTest'],
'max_age' => 180,
Expand All @@ -60,6 +64,7 @@ class ConfigProviderTest extends TestCase
'allow_origin' => ['^http://(.*)\.example\.com'],
'origin_regex' => true,
'allow_headers' => true,
'allow_private_network' => false,
'allow_methods' => ['PUT', 'POST'],
'expose_headers' => [],
'max_age' => 0,
Expand Down Expand Up @@ -138,6 +143,7 @@ protected function getProvider(): ConfigProvider
'allow_credentials' => true,
'allow_origin' => ['http://nope.example.com'],
'allow_headers' => true,
'allow_private_network' => false,
'allow_methods' => ['COPY'],
'expose_headers' => ['X-Cors-Nope'],
'max_age' => 42,
Expand Down