From 38ff4d528ee6d93c31e42476b252272db7b1af04 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Fri, 20 Aug 2021 15:47:31 +0800 Subject: [PATCH] MDL-69504 mod_lti: check role switches when setting lti roles on launch is_siteadmin isn't aware of role switches, so make sure we're not in a switched role before calling that method. --- mod/lti/locallib.php | 2 +- mod/lti/tests/locallib_test.php | 95 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index ae27f4a2528..b14a4cdddb7 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -2165,7 +2165,7 @@ function lti_get_ims_role($user, $cmid, $courseid, $islti2) { } } - if (is_siteadmin($user) || has_capability('mod/lti:admin', $context)) { + if (!is_role_switched($courseid) && (is_siteadmin($user)) || has_capability('mod/lti:admin', $context)) { // Make sure admins do not have the Learner role, then set admin role. $roles = array_diff($roles, array('Learner')); if (!$islti2) { diff --git a/mod/lti/tests/locallib_test.php b/mod/lti/tests/locallib_test.php index 51b7362edb8..79ff5208ceb 100644 --- a/mod/lti/tests/locallib_test.php +++ b/mod/lti/tests/locallib_test.php @@ -1740,6 +1740,101 @@ MwIDAQAB $this->assertEquals(get_course_history($course), [38903]); } + /** + * Test the lti_get_ims_role helper function. + * + * @dataProvider lti_get_ims_role_provider + * @param bool $islti2 whether the method is called with LTI 2.0 role names or not. + * @param string $rolename the name of the role (student, teacher, admin) + * @param null|string $switchedto the role to switch to, or false if not using the 'switch to' functionality. + * @param string $expected the expected role name. + */ + public function test_lti_get_ims_role(bool $islti2, string $rolename, ?string $switchedto, string $expected) { + global $DB; + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $user = $rolename == 'admin' ? get_admin() : $this->getDataGenerator()->create_and_enrol($course, $rolename); + + if ($switchedto) { + $this->setUser($user); + $role = $DB->get_record('role', array('shortname' => $switchedto)); + role_switch($role->id, context_course::instance($course->id)); + } + + $this->assertEquals($expected, lti_get_ims_role($user, 0, $course->id, $islti2)); + } + + /** + * Data provider for testing lti_get_ims_role. + * + * @return array[] the test case data. + */ + public function lti_get_ims_role_provider() { + return [ + 'Student, LTI 1.1, no role switch' => [ + 'islti2' => false, + 'rolename' => 'student', + 'switchedto' => null, + 'expected' => 'Learner' + ], + 'Student, LTI 2.0, no role switch' => [ + 'islti2' => true, + 'rolename' => 'student', + 'switchedto' => null, + 'expected' => 'Learner' + ], + 'Teacher, LTI 1.1, no role switch' => [ + 'islti2' => false, + 'rolename' => 'editingteacher', + 'switchedto' => null, + 'expected' => 'Instructor' + ], + 'Teacher, LTI 2.0, no role switch' => [ + 'islti2' => true, + 'rolename' => 'editingteacher', + 'switchedto' => null, + 'expected' => 'Instructor' + ], + 'Admin, LTI 1.1, no role switch' => [ + 'islti2' => false, + 'rolename' => 'admin', + 'switchedto' => null, + 'expected' => 'Instructor,urn:lti:sysrole:ims/lis/Administrator,urn:lti:instrole:ims/lis/Administrator' + ], + 'Admin, LTI 2.0, no role switch' => [ + 'islti2' => true, + 'rolename' => 'admin', + 'switchedto' => null, + 'expected' => 'Instructor,http://purl.imsglobal.org/vocab/lis/v2/person#Administrator' + ], + 'Admin, LTI 1.1, role switch student' => [ + 'islti2' => false, + 'rolename' => 'admin', + 'switchedto' => 'student', + 'expected' => 'Learner' + ], + 'Admin, LTI 2.0, role switch student' => [ + 'islti2' => true, + 'rolename' => 'admin', + 'switchedto' => 'student', + 'expected' => 'Learner' + ], + 'Admin, LTI 1.1, role switch teacher' => [ + 'islti2' => false, + 'rolename' => 'admin', + 'switchedto' => 'editingteacher', + 'expected' => 'Instructor' + ], + 'Admin, LTI 2.0, role switch teacher' => [ + 'islti2' => true, + 'rolename' => 'admin', + 'switchedto' => 'editingteacher', + 'expected' => 'Instructor' + ], + ]; + } + /** * Create an LTI Tool. *