Merge pull request #4285 from MrPetovan/task/3942-centralize-password-update

Centralize password update

boot.php

@@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM',     'Friendica');
 define('FRIENDICA_CODENAME',     'Asparagus');
 define('FRIENDICA_VERSION',      '3.6-dev');
 define('DFRN_PROTOCOL_VERSION',  '2.23');
-define('DB_UPDATE_VERSION',      1242);
+define('DB_UPDATE_VERSION',      1243);
 define('NEW_UPDATE_ROUTINE_VERSION', 1170);
 
 /**

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
 -- Friendica 3.6-dev (Asparagus)
--- DB_UPDATE_VERSION 1242
+-- DB_UPDATE_VERSION 1243
 -- ------------------------------------------
 
 
@@ -1031,7 +1031,8 @@ CREATE TABLE IF NOT EXISTS `user` (
 	`page-flags` tinyint NOT NULL DEFAULT 0 COMMENT '',
 	`account-type` tinyint NOT NULL DEFAULT 0 COMMENT '',
 	`prvnets` boolean NOT NULL DEFAULT '0' COMMENT '',
-	`pwdreset` varchar(255) NOT NULL DEFAULT '' COMMENT '',
+	`pwdreset` varchar(255) COMMENT 'Password reset request token',
+	`pwdreset_time` datetime COMMENT 'Timestamp of the last password reset request',
 	`maxreq` int NOT NULL DEFAULT 10 COMMENT '',
 	`expire` int NOT NULL DEFAULT 0 COMMENT '',
 	`account_removed` boolean NOT NULL DEFAULT '0' COMMENT '',

mod/lostpass.php

@@ -1,47 +1,47 @@
 <?php
+
 /**
  * @file mod/lostpass.php
  */
+
 use Friendica\App;
 use Friendica\Core\System;
 use Friendica\Database\DBM;
+use Friendica\Model\User;
 
+require_once 'boot.php';
+require_once 'include/datetime.php';
 require_once 'include/enotify.php';
 require_once 'include/text.php';
+require_once 'include/pgettext.php';
 
