Skip to content

Commit f19d3e0

Browse files
markstorymarkstory
committed
Don't include host and protocol in redirect URLs
When fixing #34 I neglected to fix the CakeRedirectHandler. This syncs the behavior between both redirect implementations so that they only include the path + query string, and handle base dirs as well. Fixes #115 Backport #116 to 1.x
1 parent de0899f commit f19d3e0

File tree

5 files changed

+108
-30
lines changed

5 files changed

+108
-30
lines changed

‎src/Middleware/UnauthorizedHandler/CakeRedirectHandler.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,17 @@ protected function getUrl(ServerRequestInterface $request, array $options)
6464
{
6565
$url = $options['url'];
6666
if ($options['queryParam'] !== null) {
67-
$url['?'][$options['queryParam']] = (string)$request->getUri();
67+
$uri = $request->getUri();
68+
/** @psalm-suppress NoInterfaceProperties */
69+
if (property_exists($uri, 'base')) {
70+
$uri = $uri->withPath($uri->base . $uri->getPath());
71+
}
72+
$redirect = $uri->getPath();
73+
if ($uri->getQuery()) {
74+
$redirect .= '?' . $uri->getQuery();
75+
}
76+
77+
$url['?'][$options['queryParam']] = $redirect;
6878
}
6979

7080
return Router::url($url);

‎src/Middleware/UnauthorizedHandler/RedirectHandler.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,16 @@ protected function getUrl(ServerRequestInterface $request, array $options)
9393
{
9494
$url = $options['url'];
9595
if ($options['queryParam'] !== null && $request->getMethod() === 'GET') {
96-
$query = urlencode($options['queryParam']) . '=' . urlencode($request->getRequestTarget());
96+
$uri = $request->getUri();
97+
/** @psalm-suppress NoInterfaceProperties */
98+
if (property_exists($uri, 'base')) {
99+
$uri = $uri->withPath($uri->base . $uri->getPath());
100+
}
101+
$redirect = $uri->getPath();
102+
if ($uri->getQuery()) {
103+
$redirect .= '?' . $uri->getQuery();
104+
}
105+
$query = urlencode($options['queryParam']) . '=' . urlencode($redirect);
97106
if (strpos($url, '?') !== false) {
98107
$query = '&' . $query;
99108
} else {

‎tests/TestCase/Middleware/UnauthorizedHandler/CakeRedirectHandlerTest.php‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,12 @@
1616

1717
use Authorization\Exception\Exception;
1818
use Authorization\Middleware\UnauthorizedHandler\CakeRedirectHandler;
19+
use Cake\Core\Configure;
1920
use Cake\Http\Response;
20-
use Cake\Http\ServerRequest;
21+
use Cake\Http\ServerRequestFactory;
2122
use Cake\Routing\Router;
2223
use Cake\TestSuite\TestCase;
2324

24-
/**
25-
* Description of CakeRedirectHandlerTest
26-
*/
2725
class CakeRedirectHandlerTest extends TestCase
2826
{
2927
public function setUp()
@@ -45,25 +43,28 @@ public function testHandleRedirectionDefault()
4543
$handler = new CakeRedirectHandler();
4644

4745
$exception = new Exception();
48-
$request = new ServerRequest();
46+
$request = ServerRequestFactory::fromGlobals(
47+
['REQUEST_URI' => '/admin/dashboard']
48+
);
4949
$response = new Response();
50-
5150
$response = $handler->handle($exception, $request, $response, [
5251
'exceptions' => [
5352
Exception::class,
5453
],
5554
]);
5655

5756
$this->assertEquals(302, $response->getStatusCode());
58-
$this->assertEquals('/login?redirect=http%3A%2F%2Flocalhost%2F', $response->getHeaderLine('Location'));
57+
$this->assertEquals('/login?redirect=%2Fadmin%2Fdashboard', $response->getHeaderLine('Location'));
5958
}
6059

6160
public function testHandleRedirectionNamed()
6261
{
6362
$handler = new CakeRedirectHandler();
6463

6564
$exception = new Exception();
66-
$request = new ServerRequest();
65+
$request = ServerRequestFactory::fromGlobals(
66+
['REQUEST_URI' => '/admin/dashboard']
67+
);
6768
$response = new Response();
6869

6970
$response = $handler->handle($exception, $request, $response, [
@@ -76,15 +77,17 @@ public function testHandleRedirectionNamed()
7677
]);
7778

7879
$this->assertEquals(302, $response->getStatusCode());
79-
$this->assertEquals('/login?redirect=http%3A%2F%2Flocalhost%2F', $response->getHeaderLine('Location'));
80+
$this->assertEquals('/login?redirect=%2Fadmin%2Fdashboard', $response->getHeaderLine('Location'));
8081
}
8182

8283
public function testHandleRedirectionWithQuery()
8384
{
8485
$handler = new CakeRedirectHandler();
8586

8687
$exception = new Exception();
87-
$request = new ServerRequest();
88+
$request = ServerRequestFactory::fromGlobals(
89+
['REQUEST_URI' => '/']
90+
);
8891
$response = new Response();
8992

9093
$response = $handler->handle($exception, $request, $response, [
@@ -100,15 +103,17 @@ public function testHandleRedirectionWithQuery()
100103
]);
101104

102105
$this->assertEquals(302, $response->getStatusCode());
103-
$this->assertEquals('/login?foo=bar&redirect=http%3A%2F%2Flocalhost%2F', $response->getHeaderLine('Location'));
106+
$this->assertEquals('/login?foo=bar&redirect=%2F', $response->getHeaderLine('Location'));
104107
}
105108

106109
public function testHandleRedirectionNoQuery()
107110
{
108111
$handler = new CakeRedirectHandler();
109112

110113
$exception = new Exception();
111-
$request = new ServerRequest();
114+
$request = ServerRequestFactory::fromGlobals(
115+
['REQUEST_URI' => '/']
116+
);
112117
$response = new Response();
113118

114119
$response = $handler->handle($exception, $request, $response, [
@@ -121,4 +126,31 @@ public function testHandleRedirectionNoQuery()
121126
$this->assertEquals(302, $response->getStatusCode());
122127
$this->assertEquals('/login', $response->getHeaderLine('Location'));
123128
}
129+
130+
public function testHandleRedirectWithBasePath()
131+
{
132+
$handler = new CakeRedirectHandler();
133+
$exception = new Exception();
134+
135+
Configure::write('App.base', '/basedir');
136+
$request = ServerRequestFactory::fromGlobals(
137+
['REQUEST_URI' => '/admin/dashboard']
138+
);
139+
$response = new Response();
140+
141+
$response = $handler->handle($exception, $request, $response, [
142+
'exceptions' => [
143+
Exception::class,
144+
],
145+
'url' => [
146+
'_name' => 'login',
147+
],
148+
]);
149+
150+
$this->assertEquals(302, $response->getStatusCode());
151+
$this->assertEquals(
152+
'/basedir/login?redirect=%2Fbasedir%2Fadmin%2Fdashboard',
153+
$response->getHeaderLine('Location')
154+
);
155+
}
124156
}

‎tests/TestCase/Middleware/UnauthorizedHandler/RedirectHandlerTest.php‎

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@
1616

1717
use Authorization\Exception\Exception;
1818
use Authorization\Middleware\UnauthorizedHandler\RedirectHandler;
19+
use Cake\Core\Configure;
1920
use Cake\Http\Response;
20-
use Cake\Http\ServerRequest;
21+
use Cake\Http\ServerRequestFactory;
2122
use Cake\TestSuite\TestCase;
2223

2324
class RedirectHandlerTest extends TestCase
@@ -27,9 +28,9 @@ public function testHandleRedirection()
2728
$handler = new RedirectHandler();
2829

2930
$exception = new Exception();
30-
$request = new ServerRequest([
31-
'environment' => ['REQUEST_METHOD' => 'GET'],
32-
]);
31+
$request = ServerRequestFactory::fromGlobals(
32+
['REQUEST_METHOD' => 'GET']
33+
);
3334
$response = new Response();
3435

3536
$response = $handler->handle($exception, $request, $response, [
@@ -47,13 +48,13 @@ public function testHandleRedirectionWithQuery()
4748
$handler = new RedirectHandler();
4849

4950
$exception = new Exception();
50-
$request = new ServerRequest([
51-
'environment' => [
51+
$request = ServerRequestFactory::fromGlobals(
52+
[
5253
'REQUEST_METHOD' => 'GET',
5354
'PATH_INFO' => '/path',
5455
'QUERY_STRING' => 'key=value',
55-
],
56-
]);
56+
]
57+
);
5758
$response = new Response();
5859

5960
$response = $handler->handle($exception, $request, $response, [
@@ -72,9 +73,9 @@ public function testHandleRedirectionNoQuery()
7273
$handler = new RedirectHandler();
7374

7475
$exception = new Exception();
75-
$request = new ServerRequest([
76-
'environment' => ['REQUEST_METHOD' => 'GET'],
77-
]);
76+
$request = ServerRequestFactory::fromGlobals(
77+
['REQUEST_METHOD' => 'GET']
78+
);
7879
$response = new Response();
7980

8081
$response = $handler->handle($exception, $request, $response, [
@@ -109,13 +110,13 @@ public function testHandleRedirectionIgnoreNonIdempotentMethods($method)
109110
$handler = new RedirectHandler();
110111

111112
$exception = new Exception();
112-
$request = new ServerRequest([
113-
'environment' => [
113+
$request = ServerRequestFactory::fromGlobals(
114+
[
114115
'REQUEST_METHOD' => $method,
115116
'PATH_INFO' => '/path',
116117
'QUERY_STRING' => 'key=value',
117-
],
118-
]);
118+
]
119+
);
119120
$response = new Response();
120121

121122
$response = $handler->handle($exception, $request, $response, [
@@ -129,12 +130,37 @@ public function testHandleRedirectionIgnoreNonIdempotentMethods($method)
129130
$this->assertEquals('/login?foo=bar', $response->getHeaderLine('Location'));
130131
}
131132

133+
public function testHandleRedirectWithBasePath()
134+
{
135+
$handler = new RedirectHandler();
136+
$exception = new Exception();
137+
138+
Configure::write('App.base', '/basedir');
139+
$request = ServerRequestFactory::fromGlobals(
140+
['REQUEST_URI' => '/path', 'REQUEST_METHOD' => 'GET']
141+
);
142+
$response = new Response();
143+
144+
$response = $handler->handle($exception, $request, $response, [
145+
'exceptions' => [
146+
Exception::class,
147+
],
148+
'url' => '/basedir/login',
149+
]);
150+
151+
$this->assertEquals(302, $response->getStatusCode());
152+
$this->assertEquals(
153+
'/basedir/login?redirect=%2Fbasedir%2Fpath',
154+
$response->getHeaderLine('Location')
155+
);
156+
}
157+
132158
public function testHandleException()
133159
{
134160
$handler = new RedirectHandler();
135161

136162
$exception = new Exception();
137-
$request = new ServerRequest();
163+
$request = ServerRequestFactory::fromGlobals(['REQUEST_URI' => '/']);
138164
$response = new Response();
139165

140166
$this->expectException(Exception::class);

‎tests/bootstrap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444

4545
Configure::write('debug', true);
4646
Configure::write('App', [
47+
'base' => '',
4748
'namespace' => 'TestApp',
4849
'encoding' => 'UTF-8',
4950
'paths' => [

0 commit comments

Comments
 (0)