Skip to content

Commit 64b65f2

Browse files
committed
[Security] Fix UserBadge validation bypass via identifier normalizer
1 parent 9b42217 commit 64b65f2

File tree

2 files changed

+38
-7
lines changed

2 files changed

+38
-7
lines changed

Authenticator/Passport/Badge/UserBadge.php

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,8 @@ public function __construct(
5454
private ?array $attributes = null,
5555
?\Closure $identifierNormalizer = null,
5656
) {
57-
if ('' === $userIdentifier) {
58-
trigger_deprecation('symfony/security-http', '7.2', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
59-
// throw new BadCredentialsException('Empty user identifier.');
60-
}
57+
$this->validateUserIdentifier($userIdentifier);
6158

62-
if (\strlen($userIdentifier) > self::MAX_USERNAME_LENGTH) {
63-
throw new BadCredentialsException('Username too long.');
64-
}
6559
if ($identifierNormalizer) {
6660
$this->identifierNormalizer = static fn () => $identifierNormalizer($userIdentifier);
6761
}
@@ -74,6 +68,8 @@ public function getUserIdentifier(): string
7468
if (isset($this->identifierNormalizer)) {
7569
$this->userIdentifier = ($this->identifierNormalizer)();
7670
$this->identifierNormalizer = null;
71+
72+
$this->validateUserIdentifier($this->userIdentifier);
7773
}
7874

7975
return $this->userIdentifier;
@@ -132,4 +128,16 @@ public function isResolved(): bool
132128
{
133129
return true;
134130
}
131+
132+
private function validateUserIdentifier(string $userIdentifier): void
133+
{
134+
if ('' === $userIdentifier) {
135+
trigger_deprecation('symfony/security-http', '7.2', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
136+
// throw new BadCredentialsException('Empty user identifier.');
137+
}
138+
139+
if (\strlen($userIdentifier) > self::MAX_USERNAME_LENGTH) {
140+
throw new BadCredentialsException('Username too long.');
141+
}
142+
}
135143
}

Tests/Authenticator/Passport/Badge/UserBadgeTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait;
16+
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1617
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
1718
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
1819
use Symfony\Component\String\Slugger\AsciiSlugger;
@@ -69,4 +70,26 @@ public static function provideUserIdentifierNormalizationData(): iterable
6970
yield 'Greek to ASCII' => ['ΝιΚόΛΑος', 'NIKOLAOS', $upperAndAscii];
7071
yield 'Katakana to ASCII' => ['たなかそういち', 'TANAKASOUICHI', $upperAndAscii];
7172
}
73+
74+
/**
75+
* @group legacy
76+
*/
77+
public function testUserIdentifierNormalizationTriggersDeprecationForEmptyString()
78+
{
79+
$badge = new UserBadge('valid_input', null, null, fn () => '');
80+
81+
$this->expectUserDeprecationMessage('Since symfony/security-http 7.2: Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
82+
83+
$this->assertSame('', $badge->getUserIdentifier());
84+
}
85+
86+
public function testUserIdentifierNormalizationEnforcesMaxLength()
87+
{
88+
$badge = new UserBadge('valid_input', null, null, fn () => str_repeat('a', UserBadge::MAX_USERNAME_LENGTH + 1));
89+
90+
$this->expectException(BadCredentialsException::class);
91+
$this->expectExceptionMessage('Username too long.');
92+
93+
$badge->getUserIdentifier();
94+
}
7295
}

0 commit comments

Comments
 (0)