From 101d3b763308efe823d03c09bd0a4a109d19fc26 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Mon, 14 Mar 2016 14:42:48 +0100 Subject: [ticket/10432] Don't require username when user forgets password PHPBB3-10432 --- phpBB/includes/ucp/ucp_remind.php | 122 ++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 52 deletions(-) (limited to 'phpBB/includes/ucp') 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); - if ($user_row['user_type'] == USER_IGNORE) - { - trigger_error('NO_USER'); + $template->assign_vars(array( + 'USERNAME_REQUIRED' => true, + 'EMAIL' => $email, + )); } - - if ($user_row['user_type'] == USER_INACTIVE) + else { - if ($user_row['user_inactive_reason'] == INACTIVE_MANUAL) + $user_row = $db->sql_fetchrow($result); + $db->sql_freeresult($result); + + if (!$user_row) + { + trigger_error('NO_EMAIL_USER'); + } + + if ($user_row['user_type'] == USER_IGNORE) { - trigger_error('ACCOUNT_DEACTIVATED'); + trigger_error('NO_USER'); } - else + + if ($user_row['user_type'] == USER_INACTIVE) { - trigger_error('ACCOUNT_NOT_ACTIVATED'); + 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); + // 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'); - } + if (!$auth2->acl_get('u_chgpasswd')) + { + send_status_line(403, 'Forbidden'); + trigger_error('NO_AUTH_PASSWORD_REMINDER'); + } - $server_url = generate_board_url(); + $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']))); + // 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)); + // 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'); + // 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); + $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); + include_once($phpbb_root_path . 'includes/functions_messenger.' . $phpEx); - $messenger = new messenger(false); + $messenger = new messenger(false); - $messenger->template('user_activate_passwd', $user_row['user_lang']); + $messenger->template('user_activate_passwd', $user_row['user_lang']); - $messenger->set_addresses($user_row); + $messenger->set_addresses($user_row); - $messenger->anti_abuse_headers($config, $user); + $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->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']); + $messenger->send($user_row['user_notify_type']); - meta_refresh(3, append_sid("{$phpbb_root_path}index.$phpEx")); + meta_refresh(3, append_sid("{$phpbb_root_path}index.$phpEx")); - $message = $user->lang['PASSWORD_UPDATED'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); - trigger_error($message); + $message = $user->lang['PASSWORD_UPDATED'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); + trigger_error($message); + } } $template->assign_vars(array( -- cgit v1.2.3-65-gdbad From 30d1048c8e3b66f3a3144974f9f3fc87054b2be2 Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Tue, 23 Oct 2018 15:18:57 +0200 Subject: [ticket/10432] Fix for SQLite PHPBB3-10432 --- phpBB/includes/ucp/ucp_remind.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'phpBB/includes/ucp') diff --git a/phpBB/includes/ucp/ucp_remind.php b/phpBB/includes/ucp/ucp_remind.php index 0843d6c249..8cf7ea268d 100644 --- a/phpBB/includes/ucp/ucp_remind.php +++ b/phpBB/includes/ucp/ucp_remind.php @@ -80,8 +80,9 @@ class ucp_remind $sql = $db->sql_build_query('SELECT', $sql_array); $result = $db->sql_query($sql); + $rowset = $db->sql_fetchrowset($result); - if ($db->sql_affectedrows() > 1) + if (count($rowset) > 1) { $db->sql_freeresult($result); @@ -92,7 +93,7 @@ class ucp_remind } else { - $user_row = $db->sql_fetchrow($result); + $user_row = $rowset[0]; $db->sql_freeresult($result); if (!$user_row) -- cgit v1.2.3-65-gdbad From 1d2a654ad7f34367cc5f8c8b3d5893e617b92f3f Mon Sep 17 00:00:00 2001 From: Jakub Senko Date: Sun, 28 Oct 2018 10:12:13 +0100 Subject: [ticket/10432] Fix errors and address privacy concern PHPBB3-10432 --- phpBB/includes/ucp/ucp_remind.php | 33 ++++++++++----------------- phpBB/language/en/common.php | 1 - phpBB/language/en/ucp.php | 3 +-- tests/functional/user_password_reset_test.php | 16 ++++++++++--- 4 files changed, 26 insertions(+), 27 deletions(-) (limited to 'phpBB/includes/ucp') diff --git a/phpBB/includes/ucp/ucp_remind.php b/phpBB/includes/ucp/ucp_remind.php index 8cf7ea268d..e50428bfea 100644 --- a/phpBB/includes/ucp/ucp_remind.php +++ b/phpBB/includes/ucp/ucp_remind.php @@ -79,7 +79,7 @@ class ucp_remind extract($phpbb_dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); $sql = $db->sql_build_query('SELECT', $sql_array); - $result = $db->sql_query($sql); + $result = $db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need $rowset = $db->sql_fetchrowset($result); if (count($rowset) > 1) @@ -93,29 +93,24 @@ class ucp_remind } else { - $user_row = $rowset[0]; - $db->sql_freeresult($result); + $message = $user->lang['PASSWORD_UPDATED_IF_EXISTED'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); - if (!$user_row) + if (empty($rowset)) { - trigger_error('NO_EMAIL_USER'); + trigger_error($message); } - if ($user_row['user_type'] == USER_IGNORE) + $user_row = $rowset[0]; + $db->sql_freeresult($result); + + if (!$user_row) { - trigger_error('NO_USER'); + trigger_error($message); } - if ($user_row['user_type'] == USER_INACTIVE) + if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) { - if ($user_row['user_inactive_reason'] == INACTIVE_MANUAL) - { - trigger_error('ACCOUNT_DEACTIVATED'); - } - else - { - trigger_error('ACCOUNT_NOT_ACTIVATED'); - } + trigger_error($message); } // Check users permissions @@ -124,8 +119,7 @@ class ucp_remind if (!$auth2->acl_get('u_chgpasswd')) { - send_status_line(403, 'Forbidden'); - trigger_error('NO_AUTH_PASSWORD_REMINDER'); + trigger_error($message); } $server_url = generate_board_url(); @@ -164,9 +158,6 @@ class ucp_remind $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); } } diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 213563aea0..a037c5bfe8 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -62,7 +62,6 @@ $lang = array_merge($lang, array( 'ACCOUNT_ALREADY_ACTIVATED' => 'Your account has already been activated.', 'ACCOUNT_DEACTIVATED' => 'Your account has been manually deactivated and is only able to be reactivated by an administrator.', - 'ACCOUNT_NOT_ACTIVATED' => 'Your account has not been activated yet.', 'ACP' => 'Administration Control Panel', 'ACP_SHORT' => 'ACP', 'ACTIVE' => 'active', diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 301739186c..cfcbc63144 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -373,7 +373,6 @@ $lang = array_merge($lang, array( 'NO_AUTH_EDIT_MESSAGE' => 'You are not authorised to edit private messages.', 'NO_AUTH_FORWARD_MESSAGE' => 'You are not authorised to forward private messages.', 'NO_AUTH_GROUP_MESSAGE' => 'You are not authorised to send private messages to groups.', - 'NO_AUTH_PASSWORD_REMINDER' => 'You are not authorised to request a new password.', 'NO_AUTH_PROFILEINFO' => 'You are not authorised to change your profile information.', 'NO_AUTH_READ_HOLD_MESSAGE' => 'You are not authorised to read private messages that are on hold.', 'NO_AUTH_READ_MESSAGE' => 'You are not authorised to read private messages.', @@ -412,7 +411,7 @@ $lang = array_merge($lang, array( 'PASS_TYPE_SYMBOL_EXPLAIN' => 'Password must be between %1$s and %2$s long, must contain letters in mixed case, must contain numbers and must contain symbols.', 'PASSWORD' => 'Password', 'PASSWORD_ACTIVATED' => 'Your new password has been activated.', - 'PASSWORD_UPDATED' => 'A new password was sent to your registered email address.', + 'PASSWORD_UPDATED_IF_EXISTED' => 'If your account exists a new password was sent to your registered email address. If it does not, it may be because you are banned, not activated your account yet or not allowed to change password. Contact admin if that is the case.', 'PERMISSIONS_RESTORED' => 'Successfully restored original permissions.', 'PERMISSIONS_TRANSFERRED' => 'Successfully transferred permissions from %s, you are now able to browse the board with this user’s permissions.
Please note that admin permissions were not transferred. You are able to revert to your permission set at any time.', 'PM_DISABLED' => 'Private messaging has been disabled on this board.', diff --git a/tests/functional/user_password_reset_test.php b/tests/functional/user_password_reset_test.php index af53ba2b0d..2361eed066 100644 --- a/tests/functional/user_password_reset_test.php +++ b/tests/functional/user_password_reset_test.php @@ -23,17 +23,27 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca $this->add_lang('ucp'); $user_id = $this->create_user('reset-password-test-user', 'reset-password-test-user@test.com'); + // test without email $crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}"); $form = $crawler->selectButton('submit')->form(); $crawler = self::submit($form); $this->assertContainsLang('NO_EMAIL_USER', $crawler->text()); + // test with non-existent email + $crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}"); + $form = $crawler->selectButton('submit')->form(array( + 'email' => 'non-existent@email.com', + )); + $crawler = self::submit($form); + $this->assertContainsLang('PASSWORD_UPDATED_IF_EXISTED', $crawler->text()); + + // test with correct email $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('PASSWORD_UPDATED', $crawler->text()); + $this->assertContainsLang('PASSWORD_UPDATED_IF_EXISTED', $crawler->text()); // Check if columns in database were updated for password reset $this->get_user_data('reset-password-test-user'); @@ -57,7 +67,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca 'username' => 'reset-password-test-user1', )); $crawler = self::submit($form); - $this->assertContainsLang('PASSWORD_UPDATED', $crawler->text()); + $this->assertContainsLang('PASSWORD_UPDATED_IF_EXISTED', $crawler->text()); // Check if columns in database were updated for password reset $this->get_user_data('reset-password-test-user1'); @@ -182,7 +192,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca $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 = '$username'"; + WHERE username = '" . $db->sql_escape($username) . "'"; $result = $db->sql_query($sql); $this->user_data = $db->sql_fetchrow($result); $db->sql_freeresult($result); -- cgit v1.2.3-65-gdbad