[ticket/10432] Don't require username when user forgets password

PHPBB3-10432

phpBB/includes/ucp/ucp_remind.php

@@ -50,11 +50,16 @@ class ucp_remind
 				trigger_error('FORM_INVALID');
 			}
 
+			if (empty($email))
+			{
+				trigger_error('NO_EMAIL_USER');
+			}
+
 			$sql_array = array(
 				'SELECT'	=> 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type, user_lang, user_inactive_reason',
 				'FROM'		=> array(USERS_TABLE => 'u'),
-				'WHERE'		=> "user_email_hash = '" . $db->sql_escape(phpbb_email_hash($email)) . "'
+				'WHERE'		=> "user_email_hash = '" . $db->sql_escape(phpbb_email_hash($email)) . "'" .
-									AND username_clean = '" . $db->sql_escape(utf8_clean_string($username)) . "'"
+					(!empty($username) ? " AND username_clean = '" . $db->sql_escape(utf8_clean_string($username)) . "'" : ''),
 			);
 
 			/**
@@ -75,81 +80,94 @@ class ucp_remind
 
 			$sql = $db->sql_build_query('SELECT', $sql_array);
 			$result = $db->sql_query($sql);
-			$user_row = $db->sql_fetchrow($result);
-			$db->sql_freeresult($result);
 
-			if (!$user_row)
+			if ($db->sql_affectedrows() > 1)
 			{
-				trigger_error('NO_EMAIL_USER');
+				$db->sql_freeresult($result);
+
+				$template->assign_vars(array(
+					'USERNAME_REQUIRED'	=> true,
+					'EMAIL'				=> $email,
+				));
 			}
-
+			else
-			if ($user_row['user_type'] == USER_IGNORE)
 			{
-				trigger_error('NO_USER');
+				$user_row = $db->sql_fetchrow($result);
-			}
+				$db->sql_freeresult($result);
 
-			if ($user_row['user_type'] == USER_INACTIVE)
+				if (!$user_row)
-			{
-				if ($user_row['user_inactive_reason'] == INACTIVE_MANUAL)
 				{
-					trigger_error('ACCOUNT_DEACTIVATED');
+					trigger_error('NO_EMAIL_USER');
 				}
-				else
+
+				if ($user_row['user_type'] == USER_IGNORE)
 				{
-					trigger_error('ACCOUNT_NOT_ACTIVATED');
+					trigger_error('NO_USER');
 				}
+
+				if ($user_row['user_type'] == USER_INACTIVE)
+				{
+					if ($user_row['user_inactive_reason'] == INACTIVE_MANUAL)
+					{
+						trigger_error('ACCOUNT_DEACTIVATED');
+					}
+					else
+					{
+						trigger_error('ACCOUNT_NOT_ACTIVATED');
+					}
+				}
+
+				// Check users permissions
+				$auth2 = new \phpbb\auth\auth();
+				$auth2->acl($user_row);
+
+				if (!$auth2->acl_get('u_chgpasswd'))
+				{
+					send_status_line(403, 'Forbidden');
+					trigger_error('NO_AUTH_PASSWORD_REMINDER');
+				}
+
+				$server_url = generate_board_url();
+
+				// Make password at least 8 characters long, make it longer if admin wants to.
+				// gen_rand_string() however has a limit of 12 or 13.
+				$user_password = gen_rand_string_friendly(max(8, mt_rand((int) $config['min_pass_chars'], (int) $config['max_pass_chars'])));
+
+				// For the activation key a random length between 6 and 10 will do.
+				$user_actkey = gen_rand_string(mt_rand(6, 10));
+
+				// Instantiate passwords manager
+				/* @var $manager \phpbb\passwords\manager */
+				$passwords_manager = $phpbb_container->get('passwords.manager');
+
+				$sql = 'UPDATE ' . USERS_TABLE . "
+					SET user_newpasswd = '" . $db->sql_escape($passwords_manager->hash($user_password)) . "', user_actkey = '" . $db->sql_escape($user_actkey) . "'
+					WHERE user_id = " . $user_row['user_id'];
+				$db->sql_query($sql);
+
+				include_once($phpbb_root_path . 'includes/functions_messenger.' . $phpEx);
+
+				$messenger = new messenger(false);
+
+				$messenger->template('user_activate_passwd', $user_row['user_lang']);
+
+				$messenger->set_addresses($user_row);
+
+				$messenger->anti_abuse_headers($config, $user);
+
+				$messenger->assign_vars(array(
+					'USERNAME'		=> htmlspecialchars_decode($user_row['username']),
+					'PASSWORD'		=> htmlspecialchars_decode($user_password),
+					'U_ACTIVATE'	=> "$server_url/ucp.$phpEx?mode=activate&u={$user_row['user_id']}&k=$user_actkey")
+				);
+
+				$messenger->send($user_row['user_notify_type']);
+
+				meta_refresh(3, append_sid("{$phpbb_root_path}index.$phpEx"));
+
+				$message = $user->lang['PASSWORD_UPDATED'] . '<br /><br />' . sprintf($user->lang['RETURN_INDEX'], '<a href="' . append_sid("{$phpbb_root_path}index.$phpEx") . '">', '</a>');
+				trigger_error($message);
 			}
