Skip to content

Commit

Permalink
Merge pull request #116 from cakephp/issue-115
Browse files Browse the repository at this point in the history
Don't include host and protocol in redirect URLs
  • Loading branch information
markstory authored Jan 9, 2020
2 parents 0eebd1b + 97755dd commit 3930120
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 30 deletions.
12 changes: 11 additions & 1 deletion src/Middleware/UnauthorizedHandler/CakeRedirectHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,17 @@ protected function getUrl(ServerRequestInterface $request, array $options): stri
{
$url = $options['url'];
if ($options['queryParam'] !== null) {
$url['?'][$options['queryParam']] = (string)$request->getUri();
$uri = $request->getUri();
/** @psalm-suppress NoInterfaceProperties */
if (property_exists($uri, 'base')) {
$uri = $uri->withPath($uri->base . $uri->getPath());
}
$redirect = $uri->getPath();
if ($uri->getQuery()) {
$redirect .= '?' . $uri->getQuery();
}

$url['?'][$options['queryParam']] = $redirect;
}

return Router::url($url);
Expand Down
11 changes: 10 additions & 1 deletion src/Middleware/UnauthorizedHandler/RedirectHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,16 @@ protected function getUrl(ServerRequestInterface $request, array $options): stri
{
$url = $options['url'];
if ($options['queryParam'] !== null && $request->getMethod() === 'GET') {
$query = urlencode($options['queryParam']) . '=' . urlencode($request->getRequestTarget());
$uri = $request->getUri();
/** @psalm-suppress NoInterfaceProperties */
if (property_exists($uri, 'base')) {
$uri = $uri->withPath($uri->base . $uri->getPath());
}
$redirect = $uri->getPath();
if ($uri->getQuery()) {
$redirect .= '?' . $uri->getQuery();
}
$query = urlencode($options['queryParam']) . '=' . urlencode($redirect);
if (strpos($url, '?') !== false) {
$query = '&' . $query;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@

use Authorization\Exception\Exception;
use Authorization\Middleware\UnauthorizedHandler\CakeRedirectHandler;
use Cake\Http\ServerRequest;
use Cake\Core\Configure;
use Cake\Http\ServerRequestFactory;
use Cake\Routing\Router;
use Cake\TestSuite\TestCase;

/**
* Description of CakeRedirectHandlerTest
*/
class CakeRedirectHandlerTest extends TestCase
{
public function setUp(): void
Expand All @@ -46,24 +44,27 @@ public function testHandleRedirectionDefault()
$handler = new CakeRedirectHandler();

$exception = new Exception();
$request = new ServerRequest();

$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/admin/dashboard']
);
$response = $handler->handle($exception, $request, [
'exceptions' => [
Exception::class,
],
]);

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

public function testHandleRedirectionNamed()
{
$handler = new CakeRedirectHandler();

$exception = new Exception();
$request = new ServerRequest();
$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/admin/dashboard']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -75,15 +76,17 @@ public function testHandleRedirectionNamed()
]);

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

public function testHandleRedirectionWithQuery()
{
$handler = new CakeRedirectHandler();

$exception = new Exception();
$request = new ServerRequest();
$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -98,15 +101,17 @@ public function testHandleRedirectionWithQuery()
]);

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

public function testHandleRedirectionNoQuery()
{
$handler = new CakeRedirectHandler();

$exception = new Exception();
$request = new ServerRequest();
$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -118,4 +123,30 @@ public function testHandleRedirectionNoQuery()
$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals('/login', $response->getHeaderLine('Location'));
}

public function testHandleRedirectWithBasePath()
{
$handler = new CakeRedirectHandler();
$exception = new Exception();

Configure::write('App.base', '/basedir');
$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/admin/dashboard']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Exception::class,
],
'url' => [
'_name' => 'login',
],
]);

$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(
'/basedir/login?redirect=%2Fbasedir%2Fadmin%2Fdashboard',
$response->getHeaderLine('Location')
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

use Authorization\Exception\Exception;
use Authorization\Middleware\UnauthorizedHandler\RedirectHandler;
use Cake\Http\ServerRequest;
use Cake\Core\Configure;
use Cake\Http\ServerRequestFactory;
use Cake\TestSuite\TestCase;

class RedirectHandlerTest extends TestCase
Expand All @@ -28,9 +29,9 @@ public function testHandleRedirection()
$handler = new RedirectHandler();

$exception = new Exception();
$request = new ServerRequest([
'environment' => ['REQUEST_METHOD' => 'GET'],
]);
$request = ServerRequestFactory::fromGlobals(
['REQUEST_METHOD' => 'GET']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -47,13 +48,13 @@ public function testHandleRedirectionWithQuery()
$handler = new RedirectHandler();

$exception = new Exception();
$request = new ServerRequest([
'environment' => [
$request = ServerRequestFactory::fromGlobals(
[
'REQUEST_METHOD' => 'GET',
'PATH_INFO' => '/path',
'QUERY_STRING' => 'key=value',
],
]);
]
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -71,9 +72,9 @@ public function testHandleRedirectionNoQuery()
$handler = new RedirectHandler();

$exception = new Exception();
$request = new ServerRequest([
'environment' => ['REQUEST_METHOD' => 'GET'],
]);
$request = ServerRequestFactory::fromGlobals(
['REQUEST_METHOD' => 'GET']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand Down Expand Up @@ -107,13 +108,13 @@ public function testHandleRedirectionIgnoreNonIdempotentMethods($method)
$handler = new RedirectHandler();

$exception = new Exception();
$request = new ServerRequest([
'environment' => [
$request = ServerRequestFactory::fromGlobals(
[
'REQUEST_METHOD' => $method,
'PATH_INFO' => '/path',
'QUERY_STRING' => 'key=value',
],
]);
]
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Expand All @@ -126,12 +127,36 @@ public function testHandleRedirectionIgnoreNonIdempotentMethods($method)
$this->assertEquals('/login?foo=bar', $response->getHeaderLine('Location'));
}

public function testHandleRedirectWithBasePath()
{
$handler = new RedirectHandler();
$exception = new Exception();

Configure::write('App.base', '/basedir');
$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/path', 'REQUEST_METHOD' => 'GET']
);

$response = $handler->handle($exception, $request, [
'exceptions' => [
Exception::class,
],
'url' => '/basedir/login',
]);

$this->assertEquals(302, $response->getStatusCode());
$this->assertEquals(
'/basedir/login?redirect=%2Fbasedir%2Fpath',
$response->getHeaderLine('Location')
);
}

public function testHandleException()
{
$handler = new RedirectHandler();

$exception = new Exception();
$request = new ServerRequest();
$request = ServerRequestFactory::fromGlobals(['REQUEST_URI' => '/']);

$this->expectException(Exception::class);
$handler->handle($exception, $request);
Expand Down
1 change: 1 addition & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

Configure::write('debug', true);
Configure::write('App', [
'base' => '',
'namespace' => 'TestApp',
'encoding' => 'UTF-8',
'paths' => [
Expand Down

0 comments on commit 3930120

Please sign in to comment.