bug #23580 Fix login redirect when referer contains a query string (fabpot)
This PR was squashed before being merged into the 2.7 branch (closes #23580). Discussion ---------- Fix login redirect when referer contains a query string | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19026, #23027, #23061, #23411, #23551 | License | MIT | Doc PR | n/a In 3.3, #19026 was merged to fix a bug that should have been fixed in 2.7. The fix was wrong anyway, so this PR fixes it the proper way. The first two commits refactors test (using mocks for data objects is a bad idea and using too many mocks actually makes tests test nothing). The actual fix is in the third commit. Commits -------022ac0be09
[Security] added more tests9c7a1406cb
[Security] fixed default target path when referer contains a query stringb1f1ae26b4
[Security] simplified tests3387612451
[Security] refactored tests
This commit is contained in:
commit
f4172b0bff
@ -118,8 +118,14 @@ class DefaultAuthenticationSuccessHandler implements AuthenticationSuccessHandle
|
|||||||
return $targetUrl;
|
return $targetUrl;
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->options['use_referer'] && ($targetUrl = $request->headers->get('Referer')) && $targetUrl !== $this->httpUtils->generateUri($request, $this->options['login_path'])) {
|
if ($this->options['use_referer']) {
|
||||||
return $targetUrl;
|
$targetUrl = $request->headers->get('Referer');
|
||||||
|
if (false !== $pos = strpos($targetUrl, '?')) {
|
||||||
|
$targetUrl = substr($targetUrl, 0, $pos);
|
||||||
|
}
|
||||||
|
if ($targetUrl !== $this->httpUtils->generateUri($request, $this->options['login_path'])) {
|
||||||
|
return $targetUrl;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return $this->options['default_target_path'];
|
return $this->options['default_target_path'];
|
||||||
|
@ -12,159 +12,92 @@
|
|||||||
namespace Symfony\Component\Security\Http\Tests\Authentication;
|
namespace Symfony\Component\Security\Http\Tests\Authentication;
|
||||||
|
|
||||||
use PHPUnit\Framework\TestCase;
|
use PHPUnit\Framework\TestCase;
|
||||||
use Symfony\Component\HttpFoundation\Response;
|
use Symfony\Component\HttpFoundation\Request;
|
||||||
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
|
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
|
||||||
|
use Symfony\Component\Security\Http\HttpUtils;
|
||||||
|
|
||||||
class DefaultAuthenticationSuccessHandlerTest extends TestCase
|
class DefaultAuthenticationSuccessHandlerTest extends TestCase
|
||||||
{
|
{
|
||||||
private $httpUtils = null;
|
/**
|
||||||
|
* @dataProvider getRequestRedirections
|
||||||
private $request = null;
|
*/
|
||||||
|
public function testRequestRedirections(Request $request, $options, $redirectedUrl)
|
||||||
private $token = null;
|
|
||||||
|
|
||||||
protected function setUp()
|
|
||||||
{
|
{
|
||||||
$this->httpUtils = $this->getMockBuilder('Symfony\Component\Security\Http\HttpUtils')->getMock();
|
$urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock();
|
||||||
$this->request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request')->getMock();
|
$urlGenerator->expects($this->any())->method('generate')->will($this->returnValue('http://localhost/login'));
|
||||||
$this->request->headers = $this->getMockBuilder('Symfony\Component\HttpFoundation\HeaderBag')->getMock();
|
$httpUtils = new HttpUtils($urlGenerator);
|
||||||
$this->token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
|
$token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
|
||||||
|
$handler = new DefaultAuthenticationSuccessHandler($httpUtils, $options);
|
||||||
|
if ($request->hasSession()) {
|
||||||
|
$handler->setProviderKey('admin');
|
||||||
|
}
|
||||||
|
$this->assertSame('http://localhost'.$redirectedUrl, $handler->onAuthenticationSuccess($request, $token)->getTargetUrl());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testRequestIsRedirected()
|
public function getRequestRedirections()
|
||||||
{
|
|
||||||
$response = $this->expectRedirectResponse('/');
|
|
||||||
|
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, array());
|
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
|
||||||
|
|
||||||
$this->assertSame($response, $result);
|
|
||||||
}
|
|
||||||
|
|
||||||
public function testDefaultTargetPathCanBeForced()
|
|
||||||
{
|
|
||||||
$options = array(
|
|
||||||
'always_use_default_target_path' => true,
|
|
||||||
'default_target_path' => '/dashboard',
|
|
||||||
);
|
|
||||||
|
|
||||||
$response = $this->expectRedirectResponse('/dashboard');
|
|
||||||
|
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, $options);
|
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
|
||||||
|
|
||||||
$this->assertSame($response, $result);
|
|
||||||
}
|
|
||||||
|
|
||||||
public function testTargetPathIsPassedWithRequest()
|
|
||||||
{
|
|
||||||
$this->request->expects($this->once())
|
|
||||||
->method('get')->with('_target_path')
|
|
||||||
->will($this->returnValue('/dashboard'));
|
|
||||||
|
|
||||||
$response = $this->expectRedirectResponse('/dashboard');
|
|
||||||
|
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, array());
|
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
|
||||||
|
|
||||||
$this->assertSame($response, $result);
|
|
||||||
}
|
|
||||||
|
|
||||||
public function testTargetPathParameterIsCustomised()
|
|
||||||
{
|
|
||||||
$options = array('target_path_parameter' => '_my_target_path');
|
|
||||||
|
|
||||||
$this->request->expects($this->once())
|
|
||||||
->method('get')->with('_my_target_path')
|
|
||||||
->will($this->returnValue('/dashboard'));
|
|
||||||
|
|
||||||
$response = $this->expectRedirectResponse('/dashboard');
|
|
||||||
|
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, $options);
|
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
|
||||||
|
|
||||||
$this->assertSame($response, $result);
|
|
||||||
}
|
|
||||||
|
|
||||||
public function testTargetPathIsTakenFromTheSession()
|
|
||||||
{
|
{
|
||||||
$session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock();
|
$session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock();
|
||||||
$session->expects($this->once())
|
$session->expects($this->once())->method('get')->with('_security.admin.target_path')->will($this->returnValue('/admin/dashboard'));
|
||||||
->method('get')->with('_security.admin.target_path')
|
$session->expects($this->once())->method('remove')->with('_security.admin.target_path');
|
||||||
->will($this->returnValue('/admin/dashboard'));
|
$requestWithSession = Request::create('/');
|
||||||
$session->expects($this->once())
|
$requestWithSession->setSession($session);
|
||||||
->method('remove')->with('_security.admin.target_path');
|
|
||||||
|
|
||||||
$this->request->expects($this->any())
|
return array(
|
||||||
->method('getSession')
|
'default' => array(
|
||||||
->will($this->returnValue($session));
|
Request::create('/'),
|
||||||
|
array(),
|
||||||
$response = $this->expectRedirectResponse('/admin/dashboard');
|
'/',
|
||||||
|
),
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, array());
|
'forced target path' => array(
|
||||||
$handler->setProviderKey('admin');
|
Request::create('/'),
|
||||||
|
array('always_use_default_target_path' => true, 'default_target_path' => '/dashboard'),
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
'/dashboard',
|
||||||
|
),
|
||||||
$this->assertSame($response, $result);
|
'target path as query string' => array(
|
||||||
}
|
Request::create('/?_target_path=/dashboard'),
|
||||||
|
array(),
|
||||||
public function testTargetPathIsPassedAsReferer()
|
'/dashboard',
|
||||||
{
|
),
|
||||||
$options = array('use_referer' => true);
|
'target path name as query string is customized' => array(
|
||||||
|
Request::create('/?_my_target_path=/dashboard'),
|
||||||
$this->request->headers->expects($this->once())
|
array('target_path_parameter' => '_my_target_path'),
|
||||||
->method('get')->with('Referer')
|
'/dashboard',
|
||||||
->will($this->returnValue('/dashboard'));
|
),
|
||||||
|
'target path name as query string is customized and nested' => array(
|
||||||
$response = $this->expectRedirectResponse('/dashboard');
|
Request::create('/?_target_path[value]=/dashboard'),
|
||||||
|
array('target_path_parameter' => '_target_path[value]'),
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, $options);
|
'/dashboard',
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
),
|
||||||
|
'target path in session' => array(
|
||||||
$this->assertSame($response, $result);
|
$requestWithSession,
|
||||||
}
|
array(),
|
||||||
|
'/admin/dashboard',
|
||||||
public function testRefererHasToBeDifferentThatLoginUrl()
|
),
|
||||||
{
|
'target path as referer' => array(
|
||||||
$options = array('use_referer' => true);
|
Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/dashboard')),
|
||||||
|
array('use_referer' => true),
|
||||||
$this->request->headers->expects($this->any())
|
'/dashboard',
|
||||||
->method('get')->with('Referer')
|
),
|
||||||
->will($this->returnValue('/login'));
|
'target path as referer is ignored if not configured' => array(
|
||||||
|
Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/dashboard')),
|
||||||
$this->httpUtils->expects($this->once())
|
array(),
|
||||||
->method('generateUri')->with($this->request, '/login')
|
'/',
|
||||||
->will($this->returnValue('/login'));
|
),
|
||||||
|
'target path should be different than login URL' => array(
|
||||||
$response = $this->expectRedirectResponse('/');
|
Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/login')),
|
||||||
|
array('use_referer' => true, 'login_path' => '/login'),
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, $options);
|
'/',
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
),
|
||||||
|
'target path should be different than login URL (query string does not matter)' => array(
|
||||||
$this->assertSame($response, $result);
|
Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/login?t=1&p=2')),
|
||||||
}
|
array('use_referer' => true, 'login_path' => '/login'),
|
||||||
|
'/',
|
||||||
public function testRefererTargetPathIsIgnoredByDefault()
|
),
|
||||||
{
|
'target path should be different than login URL (login_path as a route)' => array(
|
||||||
$this->request->headers->expects($this->never())->method('get');
|
Request::create('/', 'GET', array(), array(), array(), array('HTTP_REFERER' => 'http://localhost/login?t=1&p=2')),
|
||||||
|
array('use_referer' => true, 'login_path' => 'login_route'),
|
||||||
$response = $this->expectRedirectResponse('/');
|
'/',
|
||||||
|
),
|
||||||
$handler = new DefaultAuthenticationSuccessHandler($this->httpUtils, array());
|
);
|
||||||
$result = $handler->onAuthenticationSuccess($this->request, $this->token);
|
|
||||||
|
|
||||||
$this->assertSame($response, $result);
|
|
||||||
}
|
|
||||||
|
|
||||||
private function expectRedirectResponse($path)
|
|
||||||
{
|
|
||||||
$response = new Response();
|
|
||||||
$this->httpUtils->expects($this->once())
|
|
||||||
->method('createRedirectResponse')
|
|
||||||
->with($this->request, $path)
|
|
||||||
->will($this->returnValue($response));
|
|
||||||
|
|
||||||
return $response;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user