Bugfix/stable 3 3/63 fix lock message

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
+			]
+		);
 	}
 }

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);
 			}
 		});
 	}

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."