Skip to content

Commit 7e250f1

Browse files
Merge branch '6.4' into 7.0
* 6.4: [Serializer] Fix deserializing of nested snake_case attributes using CamelCaseToSnakeCaseNameConverter [Security] FormLoginAuthenticator: fail for non-string password [FrameworkBundle] Improve deprecation for handler_id/save_path config options [Serializer] Fix deserializing object collection properties [HttpKernel] Fix checking for the runtime mode in DebugLoggerConfigurator [Serializer] Fix serialized name with groups Hide username and client ip in logs
2 parents d9dd1b0 + 0b1daf3 commit 7e250f1

File tree

4 files changed

+61
-4
lines changed

4 files changed

+61
-4
lines changed

Authenticator/FormLoginAuthenticator.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ private function getCredentials(Request $request): array
131131

132132
$request->getSession()->set(SecurityRequestAttributes::LAST_USERNAME, $credentials['username']);
133133

134+
if (!\is_string($credentials['password']) && (!\is_object($credentials['password']) || !method_exists($credentials['password'], '__toString'))) {
135+
throw new BadRequestHttpException(sprintf('The key "%s" must be a string, "%s" given.', $this->options['password_parameter'], \gettype($credentials['password'])));
136+
}
137+
134138
return $credentials;
135139
}
136140

RateLimiter/DefaultLoginRateLimiter.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,20 @@ final class DefaultLoginRateLimiter extends AbstractRequestRateLimiter
2828
{
2929
private RateLimiterFactory $globalFactory;
3030
private RateLimiterFactory $localFactory;
31+
private string $secret;
3132

32-
public function __construct(RateLimiterFactory $globalFactory, RateLimiterFactory $localFactory)
33+
/**
34+
* @param non-empty-string $secret A secret to use for hashing the IP address and username
35+
*/
36+
public function __construct(RateLimiterFactory $globalFactory, RateLimiterFactory $localFactory, #[\SensitiveParameter] string $secret = '')
3337
{
38+
if ('' === $secret) {
39+
trigger_deprecation('symfony/security-http', '6.4', 'Calling "%s()" with an empty secret is deprecated. A non-empty secret will be mandatory in version 7.0.', __METHOD__);
40+
// throw new \Symfony\Component\Security\Core\Exception\InvalidArgumentException('A non-empty secret is required.');
41+
}
3442
$this->globalFactory = $globalFactory;
3543
$this->localFactory = $localFactory;
44+
$this->secret = $secret;
3645
}
3746

3847
protected function getLimiters(Request $request): array
@@ -41,8 +50,13 @@ protected function getLimiters(Request $request): array
4150
$username = preg_match('//u', $username) ? mb_strtolower($username, 'UTF-8') : strtolower($username);
4251

4352
return [
44-
$this->globalFactory->create($request->getClientIp()),
45-
$this->localFactory->create($username.'-'.$request->getClientIp()),
53+
$this->globalFactory->create($this->hash($request->getClientIp())),
54+
$this->localFactory->create($this->hash($username.'-'.$request->getClientIp())),
4655
];
4756
}
57+
58+
private function hash(string $data): string
59+
{
60+
return strtr(substr(base64_encode(hash_hmac('sha256', $data, $this->secret, true)), 0, 8), '/+', '._');
61+
}
4862
}

Tests/Authenticator/FormLoginAuthenticatorTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge;
2525
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\PasswordUpgradeBadge;
2626
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
27+
use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials;
2728
use Symfony\Component\Security\Http\HttpUtils;
2829
use Symfony\Component\Security\Http\Tests\Authenticator\Fixtures\PasswordUpgraderProvider;
2930

@@ -126,6 +127,44 @@ public function testHandleNonStringUsernameWithToString($postOnly)
126127
$this->authenticator->authenticate($request);
127128
}
128129

130+
/**
131+
* @dataProvider postOnlyDataProvider
132+
*/
133+
public function testHandleNonStringPasswordWithArray(bool $postOnly)
134+
{
135+
$this->expectException(BadRequestHttpException::class);
136+
$this->expectExceptionMessage('The key "_password" must be a string, "array" given.');
137+
138+
$request = Request::create('/login_check', 'POST', ['_username' => 'foo', '_password' => []]);
139+
$request->setSession($this->createSession());
140+
141+
$this->setUpAuthenticator(['post_only' => $postOnly]);
142+
$this->authenticator->authenticate($request);
143+
}
144+
145+
/**
146+
* @dataProvider postOnlyDataProvider
147+
*/
148+
public function testHandleNonStringPasswordWithToString(bool $postOnly)
149+
{
150+
$passwordObject = new class() {
151+
public function __toString()
152+
{
153+
return 's$cr$t';
154+
}
155+
};
156+
157+
$request = Request::create('/login_check', 'POST', ['_username' => 'foo', '_password' => $passwordObject]);
158+
$request->setSession($this->createSession());
159+
160+
$this->setUpAuthenticator(['post_only' => $postOnly]);
161+
$passport = $this->authenticator->authenticate($request);
162+
163+
/** @var PasswordCredentials $credentialsBadge */
164+
$credentialsBadge = $passport->getBadge(PasswordCredentials::class);
165+
$this->assertSame('s$cr$t', $credentialsBadge->getPassword());
166+
}
167+
129168
public static function postOnlyDataProvider()
130169
{
131170
yield [true];

Tests/EventListener/LoginThrottlingListenerTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ protected function setUp(): void
4747
'limit' => 6,
4848
'interval' => '1 minute',
4949
], new InMemoryStorage());
50-
$limiter = new DefaultLoginRateLimiter($globalLimiter, $localLimiter);
50+
$limiter = new DefaultLoginRateLimiter($globalLimiter, $localLimiter, '$3cre7');
5151

5252
$this->listener = new LoginThrottlingListener($this->requestStack, $limiter);
5353
}

0 commit comments

Comments
 (0)