Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 20bcf1e

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.1' into 6.2
* 6.1: [Security/Http] Check tokens before loading users from providers
2 parents 8d641d7 + 69aac17 commit 20bcf1e
Copy full SHA for 20bcf1e

File tree

Expand file treeCollapse file tree

9 files changed

+117
-92
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+117
-92
lines changed

‎src/Symfony/Component/Security/Core/Signature/SignatureHasher.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Signature/SignatureHasher.php
+38-6Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,24 +45,48 @@ public function __construct(PropertyAccessorInterface $propertyAccessor, array $
4545
}
4646

4747
/**
48-
* Verifies the hash using the provided user and expire time.
48+
* Verifies the hash using the provided user identifier and expire time.
49+
*
50+
* This method must be called before the user object is loaded from a provider.
4951
*
5052
* @param int $expires The expiry time as a unix timestamp
5153
* @param string $hash The plaintext hash provided by the request
5254
*
5355
* @throws InvalidSignatureException If the signature does not match the provided parameters
5456
* @throws ExpiredSignatureException If the signature is no longer valid
5557
*/
56-
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
58+
public function acceptSignatureHash(string $userIdentifier, int $expires, string $hash): void
5759
{
58-
if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
60+
if ($expires < time()) {
61+
throw new ExpiredSignatureException('Signature has expired.');
62+
}
63+
$hmac = substr($hash, 0, 44);
64+
$payload = substr($hash, 44).':'.$expires.':'.$userIdentifier;
65+
66+
if (!hash_equals($hmac, $this->generateHash($payload))) {
5967
throw new InvalidSignatureException('Invalid or expired signature.');
6068
}
69+
}
6170