-function lostpass_post(App $a) {
+function lostpass_post(App $a)
-
+{
 	$loginame = notags(trim($_POST['login-name']));
-	if(! $loginame)
+	if (!$loginame) {
-		goaway(System::baseUrl());
-
-	$r = q("SELECT * FROM `user` WHERE ( `email` = '%s' OR `nickname` = '%s' ) AND `verified` = 1 AND `blocked` = 0 LIMIT 1",
-		dbesc($loginame),
-		dbesc($loginame)
-	);
-
-	if (! DBM::is_result($r)) {
-		notice( t('No valid account found.') . EOL);
 		goaway(System::baseUrl());
 	}
 
-	$uid = $r[0]['uid'];
+	$condition = ['(`email` = ? OR `nickname` = ?) AND `verified` = 1 AND `blocked` = 0', $loginame, $loginame];
-	$username = $r[0]['username'];
+	$user = dba::selectFirst('user', ['uid', 'username', 'email'], $condition);
-	$email = $r[0]['email'];
+	if (!DBM::is_result($user)) {
+		notice(t('No valid account found.') . EOL);
+		goaway(System::baseUrl());
+	}
 
-	$new_password = autoname(12) . mt_rand(100,9999);
+	$pwdreset_token = autoname(12) . mt_rand(1000, 9999);
-	$new_password_encoded = hash('whirlpool',$new_password);
-
-	$r = q("UPDATE `user` SET `pwdreset` = '%s' WHERE `uid` = %d",
-		dbesc($new_password_encoded),
-		intval($uid)
-	);
-	if($r)
-		info( t('Password reset request issued. Check your email.') . EOL);
 
+	$fields = [
+		'pwdreset' => $pwdreset_token,
+		'pwdreset_time' => datetime_convert()
+	];
+	$result = dba::update('user', $fields, ['uid' => $user['uid']]);
+	if ($result) {
+		info(t('Password reset request issued. Check your email.') . EOL);
+	}
 
 	$sitename = $a->config['sitename'];
-	$resetlink = System::baseUrl() . '/lostpass?verify=' . $new_password;
+	$resetlink = System::baseUrl() . '/lostpass/' . $pwdreset_token;
 
 	$preamble = deindent(t('
 		Dear %1$s,
@@ -50,12 +50,12 @@ function lostpass_post(App $a) {
 		below or paste it into your web browser address bar.
 
 		If you did NOT request this change, please DO NOT follow the link
-		provided and ignore and/or delete this email.
+		provided and ignore and/or delete this email, the request will expire shortly.
 
 		Your password will not be changed unless we can verify that you
-		issued this request.'));
+		issued this request.', $user['username'], $sitename));
 	$body = deindent(t('
-		Follow this link to verify your identity:
+		Follow this link soon to verify your identity:
 
 		%1$s
 
@@ -65,109 +65,110 @@ function lostpass_post(App $a) {
 		The login details are as follows:
 
 		Site Location:	%2$s
-		Login Name:	%3$s'));
+		Login Name:	%3$s', $resetlink, System::baseUrl(), $user['email']));
-
-	$preamble = sprintf($preamble, $username, $sitename);
-	$body = sprintf($body, $resetlink, System::baseUrl(), $email);
 
 	notification([
-		'type' => SYSTEM_EMAIL,
+		'type'     => SYSTEM_EMAIL,
-		'to_email' => $email,
+		'to_email' => $user['email'],
-		'subject'=> sprintf( t('Password reset requested at %s'),$sitename),
+		'subject'  => t('Password reset requested at %s', $sitename),
-		'preamble'=> $preamble,
+		'preamble' => $preamble,
-		'body' => $body]);
+		'body'     => $body
+	]);
 
 	goaway(System::baseUrl());
-
 }
 
+function lostpass_content(App $a)
+{
+	$o = '';
+	if ($a->argc > 1) {
+		$pwdreset_token = $a->argv[1];
 
-function lostpass_content(App $a) {
+		$user = dba::selectFirst('user', ['uid', 'username', 'email', 'pwdreset_time'], ['pwdreset' => $pwdreset_token]);
+		if (!DBM::is_result($user)) {
+			notice(t("Request could not be verified. \x28You may have previously submitted it.\x29 Password reset failed."));
 
-
+			return lostpass_form();
-	if(x($_GET,'verify')) {
-		$verify = $_GET['verify'];
-		$hash = hash('whirlpool', $verify);
-
-		$r = q("SELECT * FROM `user` WHERE `pwdreset` = '%s' LIMIT 1",
-			dbesc($hash)
-		);
-		if (! DBM::is_result($r)) {
-			$o =  t("Request could not be verified. \x28You may have previously submitted it.\x29 Password reset failed.");
-			return $o;
-		}
-		$uid = $r[0]['uid'];
-		$username = $r[0]['username'];
-		$email = $r[0]['email'];
-
-		$new_password = autoname(6) . mt_rand(100,9999);
-		$new_password_encoded = hash('whirlpool',$new_password);
-
-		$r = q("UPDATE `user` SET `password` = '%s', `pwdreset` = ''  WHERE `uid` = %d",
-			dbesc($new_password_encoded),
-			intval($uid)
-		);
-
-		/// @TODO Is DBM::is_result() okay here?
-		if ($r) {
-			$tpl = get_markup_template('pwdreset.tpl');
-			$o .= replace_macros($tpl,[
-				'$lbl1' => t('Password Reset'),
-				'$lbl2' => t('Your password has been reset as requested.'),
-				'$lbl3' => t('Your new password is'),
-				'$lbl4' => t('Save or copy your new password - and then'),
-				'$lbl5' => '<a href="' . System::baseUrl() . '">' . t('click here to login') . '</a>.',
-				'$lbl6' => t('Your password may be changed from the <em>Settings</em> page after successful login.'),
-				'$newpass' => $new_password,
-				'$baseurl' => System::baseUrl()
-
-			]);
-				info("Your password has been reset." . EOL);
-
-
-			$sitename = $a->config['sitename'];
-			// $username, $email, $new_password
-			$preamble = deindent(t('
-				Dear %1$s,
-					Your password has been changed as requested. Please retain this
-				information for your records (or change your password immediately to
-				something that you will remember).
-			'));
-			$body = deindent(t('
-				Your login details are as follows:
-
-				Site Location:	%1$s
-				Login Name:	%2$s
-				Password:	%3$s
-
-				You may change that password from your account settings page after logging in.
-			'));
-
-			$preamble = sprintf($preamble, $username);
-			$body = sprintf($body, System::baseUrl(), $email, $new_password);
-
-			notification([
-				'type' => SYSTEM_EMAIL,
-				'to_email' => $email,
-				'subject'=> sprintf( t('Your password has been changed at %s'),$sitename),
-				'preamble'=> $preamble,
-				'body' => $body]);
-
-			return $o;
 		}
 
+		// Password reset requests expire in 60 minutes
+		if ($user['pwdreset_time'] < datetime_convert('UTC', 'UTC', 'now - 1 hour')) {
+			$fields = [
+				'pwdreset' => null,
+				'pwdreset_time' => null
+			];
+			dba::update('user', $fields, ['uid' => $user['uid']]);
+
+			notice(t('Request has expired, please make a new one.'));
+
+			return lostpass_form();
+		}
+
+		return lostpass_generate_password($user);
+	} else {
+		return lostpass_form();
 	}
-	else {
+}
-		$tpl = get_markup_template('lostpass.tpl');
 
-		$o .= replace_macros($tpl,[
+function lostpass_form()
-			'$title' => t('Forgot your Password?'),
+{
-			'$desc' => t('Enter your email address and submit to have your password reset. Then check your email for further instructions.'),
+	$tpl = get_markup_template('lostpass.tpl');
-			'$name' => t('Nickname or Email: '),
+	$o = replace_macros($tpl, [
-			'$submit' => t('Reset')
+		'$title' => t('Forgot your Password?'),
+		'$desc' => t('Enter your email address and submit to have your password reset. Then check your email for further instructions.'),
+		'$name' => t('Nickname or Email: '),
+		'$submit' => t('Reset')
+	]);
+
+	return $o;
+}
+
+function lostpass_generate_password($user)
+{
+	$o = '';
+
+	$new_password = User::generateNewPassword();
+	$result = User::updatePassword($user['uid'], $new_password);
+	if (DBM::is_result($result)) {
+		$tpl = get_markup_template('pwdreset.tpl');
+		$o .= replace_macros($tpl, [
+			'$lbl1'    => t('Password Reset'),
+			'$lbl2'    => t('Your password has been reset as requested.'),
+			'$lbl3'    => t('Your new password is'),
+			'$lbl4'    => t('Save or copy your new password - and then'),
+			'$lbl5'    => '<a href="' . System::baseUrl() . '">' . t('click here to login') . '</a>.',
+			'$lbl6'    => t('Your password may be changed from the <em>Settings</em> page after successful login.'),
+			'$newpass' => $new_password,
+			'$baseurl' => System::baseUrl()
 		]);
 
-		return $o;
+		info("Your password has been reset." . EOL);
+
+		$sitename = $a->config['sitename'];
+		$preamble = deindent(t('
+			Dear %1$s,
+				Your password has been changed as requested. Please retain this
+			information for your records (or change your password immediately to
+			something that you will remember).
+		', $user['username']));
+		$body = deindent(t('
+			Your login details are as follows:
+
+			Site Location:	%1$s
+			Login Name:	%2$s
+			Password:	%3$s
+
+			You may change that password from your account settings page after logging in.
+		', System::baseUrl(), $user['email'], $new_password));
+
+		notification([
+			'type'     => SYSTEM_EMAIL,
+			'to_email' => $user['email'],
+			'subject'  => t('Your password has been changed at %s', $sitename),
+			'preamble' => $preamble,
+			'body'     => $body
+		]);
 	}
 
+	return $o;
 }

mod/settings.php

@@ -2,14 +2,15 @@
 /**
  * @file mod/settings.php
  */
+
 use Friendica\App;
 use Friendica\Content\Feature;
 use Friendica\Content\Nav;
 use Friendica\Core\Addon;
-use Friendica\Core\System;
-use Friendica\Core\Worker;
 use Friendica\Core\Config;
 use Friendica\Core\PConfig;
+use Friendica\Core\System;
+use Friendica\Core\Worker;
 use Friendica\Database\DBM;
 use Friendica\Model\GContact;
 use Friendica\Model\Group;
@@ -391,12 +392,8 @@ function settings_post(App $a)
         }
 
 		if (!$err) {
-			$password = hash('whirlpool', $newpass);
+			$result = User::updatePassword(local_user(), $newpass);
-			$r = q("UPDATE `user` SET `password` = '%s' WHERE `uid` = %d",
+			if (DBM::is_result($result)) {
-				dbesc($password),
-				intval(local_user())
-			);
-			if (DBM::is_result($r)) {
 				info(t('Password changed.') . EOL);
 			} else {
 				notice(t('Password update failed. Please try again.') . EOL);

src/Database/DBStructure.php

@@ -1733,7 +1733,8 @@ class DBStructure {
 						"page-flags" => ["type" => "tinyint", "not null" => "1", "default" => "0", "comment" => ""],
 						"account-type" => ["type" => "tinyint", "not null" => "1", "default" => "0", "comment" => ""],
 						"prvnets" => ["type" => "boolean", "not null" => "1", "default" => "0", "comment" => ""],
-						"pwdreset" => ["type" => "varchar(255)", "not null" => "1", "default" => "", "comment" => ""],
+						"pwdreset" => ["type" => "varchar(255)", "comment" => "Password reset request token"],
+						"pwdreset_time" => ["type" => "datetime", "comment" => "Timestamp of the last password reset request"],
 						"maxreq" => ["type" => "int", "not null" => "1", "default" => "10", "comment" => ""],
 						"expire" => ["type" => "int", "not null" => "1", "default" => "0", "comment" => ""],
 						"account_removed" => ["type" => "boolean", "not null" => "1", "default" => "0", "comment" => ""],
@@ -1783,6 +1784,6 @@ class DBStructure {
 						]
 				];
 
-		return($database);
+		return $database;
 	}
 }

src/Model/User.php

@@ -142,7 +142,7 @@ class User
 			return false;
 		}
 
-		$password_hashed = hash('whirlpool', $password);
+		$password_hashed = self::hashPassword($password);
 
 		if ($password_hashed !== $user['password']) {
 			return false;
@@ -151,6 +151,57 @@ class User
 		return $user['uid'];
 	}
 
+	/**
+	 * Generates a human-readable random password
+	 *
+	 * @return string
+	 */
+	public static function generateNewPassword()
+	{
+		return autoname(6) . mt_rand(100, 9999);
+	}
+
+	/**
+	 * Global user password hashing function
+	 *
+	 * @param string $password
+	 * @return string
+	 */
+	private static function hashPassword($password)
+	{
+		return hash('whirlpool', $password);
+	}
+
+	/**
+	 * Updates a user row with a new plaintext password
+	 *
+	 * @param int    $uid
+	 * @param string $password
+	 * @return bool
+	 */
+	public static function updatePassword($uid, $password)
+	{
+		return self::updatePasswordHashed($uid, self::hashPassword($password));
+	}
+
+	/**
+	 * Updates a user row with a new hashed password.
+	 * Empties the password reset token field just in case.
+	 *
+	 * @param int    $uid
+	 * @param string $pasword_hashed
+	 * @return bool
+	 */
+	private static function updatePasswordHashed($uid, $pasword_hashed)
+	{
+		$fields = [
+			'password' => $pasword_hashed,
+			'pwdreset' => null,
+			'pwdreset_time' => null
+		];
+		return dba::update('user', $fields, ['uid' => $uid]);
+	}
+
 	/**
 	 * @brief Catch-all user creation function
 	 *
@@ -290,8 +341,8 @@ class User
 			throw new Exception(t('Nickname is already registered. Please choose another.'));
 		}
 
-		$new_password = strlen($password) ? $password : autoname(6) . mt_rand(100, 9999);
+		$new_password = strlen($password) ? $password : User::generateNewPassword();
-		$new_password_encoded = hash('whirlpool', $new_password);
+		$new_password_encoded = self::hashPassword($new_password);
 
 		$return['password'] = $new_password;