Merge pull request #195 from oat-sa/fix/AUT-3968/separate-exception-f…

…or-bad-request

feat: throw separate exception type for invalid request

src/Exception/LtiBadRequestException.php

@@ -0,0 +1,27 @@
+<?php
+
+/**
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; under version 2
+ * of the License (non-upgradable).
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ * Copyright (c) 2024 (original work) Open Assessment Technologies SA;
+ */
+
+declare(strict_types=1);
+
+namespace OAT\Library\Lti1p3Core\Exception;
+
+class LtiBadRequestException extends LtiException implements LtiExceptionInterface
+{
+}

src/Security/Oidc/OidcAuthenticator.php

@@ -23,6 +23,7 @@
 namespace OAT\Library\Lti1p3Core\Security\Oidc;
 
 use OAT\Library\Lti1p3Core\Exception\LtiException;
+use OAT\Library\Lti1p3Core\Exception\LtiBadRequestException;
 use OAT\Library\Lti1p3Core\Exception\LtiExceptionInterface;
 use OAT\Library\Lti1p3Core\Message\LtiMessage;
 use OAT\Library\Lti1p3Core\Message\LtiMessageInterface;
@@ -80,18 +81,22 @@ public function authenticate(ServerRequestInterface $request): LtiMessageInterfa
         try {
             $oidcRequest = LtiMessage::fromServerRequest($request);
 
-            $originalToken = $this->parser->parse($oidcRequest->getParameters()->get('lti_message_hint'));
+            if (!$oidcRequest->getParameters()->has('lti_message_hint')) {
+                throw new LtiBadRequestException('Missing LTI message hint in request');
+            }
+
+            $originalToken = $this->parser->parse($oidcRequest->getParameters()->getMandatory('lti_message_hint'));
 
             $registration = $this->repository->find(
                 $originalToken->getClaims()->getMandatory(LtiMessagePayloadInterface::CLAIM_REGISTRATION_ID)
             );
 
             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(

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()),

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;
@@ -170,7 +171,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();
@@ -203,9 +204,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();

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);
+    }
 }