71+
/**
72+
* Verifies the hash using the provided user and expire time.
73+
*
74+
* @param int $expires The expiry time as a unix timestamp
75+
* @param string $hash The plaintext hash provided by the request
76+
*
77+
* @throws InvalidSignatureException If the signature does not match the provided parameters
78+
* @throws ExpiredSignatureException If the signature is no longer valid
79+
*/
80+
public function verifySignatureHash(UserInterface $user, int $expires, string $hash): void
81+
{
6282
if ($expires < time()) {
6383
throw new ExpiredSignatureException('Signature has expired.');
6484
}
6585

86+
if (!hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
87+
throw new InvalidSignatureException('Invalid or expired signature.');
88+
}
89+
6690
if ($this->expiredSignaturesStorage && $this->maxUses) {
6791
if ($this->expiredSignaturesStorage->countUsages($hash) >= $this->maxUses) {
6892
throw new ExpiredSignatureException(sprintf('Signature can only be used "%d" times.', $this->maxUses));
@@ -79,7 +103,8 @@ public function verifySignatureHash(UserInterface $user, int $expires, string $h
79103
*/
80104
public function computeSignatureHash(UserInterface $user, int $expires): string
81105
{
82-
$signatureFields = [base64_encode($user->getUserIdentifier()), $expires];
106+
$userIdentifier = $user->getUserIdentifier();
107+
$fieldsHash = hash_init('sha256');
83108

84109
foreach ($this->signatureProperties as $property) {
85110
$value = $this->propertyAccessor->getValue($user, $property) ?? '';
@@ -90,9 +115,16 @@ public function computeSignatureHash(UserInterface $user, int $expires): string
90115
if (!\is_scalar($value) && !$value instanceof \Stringable) {
91116
throw new \InvalidArgumentException(sprintf('The property path "%s" on the user object "%s" must return a value that can be cast to a string, but "%s" was returned.', $property, $user::class, get_debug_type($value)));
92117
}
93-
$signatureFields[] = base64_encode($value);
118+
hash_update($fieldsHash, ':'.base64_encode($value));
94119
}
95120

96-
return base64_encode(hash_hmac('sha256', implode(':', $signatureFields), $this->secret));
121+
$fieldsHash = strtr(base64_encode(hash_final($fieldsHash, true)), '+/=', '-_~');
122+
123+
return $this->generateHash($fieldsHash.':'.$expires.':'.$userIdentifier).$fieldsHash;
124+
}
125+
126+
private function generateHash(string $tokenValue): string
127+
{
128+
return strtr(base64_encode(hash_hmac('sha256', $tokenValue, $this->secret, true)), '+/=', '-_~');
97129
}
98130
}

‎src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ public function consumeLoginLink(Request $request): UserInterface
8383
{
8484
$userIdentifier = $request->get('user');
8585

86-
try {
87-
$user = $this->userProvider->loadUserByIdentifier($userIdentifier);
88-
} catch (UserNotFoundException $exception) {
89-
throw new InvalidLoginLinkException('User not found.', 0, $exception);
90-
}
91-
9286
if (!$hash = $request->get('hash')) {
9387
throw new InvalidLoginLinkException('Missing "hash" parameter.');
9488
}
@@ -97,7 +91,13 @@ public function consumeLoginLink(Request $request): UserInterface
9791
}
9892

9993
try {
94+
$this->signatureHasher->acceptSignatureHash($userIdentifier, $expires, $hash);
95+
96+
$user = $this->userProvider->loadUserByIdentifier($userIdentifier);
97+
10098
$this->signatureHasher->verifySignatureHash($user, $expires, $hash);
99+
} catch (UserNotFoundException $e) {
100+
throw new InvalidLoginLinkException('User not found.', 0, $e);
101101
} catch (ExpiredSignatureException $e) {
102102
throw new ExpiredLoginLinkException(ucfirst(str_ireplace('signature', 'login link', $e->getMessage())), 0, $e);
103103
} catch (InvalidSignatureException $e) {

‎src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/PersistentRememberMeHandler.php
+22-16Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,16 @@ public function __construct(TokenProviderInterface $tokenProvider, #[\SensitiveP
5050

5151
public function createRememberMeCookie(UserInterface $user): void
5252
{
53-
$series = base64_encode(random_bytes(64));
54-
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
53+
$series = random_bytes(66);
54+
$tokenValue = strtr(base64_encode(substr($series, 33)), '+/=', '-_~');
55+
$series = strtr(base64_encode(substr($series, 0, 33)), '+/=', '-_~');
5556
$token = new PersistentToken($user::class, $user->getUserIdentifier(), $series, $tokenValue, new \DateTime());
5657

5758
$this->tokenProvider->createNewToken($token);
5859
$this->createCookie(RememberMeDetails::fromPersistentToken($token, time() + $this->options['lifetime']));
5960
}
6061

61-
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
62+
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
6263
{
6364
if (!str_contains($rememberMeDetails->getValue(), ':')) {
6465
throw new AuthenticationException('The cookie is incorrectly formatted.');
@@ -80,16 +81,26 @@ public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInte
8081
throw new AuthenticationException('The cookie has expired.');
8182
}
8283

84+
return parent::consumeRememberMeCookie($rememberMeDetails->withValue($persistentToken->getLastUsed()->getTimestamp().':'.$rememberMeDetails->getValue().':'.$persistentToken->getClass()));
85+
}
86+
87+
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
88+
{
89+
[$lastUsed, $series, $tokenValue, $class] = explode(':', $rememberMeDetails->getValue(), 4);
90+
$persistentToken = new PersistentToken($class, $rememberMeDetails->getUserIdentifier(), $series, $tokenValue, new \DateTime('@'.$lastUsed));
91+
8392
// if a token was regenerated less than a minute ago, there is no need to regenerate it
8493
// if multiple concurrent requests reauthenticate a user we do not want to update the token several times
85-
if ($persistentToken->getLastUsed()->getTimestamp() + 60 < time()) {
86-
$tokenValue = $this->generateHash(base64_encode(random_bytes(64)));
87-
$tokenLastUsed = new \DateTime();
88-
$this->tokenVerifier?->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
89-
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);
90-
91-
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
94+
if ($persistentToken->getLastUsed()->getTimestamp() + 60 >= time()) {
95+
return;
9296
}
97+
98+
$tokenValue = strtr(base64_encode(random_bytes(33)), '+/=', '-_~');
99+
$tokenLastUsed = new \DateTime();
100+
$this->tokenVerifier?->updateExistingToken($persistentToken, $tokenValue, $tokenLastUsed);
101+
$this->tokenProvider->updateToken($series, $tokenValue, $tokenLastUsed);
102+
103+
$this->createCookie($rememberMeDetails->withValue($series.':'.$tokenValue));
93104
}
94105

95106
public function clearRememberMeCookie(): void
@@ -102,7 +113,7 @@ public function clearRememberMeCookie(): void
102113
}
103114

104115
$rememberMeDetails = RememberMeDetails::fromRawCookie($cookie);
105-
[$series, ] = explode(':', $rememberMeDetails->getValue());
116+
[$series] = explode(':', $rememberMeDetails->getValue());
106117
$this->tokenProvider->deleteTokenBySeries($series);
107118
}
108119

@@ -113,9 +124,4 @@ public function getTokenProvider(): TokenProviderInterface
113124
{
114125
return $this->tokenProvider;
115126
}
116-
117-
private function generateHash(string $tokenValue): string
118-
{
119-
return hash_hmac('sha256', $tokenValue, $this->secret);
120-
}
121127
}

‎src/Symfony/Component/Security/Http/RememberMe/RememberMeDetails.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/RememberMeDetails.php
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,14 @@ public function __construct(string $userFqcn, string $userIdentifier, int $expir
3636

3737
public static function fromRawCookie(string $rawCookie): self
3838
{
39-
$cookieParts = explode(self::COOKIE_DELIMITER, base64_decode($rawCookie), 4);
39+
$cookieParts = explode(self::COOKIE_DELIMITER, $rawCookie, 4);
4040
if (4 !== \count($cookieParts)) {
4141
throw new AuthenticationException('The cookie contains invalid data.');
4242
}
43-
if (false === $cookieParts[1] = base64_decode($cookieParts[1], true)) {
43+
if (false === $cookieParts[1] = base64_decode(strtr($cookieParts[1], '-_~', '+/='), true)) {
4444
throw new AuthenticationException('The user identifier contains a character from outside the base64 alphabet.');
4545
}
46+
$cookieParts[0] = strtr($cookieParts[0], '.', '\\');
4647

4748
return new static(...$cookieParts);
4849
}
@@ -83,6 +84,6 @@ public function getValue(): string
8384
public function toString(): string
8485
{
8586
// $userIdentifier is encoded because it might contain COOKIE_DELIMITER, we assume other values don't
86-
return base64_encode(implode(self::COOKIE_DELIMITER, [$this->userFqcn, base64_encode($this->userIdentifier), $this->expires, $this->value]));
87+
return implode(self::COOKIE_DELIMITER, [strtr($this->userFqcn, '\\', '.'), strtr(base64_encode($this->userIdentifier), '+/=', '-_~'), $this->expires, $this->value]);
8788
}
8889
}

‎src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/RememberMe/SignatureRememberMeHandler.php
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,19 @@ public function createRememberMeCookie(UserInterface $user): void
5050
$this->createCookie($details);
5151
}
5252

53+
public function consumeRememberMeCookie(RememberMeDetails $rememberMeDetails): UserInterface
54+
{
55+
try {
56+
$this->signatureHasher->acceptSignatureHash($rememberMeDetails->getUserIdentifier(), $rememberMeDetails->getExpires(), $rememberMeDetails->getValue());
57+
} catch (InvalidSignatureException $e) {
58+
throw new AuthenticationException('The cookie\'s hash is invalid.', 0, $e);
59+
} catch (ExpiredSignatureException $e) {
60+
throw new AuthenticationException('The cookie has expired.', 0, $e);
61+
}
62+
63+
return parent::consumeRememberMeCookie($rememberMeDetails);
64+
}
65+
5366
public function processRememberMe(RememberMeDetails $rememberMeDetails, UserInterface $user): void
5467
{
5568
try {

‎src/Symfony/Component/Security/Http/Tests/LoginLink/LoginLinkHandlerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/LoginLink/LoginLinkHandlerTest.php
+16-24Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ protected function setUp(): void
5353

5454
/**
5555
* @group time-sensitive
56+
*
5657
* @dataProvider provideCreateLoginLinkData
5758
*/
5859
public function testCreateLoginLink($user, array $extraProperties, Request $request = null)
@@ -68,7 +69,7 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ
6869
// allow a small expiration offset to avoid time-sensitivity
6970
&& abs(time() + 600 - $parameters['expires']) <= 1
7071
// make sure hash is what we expect
71-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties));
72+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties);
7273
}),
7374
UrlGeneratorInterface::ABSOLUTE_URL
7475
)
@@ -127,7 +128,7 @@ public function testCreateLoginLinkWithLifetime()
127128
&& abs(time() + 1000 - $parameters['expires']) <= 1
128129
&& isset($parameters['hash'])
129130
// make sure hash is what we expect
130-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties));
131+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties);
131132
}),
132133
UrlGeneratorInterface::ABSOLUTE_URL
133134
)
@@ -147,7 +148,7 @@ public function testCreateLoginLinkWithLifetime()
147148
public function testConsumeLoginLink()
148149
{
149150
$expires = time() + 500;
150-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
151+
$signature = $this->createSignatureHash('weaverryan', $expires);
151152
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
152153

153154
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
@@ -163,44 +164,37 @@ public function testConsumeLoginLink()
163164

164165
public function testConsumeLoginLinkWithExpired()
165166
{
166-
$this->expectException(ExpiredLoginLinkException::class);
167167
$expires = time() - 500;
168-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
168+
$signature = $this->createSignatureHash('weaverryan', $expires);
169169
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
170170

171-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
172-
$this->userProvider->createUser($user);
173-
174171
$linker = $this->createLinker(['max_uses' => 3]);
172+
$this->expectException(ExpiredLoginLinkException::class);
175173
$linker->consumeLoginLink($request);
176174
}
177175

178176
public function testConsumeLoginLinkWithUserNotFound()
179177
{
180-
$this->expectException(InvalidLoginLinkException::class);
181-
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires=10000');
178+
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires='.(time() + 500));
182179

183180
$linker = $this->createLinker();
181+
$this->expectException(InvalidLoginLinkException::class);
184182
$linker->consumeLoginLink($request);
185183
}
186184

187185
public function testConsumeLoginLinkWithDifferentSignature()
188186
{
189-
$this->expectException(InvalidLoginLinkException::class);
190187
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=fake_hash&expires=%d', time() + 500));
191188

192-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
193-
$this->userProvider->createUser($user);
194-
195189
$linker = $this->createLinker();
190+
$this->expectException(InvalidLoginLinkException::class);
196191
$linker->consumeLoginLink($request);
197192
}
198193

199194
public function testConsumeLoginLinkExceedsMaxUsage()
200195
{
201-
$this->expectException(ExpiredLoginLinkException::class);
202196
$expires = time() + 500;
203-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
197+
$signature = $this->createSignatureHash('weaverryan', $expires);
204198
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
205199

206200
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
@@ -211,6 +205,7 @@ public function testConsumeLoginLinkExceedsMaxUsage()
211205
$this->expiredLinkCache->save($item);
212206

213207
$linker = $this->createLinker(['max_uses' => 3]);
208+
$this->expectException(ExpiredLoginLinkException::class);
214209
$linker->consumeLoginLink($request);
215210
}
216211

@@ -238,15 +233,12 @@ public function testConsumeLoginLinkWithMissingExpiration()
238233
$linker->consumeLoginLink($request);
239234
}
240235

