From 3f852a3233b2ee51c3fab7dc6d077778fd35e0fd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 8 Aug 2019 22:01:51 +0200 Subject: [PATCH 01/18] [ticket/11327] Move UCP remind functionality to a controller for password reset PHPBB3-11327 --- phpBB/config/default/container/services.yml | 1 + phpBB/config/default/container/services_ucp.yml | 15 +++++++++++++++ phpBB/config/default/routing/routing.yml | 4 ++++ phpBB/config/default/routing/ucp.yml | 3 +++ phpBB/includes/functions.php | 5 ++++- .../ucp/controller/reset_password.php} | 0 phpBB/ucp.php | 6 ++++-- 7 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 phpBB/config/default/container/services_ucp.yml create mode 100644 phpBB/config/default/routing/ucp.yml rename phpBB/{includes/ucp/ucp_remind.php => phpbb/ucp/controller/reset_password.php} (100%) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index 3ead1e6181..2d4720029d 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -27,6 +27,7 @@ imports: - { resource: services_text_formatter.yml } - { resource: services_text_reparser.yml } - { resource: services_twig.yml } + - { resource: services_ucp.yml } - { resource: services_user.yml } - { resource: tables.yml } diff --git a/phpBB/config/default/container/services_ucp.yml b/phpBB/config/default/container/services_ucp.yml new file mode 100644 index 0000000000..615a5698c4 --- /dev/null +++ b/phpBB/config/default/container/services_ucp.yml @@ -0,0 +1,15 @@ +services: + phpbb.ucp.controller.reset_password: + class: phpbb\ucp\controller\reset_password + arguments: + - '@config' + - '@dbal.conn' + - '@dispatcher' + - '@controller.helper' + - '@language' + - '@passwords.manager' + - '@request' + - '@template' + - '@user' + - '%core.root_path%' + - '%core.php_ext%' diff --git a/phpBB/config/default/routing/routing.yml b/phpBB/config/default/routing/routing.yml index 199c5229b0..a5e9265dc3 100644 --- a/phpBB/config/default/routing/routing.yml +++ b/phpBB/config/default/routing/routing.yml @@ -26,3 +26,7 @@ phpbb_help_routing: phpbb_report_routing: resource: report.yml + +phpbb_ucp_routing: + resource: ucp.yml + prefix: /user diff --git a/phpBB/config/default/routing/ucp.yml b/phpBB/config/default/routing/ucp.yml new file mode 100644 index 0000000000..be97f597a0 --- /dev/null +++ b/phpBB/config/default/routing/ucp.yml @@ -0,0 +1,3 @@ +phpbb_ucp_reset_password_controller: + path: /reset_password + defaults: { _controller: phpbb.ucp.controller.reset_password:handle } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 2aa98b384e..5ac26b450e 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2519,11 +2519,14 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa $s_hidden_fields = build_hidden_fields($s_hidden_fields); + /** @var \phpbb\controller\helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + $login_box_template_data = array( 'LOGIN_ERROR' => $err, 'LOGIN_EXPLAIN' => $l_explain, - 'U_SEND_PASSWORD' => ($config['email_enable']) ? append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=sendpassword') : '', + 'U_SEND_PASSWORD' => ($config['email_enable']) ? $controller_helper->route('phpbb_ucp_reset_password_controller') : '', 'U_RESEND_ACTIVATION' => ($config['require_activation'] == USER_ACTIVATION_SELF && $config['email_enable']) ? append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=resend_act') : '', 'U_TERMS_USE' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=terms'), 'U_PRIVACY' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=privacy'), diff --git a/phpBB/includes/ucp/ucp_remind.php b/phpBB/phpbb/ucp/controller/reset_password.php similarity index 100% rename from phpBB/includes/ucp/ucp_remind.php rename to phpBB/phpbb/ucp/controller/reset_password.php diff --git a/phpBB/ucp.php b/phpBB/ucp.php index c60d9930fc..34a6c6bb99 100644 --- a/phpBB/ucp.php +++ b/phpBB/ucp.php @@ -63,8 +63,10 @@ switch ($mode) break; case 'sendpassword': - $module->load('ucp', 'remind'); - $module->display($user->lang['UCP_REMIND']); + /** @var \phpbb\controller\helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + + redirect($controller_helper->route('phpbb_ucp_reset_password_controller')); break; case 'register': From 276f350bcb29e5db6efc1a91747f2a83bcfd9246 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 8 Aug 2019 22:03:27 +0200 Subject: [PATCH 02/18] [ticket/11327] Refactor ucp_remind to reset_password controller PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 149 +++++++++++++----- 1 file changed, 109 insertions(+), 40 deletions(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index e50428bfea..49c264be6c 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -11,34 +11,105 @@ * */ -/** -* @ignore -*/ -if (!defined('IN_PHPBB')) -{ - exit; -} +namespace phpbb\ucp\controller; + +use phpbb\config\config; +use phpbb\controller\helper; +use phpbb\db\driver\driver_interface; +use phpbb\event\dispatcher; +use phpbb\language\language; +use phpbb\passwords\manager; +use phpbb\request\request_interface; +use phpbb\template\template; +use phpbb\user; /** * ucp_remind * Sending password reminders */ -class ucp_remind +class reset_password { - var $u_action; + /** @var config */ + protected $config; - function main($id, $mode) + /** @var driver_interface */ + protected $db; + + /** @var dispatcher */ + protected $dispatcher; + + /** @var helper */ + protected $helper; + + /** @var language */ + protected $language; + + /** @var manager */ + protected $passwords_manager; + + /** @var request_interface */ + protected $request; + + /** @var template */ + protected $template; + + /** @var user */ + protected $user; + + /** @var string phpBB root path */ + protected $root_path; + + /** @var string PHP extension */ + protected $php_ext; + + /** + * ucp_remind constructor. + * + * @param config $config + * @param driver_interface $db + * @param dispatcher $dispatcher + * @param helper $helper + * @param language $language + * @param manager $passwords_manager + * @param request_interface $request + * @param template $template + * @param user $user + * @param $root_path + * @param $php_ext + */ + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, helper $helper, + language $language, manager $passwords_manager, request_interface $request, + template $template, user $user, $root_path, $php_ext) { - global $config, $phpbb_root_path, $phpEx, $request; - global $db, $user, $template, $phpbb_container, $phpbb_dispatcher; + $this->config = $config; + $this->db = $db; + $this->dispatcher = $dispatcher; + $this->helper = $helper; + $this->language = $language; + $this->passwords_manager = $passwords_manager; + $this->request = $request; + $this->template = $template; + $this->user = $user; + $this->root_path = $root_path; + $this->php_ext = $php_ext; + } - if (!$config['allow_password_reset']) + /** + * Handle controller requests + * + * @return \Symfony\Component\HttpFoundation\Response + */ + public function handle() + { + $this->language->add_lang('ucp'); + + if (!$this->config['allow_password_reset']) { - trigger_error($user->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); } - $username = $request->variable('username', '', true); - $email = strtolower($request->variable('email', '')); + $username = $this->request->variable('username', '', true); + $email = strtolower($this->request->variable('email', '')); $submit = (isset($_POST['submit'])) ? true : false; add_form_key('ucp_remind'); @@ -58,8 +129,8 @@ class ucp_remind $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)) . "'" . - (!empty($username) ? " AND username_clean = '" . $db->sql_escape(utf8_clean_string($username)) . "'" : ''), + 'WHERE' => "user_email_hash = '" . $this->db->sql_escape(phpbb_email_hash($email)) . "'" . + (!empty($username) ? " AND username_clean = '" . $this->db->sql_escape(utf8_clean_string($username)) . "'" : ''), ); /** @@ -76,24 +147,24 @@ class ucp_remind 'username', 'sql_array', ); - extract($phpbb_dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); + extract($this->dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); - $sql = $db->sql_build_query('SELECT', $sql_array); - $result = $db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need - $rowset = $db->sql_fetchrowset($result); + $sql = $this->db->sql_build_query('SELECT', $sql_array); + $result = $this->db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need + $rowset = $this->db->sql_fetchrowset($result); if (count($rowset) > 1) { - $db->sql_freeresult($result); + $this->db->sql_freeresult($result); - $template->assign_vars(array( + $this->template->assign_vars(array( 'USERNAME_REQUIRED' => true, 'EMAIL' => $email, )); } else { - $message = $user->lang['PASSWORD_UPDATED_IF_EXISTED'] . '

' . sprintf($user->lang['RETURN_INDEX'], '', ''); + $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); if (empty($rowset)) { @@ -101,7 +172,7 @@ class ucp_remind } $user_row = $rowset[0]; - $db->sql_freeresult($result); + $this->db->sql_freeresult($result); if (!$user_row) { @@ -126,21 +197,17 @@ class ucp_remind // 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']))); + $user_password = gen_rand_string_friendly(max(8, mt_rand((int) $this->config['min_pass_chars'], (int) $this->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) . "' + SET user_newpasswd = '" . $this->db->sql_escape($this->passwords_manager->hash($user_password)) . "', user_actkey = '" . $this->db->sql_escape($user_actkey) . "' WHERE user_id = " . $user_row['user_id']; - $db->sql_query($sql); + $this->db->sql_query($sql); - include_once($phpbb_root_path . 'includes/functions_messenger.' . $phpEx); + include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); $messenger = new messenger(false); @@ -148,12 +215,12 @@ class ucp_remind $messenger->set_addresses($user_row); - $messenger->anti_abuse_headers($config, $user); + $messenger->anti_abuse_headers($this->config, $this->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") + 'U_ACTIVATE' => "$server_url/ucp.{$this->php_ext}?mode=activate&u={$user_row['user_id']}&k=$user_actkey") ); $messenger->send($user_row['user_notify_type']); @@ -162,13 +229,15 @@ class ucp_remind } } - $template->assign_vars(array( + $this->template->assign_vars(array( 'USERNAME' => $username, 'EMAIL' => $email, - 'S_PROFILE_ACTION' => append_sid($phpbb_root_path . 'ucp.' . $phpEx, 'mode=sendpassword')) + 'S_PROFILE_ACTION' => append_sid($this->root_path . 'ucp.' . $this->php_ext, 'mode=sendpassword')) ); - $this->tpl_name = 'ucp_remind'; - $this->page_title = 'UCP_REMIND'; + //$this->tpl_name = 'ucp_remind'; + //$this->page_title = 'UCP_REMIND'; + + return $this->helper->render('ucp_remind.html', 'UCP_REMIND'); } } From fe030f67efc354729033ebc1726fc5b729057fb2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 8 Aug 2019 22:11:13 +0200 Subject: [PATCH 03/18] [ticket/11327] Adjust display of page title PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 49c264be6c..55397b1c0e 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -235,9 +235,6 @@ class reset_password 'S_PROFILE_ACTION' => append_sid($this->root_path . 'ucp.' . $this->php_ext, 'mode=sendpassword')) ); - //$this->tpl_name = 'ucp_remind'; - //$this->page_title = 'UCP_REMIND'; - - return $this->helper->render('ucp_remind.html', 'UCP_REMIND'); + return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); } } From f41c51d1ecc36bc6e5dffc70ad7a5ed50d4ec7ea Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Aug 2019 22:43:04 +0200 Subject: [PATCH 04/18] [ticket/11327] Add reset token columns & config setting PHPBB3-11327 --- .../db/migration/data/v330/reset_password.php | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 phpBB/phpbb/db/migration/data/v330/reset_password.php diff --git a/phpBB/phpbb/db/migration/data/v330/reset_password.php b/phpBB/phpbb/db/migration/data/v330/reset_password.php new file mode 100644 index 0000000000..87131c6e93 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v330/reset_password.php @@ -0,0 +1,48 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v330; + +class reset_password extends \phpbb\db\migration\migration +{ + static public function depends_on() + { + return array( + '\phpbb\db\migration\data\v330\dev', + ); + } + + public function update_schema() + { + return [ + 'add_columns' => [ + $this->table_prefix . 'users' => [ + 'reset_token' => ['VCHAR:64', '', 'after' => 'user_actkey'], + 'reset_token_expiration' => ['TIMESTAMP', 0, 'after' => 'reset_token'], + ], + ], + ]; + } + + public function revert_schema() + { + return [ + 'drop_columns' => [ + $this->table_prefix . 'users' => [ + 'reset_token', + 'reset_token_expiration', + ], + ], + ]; + } +} From 0a5599697fb9d52f067ac1846492641cf1adc05a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Aug 2019 22:51:51 +0200 Subject: [PATCH 05/18] [ticket/11327] Split up into forgot password and reset password PHPBB3-11327 --- .../config/default/container/services_ucp.yml | 1 + phpBB/config/default/routing/ucp.yml | 6 +- phpBB/includes/functions.php | 2 +- phpBB/phpbb/ucp/controller/reset_password.php | 168 ++++++++++++++++-- phpBB/ucp.php | 2 +- 5 files changed, 166 insertions(+), 13 deletions(-) diff --git a/phpBB/config/default/container/services_ucp.yml b/phpBB/config/default/container/services_ucp.yml index 615a5698c4..923f35033c 100644 --- a/phpBB/config/default/container/services_ucp.yml +++ b/phpBB/config/default/container/services_ucp.yml @@ -11,5 +11,6 @@ services: - '@request' - '@template' - '@user' + - '%tables%' - '%core.root_path%' - '%core.php_ext%' diff --git a/phpBB/config/default/routing/ucp.yml b/phpBB/config/default/routing/ucp.yml index be97f597a0..06bd7c3a58 100644 --- a/phpBB/config/default/routing/ucp.yml +++ b/phpBB/config/default/routing/ucp.yml @@ -1,3 +1,7 @@ phpbb_ucp_reset_password_controller: path: /reset_password - defaults: { _controller: phpbb.ucp.controller.reset_password:handle } + defaults: { _controller: phpbb.ucp.controller.reset_password:reset } + +phpbb_ucp_forgot_password_controller: + path: /forgot_password + defaults: { _controller: phpbb.ucp.controller.reset_password:request } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 5ac26b450e..e8f8b0ff46 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2526,7 +2526,7 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa 'LOGIN_ERROR' => $err, 'LOGIN_EXPLAIN' => $l_explain, - 'U_SEND_PASSWORD' => ($config['email_enable']) ? $controller_helper->route('phpbb_ucp_reset_password_controller') : '', + 'U_SEND_PASSWORD' => ($config['email_enable']) ? $controller_helper->route('phpbb_ucp_forgot_password_controller') : '', 'U_RESEND_ACTIVATION' => ($config['require_activation'] == USER_ACTIVATION_SELF && $config['email_enable']) ? append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=resend_act') : '', 'U_TERMS_USE' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=terms'), 'U_PRIVACY' => append_sid("{$phpbb_root_path}ucp.$phpEx", 'mode=privacy'), diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 55397b1c0e..57fef00f79 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -56,6 +56,9 @@ class reset_password /** @var user */ protected $user; + /** @var array phpBB DB table names */ + protected $tables; + /** @var string phpBB root path */ protected $root_path; @@ -74,12 +77,13 @@ class reset_password * @param request_interface $request * @param template $template * @param user $user + * @param array $tables * @param $root_path * @param $php_ext */ public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, helper $helper, language $language, manager $passwords_manager, request_interface $request, - template $template, user $user, $root_path, $php_ext) + template $template, user $user, $tables, $root_path, $php_ext) { $this->config = $config; $this->db = $db; @@ -94,12 +98,7 @@ class reset_password $this->php_ext = $php_ext; } - /** - * Handle controller requests - * - * @return \Symfony\Component\HttpFoundation\Response - */ - public function handle() + public function request() { $this->language->add_lang('ucp'); @@ -108,9 +107,158 @@ class reset_password trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); } + $submit = $this->request->is_set_post('submit'); $username = $this->request->variable('username', '', true); $email = strtolower($this->request->variable('email', '')); - $submit = (isset($_POST['submit'])) ? true : false; + + add_form_key('ucp_remind'); + + if ($submit) + { + if (!check_form_key('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, reset_token, reset_token_expiration', + 'FROM' => array(USERS_TABLE => 'u'), + 'WHERE' => "user_email_hash = '" . $this->db->sql_escape(phpbb_email_hash($email)) . "'" . + (!empty($username) ? " AND username_clean = '" . $this->db->sql_escape(utf8_clean_string($username)) . "'" : ''), + ); + + /** + * Change SQL query for fetching user data + * + * @event core.ucp_remind_modify_select_sql + * @var string email User's email from the form + * @var string username User's username from the form + * @var array sql_array Fully assembled SQL query with keys SELECT, FROM, WHERE + * @since 3.1.11-RC1 + */ + $vars = array( + 'email', + 'username', + 'sql_array', + ); + extract($this->dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); + + $sql = $this->db->sql_build_query('SELECT', $sql_array); + $result = $this->db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need + $rowset = $this->db->sql_fetchrowset($result); + + if (count($rowset) > 1) + { + $this->db->sql_freeresult($result); + + $this->template->assign_vars(array( + 'USERNAME_REQUIRED' => true, + 'EMAIL' => $email, + )); + } + else + { + $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); + + $user_row = empty($rowset) ? [] : $rowset[0]; + $this->db->sql_freeresult($result); + + if (!$user_row) + { + trigger_error($message); + } + + if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) + { + trigger_error($message); + } + + // Do not create multiple valid reset tokens + if (!empty($user_row['reset_token']) && (int) $user_row['reset_token_expiration'] <= (time() + $this->config['reset_token_lifetime'])) + { + trigger_error($message); + } + + // Check users permissions + $auth2 = new \phpbb\auth\auth(); + $auth2->acl($user_row); + + if (!$auth2->acl_get('u_chgpasswd')) + { + trigger_error($message); + } + + $server_url = generate_board_url(); + + // Generate reset token + $reset_token = gen_rand_string_friendly(32); + + $sql_ary = array( + 'reset_token' => $reset_token, + 'reset_token_expiration' => time() + $this->config['reset_token_lifetime'], + ); + + $sql = 'UPDATE ' . $this->tables['users'] . ' + SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . $user_row['user_id']; + $this->db->sql_query($sql); + + include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); + + /** @var \messenger $messenger */ + $messenger = new \messenger(false); + + $messenger->template('user_activate_passwd', $user_row['user_lang']); + + $messenger->set_addresses($user_row); + + $messenger->anti_abuse_headers($this->config, $this->user); + + $messenger->assign_vars(array( + 'USERNAME' => htmlspecialchars_decode($user_row['username']), + 'U_ACTIVATE' => $this->helper->route('phpbb_ucp_reset_password_controller') + )); + + $messenger->send($user_row['user_notify_type']); + + trigger_error($message); + } + } + + $this->template->assign_vars(array( + 'USERNAME' => $username, + 'EMAIL' => $email, + 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), + )); + + return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); + } + + /** + * Handle controller requests + * + * @return \Symfony\Component\HttpFoundation\Response + */ + public function reset() + { + $this->language->add_lang('ucp'); + + if (!$this->config['allow_password_reset']) + { + trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + } + + $submit = $this->request->is_set_post('submit'); + $username = $this->request->variable('username', '', true); + $email = strtolower($this->request->variable('email', '')); + $key = $this->request->variable('key', ''); + $user_id = $this->request->variable('user_id', 0); add_form_key('ucp_remind'); @@ -232,8 +380,8 @@ class reset_password $this->template->assign_vars(array( 'USERNAME' => $username, 'EMAIL' => $email, - 'S_PROFILE_ACTION' => append_sid($this->root_path . 'ucp.' . $this->php_ext, 'mode=sendpassword')) - ); + 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), + )); return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); } diff --git a/phpBB/ucp.php b/phpBB/ucp.php index 34a6c6bb99..0992e3ef90 100644 --- a/phpBB/ucp.php +++ b/phpBB/ucp.php @@ -66,7 +66,7 @@ switch ($mode) /** @var \phpbb\controller\helper $controller_helper */ $controller_helper = $phpbb_container->get('controller.helper'); - redirect($controller_helper->route('phpbb_ucp_reset_password_controller')); + redirect($controller_helper->route('phpbb_ucp_forgot_password_controller')); break; case 'register': From cfea54f8f366379de0bc75781da1c71cb06cc1ec Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Aug 2019 22:52:35 +0200 Subject: [PATCH 06/18] [ticket/11327] Rename email file to forgot password PHPBB3-11327 --- .../email/{user_activate_passwd.txt => user_forgot_password.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename phpBB/language/en/email/{user_activate_passwd.txt => user_forgot_password.txt} (100%) diff --git a/phpBB/language/en/email/user_activate_passwd.txt b/phpBB/language/en/email/user_forgot_password.txt similarity index 100% rename from phpBB/language/en/email/user_activate_passwd.txt rename to phpBB/language/en/email/user_forgot_password.txt From 1d1d963c14d9db54df69469758163f1f50a9b4b3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Aug 2019 23:32:49 +0200 Subject: [PATCH 07/18] [ticket/11327] Adjust wording of forgot password email PHPBB3-11327 --- phpBB/language/en/email/user_forgot_password.txt | 14 +++++--------- phpBB/phpbb/ucp/controller/reset_password.php | 4 ++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/phpBB/language/en/email/user_forgot_password.txt b/phpBB/language/en/email/user_forgot_password.txt index 965d5a552c..4826a7bfd9 100644 --- a/phpBB/language/en/email/user_forgot_password.txt +++ b/phpBB/language/en/email/user_forgot_password.txt @@ -1,17 +1,13 @@ -Subject: New password activation +Subject: Account password reset Hello {USERNAME} -You are receiving this notification because you have (or someone pretending to be you has) requested a new password be sent for your account on "{SITENAME}". If you did not request this notification then please ignore it, if you keep receiving it please contact the board administrator. +You are receiving this notification because you have requested to recover a forgotten password for your account on "{SITENAME}". -To use the new password you need to activate it. To do this click the link provided below. +To reset your password, please click the link provided below: -{U_ACTIVATE} +{U_RESET_PASSWORD} -If successful you will be able to login using the following password: - -Password: {PASSWORD} - -You can of course change this password yourself via the profile page. If you have any difficulties please contact the board administrator. +If you did not authorize the request you can ignore this email. Please contact the board administrator if you keep receiving it. {EMAIL_SIG} diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 57fef00f79..4b2660aebb 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -221,8 +221,8 @@ class reset_password $messenger->anti_abuse_headers($this->config, $this->user); $messenger->assign_vars(array( - 'USERNAME' => htmlspecialchars_decode($user_row['username']), - 'U_ACTIVATE' => $this->helper->route('phpbb_ucp_reset_password_controller') + 'USERNAME' => htmlspecialchars_decode($user_row['username']), + 'U_RESET_PASSWORD' => $this->helper->route('phpbb_ucp_reset_password_controller') )); $messenger->send($user_row['user_notify_type']); From e991df195baa75cc2bb36a34621eb8aea1f9f9e7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 15:09:35 +0200 Subject: [PATCH 08/18] [ticket/11327] Continue with implementation of password reset functionality PHPBB3-11327 --- phpBB/language/en/ucp.php | 2 + phpBB/phpbb/ucp/controller/reset_password.php | 285 +++++++++--------- 2 files changed, 150 insertions(+), 137 deletions(-) diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 66fb554f78..0a82e150ef 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -402,6 +402,7 @@ $lang = array_merge($lang, array( 'NO_OLDER_PM' => 'No older messages.', 'NO_PASSWORD_SUPPLIED' => 'You cannot login without a password.', 'NO_RECIPIENT' => 'No recipient defined.', + 'NO_RESET_TOKEN' => 'You did not provide a password reset token.', 'NO_RULES_DEFINED' => 'No rules defined.', 'NO_SAVED_DRAFTS' => 'No drafts saved.', 'NO_TO_RECIPIENT' => 'None', @@ -463,6 +464,7 @@ $lang = array_merge($lang, array( 'REPLIED_MESSAGE' => 'Replied to message', 'REPLY_TO_ALL' => 'Reply to sender and all recipients.', 'REPORT_PM' => 'Report private message', + 'RESET_TOKEN_EXPIRED_OR_INVALID' => 'The password reset token you supplied is invalid or has expired.', 'RESIGN_SELECTED' => 'Resign selected', 'RETURN_FOLDER' => '%1$sReturn to previous folder%2$s', 'RETURN_UCP' => '%sReturn to the User Control Panel%s', diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 4b2660aebb..3d34c4740b 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -22,6 +22,7 @@ use phpbb\passwords\manager; use phpbb\request\request_interface; use phpbb\template\template; use phpbb\user; +use Symfony\Component\HttpFoundation\Response; /** * ucp_remind @@ -94,18 +95,32 @@ class reset_password $this->request = $request; $this->template = $template; $this->user = $user; + $this->tables = $tables; $this->root_path = $root_path; $this->php_ext = $php_ext; } - public function request() + /** + * Init controller + */ + protected function init_controller() { $this->language->add_lang('ucp'); if (!$this->config['allow_password_reset']) { - trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + $this->helper->message($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); } + } + + /** + * Handle password reset request + * + * @return Response + */ + public function request() + { + $this->init_controller(); $submit = $this->request->is_set_post('submit'); $username = $this->request->variable('username', '', true); @@ -125,13 +140,13 @@ class reset_password trigger_error('NO_EMAIL_USER'); } - $sql_array = array( + $sql_array = [ 'SELECT' => 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type,' . ' user_lang, user_inactive_reason, reset_token, reset_token_expiration', - 'FROM' => array(USERS_TABLE => 'u'), + 'FROM' => [$this->tables['users'] => 'u'], 'WHERE' => "user_email_hash = '" . $this->db->sql_escape(phpbb_email_hash($email)) . "'" . (!empty($username) ? " AND username_clean = '" . $this->db->sql_escape(utf8_clean_string($username)) . "'" : ''), - ); + ]; /** * Change SQL query for fetching user data @@ -141,12 +156,13 @@ class reset_password * @var string username User's username from the form * @var array sql_array Fully assembled SQL query with keys SELECT, FROM, WHERE * @since 3.1.11-RC1 + * @changed 3.3.0-b1 Moved to reset password controller */ - $vars = array( + $vars = [ 'email', 'username', 'sql_array', - ); + ]; extract($this->dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); $sql = $this->db->sql_build_query('SELECT', $sql_array); @@ -157,10 +173,10 @@ class reset_password { $this->db->sql_freeresult($result); - $this->template->assign_vars(array( + $this->template->assign_vars([ 'USERNAME_REQUIRED' => true, 'EMAIL' => $email, - )); + ]); } else { @@ -194,15 +210,13 @@ class reset_password trigger_error($message); } - $server_url = generate_board_url(); - // Generate reset token - $reset_token = gen_rand_string_friendly(32); + $reset_token = strtolower(gen_rand_string(32)); - $sql_ary = array( + $sql_ary = [ 'reset_token' => $reset_token, 'reset_token_expiration' => time() + $this->config['reset_token_lifetime'], - ); + ]; $sql = 'UPDATE ' . $this->tables['users'] . ' SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' @@ -214,16 +228,19 @@ class reset_password /** @var \messenger $messenger */ $messenger = new \messenger(false); - $messenger->template('user_activate_passwd', $user_row['user_lang']); + $messenger->template('user_forgot_password', $user_row['user_lang']); $messenger->set_addresses($user_row); $messenger->anti_abuse_headers($this->config, $this->user); - $messenger->assign_vars(array( + $messenger->assign_vars([ 'USERNAME' => htmlspecialchars_decode($user_row['username']), - 'U_RESET_PASSWORD' => $this->helper->route('phpbb_ucp_reset_password_controller') - )); + 'U_RESET_PASSWORD' => generate_board_url(true) . $this->helper->route('phpbb_ucp_reset_password_controller', [ + 'u' => $user_row['user_id'], + 'token' => $reset_token, + ], false) + ]); $messenger->send($user_row['user_notify_type']); @@ -231,11 +248,11 @@ class reset_password } } - $this->template->assign_vars(array( + $this->template->assign_vars([ 'USERNAME' => $username, 'EMAIL' => $email, 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), - )); + ]); return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); } @@ -243,25 +260,73 @@ class reset_password /** * Handle controller requests * - * @return \Symfony\Component\HttpFoundation\Response + * @return Response */ public function reset() { - $this->language->add_lang('ucp'); + $this->init_controller(); - if (!$this->config['allow_password_reset']) + $submit = $this->request->is_set_post('submit'); + $reset_token = $this->request->variable('token', ''); + $user_id = $this->request->variable('u', 0); + + if (empty($reset_token)) { - trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + $this->helper->message('NO_RESET_TOKEN'); } - $submit = $this->request->is_set_post('submit'); - $username = $this->request->variable('username', '', true); - $email = strtolower($this->request->variable('email', '')); - $key = $this->request->variable('key', ''); - $user_id = $this->request->variable('user_id', 0); + if (!$user_id) + { + $this->helper->message('NO_USER'); + } add_form_key('ucp_remind'); + $sql_array = [ + 'SELECT' => 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type,' + . ' user_lang, user_inactive_reason, reset_token, reset_token_expiration', + 'FROM' => [$this->tables['users'] => 'u'], + 'WHERE' => 'user_id = ' . $user_id, + ]; + + /** + * Change SQL query for fetching user data + * + * @event core.ucp_reset_password_modify_select_sql + * @var int user_id User ID from the form + * @var string reset_token Reset token + * @var array sql_array Fully assembled SQL query with keys SELECT, FROM, WHERE + * @since 3.3.0-b1 + */ + $vars = [ + 'user_id', + 'reset_token', + 'sql_array', + ]; + extract($this->dispatcher->trigger_event('core.ucp_reset_password_modify_select_sql', compact($vars))); + + $sql = $this->db->sql_build_query('SELECT', $sql_array); + $result = $this->db->sql_query_limit($sql, 1); + $user_row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + $message = $this->language->lang('RESET_TOKEN_EXPIRED_OR_INVALID') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); + + if (empty($user_row)) + { + $this->helper->message($message); + } + + if (!hash_equals($reset_token, $user_row['reset_token'])) + { + $this->helper->message($message); + } + + if ($user_row['reset_token_expiration'] < time()) + { + $this->helper->message($message); + } + if ($submit) { if (!check_form_key('ucp_remind')) @@ -269,119 +334,65 @@ class reset_password trigger_error('FORM_INVALID'); } - if (empty($email)) + $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); + + if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) { - 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 = '" . $this->db->sql_escape(phpbb_email_hash($email)) . "'" . - (!empty($username) ? " AND username_clean = '" . $this->db->sql_escape(utf8_clean_string($username)) . "'" : ''), - ); - - /** - * Change SQL query for fetching user data - * - * @event core.ucp_remind_modify_select_sql - * @var string email User's email from the form - * @var string username User's username from the form - * @var array sql_array Fully assembled SQL query with keys SELECT, FROM, WHERE - * @since 3.1.11-RC1 - */ - $vars = array( - 'email', - 'username', - 'sql_array', - ); - extract($this->dispatcher->trigger_event('core.ucp_remind_modify_select_sql', compact($vars))); - - $sql = $this->db->sql_build_query('SELECT', $sql_array); - $result = $this->db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need - $rowset = $this->db->sql_fetchrowset($result); - - if (count($rowset) > 1) - { - $this->db->sql_freeresult($result); - - $this->template->assign_vars(array( - 'USERNAME_REQUIRED' => true, - 'EMAIL' => $email, - )); - } - else - { - $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); - - if (empty($rowset)) - { - trigger_error($message); - } - - $user_row = $rowset[0]; - $this->db->sql_freeresult($result); - - if (!$user_row) - { - trigger_error($message); - } - - if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) - { - trigger_error($message); - } - - // Check users permissions - $auth2 = new \phpbb\auth\auth(); - $auth2->acl($user_row); - - if (!$auth2->acl_get('u_chgpasswd')) - { - trigger_error($message); - } - - $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) $this->config['min_pass_chars'], (int) $this->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)); - - $sql = 'UPDATE ' . USERS_TABLE . " - SET user_newpasswd = '" . $this->db->sql_escape($this->passwords_manager->hash($user_password)) . "', user_actkey = '" . $this->db->sql_escape($user_actkey) . "' - WHERE user_id = " . $user_row['user_id']; - $this->db->sql_query($sql); - - include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); - - $messenger = new messenger(false); - - $messenger->template('user_activate_passwd', $user_row['user_lang']); - - $messenger->set_addresses($user_row); - - $messenger->anti_abuse_headers($this->config, $this->user); - - $messenger->assign_vars(array( - 'USERNAME' => htmlspecialchars_decode($user_row['username']), - 'PASSWORD' => htmlspecialchars_decode($user_password), - 'U_ACTIVATE' => "$server_url/ucp.{$this->php_ext}?mode=activate&u={$user_row['user_id']}&k=$user_actkey") - ); - - $messenger->send($user_row['user_notify_type']); - trigger_error($message); } + + // Check users permissions + $auth2 = new \phpbb\auth\auth(); + $auth2->acl($user_row); + + if (!$auth2->acl_get('u_chgpasswd')) + { + trigger_error($message); + } + + $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) $this->config['min_pass_chars'], (int) $this->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)); + + $sql = 'UPDATE ' . USERS_TABLE . " + SET user_newpasswd = '" . $this->db->sql_escape($this->passwords_manager->hash($user_password)) . "', user_actkey = '" . $this->db->sql_escape($user_actkey) . "' + WHERE user_id = " . $user_row['user_id']; + $this->db->sql_query($sql); + + include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); + + $messenger = new messenger(false); + + $messenger->template('user_activate_passwd', $user_row['user_lang']); + + $messenger->set_addresses($user_row); + + $messenger->anti_abuse_headers($this->config, $this->user); + + $messenger->assign_vars([ + 'USERNAME' => htmlspecialchars_decode($user_row['username']), + 'PASSWORD' => htmlspecialchars_decode($user_password), + 'U_ACTIVATE' => "$server_url/ucp.{$this->php_ext}?mode=activate&u={$user_row['user_id']}&k=$user_actkey" + ]); + + $messenger->send($user_row['user_notify_type']); + + trigger_error($message); } - $this->template->assign_vars(array( - 'USERNAME' => $username, - 'EMAIL' => $email, - 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), - )); + $this->template->assign_vars([ + 'S_IS_PASSWORD_RESET' => true, + 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), + 'S_HIDDEN_FIELDS' => build_hidden_fields([ + 'u' => $user_id, + 'token' => $reset_token, + ]), + ]); return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); } From eee18f3747592eb17ea1a16184c19e5cda1f500e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 15:11:43 +0200 Subject: [PATCH 09/18] [ticket/11327] Rename UCP reset password template PHPBB3-11327 --- .../template/{ucp_remind.html => ucp_reset_password.html} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename phpBB/styles/prosilver/template/{ucp_remind.html => ucp_reset_password.html} (100%) diff --git a/phpBB/styles/prosilver/template/ucp_remind.html b/phpBB/styles/prosilver/template/ucp_reset_password.html similarity index 100% rename from phpBB/styles/prosilver/template/ucp_remind.html rename to phpBB/styles/prosilver/template/ucp_reset_password.html From cefdf8bf19d764b7fef3d04383a41ed856af5503 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 17:18:39 +0200 Subject: [PATCH 10/18] [ticket/11327] Finish up initial version of password reset system PHPBB3-11327 --- .../config/default/container/services_ucp.yml | 1 + phpBB/language/en/ucp.php | 5 +- phpBB/phpbb/ucp/controller/reset_password.php | 130 +++++++++++------- .../template/ucp_reset_password.html | 38 +++-- 4 files changed, 111 insertions(+), 63 deletions(-) diff --git a/phpBB/config/default/container/services_ucp.yml b/phpBB/config/default/container/services_ucp.yml index 923f35033c..44e97cb546 100644 --- a/phpBB/config/default/container/services_ucp.yml +++ b/phpBB/config/default/container/services_ucp.yml @@ -7,6 +7,7 @@ services: - '@dispatcher' - '@controller.helper' - '@language' + - '@log' - '@passwords.manager' - '@request' - '@template' diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 0a82e150ef..b0df497625 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -416,7 +416,8 @@ $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_IF_EXISTED' => 'If your account exists, a new password was sent to your registered email address. If you do not receive an email, it may be because you are banned, your account is not activated, or you are not allowed to change your password. Contact admin if any of those reasons apply. Also, check your spam filter.', + 'PASSWORD_RESET' => 'Your password has been successfully reset.', + 'PASSWORD_RESET_LINK_SENT' => 'If your account exists, a password reset link was sent to your registered email address. If you do not receive an email, it may be because you are banned, your account is not activated, you have requested multiple password resets within a short time frame, or you are not allowed to change your password. Contact an admin if any of those reasons apply. Also, please check your spam filter.', '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.', @@ -464,6 +465,7 @@ $lang = array_merge($lang, array( 'REPLIED_MESSAGE' => 'Replied to message', 'REPLY_TO_ALL' => 'Reply to sender and all recipients.', 'REPORT_PM' => 'Report private message', + 'RESET_PASSWORD' => 'Reset password', 'RESET_TOKEN_EXPIRED_OR_INVALID' => 'The password reset token you supplied is invalid or has expired.', 'RESIGN_SELECTED' => 'Resign selected', 'RETURN_FOLDER' => '%1$sReturn to previous folder%2$s', @@ -480,7 +482,6 @@ $lang = array_merge($lang, array( 'SAME_PASSWORD_ERROR' => 'The new password you entered is the same as your current password.', 'SEARCH_YOUR_POSTS' => 'Show your posts', - 'SEND_PASSWORD' => 'Send password', 'SENT_AT' => 'Sent', // Used before dates in private messages 'SHOW_EMAIL' => 'Users can contact me by email', 'SIGNATURE_EXPLAIN' => 'This is a block of text that can be added to posts you make. There is a %d character limit.', diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 3d34c4740b..c686f198c5 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -18,6 +18,7 @@ use phpbb\controller\helper; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher; use phpbb\language\language; +use phpbb\log\log_interface; use phpbb\passwords\manager; use phpbb\request\request_interface; use phpbb\template\template; @@ -45,6 +46,9 @@ class reset_password /** @var language */ protected $language; + /** @var log_interface */ + protected $log; + /** @var manager */ protected $passwords_manager; @@ -74,6 +78,7 @@ class reset_password * @param dispatcher $dispatcher * @param helper $helper * @param language $language + * @param log_interface $log * @param manager $passwords_manager * @param request_interface $request * @param template $template @@ -83,14 +88,15 @@ class reset_password * @param $php_ext */ public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, helper $helper, - language $language, manager $passwords_manager, request_interface $request, - template $template, user $user, $tables, $root_path, $php_ext) + language $language, log_interface $log, manager $passwords_manager, + request_interface $request, template $template, user $user, $tables, $root_path, $php_ext) { $this->config = $config; $this->db = $db; $this->dispatcher = $dispatcher; $this->helper = $helper; $this->language = $language; + $this->log = $log; $this->passwords_manager = $passwords_manager; $this->request = $request; $this->template = $template; @@ -109,10 +115,28 @@ class reset_password if (!$this->config['allow_password_reset']) { - $this->helper->message($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); } } + /** + * Remove reset token for specified user + * + * @param int $user_id User ID + */ + protected function remove_reset_token(int $user_id) + { + $sql_ary = [ + 'reset_token' => '', + 'reset_token_expiration' => 0, + ]; + + $sql = 'UPDATE ' . $this->tables['users'] . ' + SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . $user_id; + $this->db->sql_query($sql); + } + /** * Handle password reset request * @@ -180,7 +204,7 @@ class reset_password } else { - $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); + $message = $this->language->lang('PASSWORD_RESET_LINK_SENT') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); $user_row = empty($rowset) ? [] : $rowset[0]; $this->db->sql_freeresult($result); @@ -254,7 +278,7 @@ class reset_password 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), ]); - return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); + return $this->helper->render('ucp_reset_password.html', $this->language->lang('UCP_REMIND')); } /** @@ -272,12 +296,12 @@ class reset_password if (empty($reset_token)) { - $this->helper->message('NO_RESET_TOKEN'); + return $this->helper->message('NO_RESET_TOKEN'); } if (!$user_id) { - $this->helper->message('NO_USER'); + return $this->helper->message('NO_USER'); } add_form_key('ucp_remind'); @@ -314,31 +338,33 @@ class reset_password if (empty($user_row)) { - $this->helper->message($message); + return $this->helper->message($message); } if (!hash_equals($reset_token, $user_row['reset_token'])) { - $this->helper->message($message); + return $this->helper->message($message); } if ($user_row['reset_token_expiration'] < time()) { - $this->helper->message($message); + $this->remove_reset_token($user_id); + + return $this->helper->message($message); } + $error = []; + if ($submit) { if (!check_form_key('ucp_remind')) { - trigger_error('FORM_INVALID'); + return $this->helper->message('FORM_INVALID'); } - $message = $this->language->lang('PASSWORD_UPDATED_IF_EXISTED') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); - if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) { - trigger_error($message); + return $this->helper->message($message); } // Check users permissions @@ -347,46 +373,54 @@ class reset_password if (!$auth2->acl_get('u_chgpasswd')) { - trigger_error($message); + return $this->helper->message($message); } - $server_url = generate_board_url(); + if (!function_exists('validate_data')) + { + include($this->root_path . 'includes/functions_user.' . $this->php_ext); + } - // 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) $this->config['min_pass_chars'], (int) $this->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)); - - $sql = 'UPDATE ' . USERS_TABLE . " - SET user_newpasswd = '" . $this->db->sql_escape($this->passwords_manager->hash($user_password)) . "', user_actkey = '" . $this->db->sql_escape($user_actkey) . "' - WHERE user_id = " . $user_row['user_id']; - $this->db->sql_query($sql); - - include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); - - $messenger = new messenger(false); - - $messenger->template('user_activate_passwd', $user_row['user_lang']); - - $messenger->set_addresses($user_row); - - $messenger->anti_abuse_headers($this->config, $this->user); - - $messenger->assign_vars([ - 'USERNAME' => htmlspecialchars_decode($user_row['username']), - 'PASSWORD' => htmlspecialchars_decode($user_password), - 'U_ACTIVATE' => "$server_url/ucp.{$this->php_ext}?mode=activate&u={$user_row['user_id']}&k=$user_actkey" - ]); - - $messenger->send($user_row['user_notify_type']); - - trigger_error($message); + $data = [ + 'new_password' => $this->request->untrimmed_variable('new_password', '', true), + 'password_confirm' => $this->request->untrimmed_variable('new_password_confirm', '', true), + ]; + $check_data = [ + 'new_password' => [ + ['string', false, $this->config['min_pass_chars'], $this->config['max_pass_chars']], + ['password'], + ], + 'password_confirm' => ['string', true, $this->config['min_pass_chars'], $this->config['max_pass_chars']], + ]; + $error = array_merge($error, validate_data($data, $check_data)); + if (strcmp($data['new_password'], $data['password_confirm']) !== 0) + { + $error[] = ($data['password_confirm']) ? 'NEW_PASSWORD_ERROR' : 'NEW_PASSWORD_CONFIRM_EMPTY'; + } + if (empty($error)) + { + $sql_ary = [ + 'user_password' => $this->passwords_manager->hash($data['new_password']), + 'user_login_attempts' => 0, + 'reset_token' => '', + 'reset_token_expiration' => 0, + ]; + $sql = 'UPDATE ' . $this->tables['users'] . ' + SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . (int) $user_row['user_id']; + $this->db->sql_query($sql); + $this->log->add('user', $user_row['user_id'], $this->user->ip, 'LOG_USER_NEW_PASSWORD', false, [ + 'reportee_id' => $user_row['user_id'], + $user_row['username'] + ]); + meta_refresh(3, append_sid("{$this->root_path}index.{$this->php_ext}")); + trigger_error($this->language->lang('PASSWORD_RESET')); + } } $this->template->assign_vars([ 'S_IS_PASSWORD_RESET' => true, + 'ERROR' => !empty($error) ? implode('
', array_map([$this->language, 'lang'], $error)) : '', 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), 'S_HIDDEN_FIELDS' => build_hidden_fields([ 'u' => $user_id, @@ -394,6 +428,6 @@ class reset_password ]), ]); - return $this->helper->render('ucp_remind.html', $this->language->lang('UCP_REMIND')); + return $this->helper->render('ucp_reset_password.html', $this->language->lang('UCP_REMIND')); } } diff --git a/phpBB/styles/prosilver/template/ucp_reset_password.html b/phpBB/styles/prosilver/template/ucp_reset_password.html index 8b700de430..3f9ffce519 100644 --- a/phpBB/styles/prosilver/template/ucp_reset_password.html +++ b/phpBB/styles/prosilver/template/ucp_reset_password.html @@ -1,32 +1,44 @@ -
+
-

{L_SEND_PASSWORD}

+

{{ lang('RESET_PASSWORD') }}

+ {% if S_IS_PASSWORD_RESET %} + {% if ERROR %}

{{ ERROR }}

{% endif %} +
+
+
+
+
+
+
+
+ {% else %} {% if USERNAME_REQUIRED %}

{{ lang('EMAIL_NOT_UNIQUE') }}

{% endif %} -
-

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

{{ lang('EMAIL_REMIND') }}
+
+
+ {% if USERNAME_REQUIRED %} +
+
+
+
+ {% endif %} {% endif %}
 
-
{S_HIDDEN_FIELDS} 
+
{{ S_HIDDEN_FIELDS }} 
- {S_FORM_TOKEN} + {{ S_FORM_TOKEN }}
From fa5a0d5e210646d0d271f5ed7433e4cc028b5cf1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 21:18:55 +0200 Subject: [PATCH 11/18] [ticket/11327] Update tests to reflect changes PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 4 +- tests/functional/user_password_reset_test.php | 93 ++++++++++++------- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index c686f198c5..679c659eb0 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -220,7 +220,7 @@ class reset_password } // Do not create multiple valid reset tokens - if (!empty($user_row['reset_token']) && (int) $user_row['reset_token_expiration'] <= (time() + $this->config['reset_token_lifetime'])) + if (!empty($user_row['reset_token']) && (int) $user_row['reset_token_expiration'] >= time()) { trigger_error($message); } @@ -239,7 +239,7 @@ class reset_password $sql_ary = [ 'reset_token' => $reset_token, - 'reset_token_expiration' => time() + $this->config['reset_token_lifetime'], + 'reset_token_expiration' => strtotime('+1 day'), ]; $sql = 'UPDATE ' . $this->tables['users'] . ' diff --git a/tests/functional/user_password_reset_test.php b/tests/functional/user_password_reset_test.php index 2361eed066..a97300b9ee 100644 --- a/tests/functional/user_password_reset_test.php +++ b/tests/functional/user_password_reset_test.php @@ -25,36 +25,53 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca // test without email $crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}"); + $this->assertContains('app.php/user/forgot_password', $crawler->getUri()); $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}"); + $crawler = self::request('GET', "app.php/user/forgot_password?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()); + $this->assertContainsLang('PASSWORD_RESET_LINK_SENT', $crawler->text()); // test with correct email - $crawler = self::request('GET', "ucp.php?mode=sendpassword&sid={$this->sid}"); + $crawler = self::request('GET', "app.php/user/forgot_password?sid={$this->sid}"); $form = $crawler->selectButton('submit')->form(array( 'email' => 'reset-password-test-user@test.com', )); $crawler = self::submit($form); - $this->assertContainsLang('PASSWORD_UPDATED_IF_EXISTED', $crawler->text()); + $this->assertContainsLang('PASSWORD_RESET_LINK_SENT', $crawler->text()); // Check if columns in database were updated for password reset $this->get_user_data('reset-password-test-user'); - $this->assertNotNull($this->user_data['user_actkey']); - $this->assertNotNull($this->user_data['user_newpasswd']); + $this->assertNotEmpty($this->user_data['reset_token']); + $this->assertNotEmpty($this->user_data['reset_token_expiration']); + $reset_token = $this->user_data['reset_token']; + $reset_token_expiration = $this->user_data['reset_token_expiration']; + + // Check that reset token is only created once per day + $crawler = self::request('GET', "app.php/user/forgot_password?sid={$this->sid}"); + $form = $crawler->selectButton('submit')->form(array( + 'email' => 'reset-password-test-user@test.com', + )); + $crawler = self::submit($form); + $this->assertContainsLang('PASSWORD_RESET_LINK_SENT', $crawler->text()); + + $this->get_user_data('reset-password-test-user'); + $this->assertNotEmpty($this->user_data['reset_token']); + $this->assertNotEmpty($this->user_data['reset_token_expiration']); + $this->assertEquals($reset_token, $this->user_data['reset_token']); + $this->assertEquals($reset_token_expiration, $this->user_data['reset_token_expiration']); // 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}"); + $crawler = self::request('GET', "app.php/user/forgot_password?sid={$this->sid}"); $form = $crawler->selectButton('submit')->form(array( 'email' => 'reset-password-test-user@test.com', )); @@ -67,20 +84,13 @@ 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_IF_EXISTED', $crawler->text()); + $this->assertContainsLang('PASSWORD_RESET_LINK_SENT', $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']); - - // Make sure we know the password - $db = $this->get_db(); - $this->passwords_manager = $this->get_passwords_manager(); - $sql = 'UPDATE ' . USERS_TABLE . " - SET user_newpasswd = '" . $db->sql_escape($this->passwords_manager->hash('reset-password-test-user')) . "' - WHERE user_id = " . $user_id; - $db->sql_query($sql); + $this->assertNotEmpty($this->user_data['reset_token']); + $this->assertNotEmpty($this->user_data['reset_token_expiration']); + $this->assertGreaterThan(time(), $this->user_data['reset_token_expiration']); } public function test_login_after_reset() @@ -88,28 +98,45 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca $this->login('reset-password-test-user'); } - public function data_activate_new_password() + public function data_reset_user_password() { - return array( - array('WRONG_ACTIVATION', false, 'FOOBAR'), - array('ALREADY_ACTIVATED', 2, 'FOOBAR'), - array('PASSWORD_ACTIVATED', false, false), - array('ALREADY_ACTIVATED', false, false), - ); + return [ + ['RESET_TOKEN_EXPIRED_OR_INVALID', 0, 'abcdef'], + ['NO_USER', ' ', 'abcdef'], + ['NO_RESET_TOKEN', 0, ' '], + ['RESET_TOKEN_EXPIRED_OR_INVALID', 2, ''], + ['RESET_TOKEN_EXPIRED_OR_INVALID', 1e7, ''], + ['', 0, ''], + ['NO_RESET_TOKEN', 0, ''], // already reset + ]; } /** - * @dataProvider data_activate_new_password - */ - public function test_activate_new_password($expected, $user_id, $act_key) + * @dataProvider data_reset_user_password + */ + public function test_reset_user_password($expected, $user_id, $token) { $this->add_lang('ucp'); $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; + $user_id = !$user_id ? $this->user_data['user_id'] : $user_id; + $token = !$token ? $this->user_data['reset_token'] : $token; - $crawler = self::request('GET', "ucp.php?mode=activate&u=$user_id&k=$act_key&sid={$this->sid}"); - $this->assertContainsLang($expected, $crawler->text()); + $crawler = self::request('GET', "app.php/user/reset_password?u=$user_id&token=$token"); + + if ($expected) + { + $this->assertContainsLang($expected, $crawler->text()); + } + else + { + $form = $crawler->filter('input[type=submit]')->form(); + $values = array_merge($form->getValues(), [ + 'new_password' => 'reset-password-test-user', + 'new_password_confirm' => 'reset-password-test-user', + ]); + $crawler = self::submit($form, $values); + $this->assertContainsLang('PASSWORD_RESET', $crawler->text()); + } } public function test_login() @@ -190,7 +217,7 @@ class phpbb_functional_user_password_reset_test extends phpbb_functional_test_ca 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 + $sql = 'SELECT user_id, username, user_type, user_email, user_newpasswd, user_lang, user_notify_type, user_actkey, user_inactive_reason, reset_token, reset_token_expiration FROM ' . USERS_TABLE . " WHERE username = '" . $db->sql_escape($username) . "'"; $result = $db->sql_query($sql); From ba92e7d2d6fa946de715e3ff6b72275374824f8d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 21:23:54 +0200 Subject: [PATCH 12/18] [ticket/11327] Clean up code a bit PHPBB3-11327 --- phpBB/language/en/ucp.php | 1 - phpBB/phpbb/ucp/controller/reset_password.php | 43 ++++++++++--------- .../template/ucp_reset_password.html | 2 +- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index b0df497625..2fb6a93754 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -565,7 +565,6 @@ $lang = array_merge($lang, array( 'UCP_PASSWORD_RESET_DISABLED' => 'The password reset functionality has been disabled. If you need help accessing your account, please contact the %sBoard Administrator%s', 'UCP_REGISTER_DISABLE' => 'Creating a new account is currently not possible.', - 'UCP_REMIND' => 'Send password', 'UCP_RESEND' => 'Send activation email', 'UCP_WELCOME' => 'Welcome to the User Control Panel. From here you can monitor, view and update your profile, preferences, subscribed forums and topics. You can also send messages to other users (if permitted). Please ensure you read any announcements before continuing.', 'UCP_ZEBRA' => 'Friends & Foes', diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 679c659eb0..50d3ce91eb 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -13,6 +13,7 @@ namespace phpbb\ucp\controller; +use phpbb\auth\auth; use phpbb\config\config; use phpbb\controller\helper; use phpbb\db\driver\driver_interface; @@ -26,8 +27,7 @@ use phpbb\user; use Symfony\Component\HttpFoundation\Response; /** -* ucp_remind -* Sending password reminders +* Handling forgotten passwords via reset password functionality */ class reset_password { @@ -71,7 +71,7 @@ class reset_password protected $php_ext; /** - * ucp_remind constructor. + * Reset password controller constructor. * * @param config $config * @param driver_interface $db @@ -150,18 +150,18 @@ class reset_password $username = $this->request->variable('username', '', true); $email = strtolower($this->request->variable('email', '')); - add_form_key('ucp_remind'); + add_form_key('ucp_reset_password'); if ($submit) { - if (!check_form_key('ucp_remind')) + if (!check_form_key('ucp_reset_password')) { - trigger_error('FORM_INVALID'); + return $this->helper->message('FORM_INVALID'); } if (empty($email)) { - trigger_error('NO_EMAIL_USER'); + return $this->helper->message('NO_EMAIL_USER'); } $sql_array = [ @@ -211,27 +211,27 @@ class reset_password if (!$user_row) { - trigger_error($message); + return $this->helper->message($message); } if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) { - trigger_error($message); + return $this->helper->message($message); } // Do not create multiple valid reset tokens if (!empty($user_row['reset_token']) && (int) $user_row['reset_token_expiration'] >= time()) { - trigger_error($message); + return $this->helper->message($message); } // Check users permissions - $auth2 = new \phpbb\auth\auth(); + $auth2 = new auth(); $auth2->acl($user_row); if (!$auth2->acl_get('u_chgpasswd')) { - trigger_error($message); + return $this->helper->message($message); } // Generate reset token @@ -247,7 +247,10 @@ class reset_password WHERE user_id = ' . $user_row['user_id']; $this->db->sql_query($sql); - include_once($this->root_path . 'includes/functions_messenger.' . $this->php_ext); + if (!class_exists('messenger')) + { + include($this->root_path . 'includes/functions_messenger.' . $this->php_ext); + } /** @var \messenger $messenger */ $messenger = new \messenger(false); @@ -268,7 +271,7 @@ class reset_password $messenger->send($user_row['user_notify_type']); - trigger_error($message); + return $this->helper->message($message); } } @@ -278,7 +281,7 @@ class reset_password 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), ]); - return $this->helper->render('ucp_reset_password.html', $this->language->lang('UCP_REMIND')); + return $this->helper->render('ucp_reset_password.html', $this->language->lang('RESET_PASSWORD')); } /** @@ -304,7 +307,7 @@ class reset_password return $this->helper->message('NO_USER'); } - add_form_key('ucp_remind'); + add_form_key('ucp_reset_password'); $sql_array = [ 'SELECT' => 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type,' @@ -357,7 +360,7 @@ class reset_password if ($submit) { - if (!check_form_key('ucp_remind')) + if (!check_form_key('ucp_reset_password')) { return $this->helper->message('FORM_INVALID'); } @@ -368,7 +371,7 @@ class reset_password } // Check users permissions - $auth2 = new \phpbb\auth\auth(); + $auth2 = new auth(); $auth2->acl($user_row); if (!$auth2->acl_get('u_chgpasswd')) @@ -414,7 +417,7 @@ class reset_password $user_row['username'] ]); meta_refresh(3, append_sid("{$this->root_path}index.{$this->php_ext}")); - trigger_error($this->language->lang('PASSWORD_RESET')); + return $this->helper->message($this->language->lang('PASSWORD_RESET')); } } @@ -428,6 +431,6 @@ class reset_password ]), ]); - return $this->helper->render('ucp_reset_password.html', $this->language->lang('UCP_REMIND')); + return $this->helper->render('ucp_reset_password.html', $this->language->lang('RESET_PASSWORD')); } } diff --git a/phpBB/styles/prosilver/template/ucp_reset_password.html b/phpBB/styles/prosilver/template/ucp_reset_password.html index 3f9ffce519..8003a5646a 100644 --- a/phpBB/styles/prosilver/template/ucp_reset_password.html +++ b/phpBB/styles/prosilver/template/ucp_reset_password.html @@ -1,6 +1,6 @@ - +
From 9e772d1421e059d8898f2b095e341107a1502914 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 10 Aug 2019 22:21:14 +0200 Subject: [PATCH 13/18] [ticket/11327] Adjust tests for new reset password changes PHPBB3-11327 --- tests/auth/provider_apache_test.php | 2 ++ tests/functional/forgot_password_test.php | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/auth/provider_apache_test.php b/tests/auth/provider_apache_test.php index 0c26a0a186..58d6354228 100644 --- a/tests/auth/provider_apache_test.php +++ b/tests/auth/provider_apache_test.php @@ -202,6 +202,8 @@ class phpbb_auth_provider_apache_test extends phpbb_database_test_case 'user_new' => '1', 'user_reminded' => '0', 'user_reminded_time' => '0', + 'reset_token' => '', + 'reset_token_expiration' => '0', ); $this->assertEquals($expected, $this->provider->autologin()); diff --git a/tests/functional/forgot_password_test.php b/tests/functional/forgot_password_test.php index 2fd5b45f7d..10946fe5a9 100644 --- a/tests/functional/forgot_password_test.php +++ b/tests/functional/forgot_password_test.php @@ -20,8 +20,8 @@ class phpbb_functional_forgot_password_test extends phpbb_functional_test_case { global $config; $this->add_lang('ucp'); - $crawler = self::request('GET', 'ucp.php?mode=sendpassword'); - $this->assertEquals($this->lang('SEND_PASSWORD'), $crawler->filter('h2')->text()); + $crawler = self::request('GET', 'app.php/user/forgot_password'); + $this->assertEquals($this->lang('RESET_PASSWORD'), $crawler->filter('h2')->text()); } public function test_forgot_password_disabled() @@ -40,7 +40,7 @@ class phpbb_functional_forgot_password_test extends phpbb_functional_test_case $this->logout(); - $crawler = self::request('GET', 'ucp.php?mode=sendpassword'); + $crawler = self::request('GET', 'app.php/user/forgot_password'); $this->assertContains($this->lang('UCP_PASSWORD_RESET_DISABLED', '', ''), $crawler->text()); } From 7a3e351178fde9d8b785867868fb40cbae4c8ab2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 22 Aug 2019 14:05:58 +0200 Subject: [PATCH 14/18] [ticket/11327] Clean up code style a bit PHPBB3-11327 --- .../db/migration/data/v330/reset_password.php | 4 ++-- phpBB/phpbb/ucp/controller/reset_password.php | 23 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v330/reset_password.php b/phpBB/phpbb/db/migration/data/v330/reset_password.php index 87131c6e93..953d478ccc 100644 --- a/phpBB/phpbb/db/migration/data/v330/reset_password.php +++ b/phpBB/phpbb/db/migration/data/v330/reset_password.php @@ -17,9 +17,9 @@ class reset_password extends \phpbb\db\migration\migration { static public function depends_on() { - return array( + return [ '\phpbb\db\migration\data\v330\dev', - ); + ]; } public function update_schema() diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 50d3ce91eb..82b9083175 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -18,6 +18,7 @@ use phpbb\config\config; use phpbb\controller\helper; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher; +use phpbb\exception\http_exception; use phpbb\language\language; use phpbb\log\log_interface; use phpbb\passwords\manager; @@ -156,7 +157,7 @@ class reset_password { if (!check_form_key('ucp_reset_password')) { - return $this->helper->message('FORM_INVALID'); + throw new http_exception(Response::HTTP_UNAUTHORIZED, 'FORM_INVALID'); } if (empty($email)) @@ -192,11 +193,10 @@ class reset_password $sql = $this->db->sql_build_query('SELECT', $sql_array); $result = $this->db->sql_query_limit($sql, 2); // don't waste resources on more rows than we need $rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); if (count($rowset) > 1) { - $this->db->sql_freeresult($result); - $this->template->assign_vars([ 'USERNAME_REQUIRED' => true, 'EMAIL' => $email, @@ -206,14 +206,13 @@ class reset_password { $message = $this->language->lang('PASSWORD_RESET_LINK_SENT') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); - $user_row = empty($rowset) ? [] : $rowset[0]; - $this->db->sql_freeresult($result); - - if (!$user_row) + if ($rowset === false) { return $this->helper->message($message); } + $user_row = $rowset[0]; + if ($user_row['user_type'] == USER_IGNORE || $user_row['user_type'] == USER_INACTIVE) { return $this->helper->message($message); @@ -356,7 +355,7 @@ class reset_password return $this->helper->message($message); } - $error = []; + $errors = []; if ($submit) { @@ -395,12 +394,12 @@ class reset_password ], 'password_confirm' => ['string', true, $this->config['min_pass_chars'], $this->config['max_pass_chars']], ]; - $error = array_merge($error, validate_data($data, $check_data)); + $errors = array_merge($errors, validate_data($data, $check_data)); if (strcmp($data['new_password'], $data['password_confirm']) !== 0) { - $error[] = ($data['password_confirm']) ? 'NEW_PASSWORD_ERROR' : 'NEW_PASSWORD_CONFIRM_EMPTY'; + $errors[] = $data['password_confirm'] ? 'NEW_PASSWORD_ERROR' : 'NEW_PASSWORD_CONFIRM_EMPTY'; } - if (empty($error)) + if (empty($errors)) { $sql_ary = [ 'user_password' => $this->passwords_manager->hash($data['new_password']), @@ -423,7 +422,7 @@ class reset_password $this->template->assign_vars([ 'S_IS_PASSWORD_RESET' => true, - 'ERROR' => !empty($error) ? implode('
', array_map([$this->language, 'lang'], $error)) : '', + 'ERROR' => !empty($errors) ? implode('
', array_map([$this->language, 'lang'], $errors)) : '', 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), 'S_HIDDEN_FIELDS' => build_hidden_fields([ 'u' => $user_id, From 454ea081f17c0dfb9eb75287698a301e5c5d275b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 26 Aug 2019 18:14:39 +0200 Subject: [PATCH 15/18] [ticket/11327] Use U_ prefix for reset password URL template variable PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 14 +++++++------- .../prosilver/template/ucp_reset_password.html | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 82b9083175..9d736b26b8 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -275,9 +275,9 @@ class reset_password } $this->template->assign_vars([ - 'USERNAME' => $username, - 'EMAIL' => $email, - 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), + 'USERNAME' => $username, + 'EMAIL' => $email, + 'U_RESET_PASSWORD_ACTION' => $this->helper->route('phpbb_ucp_forgot_password_controller'), ]); return $this->helper->render('ucp_reset_password.html', $this->language->lang('RESET_PASSWORD')); @@ -421,10 +421,10 @@ class reset_password } $this->template->assign_vars([ - 'S_IS_PASSWORD_RESET' => true, - 'ERROR' => !empty($errors) ? implode('
', array_map([$this->language, 'lang'], $errors)) : '', - 'S_PROFILE_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), - 'S_HIDDEN_FIELDS' => build_hidden_fields([ + 'S_IS_PASSWORD_RESET' => true, + 'ERROR' => !empty($errors) ? implode('
', array_map([$this->language, 'lang'], $errors)) : '', + 'U_RESET_PASSWORD_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), + 'S_HIDDEN_FIELDS' => build_hidden_fields([ 'u' => $user_id, 'token' => $reset_token, ]), diff --git a/phpBB/styles/prosilver/template/ucp_reset_password.html b/phpBB/styles/prosilver/template/ucp_reset_password.html index 8003a5646a..fffaa1b59e 100644 --- a/phpBB/styles/prosilver/template/ucp_reset_password.html +++ b/phpBB/styles/prosilver/template/ucp_reset_password.html @@ -1,6 +1,6 @@ - +
From 8048d817ca0198b214457066a549db6f92b85bc0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 30 Aug 2019 09:44:10 +0200 Subject: [PATCH 16/18] [ticket/11327] Move html output to reset password html file PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 8 ++++++-- phpBB/styles/prosilver/template/ucp_reset_password.html | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index 9d736b26b8..fb66ea1b99 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -206,7 +206,7 @@ class reset_password { $message = $this->language->lang('PASSWORD_RESET_LINK_SENT') . '

' . $this->language->lang('RETURN_INDEX', 'root_path}index.{$this->php_ext}") . '">', ''); - if ($rowset === false) + if (empty($rowset)) { return $this->helper->message($message); } @@ -420,9 +420,13 @@ class reset_password } } + if (!empty($errors)) + { + $this->template->assign_block_vars_array('PASSWORD_RESET_ERRORS', array_map([$this->language, 'lang'], $errors)); + } + $this->template->assign_vars([ 'S_IS_PASSWORD_RESET' => true, - 'ERROR' => !empty($errors) ? implode('
', array_map([$this->language, 'lang'], $errors)) : '', 'U_RESET_PASSWORD_ACTION' => $this->helper->route('phpbb_ucp_reset_password_controller'), 'S_HIDDEN_FIELDS' => build_hidden_fields([ 'u' => $user_id, diff --git a/phpBB/styles/prosilver/template/ucp_reset_password.html b/phpBB/styles/prosilver/template/ucp_reset_password.html index fffaa1b59e..0a05f69aed 100644 --- a/phpBB/styles/prosilver/template/ucp_reset_password.html +++ b/phpBB/styles/prosilver/template/ucp_reset_password.html @@ -10,7 +10,7 @@
{% if S_IS_PASSWORD_RESET %} - {% if ERROR %}

{{ ERROR }}

{% endif %} + {% if PASSWORD_RESET_ERRORS %}

{{ PASSWORD_RESET_ERRORS | join('
') }}

{% endif %}
From f920336be4dcb3ae2df43c10dc8ef9ff1346ceb8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 1 Sep 2019 11:36:57 +0200 Subject: [PATCH 17/18] [ticket/11327] Use http_exception instead of trigger_error PHPBB3-11327 --- phpBB/phpbb/ucp/controller/reset_password.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index fb66ea1b99..d7b96c51c4 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -116,7 +116,10 @@ class reset_password if (!$this->config['allow_password_reset']) { - trigger_error($this->language->lang('UCP_PASSWORD_RESET_DISABLED', '', '')); + throw new http_exception(Response::HTTP_OK, 'UCP_PASSWORD_RESET_DISABLED', [ + '', + '' + ]); } } From 3a443b56233c58df49d15861c1c4add996b7660b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Oct 2019 16:56:17 +0200 Subject: [PATCH 18/18] [ticket/11327] Adjust code per review comments PHPBB3-11327 --- .../config/default/container/services_ucp.yml | 2 +- phpBB/language/en/ucp.php | 2 +- phpBB/phpbb/ucp/controller/reset_password.php | 35 ++++++++++--------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/phpBB/config/default/container/services_ucp.yml b/phpBB/config/default/container/services_ucp.yml index 44e97cb546..861fa4ac75 100644 --- a/phpBB/config/default/container/services_ucp.yml +++ b/phpBB/config/default/container/services_ucp.yml @@ -12,6 +12,6 @@ services: - '@request' - '@template' - '@user' - - '%tables%' + - '%tables.users%' - '%core.root_path%' - '%core.php_ext%' diff --git a/phpBB/language/en/ucp.php b/phpBB/language/en/ucp.php index 2fb6a93754..542d814911 100644 --- a/phpBB/language/en/ucp.php +++ b/phpBB/language/en/ucp.php @@ -417,7 +417,7 @@ $lang = array_merge($lang, array( 'PASSWORD' => 'Password', 'PASSWORD_ACTIVATED' => 'Your new password has been activated.', 'PASSWORD_RESET' => 'Your password has been successfully reset.', - 'PASSWORD_RESET_LINK_SENT' => 'If your account exists, a password reset link was sent to your registered email address. If you do not receive an email, it may be because you are banned, your account is not activated, you have requested multiple password resets within a short time frame, or you are not allowed to change your password. Contact an admin if any of those reasons apply. Also, please check your spam filter.', + 'PASSWORD_RESET_LINK_SENT' => 'If your account exists, a password reset link was sent to your registered email address. If you do not receive an email, it may be because you are banned, your account is not activated, you have requested multiple password resets within a short time frame, or you are not allowed to change your password. Contact an administrator if any of those reasons apply. Also, please check your spam filter.', '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/phpBB/phpbb/ucp/controller/reset_password.php b/phpBB/phpbb/ucp/controller/reset_password.php index d7b96c51c4..666957b0dc 100644 --- a/phpBB/phpbb/ucp/controller/reset_password.php +++ b/phpBB/phpbb/ucp/controller/reset_password.php @@ -63,7 +63,7 @@ class reset_password protected $user; /** @var array phpBB DB table names */ - protected $tables; + protected $users_table; /** @var string phpBB root path */ protected $root_path; @@ -84,13 +84,14 @@ class reset_password * @param request_interface $request * @param template $template * @param user $user - * @param array $tables - * @param $root_path - * @param $php_ext + * @param string $users_table + * @param string $root_path + * @param string $php_ext */ public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, helper $helper, language $language, log_interface $log, manager $passwords_manager, - request_interface $request, template $template, user $user, $tables, $root_path, $php_ext) + request_interface $request, template $template, user $user, string $users_table, + string $root_path, string $php_ext) { $this->config = $config; $this->db = $db; @@ -102,7 +103,7 @@ class reset_password $this->request = $request; $this->template = $template; $this->user = $user; - $this->tables = $tables; + $this->users_table = $users_table; $this->root_path = $root_path; $this->php_ext = $php_ext; } @@ -135,7 +136,7 @@ class reset_password 'reset_token_expiration' => 0, ]; - $sql = 'UPDATE ' . $this->tables['users'] . ' + $sql = 'UPDATE ' . $this->users_table . ' SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' WHERE user_id = ' . $user_id; $this->db->sql_query($sql); @@ -171,7 +172,7 @@ class reset_password $sql_array = [ 'SELECT' => 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type,' . ' user_lang, user_inactive_reason, reset_token, reset_token_expiration', - 'FROM' => [$this->tables['users'] => 'u'], + 'FROM' => [$this->users_table => 'u'], 'WHERE' => "user_email_hash = '" . $this->db->sql_escape(phpbb_email_hash($email)) . "'" . (!empty($username) ? " AND username_clean = '" . $this->db->sql_escape(utf8_clean_string($username)) . "'" : ''), ]; @@ -228,10 +229,10 @@ class reset_password } // Check users permissions - $auth2 = new auth(); - $auth2->acl($user_row); + $auth = new auth(); + $auth->acl($user_row); - if (!$auth2->acl_get('u_chgpasswd')) + if (!$auth->acl_get('u_chgpasswd')) { return $this->helper->message($message); } @@ -244,7 +245,7 @@ class reset_password 'reset_token_expiration' => strtotime('+1 day'), ]; - $sql = 'UPDATE ' . $this->tables['users'] . ' + $sql = 'UPDATE ' . $this->users_table . ' SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' WHERE user_id = ' . $user_row['user_id']; $this->db->sql_query($sql); @@ -314,7 +315,7 @@ class reset_password $sql_array = [ 'SELECT' => 'user_id, username, user_permissions, user_email, user_jabber, user_notify_type, user_type,' . ' user_lang, user_inactive_reason, reset_token, reset_token_expiration', - 'FROM' => [$this->tables['users'] => 'u'], + 'FROM' => [$this->users_table => 'u'], 'WHERE' => 'user_id = ' . $user_id, ]; @@ -373,10 +374,10 @@ class reset_password } // Check users permissions - $auth2 = new auth(); - $auth2->acl($user_row); + $auth = new auth(); + $auth->acl($user_row); - if (!$auth2->acl_get('u_chgpasswd')) + if (!$auth->acl_get('u_chgpasswd')) { return $this->helper->message($message); } @@ -410,7 +411,7 @@ class reset_password 'reset_token' => '', 'reset_token_expiration' => 0, ]; - $sql = 'UPDATE ' . $this->tables['users'] . ' + $sql = 'UPDATE ' . $this->users_table . ' SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' WHERE user_id = ' . (int) $user_row['user_id']; $this->db->sql_query($sql);