Skip to content

Commit fbbb035

Browse files
committed
fix: always hide password and reset token + token fixes
1 parent 4f6ee76 commit fbbb035

2 files changed

Lines changed: 59 additions & 6 deletions

File tree

‎src/Auth/User.php‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ class User
4848
*/
4949
protected array $tokens = [];
5050

51+
/**
52+
* Expiry used when tokens are lazily minted
53+
* @var int
54+
*/
55+
protected $tokenLifetime;
56+
5157
/**
5258
* All errors caught
5359
* @var array{
@@ -103,8 +109,7 @@ public function __construct($data, $session = true)
103109
? strtotime($sessionLifetime)
104110
: (time() + intval($sessionLifetime));
105111

106-
$this->tokens['access'] = $this->generateToken($sessionLifetime);
107-
$this->tokens['refresh'] = $this->generateToken($sessionLifetime + 259200);
112+
$this->tokenLifetime = $sessionLifetime;
108113

109114
if (function_exists('crash')) {
110115
crash()->context(['user' => array_filter([
@@ -299,10 +304,12 @@ public function resetPassword(string $newPassword): bool
299304
*/
300305
public function getAuthInfo(): object
301306
{
307+
$tokens = $this->tokens();
308+
302309
$dataToReturn = (object) [
303310
'user' => $this->get(),
304-
'accessToken' => $this->tokens['access'] ?? null,
305-
'refreshToken' => $this->tokens['refresh'] ?? null,
311+
'accessToken' => $tokens['access'] ?? null,
312+
'refreshToken' => $tokens['refresh'] ?? null,
306313
];
307314

308315
if (count($this->roles)) {
@@ -325,6 +332,11 @@ public function getAuthInfo(): object
325332
*/
326333
public function tokens(): array
327334
{
335+
if (!isset($this->tokens['access'])) {
336+
$this->tokens['access'] = $this->generateToken($this->tokenLifetime);
337+
$this->tokens['refresh'] = $this->generateToken($this->tokenLifetime + 259200);
338+
}
339+
328340
return $this->tokens;
329341
}
330342

@@ -420,10 +432,10 @@ public function get()
420432
$hidden = array_merge(Config::get('hidden'), [Config::get('roles.key')]);
421433
$passwordKey = Config::get('password.key');
422434

435+
unset($userData[$passwordKey], $userData['remember_token']);
436+
423437
if (count($hidden) > 0) {
424438
foreach ($hidden as $item) {
425-
// array_key_exists, not isset: a present-but-null column
426-
// (like the roles column before first assign) must hide too
427439
if (array_key_exists($item, $userData)) {
428440
unset($userData[$item]);
429441
}

‎tests/tokens.test.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,44 @@
112112

113113
expect($user->get())->not->toHaveKey(Config::get('roles.key'));
114114
});
115+
116+
test('the password hash never leaks from get(), whatever hidden says', function () {
117+
// following the old docs (literal 'password') with a custom
118+
// password.key put the bcrypt hash in the session and Inertia props
119+
Config::set(['password.key' => 'pass', 'hidden' => ['password', 'remember_token']]);
120+
121+
$user = new \Leaf\Auth\User([
122+
'id' => 1,
123+
'email' => 'leak-check@example.com',
124+
'pass' => '$2y$10$fakehashfakehashfakehash',
125+
'remember_token' => 'session-equivalent-secret',
126+
], false);
127+
128+
expect($user->get())->not->toHaveKey('pass')
129+
->and($user->get())->not->toHaveKey('remember_token');
130+
131+
Config::set(['password.key' => 'password', 'hidden' => ['field.id', 'field.password', 'remember_token']]);
132+
});
133+
134+
test('session-only users need no token secret until tokens are read', function () {
135+
Config::set(['token.secret' => null]);
136+
unset($_ENV['APP_KEY'], $_ENV['AUTH_TOKEN_SECRET']);
137+
138+
// constructing a user must not mint JWTs — session apps never read them
139+
$user = new \Leaf\Auth\User(['id' => 5, 'email' => 'lazy@example.com'], false);
140+
141+
expect($user->get())->toHaveKey('email');
142+
143+
// reading tokens is the moment the secret becomes required
144+
expect(fn () => $user->tokens())
145+
->toThrow(RuntimeException::class, 'No auth token secret');
146+
});
147+
148+
test('middleware redirect targets are configurable', function () {
149+
expect(Config::get('redirect.login'))->toBe('/auth/login')
150+
->and(Config::get('redirect.guest'))->toBe('/dashboard');
151+
152+
Config::set(['redirect.guest' => '/']);
153+
expect(Config::get('redirect.guest'))->toBe('/');
154+
Config::set(['redirect.guest' => '/dashboard']);
155+
});

0 commit comments

Comments
 (0)