Merge pull request #1560 from ArthurHoaro/fix/redirect-wrong-path

Fix invalid redirection using the path of an external domain
This commit is contained in:
ArthurHoaro 2020-09-25 10:59:51 +02:00 committed by GitHub
commit 585fc700fa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 54 additions and 16 deletions

View file

@ -142,6 +142,13 @@ protected function redirectFromReferer(
if (null !== $referer) { if (null !== $referer) {
$currentUrl = parse_url($referer); $currentUrl = parse_url($referer);
// If the referer is not related to Shaarli instance, redirect to default
if (isset($currentUrl['host'])
&& strpos(index_url($this->container->environment), $currentUrl['host']) === false
) {
return $response->withRedirect($defaultPath);
}
parse_str($currentUrl['query'] ?? '', $params); parse_str($currentUrl['query'] ?? '', $params);
$path = $currentUrl['path'] ?? $defaultPath; $path = $currentUrl['path'] ?? $defaultPath;
} else { } else {

View file

@ -43,7 +43,7 @@ public function testSaveBookmark(): void
'lf_description' => 'Provided description.', 'lf_description' => 'Provided description.',
'lf_tags' => 'abc def', 'lf_tags' => 'abc def',
'lf_private' => '1', 'lf_private' => '1',
'returnurl' => 'http://shaarli.tld/subfolder/admin/add-shaare' 'returnurl' => 'http://shaarli/subfolder/admin/add-shaare'
]; ];
$request = $this->createMock(Request::class); $request = $this->createMock(Request::class);
@ -124,7 +124,7 @@ public function testSaveExistingBookmark(): void
'lf_description' => 'Provided description.', 'lf_description' => 'Provided description.',
'lf_tags' => 'abc def', 'lf_tags' => 'abc def',
'lf_private' => '1', 'lf_private' => '1',
'returnurl' => 'http://shaarli.tld/subfolder/?page=2' 'returnurl' => 'http://shaarli/subfolder/?page=2'
]; ];
$request = $this->createMock(Request::class); $request = $this->createMock(Request::class);

View file

