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 e480a66

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

File tree

Expand file treeCollapse file tree

9 files changed

+121
-94
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+121
-94
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
+20-26Lines changed: 20 additions & 26 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)
@@ -61,13 +62,14 @@ public function testCreateLoginLink($user, array $extraProperties, Request $requ
6162
->method('generate')
6263
->with(
6364
'app_check_login_link_route',
64-
$this->callback(fn ($parameters) => 'weaverryan' == $parameters['user']
65+
$this->callback(fn ($parameters) => 'weaverryan' === $parameters['user']
6566
&& isset($parameters['expires'])
6667
&& isset($parameters['hash'])
6768
// allow a small expiration offset to avoid time-sensitivity
6869
&& abs(time() + 600 - $parameters['expires']) <= 1
6970
// make sure hash is what we expect
70-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties))),
71+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties)
72+
),
7173
UrlGeneratorInterface::ABSOLUTE_URL
7274
)
7375
->willReturn('https://example.com/login/verify?user=weaverryan&hash=abchash&expires=1601235000');
@@ -118,13 +120,14 @@ public function testCreateLoginLinkWithLifetime()
118120
->method('generate')
119121
->with(
120122
'app_check_login_link_route',
121-
$this->callback(fn ($parameters) => 'weaverryan' == $parameters['user']
123+
$this->callback(fn ($parameters) => 'weaverryan' === $parameters['user']
122124
&& isset($parameters['expires'])
123125
// allow a small expiration offset to avoid time-sensitivity
124126
&& abs(time() + 1000 - $parameters['expires']) <= 1
125127
&& isset($parameters['hash'])
126128
// make sure hash is what we expect
127-
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], array_values($extraProperties))),
129+
&& $parameters['hash'] === $this->createSignatureHash('weaverryan', $parameters['expires'], $extraProperties)
130+
),
128131
UrlGeneratorInterface::ABSOLUTE_URL
129132
)
130133
->willReturn('https://example.com/login/verify?user=weaverryan&hash=abchash&expires=1654244256');
@@ -143,7 +146,7 @@ public function testCreateLoginLinkWithLifetime()
143146
public function testConsumeLoginLink()
144147
{
145148
$expires = time() + 500;
146-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
149+
$signature = $this->createSignatureHash('weaverryan', $expires);
147150
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
148151

149152
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
@@ -159,44 +162,37 @@ public function testConsumeLoginLink()
159162

160163
public function testConsumeLoginLinkWithExpired()
161164
{
162-
$this->expectException(ExpiredLoginLinkException::class);
163165
$expires = time() - 500;
164-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
166+
$signature = $this->createSignatureHash('weaverryan', $expires);
165167
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
166168

167-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
168-
$this->userProvider->createUser($user);
169-
170169
$linker = $this->createLinker(['max_uses' => 3]);
170+
$this->expectException(ExpiredLoginLinkException::class);
171171
$linker->consumeLoginLink($request);
172172
}
173173

174174
public function testConsumeLoginLinkWithUserNotFound()
175175
{
176-
$this->expectException(InvalidLoginLinkException::class);
177-
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires=10000');
176+
$request = Request::create('/login/verify?user=weaverryan&hash=thehash&expires='.(time() + 500));
178177

179178
$linker = $this->createLinker();
179+
$this->expectException(InvalidLoginLinkException::class);
180180
$linker->consumeLoginLink($request);
181181
}
182182

183183
public function testConsumeLoginLinkWithDifferentSignature()
184184
{
185-
$this->expectException(InvalidLoginLinkException::class);
186185
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=fake_hash&expires=%d', time() + 500));
187186

188-
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
189-
$this->userProvider->createUser($user);
190-
191187
$linker = $this->createLinker();
188+
$this->expectException(InvalidLoginLinkException::class);
192189
$linker->consumeLoginLink($request);
193190
}
194191

195192
public function testConsumeLoginLinkExceedsMaxUsage()
196193
{
197-
$this->expectException(ExpiredLoginLinkException::class);
198194
$expires = time() + 500;
199-
$signature = $this->createSignatureHash('weaverryan', $expires, ['ryan@symfonycasts.com', 'pwhash']);
195+
$signature = $this->createSignatureHash('weaverryan', $expires);
200196
$request = Request::create(sprintf('/login/verify?user=weaverryan&hash=%s&expires=%d', $signature, $expires));
201197

202198
$user = new TestLoginLinkHandlerUser('weaverryan', 'ryan@symfonycasts.com', 'pwhash');
@@ -207,6 +203,7 @@ public function testConsumeLoginLinkExceedsMaxUsage()
207203
$this->expiredLinkCache->save($item);
208204

209205
$linker = $this->createLinker(['max_uses' => 3]);
206+
$this->expectException(ExpiredLoginLinkException::class);
210207
$linker->consumeLoginLink($request);
211208
}
212209

@@ -234,15 +231,12 @@ public function testConsumeLoginLinkWithMissingExpiration()
234231
$linker->consumeLoginLink($request);
235232
}
236233

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

244-
// matches hash logic in the class
245-
return base64_encode(hash_hmac('sha256', implode(':', $fields), 's3cret'));
239+
return $hasher->computeSignatureHash($user, $expires);
246240
}
247241

248242
private function createLinker(array $options = [], array $extraProperties = ['emailProperty', 'passwordProperty']): LoginLinkHandler
@@ -326,7 +320,7 @@ public function loadUserByIdentifier(string $userIdentifier): TestLoginLinkHandl
326320

327321
public function refreshUser(UserInterface $user): TestLoginLinkHandlerUser
328322
{
329-
return $this->users[$username];
323+
return $this->users[$user->getUserIdentifier()];
330324
}
331325

332326
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
@@ -91,8 +91,8 @@ public function testConsumeRememberMeCookieValid()
9191

9292
/** @var Cookie $cookie */
9393
$cookie = $this->request->attributes->get(ResponseListener::COOKIE_ATTR_NAME);
94-
$rememberParts = explode(':', base64_decode($rememberMeDetails->toString()), 4);
95-
$cookieParts = explode(':', base64_decode($cookie->getValue()), 4);
94+
$rememberParts = explode(':', $rememberMeDetails->toString(), 4);
95+
$cookieParts = explode(':', $cookie->getValue(), 4);
9696

9797
$this->assertSame($rememberParts[0], $cookieParts[0]); // class
9898
$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.