From b46c7ea41c1cc0dca6e24f36f3588e08208dc450 Mon Sep 17 00:00:00 2001 From: Slaveek Date: Sun, 10 Jan 2016 12:04:25 +0000 Subject: [PATCH 1/4] Hash user's id and user's activation hash together. We get nice looking hashed value without any special chars. This works good in url's without add ?param_name=param_value --- application/model/RegistrationModel.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/application/model/RegistrationModel.php b/application/model/RegistrationModel.php index 53dce9887..e71ebbe09 100644 --- a/application/model/RegistrationModel.php +++ b/application/model/RegistrationModel.php @@ -248,8 +248,14 @@ public static function rollbackRegistrationByUserId($user_id) */ public static function sendVerificationEmail($user_id, $user_email, $user_activation_hash) { + // Hash user's id and user's activation hash together. + // We get nice looking hashed value without any special chars. + // Sha256 used because of discussion here https://github.com/panique/huge/issues/776 (not sure is it good or not yet) + // To be more secure, salt or secret key can be added to hash function. + $hashed_user_data = hash('sha256' , $user_id . $user_activation_hash); + $body = Config::get('EMAIL_VERIFICATION_CONTENT') . Config::get('URL') . Config::get('EMAIL_VERIFICATION_URL') - . '/' . urlencode($user_id) . '/' . urlencode($user_activation_hash); + . '/' . urlencode($hashed_user_data) . '/' . urlencode($user_activation_hash); $mail = new Mail; $mail_sent = $mail->sendMail($user_email, Config::get('EMAIL_VERIFICATION_FROM_EMAIL'), From e51d064118b62531ed033d7a7c00f02fdac18c49 Mon Sep 17 00:00:00 2001 From: Slaveek Date: Sun, 10 Jan 2016 12:10:32 +0000 Subject: [PATCH 2/4] Added new method getUserDataByUserActivationHash() Get user's id and activation hash. This will be hashed the same way as sent by email to user, then check that is the same as in email --- application/model/UserModel.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/application/model/UserModel.php b/application/model/UserModel.php index 1321eb3e4..6b56ea59b 100644 --- a/application/model/UserModel.php +++ b/application/model/UserModel.php @@ -340,4 +340,24 @@ public static function getUserDataByUserIdAndToken($user_id, $token) // return one row (we only have one result or nothing) return $query->fetch(); } + + /** + * Gets the user's data by user activation hash. + * @param string $user_activation_hash_code Hash code generated in registration process. + * @return mixed Returns false if activation hash code does not exist, returns object with user's data + */ + public static function getUserDataByUserActivationHash($user_activation_hash_code) + { + $database = DatabaseFactory::getFactory()->getConnection(); + + $query = $database->prepare("SELECT user_id, user_activation_hash + FROM users + WHERE user_activation_hash = :user_activation_hash_code + LIMIT 1 + "); + $query->execute(array(':user_activation_hash_code' => $user_activation_hash_code)); + + // return one row (we only have one result or nothing) + return $query->fetch(); + } } From e5e832fa03286ac544299580602aef5ad672d3ce Mon Sep 17 00:00:00 2001 From: Slaveek Date: Sun, 10 Jan 2016 12:16:01 +0000 Subject: [PATCH 3/4] Renamed parameter. --- application/controller/RegisterController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/controller/RegisterController.php b/application/controller/RegisterController.php index 32408973f..39d1707d6 100644 --- a/application/controller/RegisterController.php +++ b/application/controller/RegisterController.php @@ -46,13 +46,13 @@ public function register_action() /** * Verify user after activation mail link opened - * @param int $user_id user's id + * @param string $hashed_user_data Hashed user's id and user's activation verification code together. * @param string $user_activation_verification_code user's verification token */ - public function verify($user_id, $user_activation_verification_code) + public function verify($hashed_user_data, $user_activation_verification_code) { - if (isset($user_id) && isset($user_activation_verification_code)) { - RegistrationModel::verifyNewUser($user_id, $user_activation_verification_code); + if (isset($hashed_user_data) && isset($user_activation_verification_code)) { + RegistrationModel::verifyNewUser($hashed_user_data, $user_activation_verification_code); $this->View->render('register/verify'); } else { Redirect::to('login/index'); From 7223e83fc031694a684e4cfcf7323361f798bff3 Mon Sep 17 00:00:00 2001 From: Slaveek Date: Sun, 10 Jan 2016 12:33:11 +0000 Subject: [PATCH 4/4] Added hashes check If hashed user's id + user's activation code from data base is the same as hash code from email, registration is passed. --- application/model/RegistrationModel.php | 42 +++++++++++++++++++------ 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/application/model/RegistrationModel.php b/application/model/RegistrationModel.php index e71ebbe09..9de699ccd 100644 --- a/application/model/RegistrationModel.php +++ b/application/model/RegistrationModel.php @@ -274,23 +274,47 @@ public static function sendVerificationEmail($user_id, $user_email, $user_activa /** * checks the email/verification code combination and set the user's activation status to true in the database * - * @param int $user_id user id + * @param string $hashed_user_data Hashed user's id and user's activation verification code together. * @param string $user_activation_verification_code verification token * * @return bool success status */ - public static function verifyNewUser($user_id, $user_activation_verification_code) + public static function verifyNewUser($hashed_user_data, $user_activation_verification_code) { $database = DatabaseFactory::getFactory()->getConnection(); - $sql = "UPDATE users SET user_active = 1, user_activation_hash = NULL - WHERE user_id = :user_id AND user_activation_hash = :user_activation_hash LIMIT 1"; - $query = $database->prepare($sql); - $query->execute(array(':user_id' => $user_id, ':user_activation_hash' => $user_activation_verification_code)); + // Get user data (id and user_activation_hash) by activation code sent by email to user + // Data from DB wiil be checked with parameters passed to method. + $user_data = UserModel::getUserDataByUserActivationHash($user_activation_verification_code); - if ($query->rowCount() == 1) { - Session::add('feedback_positive', Text::get('FEEDBACK_ACCOUNT_ACTIVATION_SUCCESSFUL')); - return true; + // No user with that verification code -> return false + if (!$user_data) { + Session::add('feedback_negative', Text::get('FEEDBACK_ACCOUNT_ACTIVATION_FAILED')); + return false; + } + + // Hash user_id and user_activation_hash from DB the same way as sent in email (see line 255 in this file) + $user_db_data_hash = hash('sha256', $user_data->user_id . $user_data->user_activation_hash); + + if ($user_db_data_hash === $hashed_user_data) { + + $sql = "UPDATE users + SET user_active = 1, + user_activation_hash = NULL + WHERE user_id = :user_id + AND user_activation_hash = :user_activation_hash + LIMIT 1 + "; + $query = $database->prepare($sql); + $query->execute(array( + ':user_id' => $user_data->user_id, + ':user_activation_hash' => $user_data->user_activation_hash + )); + + if ($query->rowCount() == 1) { + Session::add('feedback_positive', Text::get('FEEDBACK_ACCOUNT_ACTIVATION_SUCCESSFUL')); + return true; + } } Session::add('feedback_negative', Text::get('FEEDBACK_ACCOUNT_ACTIVATION_FAILED'));