diff --git a/classes/BadpwFailedLoginsDAO.inc.php b/classes/BadpwFailedLoginsDAO.inc.php index c1c19a5..4c45412 100644 --- a/classes/BadpwFailedLoginsDAO.inc.php +++ b/classes/BadpwFailedLoginsDAO.inc.php @@ -81,10 +81,33 @@ 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()]); + } + + /** + * 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 + ] + ); } } diff --git a/features/LimitRetry.inc.php b/features/LimitRetry.inc.php index 0d97913..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; @@ -55,28 +57,33 @@ 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'); + $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."