controllers/Login.php

@@ -24,7 +24,9 @@ class Login extends HTMLController
 			if (Authentication::checkPassword($_POST['emailaddress'], $_POST['password']))
 			{
 				parent::__construct('Login');
-				$_SESSION['user_id'] = Authentication::getUserId($_POST['emailaddress']);
+
+				$user = Member::fromEmailAddress($_POST['emailaddress']);
+				$_SESSION['user_id'] = $user->getUserId();
 
 				if (isset($_POST['redirect_url']))
 					header('Location: ' . base64_decode($_POST['redirect_url']));

controllers/ResetPassword.php

@@ -16,14 +16,65 @@ class ResetPassword extends HTMLController
 
 		// Verifying an existing reset key?
 		if (isset($_GET['step'], $_GET['email'], $_GET['key']) && $_GET['step'] == 2)
+			$this->verifyResetKey();
+		else
+			$this->requestResetKey();
+	}
+
+	private function requestResetKey()
+	{
+		parent::__construct('Reset password - ' . SITE_TITLE);
+		$form = new ForgotPasswordForm();
+		$this->page->adopt($form);
+
+		// Have they submitted an email address yet?
+		if (isset($_POST['emailaddress']) && preg_match('~^.+@.+\.[a-z]+$~', trim($_POST['emailaddress'])))
+		{
+			$user = Member::fromEmailAddress($_POST['emailaddress']);
+			if (!$user)
+			{
+				$form->adopt(new Alert('Invalid email address', 'The email address you provided could not be found in our system. Please try again.', 'danger'));
+				return;
+			}
+
+			if (Authentication::getResetTimeOut($user->getUserId()) > 0)
+			{
+				// Update the reset time-out to prevent hammering
+				$resetTimeOut = Authentication::updateResetTimeOut($user->getUserId());
+
+				// Present it to the user in a readable way
+				if ($resetTimeOut > 3600)
+					$timeOut = sprintf('%d hours', ceil($resetTimeOut / 3600));
+				elseif ($resetTimeOut > 60)
+					$timeOut = sprintf('%d minutes', ceil($resetTimeOut / 60));
+				else
+					$timeOut = sprintf('%d seconds', $resetTimeOut);
+
+				$form->adopt(new Alert('Password reset token already sent', 'We already sent a password reset token to this email address recently. ' .
+					'If no email was received, please wait ' . $timeOut . ' to try again.', 'error'));
+				return;
+			}
+
+			Authentication::setResetKey($user->getUserId());
+			Email::resetMail($user->getUserId());
+
+			// Show the success message
+			$this->page->clear();
+			$box = new DummyBox('An email has been sent');
+			$box->adopt(new Alert('', 'We have sent an email to ' . $_POST['emailaddress'] . ' containing details on how to reset your password.', 'success'));
+			$this->page->adopt($box);
+		}
+	}
+
+	private function verifyResetKey()
 	{
 		$email = rawurldecode($_GET['email']);
-			$id_user = Authentication::getUserid($email);
+		$user = Member::fromEmailAddress($email);
-			if ($id_user === false)
+		if (!$user)
 			throw new UserFacingException('Invalid email address. Please make sure you copied the full link in the email you received.');
 
 		$key = $_GET['key'];
-			if (!Authentication::checkResetKey($id_user, $key))
+		if (!Authentication::checkResetKey($user->getUserId(), $key))
 			throw new UserFacingException('Invalid reset token. Please make sure you copied the full link in the email you received. Note: you cannot use the same token twice.');
 
 		parent::__construct('Reset password - ' . SITE_TITLE);
@@ -42,7 +93,11 @@ class ResetPassword extends HTMLController
 			// So, are we good to go?
 			if (empty($missing))
 			{
-					Authentication::updatePassword($id_user, Authentication::computeHash($_POST['password1']));
+				Authentication::updatePassword($user->getUserId(), Authentication::computeHash($_POST['password1']));
+
+				// Consume token, ensuring it isn't used again
+				Authentication::consumeResetKey($user->getUserId());
+
 				$_SESSION['login_msg'] = ['Your password has been reset', 'You can now use the form below to log in to your account.', 'success'];
 				header('Location: ' . BASEURL . '/login/');
 				exit;
@@ -51,31 +106,4 @@ class ResetPassword extends HTMLController
 				$form->adopt(new Alert('Some fields require your attention', '<ul><li>' . implode('</li><li>', $missing) . '</li></ul>', 'danger'));
 		}
 	}
