Skip to content

Commit 7ffb4f9

Browse files
authored
Merge pull request #105 from kubajaburek/redirect-to-original-page
Redirect user to original URL after authentication
2 parents 0746938 + 7b8516c commit 7ffb4f9

8 files changed

Lines changed: 160 additions & 7 deletions

File tree

Controller/SamlController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function loginAction(Request $request)
2727
throw new \RuntimeException($error->getMessage());
2828
}
2929

30-
$this->get('onelogin_auth')->login();
30+
$this->get('onelogin_auth')->login($session->get('_security.main.target_path'));
3131
}
3232

3333
public function metadataAction()

DependencyInjection/Security/Factory/SamlFactory.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ public function __construct()
1818
$this->addOption('user_factory');
1919
$this->addOption('token_factory');
2020
$this->addOption('persist_user', false);
21-
21+
22+
if (!isset($this->options['success_handler'])) {
23+
$this->options['success_handler'] = 'hslavich_onelogin_saml.saml_authentication_success_handler';
24+
}
2225
$this->defaultFailureHandlerOptions['login_path'] = '/saml/login';
2326
}
2427

Resources/config/services.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ services:
1616
hslavich_onelogin_saml.saml_token_factory:
1717
class: Hslavich\OneloginSamlBundle\Security\Authentication\Token\SamlTokenFactory
1818

19+
hslavich_onelogin_saml.saml_authentication_success_handler:
20+
class: Hslavich\OneloginSamlBundle\Security\Authentication\SamlAuthenticationSuccessHandler
21+
parent: security.authentication.success_handler
22+
1923
hslavich_onelogin_saml.saml_listener:
2024
class: Hslavich\OneloginSamlBundle\Security\Firewall\SamlListener
2125
parent: security.authentication.listener.abstract
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
namespace Hslavich\OneloginSamlBundle\Security\Authentication;
4+
5+
use Symfony\Component\Security\Http\Authentication\DefaultAuthenticationSuccessHandler;
6+
use Symfony\Component\HttpFoundation\Request;
7+
8+
class SamlAuthenticationSuccessHandler extends DefaultAuthenticationSuccessHandler
9+
{
10+
protected function determineTargetUrl(Request $request)
11+
{
12+
if ($this->options['always_use_default_target_path']) {
13+
return $this->options['default_target_path'];
14+
}
15+
16+
$relayState = $request->get('RelayState');
17+
if (null !== $relayState && $relayState !== $this->httpUtils->generateUri($request, $this->options['login_path'])) {
18+
return $relayState;
19+
}
20+
21+
return parent::determineTargetUrl($request);
22+
}
23+
}

Tests/Authentication/Provider/SamlProviderTest.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ public function testAuthenticate()
2626

2727
$this->assertInstanceOf('Hslavich\\OneloginSamlBundle\\Security\\Authentication\\Token\\SamlToken', $token);
2828
$this->assertEquals(array('foo' => 'bar'), $token->getAttributes());
29-
$this->assertEquals(array(), $token->getRoles());
29+
if (\Symfony\Component\HttpKernel\Kernel::VERSION_ID >= 40300) {
30+
$this->assertEquals(array(), $token->getRoleNames());
31+
} else {
32+
$this->assertEquals(array(), $token->getRoles());
33+
}
3034
$this->assertTrue($token->isAuthenticated());
3135
$this->assertSame($user, $token->getUser());
3236
}
@@ -53,7 +57,11 @@ public function testAuthenticateWithUserFactory()
5357