241-
private function createSignatureHash(string $username, int $expires, array $extraFields): string
236+
private function createSignatureHash(string $username, int $expires, array $extraFields = ['emailProperty' => 'ryan@symfonycasts.com', 'passwordProperty' => 'pwhash']): string
242237
{
243-
$fields = [base64_encode($username), $expires];
244-
foreach ($extraFields as $extraField) {
245-
$fields[] = base64_encode($extraField);
246-
}
238+
$hasher = new SignatureHasher($this->propertyAccessor, array_keys($extraFields), 's3cret');
239+
$user = new TestLoginLinkHandlerUser($username, $extraFields['emailProperty'] ?? '', $extraFields['passwordProperty'] ?? '', $extraFields['lastAuthenticatedAt'] ?? null);
247240

248-
// matches hash logic in the class
249-
return base64_encode(hash_hmac('sha256', implode(':', $fields), 's3cret'));
241+
return $hasher->computeSignatureHash($user, $expires);
250242
}
251243

252244
private function createLinker(array $options = [], array $extraProperties = ['emailProperty', 'passwordProperty']): LoginLinkHandler
@@ -330,7 +322,7 @@ public function loadUserByIdentifier(string $userIdentifier): TestLoginLinkHandl
330322

331323
public function refreshUser(UserInterface $user): TestLoginLinkHandlerUser
332324
{
333-
return $this->users[$username];
325+
return $this->users[$user->getUserIdentifier()];
334326
}
335327

336328
public function supportsClass(string $class): bool

‎src/Symfony/Component/Security/Http/Tests/RememberMe/PersistentRememberMeHandlerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/RememberMe/PersistentRememberMeHandlerTest.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ public function testConsumeRememberMeCookieValid()
9393

9494
/** @var Cookie $cookie */
9595
$cookie = $this->request->attributes->get(ResponseListener::COOKIE_ATTR_NAME);
96-
$rememberParts = explode(':', base64_decode($rememberMeDetails->toString()), 4);
97-
$cookieParts = explode(':', base64_decode($cookie->getValue()), 4);
96+
$rememberParts = explode(':', $rememberMeDetails->toString(), 4);
97+
$cookieParts = explode(':', $cookie->getValue(), 4);
9898

9999
$this->assertSame($rememberParts[0], $cookieParts[0]); // class
100100
$this->assertSame($rememberParts[1], $cookieParts[1]); // identifier

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.