-		else
-		{
-			parent::__construct('Reset password - ' . SITE_TITLE);
-			$form = new ForgotPasswordForm();
-			$this->page->adopt($form);
-
-			// Have they submitted an email address yet?
-			if (isset($_POST['emailaddress']) && preg_match('~^.+@.+\.[a-z]+$~', trim($_POST['emailaddress'])))
-			{
-				$id_user = Authentication::getUserid(trim($_POST['emailaddress']));
-				if ($id_user === false)
-				{
-					$form->adopt(new Alert('Invalid email address', 'The email address you provided could not be found in our system. Please try again.', 'danger'));
-					return;
-				}
-
-				Authentication::setResetKey($id_user);
-				Email::resetMail($id_user);
-
-				// Show the success message
-				$this->page->clear();
-				$box = new DummyBox('An email has been sent');
-				$box->adopt(new Alert('', 'We have sent an email to ' . $_POST['emailaddress'] . ' containing details on how to reset your password.', 'success'));
-				$this->page->adopt($box);
-			}
-		}
-	}
 }

migrations/2024-11-05.sql

@@ -0,0 +1,2 @@
+/* Add time-out to password reset keys, and prevent repeated mails */
+ALTER TABLE `users` ADD `reset_blocked_until` INT  UNSIGNED  NULL  AFTER `reset_key`;

models/Authentication.php

@@ -12,92 +12,7 @@
  */
 class Authentication
 {
-	/**
+	const DEFAULT_RESET_TIMEOUT = 30;
-	 * Checks whether a user still exists in the database.
-	 */
-	public static function checkExists($id_user)
-	{
-		$res = Registry::get('db')->queryValue('
-			SELECT id_user
-			FROM users
-			WHERE id_user = {int:id}',
-			[
-				'id' => $id_user,
-			]);
-
-		return $res !== null;
-	}
-
-	/**
-	 * Finds the user id belonging to a certain emailaddress.
-	 */
-	public static function getUserId($emailaddress)
-	{
-		$res = Registry::get('db')->queryValue('
-			SELECT id_user
-			FROM users
-			WHERE emailaddress = {string:emailaddress}',
-			[
-				'emailaddress' => $emailaddress,
-			]);
-
-		return empty($res) ? false : $res;
-	}
-
-	public static function setResetKey($id_user)
-	{
-		return Registry::get('db')->query('
-			UPDATE users
-			SET reset_key = {string:key}
-			WHERE id_user = {int:id}',
-			[
-				'id' => $id_user,
-				'key' => self::newActivationKey(),
-			]);
-	}
-
-	public static function checkResetKey($id_user, $reset_key)
-	{
-		$key = Registry::get('db')->queryValue('
-			SELECT reset_key
-			FROM users
-			WHERE id_user = {int:id}',
-			[
-				'id' => $id_user,
-			]);
-
-		return $key == $reset_key;
-	}
-
-	/**
-	 * Verifies whether the user is currently logged in.
-	 */
-	public static function isLoggedIn()
-	{
-		// Check whether the active session matches the current user's environment.
-		if (isset($_SESSION['ip_address'], $_SESSION['user_agent']) && (
-			 (isset($_SERVER['REMOTE_ADDR']) && $_SESSION['ip_address'] != $_SERVER['REMOTE_ADDR']) ||
-			 (isset($_SERVER['HTTP_USER_AGENT']) && $_SESSION['user_agent'] != $_SERVER['HTTP_USER_AGENT'])))
-		{
-			session_destroy();
-			return false;
-		}
-
-		// A user is logged in if a user id exists in the session and this id is (still) in the database.
-		return isset($_SESSION['user_id']) && self::checkExists($_SESSION['user_id']);
-	}
-
-	/**
-	 * Generates a new activation key.
-	 */
-	public static function newActivationKey()
-	{
-		$alpha = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
-		$string = '';
-		for ($i = 0; $i < 16; $i++)
-			$string .= $alpha[mt_rand(0, strlen($alpha) - 1)];
-		return $string;
-	}
 
 	/**
 	 * Checks a password for a given username against the database.
@@ -120,6 +35,19 @@ class Authentication
 		return password_verify($password, $password_hash);
 	}
 
+	public static function checkResetKey($id_user, $reset_key)
+	{
+		$key = Registry::get('db')->queryValue('
+			SELECT reset_key
+			FROM users
+			WHERE id_user = {int:id}',
+			[
+				'id' => $id_user,
+			]);
+
+		return $key == $reset_key;
+	}
+
 	/**
 	 * Computes a password hash.
 	 */
@@ -131,6 +59,71 @@ class Authentication
 		return $hash;
 	}
 
