Merge pull request #457 from pupi1985/patch-70

Remove unnecessary step in password reset

qa-include/app/users-edit.php

@@ -358,10 +358,12 @@
 	}
 
 
+/**
+ * Successfully finish the 'I forgot my password' process for $userid, sending new password
+ *
+ * @deprecated This function has been replaced by qa_finish_reset_user since Q2A 1.8
+ */
 	function qa_complete_reset_user($userid)
-/*
-	Successfully finish the 'I forgot my password' process for $userid, sending new password
-*/
 	{
 		if (qa_to_override(__FUNCTION__)) { $args=func_get_args(); return qa_call_override(__FUNCTION__, $args); }
 
@@ -390,6 +392,44 @@
 	}
 
 
+/**
+ * Successfully finish the 'I forgot my password' process for $userid, cleaning the emailcode field and logging in the user
+ * @param mixed $userId The userid identifiying the user who will have the password reset
+ * @param string $newPassword The new password for the user
+ * @return void
+ */
+	function qa_finish_reset_user($userId, $newPassword)
+	{
+		if (qa_to_override(__FUNCTION__)) { $args=func_get_args(); return qa_call_override(__FUNCTION__, $args); }
+
+		// For qa_db_user_set_password(), qa_db_user_set()
+		require_once QA_INCLUDE_DIR . 'db/users.php';
+
+		// For qa_set_logged_in_user()
+		require_once QA_INCLUDE_DIR . 'app/options.php';
+
+		// For qa_cookie_get()
+		require_once QA_INCLUDE_DIR . 'app/cookies.php';
+
+		// For qa_db_select_with_pending(), qa_db_user_account_selectspec()
+		require_once QA_INCLUDE_DIR . 'db/selects.php';
+
+		// For qa_set_logged_in_user()
+		require_once QA_INCLUDE_DIR . 'app/users.php';
+
+		qa_db_user_set_password($userId, $newPassword);
+
+		qa_db_user_set($userId, 'emailcode', ''); // to prevent re-use of the code
+
+		$userInfo = qa_db_select_with_pending(qa_db_user_account_selectspec($userId, true));
+
+		qa_set_logged_in_user($userId, $userInfo['handle'], false, $userInfo['sessionsource']); // reinstate this specific session
+
+		qa_report_event('u_reset', $userId, $userInfo['handle'], qa_cookie_get(), array(
+			'email' => $userInfo['email'],
+		));
+	}
+
 	function qa_logged_in_user_flush()
 /*
 	Flush any information about the currently logged in user, so it is retrieved from database again

qa-include/lang/qa-lang-misc.php

@@ -76,6 +76,7 @@
 		'nav_user_pms' => 'Private messages',
 		'nav_user_qs' => 'All questions',
 		'nav_user_wall' => 'Wall',
+		'next_step' => 'Next step',
 		'no_activity_from_x' => 'No activity from ^',
 		'no_favorite_categories' => 'No favorite categories',
 		'no_favorite_qs' => 'No favorite questions',

qa-include/lang/qa-lang-users.php

@@ -118,7 +118,6 @@
 		'save_profile' => 'Save Profile',
 		'save_user' => 'Save User',
 		'send_confirm_button' => 'Send Confirmation Link',
-		'send_password_button' => 'Send New Password',
 		'send_reset_button' => 'Send Reset Password Email',
 		'send_reset_note' => 'A message will be sent to your email address with instructions.',
 		'special_users' => 'Special users',

qa-include/pages/forgot.php

@@ -54,13 +54,12 @@ if (qa_clicked('doforgot')) {
 		if (strpos($inemailhandle, '@') === false) { // handles can't contain @ symbols
 			$matchusers = qa_db_user_find_by_handle($inemailhandle);
 			$passemailhandle = !qa_opt('allow_login_email_only');
-
 		} else {
 			$matchusers = qa_db_user_find_by_email($inemailhandle);
 			$passemailhandle = true;
 		}
 
-		if (count($matchusers) != 1) // if we get more than one match (should be impossible) also give an error
+		if (count($matchusers) != 1 || !$passemailhandle) // if we get more than one match (should be impossible) also give an error
 			$errors['emailhandle'] = qa_lang('users/user_not_found');
 
 		if (qa_opt('captcha_on_reset_password'))
@@ -69,7 +68,7 @@ if (qa_clicked('doforgot')) {
 		if (empty($errors)) {
 			$inuserid = $matchusers[0];
 			qa_start_reset_user($inuserid);
-			qa_redirect('reset', $passemailhandle ? array('e' => $inemailhandle) : null); // redirect to page where code is entered
+			qa_redirect('reset', $passemailhandle ? array('e' => $inemailhandle, 's' => '1') : null); // redirect to page where code is entered
 		}
 	}
 

qa-include/pages/reset.php

@@ -25,58 +25,145 @@ if (!defined('QA_VERSION')) { // don't allow this page to be requested directly
 	exit;
 }
 
-
 //	Check we're not using single-sign on integration and that we're not logged in
 
-if (QA_FINAL_EXTERNAL_USERS)
+if (QA_FINAL_EXTERNAL_USERS) {
 	qa_fatal_error('User login is handled by external code');
+}
 
-if (qa_is_logged_in())
+if (qa_is_logged_in()) {
 	qa_redirect('');
+}
 
+// Fetch the email or handle from POST or GET
+$emailHandle = qa_post_text('emailhandle');
+if (!isset($emailHandle)) {
+	$emailHandle = qa_get('e');
+}
+$emailHandle = trim($emailHandle); // if $emailHandle is null, trim returns an empty string
 
-//	Process incoming form
-
-if (qa_clicked('doreset')) {
-	require_once QA_INCLUDE_DIR . 'app/users-edit.php';
-	require_once QA_INCLUDE_DIR . 'db/users.php';
-
-	$inemailhandle = qa_post_text('emailhandle');
-	$incode = trim(qa_post_text('code')); // trim to prevent passing in blank values to match uninitiated DB rows
-
-	$errors = array();
+// Fetch the code from POST or GET
+$code = qa_post_text('code');
+if (!isset($code)) {
+	$code = qa_get('c');
+}
+$code = trim($code); // if $code is null, trim returns an empty string
 
-	if (!qa_check_form_security_code('reset', qa_post_text('formcode')))
-		$errors['page'] = qa_lang_html('misc/form_security_again');
+$forgotPath = strlen($emailHandle) > 0 ? qa_path('forgot', array('e' => $emailHandle)) : qa_path('forgot');
 
-	else {
-		if (qa_opt('allow_login_email_only') || (strpos($inemailhandle, '@') !== false)) // handles can't contain @ symbols
-			$matchusers = qa_db_user_find_by_email($inemailhandle);
-		else
-			$matchusers = qa_db_user_find_by_handle($inemailhandle);
+$focusId = 'code';
 
-		if (count($matchusers) == 1) { // if match more than one (should be impossible), consider it a non-match
-			require_once QA_INCLUDE_DIR . 'db/selects.php';
+$errors = array();
+$fields = array(
+	'email_handle' => array(
+		'type' => 'static',
+		'label' => qa_opt('allow_login_email_only') ? qa_lang_html('users/email_label') : qa_lang_html('users/email_handle_label'),
+		'value' => qa_html($emailHandle),
+	),
+	'code' => array(
+		'label' => qa_lang_html('users/reset_code_label'),
+		'tags' => 'name="code" id="code"',
+		'value' => isset($code) ? qa_html($code) : null,
+		'note_force' => true,
+		'note' => qa_lang_html('users/reset_code_emailed') . ' - ' .
+			'<a href="' . qa_html($forgotPath) . '">' . qa_lang_html('users/reset_code_another') . '</a>',
+	),
+);
+$buttons = array(
+	'next' => array(
+		'tags' => 'name="donext"',
+		'label' => qa_lang_html('misc/next_step'),
+	),
+);
+$hidden = array(
+	'formcode' => qa_get_form_security_code('reset'),
+);
 
-			$inuserid = $matchusers[0];
-			$userinfo = qa_db_select_with_pending(qa_db_user_account_selectspec($inuserid, true));
+if (strlen($emailHandle) > 0) {
+	require_once QA_INCLUDE_DIR . 'app/users-edit.php';
+	require_once QA_INCLUDE_DIR . 'db/users.php';
 
-			// strlen() check is vital otherwise we can reset code for most users by entering the empty string
-			if (strlen($incode) && strtolower(trim($userinfo['emailcode'])) == strtolower($incode)) {
-				qa_complete_reset_user($inuserid);
-				qa_redirect('login', array('e' => $inemailhandle, 'ps' => '1')); // redirect to login page
+	$hidden['emailhandle'] = $emailHandle;
+
+	$matchingUsers = qa_opt('allow_login_email_only') || strpos($emailHandle, '@') !== false // handles can't contain @ symbols
+		? qa_db_user_find_by_email($emailHandle)
+		: qa_db_user_find_by_handle($emailHandle);
+
+	// Make sure there is only one match
+	if (count($matchingUsers) == 1) {
+		require_once QA_INCLUDE_DIR . 'db/selects.php';
+
+		$userId = $matchingUsers[0];
+		$userInfo = qa_db_select_with_pending(qa_db_user_account_selectspec($userId, true));
+
+		// strlen() check is vital otherwise we can reset code for most users by entering the empty string
+		if (strlen($code) > 0) {
+			if (strtolower(trim($userInfo['emailcode'])) == strtolower($code)) {
+				// User input a valid code so no need to ask for it but pass it to the next step
+				unset($fields['code']);
+				$hidden['code'] = $code;
+
+				$buttons = array(
+					'change' => array(
+						'tags' => 'name="dochangepassword"',
+						'label' => qa_lang_html('users/change_password'),
+					),
+				);
+
+				$focusId = 'newpassword1';
+
+				if (qa_clicked('dochangepassword')) {
+					$newPassword = qa_post_text('newpassword1');
+					$repeatPassword = qa_post_text('newpassword2');
+
+					if (!qa_check_form_security_code('reset', qa_post_text('formcode'))) {
+						$errors['page'] = qa_lang_html('misc/form_security_again');
+					} else {
+						$passwordError = qa_password_validate($newPassword, $userInfo);
+						if (!empty($passwordError)) {
+							$errors['new_1'] = $passwordError['password'];
+						}
+
+						if ($newPassword != $repeatPassword) {
+							$errors['new_2'] = qa_lang('users/password_mismatch');
+						}
+
+						if (empty($errors)) {
+							// Update password, login user, fire events and redirect to home page
+							qa_finish_reset_user($userId, $newPassword);
+							qa_redirect('');
+						}
+					}
+				}
+
+				$fields['new_1'] = array(
+					'label' => qa_lang_html('users/new_password_1'),
+					'tags' => 'name="newpassword1" id="newpassword1"',
+					'type' => 'password',
+					'error' => qa_html(isset($errors['new_1']) ? $errors['new_1'] : null),
+				);
+
+				$fields['new_2'] = array(
+					'label' => qa_lang_html('users/new_password_2'),
+					'tags' => 'name="newpassword2"',
+					'type' => 'password',
+					'error' => qa_html(isset($errors['new_2']) ? $errors['new_2'] : null),
+				);
 			} else {
-				$errors['code'] = qa_lang('users/reset_code_wrong');
+				// User input wrong code so show field with error
+				$fields['code']['error'] = qa_lang('users/reset_code_wrong');
 			}
-
-		} else {
-			$errors['emailhandle'] = qa_lang('users/user_not_found');
+		} else if (qa_clicked('donext')) {
+			// If user submitted the form with an empty code
+			$fields['code']['error'] = qa_lang('users/reset_code_wrong');
 		}
+	} else {
+		// If match more than one (should be impossible), consider it a non-match
+		$errors['page'] = qa_lang_html('users/user_not_found');
 	}
-
 } else {
-	$inemailhandle = qa_get('e');
-	$incode = qa_get('c');
+	// If there is no handle notify the user
+	$errors['page'] = qa_lang_html('users/user_not_found');
 }
 
 
@@ -85,51 +172,25 @@ if (qa_clicked('doreset')) {
 $qa_content = qa_content_prepare();
 
 $qa_content['title'] = qa_lang_html('users/reset_title');
-$qa_content['error'] = @$errors['page'];
-
-if (empty($inemailhandle) || isset($errors['emailhandle']))
-	$forgotpath = qa_path('forgot');
-else
-	$forgotpath = qa_path('forgot', array('e' => $inemailhandle));
-
-$qa_content['form'] = array(
-	'tags' => 'method="post" action="' . qa_self_html() . '"',
-
-	'style' => 'tall',
-
-	'ok' => empty($incode) ? qa_lang_html('users/reset_code_emailed') : null,
-
-	'fields' => array(
-		'email_handle' => array(
-			'label' => qa_opt('allow_login_email_only') ? qa_lang_html('users/email_label') : qa_lang_html('users/email_handle_label'),
-			'tags' => 'name="emailhandle" id="emailhandle"',
-			'value' => qa_html(@$inemailhandle),
-			'error' => qa_html(@$errors['emailhandle']),
-		),
-
-		'code' => array(
-			'label' => qa_lang_html('users/reset_code_label'),
-			'tags' => 'name="code" id="code"',
-			'value' => qa_html(@$incode),
-			'error' => qa_html(@$errors['code']),
-			'note' => qa_lang_html('users/reset_code_emailed') . ' - ' .
-				'<a href="' . qa_html($forgotpath) . '">' . qa_lang_html('users/reset_code_another') . '</a>',
-		),
-	),
+$qa_content['error'] = isset($errors['page']) ? $errors['page'] : null;
 
-	'buttons' => array(
-		'reset' => array(
-			'label' => qa_lang_html('users/send_password_button'),
-		),
-	),
+if (!isset($errors['page'])) {
+	// Using this form action instead of qa_self_html() to get rid of the 's' (success) GET parameter from forgot.php
+	$qa_content['form'] = array(
+		'tags' => 'method="post" action="' . qa_path('reset') . '"',
 
-	'hidden' => array(
-		'doreset' => '1',
-		'formcode' => qa_get_form_security_code('reset'),
-	),
-);
+		'style' => 'tall',
+
+		'ok' => qa_get('s') ? qa_lang_html('users/reset_code_emailed') : null,
 
-$qa_content['focusid'] = (isset($errors['emailhandle']) || !strlen(@$inemailhandle)) ? 'emailhandle' : 'code';
+		'fields' => $fields,
+
+		'buttons' => $buttons,
+
+		'hidden' => $hidden,
+	);
+}
 
+$qa_content['focusid'] = $focusId;
 
 return $qa_content;