Skip to content

Commit 6395003

Browse files
Allow skipping redirects for extensions. (#308)
* Allow skipping redirects for extensions. * Update docs/en/middleware.rst Co-authored-by: Mark Story <mark@mark-story.com> * Update tests/TestCase/Middleware/UnauthorizedHandler/RedirectHandlerTest.php Co-authored-by: Mark Story <mark@mark-story.com> * Update tests/TestCase/Middleware/UnauthorizedHandler/RedirectHandlerTest.php Co-authored-by: Mark Story <mark@mark-story.com> * Allow skipping redirects for extensions. * Remove LogicException - We can use one of the existing exceptions from the plugin. - Expand test coverage to cover all the new branches --------- Co-authored-by: Mark Story <mark@mark-story.com>
1 parent 2482ff9 commit 6395003

4 files changed

Lines changed: 124 additions & 1 deletion

File tree

docs/en/middleware.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,11 @@ Both redirect handlers share the same configuration options:
198198
* ``queryParam`` - the accessed request URL will be attached to the redirect URL
199199
query parameter (``redirect`` by default).
200200
* ``statusCode`` - HTTP status code of a redirect, ``302`` by default.
201+
* ``allowedRedirectExtensions`` - an array of allowed file extensions for redirecting.
202+
If the request URL has a file extension that is not in this list, the redirect will not
203+
happen and the exception will be rethrown. Can also be a boolean to toggle on/off
204+
redirects entirely. This is useful to prevent unauthorized access to API based
205+
responses, that should not be redirecting in any case. `true` by default and not enabled then.
201206

202207
For example::
203208

@@ -212,6 +217,7 @@ For example::
212217
MissingIdentityException::class,
213218
OtherException::class,
214219
],
220+
'allowedRedirectExtensions' => ['csv', 'pdf'],
215221
],
216222
]));
217223

src/Middleware/UnauthorizedHandler/CakeRedirectHandler.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class CakeRedirectHandler extends RedirectHandler
4141
],
4242
'queryParam' => 'redirect',
4343
'statusCode' => 302,
44+
'allowedRedirectExtensions' => true,
4445
];
4546

4647
/**

src/Middleware/UnauthorizedHandler/RedirectHandler.php

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ class RedirectHandler implements HandlerInterface
3434
* - `url` - Url to redirect to.
3535
* - `queryParam` - Query parameter name for the target url.
3636
* - `statusCode` - Redirection status code.
37+
* - `allowedRedirectExtensions` - If true, redirects are allowed for all extensions.
38+
* Pass specific ones to allow list, or false to disallow redirects for any extension.
3739
*
3840
* @var array
3941
*/
@@ -44,6 +46,7 @@ class RedirectHandler implements HandlerInterface
4446
'url' => '/login',
4547
'queryParam' => 'redirect',
4648
'statusCode' => 302,
49+
'allowedRedirectExtensions' => true,
4750
];
4851

4952
/**
@@ -58,7 +61,7 @@ public function handle(
5861
): ResponseInterface {
5962
$options += $this->defaultOptions;
6063

61-
if (!$this->checkException($exception, $options['exceptions'])) {
64+
if (!$this->redirectAllowed($request, $options) || !$this->checkException($exception, $options['exceptions'])) {
6265
throw $exception;
6366
}
6467

@@ -117,4 +120,28 @@ protected function getUrl(ServerRequestInterface $request, array $options): stri
117120

118121
return $url;
119122
}
123+
124+
/**
125+
* @param \Psr\Http\Message\ServerRequestInterface $request
126+
* @param array $options
127+
* @return bool
128+
*/
129+
protected function redirectAllowed(ServerRequestInterface $request, array $options): bool
130+
{
131+
$extensions = $options['allowedRedirectExtensions'] ?? true;
132+
if ($extensions === false) {
133+
return false;
134+
}
135+
if ($extensions === true) {
136+
return true;
137+
}
138+
139+
/** @var \Cake\Http\ServerRequest $request */
140+
$currentExtension = $request->getParam('_ext');
141+
if (!$currentExtension) {
142+
return true;
143+
}
144+
145+
return in_array($currentExtension, (array)$extensions, true);
146+
}
120147
}

