From d2af7e4edb452f030e2dd2a0bb54dfb9d8705ad8 Mon Sep 17 00:00:00 2001 From: Nikita Pimenov Date: Wed, 20 Nov 2024 22:24:28 +0400 Subject: [PATCH 1/3] feat: throw separate exception type for bad request --- src/Exception/LtiBadRequestException.php | 29 +++++++++++++++++++ src/Security/Oidc/OidcAuthenticator.php | 9 ++++-- .../Security/Oidc/OidcAuthenticatorTest.php | 15 ++++++++-- 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 src/Exception/LtiBadRequestException.php diff --git a/src/Exception/LtiBadRequestException.php b/src/Exception/LtiBadRequestException.php new file mode 100644 index 00000000..4a10f1bd --- /dev/null +++ b/src/Exception/LtiBadRequestException.php @@ -0,0 +1,29 @@ +getParameters()->has('lti_message_hint')) { + throw new LtiBadRequestException('Missing LTI message hint in request'); + } + $originalToken = $this->parser->parse($oidcRequest->getParameters()->get('lti_message_hint')); $registration = $this->repository->find( @@ -87,11 +92,11 @@ public function authenticate(ServerRequestInterface $request): LtiMessageInterfa ); if (null === $registration) { - throw new LtiException('Invalid message hint registration id claim'); + throw new LtiBadRequestException('Invalid message hint registration id claim'); } if (!$this->validator->validate($originalToken, $registration->getPlatformKeyChain()->getPublicKey())) { - throw new LtiException('Invalid message hint'); + throw new LtiBadRequestException('Invalid message hint'); } $authenticationResult = $this->authenticator->authenticate( diff --git a/tests/Unit/Security/Oidc/OidcAuthenticatorTest.php b/tests/Unit/Security/Oidc/OidcAuthenticatorTest.php index 56ad2c35..bf93c8dc 100644 --- a/tests/Unit/Security/Oidc/OidcAuthenticatorTest.php +++ b/tests/Unit/Security/Oidc/OidcAuthenticatorTest.php @@ -24,6 +24,7 @@ use Carbon\Carbon; use Exception; +use OAT\Library\Lti1p3Core\Exception\LtiBadRequestException; use OAT\Library\Lti1p3Core\Exception\LtiException; use OAT\Library\Lti1p3Core\Message\LtiMessageInterface; use OAT\Library\Lti1p3Core\Message\Payload\LtiMessagePayloadInterface; @@ -165,7 +166,7 @@ public function testAuthenticationSuccessWithUserIdentityClaimsSanitization(): v public function testAuthenticationFailureOnExpiredMessageHint(): void { - $this->expectException(LtiException::class); + $this->expectException(LtiBadRequestException::class); $this->expectExceptionMessage('Invalid message hint'); $registration = $this->createTestRegistration(); @@ -198,9 +199,19 @@ public function testAuthenticationFailureOnExpiredMessageHint(): void $this->subject->authenticate($request); } + public function testAuthenticationFailureOnMissingMessageHint(): void + { + $this->expectException(LtiBadRequestException::class); + $this->expectExceptionMessage('Missing LTI message hint in request'); + + $request = $this->createServerRequest('GET','http://platform.com/init'); + + $this->subject->authenticate($request); + } + public function testAuthenticationFailureOnRegistrationNotFound(): void { - $this->expectException(LtiException::class); + $this->expectException(LtiBadRequestException::class); $this->expectExceptionMessage('Invalid message hint registration id claim'); $registration = $this->createTestRegistration(); From 2a7a9a0607b67d66cdcc4f41ab142cec3cba2d69 Mon Sep 17 00:00:00 2001 From: Nikita Pimenov Date: Thu, 21 Nov 2024 15:58:08 +0400 Subject: [PATCH 2/3] feat: OIDC initiator throw separate exception type for bad request --- src/Security/Oidc/OidcInitiator.php | 8 ++++++++ tests/Unit/Security/Oidc/OidcInitiatorTest.php | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/Security/Oidc/OidcInitiator.php b/src/Security/Oidc/OidcInitiator.php index 7a35b77a..25c9a5d4 100644 --- a/src/Security/Oidc/OidcInitiator.php +++ b/src/Security/Oidc/OidcInitiator.php @@ -22,6 +22,8 @@ namespace OAT\Library\Lti1p3Core\Security\Oidc; +use InvalidArgumentException; +use OAT\Library\Lti1p3Core\Exception\LtiBadRequestException; use OAT\Library\Lti1p3Core\Exception\LtiException; use OAT\Library\Lti1p3Core\Exception\LtiExceptionInterface; use OAT\Library\Lti1p3Core\Message\LtiMessage; @@ -121,6 +123,12 @@ public function initiate(ServerRequestInterface $request): LtiMessageInterface } catch (LtiExceptionInterface $exception) { throw $exception; + } catch (InvalidArgumentException $exception) { + throw new LtiBadRequestException( + sprintf('OIDC initiation request failed: %s', $exception->getMessage()), + $exception->getCode(), + $exception + ); } catch (Throwable $exception) { throw new LtiException( sprintf('OIDC initiation failed: %s', $exception->getMessage()), diff --git a/tests/Unit/Security/Oidc/OidcInitiatorTest.php b/tests/Unit/Security/Oidc/OidcInitiatorTest.php index de8362a0..15865d66 100644 --- a/tests/Unit/Security/Oidc/OidcInitiatorTest.php +++ b/tests/Unit/Security/Oidc/OidcInitiatorTest.php @@ -23,6 +23,7 @@ namespace OAT\Library\Lti1p3Core\Tests\Unit\Security\Oidc; use Exception; +use OAT\Library\Lti1p3Core\Exception\LtiBadRequestException; use OAT\Library\Lti1p3Core\Exception\LtiException; use OAT\Library\Lti1p3Core\Registration\RegistrationInterface; use OAT\Library\Lti1p3Core\Registration\RegistrationRepositoryInterface; @@ -150,4 +151,14 @@ public function testInitiationFailureOnGenericError(): void $this->subject->initiate($request); } + + public function testInitiationFailureOnMissingMandatoryRequestParams(): void + { + $this->expectException(LtiBadRequestException::class); + $this->expectExceptionMessage('OIDC initiation request failed: Missing mandatory iss'); + + $request = $this->createServerRequest('GET','http://tool.com/init'); + + $this->subject->initiate($request); + } } From e8c481940f423690031707f83efec85c029f734a Mon Sep 17 00:00:00 2001 From: Nikita Pimenov Date: Thu, 21 Nov 2024 16:51:32 +0400 Subject: [PATCH 3/3] refactor: inherit LtiException for backward compatibility --- src/Exception/LtiBadRequestException.php | 4 +--- src/Security/Oidc/OidcAuthenticator.php | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Exception/LtiBadRequestException.php b/src/Exception/LtiBadRequestException.php index 4a10f1bd..bf207eb1 100644 --- a/src/Exception/LtiBadRequestException.php +++ b/src/Exception/LtiBadRequestException.php @@ -22,8 +22,6 @@ namespace OAT\Library\Lti1p3Core\Exception; -use Exception; - -class LtiBadRequestException extends Exception implements LtiExceptionInterface +class LtiBadRequestException extends LtiException implements LtiExceptionInterface { } diff --git a/src/Security/Oidc/OidcAuthenticator.php b/src/Security/Oidc/OidcAuthenticator.php index c3eb05c6..800d957d 100644 --- a/src/Security/Oidc/OidcAuthenticator.php +++ b/src/Security/Oidc/OidcAuthenticator.php @@ -85,7 +85,7 @@ public function authenticate(ServerRequestInterface $request): LtiMessageInterfa throw new LtiBadRequestException('Missing LTI message hint in request'); } - $originalToken = $this->parser->parse($oidcRequest->getParameters()->get('lti_message_hint')); + $originalToken = $this->parser->parse($oidcRequest->getParameters()->getMandatory('lti_message_hint')); $registration = $this->repository->find( $originalToken->getClaims()->getMandatory(LtiMessagePayloadInterface::CLAIM_REGISTRATION_ID)