From 360a7a746b6e8e6e078e7a65be9ffb3c63b2baee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 6 Dec 2023 16:42:49 +0100 Subject: [PATCH 01/11] Add missing roles for http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor Add new role: http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator --- install/ontology/ltiroles_membership.rdf | 2 +- install/ontology/ltiroles_person.rdf | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/install/ontology/ltiroles_membership.rdf b/install/ontology/ltiroles_membership.rdf index d00faed8..3f0aa2dd 100644 --- a/install/ontology/ltiroles_membership.rdf +++ b/install/ontology/ltiroles_membership.rdf @@ -191,7 +191,7 @@ - + diff --git a/install/ontology/ltiroles_person.rdf b/install/ontology/ltiroles_person.rdf index 76353420..e2daf96b 100755 --- a/install/ontology/ltiroles_person.rdf +++ b/install/ontology/ltiroles_person.rdf @@ -43,4 +43,13 @@ + + + + + + + + + From 870e922913a77e8a1785795c076e7959c029718a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 6 Dec 2023 16:43:13 +0100 Subject: [PATCH 02/11] fix: Add missing constants --- models/classes/LtiRoles.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/classes/LtiRoles.php b/models/classes/LtiRoles.php index b3210baa..9adf0a1b 100644 --- a/models/classes/LtiRoles.php +++ b/models/classes/LtiRoles.php @@ -110,4 +110,6 @@ interface LtiRoles public const CONTEXT_LTI1P3_ADMINISTRATOR_SUB_EXTERNAL_SYSTEM_ADMINISTRATOR = 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#ExternalSystemAdministrator'; public const CONTEXT_LTI1P3_ADMINISTRATOR_SUB_SUPPORT = 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#Support'; public const CONTEXT_LTI1P3_ADMINISTRATOR_SUB_SYSTEM_ADMINISTRATOR = 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#SystemAdministrator'; + + public const CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR = 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator'; } From 8322c5c6abec8452de616dc1643eb2bccad5eb94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 6 Dec 2023 16:45:02 +0100 Subject: [PATCH 03/11] fix: missing return type --- models/classes/LtiLaunchData.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/classes/LtiLaunchData.php b/models/classes/LtiLaunchData.php index 069e0c8d..3b272654 100644 --- a/models/classes/LtiLaunchData.php +++ b/models/classes/LtiLaunchData.php @@ -579,7 +579,7 @@ public function getReturnUrl() * which is a value of any type other than a resource. * @since 5.4.0 */ - public function jsonSerialize() + public function jsonSerialize(): array { return [ 'variables' => array_map( From b82136584f670d189745c40dd626489bcdef611d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 6 Dec 2023 16:45:41 +0100 Subject: [PATCH 04/11] feat: new service to filter and assign role --- .../Version202312051317263774_taoLti.php | 82 +++++++++++++++++++ .../ServiceProvider/LtiServiceProvider.php | 21 +++++ .../Tool/Service/AuthoringLtiRoleService.php | 36 ++++++++ 3 files changed, 139 insertions(+) create mode 100644 migrations/Version202312051317263774_taoLti.php create mode 100644 models/classes/Tool/Service/AuthoringLtiRoleService.php diff --git a/migrations/Version202312051317263774_taoLti.php b/migrations/Version202312051317263774_taoLti.php new file mode 100644 index 00000000..fd956482 --- /dev/null +++ b/migrations/Version202312051317263774_taoLti.php @@ -0,0 +1,82 @@ +getLaunchActionRule()); + AclProxy::applyRule($this->getRunActionRule(LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR)); + AclProxy::applyRule($this->getRunActionRule(LtiRoles::CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR)); + + $this->addReport( + Report::createInfo( + sprintf( + 'Clearing the Generis cache for roles %s', + LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR, + ) + ) + ); + core_kernel_users_Cache::removeIncludedRoles( + new core_kernel_classes_Resource(LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR) + ); + core_kernel_users_Cache::removeIncludedRoles( + new core_kernel_classes_Resource(LtiRoles::CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR) + ); + + OntologyUpdater::syncModels(); + + $this->addReport(Report::createInfo('Apply new permission for AuthoringTool')); + } + + public function down(Schema $schema): void + { + AclProxy::revokeRule($this->getLaunchActionRule()); + AclProxy::revokeRule($this->getRunActionRule(LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR)); + AclProxy::revokeRule($this->getRunActionRule(LtiRoles::CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR)); + + $this->addReport(Report::createInfo('Revoke CONTEXT_INSTRUCTOR, CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR permission for AuthoringTool')); + } + + private function getLaunchActionRule(): AccessRule + { + return new AccessRule( + AccessRule::GRANT, + TaoRoles::ANONYMOUS, + ['ext' => 'taoLti', 'mod' => 'AuthoringTool', 'act' => 'launch'] + ); + } + + private function getRunActionRule(string $role): AccessRule + { + return new AccessRule( + AccessRule::GRANT, + $role, + ['ext' => 'taoLti', 'mod' => 'AuthoringTool', 'act' => 'run'] + ); + } +} diff --git a/models/classes/ServiceProvider/LtiServiceProvider.php b/models/classes/ServiceProvider/LtiServiceProvider.php index 028ba091..3f8a6725 100644 --- a/models/classes/ServiceProvider/LtiServiceProvider.php +++ b/models/classes/ServiceProvider/LtiServiceProvider.php @@ -50,6 +50,7 @@ use oat\taoLti\models\classes\Client\LtiClientFactory; use oat\taoLti\models\classes\LtiAgs\LtiAgsScoreService; use oat\taoLti\models\classes\LtiAgs\LtiAgsScoreServiceInterface; +use oat\taoLti\models\classes\LtiRoles; use oat\taoLti\models\classes\Platform\Repository\DefaultToolConfig; use oat\taoLti\models\classes\Platform\Repository\Lti1p3RegistrationRepository; use oat\taoLti\models\classes\Platform\Repository\Lti1p3RegistrationSnapshotRepository; @@ -57,6 +58,7 @@ use oat\taoLti\models\classes\Platform\Service\UpdatePlatformRegistrationSnapshotListener; use oat\taoLti\models\classes\Security\DataAccess\Repository\CachedPlatformKeyChainRepository; use oat\taoLti\models\classes\Security\DataAccess\Repository\PlatformKeyChainRepository; +use oat\taoLti\models\classes\Tool\Service\AuthoringLtiRoleService; use oat\taoLti\models\classes\Tool\Validation\AuthoringToolValidator; use oat\taoLti\models\classes\Tool\Validation\Lti1p3Validator; use Psr\Cache\CacheItemPoolInterface; @@ -79,6 +81,16 @@ public function __invoke(ContainerConfigurator $configurator): void $_ENV['LTI_DEFAULT_SCOPE'] ?? 'https://purl.imsglobal.org/spec/lti-bo/scope/basicoutcome' ); + $parameters->set( + 'rolesAllowed', + [ + LtiRoles::CONTEXT_LTI1P3_ADMINISTRATOR_SUB_DEVELOPER, + LtiRoles::CONTEXT_LTI1P3_CONTENT_DEVELOPER_SUB_CONTENT_DEVELOPER, + LTIRoles::CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR, + LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR + ] + ); + $services ->set(LtiClientFactory::class) ->args( @@ -238,5 +250,14 @@ public function __invoke(ContainerConfigurator $configurator): void service(AuthoringToolValidator::class), ] ); + + $services + ->set(AuthoringLtiRoleService::class, AuthoringLtiRoleService::class) + ->public() + ->args( + [ + param('rolesAllowed') + ] + ); } } diff --git a/models/classes/Tool/Service/AuthoringLtiRoleService.php b/models/classes/Tool/Service/AuthoringLtiRoleService.php new file mode 100644 index 00000000..996596d6 --- /dev/null +++ b/models/classes/Tool/Service/AuthoringLtiRoleService.php @@ -0,0 +1,36 @@ +roleAllowed = $roleAllowed; + } + + public function getValidRole(array $roles): string + { + return current(array_intersect($roles, $this->roleAllowed)); + } +} From c8d98a46fa8888a028cd5018308cfc362ed57b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Mon, 11 Dec 2023 11:16:35 +0100 Subject: [PATCH 05/11] feat: add exception, add unit test --- controller/AuthoringTool.php | 2 + .../Tool/Exception/WrongLtiRolesException.php | 34 +++++++ .../Tool/Service/AuthoringLtiRoleService.php | 12 ++- .../Service/AuthoringLtiRoleServiceTest.php | 95 +++++++++++++++++++ 4 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 models/classes/Tool/Exception/WrongLtiRolesException.php create mode 100644 test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php diff --git a/controller/AuthoringTool.php b/controller/AuthoringTool.php index f4cd62db..1f0e0a2d 100644 --- a/controller/AuthoringTool.php +++ b/controller/AuthoringTool.php @@ -79,6 +79,7 @@ protected function getValidatedLtiMessagePayload(): LtiMessagePayloadInterface * @throws ContainerExceptionInterface * @throws NotFoundExceptionInterface * @throws common_exception_Error + * @throws WrongLtiRolesException */ public function launch(): void { @@ -92,6 +93,7 @@ public function launch(): void helpers_Random::generateString(UserService::PASSWORD_LENGTH), new core_kernel_classes_Resource(current($ltiMessage->getRoles())) ); + $this->getServiceLocator() ->getContainer() ->get(LtiService::class) diff --git a/models/classes/Tool/Exception/WrongLtiRolesException.php b/models/classes/Tool/Exception/WrongLtiRolesException.php new file mode 100644 index 00000000..d49682ed --- /dev/null +++ b/models/classes/Tool/Exception/WrongLtiRolesException.php @@ -0,0 +1,34 @@ +roleAllowed = $roleAllowed; } + /** + * @throws WrongLtiRolesException + */ public function getValidRole(array $roles): string { - return current(array_intersect($roles, $this->roleAllowed)); + $commonRoles = array_intersect($roles, $this->roleAllowed); + + if (empty($commonRoles)) { + throw new WrongLtiRolesException(); + } + return current($commonRoles); } } diff --git a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php new file mode 100644 index 00000000..a19b057f --- /dev/null +++ b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php @@ -0,0 +1,95 @@ +subject = new AuthoringLtiRoleService( + [ + LtiRoles::CONTEXT_LTI1P3_ADMINISTRATOR_SUB_DEVELOPER, + LtiRoles::CONTEXT_LTI1P3_CONTENT_DEVELOPER_SUB_CONTENT_DEVELOPER, + LTIRoles::CONTEXT_INSTITUTION_LTI1P3_ADMINISTRATOR, + LtiRoles::CONTEXT_LTI1P3_INSTRUCTOR + ] + ); + } + + /** + * @dataProvider ltiMessageRolesProvider + */ + public function testValidRole(array $rolesProvided, string $expected): void + { + self::assertEquals($expected, $this->subject->getValidRole($rolesProvided)); + } + + /** + * @dataProvider invalidRolesProvider + * @throws WrongLtiRolesException + */ + public function testExpectException(array $roles): void + { + $this->expectException(WrongLtiRolesException::class); + $this->subject->getValidRole($roles); + } + + private function invalidRolesProvider(): array + { + return [ + 'Empty array' => [ + 'roles' => [], + ], + 'UnsupportedRole' => [ + 'roles' => ['http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#Support'] + ] + ]; + } + + private + function ltiMessageRolesProvider(): array + { + return [ + 'When one valid roles' => [ + 'rolesProvided' => [ + 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator', + 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#Support' + ], + 'expected' => 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator' + ], + 'When more then one valid roles' => [ + 'rolesProvided' => [ + 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator', + 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#Support', + 'http://purl.imsglobal.org/vocab/lis/v2/membership/ContentDeveloper#ContentDeveloper' + ], + 'expected' => 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator' + ] + ]; + } +} From d8338221c1ccc45c24c3c4fd76e16cd423e4253b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Mon, 11 Dec 2023 17:01:08 +0100 Subject: [PATCH 06/11] fix: broken function --- .../classes/Tool/Service/AuthoringLtiRoleServiceTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php index a19b057f..03ec06a9 100644 --- a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php +++ b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php @@ -71,8 +71,7 @@ private function invalidRolesProvider(): array ]; } - private - function ltiMessageRolesProvider(): array + private function ltiMessageRolesProvider(): array { return [ 'When one valid roles' => [ From 2924154ace24fb2b5f8e28ab8d2366e1b4276633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Mon, 11 Dec 2023 17:51:53 +0100 Subject: [PATCH 07/11] fix: test method data provider protected --- .../classes/Tool/Service/AuthoringLtiRoleServiceTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php index 03ec06a9..c84f4e6c 100644 --- a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php +++ b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php @@ -59,7 +59,7 @@ public function testExpectException(array $roles): void $this->subject->getValidRole($roles); } - private function invalidRolesProvider(): array + protected function invalidRolesProvider(): array { return [ 'Empty array' => [ @@ -71,7 +71,7 @@ private function invalidRolesProvider(): array ]; } - private function ltiMessageRolesProvider(): array + protected function ltiMessageRolesProvider(): array { return [ 'When one valid roles' => [ From 1cecc1edfc64feec348dfff173241fb51f99cb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 3 Jan 2024 11:29:25 +0100 Subject: [PATCH 08/11] fix: extract method --- controller/AuthoringTool.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/controller/AuthoringTool.php b/controller/AuthoringTool.php index 1f0e0a2d..acaeb3fe 100644 --- a/controller/AuthoringTool.php +++ b/controller/AuthoringTool.php @@ -33,6 +33,8 @@ use oat\taoLti\models\classes\LtiException; use oat\taoLti\models\classes\LtiMessages\LtiErrorMessage; use oat\taoLti\models\classes\LtiService; +use oat\taoLti\models\classes\Tool\Exception\WrongLtiRolesException; +use oat\taoLti\models\classes\Tool\Service\AuthoringLtiRoleService; use oat\taoLti\models\classes\Tool\Validation\Lti1p3Validator; use oat\taoLti\models\classes\user\UserService; use Psr\Container\ContainerExceptionInterface; @@ -91,7 +93,9 @@ public function launch(): void ->addUser( $ltiMessage->getUserIdentity()->getIdentifier(), helpers_Random::generateString(UserService::PASSWORD_LENGTH), - new core_kernel_classes_Resource(current($ltiMessage->getRoles())) + new core_kernel_classes_Resource( + $this->getAuthoringRoleService()->getValidRole($ltiMessage->getRoles()) + ) ); $this->getServiceLocator() @@ -147,4 +151,9 @@ private function getLtiMessageOrRedirectToLogin(): LtiMessagePayloadInterface return $message; } + + private function getAuthoringRoleService(): AuthoringLtiRoleService + { + return $this->getPsrContainer()->get(AuthoringLtiRoleService::class); + } } From 2bae62ff1884f39fb3191448389b1019a258e8f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 3 Jan 2024 11:29:49 +0100 Subject: [PATCH 09/11] fix: change method access --- .../classes/Tool/Service/AuthoringLtiRoleServiceTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php index c84f4e6c..d8ab7857 100644 --- a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php +++ b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php @@ -29,7 +29,7 @@ class AuthoringLtiRoleServiceTest extends TestCase { - protected function setUp(): void + public function setUp(): void { $this->subject = new AuthoringLtiRoleService( [ @@ -59,7 +59,7 @@ public function testExpectException(array $roles): void $this->subject->getValidRole($roles); } - protected function invalidRolesProvider(): array + public function invalidRolesProvider(): array { return [ 'Empty array' => [ @@ -71,7 +71,7 @@ protected function invalidRolesProvider(): array ]; } - protected function ltiMessageRolesProvider(): array + public function ltiMessageRolesProvider(): array { return [ 'When one valid roles' => [ From fddc540febd84019059f969ea9757eaf9e154f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Wed, 3 Jan 2024 11:30:10 +0100 Subject: [PATCH 10/11] fix: add missing return type --- models/classes/user/LtiUser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/classes/user/LtiUser.php b/models/classes/user/LtiUser.php index 154d3ec8..87418fb0 100755 --- a/models/classes/user/LtiUser.php +++ b/models/classes/user/LtiUser.php @@ -216,7 +216,7 @@ public function refresh() // nothing to do } - public function jsonSerialize() + public function jsonSerialize(): array { return [ self::USER_IDENTIFIER => $this->userUri, From 28b4f2bcf857c624578bdbe0d9bd172ee5c3ef0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Marsza=C5=82?= Date: Thu, 4 Jan 2024 10:22:46 +0100 Subject: [PATCH 11/11] fix: fix typo --- .../models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php index d8ab7857..76f259f7 100644 --- a/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php +++ b/test/unit/models/classes/Tool/Service/AuthoringLtiRoleServiceTest.php @@ -81,7 +81,7 @@ public function ltiMessageRolesProvider(): array ], 'expected' => 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator' ], - 'When more then one valid roles' => [ + 'When more than one valid roles' => [ 'rolesProvided' => [ 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator', 'http://purl.imsglobal.org/vocab/lis/v2/membership/Administrator#Support',