From cee1103becae2ef766ac4f9aeb793f13f92522cb Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Wed, 13 Sep 2023 16:26:36 +0100
Subject: [PATCH] MDL-68712 enrol_self: improve detection/re-use of group
 enrolment keys.

---
 enrol/self/lang/en/enrol_self.php |  1 +
 enrol/self/lib.php                | 16 ++++++++++--
 enrol/self/tests/self_test.php    | 42 +++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/enrol/self/lang/en/enrol_self.php b/enrol/self/lang/en/enrol_self.php
index 4479ae1c3c1..e610b3f8747 100644
--- a/enrol/self/lang/en/enrol_self.php
+++ b/enrol/self/lang/en/enrol_self.php
@@ -92,6 +92,7 @@ If an enrolment key is specified, any user attempting to enrol in the course wil
 $string['passwordinvalid'] = 'Incorrect enrolment key, please try again';
 $string['passwordinvalidhint'] = 'That enrolment key was incorrect, please try again<br />
 (Here\'s a hint - it starts with \'{$a}\')';
+$string['passwordmatchesgroupkey'] = 'Enrolment key matches an existing group enrolment key';
 $string['pluginname'] = 'Self enrolment';
 $string['pluginname_desc'] = 'The self enrolment plugin allows users to choose which courses they want to participate in. The courses may be protected by an enrolment key. Internally the enrolment is done via the manual enrolment plugin which has to be enabled in the same course.';
 $string['requirepassword'] = 'Require enrolment key';
diff --git a/enrol/self/lib.php b/enrol/self/lib.php
index 1c931762e7f..8bf5182502b 100644
--- a/enrol/self/lib.php
+++ b/enrol/self/lib.php
@@ -159,8 +159,8 @@ class enrol_self_plugin extends enrol_plugin {
 
         \core\notification::success(get_string('youenrolledincourse', 'enrol'));
 
-        if ($instance->password and $instance->customint1 and $data->enrolpassword !== $instance->password) {
-            // It must be a group enrolment, let's assign group too.
+        // Test whether the password is also used as a group key.
+        if ($instance->password && $instance->customint1) {
             $groups = $DB->get_records('groups', array('courseid'=>$instance->courseid), 'id', 'id, enrolmentkey');
             foreach ($groups as $group) {
                 if (empty($group->enrolmentkey)) {
@@ -876,6 +876,9 @@ class enrol_self_plugin extends enrol_plugin {
      * @return void
      */
     public function edit_instance_validation($data, $files, $instance, $context) {
+        global $CFG;
+        require_once("{$CFG->dirroot}/enrol/self/locallib.php");
+
         $errors = array();
 
         $checkpassword = false;
@@ -890,6 +893,11 @@ class enrol_self_plugin extends enrol_plugin {
             if (($data['status'] == ENROL_INSTANCE_ENABLED) && ($instance->password !== $data['password'])) {
                 $checkpassword = true;
             }
+
+            // Check the password if we are enabling group enrolment keys.
+            if (!$instance->customint1 && $data['customint1']) {
+                $checkpassword = true;
+            }
         } else {
             $checkpassword = true;
         }
@@ -904,6 +912,10 @@ class enrol_self_plugin extends enrol_plugin {
                 if (!check_password_policy($data['password'], $errmsg)) {
                     $errors['password'] = $errmsg;
                 }
+            } else if (!empty($data['password']) && $data['customint1'] &&
+                    enrol_self_check_group_enrolment_key($data['courseid'], $data['password'])) {
+
+                $errors['password'] = get_string('passwordmatchesgroupkey', 'enrol_self');
             }
         }
 
diff --git a/enrol/self/tests/self_test.php b/enrol/self/tests/self_test.php
index 600c1ba10a3..351fdfeec34 100644
--- a/enrol/self/tests/self_test.php
+++ b/enrol/self/tests/self_test.php
@@ -16,6 +16,9 @@
 
 namespace enrol_self;
 
+use context_course;
+use enrol_self_plugin;
+
 defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
@@ -29,6 +32,7 @@ require_once($CFG->dirroot.'/enrol/self/locallib.php');
  * @category   phpunit
  * @copyright  2012 Petr Skoda {@link http://skodak.org}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @coversDefaultClass \enrol_self_plugin
  */
 class self_test extends \advanced_testcase {
 
@@ -761,6 +765,44 @@ class self_test extends \advanced_testcase {
         $this->assertEquals($canntenrolerror, $selfplugin->is_self_enrol_available($instance));
     }
 
+    /**
+     * Test custom validation of instance data for group enrolment key
+     *
+     * @covers ::edit_instance_validation
+     */
+    public function test_edit_instance_validation_group_enrolment_key(): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        $course = $this->getDataGenerator()->create_course();
+        $context = context_course::instance($course->id);
+
+        /** @var enrol_self_plugin $plugin */
+        $plugin = enrol_get_plugin('self');
+
+        $instance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => $plugin->get_name()], '*', MUST_EXIST);
+
+        // Enable group enrolment keys.
+        $errors = $plugin->edit_instance_validation([
+            'customint1' => 1,
+            'password' => 'cat',
+        ] + (array) $instance, [], $instance, $context);
+
+        $this->assertEmpty($errors);
+
+        // Now create a group with the same enrolment key we want to use.
+        $this->getDataGenerator()->create_group(['courseid' => $course->id, 'enrolmentkey' => 'cat']);
+
+        $errors = $plugin->edit_instance_validation([
+            'customint1' => 1,
+            'password' => 'cat',
+        ] + (array) $instance, [], $instance, $context);
+
+        $this->assertArrayHasKey('password', $errors);
+        $this->assertEquals('Enrolment key matches an existing group enrolment key', $errors['password']);
+    }
+
     /**
      * Test enrol_self_check_group_enrolment_key
      */