diff --git a/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php b/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php index c390865e7c7..4a7de30821f 100644 --- a/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php +++ b/enrol/lti/classes/local/ltiadvantage/service/tool_launch_service.php @@ -87,7 +87,9 @@ class tool_launch_service { $launchdata = $launch->getLaunchData(); $data = [ 'platform' => $launchdata['iss'], - 'clientid' => $launchdata['aud'], // See LTI_Message_Launch::validate_registration for details about aud. + // The 'aud' property may be an array with one or more values, but can be a string if there is only one value. + // https://www.imsglobal.org/spec/security/v1p1#id-token. + 'clientid' => is_array($launchdata['aud']) ? $launchdata['aud'][0] : $launchdata['aud'], 'exp' => $launchdata['exp'], 'nonce' => $launchdata['nonce'], 'sub' => $launchdata['sub'], diff --git a/enrol/lti/tests/local/ltiadvantage/lti_advantage_testcase.php b/enrol/lti/tests/local/ltiadvantage/lti_advantage_testcase.php index f64ee6c9aaf..31535c9e5d2 100644 --- a/enrol/lti/tests/local/ltiadvantage/lti_advantage_testcase.php +++ b/enrol/lti/tests/local/ltiadvantage/lti_advantage_testcase.php @@ -130,11 +130,12 @@ abstract class lti_advantage_testcase extends \advanced_testcase { * @param bool $nrps whether to include a mock NRPS claim or not. * @param array|null $migrationclaiminfo contains consumer key, secret and any fields which are sent in the claim. * @param array|null $customparams an array of custom params to send, or null to just use defaults. + * @param mixed $aud the array or string value of aud to use in the mock launch data. * @return LtiMessageLaunch the mock launch object with test launch data. */ protected function get_mock_launch(\stdClass $resource, array $mockuser, ?string $resourcelinkid = null, bool $ags = true, bool $nrps = true, ?array $migrationclaiminfo = null, - ?array $customparams = null): LtiMessageLaunch { + ?array $customparams = null, $aud = '123'): LtiMessageLaunch { $mocklaunch = $this->getMockBuilder(LtiMessageLaunch::class) ->onlyMethods(['getLaunchData']) @@ -144,14 +145,14 @@ abstract class lti_advantage_testcase extends \advanced_testcase { ->method('getLaunchData') ->will($this->returnCallback( function() - use ($resource, $mockuser, $resourcelinkid, $migrationclaiminfo, $ags, $nrps, $customparams) { + use ($resource, $mockuser, $resourcelinkid, $migrationclaiminfo, $ags, $nrps, $customparams, $aud) { // This simulates the data in the jwt['body'] of a real resource link launch. // Real launches would of course have this data and authenticity of the user verified. $rltitle = $resourcelinkid ? "Resource link $resourcelinkid in platform" : "Resource link in platform"; $rlid = $resourcelinkid ?: '12345'; $data = [ 'iss' => $this->issuer, // Must match registration in create_test_environment. - 'aud' => '123', // Must match registration in create_test_environment. + 'aud' => $aud, // Must match registration in create_test_environment. 'sub' => $mockuser['user_id'], // User id on the platform site. 'exp' => time() + 60, 'nonce' => 'some-nonce-value-123', diff --git a/enrol/lti/tests/local/ltiadvantage/service/tool_launch_service_test.php b/enrol/lti/tests/local/ltiadvantage/service/tool_launch_service_test.php index d897859d18c..340680f50ad 100644 --- a/enrol/lti/tests/local/ltiadvantage/service/tool_launch_service_test.php +++ b/enrol/lti/tests/local/ltiadvantage/service/tool_launch_service_test.php @@ -520,4 +520,92 @@ class tool_launch_service_test extends \lti_advantage_testcase { $launchservice->user_launches_tool($learneruser, $mocklearnerlaunch); $this->assertEquals('embedded', $SESSION->forcepagelayout); } + + /** + * Test launching the tool with different 'aud' values, confirming the service handles all variations appropriately. + * + * @param mixed $aud the aud value to test + * @param array $expected the array of expectations to check + * @dataProvider aud_data_provider + * @covers ::user_launches_tool + */ + public function test_user_launches_tool_aud_variations($aud, array $expected) { + $this->resetAfterTest(); + [$course, $modresource] = $this->create_test_environment(); + $instructoruser = $this->getDataGenerator()->create_user(); + $mockinstructoruser = $this->get_mock_launch_users_with_ids(['1'])[0]; + $mockinstructorlaunch = $this->get_mock_launch($modresource, $mockinstructoruser, null, false, false, null, [ + 'id' => $modresource->uuid, + ], $aud); + + $launchservice = $this->get_tool_launch_service(); + if (isset($expected['exception'])) { + $this->expectException($expected['exception']); + $this->expectExceptionMessage($expected['exceptionmessage']); + } + [$userid, $resource] = $launchservice->user_launches_tool($instructoruser, $mockinstructorlaunch); + + $this->assertNotEmpty($userid); + $this->assertNotEmpty($resource); + } + + /** + * Data provider for testing variations of the 'aud' JWT property. + * + * @return array the test case data + */ + public function aud_data_provider(): array { + return [ + 'valid, array having multiple entries with the first one being clientid' => [ + 'aud' => ['123', 'something else', 'blah'], + 'expected' => [ + 'valid' => true + ] + ], + 'valid, array having a single entry being clientid' => [ + 'aud' => ['123'], + 'expected' => [ + 'valid' => true + ] + ], + 'valid, string containing the single clientid' => [ + 'aud' => '123', + 'expected' => [ + 'valid' => true + ] + ], + 'invalid, array having multiple values where the first item is not clientid' => [ + 'aud' => ['cat', 'dog', '123'], + 'expected' => [ + 'valid' => false, + 'exception' => \moodle_exception::class, + 'exceptionmessage' => get_string('ltiadvlauncherror:invalidregistration', 'enrol_lti') + ] + ], + 'invalid, array containing a single item which is not clientid' => [ + 'aud' => ['cat'], + 'expected' => [ + 'valid' => false, + 'exception' => \moodle_exception::class, + 'exceptionmessage' => get_string('ltiadvlauncherror:invalidregistration', 'enrol_lti') + ] + ], + 'invalid, string contains the item and it is not the clientid' => [ + 'aud' => 'cat', + 'expected' => [ + 'valid' => false, + 'exception' => \moodle_exception::class, + 'exceptionmessage' => get_string('ltiadvlauncherror:invalidregistration', 'enrol_lti') + ] + ], + 'invalid, empty string' => [ + 'aud' => '', + 'expected' => [ + 'valid' => false, + 'exception' => \moodle_exception::class, + 'exceptionmessage' => get_string('ltiadvlauncherror:invalidregistration', 'enrol_lti') + ] + ], + ]; + } }