diff --git a/admin/tool/uploadcourse/classes/course.php b/admin/tool/uploadcourse/classes/course.php index 59477e3ac662e..f808817ccfc27 100644 --- a/admin/tool/uploadcourse/classes/course.php +++ b/admin/tool/uploadcourse/classes/course.php @@ -247,10 +247,10 @@ protected function delete() { * Log an error * * @param string $code error code. - * @param lang_string $message error message. + * @param string $message error message. * @return void */ - protected function error($code, lang_string $message) { + protected function error($code, string $message) { if (array_key_exists($code, $this->errors)) { throw new coding_exception('Error code already defined'); } @@ -1129,34 +1129,7 @@ protected function process_enrolment_data($course) { break; } - // Now update values. - $modifiedinstance = $instance; - - // Sort out the start, end and date. - $modifiedinstance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0); - $modifiedinstance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0); - - // Is the enrolment period set? - if (isset($method['enrolperiod']) && !empty($method['enrolperiod'])) { - if (preg_match('/^\d+$/', $method['enrolperiod'])) { - $method['enrolperiod'] = (int)$method['enrolperiod']; - } else { - // Try and convert period to seconds. - $method['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $method['enrolperiod']); - } - $modifiedinstance->enrolperiod = $method['enrolperiod']; - } - if ($instance->enrolstartdate > 0 && isset($method['enrolperiod'])) { - $modifiedinstance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod']; - } - if ($instance->enrolenddate > 0) { - $modifiedinstance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate; - } - if ($instance->enrolenddate < $instance->enrolstartdate) { - $modifiedinstance->enrolenddate = $instance->enrolstartdate; - } - - // Sort out the given role. + // Validate role context again since course is created. if (isset($method['role']) || isset($method['roleid'])) { if (isset($method['role'])) { $role = $method['role']; @@ -1166,22 +1139,14 @@ protected function process_enrolment_data($course) { $role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST); } if (!$this->validate_role_context($course->id, $roleid)) { - $this->error('contextrolenotallowed', - new lang_string('contextrolenotallowed', 'core_role', $role)); + $this->error('contextrolenotallowed', new lang_string('contextrolenotallowed', 'core_role', $role)); break; } - - $roleids = tool_uploadcourse_helper::get_role_ids(); - if (in_array($roleid, $roleids)) { - $modifiedinstance->roleid = $roleid; - } - } - - // Sort out custom instance name. - if (isset($method['name'])) { - $modifiedinstance->name = $method['name']; } + // Now update values. + // Sort out plugin specific fields. + $modifiedinstance = $plugin->update_enrol_plugin_data($course->id, $method, $instance); $plugin->update_instance($instance, $modifiedinstance); } else { foreach ($errors as $key => $message) { diff --git a/admin/tool/uploadcourse/tests/behat/cohorts.feature b/admin/tool/uploadcourse/tests/behat/cohorts.feature index 4441ed6099543..9a3676c6ea683 100644 --- a/admin/tool/uploadcourse/tests/behat/cohorts.feature +++ b/admin/tool/uploadcourse/tests/behat/cohorts.feature @@ -31,7 +31,7 @@ Feature: An admin can create courses with cohort enrolments using a CSV file | enrol_plugins_enabled | manual,guest,self | And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv" file to "File" filemanager When I click on "Preview" "button" - Then I should see "Cohort sync plugin is disabled" + Then I should see "Cohort sync enrol plugin is disabled" @javascript Scenario: Validation of cohorts for uploaded courses diff --git a/admin/tool/uploadcourse/tests/behat/guest.feature b/admin/tool/uploadcourse/tests/behat/guest.feature new file mode 100644 index 0000000000000..b01892ffaa749 --- /dev/null +++ b/admin/tool/uploadcourse/tests/behat/guest.feature @@ -0,0 +1,104 @@ +@tool @tool_uploadcourse @_file_upload +Feature: An admin can create courses with guest enrolments using a CSV file + In order to create courses using a CSV file with guest enrolment + As an admin + I need to be able to upload a CSV file and navigate through the import process + + Background: + Given the following "categories" exist: + | name | category | idnumber | + | Cat 0 | 0 | CAT0 | + | Cat 1 | CAT0 | CAT1 | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | CAT1 | + And I log in as "admin" + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + + @javascript + Scenario: Validation of password for uploaded courses with guest enrolments + # usepasswordpolicy is not set. + Given I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv" file to "File" filemanager + And I click on "Preview" "button" + And I click on "Upload courses" "button" + And I should see "Courses created: 2" + And I should see "Courses updated: 1" + And I should see "Courses errors: 0" + And I am on the "Course 1" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "test" + And I press "Cancel" + And I click on "Delete" "link" in the "Guest access" "table_row" + And I press "Continue" + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "" + And I press "Cancel" + And I click on "Delete" "link" in the "Guest access" "table_row" + And I press "Continue" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "Test123@" + And I press "Cancel" + And I click on "Delete" "link" in the "Guest access" "table_row" + And I press "Continue" + + # Policy is used, but password not required so it will not be generated if omitted. + And the following config values are set as admin: + | config | value | plugin | + | usepasswordpolicy | 1 | enrol_guest | + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "Passwords must be at least 8 characters long." + And I should see "Passwords must have at least 1 digit(s)." + And I should see "Passwords must have at least 1 upper case letter(s)." + And I should see "The password must have at least 1 special character(s) such as *, -, or #." + And I click on "Upload courses" "button" + And I should see "Courses created: 0" + And I should see "Courses updated: 2" + And I should see "Courses errors: 1" + And I am on the "Course 1" "enrolment methods" page + And "Guest access" "table_row" should not exist + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "" + And I press "Cancel" + And I click on "Delete" "link" in the "Guest access" "table_row" + And I press "Continue" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "Test123@" + And I press "Cancel" + And I click on "Delete" "link" in the "Guest access" "table_row" + And I press "Continue" + + # Policy is used and password not required so it will be generated if omitted. + And the following config values are set as admin: + | config | value | plugin | + | requirepassword | 1 | enrol_guest | + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "Passwords must be at least 8 characters long." + And I should see "Passwords must have at least 1 digit(s)." + And I should see "Passwords must have at least 1 upper case letter(s)." + And I should see "The password must have at least 1 special character(s) such as *, -, or #." + And I click on "Upload courses" "button" + And I should see "Courses created: 0" + And I should see "Courses updated: 2" + And I should see "Courses errors: 1" + And I am on the "Course 1" "enrolment methods" page + And "Guest access" "table_row" should not exist + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" does not match value "" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Guest access" "table_row" + And the field "Password" matches value "Test123@" diff --git a/admin/tool/uploadcourse/tests/behat/self.feature b/admin/tool/uploadcourse/tests/behat/self.feature new file mode 100644 index 0000000000000..8817bb6a02c1d --- /dev/null +++ b/admin/tool/uploadcourse/tests/behat/self.feature @@ -0,0 +1,122 @@ +@tool @tool_uploadcourse @_file_upload +Feature: An admin can create courses with self enrolments using a CSV file + In order to create courses using a CSV file with self enrolment + As an admin + I need to be able to upload a CSV file and navigate through the import process + + Background: + Given the following "categories" exist: + | name | category | idnumber | + | Cat 0 | 0 | CAT0 | + | Cat 1 | CAT0 | CAT1 | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | CAT1 | + And the following "groups" exist: + | name | course | idnumber | enrolmentkey | + | group1 | C1 | G1 | test | + And I log in as "admin" + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + + @javascript + Scenario: Validation of password for uploaded courses with self enrolments + # usepasswordpolicy is not set. + Given I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv" file to "File" filemanager + And I click on "Preview" "button" + And I click on "Upload courses" "button" + And I should see "Courses created: 2" + And I should see "Courses updated: 1" + And I should see "Courses errors: 0" + And I am on the "Course 1" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "test" + And I press "Cancel" + And I click on "Delete" "link" in the "Self enrolment" "table_row" + And I press "Continue" + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "" + And I press "Cancel" + And I click on "Delete" "link" in the "Self enrolment" "table_row" + And I press "Continue" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "Test123@" + And I press "Cancel" + And I click on "Delete" "link" in the "Self enrolment" "table_row" + And I press "Continue" + + # Policy is used, but password not required so it will not be generated if omitted. + And the following config values are set as admin: + | config | value | plugin | + | usepasswordpolicy | 1 | enrol_self | + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "Passwords must be at least 8 characters long." + And I should see "Passwords must have at least 1 digit(s)." + And I should see "Passwords must have at least 1 upper case letter(s)." + And I should see "The password must have at least 1 special character(s) such as *, -, or #." + And I click on "Upload courses" "button" + And I should see "Courses created: 0" + And I should see "Courses updated: 2" + And I should see "Courses errors: 1" + And I am on the "Course 1" "enrolment methods" page + And "Self enrolment (Student)" "table_row" should not exist + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "" + And I press "Cancel" + And I click on "Delete" "link" in the "Self enrolment" "table_row" + And I press "Continue" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "Test123@" + And I press "Cancel" + And I click on "Delete" "link" in the "Self enrolment" "table_row" + And I press "Continue" + + # Policy is used and password not required so it will be generated if omitted. + And the following config values are set as admin: + | config | value | plugin | + | requirepassword | 1 | enrol_self | + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "Passwords must be at least 8 characters long." + And I should see "Passwords must have at least 1 digit(s)." + And I should see "Passwords must have at least 1 upper case letter(s)." + And I should see "The password must have at least 1 special character(s) such as *, -, or #." + And I click on "Upload courses" "button" + And I should see "Courses created: 0" + And I should see "Courses updated: 2" + And I should see "Courses errors: 1" + And I am on the "Course 1" "enrolment methods" page + And "Self enrolment (Student)" "table_row" should not exist + And I am on the "Course 2" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" does not match value "" + And I am on the "Course 3" "enrolment methods" page + And I click on "Edit" "link" in the "Self enrolment" "table_row" + And the field "Enrolment key" matches value "Test123@" + + # Use group enrolment keys is set to yes, so the password must be different from group enrolment key. + And I am on the "Course 1" "enrolment methods" page + And I add "Self enrolment" enrolment method in "Course 1" with: + | Enrolment key | Abcejfd12@ | + | Use group enrolment keys | Yes | + And the following config values are set as admin: + | config | value | plugin | + | usepasswordpolicy | 0 | enrol_self | + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "This enrolment key is already used as a group enrolment key." diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv new file mode 100644 index 0000000000000..7fd3872d53761 --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_guest.csv @@ -0,0 +1,4 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_password +C1,Course 1,CAT1,guest,student,test +C2,Course 2,CAT1,guest,student, +C3,Course 3,CAT1,guest,student,Test123@ diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv new file mode 100644 index 0000000000000..ba848cbb48800 --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_self.csv @@ -0,0 +1,4 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_password +C1,Course 1,CAT1,self,student,test +C2,Course 2,CAT1,self,student, +C3,Course 3,CAT1,self,student,Test123@ \ No newline at end of file diff --git a/enrol/cohort/lang/en/deprecated.txt b/enrol/cohort/lang/en/deprecated.txt new file mode 100644 index 0000000000000..b46113e9e7d08 --- /dev/null +++ b/enrol/cohort/lang/en/deprecated.txt @@ -0,0 +1 @@ +plugindisabled,enrol_cohort diff --git a/enrol/cohort/lang/en/enrol_cohort.php b/enrol/cohort/lang/en/enrol_cohort.php index 77bf9d601a52d..a843bece15f9e 100644 --- a/enrol/cohort/lang/en/enrol_cohort.php +++ b/enrol/cohort/lang/en/enrol_cohort.php @@ -29,9 +29,11 @@ $string['defaultgroupnametext'] = '{$a->name} cohort {$a->increment}'; $string['enrolcohortsynctask'] = 'Cohort enrolment sync task'; $string['instanceexists'] = 'Cohort is already synchronised with selected role'; -$string['plugindisabled'] = 'Cohort sync plugin is disabled'; $string['pluginname'] = 'Cohort sync'; $string['pluginname_desc'] = 'Cohort enrolment plugin synchronises cohort members with course participants.'; $string['status'] = 'Active'; $string['creategroup'] = 'Create new group'; $string['privacy:metadata:core_group'] = 'Enrol cohort plugin can create a new group or use an existing group to add all the members of the cohort.'; + +// Deprecated since Moodle 4.5. +$string['plugindisabled'] = 'Cohort sync plugin is disabled'; diff --git a/enrol/cohort/lib.php b/enrol/cohort/lib.php index b95f92069622c..eba1675ea64f3 100644 --- a/enrol/cohort/lib.php +++ b/enrol/cohort/lib.php @@ -515,11 +515,7 @@ public function edit_instance_validation($data, $files, $instance, $context) { public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array { global $DB; - $errors = []; - if (!enrol_is_enabled('cohort')) { - $errors['plugindisabled'] = - new lang_string('plugindisabled', 'enrol_cohort'); - } + $errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid); if (isset($enrolmentdata['addtogroup'])) { $addtogroup = $enrolmentdata['addtogroup']; diff --git a/enrol/guest/lib.php b/enrol/guest/lib.php index 66e2465e845ff..e06e6a6f3d10e 100644 --- a/enrol/guest/lib.php +++ b/enrol/guest/lib.php @@ -529,6 +529,43 @@ public function find_instance(array $enrolmentdata, int $courseid): ?stdClass { public function fill_enrol_custom_fields(array $enrolmentdata, int $courseid): array { return $enrolmentdata + ['password' => '']; } + + /** + * Updates enrol plugin instance with provided data. + * @param int $courseid Course ID. + * @param array $enrolmentdata enrolment data. + * @param stdClass $instance Instance to update. + * + * @return stdClass updated instance + */ + public function update_enrol_plugin_data(int $courseid, array $enrolmentdata, stdClass $instance): stdClass { + if (!empty($enrolmentdata['password'])) { + $instance->password = $enrolmentdata['password']; + } + return parent::update_enrol_plugin_data($courseid, $enrolmentdata, $instance); + } + + /** + * Check if data is valid for a given enrolment plugin + * + * @param array $enrolmentdata enrolment data to validate. + * @param int|null $courseid Course ID. + * @return array Errors + */ + public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array { + // If password is omitted or empty in csv it will be generated automatically if it is a required policy. + + $errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid); + + $policy = $this->get_config('usepasswordpolicy'); + if (!empty($enrolmentdata['password']) && $policy) { + $errarray = get_password_policy_errors($enrolmentdata['password']); + foreach ($errarray as $i => $err) { + $errors['enrol_guest' . $i] = $err; + } + } + return $errors; + } } /** diff --git a/enrol/guest/tests/lib_test.php b/enrol/guest/tests/lib_test.php index 1a35b5801b764..2003a0652383e 100644 --- a/enrol/guest/tests/lib_test.php +++ b/enrol/guest/tests/lib_test.php @@ -26,6 +26,77 @@ class lib_test extends \advanced_testcase { + /** + * Test the behaviour of validate_enrol_plugin_data(). + * + * @covers ::validate_enrol_plugin_data + */ + public function test_validate_enrol_plugin_data(): void { + global $CFG; + + $this->resetAfterTest(); + + $guestplugin = enrol_get_plugin('guest'); + + $guestplugin->set_config('usepasswordpolicy', false); + $enrolmentdata = []; + $errors = $guestplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertEmpty($errors); + + // Now enable some controls, and check that the policy responds with policy text. + $guestplugin->set_config('usepasswordpolicy', true); + $CFG->minpasswordlength = 8; + $CFG->minpassworddigits = 1; + $CFG->minpasswordlower = 1; + $CFG->minpasswordupper = 1; + $CFG->minpasswordnonalphanum = 1; + $CFG->maxconsecutiveidentchars = 1; + $errors = $guestplugin->validate_enrol_plugin_data($enrolmentdata); + // If password is omitted it will be autocreated so nothing to validate. + $this->assertEmpty($errors); + + $enrolmentdata = ['password' => 'test']; + $errors = $guestplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertCount(4, $errors); + $this->assertEquals(get_string('errorminpasswordlength', 'auth', $CFG->minpasswordlength), $errors['enrol_guest0']); + $this->assertEquals(get_string('errorminpassworddigits', 'auth', $CFG->minpassworddigits), $errors['enrol_guest1']); + $this->assertEquals(get_string('errorminpasswordupper', 'auth', $CFG->minpasswordupper), $errors['enrol_guest2']); + $this->assertEquals(get_string('errorminpasswordnonalphanum', 'auth', $CFG->minpasswordnonalphanum), $errors['enrol_guest3']); + + $enrolmentdata = ['password' => 'Testingtest123@']; + $errors = $guestplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertEmpty($errors); + } + + /** + * Test the behaviour of update_enrol_plugin_data(). + * + * @covers ::update_enrol_plugin_data + */ + public function test_update_enrol_plugin_data(): void { + global $DB; + $this->resetAfterTest(); + $manualplugin = enrol_get_plugin('guest'); + + $admin = get_admin(); + $this->setUser($admin); + + $enrolmentdata = []; + + $cat = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + $instance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'guest'], '*', MUST_EXIST); + + $expectedinstance = $instance; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['password'] = 'test'; + $expectedinstance->password = 'test'; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + } + /** * Test the behaviour of find_instance(). * diff --git a/enrol/meta/lib.php b/enrol/meta/lib.php index f39b62b2a6ae7..6c57bcc7c701a 100644 --- a/enrol/meta/lib.php +++ b/enrol/meta/lib.php @@ -393,11 +393,7 @@ public function edit_instance_validation($data, $files, $instance, $context) { public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array { global $DB; - $errors = []; - if (!enrol_is_enabled('meta')) { - $errors['plugindisabled'] = - new lang_string('plugindisabled', 'plugin'); - } + $errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid); if (isset($enrolmentdata['addtogroup'])) { $addtogroup = $enrolmentdata['addtogroup']; diff --git a/enrol/self/lib.php b/enrol/self/lib.php index 2001f77e97de6..80ad7d48e2844 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -1015,18 +1015,19 @@ public function edit_instance_validation($data, $files, $instance, $context) { if ($checkpassword) { $require = $this->get_config('requirepassword'); - $policy = $this->get_config('usepasswordpolicy'); + $policy = $this->get_config('usepasswordpolicy'); if ($require and trim($data['password']) === '') { $errors['password'] = get_string('required'); - } else if (!empty($data['password']) && $policy) { - $errmsg = ''; - if (!check_password_policy($data['password'], $errmsg)) { - $errors['password'] = $errmsg; + } else if (!empty($data['password'])) { + if ($policy) { + $errmsg = ''; + if (!check_password_policy($data['password'], $errmsg)) { + $errors['password'] = $errmsg; + } + } + if ($data['customint1'] && enrol_self_check_group_enrolment_key($instance->courseid, $data['password'])) { + $errors['password'] = get_string('passwordmatchesgroupkey', 'enrol_self'); } - } else if (!empty($data['password']) && $data['customint1'] && - enrol_self_check_group_enrolment_key($data['courseid'], $data['password'])) { - - $errors['password'] = get_string('passwordmatchesgroupkey', 'enrol_self'); } } @@ -1204,6 +1205,59 @@ public function find_instance(array $enrolmentdata, int $courseid): ?stdClass { public function fill_enrol_custom_fields(array $enrolmentdata, int $courseid): array { return $enrolmentdata + ['password' => '']; } + + /** + * Updates enrol plugin instance with provided data. + * @param int $courseid Course ID. + * @param array $enrolmentdata enrolment data. + * @param stdClass $instance Instance to update. + * + * @return stdClass updated instance + */ + public function update_enrol_plugin_data(int $courseid, array $enrolmentdata, stdClass $instance): stdClass { + if (!empty($enrolmentdata['password'])) { + $instance->password = $enrolmentdata['password']; + } + return parent::update_enrol_plugin_data($courseid, $enrolmentdata, $instance); + } + + /** + * Check if data is valid for a given enrolment plugin + * + * @param array $enrolmentdata enrolment data to validate. + * @param int|null $courseid Course ID. + * @return array Errors + */ + public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array { + global $CFG, $DB; + + require_once($CFG->dirroot . "/enrol/self/locallib.php"); + + // If password is omitted or empty in csv it will be generated automatically if it is a required policy. + + $errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid); + $policy = $this->get_config('usepasswordpolicy'); + if (!empty($enrolmentdata['password'])) { + if ($policy) { + $errarray = get_password_policy_errors($enrolmentdata['password']); + foreach ($errarray as $i => $err) { + $errors['enrol_self' . $i] = $err; + } + } + + if ($courseid) { + // This is bad - no way to identify which instance it is. + // So if any instance in course uses group key we should error. + $usegroupenrolmentkeys = + $DB->count_records('enrol', ['courseid' => $courseid, 'enrol' => 'self', 'customint1' => 1]); + if ($usegroupenrolmentkeys && enrol_self_check_group_enrolment_key($courseid, $enrolmentdata['password'])) { + $errors['errorpasswordmatchesgroupkey'] = + new lang_string('passwordmatchesgroupkey', 'enrol_self', $enrolmentdata['password']); + } + } + } + return $errors; + } } /** diff --git a/enrol/self/tests/self_test.php b/enrol/self/tests/self_test.php index f082499123e3c..3cf850b4ba73a 100644 --- a/enrol/self/tests/self_test.php +++ b/enrol/self/tests/self_test.php @@ -1143,4 +1143,86 @@ public function test_find_instance() { $this->assertEquals($instanceid1->id, $actual->id); } + /** + * Test the behaviour of validate_enrol_plugin_data(). + * + * @covers ::validate_enrol_plugin_data + */ + public function test_validate_enrol_plugin_data(): void { + global $CFG; + + $this->resetAfterTest(); + + // Test in course with groups. + $course = self::getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]); + + $selfplugin = enrol_get_plugin('self'); + + $selfplugin->set_config('usepasswordpolicy', false); + $enrolmentdata = []; + $errors = $selfplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertEmpty($errors); + + // Now enable some controls, and check that the policy responds with policy text. + $selfplugin->set_config('usepasswordpolicy', true); + $CFG->minpasswordlength = 8; + $CFG->minpassworddigits = 1; + $CFG->minpasswordlower = 1; + $CFG->minpasswordupper = 1; + $CFG->minpasswordnonalphanum = 1; + $CFG->maxconsecutiveidentchars = 1; + $errors = $selfplugin->validate_enrol_plugin_data($enrolmentdata); + // If password is omitted it will be autocreated so nothing to validate. + $this->assertEmpty($errors); + + $enrolmentdata = ['password' => 'test']; + $errors = $selfplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertCount(4, $errors); + $this->assertEquals(get_string('errorminpasswordlength', 'auth', $CFG->minpasswordlength), $errors['enrol_self0']); + $this->assertEquals(get_string('errorminpassworddigits', 'auth', $CFG->minpassworddigits), $errors['enrol_self1']); + $this->assertEquals(get_string('errorminpasswordupper', 'auth', $CFG->minpasswordupper), $errors['enrol_self2']); + $this->assertEquals(get_string('errorminpasswordnonalphanum', 'auth', $CFG->minpasswordnonalphanum), $errors['enrol_self3']); + + $enrolmentdata = ['password' => 'Testingtest123@']; + $errors = $selfplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertEmpty($errors); + + $this->getDataGenerator()->create_group(['courseid' => $course->id, 'enrolmentkey' => 'Abirvalg123@']); + $instance = $selfplugin->find_instance([], $course->id); + $instance->customint1 = 1; + $selfplugin->update_instance($instance, $instance); + $enrolmentdata = ['password' => 'Abirvalg123@']; + $errors = $selfplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); + $this->assertArrayHasKey('errorpasswordmatchesgroupkey', $errors); + } + + /** + * Test the behaviour of update_enrol_plugin_data(). + * + * @covers ::update_enrol_plugin_data + */ + public function test_update_enrol_plugin_data(): void { + global $DB; + $this->resetAfterTest(); + $manualplugin = enrol_get_plugin('self'); + + $admin = get_admin(); + $this->setUser($admin); + + $enrolmentdata = []; + + $cat = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + $instance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'self'], '*', MUST_EXIST); + + $expectedinstance = $instance; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['password'] = 'test'; + $expectedinstance->password = 'test'; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + } + } diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php index add5167a5094e..db7efa102a2d1 100644 --- a/enrol/tests/enrollib_test.php +++ b/enrol/tests/enrollib_test.php @@ -23,6 +23,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +use core\plugininfo\enrol; + defined('MOODLE_INTERNAL') || die(); @@ -1733,4 +1735,96 @@ public function test_enrol_selfenrol_available() { $this->setUser($user2); $this->assertFalse(enrol_selfenrol_available($course->id)); } + + /** + * Test the behaviour of validate_enrol_plugin_data(). + * + * @covers ::validate_enrol_plugin_data + */ + public function test_validate_enrol_plugin_data(): void { + $this->resetAfterTest(); + + // Plugin is disabled in system. + enrol::enable_plugin('manual', false); + $manualplugin = enrol_get_plugin('manual'); + + $enrolmentdata = []; + $errors = $manualplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertArrayHasKey('plugindisabled', $errors); + $this->assertArrayNotHasKey('errorunsupportedmethod', $errors); + + $categoryplugin = enrol_get_plugin('category'); + $errors = $categoryplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertArrayHasKey('errorunsupportedmethod', $errors); + } + + /** + * Test the behaviour of update_enrol_plugin_data(). + * + * @covers ::update_enrol_plugin_data + */ + public function test_update_enrol_plugin_data(): void { + global $DB; + $this->resetAfterTest(); + $manualplugin = enrol_get_plugin('manual'); + + $admin = get_admin(); + $this->setUser($admin); + + $enrolmentdata = []; + + $cat = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + $instance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual'], '*', MUST_EXIST); + + $teacherroleid = $DB->get_field('role', 'id', ['shortname' => 'teacher']); + $editingteacherroleid = $DB->get_field('role', 'id', ['shortname' => 'editingteacher']); + + $enrolmentdata['startdate'] = '3 Feb 2024'; + $enrolmentdata['enddate'] = '4 Feb 2024'; + $enrolmentdata['role'] = 'teacher'; + $enrolmentdata['name'] = 'testinstance'; + + $expectedinstance = $instance; + $expectedinstance->enrolstartdate = strtotime($enrolmentdata['startdate']); + $expectedinstance->enrolenddate = strtotime($enrolmentdata['enddate']); + $expectedinstance->role = $teacherroleid; + $expectedinstance->name = $enrolmentdata['name']; + $expectedinstance->enrolperiod = $expectedinstance->enrolenddate - $expectedinstance->enrolstartdate; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['roleid'] = $editingteacherroleid; + unset($enrolmentdata['startdate']); + unset($enrolmentdata['enddate']); + unset($enrolmentdata['role']); + $enrolmentdata['enrolperiod'] = $modifiedinstance->enrolperiod++; + $expectedinstance->roleid = $editingteacherroleid; + $expectedinstance->enrolstartdate = 0; + $expectedinstance->enrolenddate = 0; + $expectedinstance->enrolperiod++; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['startdate'] = '3 Feb 2024'; + $enrolmentdata['enrolperiod'] = 3600; + $expectedinstance->enrolstartdate = strtotime($enrolmentdata['startdate']); + $expectedinstance->enrolperiod = $enrolmentdata['enrolperiod']; + $expectedinstance->enrolenddate = $expectedinstance->enrolstartdate + $enrolmentdata['enrolperiod']; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['enddate'] = '5 Feb 2024'; + unset($enrolmentdata['enrolperiod']); + $expectedinstance->enrolenddate = strtotime($enrolmentdata['startdate']); + $expectedinstance->enrolperiod = $expectedinstance->enrolenddate - $expectedinstance->enrolstartdate; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + + $enrolmentdata['enrolperiod'] = '2hours'; + $expectedinstance->enrolperiod = 7200; + $expectedinstance->enrolenddate = $expectedinstance->enrolstartdate + $expectedinstance->enrolperiod; + $modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance); + $this->assertEquals($expectedinstance, $modifiedinstance); + } } diff --git a/lang/en/enrol.php b/lang/en/enrol.php index 7d6ad4b494c15..72119f2456e21 100644 --- a/lang/en/enrol.php +++ b/lang/en/enrol.php @@ -132,6 +132,7 @@ $string['periodnone'] = 'enrolled {$a}'; $string['periodstart'] = 'from {$a}'; $string['periodstartend'] = 'from {$a->start} until {$a->end}'; +$string['plugindisabled'] = '{$a} enrol plugin is disabled'; $string['recovergrades'] = 'Recover user\'s old grades if possible'; $string['rolefromthiscourse'] = '{$a->role} (Assigned in this course)'; $string['rolefrommetacourse'] = '{$a->role} (Inherited from parent course)'; diff --git a/lib/enrollib.php b/lib/enrollib.php index 30031a9d343f8..11a83a2d5d6a5 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -3536,7 +3536,12 @@ public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid $errors['errorunsupportedmethod'] = new lang_string('errorunsupportedmethod', 'tool_uploadcourse', get_class($this)); - + } else { + $plugin = $this->get_name(); + if (!enrol_is_enabled($plugin)) { + $pluginname = get_string('pluginname', 'enrol_' . $plugin); + $errors['plugindisabled'] = new lang_string('plugindisabled', 'enrol', $pluginname); + } } return $errors; } @@ -3728,4 +3733,56 @@ public function send_course_welcome_message_to_user( message_send($message); } + + /** + * Updates enrol plugin instance with provided data. + * @param int $courseid Course ID. + * @param array $enrolmentdata enrolment data. + * @param stdClass $instance Instance to update. + * + * @return stdClass updated instance + */ + public function update_enrol_plugin_data(int $courseid, array $enrolmentdata, stdClass $instance): stdClass { + global $DB; + + // Sort out the start, end and date. + $instance->enrolstartdate = (isset($enrolmentdata['startdate']) ? strtotime($enrolmentdata['startdate']) : 0); + $instance->enrolenddate = (isset($enrolmentdata['enddate']) ? strtotime($enrolmentdata['enddate']) : 0); + + // Is the enrolment period set? + if (!empty($enrolmentdata['enrolperiod'])) { + if (preg_match('/^\d+$/', $enrolmentdata['enrolperiod'])) { + $enrolmentdata['enrolperiod'] = (int)$enrolmentdata['enrolperiod']; + } else { + // Try and convert period to seconds. + $enrolmentdata['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $enrolmentdata['enrolperiod']); + } + $instance->enrolperiod = $enrolmentdata['enrolperiod']; + } + if ($instance->enrolstartdate > 0 && isset($enrolmentdata['enrolperiod'])) { + $instance->enrolenddate = $instance->enrolstartdate + $enrolmentdata['enrolperiod']; + } + if ($instance->enrolenddate > 0) { + $instance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate; + } + if ($instance->enrolenddate < $instance->enrolstartdate) { + $instance->enrolenddate = $instance->enrolstartdate; + } + + // Sort out the given role. + if (isset($enrolmentdata['role']) || isset($enrolmentdata['roleid'])) { + if (isset($enrolmentdata['role'])) { + $roleid = $DB->get_field('role', 'id', ['shortname' => $enrolmentdata['role']], MUST_EXIST); + } else { + $roleid = $enrolmentdata['roleid']; + } + $instance->roleid = $roleid; + } + + // Sort out custom instance name. + if (isset($enrolmentdata['name'])) { + $instance->name = $enrolmentdata['name']; + } + return $instance; + } } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 7e0b0d9513441..94e62c601ea01 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4549,56 +4549,72 @@ function get_complete_user_data($field, $value, $mnethostid = null, $throwexcept * Validate a password against the configured password policy * * @param string $password the password to be checked against the password policy - * @param string $errmsg the error message to display when the password doesn't comply with the policy. - * @param stdClass $user the user object to perform password validation against. Defaults to null if not provided. + * @param string|null $errmsg the error message to display when the password doesn't comply with the policy. + * @param stdClass|null $user the user object to perform password validation against. Defaults to null if not provided. * * @return bool true if the password is valid according to the policy. false otherwise. */ -function check_password_policy($password, &$errmsg, $user = null) { +function check_password_policy(string $password, ?string &$errmsg, ?stdClass $user = null) { global $CFG; - if (!empty($CFG->passwordpolicy) && !isguestuser($user)) { - $errmsg = ''; - if (core_text::strlen($password) < $CFG->minpasswordlength) { - $errmsg .= '