-
-			// Check users permissions
-			$auth2 = new \phpbb\auth\auth();
-			$auth2->acl($user_row);
-
-			if (!$auth2->acl_get('u_chgpasswd'))
-			{
-				send_status_line(403, 'Forbidden');
-				trigger_error('NO_AUTH_PASSWORD_REMINDER');
-			}
-
-			$server_url = generate_board_url();
-
-			// Make password at least 8 characters long, make it longer if admin wants to.
-			// gen_rand_string() however has a limit of 12 or 13.
-			$user_password = gen_rand_string_friendly(max(8, mt_rand((int) $config['min_pass_chars'], (int) $config['max_pass_chars'])));
-
-			// For the activation key a random length between 6 and 10 will do.
-			$user_actkey = gen_rand_string(mt_rand(6, 10));
-
-			// Instantiate passwords manager
-			/* @var $manager \phpbb\passwords\manager */
-			$passwords_manager = $phpbb_container->get('passwords.manager');
-
-			$sql = 'UPDATE ' . USERS_TABLE . "
-				SET user_newpasswd = '" . $db->sql_escape($passwords_manager->hash($user_password)) . "', user_actkey = '" . $db->sql_escape($user_actkey) . "'
-				WHERE user_id = " . $user_row['user_id'];
-			$db->sql_query($sql);
-
-			include_once($phpbb_root_path . 'includes/functions_messenger.' . $phpEx);
-
-			$messenger = new messenger(false);
-
-			$messenger->template('user_activate_passwd', $user_row['user_lang']);
-
-			$messenger->set_addresses($user_row);
-
-			$messenger->anti_abuse_headers($config, $user);
-
-			$messenger->assign_vars(array(
-				'USERNAME'		=> htmlspecialchars_decode($user_row['username']),
-				'PASSWORD'		=> htmlspecialchars_decode($user_password),
-				'U_ACTIVATE'	=> "$server_url/ucp.$phpEx?mode=activate&u={$user_row['user_id']}&k=$user_actkey")
-			);
-
-			$messenger->send($user_row['user_notify_type']);
-
-			meta_refresh(3, append_sid("{$phpbb_root_path}index.$phpEx"));
-
-			$message = $user->lang['PASSWORD_UPDATED'] . '<br /><br />' . sprintf($user->lang['RETURN_INDEX'], '<a href="' . append_sid("{$phpbb_root_path}index.$phpEx") . '">', '</a>');
-			trigger_error($message);
 		}
 
 		$template->assign_vars(array(

phpBB/language/en/ucp.php

@@ -386,6 +386,7 @@ $lang = array_merge($lang, array(
 	'NO_BOOKMARKS_SELECTED'		=> 'You have selected no bookmarks.',
 	'NO_EDIT_READ_MESSAGE'		=> 'Private message cannot be edited because it has already been read.',
 	'NO_EMAIL_USER'				=> 'The email/username information submitted could not be found.',
+	'EMAIL_NOT_UNIQUE'			=> 'Email you specified is used by multiple users. You must specify username as well.',
 	'NO_FOES'					=> 'No foes currently defined',
 	'NO_FRIENDS'				=> 'No friends currently defined',
 	'NO_FRIENDS_OFFLINE'		=> 'No friends offline',

phpBB/styles/prosilver/template/ucp_remind.html

@@ -9,14 +9,19 @@
 		<h2>{L_SEND_PASSWORD}</h2>
 
 		<fieldset>
+			{% if USERNAME_REQUIRED %}
+				<p class="error">{{ lang('EMAIL_NOT_UNIQUE') }}</p>
+			{% endif %}
+		<dl>
+			<dt><label for="email">{L_EMAIL_ADDRESS}{L_COLON}</label><br /><span>{L_EMAIL_REMIND}</span></dt>
+			<dd><input class="inputbox narrow" type="email" name="email" id="email" size="25" maxlength="100" value="{{ EMAIL }}" autofocus /></dd>
+		</dl>
+		{% if USERNAME_REQUIRED %}
 		<dl>
 			<dt><label for="username">{L_USERNAME}{L_COLON}</label></dt>
 			<dd><input class="inputbox narrow" type="text" name="username" id="username" size="25" /></dd>
 		</dl>
-		<dl>
+		{% endif %}
-			<dt><label for="email">{L_EMAIL_ADDRESS}{L_COLON}</label><br /><span>{L_EMAIL_REMIND}</span></dt>
-			<dd><input class="inputbox narrow" type="email" name="email" id="email" size="25" maxlength="100" /></dd>
-		</dl>
 		<dl>
 			<dt>&nbsp;</dt>
 			<dd>{S_HIDDEN_FIELDS}<input type="submit" name="submit" id="submit" class="button1" value="{L_SUBMIT}" tabindex="2" />&nbsp; <input type="reset" value="{L_RESET}" name="reset" class="button2" /></dd>

tests/functional/user_password_reset_test.php

@@ -21,25 +21,46 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca
 	public function test_password_reset()
 	{
 		$this->add_lang('ucp');
-		$user_id = $this->create_user('reset-password-test-user');
+		$user_id = $this->create_user('reset-password-test-user', 'reset-password-test-user@test.com');
 
 		$crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}");
-		$form = $crawler->selectButton('submit')->form(array(
+		$form = $crawler->selectButton('submit')->form();
-			'username'	=> 'reset-password-test-user',
-		));
 		$crawler = self::submit($form);
 		$this->assertContainsLang('NO_EMAIL_USER', $crawler->text());
 
 		$crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}");
 		$form = $crawler->selectButton('submit')->form(array(
-			'username'	=> 'reset-password-test-user',
+			'email'		=> 'reset-password-test-user@test.com',
-			'email'		=> 'nobody@example.com',
 		));
 		$crawler = self::submit($form);
 		$this->assertContainsLang('PASSWORD_UPDATED', $crawler->text());
 
 		// Check if columns in database were updated for password reset
-		$this->get_user_data();
+		$this->get_user_data('reset-password-test-user');
+		$this->assertNotNull($this->user_data['user_actkey']);
+		$this->assertNotNull($this->user_data['user_newpasswd']);
+
+		// Create another user with the same email
+		$this->create_user('reset-password-test-user1', 'reset-password-test-user@test.com');
+
+		// Test that username is now also required
+		$crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}");
+		$form = $crawler->selectButton('submit')->form(array(
+			'email'		=> 'reset-password-test-user@test.com',
+		));
+		$crawler = self::submit($form);
+		$this->assertContainsLang('EMAIL_NOT_UNIQUE', $crawler->text());
+
+		// Provide both username and email
+		$form = $crawler->selectButton('submit')->form(array(
+			'email'		=> 'reset-password-test-user@test.com',
+			'username'	=> 'reset-password-test-user1',
+		));
+		$crawler = self::submit($form);
+		$this->assertContainsLang('PASSWORD_UPDATED', $crawler->text());
+
+		// Check if columns in database were updated for password reset
+		$this->get_user_data('reset-password-test-user1');
 		$this->assertNotNull($this->user_data['user_actkey']);
 		$this->assertNotNull($this->user_data['user_newpasswd']);
 
@@ -73,7 +94,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca
 	public function test_activate_new_password($expected, $user_id, $act_key)
 	{
 		$this->add_lang('ucp');
-		$this->get_user_data();
+		$this->get_user_data('reset-password-test-user');
 		$user_id = (!$user_id) ? $this->user_data['user_id'] : $user_id;
 		$act_key = (!$act_key) ? $this->user_data['user_actkey'] : $act_key;
 
@@ -119,7 +140,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca
 	public function test_acivateAfterDeactivate()
 	{
 		// User is active, actkey should not exist
-		$this->get_user_data();
+		$this->get_user_data('reset-password-test-user');
 		$this->assertEmpty($this->user_data['user_actkey']);
 
 		$this->login();
@@ -143,7 +164,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca
 		$crawler = self::request('GET', preg_replace('#(.+)(adm/index.php.+)#', '$2', $link->getUri()));
 
 		// Ensure again that actkey is empty after deactivation
-		$this->get_user_data();
+		$this->get_user_data('reset-password-test-user');
 		$this->assertEmpty($this->user_data['user_actkey']);
 
 		// Force reactivation of account and check that act key is not empty anymore
@@ -152,16 +173,16 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca
 		$crawler = self::submit($form, array('action' => 'reactivate'));
 		$this->assertContainsLang('FORCE_REACTIVATION_SUCCESS', $crawler->filter('html')->text());
 
-		$this->get_user_data();
+		$this->get_user_data('reset-password-test-user');
 		$this->assertNotEmpty($this->user_data['user_actkey']);
 	}
 
-	protected function get_user_data()
+	protected function get_user_data($username)
 	{
 		$db = $this->get_db();
 		$sql = 'SELECT user_id, username, user_type, user_email, user_newpasswd, user_lang, user_notify_type, user_actkey, user_inactive_reason
 			FROM ' . USERS_TABLE . "
-			WHERE username = 'reset-password-test-user'";
+			WHERE username = '$username'";
 		$result = $db->sql_query($sql);
 		$this->user_data = $db->sql_fetchrow($result);
 		$db->sql_freeresult($result);

tests/test_framework/phpbb_functional_test_case.php

@@ -550,9 +550,10 @@ class phpbb_functional_test_case extends phpbb_test_case
 	* Creates a new user with limited permissions
 	*
 	* @param string $username Also doubles up as the user's password
+	* @param string $email User email (defaults to nobody@example.com)
 	* @return int ID of created user
 	*/
-	protected function create_user($username)
+	protected function create_user($username, $email = 'nobody@example.com')
 	{
 		// Required by unique_id
 		global $config;
@@ -604,7 +605,7 @@ class phpbb_functional_test_case extends phpbb_test_case
 		$user_row = array(
 			'username' => $username,
 			'group_id' => 2,
-			'user_email' => 'nobody@example.com',
+			'user_email' => $email,
 			'user_type' => 0,
 			'user_lang' => 'en',
 			'user_timezone' => 'UTC',