diff --git a/controllers/Login.php b/controllers/Login.php index d91785d..62373ba 100644 --- a/controllers/Login.php +++ b/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'])); diff --git a/controllers/ResetPassword.php b/controllers/ResetPassword.php index 24fa7b6..cd0d56e 100644 --- a/controllers/ResetPassword.php +++ b/controllers/ResetPassword.php @@ -16,66 +16,94 @@ class ResetPassword extends HTMLController // Verifying an existing reset key? if (isset($_GET['step'], $_GET['email'], $_GET['key']) && $_GET['step'] == 2) - { - $email = rawurldecode($_GET['email']); - $id_user = Authentication::getUserid($email); - if ($id_user === false) - 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)) - 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); - $form = new PasswordResetForm($email, $key); - $this->page->adopt($form); - - // Are they trying to set something already? - if (isset($_POST['password1'], $_POST['password2'])) - { - $missing = []; - if (strlen($_POST['password1']) < 6 || !preg_match('~[^A-z]~', $_POST['password1'])) - $missing[] = 'Please fill in a password that is at least six characters long and contains at least one non-alphabetic character (e.g. a number or symbol).'; - if ($_POST['password1'] != $_POST['password2']) - $missing[] = 'The passwords you entered do not match.'; - - // So, are we good to go? - if (empty($missing)) - { - Authentication::updatePassword($id_user, Authentication::computeHash($_POST['password1'])); - $_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; - } - else - $form->adopt(new Alert('Some fields require your attention', '', 'danger')); - } - } + $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']))) { - 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) { - $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); + $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']); + $user = Member::fromEmailAddress($email); + 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($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); + $form = new PasswordResetForm($email, $key); + $this->page->adopt($form); + + // Are they trying to set something already? + if (isset($_POST['password1'], $_POST['password2'])) + { + $missing = []; + if (strlen($_POST['password1']) < 6 || !preg_match('~[^A-z]~', $_POST['password1'])) + $missing[] = 'Please fill in a password that is at least six characters long and contains at least one non-alphabetic character (e.g. a number or symbol).'; + if ($_POST['password1'] != $_POST['password2']) + $missing[] = 'The passwords you entered do not match.'; + + // So, are we good to go? + if (empty($missing)) + { + 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; + } + else + $form->adopt(new Alert('Some fields require your attention', '', 'danger')); } } } diff --git a/migrations/2024-11-05.sql b/migrations/2024-11-05.sql new file mode 100644 index 0000000..ee37d8f --- /dev/null +++ b/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`; diff --git a/models/Authentication.php b/models/Authentication.php index 55f9dc3..324ad91 100644 --- a/models/Authentication.php +++ b/models/Authentication.php @@ -12,92 +12,7 @@ */ class Authentication { - /** - * 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; - } + const DEFAULT_RESET_TIMEOUT = 30; /** * 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; + } } diff --git a/models/Database.php b/models/Database.php index 226cd74..8bc0fad 100644 --- a/models/Database.php +++ b/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. */ diff --git a/models/Member.php b/models/Member.php index 84912a4..d276313 100644 --- a/models/Member.php +++ b/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(' diff --git a/models/User.php b/models/User.php index b69790d..a26241d 100644 --- a/models/User.php +++ b/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;