tests/TestCase/Middleware/UnauthorizedHandler/RedirectHandlerTest.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
namespace Authorization\Test\TestCase\Middleware\UnauthorizedHandler;
1818

1919
use Authorization\Exception\Exception;
20+
use Authorization\Exception\MissingIdentityException;
2021
use Authorization\Middleware\UnauthorizedHandler\RedirectHandler;
2122
use Cake\Core\Configure;
2223
use Cake\Http\ServerRequestFactory;
2324
use Cake\TestSuite\TestCase;
25+
use LogicException;
2426
use PHPUnit\Framework\Attributes\DataProvider;
2527

2628
class RedirectHandlerTest extends TestCase
@@ -160,4 +162,91 @@ public function testHandleException()
160162
$this->expectException(Exception::class);
161163
$handler->handle($exception, $request);
162164
}
165+
166+
public function testHandleRedirectionWithExtensionsFalse(): void
167+
{
168+
$handler = new RedirectHandler();
169+
170+
$exception = new MissingIdentityException();
171+
$request = ServerRequestFactory::fromGlobals(
172+
['REQUEST_METHOD' => 'GET'],
173+
);
174+
$request = $request->withParam('_ext', 'csv');
175+
176+
$this->expectException(MissingIdentityException::class);
177+
178+
$handler->handle($exception, $request, [
179+
'exceptions' => [
180+
LogicException::class,
181+
],
182+
'url' => '/users/login',
183+
'allowedRedirectExtensions' => false,
184+
]);
185+
}
186+
187+
public function testHandleRedirectionWithExtension(): void
188+
{
189+
$handler = new RedirectHandler();
190+
191+
$exception = new MissingIdentityException();
192+
$request = ServerRequestFactory::fromGlobals(
193+
['REQUEST_METHOD' => 'GET'],
194+
);
195+
$request = $request->withParam('_ext', 'csv');
196+
197+
$this->expectException(MissingIdentityException::class);
198+
199+
$handler->handle($exception, $request, [
200+
'exceptions' => [
201+
MissingIdentityException::class,
202+
],
203+
'url' => '/users/login',
204+
'allowedRedirectExtensions' => [],
205+
]);
206+
}
207+
208+
public function testHandleRedirectionWithExtensionAllowlisted(): void
209+
{
210+
$handler = new RedirectHandler();
211+
212+
$exception = new MissingIdentityException();
213+
$request = ServerRequestFactory::fromGlobals(
214+
['REQUEST_METHOD' => 'GET'],
215+
);
216+
$request = $request->withParam('_ext', 'csv');
217+
218+
$response = $handler->handle($exception, $request, [
219+
'exceptions' => [
220+
Exception::class,
221+
],
222+
'url' => '/users/login',
223+
'queryParam' => null,
224+
'allowedRedirectExtensions' => ['csv'],
225+
]);
226+
227+
$this->assertSame(302, $response->getStatusCode());
228+
$this->assertSame('/users/login', $response->getHeaderLine('Location'));
229+
}
230+
231+
public function testHandleRedirectionWithExtensionAllowedNoExtensionInRequest(): void
232+
{
233+
$handler = new RedirectHandler();
234+
235+
$exception = new Exception();
236+
$request = ServerRequestFactory::fromGlobals(
237+
['REQUEST_METHOD' => 'GET'],
238+
);
239+
240+
$response = $handler->handle($exception, $request, [
241+
'exceptions' => [
242+
Exception::class,
243+
],
244+
'url' => '/users/login',
245+
'queryParam' => null,
246+
'allowedRedirectExtensions' => ['csv'],
247+
]);
248+
249+
$this->assertSame(302, $response->getStatusCode());
250+
$this->assertSame('/users/login', $response->getHeaderLine('Location'));
251+
}
163252
}

0 commit comments

Comments
 (0)