From feb154d8b2be1c8b5e936ec84475fd8a84514dcb Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Fri, 22 Feb 2019 10:39:43 +0800 Subject: [PATCH 1/3] MDL-29320 user: Remove @javascript tag for add new user behat test * It's not necessary and will improve Behat execution time --- user/tests/behat/addnewuser.feature | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/user/tests/behat/addnewuser.feature b/user/tests/behat/addnewuser.feature index 1e3b04900d3..1176384d3e8 100644 --- a/user/tests/behat/addnewuser.feature +++ b/user/tests/behat/addnewuser.feature @@ -1,16 +1,14 @@ @core @core_user -Feature: Create manually an user. +Feature: Manually create a user In order create a user properly As an admin I need to be able to add new users and edit their fields. - @javascript Scenario: Change default language for a new user Given I log in as "admin" When I navigate to "Users > Accounts > Add a new user" in site administration Then I should see "Preferred language" - @javascript Scenario: Language not displayed when editing an existing user Given the following "users" exist: | username | firstname | lastname | email | From 5a158f82bb2348be6c596ef95bba053ba82bdf0b Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Fri, 22 Feb 2019 10:43:27 +0800 Subject: [PATCH 2/3] MDL-29320 admin: Improve test for multiple accounts with same email * Use Scenario Outlines * More coverage: - Test for email case-sensitivity - Updating user profile as admin - Updating own user profile --- ...e_multiple_accounts_use_same_email.feature | 117 ++++++++++-------- 1 file changed, 67 insertions(+), 50 deletions(-) diff --git a/admin/tests/behat/enable_multiple_accounts_use_same_email.feature b/admin/tests/behat/enable_multiple_accounts_use_same_email.feature index 65dffa37bad..29eeb4ba942 100644 --- a/admin/tests/behat/enable_multiple_accounts_use_same_email.feature +++ b/admin/tests/behat/enable_multiple_accounts_use_same_email.feature @@ -1,57 +1,74 @@ -@core @core_admin -Feature: Enable multiple accounts to have the same email address - In order to have multiple accounts registerd on the system with the same email address +@core @core_admin @core_user +Feature: Allowing multiple accounts to have the same email address + In order to manage user accounts As an admin - I need to enable multiple accounts to be registered with the same email address and verify it is applied + I need to be able to set whether to allow multiple accounts with the same email or not - Background: - Given I log in as "admin" - - Scenario: Enable registration of multiple accounts with the same email address + Scenario Outline: Create a user with the same email as an existing user Given the following config values are set as admin: - | allowaccountssameemail | 1 | - When I navigate to "Users > Accounts > Add a new user" in site administration + | allowaccountssameemail | | + And the following "users" exist: + | username | firstname | lastname | email | + | s1 | John | Doe | s1@example.com | + When I log in as "admin" + And I navigate to "Users > Accounts > Add a new user" in site administration And I set the following fields to these values: - | Username | testmultiemailuser1 | - | Choose an authentication method | Manual accounts | - | New password | test@User1 | - | First name | Test | - | Surname | Multi1 | - | Email address | testmultiemailuser@example.com | + | Username | s2 | + | First name | Jane | + | Surname | Doe | + | Email address | | + | New password | test | And I press "Create user" - And I should see "Test Multi1" - And I press "Add a new user" - And I set the following fields to these values: - | Username | testmultiemailuser2 | - | Choose an authentication method | Manual accounts | - | New password | test@User2 | - | First name | Test | - | Surname | Multi2 | - | Email address | testmultiemailuser@example.com | - And I press "Create user" - Then I should see "Test Multi2" - And I should not see "This email address is already registered" + Then I should "This email address is already registered." - Scenario: Disable registration of multiple accounts with the same email address + Examples: + | allowsameemail | email | expect | + | 0 | s1@example.com | see | + | 0 | S1@EXAMPLE.COM | see | + | 1 | s1@example.com | not see | + | 1 | S1@EXAMPLE.COM | not see | + + Scenario Outline: Update a user with the same email as an existing user Given the following config values are set as admin: - | allowaccountssameemail | 0 | - When I navigate to "Users > Accounts > Add a new user" in site administration - And I set the following fields to these values: - | Username | testmultiemailuser1 | - | Choose an authentication method | Manual accounts | - | New password | test@User1 | - | First name | Test | - | Surname | Multi1 | - | Email address | testmultiemailuser@example.com | - And I press "Create user" - And I should see "Test Multi1" - And I press "Add a new user" - And I set the following fields to these values: - | Username | testmultiemailuser2 | - | Choose an authentication method | Manual accounts | - | New password | test@User2 | - | First name | Test | - | Surname | Multi2 | - | Email address | testmultiemailuser@example.com | - And I press "Create user" - Then I should see "This email address is already registered" \ No newline at end of file + | allowaccountssameemail | | + And the following "users" exist: + | username | firstname | lastname | email | + | s1 | John | Doe | s1@example.com | + | s2 | Jane | Doe | s2@example.com | + When I log in as "admin" + And I navigate to "Users > Accounts > Browse list of users" in site administration + And I click on "Edit" "link" in the "Jane Doe" "table_row" + And I set the field "Email address" to "" + And I press "Update profile" + Then I should "This email address is already registered." + + Examples: + | allowsameemail | email | expect | + | 0 | s1@example.com | see | + | 0 | S1@EXAMPLE.COM | see | + | 1 | s1@example.com | not see | + | 1 | S1@EXAMPLE.COM | not see | + | 0 | S2@EXAMPLE.COM | not see | + | 1 | S2@EXAMPLE.COM | not see | + + Scenario Outline: Update own user profile with the same email as an existing user + Given the following config values are set as admin: + | allowaccountssameemail | | + And the following "users" exist: + | username | firstname | lastname | email | + | s1 | John | Doe | s1@example.com | + | s2 | Jane | Doe | s2@example.com | + When I log in as "s2" + And I open my profile in edit mode + And I set the field "Email address" to "" + And I press "Update profile" + Then I should "This email address is already registered." + + Examples: + | allowsameemail | email | expect | + | 0 | s1@example.com | see | + | 0 | S1@EXAMPLE.COM | see | + | 1 | s1@example.com | not see | + | 1 | S1@EXAMPLE.COM | not see | + | 0 | S2@EXAMPLE.COM | not see | + | 1 | S2@EXAMPLE.COM | not see | From 1b3a6e7762007c663456c83d2af9bfac07be6ac5 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 21 Feb 2019 16:30:54 +0800 Subject: [PATCH 3/3] MDL-29320 user: Make email query case-insensitive --- user/edit_form.php | 16 ++++++++++++---- user/editadvanced_form.php | 15 ++++++++++++--- user/emailupdate.php | 38 ++++++++++++++++++++++++-------------- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/user/edit_form.php b/user/edit_form.php index 4f2214f4c44..595c733389e 100644 --- a/user/edit_form.php +++ b/user/edit_form.php @@ -210,10 +210,18 @@ class user_edit_form extends moodleform { // Mail not confirmed yet. } else if (!validate_email($usernew->email)) { $errors['email'] = get_string('invalidemail'); - } else if (($usernew->email !== $user->email) - and empty($CFG->allowaccountssameemail) - and $DB->record_exists('user', array('email' => $usernew->email, 'mnethostid' => $CFG->mnet_localhost_id))) { - $errors['email'] = get_string('emailexists'); + } else if (($usernew->email !== $user->email) && empty($CFG->allowaccountssameemail)) { + // Make a case-insensitive query for the given email address. + $select = $DB->sql_equal('email', ':email', false) . ' AND mnethostid = :mnethostid AND id <> :userid'; + $params = array( + 'email' => $usernew->email, + 'mnethostid' => $CFG->mnet_localhost_id, + 'userid' => $usernew->id + ); + // If there are other user(s) that already have the same email, show an error. + if ($DB->record_exists_select('user', $select, $params)) { + $errors['email'] = get_string('emailexists'); + } } if (isset($usernew->email) and $usernew->email === $user->email and over_bounce_threshold($user)) { diff --git a/user/editadvanced_form.php b/user/editadvanced_form.php index 31dd330cd85..6506249ae57 100644 --- a/user/editadvanced_form.php +++ b/user/editadvanced_form.php @@ -298,9 +298,18 @@ class user_editadvanced_form extends moodleform { if (!$user or (isset($usernew->email) && $user->email !== $usernew->email)) { if (!validate_email($usernew->email)) { $err['email'] = get_string('invalidemail'); - } else if (empty($CFG->allowaccountssameemail) - and $DB->record_exists('user', array('email' => $usernew->email, 'mnethostid' => $CFG->mnet_localhost_id))) { - $err['email'] = get_string('emailexists'); + } else if (empty($CFG->allowaccountssameemail)) { + // Make a case-insensitive query for the given email address. + $select = $DB->sql_equal('email', ':email', false) . ' AND mnethostid = :mnethostid AND id <> :userid'; + $params = array( + 'email' => $usernew->email, + 'mnethostid' => $CFG->mnet_localhost_id, + 'userid' => $usernew->id + ); + // If there are other user(s) that already have the same email, show an error. + if ($DB->record_exists_select('user', $select, $params)) { + $err['email'] = get_string('emailexists'); + } } } diff --git a/user/emailupdate.php b/user/emailupdate.php index bfc3945c20a..4080eada0ff 100644 --- a/user/emailupdate.php +++ b/user/emailupdate.php @@ -60,22 +60,32 @@ if (empty($preferences['newemailattemptsleft'])) { $user->email = $preferences['newemail']; // Detect duplicate before saving. - if ($DB->get_record('user', array('email' => $user->email))) { - redirect(new moodle_url('/user/view.php', ['id' => $user->id]), get_string('emailnowexists', 'auth')); - } else { - // Update user email. - $authplugin = get_auth_plugin($user->auth); - $authplugin->user_update($olduser, $user); - user_update_user($user, false); - $a->email = $user->email; - redirect( - new moodle_url('/user/view.php', ['id' => $user->id]), - get_string('emailupdatesuccess', 'auth', $a), - null, - \core\output\notification::NOTIFY_SUCCESS - ); + if (empty($CFG->allowaccountssameemail)) { + // Make a case-insensitive query for the given email address. + $select = $DB->sql_equal('email', ':email', false) . ' AND mnethostid = :mnethostid AND id <> :userid'; + $params = array( + 'email' => $user->email, + 'mnethostid' => $CFG->mnet_localhost_id, + 'userid' => $user->id + ); + // If there are other user(s) that already have the same email, cancel and redirect. + if ($DB->record_exists_select('user', $select, $params)) { + redirect(new moodle_url('/user/view.php', ['id' => $user->id]), get_string('emailnowexists', 'auth')); + } } + // Update user email. + $authplugin = get_auth_plugin($user->auth); + $authplugin->user_update($olduser, $user); + user_update_user($user, false); + $a->email = $user->email; + redirect( + new moodle_url('/user/view.php', ['id' => $user->id]), + get_string('emailupdatesuccess', 'auth', $a), + null, + \core\output\notification::NOTIFY_SUCCESS + ); + } else { $preferences['newemailattemptsleft']--; set_user_preference('newemailattemptsleft', $preferences['newemailattemptsleft'], $user->id);