@ -31,7 +31,7 @@ public function testVisibility(): void
{ {
$arg = ['visibility' => 'private']; $arg = ['visibility' => 'private'];
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$this->container->loginManager->method('isLoggedIn')->willReturn(true); $this->container->loginManager->method('isLoggedIn')->willReturn(true);
$this->container->sessionManager $this->container->sessionManager
@ -57,7 +57,7 @@ public function testVisibilityToggleOff(): void
{ {
$arg = ['visibility' => 'private']; $arg = ['visibility' => 'private'];
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$this->container->loginManager->method('isLoggedIn')->willReturn(true); $this->container->loginManager->method('isLoggedIn')->willReturn(true);
$this->container->sessionManager $this->container->sessionManager
@ -121,7 +121,7 @@ public function testVisibilityInvalidValue(): void
{ {
$arg = ['visibility' => 'test']; $arg = ['visibility' => 'test'];
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$this->container->loginManager->method('isLoggedIn')->willReturn(true); $this->container->loginManager->method('isLoggedIn')->willReturn(true);
$this->container->sessionManager $this->container->sessionManager
@ -151,7 +151,7 @@ public function testVisibilityLoggedOut(): void
{ {
$arg = ['visibility' => 'test']; $arg = ['visibility' => 'test'];
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$this->container->loginManager = $this->createMock(LoginManager::class); $this->container->loginManager = $this->createMock(LoginManager::class);
$this->container->loginManager->method('isLoggedIn')->willReturn(false); $this->container->loginManager->method('isLoggedIn')->willReturn(false);

View file

@ -28,7 +28,7 @@ public function setUp(): void
*/ */
public function testLinksPerPage(): void public function testLinksPerPage(): void
{ {
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$request = $this->createMock(Request::class); $request = $this->createMock(Request::class);
$request->method('getParam')->with('nb')->willReturn('8'); $request->method('getParam')->with('nb')->willReturn('8');
@ -74,7 +74,7 @@ public function testLinksPerPageNotValid(): void
*/ */
public function testUntaggedOnly(): void public function testUntaggedOnly(): void
{ {
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$request = $this->createMock(Request::class); $request = $this->createMock(Request::class);
$response = new Response(); $response = new Response();
@ -97,7 +97,7 @@ public function testUntaggedOnly(): void
*/ */
public function testUntaggedOnlyToggleOff(): void public function testUntaggedOnlyToggleOff(): void
{ {
$this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc']; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller/?searchtag=abc';
$request = $this->createMock(Request::class); $request = $this->createMock(Request::class);
$response = new Response(); $response = new Response();

View file

@ -110,7 +110,7 @@ public function testRender(): void
*/ */
public function testRedirectFromRefererDefault(): void public function testRedirectFromRefererDefault(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -125,7 +125,7 @@ public function testRedirectFromRefererDefault(): void
*/ */
public function testRedirectFromRefererWithUnmatchedLoopTerm(): void public function testRedirectFromRefererWithUnmatchedLoopTerm(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -140,7 +140,7 @@ public function testRedirectFromRefererWithUnmatchedLoopTerm(): void
*/ */
public function testRedirectFromRefererWithMatchingLoopTermInPath(): void public function testRedirectFromRefererWithMatchingLoopTermInPath(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -155,7 +155,7 @@ public function testRedirectFromRefererWithMatchingLoopTermInPath(): void
*/ */
public function testRedirectFromRefererWithMatchingLoopTermInQueryParam(): void public function testRedirectFromRefererWithMatchingLoopTermInQueryParam(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -171,7 +171,7 @@ public function testRedirectFromRefererWithMatchingLoopTermInQueryParam(): void
*/ */
public function testRedirectFromRefererWithMatchingLoopTermInQueryValue(): void public function testRedirectFromRefererWithMatchingLoopTermInQueryValue(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -187,7 +187,7 @@ public function testRedirectFromRefererWithMatchingLoopTermInQueryValue(): void
*/ */
public function testRedirectFromRefererWithLoopTermInDomain(): void public function testRedirectFromRefererWithLoopTermInDomain(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -203,7 +203,7 @@ public function testRedirectFromRefererWithLoopTermInDomain(): void
*/ */
public function testRedirectFromRefererWithMatchingClearedParam(): void public function testRedirectFromRefererWithMatchingClearedParam(): void
{ {
$this->container->environment['HTTP_REFERER'] = 'http://shaarli.tld/subfolder/controller?query=param&other=2'; $this->container->environment['HTTP_REFERER'] = 'http://shaarli/subfolder/controller?query=param&other=2';
$response = new Response(); $response = new Response();
@ -212,4 +212,35 @@ public function testRedirectFromRefererWithMatchingClearedParam(): void
static::assertSame(302, $result->getStatusCode()); static::assertSame(302, $result->getStatusCode());
static::assertSame(['/subfolder/controller?other=2'], $result->getHeader('location')); static::assertSame(['/subfolder/controller?other=2'], $result->getHeader('location'));
} }
/**
* Test redirectFromReferer() - From another domain -> we ignore the given referrer.
*/
public function testRedirectExternalReferer(): void
{
$this->container->environment['HTTP_REFERER'] = 'http://other.domain.tld/controller?query=param&other=2';
$response = new Response();
$result = $this->controller->redirectFromReferer($this->request, $response, ['query'], ['query']);
static::assertSame(302, $result->getStatusCode());
static::assertSame(['/subfolder/'], $result->getHeader('location'));
}
/**
* Test redirectFromReferer() - From another domain -> we ignore the given referrer.
*/
public function testRedirectExternalRefererExplicitDomainName(): void
{
$this->container->environment['SERVER_NAME'] = 'my.shaarli.tld';
$this->container->environment['HTTP_REFERER'] = 'http://your.shaarli.tld/controller?query=param&other=2';
$response = new Response();
$result = $this->controller->redirectFromReferer($this->request, $response, ['query'], ['query']);
static::assertSame(302, $result->getStatusCode());
static::assertSame(['/subfolder/'], $result->getHeader('location'));
}
} }