+	public static function consumeResetKey($id_user)
+	{
+		return Registry::get('db')->query('
+			UPDATE users
+			SET reset_key = NULL,
+				reset_blocked_until = NULL
+			WHERE id_user = {int:id_user}',
+			['id_user' => $id_user]);
+	}
+
+	public static function getResetTimeOut($id_user)
+	{
+		$resetTime = Registry::get('db')->queryValue('
+			SELECT reset_blocked_until
+			FROM users
+			WHERE id_user = {int:id_user}',
+			['id_user' => $id_user]);
+
+		return max(0, $resetTime - time());
+	}
+
+	/**
+	 * Verifies whether the user is currently logged in.
+	 */
+	public static function isLoggedIn()
+	{
+		if (!isset($_SESSION['user_id']))
+			return false;
+
+		try
+		{
+			$exists = Member::fromId($_SESSION['user_id']);
+			return true;
+		}
+		catch (NotFoundException $e)
+		{
+			return false;
+		}
+	}
+
+	/**
+	 * Generates a new activation key.
+	 */
+	public static function newActivationKey()
+	{
+		$alpha = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+		$string = '';
+		for ($i = 0; $i < 16; $i++)
+			$string .= $alpha[mt_rand(0, strlen($alpha) - 1)];
+		return $string;
+	}
+
+	public static function setResetKey($id_user)
+	{
+		return Registry::get('db')->query('
+			UPDATE users
+			SET reset_key = {string:key},
+				reset_blocked_until = UNIX_TIMESTAMP() + ' . static::DEFAULT_RESET_TIMEOUT . '
+			WHERE id_user = {int:id}',
+			[
+				'id' => $id_user,
+				'key' => self::newActivationKey(),
+			]);
+	}
+
 	/**
 	 * Resets a password for a certain user.
 	 */
@@ -148,4 +141,26 @@ class Authentication
 				'blank' => '',
 			]);
 	}
+
+	public static function updateResetTimeOut($id_user)
+	{
+		$currentResetTimeOut = static::getResetTimeOut($id_user);
+
+		// New timeout: between 30 seconds, double the current timeout, and a full day
+		$newResetTimeOut = min(max(static::DEFAULT_RESET_TIMEOUT, $currentResetTimeOut * 2), 60 * 60 * 24);
+
+		$success = Registry::get('db')->query('
+			UPDATE users
+			SET reset_blocked_until = {int:new_time_out}
+			WHERE id_user = {int:id_user}',
+			[
+				'id_user' => $id_user,
+				'new_time_out' => time() + $newResetTimeOut,
+			]);
+
+		if (!$success)
+			throw new UnexpectedValueException('Could not set password reset timeout!');
+
+		return $newResetTimeOut;
+	}
 }

models/Database.php

@@ -59,6 +59,14 @@ class Database
 		return mysqli_fetch_assoc($resource);
 	}
 
+	/**
+	 * Fetches a row from a given recordset, encapsulating into an object.
+	 */
+	public function fetch_object($resource, $class)
+	{
+		return mysqli_fetch_object($resource, $class);
+	}
+
 	/**
 	 * Fetches a row from a given recordset, using numeric keys.
 	 */
@@ -341,6 +349,41 @@ class Database
 		return $db_string;
 	}
 
+	/**
+	 * Executes a query, returning an object of the row it returns.
+	 */
+	public function queryObject($class, $db_string, $db_values = [])
+	{
+		$res = $this->query($db_string, $db_values);
+
+		if (!$res || $this->num_rows($res) == 0)
+			return null;
+
+		$object = $this->fetch_object($res, $class);
+		$this->free_result($res);
+
+		return $object;
+	}
+
+	/**
+	 * Executes a query, returning an array of objects of all the rows returns.
+	 */
+	public function queryObjects($class, $db_string, $db_values = [])
+	{
+		$res = $this->query($db_string, $db_values);
+
+		if (!$res || $this->num_rows($res) == 0)
+			return [];
+
+		$rows = [];
+		while ($object = $this->fetch_object($res, $class))
+			$rows[] = $object;
+
+		$this->free_result($res);
+
+		return $rows;
+	}
+
 	/**
 	 * Executes a query, returning an array of all the rows it returns.
 	 */

models/Member.php

@@ -8,7 +8,7 @@
 
 class Member extends User
 {
-	private function __construct($data)
+	private function __construct($data = [])
 	{
 		foreach ($data as $key => $value)
 			$this->$key = $value;
@@ -18,6 +18,15 @@ class Member extends User
 		$this->is_admin = $this->is_admin == 1;
 	}
 
+	public static function fromEmailAddress($email_address)
+	{
+		return Registry::get('db')->queryObject(static::class, '
+			SELECT *
+			FROM users
+			WHERE emailaddress = {string:email_address}',
+			['email_address' => $email_address]);
+	}
+
 	public static function fromId($id_user)
 	{
 		$row = Registry::get('db')->queryAssoc('

models/User.php

@@ -23,6 +23,7 @@ abstract class User
 	protected $ip_address;
 	protected $is_admin;
 	protected $reset_key;
+	protected $reset_blocked_until;
 
 	protected bool $is_logged;
 	protected bool $is_guest;