Skip to content

Commit 801c4fd

Browse files
ioigoumeioigoume
committed
Use RunnableResponse instead of redirect
1 parent 640c7eb commit 801c4fd

File tree

2 files changed

+43
-37
lines changed

2 files changed

+43
-37
lines changed

‎src/Controller/LogoutController.php‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
namespace SimpleSAML\Module\casserver\Controller;
66

77
use SimpleSAML\Auth\Simple;
8-
use SimpleSAML\Compat\SspContainer;
98
use SimpleSAML\Configuration;
9+
use SimpleSAML\HTTP\RunnableResponse;
1010
use SimpleSAML\Logger;
1111
use SimpleSAML\Module;
1212
use SimpleSAML\Module\casserver\Cas\Factories\TicketFactory;
1313
use SimpleSAML\Module\casserver\Cas\Ticket\TicketStore;
1414
use SimpleSAML\Module\casserver\Controller\Traits\UrlTrait;
1515
use SimpleSAML\Session;
16-
use Symfony\Component\HttpFoundation\RedirectResponse;
16+
use SimpleSAML\Utils;
1717
use Symfony\Component\HttpFoundation\Request;
1818
use Symfony\Component\HttpKernel\Attribute\AsController;
1919
use Symfony\Component\HttpKernel\Attribute\MapQueryParameter;
@@ -35,17 +35,18 @@ class LogoutController
3535
/** @var Simple */
3636
protected Simple $authSource;
3737

38-
/** @var SspContainer */
39-
protected SspContainer $container;
38+
/** @var Utils\HTTP */
39+
protected Utils\HTTP $httpUtils;
4040

4141
/** @var TicketStore */
4242
protected TicketStore $ticketStore;
4343

4444

4545
/**
46+
* @param Configuration $sspConfig
4647
* @param Configuration|null $casConfig
4748
* @param Simple|null $source
48-
* @param SspContainer|null $container
49+
* @param Utils\HTTP|null $httpUtils
4950
*
5051
* @throws \Exception
5152
*/
@@ -54,15 +55,15 @@ public function __construct(
5455
// Facilitate testing
5556
Configuration $casConfig = null,
5657
Simple $source = null,
57-
SspContainer $container = null,
58+
Utils\HTTP $httpUtils = null,
5859
) {
5960
// We are using this work around in order to bypass Symfony's autowiring for cas configuration. Since
6061
// the configuration class is the same, it loads the ssp configuration twice. Still, we need the constructor
6162
// argument in order to facilitate testin.
6263
$this->casConfig = ($casConfig === null || $casConfig === $sspConfig)
6364
? Configuration::getConfig('module_casserver.php') : $casConfig;
6465
$this->authSource = $source ?? new Simple($this->casConfig->getValue('authsource'));
65-
$this->container = $container ?? new SspContainer();
66+
$this->httpUtils = $httpUtils ?? new Utils\HTTP();
6667

6768
/* Instantiate ticket factory */
6869
$this->ticketFactory = new TicketFactory($this->casConfig);
@@ -80,12 +81,12 @@ public function __construct(
8081
* @param Request $request
8182
* @param string|null $url
8283
*
83-
* @return RedirectResponse|null
84+
* @return RunnableResponse|null
8485
*/
8586
public function logout(
8687
Request $request,
8788
#[MapQueryParameter] ?string $url = null,
88-
): RedirectResponse|null {
89+
): RunnableResponse|null {
8990
if (!$this->casConfig->getOptionalValue('enable_logout', false)) {
9091
$this->handleExceptionThrown('Logout not allowed');
9192
}
@@ -115,7 +116,7 @@ public function logout(
115116

116117
// Redirect
117118
if (!$this->authSource->isAuthenticated()) {
118-
$this->container->redirect($logoutRedirectUrl, $params);
119+
return new RunnableResponse([$this->httpUtils, 'redirectTrustedURL'], [$logoutRedirectUrl, $params]);
119120
}
120121

121122
// Logout and redirect

‎tests/src/Controller/LogoutControllerTest.php‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
use PHPUnit\Framework\MockObject\MockObject;
88
use PHPUnit\Framework\TestCase;
99
use SimpleSAML\Auth\Simple;
10-
use SimpleSAML\Compat\SspContainer;
1110
use SimpleSAML\Configuration;
1211
use SimpleSAML\Module;
1312
use SimpleSAML\Module\casserver\Controller\LogoutController;
1413
use SimpleSAML\Session;
14+
use SimpleSAML\Utils;
1515
use Symfony\Component\HttpFoundation\Request;
1616

1717
class LogoutControllerTest extends TestCase
@@ -20,7 +20,7 @@ class LogoutControllerTest extends TestCase
2020

2121
private Simple|MockObject $authSimpleMock;
2222

23-
private SspContainer|MockObject $sspContainer;
23+
private Utils\HTTP $httpUtils;
2424

2525
private Configuration $sspConfig;
2626

@@ -31,18 +31,14 @@ protected function setUp(): void
3131
->onlyMethods(['logout', 'isAuthenticated'])
3232
->getMock();
3333

34-
$this->sspContainer = $this->getMockBuilder(SspContainer::class)
35-
->disableOriginalConstructor()
36-
->onlyMethods(['redirect'])
37-
->getMock();
38-
3934
$this->moduleConfig = [
4035
'ticketstore' => [
4136
'class' => 'casserver:FileSystemTicketStore', //Not intended for production
4237
'directory' => __DIR__ . '../../../../tests/ticketcache',
4338
],
4439
];
4540

41+
$this->httpUtils = new Utils\HTTP();
4642
$this->sspConfig = Configuration::getConfig('config.php');
4743
}
4844

@@ -91,20 +87,23 @@ public function testLogoutWithRedirectUrlOnSkipLogout(): void
9187

9288
// Unauthenticated
9389
$this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(false);
94-
$this->sspContainer->expects($this->once())->method('redirect')->with(
95-
$this->equalTo($urlParam),
96-
[],
97-
);
9890

99-
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->sspContainer);
91+
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils);
10092

10193
$logoutUrl = Module::getModuleURL('casserver/logout.php');
10294

10395
$request = Request::create(
10496
uri: $logoutUrl,
10597
parameters: ['url' => $urlParam],
10698
);
107-
$controller->logout($request, $urlParam);
99+
$response = $controller->logout($request, $urlParam);
100+
101+
$callable = $response->getCallable();
102+
$method = is_array($callable) ? $callable[1] : 'unknown';
103+
$arguments = $response->getArguments();
104+
$this->assertEquals('redirectTrustedURL', $method);
105+
$this->assertEquals(200, $response->getStatusCode());
106+
$this->assertEquals($urlParam, $arguments[0]);
108107
}
109108

