Skip to content

Commit

Permalink
Merge pull request #331 from martijnc/feature/external-redirect-response
Browse files Browse the repository at this point in the history
Introduce `ExternalRedirectResponse`
  • Loading branch information
Seldaek committed Apr 10, 2024
2 parents b9b68b4 + 2e021f2 commit 6a6c75e
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/EventListener/ExternalRedirectListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Nelmio\SecurityBundle\EventListener;

use Nelmio\SecurityBundle\ExternalRedirect\AllowListBasedTargetValidator;
use Nelmio\SecurityBundle\ExternalRedirect\ExternalRedirectResponse;
use Nelmio\SecurityBundle\ExternalRedirect\TargetValidator;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
Expand Down Expand Up @@ -86,6 +87,13 @@ public function onKernelResponse(ResponseEvent $e): void
return;
}

if ($response instanceof ExternalRedirectResponse) {
$targetValidator = new AllowListBasedTargetValidator($response->getAllowedHosts());
if ($targetValidator->isTargetAllowed($target)) {
return;
}
}

if (null !== $this->targetValidator && $this->targetValidator->isTargetAllowed($target)) {
return;
}
Expand Down
42 changes: 42 additions & 0 deletions src/ExternalRedirect/ExternalRedirectResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <hello@nelm.io>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ExternalRedirect;

use Symfony\Component\HttpFoundation\RedirectResponse;

class ExternalRedirectResponse extends RedirectResponse
{
/**
* @var string[]
*/
private array $allowedHosts;

/**
* @param string[] $allowedHosts
* @param string[] $headers
*/
public function __construct(string $url, array $allowedHosts, int $status = 302, array $headers = [])
{
$this->allowedHosts = $allowedHosts;
parent::__construct($url, $status, $headers);
}

/**
* @return string[]
*/
public function getAllowedHosts(): array
{
return $this->allowedHosts;
}
}
26 changes: 26 additions & 0 deletions src/Resources/doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,32 @@ subdomains if needed.
- twitter.com
- facebook.com
If you have a controller that can redirect to another host, you can also use `ExternalRedirectResponse` to allow the
redirect without having to configure the hosts globally. Any hosts passed to `ExternalRedirectResponse` are in
addition to those already configured globally.

.. code-block:: yaml
# config/packages/nelmio_security.yaml
nelmio_security:
external_redirects:
abort: true
allow_list:
- bar.com
.. code-block:: php
use Nelmio\SecurityBundle\ExternalRedirect\ExternalRedirectResponse;
// Will be allowed even though "foo.com" is not allowed globally through the config.
return new ExternalRedirectResponse('https://foo.com', ['foo.com', 'auth-provider.test']);
// Will not be allowed.
return new ExternalRedirectResponse('https://not-allowed.com', ['foo.com', 'auth-provider.test']);
// Will be allowed because "bar.com" is allowed globally through the config.
return new ExternalRedirectResponse('https://bar.com', ['foo.com', 'auth-provider.test']);
Forced HTTPS/SSL Handling
-------------------------

Expand Down
44 changes: 44 additions & 0 deletions tests/Listener/ExternalRedirectListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use Nelmio\SecurityBundle\EventListener\ExternalRedirectListener;
use Nelmio\SecurityBundle\ExternalRedirect\AllowListBasedTargetValidator;
use Nelmio\SecurityBundle\ExternalRedirect\ExternalRedirectResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\HttpException;
Expand Down Expand Up @@ -110,6 +111,31 @@ public function testRedirectSkipsAllowedTargets(): void
$this->assertTrue($response->isRedirect());
}

public function testExternalRedirectResponseSkipsAllowedTargets(): void
{
$listener = new ExternalRedirectListener(true, null, null, new AllowListBasedTargetValidator(['bar.com']));

$response = $this->filterRedirectResponse($listener, 'http://foo.com/', 'http://allowed.com', ['allowed.com']);
$this->assertTrue($response->isRedirect());
}

public function testExternalRedirectResponseSkipsGlobalAllowedTargets(): void
{
$listener = new ExternalRedirectListener(true, null, null, new AllowListBasedTargetValidator(['bar.com']));

$response = $this->filterRedirectResponse($listener, 'http://foo.com/', 'http://bar.com', ['allowed.com']);
$this->assertTrue($response->isRedirect());
}

public function testExternalRedirectResponseAborts(): void
{
$this->expectException(HttpException::class);
$listener = new ExternalRedirectListener(true, null, null, new AllowListBasedTargetValidator(['bar.com']));

$response = $this->filterRedirectResponse($listener, 'http://foo.com/', 'http://not-allowed.com', ['allowed.com']);
$this->assertTrue($response->isRedirect());
}

/**
* @dataProvider provideRedirectAllowedListFailing
*
Expand Down Expand Up @@ -184,4 +210,22 @@ private function filterResponse(ExternalRedirectListener $listener, string $sour

return $response;
}

/**
* @param string[] $allowedHosts
*/
private function filterRedirectResponse(
ExternalRedirectListener $listener,
string $source,
string $target,
array $allowedHosts
): ExternalRedirectResponse {
$request = Request::create($source);
$response = new ExternalRedirectResponse($target, $allowedHosts);

$event = $this->createResponseEvent($request, true, $response);
$listener->onKernelResponse($event);

return $response;
}
}

0 comments on commit 6a6c75e

Please sign in to comment.