MDL-73736 core_auth: Fix concurrency issue in login_attempt_failed()

This patch wraps the login_failed_count logic in a resource lock and
forces a user preferences cache reload. Each thread must wait for the
lock and must fetch the current count before incrementing it. This
ensures that login_failed_count is correct across threads and that the
lockout threshold is correctly honoured.

Co-Authored-By: Sujith Haridasan <sujith@moodle.com>
This commit is contained in:
Jake Dallimore 2022-04-26 16:52:20 +08:00 committed by Ilya Tregubov
parent a0f47c8bc4
commit 59b5858da2

View File

@ -877,6 +877,7 @@ function login_attempt_valid($user) {
/**
* To be called after failed user login.
* @param stdClass $user
* @throws moodle_exception
*/
function login_attempt_failed($user) {
global $CFG;
@ -888,30 +889,53 @@ function login_attempt_failed($user) {
return;
}
$count = get_user_preferences('login_failed_count', 0, $user);
$last = get_user_preferences('login_failed_last', 0, $user);
$sincescuccess = get_user_preferences('login_failed_count_since_success', $count, $user);
$sincescuccess = $sincescuccess + 1;
set_user_preference('login_failed_count_since_success', $sincescuccess, $user);
// Force user preferences cache reload to ensure the most up-to-date login_failed_count is fetched.
// This is perhaps overzealous but is the documented way of reloading the cache, as per the test method
// 'test_check_user_preferences_loaded'.
unset($user->preference);
if (empty($CFG->lockoutthreshold)) {
// No threshold means no lockout.
// Always unlock here, there might be some race conditions or leftovers when switching threshold.
login_unlock_account($user);
return;
}
$resource = 'user:' . $user->id;
$lockfactory = \core\lock\lock_config::get_lock_factory('core_failed_login_count_lock');
if (!empty($CFG->lockoutwindow) and time() - $last > $CFG->lockoutwindow) {
$count = 0;
}
// Get a new lock for the resource, waiting for it for a maximum of 10 seconds.
if ($lock = $lockfactory->get_lock($resource, 10)) {
try {
$count = get_user_preferences('login_failed_count', 0, $user);
$last = get_user_preferences('login_failed_last', 0, $user);
$sincescuccess = get_user_preferences('login_failed_count_since_success', $count, $user);
$sincescuccess = $sincescuccess + 1;
set_user_preference('login_failed_count_since_success', $sincescuccess, $user);
$count = $count+1;
if (empty($CFG->lockoutthreshold)) {
// No threshold means no lockout.
// Always unlock here, there might be some race conditions or leftovers when switching threshold.
login_unlock_account($user);
$lock->release();
return;
}
set_user_preference('login_failed_count', $count, $user);
set_user_preference('login_failed_last', time(), $user);
if (!empty($CFG->lockoutwindow) and time() - $last > $CFG->lockoutwindow) {
$count = 0;
}
if ($count >= $CFG->lockoutthreshold) {
login_lock_account($user);
$count = $count + 1;
set_user_preference('login_failed_count', $count, $user);
set_user_preference('login_failed_last', time(), $user);
if ($count >= $CFG->lockoutthreshold) {
login_lock_account($user);
}
// Release locks when we're done.
$lock->release();
} catch (Exception $e) {
// Always release the lock on a failure.
$lock->release();
}
} else {
// We did not get access to the resource in time, give up.
throw new moodle_exception('locktimeout');
}
}