From 318388af70002e51916c3cc0426e1efaf0acaa53 Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Thu, 27 Mar 2025 10:04:25 +0000
Subject: [PATCH] MDL-84996 enrol_manual: ensure externally supplied users are
 valid.

---
 enrol/manual/externallib.php            | 17 +++++++++--------
 enrol/manual/tests/externallib_test.php | 13 ++++++++++++-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/enrol/manual/externallib.php b/enrol/manual/externallib.php
index 27ab3721beb..005ff8619c5 100644
--- a/enrol/manual/externallib.php
+++ b/enrol/manual/externallib.php
@@ -91,13 +91,16 @@ class enrol_manual_external extends external_api {
             // Check that the user has the permission to manual enrol.
             require_capability('enrol/manual:enrol', $context);
 
+            $user = core_user::get_user($enrolment['userid'], strictness: MUST_EXIST);
+            core_user::require_active_user($user);
+
             // Throw an exception if user is not able to assign the role.
             $roles = get_assignable_roles($context);
             if (!array_key_exists($enrolment['roleid'], $roles)) {
                 $errorparams = new stdClass();
                 $errorparams->roleid = $enrolment['roleid'];
                 $errorparams->courseid = $enrolment['courseid'];
-                $errorparams->userid = $enrolment['userid'];
+                $errorparams->userid = $user->id;
                 throw new moodle_exception('wsusercannotassign', 'enrol_manual', '', $errorparams);
             }
 
@@ -121,7 +124,7 @@ class enrol_manual_external extends external_api {
                 $errorparams = new stdClass();
                 $errorparams->roleid = $enrolment['roleid'];
                 $errorparams->courseid = $enrolment['courseid'];
-                $errorparams->userid = $enrolment['userid'];
+                $errorparams->userid = $user->id;
                 throw new moodle_exception('wscannotenrol', 'enrol_manual', '', $errorparams);
             }
 
@@ -131,7 +134,7 @@ class enrol_manual_external extends external_api {
             $enrolment['status'] = (isset($enrolment['suspend']) && !empty($enrolment['suspend'])) ?
                     ENROL_USER_SUSPENDED : ENROL_USER_ACTIVE;
 
-            $enrol->enrol_user($instance, $enrolment['userid'], $enrolment['roleid'],
+            $enrol->enrol_user($instance, $user->id, $enrolment['roleid'],
                     $enrolment['timestart'], $enrolment['timeend'], $enrolment['status']);
 
         }
@@ -197,14 +200,12 @@ class enrol_manual_external extends external_api {
             if (!$instance) {
                 throw new moodle_exception('wsnoinstance', 'enrol_manual', '', $enrolment);
             }
-            $user = $DB->get_record('user', array('id' => $enrolment['userid']));
-            if (!$user) {
-                throw new invalid_parameter_exception('User id not exist: '.$enrolment['userid']);
-            }
+            $user = core_user::get_user($enrolment['userid'], strictness: MUST_EXIST);
+            core_user::require_active_user($user);
             if (!$enrol->allow_unenrol($instance)) {
                 throw new moodle_exception('wscannotunenrol', 'enrol_manual', '', $enrolment);
             }
-            $enrol->unenrol_user($instance, $enrolment['userid']);
+            $enrol->unenrol_user($instance, $user->id);
         }
         $transaction->allow_commit();
     }
diff --git a/enrol/manual/tests/externallib_test.php b/enrol/manual/tests/externallib_test.php
index e04606cf6a7..f83b7a3756d 100644
--- a/enrol/manual/tests/externallib_test.php
+++ b/enrol/manual/tests/externallib_test.php
@@ -33,6 +33,7 @@ require_once($CFG->dirroot . '/enrol/manual/externallib.php');
  * @category   phpunit
  * @copyright  2012 Jerome Mouneyrac
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers     \enrol_manual_external
  * @since Moodle 2.4
  */
 final class externallib_test extends externallib_advanced_testcase {
@@ -94,6 +95,16 @@ final class externallib_test extends externallib_advanced_testcase {
         $this->assignUserCapability('enrol/manual:enrol', $context1->id, $roleid);
         $this->assertEquals(0, $DB->count_records('user_enrolments'));
 
+        // Call with invalid user.
+        try {
+            enrol_manual_external::enrol_users([
+                ['roleid' => 1, 'userid' => 654321, 'courseid' => $course1->id],
+            ]);
+            $this->fail('Exception expected for invalid user.');
+        } catch (\moodle_exception $e) {
+            $this->assertSame('invaliduser', $e->errorcode);
+        }
+
         // Call with forbidden role.
         try {
             enrol_manual_external::enrol_users(array(
@@ -276,7 +287,7 @@ final class externallib_test extends externallib_advanced_testcase {
             ));
             $this->fail('Exception expected: invalid student id');
         } catch (\Exception $ex) {
-            $this->assertTrue($ex instanceof \invalid_parameter_exception);
+            $this->assertSame('invaliduser', $ex->errorcode);
         }
 
         // Call for course without manual instance.