From cce38adfb3e09121d813281fee32de117a14abe0 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Mon, 30 Mar 2026 15:35:20 +0300 Subject: [PATCH 1/4] #63 Updated resetCount() to drop the record --- classes/BadpwFailedLoginsDAO.inc.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/classes/BadpwFailedLoginsDAO.inc.php b/classes/BadpwFailedLoginsDAO.inc.php index c1c19a5..2cf8434 100644 --- a/classes/BadpwFailedLoginsDAO.inc.php +++ b/classes/BadpwFailedLoginsDAO.inc.php @@ -81,10 +81,6 @@ public function getByUsername(string $username) : ?BadpwFailedLogins { * @return boolean True if reset is successful */ public function resetCount(BadpwFailedLogins $badpwObj) : bool { - return $this->update(' - UPDATE badpw_failedlogins - SET count = 0 - WHERE username = ? - ', [$badpwObj->getUsername()]); + return $this->update('DELETE FROM badpw_failedlogins WHERE username = ?', [$badpwObj->getUsername()]); } } From eb22f7694277ae43c251cb428454da4751e77f13 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Mon, 30 Mar 2026 15:36:03 +0300 Subject: [PATCH 2/4] #63 Added cleanup method to remove expired entries --- classes/BadpwFailedLoginsDAO.inc.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/classes/BadpwFailedLoginsDAO.inc.php b/classes/BadpwFailedLoginsDAO.inc.php index 2cf8434..4c45412 100644 --- a/classes/BadpwFailedLoginsDAO.inc.php +++ b/classes/BadpwFailedLoginsDAO.inc.php @@ -83,4 +83,31 @@ public function getByUsername(string $username) : ?BadpwFailedLogins { public function resetCount(BadpwFailedLogins $badpwObj) : bool { return $this->update('DELETE FROM badpw_failedlogins WHERE username = ?', [$badpwObj->getUsername()]); } + + /** + * Cleanup stale failed login attempts. + * + * Rules: + * - If the account is locked (count > $maxRetries), then `failed_login_time` must be old enough + * to satisfy both `$lockExpiresSeconds` and `$lockSeconds`. + * - Otherwise, only `$lockExpiresSeconds` must be respected. + * + * @param int $maxRetries Maximum amount of retries before an account is considered locked + * @param int $lockSeconds Lock duration (seconds) + * @param int $lockExpiresSeconds How long we keep bad-password attempts (seconds) + */ + public function cleanup(int $maxRetries, int $lockSeconds, int $attemptExpiresSeconds): void { + $now = time(); + $type = 'date'; + $attemptsExpiresAt = $this->convertToDB($now + $attemptExpiresSeconds, $type); + $lockExpiresAt = $this->convertToDB($now + max($attemptExpiresSeconds, $lockSeconds), $type); + $this->update( + 'DELETE FROM badpw_failedlogins WHERE failed_login_time >= CASE WHEN count >= ? THEN ? ELSE ? END', + [ + $maxRetries, + $lockExpiresAt, + $attemptsExpiresAt + ] + ); + } } From f82c9304512d811d272f81df148981b98385df4e Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Mon, 30 Mar 2026 15:36:47 +0300 Subject: [PATCH 3/4] #63 Updated code to use the new cleanup method --- features/LimitRetry.inc.php | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/features/LimitRetry.inc.php b/features/LimitRetry.inc.php index 0d97913..7563cd2 100644 --- a/features/LimitRetry.inc.php +++ b/features/LimitRetry.inc.php @@ -55,25 +55,11 @@ private function _handleTemplateDisplay() : void { } /** @var BadpwFailedLoginsDAO */ $badpwFailedLoginsDao = DAORegistry::getDAO('BadpwFailedLoginsDAO'); + $badpwFailedLoginsDao->cleanup($this->_maxRetries, $this->_lockSeconds, $this->_lockExpiresSeconds); $user = $badpwFailedLoginsDao->getByUsername($username); - if (!$user) { - return; - } - $count = $user->getCount(); - $time = $user->getFailedTime(); - - // Discard old bad password attempts - // When the memory has expired - if ($count && $time < time() - $this->_lockExpiresSeconds) { - // And the user is not currently locked - if ($user->getCount() < $this->_maxRetries || $user->getFailedTime() <= time() - $this->_lockSeconds) { - $badpwFailedLoginsDao->resetCount($user); - } - } - // Update the count to represent this failed attempt $badpwFailedLoginsDao->incCount($user); - + $count = $user->getCount() + 1; // Warn the user if the attempts have been exhausted if ($count >= $this->_maxRetries) { $templateManager->assign('error', 'plugins.generic.betterPassword.validation.betterPasswordLocked'); From f445dde02be4479571c7e3a3647854180bd34f05 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Mon, 30 Mar 2026 19:02:50 +0300 Subject: [PATCH 4/4] #63 Added improved message --- features/LimitRetry.inc.php | 23 ++++++++++++++++++++++- locale/en_US/locale.po | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/features/LimitRetry.inc.php b/features/LimitRetry.inc.php index 7563cd2..10753df 100644 --- a/features/LimitRetry.inc.php +++ b/features/LimitRetry.inc.php @@ -12,6 +12,8 @@ * @brief Implements the feature to limit retries and lock the account */ +use Carbon\CarbonInterval; + class LimitRetry { /** @var int Max amount of retries */ private $_maxRetries; @@ -62,7 +64,26 @@ private function _handleTemplateDisplay() : void { $count = $user->getCount() + 1; // Warn the user if the attempts have been exhausted if ($count >= $this->_maxRetries) { - $templateManager->assign('error', 'plugins.generic.betterPassword.validation.betterPasswordLocked'); + $localeKey = 'plugins.generic.betterPassword.validation.betterPasswordLocked'; + $enhancedLocaleKey = "{$localeKey}.enhancedByHook"; + HookRegistry::register('PKPLocale::translate', function ($hook, $args) use ($localeKey, $enhancedLocaleKey) { + $key = $args[0]; + $value = &$args[4]; + if ($key === $enhancedLocaleKey) { + $label = CarbonInterval::seconds(max($this->_lockExpiresSeconds, $this->_lockSeconds)) + ->cascade() + ->locale(AppLocale::getLocale()) + ->forHumans([ + 'parts' => 2, + 'short' => false, + ]); + $value = AppLocale::translate($localeKey, ['remainingTime' => $label]); + return true; + } + + return false; + }); + $templateManager->assign('error', $enhancedLocaleKey); } }); } diff --git a/locale/en_US/locale.po b/locale/en_US/locale.po index 6ecb358..bcfad38 100644 --- a/locale/en_US/locale.po +++ b/locale/en_US/locale.po @@ -37,7 +37,7 @@ msgid "plugins.generic.betterPassword.validation.betterPasswordCheckBlocklist" msgstr "Your password was found in a blocklist of known bad passwords." msgid "plugins.generic.betterPassword.validation.betterPasswordLocked" -msgstr "Your account has been temporarily locked due to bad password attempts." +msgstr "Your account has been temporarily locked due to bad password attempts. It will unlock automatically within {$remainingTime}. For immediate access, use the \"Forgot you password?\" link to reset your password." msgid "plugins.generic.betterPassword.validation.betterPasswordUnexpectedError" msgstr "Your password could not be changed at this time. Please try again later."