Skip to content

Commit 64e9926

Browse files
committed
bug #47317 [Security] Fix login url matching when app is not run with url rewriting or from a sub folder (sgehrig)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Security] Fix login url matching when app is not run with url rewriting or from a sub folder | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44893 | License | MIT | Doc PR | not required Uses the fix suggested by `@weaverryan` in symfony/symfony#44893 (comment). I also added three tests for scenarios which I could replicate from running a simple app on a real webserver (Apache and Nginx). This, however, might not be sufficient because there could be other combinations of server variables like `DOCUMENT_ROOT`, `PHP_SELF`, `SCRIPT_FILENAME`, `SCRIPT_NAME` and possibly others depending on the server configuration and setup. As long as `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` and `\Symfony\Component\HttpFoundation\Request::getPathInfo()` work correctly, I assume that the fix will also be correct in all those constellations. The fix is based on the assumptions that: - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns an empty string when the application is run from root without the front controller script in the URL (using URL rewriting for example) - `\Symfony\Component\HttpFoundation\Request::getBaseUrl()` always returns the path from the server root to the application base path (possibly including the front controller script) - `\Symfony\Component\HttpFoundation\Request::getPathInfo()` always returns just the *routed* part of the request Please advise if you'd need some more tests. Commits ------- ff340e2128 [Security] Fix login url matching when app is not run with url rewriting or from a sub folder
2 parents 45ced25 + 72f0a16 commit 64e9926

File tree

2 files changed

+122
-1
lines changed

2 files changed

+122
-1
lines changed

Authenticator/AbstractLoginFormAuthenticator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ abstract protected function getLoginUrl(Request $request): string;
4141
*/
4242
public function supports(Request $request): bool
4343
{
44-
return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getPathInfo();
44+
return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getBaseUrl().$request->getPathInfo();
4545
}
4646

4747
/**
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Http\Tests\Authenticator;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\HttpFoundation\Response;
17+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
18+
use Symfony\Component\Security\Http\Authenticator\AbstractLoginFormAuthenticator;
19+
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
20+
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
21+
22+
class AbstractLoginFormAuthenticatorTest extends TestCase
23+
{
24+
/**
25+
* @dataProvider provideSupportsData
26+
*/
27+
public function testSupports(string $loginUrl, Request $request, bool $expected)
28+
{
29+
$authenticator = new ConcreteFormAuthenticator($loginUrl);
30+
$this->assertSame($expected, $authenticator->supports($request));
31+
}
32+
33+
public function provideSupportsData(): iterable
34+
{
35+
yield [
36+
'/login',
37+
Request::create('http://localhost/login', Request::METHOD_POST, [], [], [], [
38+
'DOCUMENT_ROOT' => '/var/www/app/public',
39+
'PHP_SELF' => '/index.php',
40+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
41+
'SCRIPT_NAME' => '/index.php',
42+
]),
43+
true,
44+
];
45+
yield [
46+
'/login',
47+
Request::create('http://localhost/somepath', Request::METHOD_POST, [], [], [], [
48+
'DOCUMENT_ROOT' => '/var/www/app/public',
49+
'PHP_SELF' => '/index.php',
50+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
51+
'SCRIPT_NAME' => '/index.php',
52+
]),
53+
false,
54+
];
55+
yield [
56+
'/folder/login',
57+
Request::create('http://localhost/folder/login', Request::METHOD_POST, [], [], [], [
58+
'DOCUMENT_ROOT' => '/var/www/app/public',
59+
'PHP_SELF' => '/folder/index.php',
60+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
61+
'SCRIPT_NAME' => '/folder/index.php',
62+
]),
63+
true,
64+
];
65+
yield [
66+
'/folder/login',
67+
Request::create('http://localhost/folder/somepath', Request::METHOD_POST, [], [], [], [
68+
'DOCUMENT_ROOT' => '/var/www/app/public',
69+
'PHP_SELF' => '/folder/index.php',
70+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
71+
'SCRIPT_NAME' => '/folder/index.php',
72+
]),
73+
false,
74+
];
75+
yield [
76+
'/index.php/login',
77+
Request::create('http://localhost/index.php/login', Request::METHOD_POST, [], [], [], [
78+
'DOCUMENT_ROOT' => '/var/www/app/public',
79+
'PHP_SELF' => '/index.php',
80+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
81+
'SCRIPT_NAME' => '/index.php',
82+
]),
83+
true,
84+
];
85+
yield [
86+
'/index.php/login',
87+
Request::create('http://localhost/index.php/somepath', Request::METHOD_POST, [], [], [], [
88+
'DOCUMENT_ROOT' => '/var/www/app/public',
89+
'PHP_SELF' => '/index.php',
90+
'SCRIPT_FILENAME' => '/var/www/app/public/index.php',
91+
'SCRIPT_NAME' => '/index.php',
92+
]),
93+
false,
94+
];
95+
}
96+
}
97+
98+
class ConcreteFormAuthenticator extends AbstractLoginFormAuthenticator
99+
{
100+
private $loginUrl;
101+
102+
public function __construct(string $loginUrl)
103+
{
104+
$this->loginUrl = $loginUrl;
105+
}
106+
107+
protected function getLoginUrl(Request $request): string
108+
{
109+
return $this->loginUrl;
110+
}
111+
112+
public function authenticate(Request $request)
113+
{
114+
return new SelfValidatingPassport(new UserBadge('dummy'));
115+
}
116+
117+
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
118+
{
119+
return null;
120+
}
121+
}

0 commit comments

Comments
 (0)