Merge branch 'MDL-76821-400' of https://github.com/snake/moodle into …

…MOODLE_400_STABLE
moodle · Jun 8, 2023 · bbe3109 · bbe3109
2 parents 72e923f + 660ea3d
commit bbe3109
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 2 deletions.
diff --git a/enrol/lti/classes/local/ltiadvantage/repository/user_repository.php b/enrol/lti/classes/local/ltiadvantage/repository/user_repository.php
@@ -235,7 +235,8 @@ public function find(int $id): ?user {
                       FROM {{$this->ltiuserstable}} lu
                       JOIN {user} u
                         ON (u.id = lu.userid)
-                     WHERE lu.id = :id";
+                     WHERE lu.id = :id
+                       AND lu.ltideploymentid IS NOT NULL";
 
             $record = $DB->get_record_sql($sql, ['id' => $id], MUST_EXIST);
             return $this->user_from_record($record);
@@ -262,7 +263,8 @@ public function find_single_user_by_resource(int $userid, int $resourceid): ?use
                       JOIN {user} u
                         ON (u.id = lu.userid)
                      WHERE lu.userid = :userid
-                       AND lu.toolid = :resourceid";
+                       AND lu.toolid = :resourceid
+                       AND lu.ltideploymentid IS NOT NULL";
 
             $params = ['userid' => $userid, 'resourceid' => $resourceid];
             $record = $DB->get_record_sql($sql, $params, MUST_EXIST);
@@ -287,6 +289,7 @@ public function find_by_resource(int $resourceid): array {
                   JOIN {user} u
                     ON (u.id = lu.userid)
                  WHERE lu.toolid = :resourceid
+                   AND lu.ltideploymentid IS NOT NULL
               ORDER BY lu.lastaccess DESC";
 
         $records = $DB->get_records_sql($sql, ['resourceid' => $resourceid]);

diff --git a/enrol/lti/lang/en/enrol_lti.php b/enrol/lti/lang/en/enrol_lti.php
@@ -54,6 +54,7 @@
 $string['enrolenddate_help'] = 'If enabled, users can access until this date only.';
 $string['enrolenddateerror'] = 'Enrolment end date cannot be earlier than start date';
 $string['enrolisdisabled'] = 'The \'Publish as LTI tool\' plugin is disabled.';
+$string['enrolltiversionincorrect'] = 'The resource is not set up for use over legacy LTI (versions 1.1/2.0). Please contact the administrator of this tool.';
 $string['enrolperiod'] = 'Enrolment duration';
 $string['enrolperiod_help'] = 'Length of time that the enrolment is valid, starting with the moment the user enrols themselves from the remote system. If disabled, the enrolment duration will be unlimited.';
 $string['enrolmentfinished'] = 'Enrolment finished.';

diff --git a/enrol/lti/tests/local/ltiadvantage/repository/user_repository_test.php b/enrol/lti/tests/local/ltiadvantage/repository/user_repository_test.php
@@ -510,4 +510,43 @@ public function test_save_deleted() {
         $this->assertEquals($saveduser->get_localid(), $saveduser2->get_localid());
         $this->assertNotEquals($saveduser->get_id(), $saveduser2->get_id());
     }
+
+    /**
+     * Test confirming that any associated legacy lti user records are not returned by the repository.
+     *
+     * This test ensures that any enrolment methods (resources) updated in-place from legacy LTI to 1.3 only return LTI 1.3 users.
+     *
+     * @covers ::find
+     * @covers ::find_single_user_by_resource
+     * @covers ::find_by_resource
+     */
+    public function test_find_filters_legacy_lti_users(): void {
+        $this->resetAfterTest();
+        global $DB;
+        $user = $this->getDataGenerator()->create_user();
+        $course = $this->getDataGenerator()->create_course();
+        $resource = $this->getDataGenerator()->create_lti_tool((object)['courseid' => $course->id]);
+        $ltiuserdata = [
+            'userid' => $user->id,
+            'toolid' => $resource->id,
+            'sourceid' => '1001',
+        ];
+        $ltiuserid = $DB->insert_record('enrol_lti_users', $ltiuserdata);
+        $userrepo = new user_repository();
+
+        $this->assertNull($userrepo->find($ltiuserid));
+        $this->assertNull($userrepo->find_single_user_by_resource($user->id, $resource->id));
+        $this->assertEmpty($userrepo->find_by_resource($resource->id));
+
+        // Set deploymentid, indicating the user originated from an LTI 1.3 launch and should now be returned.
+        $ltiuserdata['id'] = $ltiuserid;
+        $ltiuserdata['ltideploymentid'] = '234';
+        $DB->update_record('enrol_lti_users', $ltiuserdata);
+
+        $this->assertInstanceOf(user::class, $userrepo->find($ltiuserid));
+        $this->assertInstanceOf(user::class, $userrepo->find_single_user_by_resource($user->id, $resource->id));
+        $ltiusers = $userrepo->find_by_resource($resource->id);
+        $this->assertCount(1, $ltiusers);
+        $this->assertInstanceOf(user::class, reset($ltiusers));
+    }
 }
diff --git a/enrol/lti/tool.php b/enrol/lti/tool.php
@@ -53,6 +53,12 @@
     exit();
 }
 
+// Check if the enrolment instance has been upgraded to a newer LTI version.
+if ($tool->ltiversion != 'LTI-1p0/LTI-2p0') {
+    throw new \moodle_exception('enrolltiversionincorrect', 'enrol_lti');
+    exit();
+}
+
 $consumerkey = required_param('oauth_consumer_key', PARAM_TEXT);
 $ltiversion = optional_param('lti_version', null, PARAM_TEXT);
 $messagetype = required_param('lti_message_type', PARAM_TEXT);