5458
$this->assertInstanceOf('Hslavich\\OneloginSamlBundle\\Security\\Authentication\\Token\\SamlToken', $token);
5559
$this->assertEquals(array('foo' => 'bar'), $token->getAttributes());
56-
$this->assertEquals(array(), $token->getRoles());
60+
if (\Symfony\Component\HttpKernel\Kernel::VERSION_ID >= 40300) {
61+
$this->assertEquals(array(), $token->getRoleNames());
62+
} else {
63+
$this->assertEquals(array(), $token->getRoles());
64+
}
5765
$this->assertTrue($token->isAuthenticated());
5866
$this->assertSame($user, $token->getUser());
5967
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
<?php
2+
3+
namespace Hslavich\OneloginSamlBundle\Tests\Authentication;
4+
5+
use Hslavich\OneloginSamlBundle\Security\Authentication\SamlAuthenticationSuccessHandler;
6+
use Symfony\Component\HttpFoundation\Request;
7+
use Symfony\Component\Security\Http\HttpUtils;
8+
9+
class SamlAuthenticationSuccessHandlerTest extends \PHPUnit_Framework_TestCase
10+
{
11+
private $handler;
12+
13+
public function testWithAlwaysUseDefaultTargetPath()
14+
{
15+
$httpUtils = new HttpUtils($this->getUrlGenerator());
16+
$handler = new SamlAuthenticationSuccessHandler($httpUtils, array('always_use_default_target_path' => true));
17+
$defaultTargetPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'default_target_path', '/'));
18+
$response = $handler->onAuthenticationSuccess($this->getRequest('/login', 'http://localhost/relayed'), $this->getSamlToken());
19+
$this->assertTrue($response->isRedirect($defaultTargetPath), 'SamlAuthenticationSuccessHandler does not honor the always_use_default_target_path option.');
20+
}
21+
22+
public function testRelayState()
23+
{
24+
$handler = new SamlAuthenticationSuccessHandler(new HttpUtils($this->getUrlGenerator()), array('always_use_default_target_path' => false));
25+
$response = $handler->onAuthenticationSuccess($this->getRequest('/sso/login', 'http://localhost/relayed'), $this->getSamlToken());
26+
$this->assertTrue($response->isRedirect('http://localhost/relayed'), 'SamlAuthenticationSuccessHandler is not processing the RelayState parameter properly.');
27+
}
28+
29+
public function testWithoutRelayState()
30+
{
31+
$httpUtils = new HttpUtils($this->getUrlGenerator());
32+
$handler = new SamlAuthenticationSuccessHandler($httpUtils, array('always_use_default_target_path' => false));
33+
$defaultTargetPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'default_target_path', '/'));
34+
$response = $handler->onAuthenticationSuccess($this->getRequest(), $this->getSamlToken());
35+
$this->assertTrue($response->isRedirect($defaultTargetPath));
36+
}
37+
38+
public function testRelayStateLoop()
39+
{
40+
$httpUtils = new HttpUtils($this->getUrlGenerator());
41+
$handler = new SamlAuthenticationSuccessHandler($httpUtils, array('always_use_default_target_path' => false));
42+
$loginPath = $httpUtils->generateUri($this->getRequest('/sso/login'), $this->getOption($handler, 'login_path', '/login'));
43+
$response = $handler->onAuthenticationSuccess($this->getRequest($loginPath), $this->getSamlToken());
44+
$this->assertTrue(!$response->isRedirect($loginPath), 'SamlAuthenticationSuccessHandler causes a redirect loop when RelayState points to login_path.');
45+
}
46+
47+
48+
private function getUrlGenerator()
49+
{
50+
$urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock();
51+
$urlGenerator
52+
->expects($this->any())
53+
->method('generate')
54+
->will($this->returnCallback(function($name)
55+
{
56+
return (string) $name;
57+
}))
58+
;
59+
60+
return $urlGenerator;
61+
}
62+
63+
private function getRequest($path = '/', $relayState = null)
64+
{
65+
$params = array();
66+
if (null !== $relayState) {
67+
$params['RelayState'] = $relayState;
68+
}
69+
return Request::create($path, 'get', $params);
70+
}
71+
72+
private function getSamlToken()
73+
{
74+
$token = $this->createMock('Hslavich\OneloginSamlBundle\Security\Authentication\Token\SamlToken');
75+
$token->expects($this->any())->method('getUsername')->willReturn('admin');
76+
$token->method('getAttributes')->willReturn(array('foo' => 'bar'));
77+
78+
return $token;
79+
}
80+
81+
private function getOption($handler, $name, $default = null)
82+
{
83+
$reflection = new \ReflectionObject($handler);
84+
$options = $reflection->getProperty('options');
85+
$options->setAccessible(true);
86+
$arr = $options->getValue($handler);
87+
if (!is_array($arr) || !isset($arr[$name])) {
88+
return $default;
89+
}
90+
return $arr[$name];
91+
}
92+
}

Tests/Firewall/SamlListenerTest.php

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
class SamlProviderTest extends \PHPUnit_Framework_TestCase
99
{
10+
private $httpKernel;
1011
private $authenticationManager;
1112
private $dispatcher;
1213
private $event;
@@ -29,7 +30,11 @@ public function testHandleValidAuthenticationWithAttribute()
2930
;
3031
$listener->setOneLoginAuth($onelogin);
3132

32-
$listener->handle($this->event);
33+
if (\Symfony\Component\HttpKernel\Kernel::VERSION_ID >= 40300) {
34+
$listener($this->event);
35+
} else {
36+
$listener->handle($this->event);
37+
}
3338
}
3439

3540
public function testHandleValidAuthenticationWithEmptyOptions()
@@ -50,7 +55,11 @@ public function testHandleValidAuthenticationWithEmptyOptions()
5055
;
5156
$listener->setOneLoginAuth($onelogin);
5257

53-
$listener->handle($this->event);
58+
if (\Symfony\Component\HttpKernel\Kernel::VERSION_ID >= 40300) {
59+
$listener($this->event);
60+
} else {
61+
$listener->handle($this->event);
62+
}
5463
}
5564

5665
protected function getListener($options = array())
@@ -69,6 +78,7 @@ protected function getListener($options = array())
6978

7079
protected function setUp()
7180
{
81+
$this->httpKernel = $this->createMock('Symfony\Component\HttpKernel\HttpKernelInterface');
7282
$this->authenticationManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager')
7383
->disableOriginalConstructor()
7484
->getMock()
@@ -86,12 +96,21 @@ protected function setUp()
8696
->will($this->returnValue(true))
8797
;
8898

89-
$this->event = $this->createMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
99+
if (class_exists('Symfony\Component\HttpKernel\Event\RequestEvent')) {
100+
$this->event = $this->createMock('Symfony\Component\HttpKernel\Event\RequestEvent', array(), array(), '', false);
101+
} else {
102+
$this->event = $this->createMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
103+
}
90104
$this->event
91105
->expects($this->any())
92106
->method('getRequest')
93107
->will($this->returnValue($this->request))
94108
;
109+
$this->event
110+
->expects($this->any())
111+
->method('getKernel')
112+
->will($this->returnValue($this->httpKernel))
113+
;
95114
$this->sessionStrategy = $this->createMock('Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface');
96115
$this->httpUtils = $this->createMock('Symfony\Component\Security\Http\HttpUtils');
97116
$this->httpUtils

phpunit.xml.dist

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,8 @@
2222
<logging>
2323
<log type="coverage-clover" target="build/logs/clover.xml"/>
2424
</logging>
25+
26+
<php>
27+
<env name="SYMFONY_DEPRECATIONS_HELPER" value="max[direct]=0" />
28+
</php>
2529
</phpunit>

0 commit comments

Comments
 (0)