From 911725a5812cff5c1a0daa37b99767b5144e8a11 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 5 May 2014 16:59:55 +0200 Subject: [PATCH] [ticket/10073] Split email validation from email ban and taken checks PHPBB3-10073 --- phpBB/includes/acp/acp_users.php | 2 +- phpBB/includes/functions_user.php | 45 ++++++++--- phpBB/includes/ucp/ucp_profile.php | 2 +- phpBB/includes/ucp/ucp_register.php | 2 +- phpBB/phpbb/avatar/driver/gravatar.php | 3 +- ..._test.php => validate_user_email_test.php} | 76 +++++++++---------- 6 files changed, 75 insertions(+), 55 deletions(-) rename tests/functions/{validate_email_test.php => validate_user_email_test.php} (58%) diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index b653ddd13b..83ab88d48c 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -814,7 +814,7 @@ class acp_users $check_ary += array( 'email' => array( array('string', false, 6, 60), - array('email', $user_row['user_email']) + array('user_email', $user_row['user_email']), ), ); } diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 3dcb32350e..fafe29f957 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -1746,24 +1746,20 @@ function validate_password($password) } /** -* Check to see if email address is banned or already present in the DB +* Check to see if email address is a valid address and contains a MX record * * @param string $email The email to check -* @param string $allowed_email An allowed email, default being $user->data['user_email'] * * @return mixed Either false if validation succeeded or a string which will be used as the error message (with the variable name appended) */ -function validate_email($email, $allowed_email = false) +function phpbb_validate_email($email, $config = null) { - global $config, $db, $user; + if ($config === null) + { + global $config; + } $email = strtolower($email); - $allowed_email = ($allowed_email === false) ? strtolower($user->data['user_email']) : strtolower($allowed_email); - - if ($allowed_email == $email) - { - return false; - } if (!preg_match('/^' . get_preg_expression('email') . '$/i', $email)) { @@ -1782,6 +1778,35 @@ function validate_email($email, $allowed_email = false) } } + return false; +} + +/** +* Check to see if email address is banned or already present in the DB +* +* @param string $email The email to check +* @param string $allowed_email An allowed email, default being $user->data['user_email'] +* +* @return mixed Either false if validation succeeded or a string which will be used as the error message (with the variable name appended) +*/ +function validate_user_email($email, $allowed_email = false) +{ + global $config, $db, $user; + + $email = strtolower($email); + $allowed_email = ($allowed_email === false) ? strtolower($user->data['user_email']) : strtolower($allowed_email); + + if ($allowed_email == $email) + { + return false; + } + + $validate_email = phpbb_validate_email($email, $config); + if ($validate_email) + { + return $validate_email; + } + if (($ban_reason = $user->check_ban(false, false, $email, true)) !== false) { return ($ban_reason === true) ? 'EMAIL_BANNED' : $ban_reason; diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index c7ed12f4ee..5ba5f1e830 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -66,7 +66,7 @@ class ucp_profile 'password_confirm' => array('string', true, $config['min_pass_chars'], $config['max_pass_chars']), 'email' => array( array('string', false, 6, 60), - array('email')), + array('user_email')), ); if ($auth->acl_get('u_chgname') && $config['allow_namechange']) diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index cdd5532429..97934fc32d 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -211,7 +211,7 @@ class ucp_register 'password_confirm' => array('string', false, $config['min_pass_chars'], $config['max_pass_chars']), 'email' => array( array('string', false, 6, 60), - array('email')), + array('user_email')), 'tz' => array('timezone'), 'lang' => array('language_iso_name'), )); diff --git a/phpBB/phpbb/avatar/driver/gravatar.php b/phpBB/phpbb/avatar/driver/gravatar.php index 34b894c2a7..c4344ee6e8 100644 --- a/phpBB/phpbb/avatar/driver/gravatar.php +++ b/phpBB/phpbb/avatar/driver/gravatar.php @@ -81,7 +81,8 @@ class gravatar extends \phpbb\avatar\driver\driver array( 'email' => array( array('string', false, 6, 60), - array('email')) + array('email'), + ), ) ); diff --git a/tests/functions/validate_email_test.php b/tests/functions/validate_user_email_test.php similarity index 58% rename from tests/functions/validate_email_test.php rename to tests/functions/validate_user_email_test.php index dbd4b05520..951d5794e6 100644 --- a/tests/functions/validate_email_test.php +++ b/tests/functions/validate_user_email_test.php @@ -16,7 +16,7 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions_user.php'; require_once dirname(__FILE__) . '/../mock/user.php'; require_once dirname(__FILE__) . '/validate_data_helper.php'; -class phpbb_functions_validate_email_test extends phpbb_database_test_case +class phpbb_functions_validate_user_email_test extends phpbb_database_test_case { protected $db; protected $user; @@ -51,61 +51,55 @@ class phpbb_functions_validate_email_test extends phpbb_database_test_case $user->optionset('banned_users', array('banned@example.com')); } - public function test_validate_email() + public static function validate_user_email_data() + { + return array( + array('empty', array(), ''), + array('allowed', array(), 'foobar@example.com'), + array('valid_complex', array(), "'%$~test@example.com"), + array('invalid', array('EMAIL_INVALID'), 'fööbar@example.com'), + array('taken', array('EMAIL_TAKEN'), 'admin@example.com'), + array('banned', array('EMAIL_BANNED'), 'banned@example.com'), + ); + } + + /** + * @dataProvider validate_user_email_data + */ + public function test_validate_user_email($case, $errors, $email) { $this->set_validation_prerequisites(false); $this->helper->assert_valid_data(array( - 'empty' => array( - array(), - '', - array('email'), - ), - 'allowed' => array( - array(), - 'foobar@example.com', - array('email', 'foobar@example.com'), - ), - 'invalid' => array( - array('EMAIL_INVALID'), - 'fööbar@example.com', - array('email'), - ), - 'valid_complex' => array( - array(), - "'%$~test@example.com", - array('email'), - ), - 'taken' => array( - array('EMAIL_TAKEN'), - 'admin@example.com', - array('email'), - ), - 'banned' => array( - array('EMAIL_BANNED'), - 'banned@example.com', - array('email'), + $case => array( + $errors, + $email, + array('user_email'), ), )); } + public static function validate_user_email_mx_data() + { + return array( + array('valid', array(), 'foobar@phpbb.com'), + array('no_mx', array('DOMAIN_NO_MX_RECORD'), 'test@does-not-exist.phpbb.com'), + ); + } + /** + * @dataProvider validate_user_email_mx_data * @group slow */ - public function test_validate_email_mx() + public function test_validate_user_email_mx($case, $errors, $email) { $this->set_validation_prerequisites(true); $this->helper->assert_valid_data(array( - 'valid' => array( - array(), - 'foobar@phpbb.com', - array('email'), - ), - 'no_mx' => array( - array('DOMAIN_NO_MX_RECORD'), - 'test@does-not-exist.phpbb.com', - array('email'), + $case => array( + $errors, + $email, + array('user_email'), ), )); }