From 634cb477efea54cf7dd2a6644443f539684ca51e Mon Sep 17 00:00:00 2001 From: "Florine W. Dekker" Date: Mon, 14 Nov 2022 11:46:07 +0100 Subject: [PATCH] Specify pass algorithm, remove some duplication --- composer.json | 2 +- composer.lock | Bin 14959 -> 14959 bytes package-lock.json | Bin 225103 -> 225103 bytes package.json | 2 +- src/main/config.default.ini.php | 3 ++ src/main/index.html | 2 + src/main/php/UserManager.php | 89 +++++++++++++------------------- 7 files changed, 43 insertions(+), 55 deletions(-) diff --git a/composer.json b/composer.json index f9e5cb3..877f5a9 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "fwdekker/death-notifier", "description": "Get notified when a famous person dies.", - "version": "0.0.30", "_comment_version": "Also update version in `package.json`!", + "version": "0.0.31", "_comment_version": "Also update version in `package.json`!", "type": "project", "license": "MIT", "homepage": "https://git.fwdekker.com/tools/death-notifier", diff --git a/composer.lock b/composer.lock index ee0f2ceeed4c559b99cfd25164b53a66befb3ee5..154d6f6fd3ef2dad93a1931ca3274f90d18463c6 100644 GIT binary patch delta 45 zcmaD~^1ft)Fr$Kzk*Tq1QfiucYD%g}npvWynX!SPiKV$ovU!Suaa!_bV@4ZG0A9BZ A&;S4c delta 45 zcmaD~^1ft)Fr$KbqOrMAVzNb&nOTyBiD9adxrMoLT9T=mnPp;1iiyc)V@4ZG09zUj ArvLx| diff --git a/package-lock.json b/package-lock.json index b2bdef7104a8df25c64a7672b0f7a010b1b58a73..102831c2082c9f2af5c97f2ba1f808f902507acf 100644 GIT binary patch delta 31 ncmX?qkN5mN-U+6Rh7--EO&6ZZDA;(n^)6%UU8b#fnTzrO-mwj| delta 32 ocmX?qkN5mN-U+6R1{2MuF&a!} +# TODO: Add feature for global site message +# TODO: Add i18n + [admin] # Password to use the CLI of `api.php`. Until this value is changed from its default, the feature is disabled cli_secret = REPLACE THIS WITH A SECRET VALUE diff --git a/src/main/index.html b/src/main/index.html index 3842cde..84baef6 100644 --- a/src/main/index.html +++ b/src/main/index.html @@ -173,6 +173,7 @@ diff --git a/src/main/php/UserManager.php b/src/main/php/UserManager.php index 3e80614..7aa3717 100644 --- a/src/main/php/UserManager.php +++ b/src/main/php/UserManager.php @@ -78,6 +78,22 @@ class UserManager } + /** + * Returns `true` if there is a user with the given email address. + * + * @param string $email the email address to check + * @return bool `true` if there is a user with the given email address + */ + private function email_is_used(string $email): bool + { + $stmt = $this->conn->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE email=:email);"); + $stmt->bindValue(":email", $email); + $stmt->execute(); + $result = $stmt->fetch(); + return $result[0] === 1; + } + + /** * Registers a new user. * @@ -99,11 +115,7 @@ class UserManager $this->conn->beginTransaction(); // Check if email address is already in use - $stmt = $this->conn->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE email=:email);"); - $stmt->bindValue(":email", $email); - $stmt->execute(); - $result = $stmt->fetch(); - if ($result[0] === 1) { + if ($this->email_is_used($email)) { $this->conn->rollBack(); return new Response( payload: ["target" => "email", "message" => "Email address already in use."], @@ -116,8 +128,7 @@ class UserManager VALUES (:email, :password) RETURNING email_verification_token;"); $stmt->bindValue(":email", $email); - // TODO: Specify password hash function, for forwards compatibility - $stmt->bindValue(":password", password_hash($password, PASSWORD_DEFAULT)); + $stmt->bindValue(":password", password_hash($password, PASSWORD_BCRYPT)); $stmt->execute(); $email_verification_token = $stmt->fetchAll(PDO::FETCH_ASSOC)[0]["email_verification_token"]; $this->mailer->queue_register_password($email, $email_verification_token); @@ -239,14 +250,8 @@ class UserManager ); } - // TODO: Reject update if email address was changed too recently (within, say, 5 minutes) - // Check if email address is already in use - $stmt = $this->conn->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE email=:email);"); - $stmt->bindValue(":email", $email); - $stmt->execute(); - $result = $stmt->fetch(); - if ($result[0] === 1) { + if ($this->email_is_used($email)) { $this->conn->rollBack(); return new Response( payload: ["target" => "email", "message" => "Email address already in use."], @@ -283,6 +288,7 @@ class UserManager { $this->conn->beginTransaction(); + // Check if email is verified $stmt = $this->conn->prepare("SELECT email, email_verification_token, email_verification_token_timestamp FROM users WHERE uuid=:uuid;"); @@ -297,6 +303,7 @@ class UserManager ); } + // Check if new verification can be sent $minutes_since_last_verify_email = (new DateTime("@{$user["email_verification_token_timestamp"]}"))->diff(new DateTime(), absolute: true)->i; if ($minutes_since_last_verify_email < self::MINUTES_BETWEEN_VERIFICATION_EMAILS) { @@ -313,6 +320,7 @@ class UserManager ); } + // Queue verification email $stmt = $this->conn->prepare("UPDATE users SET email_verification_token_timestamp=unixepoch() WHERE uuid=:uuid;"); @@ -336,7 +344,7 @@ class UserManager { $this->conn->beginTransaction(); - + // Check if token is correct for email $stmt = $this->conn->prepare("SELECT email_verification_token_timestamp FROM users WHERE email=:email AND email_verification_token=:token;"); @@ -357,6 +365,7 @@ class UserManager ); } + // Check if token is still valid $token_timestamp = $results[0]["email_verification_token_timestamp"]; $minutes_since_creation = (new DateTime("@$token_timestamp"))->diff(new DateTime(), absolute: true)->i; if ($minutes_since_creation > self::MINUTES_VALID_VERIFICATION) { @@ -370,6 +379,7 @@ class UserManager ); } + // Set as verified $stmt = $this->conn->prepare("UPDATE users SET email_verification_token=null WHERE email=:email;"); $stmt->bindValue(":email", $email); $stmt->execute(); @@ -419,7 +429,7 @@ class UserManager password_reset_token=null WHERE uuid=:uuid;"); $stmt->bindValue(":uuid", $uuid); - $stmt->bindValue(":password", password_hash($password_new, PASSWORD_DEFAULT)); + $stmt->bindValue(":password", password_hash($password_new, PASSWORD_BCRYPT)); $stmt->execute(); // Respond @@ -491,9 +501,9 @@ class UserManager * @return Response a satisfied `Response` with payload `null` if the password reset token is currently valid, or an * unsatisfied `Response` otherwise */ - public function validate_password_reset_token(string $email, string $token): Response + public function validate_password_reset_token(string $email, string $token, bool $use_transaction = true): Response { - $this->conn->beginTransaction(); + if ($use_transaction) $this->conn->beginTransaction(); $stmt = $this->conn->prepare("SELECT password_reset_token_timestamp FROM users WHERE email=:email AND password_reset_token=:token;"); @@ -502,7 +512,7 @@ class UserManager $stmt->execute(); $results = $stmt->fetchAll(PDO::FETCH_ASSOC); if (sizeof($results) === 0) { - $this->conn->rollBack(); + if ($use_transaction) $this->conn->rollBack(); return new Response( payload: [ "target" => null, @@ -515,7 +525,7 @@ class UserManager $token_timestamp = $results[0]["password_reset_token_timestamp"]; $minutes_since_creation = (new DateTime("@$token_timestamp"))->diff(new DateTime(), absolute: true)->i; if ($minutes_since_creation > self::MINUTES_VALID_PASSWORD_RESET) { - $this->conn->rollBack(); + if ($use_transaction) $this->conn->rollBack(); return new Response( payload: [ "target" => null, @@ -525,7 +535,7 @@ class UserManager ); } - $this->conn->commit(); + if ($use_transaction) $this->conn->commit(); return new Response(payload: null, satisfied: true); } @@ -544,35 +554,9 @@ class UserManager { $this->conn->beginTransaction(); - $stmt = $this->conn->prepare("SELECT password_reset_token_timestamp - FROM users WHERE email=:email AND password_reset_token=:token;"); - $stmt->bindValue(":email", $email); - $stmt->bindValue(":token", $token); - $stmt->execute(); - $results = $stmt->fetchAll(PDO::FETCH_ASSOC); - if (sizeof($results) === 0) { - $this->conn->rollBack(); - return new Response( - payload: [ - "target" => null, - "message" => "This password reset link is invalid. Maybe you already reset your password?" - ], - satisfied: false - ); - } - - $token_timestamp = $results[0]["password_reset_token_timestamp"]; - $minutes_since_creation = (new DateTime("@$token_timestamp"))->diff(new DateTime(), absolute: true)->i; - if ($minutes_since_creation > self::MINUTES_VALID_PASSWORD_RESET) { - $this->conn->rollBack(); - return new Response( - payload: [ - "target" => null, - "message" => "This password reset link has expired." - ], - satisfied: false - ); - } + $token_is_valid = $this->validate_password_reset_token($email, $token, false); + if (!$token_is_valid->satisfied) + return $token_is_valid; if ($password_new !== $password_confirm) { $this->conn->rollBack(); @@ -583,10 +567,9 @@ class UserManager } $stmt = $this->conn->prepare("UPDATE users - SET password=:password, - password_reset_token=null + SET password=:password, password_reset_token=null WHERE email=:email;"); - $stmt->bindValue(":password", password_hash($password_new, PASSWORD_DEFAULT)); + $stmt->bindValue(":password", password_hash($password_new, PASSWORD_BCRYPT)); $stmt->bindValue(":email", $email); $stmt->execute();