Provide email address verification feature #2481
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,12 +36,23 @@ public static function deleteFeverKey($username) { | |
| return false; | ||
| } | ||
|
|
||
| public static function updateUser($user, $email, $passwordPlain, $apiPasswordPlain, $userConfigUpdated = array()) { | ||
| $userConfig = get_user_configuration($user); | ||
| if ($userConfig === null) { | ||
| return false; | ||
| } | ||
|
|
||
| if ($email !== null && $userConfig->mail_login !== $email) { | ||
| $userConfig->mail_login = $email; | ||
|
Member
There was a problem hiding this comment. Somewhere we need to sanitize this user input. Either here or in
Member
Author
There was a problem hiding this comment. I would simply check that the email contains a
I don't understand why they would respect the RFC, but in fact not… The feature itself is about validating that the address is valid by sending an email so I don't think we have to be too strict.
Member
There was a problem hiding this comment. That's definitely very odd about dotless domains. The whitespace & comments things is more common. For example, the W3C recommends this for input type=email:
Member
There was a problem hiding this comment. There are still some classes of characters, which should be discarded, and potentially also a min and max length. I think it is dangerous to pass some user input to the backend without any form of sanitization. If you do not like the PHP built-in validation (which I personally think would be reasonable - I an not even sure the rest of the back-end is able to handle what the PHP validation rejects anyway), then at least the sanitize filter
Member
Author
There was a problem hiding this comment. Ok, I'll take a look to clean at least a bit this param!
Member
There was a problem hiding this comment. Ah, I did not realise that the HTML5 version does not allow UTF-8 before the
Member
There was a problem hiding this comment. So, it seems that the conclusion is to use the PHPMailer's
Member
Author
There was a problem hiding this comment. Thanks for digging into the problem! (I was finishing my holidays :)) I didn't thought that PHPMailer could have a verification function (sounds obvious now!) I'll try to finish this tomorrow 👍
Member
Author
There was a problem hiding this comment. It looks like none of the patterns allow internationalized email addresses (not even I found that PHPMailer provides a
Member
There was a problem hiding this comment.
Sure, Microsoft has announced support, this one does http://www.postfix.org/SMTPUTF8_README.html There's an incomplete list on Wikipedia: Edit: to be clear, that list is from like 4 years ago and even then I doubt it was complete. |
||
|
|
||
| if (FreshRSS_Context::$system_conf->force_email_validation) { | ||
| $salt = FreshRSS_Context::$system_conf->salt; | ||
| $userConfig->email_validation_token = sha1($salt . uniqid(mt_rand(), true)); | ||
| $mailer = new FreshRSS_User_Mailer(); | ||
| $mailer->send_email_need_validation($user, $userConfig); | ||
| } | ||
| } | ||
|
|
||
| if ($passwordPlain != '') { | ||
| $passwordHash = self::hashPassword($passwordPlain); | ||
| $userConfig->passwordHash = $passwordHash; | ||
|
|
@@ -87,7 +98,7 @@ public function updateAction() { | |
| $apiPasswordPlain = Minz_Request::param('apiPasswordPlain', '', true); | ||
|
|
||
| $username = Minz_Request::param('username'); | ||
| $ok = self::updateUser($username, null, $passwordPlain, $apiPasswordPlain, array( | ||
| 'token' => Minz_Request::param('token', null), | ||
| )); | ||
|
|
||
|
|
@@ -114,25 +125,58 @@ public function profileAction() { | |
| Minz_Error::error(403); | ||
| } | ||
|
|
||
| $email_not_verified = FreshRSS_Context::$user_conf->email_validation_token !== ''; | ||
| if ($email_not_verified) { | ||
| $this->view->_layout('simple'); | ||
| $this->view->disable_aside = true; | ||
| } | ||
|
|
||
| Minz_View::prependTitle(_t('conf.profile.title') . ' · '); | ||
|
|
||
| Minz_View::appendScript(Minz_Url::display('/scripts/bcrypt.min.js?' . @filemtime(PUBLIC_PATH . '/scripts/bcrypt.min.js'))); | ||
|
|
||
| if (Minz_Request::isPost()) { | ||
| $system_conf = FreshRSS_Context::$system_conf; | ||
| $user_config = FreshRSS_Context::$user_conf; | ||
| $old_email = $user_config->mail_login; | ||
|
|
||
| $email = trim(Minz_Request::param('email', '')); | ||
| $passwordPlain = Minz_Request::param('newPasswordPlain', '', true); | ||
| Minz_Request::_param('newPasswordPlain'); //Discard plain-text password ASAP | ||
| $_POST['newPasswordPlain'] = ''; | ||
|
|
||
| $apiPasswordPlain = Minz_Request::param('apiPasswordPlain', '', true); | ||
|
|
||
| if ($system_conf->force_email_validation && empty($email)) { | ||
| Minz_Request::bad( | ||
| _t('user.email.feedback.required'), | ||
| array('c' => 'user', 'a' => 'profile') | ||
| ); | ||
| } | ||
|
|
||
| if (!empty($email) && !validateEmailAddress($email)) { | ||
| Minz_Request::bad( | ||
| _t('user.email.feedback.invalid'), | ||
| array('c' => 'user', 'a' => 'profile') | ||
| ); | ||
| } | ||
|
|
||
| $ok = self::updateUser( | ||
| Minz_Session::param('currentUser'), | ||
| $email, | ||
| $passwordPlain, | ||
| $apiPasswordPlain, | ||
| array( | ||
| 'token' => Minz_Request::param('token', null), | ||
| ) | ||
| ); | ||
|
|
||
| Minz_Session::_param('passwordHash', FreshRSS_Context::$user_conf->passwordHash); | ||
|
|
||
| if ($ok) { | ||
| if ($system_conf->force_email_validation && $email !== $old_email) { | ||
| Minz_Request::good(_t('feedback.profile.updated'), array('c' => 'user', 'a' => 'validateEmail')); | ||
| } elseif ($passwordPlain == '') { | ||
| Minz_Request::good(_t('feedback.profile.updated'), array('c' => 'user', 'a' => 'profile')); | ||
| } else { | ||
| Minz_Request::good(_t('feedback.profile.updated'), array('c' => 'index', 'a' => 'index')); | ||
|
|
@@ -154,6 +198,7 @@ public function manageAction() { | |
|
|
||
| Minz_View::prependTitle(_t('admin.user.title') . ' · '); | ||
|
|
||
| $this->view->show_email_field = FreshRSS_Context::$system_conf->force_email_validation; | ||
| $this->view->current_user = Minz_Request::param('u'); | ||
|
|
||
| $this->view->nb_articles = 0; | ||
|
|
@@ -168,7 +213,7 @@ public function manageAction() { | |
| } | ||
| } | ||
|
|
||
| public static function createUser($new_user_name, $email, $passwordPlain, $apiPasswordPlain, $userConfig = array(), $insertDefaultFeeds = true) { | ||
| if (!is_array($userConfig)) { | ||
| $userConfig = array(); | ||
| } | ||
|
|
@@ -196,7 +241,7 @@ public static function createUser($new_user_name, $passwordPlain, $apiPasswordPl | |
| if ($ok) { | ||
| $userDAO = new FreshRSS_UserDAO(); | ||
| $ok &= $userDAO->createUser($new_user_name, $userConfig['language'], $insertDefaultFeeds); | ||
| $ok &= self::updateUser($new_user_name, $email, $passwordPlain, $apiPasswordPlain); | ||
| } | ||
| return $ok; | ||
| } | ||
|
|
@@ -207,6 +252,7 @@ public static function createUser($new_user_name, $passwordPlain, $apiPasswordPl | |
| * Request parameters are: | ||
| * - new_user_language | ||
| * - new_user_name | ||
| * - new_user_email | ||
| * - new_user_passwordPlain | ||
| * - r (i.e. a redirection url, optional) | ||
| * | ||
|
|
@@ -219,11 +265,28 @@ public function createAction() { | |
| } | ||
|
|
||
| if (Minz_Request::isPost()) { | ||
| $system_conf = FreshRSS_Context::$system_conf; | ||
|
|
||
| $new_user_name = Minz_Request::param('new_user_name'); | ||
| $email = Minz_Request::param('new_user_email', ''); | ||
| $passwordPlain = Minz_Request::param('new_user_passwordPlain', '', true); | ||
| $new_user_language = Minz_Request::param('new_user_language', FreshRSS_Context::$user_conf->language); | ||
|
|
||
| if ($system_conf->force_email_validation && empty($email)) { | ||
| Minz_Request::bad( | ||
| _t('user.email.feedback.required'), | ||
| array('c' => 'auth', 'a' => 'register') | ||
| ); | ||
| } | ||
|
|
||
| if (!empty($email) && !validateEmailAddress($email)) { | ||
| Minz_Request::bad( | ||
| _t('user.email.feedback.invalid'), | ||
| array('c' => 'auth', 'a' => 'register') | ||
| ); | ||
| } | ||
|
|
||
| $ok = self::createUser($new_user_name, $email, $passwordPlain, '', array('language' => $new_user_language)); | ||
| Minz_Request::_param('new_user_passwordPlain'); //Discard plain-text password ASAP | ||
| $_POST['new_user_passwordPlain'] = ''; | ||
| invalidateHttpCache(); | ||
|
|
@@ -275,6 +338,122 @@ public static function deleteUser($username) { | |
| return $ok; | ||
| } | ||
|
|
||
| /** | ||
| * This action validates an email address, based on the token sent by email. | ||
| * It also serves the main page when user is blocked. | ||
| * | ||
| * Request parameters are: | ||
| * - username | ||
| * - token | ||
| * | ||
| * This route works with GET requests since the URL is provided by email. | ||
| * The security risks (e.g. forged URL by an attacker) are not very high so | ||
| * it's ok. | ||
| * | ||
| * It returns 404 error if `force_email_validation` is disabled or if the | ||
| * user doesn't exist. | ||
| * | ||
| * It returns 403 if user isn't logged in and `username` param isn't passed. | ||
| */ | ||
| public function validateEmailAction() { | ||
| if (!FreshRSS_Context::$system_conf->force_email_validation) { | ||
| Minz_Error::error(404); | ||
| } | ||
|
|
||
| Minz_View::prependTitle(_t('user.email.validation.title') . ' · '); | ||
| $this->view->_layout('simple'); | ||
|
|
||
| $username = Minz_Request::param('username'); | ||
| $token = Minz_Request::param('token'); | ||
|
|
||
| if ($username) { | ||
| $user_config = get_user_configuration($username); | ||
| } elseif (FreshRSS_Auth::hasAccess()) { | ||
| $user_config = FreshRSS_Context::$user_conf; | ||
| } else { | ||
| Minz_Error::error(403); | ||
| } | ||
|
|
||
| if (!FreshRSS_UserDAO::exists($username) || $user_config === null) { | ||
| Minz_Error::error(404); | ||
| } | ||
|
|
||
| if ($user_config->email_validation_token === '') { | ||
| Minz_Request::good( | ||
| _t('user.email.validation.feedback.unnecessary'), | ||
| array('c' => 'index', 'a' => 'index') | ||
| ); | ||
| } | ||
|
|
||
| if ($token) { | ||
| if ($user_config->email_validation_token !== $token) { | ||
| Minz_Request::bad( | ||
| _t('user.email.validation.feedback.wrong_token'), | ||
| array('c' => 'user', 'a' => 'validateEmail') | ||
| ); | ||
| } | ||
|
|
||
| $user_config->email_validation_token = ''; | ||
| if ($user_config->save()) { | ||
| Minz_Request::good( | ||
| _t('user.email.validation.feedback.ok'), | ||
| array('c' => 'index', 'a' => 'index') | ||
| ); | ||
| } else { | ||
| Minz_Request::bad( | ||
| _t('user.email.validation.feedback.error'), | ||
| array('c' => 'user', 'a' => 'validateEmail') | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * This action resends a validation email to the current user. | ||
| * | ||
| * It only acts on POST requests but doesn't require any param (except the | ||
| * CSRF token). | ||
| * | ||
| * It returns 403 error if the user is not logged in or 404 if request is | ||
| * not POST. Else it redirects silently to the index if user has already | ||
| * validated its email, or to the user#validateEmail route. | ||
| */ | ||
| public function sendValidationEmailAction() { | ||
| if (!FreshRSS_Auth::hasAccess()) { | ||
| Minz_Error::error(403); | ||
| } | ||
|
|
||
| if (!Minz_Request::isPost()) { | ||
| Minz_Error::error(404); | ||
| } | ||
|
|
||
| $username = Minz_Session::param('currentUser', '_'); | ||
| $user_config = FreshRSS_Context::$user_conf; | ||
|
|
||
| if ($user_config->email_validation_token === '') { | ||
| Minz_Request::forward(array( | ||
| 'c' => 'index', | ||
| 'a' => 'index', | ||
| ), true); | ||
| } | ||
|
|
||
| $mailer = new FreshRSS_User_Mailer(); | ||
| $ok = $mailer->send_email_need_validation($username, $user_config); | ||
|
|
||
| $redirect_url = array('c' => 'user', 'a' => 'validateEmail'); | ||
| if ($ok) { | ||
| Minz_Request::good( | ||
| _t('user.email.validation.feedback.email_sent'), | ||
| $redirect_url | ||
| ); | ||
| } else { | ||
| Minz_Request::bad( | ||
| _t('user.email.validation.feedback.email_failed'), | ||
| $redirect_url | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * This action delete an existing user. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * Manage the emails sent to the users. | ||
| */ | ||
| class FreshRSS_User_Mailer extends Minz_Mailer { | ||
| public function send_email_need_validation($username, $user_config) { | ||
| $this->view->_path('user_mailer/email_need_validation.txt'); | ||
|
|
||
| $this->view->username = $username; | ||
| $this->view->site_title = FreshRSS_Context::$system_conf->title; | ||
| $this->view->validation_url = Minz_Url::display( | ||
| array( | ||
| 'c' => 'user', | ||
| 'a' => 'validateEmail', | ||
| 'params' => array( | ||
| 'username' => $username, | ||
| 'token' => $user_config->email_validation_token | ||
| ) | ||
| ), | ||
| 'txt', | ||
| true | ||
| ); | ||
|
|
||
| $subject_prefix = '[' . FreshRSS_Context::$system_conf->title . ']'; | ||
| return $this->mail( | ||
| $user_config->mail_login, | ||
| $subject_prefix . ' ' ._t('user.mailer.email_need_validation.title') | ||
| ); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$email != ''would be safer here, especially since it might come fromMinz_Request::param('email', '');There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was on purpose since the email should not be required when
force_email_validationisfalse. But I realize that I should enforce the value if it'strue.