110109
public function testLogoutNoRedirectUrlOnNoSkipLogoutUnAuthenticated(): void
@@ -115,13 +114,16 @@ public function testLogoutNoRedirectUrlOnNoSkipLogoutUnAuthenticated(): void
115114

116115
// Unauthenticated
117116
$this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(false);
118-
$this->sspContainer->expects($this->once())->method('redirect')->with(
119-
$this->equalTo('http://localhost/module.php/casserver/loggedOut'),
120-
[],
121-
);
122117

123-
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->sspContainer);
124-
$controller->logout(Request::create('/'));
118+
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils);
119+
$response = $controller->logout(Request::create('/'));
120+
121+
$callable = $response->getCallable();
122+
$method = is_array($callable) ? $callable[1] : 'unknown';
123+
$arguments = $response->getArguments();
124+
$this->assertEquals('redirectTrustedURL', $method);
125+
$this->assertEquals(200, $response->getStatusCode());
126+
$this->assertEquals('http://localhost/module.php/casserver/loggedOut', $arguments[0]);
125127
}
126128

127129
public function testLogoutWithRedirectUrlOnNoSkipLogoutUnAuthenticated(): void
@@ -134,17 +136,20 @@ public function testLogoutWithRedirectUrlOnNoSkipLogoutUnAuthenticated(): void
134136

135137
// Unauthenticated
136138
$this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(false);
137-
$this->sspContainer->expects($this->once())->method('redirect')->with(
138-
$this->equalTo($logoutUrl),
139-
['url' => $urlParam],
140-
);
141139

142-
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->sspContainer);
140+
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils);
143141
$request = Request::create(
144142
uri: $logoutUrl,
145143
parameters: ['url' => $urlParam],
146144
);
147-
$controller->logout($request, $urlParam);
145+
$response = $controller->logout($request, $urlParam);
146+
147+
$callable = $response->getCallable();
148+
$method = is_array($callable) ? $callable[1] : 'unknown';
149+
$arguments = $response->getArguments();
150+
$this->assertEquals('redirectTrustedURL', $method);
151+
$this->assertEquals(200, $response->getStatusCode());
152+
$this->assertEquals('http://localhost/module.php/casserver/loggedOut', $arguments[0]);
148153
}
149154

150155
public function testLogoutNoRedirectUrlOnNoSkipLogoutAuthenticated(): void
@@ -158,7 +163,7 @@ public function testLogoutNoRedirectUrlOnNoSkipLogoutAuthenticated(): void
158163
$this->authSimpleMock->expects($this->once())->method('logout')
159164
->with('http://localhost/module.php/casserver/loggedOut');
160165

161-
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->sspContainer);
166+
$controller = new LogoutController($this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils);
162167
$controller->logout(Request::create('/'));
163168
}
164169

@@ -169,7 +174,7 @@ public function testTicketIdGetsDeletedOnLogout(): void
169174
$config = Configuration::loadFromArray($this->moduleConfig);
170175

171176
$controllerMock = $this->getMockBuilder(LogoutController::class)
172-
->setConstructorArgs([$this->sspConfig, $config, $this->authSimpleMock, $this->sspContainer])
177+
->setConstructorArgs([$this->sspConfig, $config, $this->authSimpleMock, $this->httpUtils])
173178
->onlyMethods(['getSession'])
174179
->getMock();
175180

0 commit comments

Comments
 (0)