From 224796f0fa50d7889db1340770b35ecc31d4b988 Mon Sep 17 00:00:00 2001 From: David Balch Date: Fri, 15 Dec 2017 13:28:32 +0000 Subject: [PATCH 1/3] MDL-60548 profile: Behat tests for email display in profiles. --- user/tests/behat/set_email_display.feature | 78 ++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 user/tests/behat/set_email_display.feature diff --git a/user/tests/behat/set_email_display.feature b/user/tests/behat/set_email_display.feature new file mode 100644 index 00000000000..f9e23f3ae2b --- /dev/null +++ b/user/tests/behat/set_email_display.feature @@ -0,0 +1,78 @@ +@core @core_user +Feature: Set email display preference + In order to control who can see my email address on my profile page + As a student + I need my email to be shown to only the user groups chosen + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | maildisplay | + | teacher1 | Teacher | 1 | teacher1@example.com | 2 | + | studentP | Student | PEER | studentP@example.com | 2 | + | studentN | Student | NONE | studentN@example.com | 0 | + | studentE | Student | EVERYONE | studentE@example.com | 1 | + | studentM | Student | MEMBERS | studentM@example.com | 2 | + And the following "courses" exist: + | fullname | shortname | format | + | Course 1 | C1 | topics | + And the following "course enrolments" exist: + | user | course | role | status | timeend | + | teacher1 | C1 | teacher | 0 | 0 | + | studentP | C1 | student | 0 | 0 | + | studentN | C1 | student | 0 | 0 | + | studentE | C1 | student | 0 | 0 | + | studentM | C1 | student | 0 | 0 | + + @javascript + Scenario: Student viewing own profile + Given I log in as "studentP" + When I follow "Profile" in the user menu + Then I should see "studentP@example.com" + + @javascript + Scenario: Student peer on the same course viewing profiles + Given I log in as "studentP" + And I am on "Course 1" course homepage + And I navigate to course participants + When I follow "Student NONE" + Then I should not see "studentN@example.com" + And I navigate to course participants + When I follow "Student EVERYONE" + Then I should see "studentE@example.com" + And I navigate to course participants + When I follow "Student MEMBERS" + Then I should see "studentM@example.com" + + @javascript + Scenario: Student viewing teacher email (whose maildisplay = MEMBERS) + Given I log in as "studentP" + And I am on "Course 1" course homepage + And I navigate to course participants + When I follow "Teacher 1" + Then I should see "teacher1@example.com" + + @javascript + Scenario: Teacher viewing student email, whilst site:showuseridentity = “email” + Given the following config values are set as admin: + | showuseridentity | email | + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to course participants + When I follow "Student NONE" + Then I should see "studentN@example.com" + And I navigate to course participants + When I follow "Student MEMBERS" + Then I should see "studentM@example.com" + + @javascript + Scenario: Teacher viewing student email, whilst site:showuseridentity = “” + Given I log in as "teacher1" + And the following config values are set as admin: + | showuseridentity | | + And I am on "Course 1" course homepage + And I navigate to course participants + When I follow "Student NONE" + Then I should not see "studentN@example.com" + And I navigate to course participants + When I follow "Student MEMBERS" + Then I should see "studentM@example.com" From 565f3f25f0775cbb9aeae0b7f41506116bfdbc4b Mon Sep 17 00:00:00 2001 From: David Balch Date: Mon, 23 Oct 2017 12:48:31 +0100 Subject: [PATCH 2/3] MDL-60548 profile: Show profile email when 'maildisplay' allows it. Adds 'email' to hiddenuserfields, which allows: * user to view email on their own profile, * users with cap site:viewuseridentity to view email on any profile (admins/teachers) * admin to veto display of email to users without site:viewuseridentity (except viewing their own) * unprivileged users to view if the profile's maildisplay permits it. Changes in user/lib.php include removal of is_siteadmin() test, which is redundant due to checks via has_capability(). Fixes regression from 2.9 (MDL-45774). --- admin/settings/users.php | 1 + lib/myprofilelib.php | 16 ++++++++++------ user/lib.php | 15 +++++++++------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/admin/settings/users.php b/admin/settings/users.php index 7fc752f38e3..8d93b5a4440 100644 --- a/admin/settings/users.php +++ b/admin/settings/users.php @@ -150,6 +150,7 @@ if ($hassiteconfig $temp->add(new admin_setting_configmultiselect('hiddenuserfields', new lang_string('hiddenuserfields', 'admin'), new lang_string('confighiddenuserfields', 'admin'), array(), array('description' => new lang_string('description'), + 'email' => new lang_string('email'), 'city' => new lang_string('city'), 'country' => new lang_string('country'), 'timezone' => new lang_string('timezone'), diff --git a/lib/myprofilelib.php b/lib/myprofilelib.php index 5ce910486a6..397d5b3e16c 100644 --- a/lib/myprofilelib.php +++ b/lib/myprofilelib.php @@ -127,7 +127,8 @@ function core_myprofile_navigation(core_user\output\myprofile\tree $tree, $user, } else { $hiddenfields = array_flip(explode(',', $CFG->hiddenuserfields)); } - if (has_capability('moodle/site:viewuseridentity', $courseorusercontext)) { + $canviewuseridentity = has_capability('moodle/site:viewuseridentity', $courseorusercontext); + if ($canviewuseridentity) { $identityfields = array_flip(explode(',', $CFG->showuseridentity)); } else { $identityfields = array(); @@ -151,11 +152,14 @@ function core_myprofile_navigation(core_user\output\myprofile\tree $tree, $user, $tree->add_node($node); } - if (isset($identityfields['email']) and ($iscurrentuser - or $user->maildisplay == 1 - or has_capability('moodle/course:useremail', $courseorusercontext) - or has_capability('moodle/site:viewuseridentity', $courseorusercontext) - or ($user->maildisplay == 2 and enrol_sharing_course($user, $USER)))) { + if ($iscurrentuser + or (!isset($hiddenfields['email']) and ( + $user->maildisplay == core_user::MAILDISPLAY_EVERYONE + or ($user->maildisplay == core_user::MAILDISPLAY_COURSE_MEMBERS_ONLY and enrol_sharing_course($user, $USER)) + or has_capability('moodle/course:useremail', $courseorusercontext) // TODO: Deprecate/remove for MDL-37479. + )) + or (isset($identityfields['email']) and $canviewuseridentity) + ) { $node = new core_user\output\myprofile\node('contact', 'email', get_string('email'), null, null, obfuscate_mailto($user->email, '')); $tree->add_node($node); diff --git a/user/lib.php b/user/lib.php index 6d1cf602214..5151d79572b 100644 --- a/user/lib.php +++ b/user/lib.php @@ -463,12 +463,15 @@ function user_get_user_details($user, $course = null, array $userfields = array( } } - if (in_array('email', $userfields) && ($isadmin // The admin is allowed the users email. - or $currentuser // Of course the current user is as well. - or $canviewuseremail // This is a capability in course context, it will be false in usercontext. - or in_array('email', $showuseridentityfields) - or $user->maildisplay == 1 - or ($user->maildisplay == 2 and enrol_sharing_course($user, $USER)))) { + if (in_array('email', $userfields) && ( + $currentuser + or (!isset($hiddenfields['email']) and ( + $user->maildisplay == core_user::MAILDISPLAY_EVERYONE + or ($user->maildisplay == core_user::MAILDISPLAY_COURSE_MEMBERS_ONLY and enrol_sharing_course($user, $USER)) + or $canviewuseremail // TODO: Deprecate/remove for MDL-37479. + )) + or in_array('email', $showuseridentityfields) + )) { $userdetails['email'] = $user->email; } From d2bba97d7760e16e5d22243cdc0aa3afbd643e7c Mon Sep 17 00:00:00 2001 From: David Balch Date: Mon, 13 Nov 2017 17:08:26 +0000 Subject: [PATCH 3/3] MDL-60548 profile: Clarify some users can always see email addresses. --- admin/settings/users.php | 2 +- admin/tool/uploaduser/user_form.php | 1 + enrol/lti/lib.php | 1 + enrol/lti/settings.php | 4 ++-- lang/en/moodle.php | 3 ++- user/editlib.php | 1 + 6 files changed, 8 insertions(+), 4 deletions(-) diff --git a/admin/settings/users.php b/admin/settings/users.php index 8d93b5a4440..d4449614e75 100644 --- a/admin/settings/users.php +++ b/admin/settings/users.php @@ -28,7 +28,7 @@ if ($hassiteconfig $choices['1'] = new lang_string('emaildisplayyes'); $choices['2'] = new lang_string('emaildisplaycourse'); $temp->add(new admin_setting_configselect('defaultpreference_maildisplay', new lang_string('emaildisplay'), - '', 2, $choices)); + new lang_string('emaildisplay_help'), 2, $choices)); $choices = array(); $choices['0'] = new lang_string('textformat'); diff --git a/admin/tool/uploaduser/user_form.php b/admin/tool/uploaduser/user_form.php index c291a735299..c0eb764883a 100644 --- a/admin/tool/uploaduser/user_form.php +++ b/admin/tool/uploaduser/user_form.php @@ -229,6 +229,7 @@ class admin_uploaduser_form2 extends moodleform { $choices = array(0 => get_string('emaildisplayno'), 1 => get_string('emaildisplayyes'), 2 => get_string('emaildisplaycourse')); $mform->addElement('select', 'maildisplay', get_string('emaildisplay'), $choices); $mform->setDefault('maildisplay', core_user::get_property_default('maildisplay')); + $mform->addHelpButton('maildisplay', 'emaildisplay'); $choices = array(0 => get_string('textformat'), 1 => get_string('htmlformat')); $mform->addElement('select', 'mailformat', get_string('emailformat'), $choices); diff --git a/enrol/lti/lib.php b/enrol/lti/lib.php index 9442c26e028..df7c08e24a4 100644 --- a/enrol/lti/lib.php +++ b/enrol/lti/lib.php @@ -298,6 +298,7 @@ class enrol_lti_plugin extends enrol_plugin { ); $mform->addElement('select', 'maildisplay', get_string('emaildisplay'), $choices); $mform->setDefault('maildisplay', $emaildisplay); + $mform->addHelpButton('maildisplay', 'emaildisplay'); $city = get_config('enrol_lti', 'city'); $mform->addElement('text', 'city', get_string('city'), 'maxlength="100" size="25"'); diff --git a/enrol/lti/settings.php b/enrol/lti/settings.php index b6d31a0dcb4..b839f76942d 100644 --- a/enrol/lti/settings.php +++ b/enrol/lti/settings.php @@ -47,8 +47,8 @@ if ($ADMIN->fulltree) { 1 => get_string('emaildisplayyes'), 2 => get_string('emaildisplaycourse')); $maildisplay = isset($CFG->defaultpreference_maildisplay) ? $CFG->defaultpreference_maildisplay : 2; - $settings->add(new admin_setting_configselect('enrol_lti/emaildisplay', get_string('emaildisplay'), '', - $maildisplay, $choices)); + $settings->add(new admin_setting_configselect('enrol_lti/emaildisplay', get_string('emaildisplay'), + get_string('emaildisplay_help'), $maildisplay, $choices)); $city = ''; if (!empty($CFG->defaultcity)) { diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 7df89eb5cc3..290be0a3df2 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -615,9 +615,10 @@ $string['emaildigestsubjects'] = 'Subjects (daily email with subjects only)'; $string['emaildisable'] = 'This email address is disabled'; $string['emaildisableclick'] = 'Click here to disable all email from being sent to this address'; $string['emaildisplay'] = 'Email display'; +$string['emaildisplay_help'] = 'Privileged users (such as teachers and managers) will always be able to see your email address.'; $string['emaildisplaycourse'] = 'Allow only other course members to see my email address'; $string['emaildisplayhidden'] = 'Email hidden'; -$string['emaildisplayno'] = 'Hide my email address from everyone'; +$string['emaildisplayno'] = 'Hide my email address from non-privileged users'; $string['emaildisplayyes'] = 'Allow everyone to see my email address'; $string['emailenable'] = 'This email address is enabled'; $string['emailenableclick'] = 'Click here to re-enable all email being sent to this address'; diff --git a/user/editlib.php b/user/editlib.php index 6a42d72fa8d..16fd1684893 100644 --- a/user/editlib.php +++ b/user/editlib.php @@ -302,6 +302,7 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions $choices['2'] = get_string('emaildisplaycourse'); $mform->addElement('select', 'maildisplay', get_string('emaildisplay'), $choices); $mform->setDefault('maildisplay', core_user::get_property_default('maildisplay')); + $mform->addHelpButton('maildisplay', 'emaildisplay'); $mform->addElement('text', 'city', get_string('city'), 'maxlength="120" size="21"'); $mform->setType('city', PARAM_TEXT);