From 101d3b763308efe823d03c09bd0a4a109d19fc26 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Mon, 14 Mar 2016 14:42:48 +0100 Subject: [PATCH] [ticket/10432] Don't require username when user forgets password PHPBB3-10432 --- phpBB/includes/ucp/ucp_remind.php | 152 ++++++++++-------- phpBB/language/en/ucp.php | 1 + .../styles/prosilver/template/ucp_remind.html | 13 +- tests/functional/user_password_reset_test.php | 47 ++++-- .../phpbb_functional_test_case.php | 5 +- 5 files changed, 132 insertions(+), 86 deletions(-) diff --git a/phpBB/includes/ucp/ucp_remind.php b/phpBB/includes/ucp/ucp_remind.php index f46df99edb..0843d6c249 100644 --- a/phpBB/includes/ucp/ucp_remind.php +++ b/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)) . "' - AND username_clean = '" . $db->sql_escape(utf8_clean_string($username)) . "'" + 'WHERE' => "user_email_hash = '" . $db->sql_escape(phpbb_email_hash($email)) . "'" . + (!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, + )); } - - if ($user_row['user_type'] == USER_IGNORE) + else { - trigger_error('NO_USER'); - } + $user_row = $db->sql_fetchrow($result); + $db->sql_freeresult($result); - if ($user_row['user_type'] == USER_INACTIVE) - { - if ($user_row['user_inactive_reason'] == INACTIVE_MANUAL) + if (!$user_row) { - 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'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); + 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'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); - trigger_error($message); } $template->assign_vars(array( diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 5056e23d24..301739186c 100644 --- a/phpBB/language/en/ucp.php +++ b/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', diff --git a/phpBB/styles/prosilver/template/ucp_remind.html b/phpBB/styles/prosilver/template/ucp_remind.html index 0ab1251d9e..8b700de430 100644 --- a/phpBB/styles/prosilver/template/ucp_remind.html +++ b/phpBB/styles/prosilver/template/ucp_remind.html @@ -9,14 +9,19 @@

{L_SEND_PASSWORD}

+ {% if USERNAME_REQUIRED %} +

{{ lang('EMAIL_NOT_UNIQUE') }}

+ {% endif %} +
+

{L_EMAIL_REMIND}
+
+
+ {% if USERNAME_REQUIRED %}
-
-

{L_EMAIL_REMIND}
-
-
+ {% endif %}
 
{S_HIDDEN_FIELDS} 
diff --git a/tests/functional/user_password_reset_test.php b/tests/functional/user_password_reset_test.php index 3da78407cf..af53ba2b0d 100644 --- a/tests/functional/user_password_reset_test.php +++ b/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( - 'username' => 'reset-password-test-user', - )); + $form = $crawler->selectButton('submit')->form(); $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' => 'nobody@example.com', + 'email' => 'reset-password-test-user@test.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); diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 12a296a4bf..d4856f954a 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/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',