Skip to content

Commit 103c9dd

Browse files
authored
fix: delete stale password tokens when requesting a new reset (#4547)
Each password reset request generated a new token without removing prior ones, leaving multiple valid tokens accumulating in the database indefinitely. Any of these tokens could be exploited if obtained. Now clears all existing tokens for the user before generating a new one.
1 parent 5f08029 commit 103c9dd

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

framework/core/src/User/Job/RequestPasswordResetJob.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public function handle(
5050
return;
5151
}
5252

53+
PasswordToken::where('user_id', $user->id)->delete();
54+
5355
$token = PasswordToken::generate($user->id);
5456
$token->save();
5557

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
/*
4+
* This file is part of Flarum.
5+
*
6+
* For detailed copyright and license information, please view the
7+
* LICENSE file that was distributed with this source code.
8+
*/
9+
10+
namespace Flarum\Tests\integration\api\users;
11+
12+
use Carbon\Carbon;
13+
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
14+
use Flarum\Testing\integration\TestCase;
15+
use Flarum\User\PasswordToken;
16+
17+
class PasswordResetTokenAccumulationTest extends TestCase
18+
{
19+
use RetrievesAuthorizedUsers;
20+
21+
protected function setUp(): void
22+
{
23+
parent::setUp();
24+
25+
$this->prepareDatabase([
26+
'users' => [
27+
$this->normalUser(),
28+
[
29+
'id' => 3,
30+
'username' => 'normal2',
31+
'password' => '$2y$10$LO59tiT7uggl6Oe23o/O6.utnF6ipngYjvMvaxo1TciKqBttDNKim',
32+
'email' => 'normal2@machine.local',
33+
'is_email_confirmed' => 1,
34+
],
35+
],
36+
]);
37+
}
38+
39+
/** @test */
40+
public function existing_password_tokens_are_deleted_when_new_reset_is_requested()
41+
{
42+
$this->app();
43+
44+
// Simulate two prior reset requests, old enough to be outside the throttle window
45+
$old = PasswordToken::generate(3);
46+
$old->created_at = Carbon::now()->subHours(2);
47+
$old->save();
48+
49+
$old2 = PasswordToken::generate(3);
50+
$old2->created_at = Carbon::now()->subHours(1);
51+
$old2->save();
52+
53+
$this->assertEquals(2, PasswordToken::query()->where('user_id', 3)->count());
54+
55+
// Request a new password reset
56+
$response = $this->send(
57+
$this->request('POST', '/api/forgot', [
58+
'authenticatedAs' => 3,
59+
'json' => ['email' => 'normal2@machine.local'],
60+
])
61+
);
62+
63+
$this->assertEquals(204, $response->getStatusCode());
64+
65+
// Should have exactly one token — the new one — not three
66+
$this->assertEquals(1, PasswordToken::query()->where('user_id', 3)->count());
67+
}
68+
}

0 commit comments

Comments
 (0)