Skip to content

Commit bfe2398

Browse files
Guard remember cookie cache deletion
1 parent 6c19a2b commit bfe2398

2 files changed

Lines changed: 99 additions & 19 deletions

File tree

lib/auth.php

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,6 @@ function check_auth_cookie() : int|false {
142142
}
143143

144144
if (cacti_sizeof($user_info)) {
145-
if (!auth_cookie_user_currently_allowed($user_info)) {
146-
db_execute_prepared('DELETE FROM user_auth_cache
147-
WHERE user_id = ?',
148-
[$user_info['id']]);
149-
150-
return false;
151-
}
152-
153145
$secret = hash('sha512', $token, false);
154146

155147
$found = db_fetch_cell_prepared('SELECT user_id
@@ -161,20 +153,28 @@ function check_auth_cookie() : int|false {
161153

162154
if (empty($found)) {
163155
return false;
164-
} else {
165-
set_auth_cookie($user_info);
166-
167-
cacti_log(sprintf('LOGIN: User %s Authenticated via Authentication Cookie from IP Address %s', $user_info['username'], get_client_addr()), false, 'AUTH');
156+
}
168157

169-
db_execute_prepared('INSERT IGNORE INTO user_log
170-
(username, user_id, result, ip, time)
171-
VALUES
172-
(?, ?, 2, ?, NOW())',
173-
[$user_info['username'], $user_info['id'], get_client_addr()]
174-
);
158+
if (!auth_cookie_user_currently_allowed($user_info)) {
159+
db_execute_prepared('DELETE FROM user_auth_cache
160+
WHERE user_id = ?',
161+
[$user_info['id']]);
175162

176-
return $user_info['id'];
163+
return false;
177164
}
165+
166+
set_auth_cookie($user_info);
167+
168+
cacti_log(sprintf('LOGIN: User %s Authenticated via Authentication Cookie from IP Address %s', $user_info['username'], get_client_addr()), false, 'AUTH');
169+
170+
db_execute_prepared('INSERT IGNORE INTO user_log
171+
(username, user_id, result, ip, time)
172+
VALUES
173+
(?, ?, 2, ?, NOW())',
174+
[$user_info['username'], $user_info['id'], get_client_addr()]
175+
);
176+
177+
return $user_info['id'];
178178
}
179179
}
180180
}

tests/integration/AuthSystemRegressionIntegrationTest.php

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ function db_fetch_cell_prepared($sql, $params = []) {
4646
return $GLOBALS['auth_integration_group_realms'] ?? 0;
4747
}
4848

49+
if (str_contains($sql, 'FROM user_auth_cache')) {
50+
$cache = $GLOBALS['auth_integration_cache'] ?? [];
51+
52+
foreach ($cache as $row) {
53+
if ($row['user_id'] == $params[0] && $row['token'] == $params[1]) {
54+
return $row['user_id'];
55+
}
56+
}
57+
}
58+
4959
return 0;
5060
}
5161
}
@@ -56,6 +66,35 @@ function db_fetch_assoc_prepared($sql, $params = []) {
5666
}
5767
}
5868

69+
if (!function_exists('db_fetch_row_prepared')) {
70+
function db_fetch_row_prepared($sql, $params = []) {
71+
return $GLOBALS['auth_integration_users'][$params[0]] ?? [];
72+
}
73+
}
74+
75+
if (!function_exists('db_execute_prepared')) {
76+
function db_execute_prepared($sql, $params = []) {
77+
$GLOBALS['auth_integration_executed'][] = [
78+
'sql' => $sql,
79+
'params' => $params,
80+
];
81+
82+
return true;
83+
}
84+
}
85+
86+
if (!function_exists('db_table_exists')) {
87+
function db_table_exists($table) {
88+
return $table == 'user_auth_cache';
89+
}
90+
}
91+
92+
if (!function_exists('get_guest_account')) {
93+
function get_guest_account() {
94+
return (int) read_config_option('guest_user');
95+
}
96+
}
97+
5998
require_once $root . '/lib/auth.php';
6099

61100
test('auth subsystem regression coverage spans cookie login, 2fa, reset tokens, basic auth, and profile mutations', function () use ($root) {
@@ -117,3 +156,44 @@ function db_fetch_assoc_prepared($sql, $params = []) {
117156
'show_preview' => '',
118157
]))->toBeTrue();
119158
});
159+
160+
test('remember-me cookie authorization verifies token before deleting cache rows', function () {
161+
$GLOBALS['auth_integration_config'] = [
162+
'auth_cache_enabled' => 'on',
163+
'guest_user' => 0,
164+
];
165+
$GLOBALS['auth_integration_realms'] = 1;
166+
$GLOBALS['auth_integration_groups'] = [];
167+
$GLOBALS['auth_integration_users'] = [
168+
42 => [
169+
'id' => 42,
170+
'username' => 'disabled',
171+
'realm' => 0,
172+
'enabled' => '',
173+
'show_tree' => '',
174+
'show_list' => '',
175+
'show_preview' => '',
176+
],
177+
];
178+
$GLOBALS['auth_integration_cache'] = [
179+
[
180+
'user_id' => 42,
181+
'token' => hash('sha512', 'valid-token', false),
182+
],
183+
];
184+
$GLOBALS['auth_integration_executed'] = [];
185+
186+
$_COOKIE['cacti_remembers'] = '42,-1,forged-token';
187+
188+
expect(check_auth_cookie())->toBeFalse()
189+
->and($GLOBALS['auth_integration_executed'])->toBeEmpty();
190+
191+
$_COOKIE['cacti_remembers'] = '42,-1,valid-token';
192+
193+
expect(check_auth_cookie())->toBeFalse()
194+
->and($GLOBALS['auth_integration_executed'])->toHaveCount(1)
195+
->and($GLOBALS['auth_integration_executed'][0]['sql'])->toContain('DELETE FROM user_auth_cache')
196+
->and($GLOBALS['auth_integration_executed'][0]['params'])->toBe([42]);
197+
198+
unset($_COOKIE['cacti_remembers']);
199+
});

0 commit comments

Comments
 (0)