Merge pull request #426 from oat-sa/fix/AUT-3968/bad-request-status-code

Fix/aut 3968/bad request status code
oat-sa · Dec 4, 2024 · 108473a · 108473a
2 parents fead0a3 + f7b20f0
commit 108473a
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 16 deletions.
diff --git a/composer.json b/composer.json
@@ -60,7 +60,7 @@
         "guzzlehttp/guzzle": "^6.5 || ^7.0",
         "oat-sa/oatbox-extension-installer": "~1.1||dev-master",
         "oat-sa/lib-lti1p3-ags": "~2",
-        "oat-sa/lib-lti1p3-core": "~7",
+        "oat-sa/lib-lti1p3-core": "~7.1",
         "oat-sa/generis": ">=16.0.0",
         "oat-sa/tao-core": ">=54.26.0"
     },

diff --git a/controller/Security.php b/controller/Security.php
@@ -21,6 +21,7 @@
 namespace oat\taoLti\controller;
 
 use League\OAuth2\Server\Exception\OAuthServerException;
+use OAT\Library\Lti1p3Core\Exception\LtiBadRequestException;
 use OAT\Library\Lti1p3Core\Registration\RegistrationRepositoryInterface;
 use OAT\Library\Lti1p3Core\Security\Key\KeyChainRepositoryInterface;
 use OAT\Library\Lti1p3Core\Security\OAuth2\Factory\AuthorizationServerFactory;
@@ -29,14 +30,14 @@
 use OAT\Library\Lti1p3Core\Security\Oidc\OidcInitiator;
 use oat\tao\model\http\Controller;
 use oat\tao\model\security\Business\Contract\JwksRepositoryInterface;
-use oat\taoLti\models\classes\Platform\Repository\Lti1p3RegistrationRepository;
 use oat\taoLti\models\classes\Platform\Service\Oidc\OidcLoginAuthenticatorInterface;
 use oat\taoLti\models\classes\Platform\Service\Oidc\OidcLoginAuthenticatorProxy;
 use oat\taoLti\models\classes\Security\DataAccess\Repository\CachedPlatformJwksRepository;
 use oat\taoLti\models\classes\Security\DataAccess\Repository\CachedPlatformKeyChainRepository;
 use oat\taoLti\models\classes\Security\DataAccess\Repository\PlatformKeyChainRepository;
 use Zend\ServiceManager\ServiceLocatorAwareInterface;
 use Zend\ServiceManager\ServiceLocatorAwareTrait;
+use common_exception_BadRequest;
 
 use function GuzzleHttp\Psr7\stream_for;
 
@@ -70,23 +71,31 @@ public function jwks(): void
 
     public function oidc(): void
     {
-        $response = $this->getOidcLoginAuthenticator()
-            ->authenticate($this->getPsrRequest(), $this->getPsrResponse());
+        try {
+            $response = $this->getOidcLoginAuthenticator()
+                ->authenticate($this->getPsrRequest(), $this->getPsrResponse());
 
-        $this->setResponse($response);
+            $this->setResponse($response);
+        } catch (LtiBadRequestException $exception) {
+            throw new common_exception_BadRequest($exception->getMessage());
+        }
     }
 
     public function oidcInitiation(): void
     {
-        // Create the OIDC initiator
-        $initiator = new OidcInitiator(
-            $this->getPsrContainer()->get(RegistrationRepositoryInterface::class)
-        );
+        try {
+            // Create the OIDC initiator
+            $initiator = new OidcInitiator(
+                $this->getPsrContainer()->get(RegistrationRepositoryInterface::class)
+            );
 
-        // Perform the OIDC initiation (including state generation)
-        $message = $initiator->initiate($this->getPsrRequest());
+            // Perform the OIDC initiation (including state generation)
+            $message = $initiator->initiate($this->getPsrRequest());
 
-        $this->redirect($message->toUrl());
+            $this->redirect($message->toUrl());
+        } catch (LtiBadRequestException $exception) {
+            throw new common_exception_BadRequest($exception->getMessage());
+        }
     }
 
     private function getKeyChainRepository(): KeyChainRepositoryInterface

diff --git a/models/classes/LtiReturnResponse.php b/models/classes/LtiReturnResponse.php
@@ -36,6 +36,8 @@
  */
 class LtiReturnResponse extends ResponseAbstract
 {
+    protected $httpCode = 302;
+
     /**
      * @var LtiException
      */
@@ -64,7 +66,7 @@ public function __construct(Renderer $renderer)
      */
     public function setHttpCode($code)
     {
-        $this->httpCode = 302;
+        $this->httpCode = $code;
         return $this;
     }
 
@@ -81,7 +83,7 @@ public function send()
             if ($this->requiresRedirect() && !empty($this->getReturnBaseUrl())) {
                 $this->errorRedirectResponse();
             } else {
-                echo $this->showLtiErrorPage();
+                $this->responseWithCode(400, $this->showLtiErrorPage());
             }
         } catch (\Exception $e) {
             $this->renderer->setTemplate(Template::getTemplate('error/error500.tpl', 'tao'));
@@ -119,7 +121,7 @@ protected function getLtiErrorMessage()
      * @throws LtiVariableMissingException
      * @throws \common_Exception
      */
-    protected function showLtiErrorPage()
+    protected function showLtiErrorPage(): string
     {
         if (isset($this->requestParams[LtiLaunchData::TOOL_CONSUMER_INSTANCE_NAME])) {
             $this->renderer->setData(
@@ -160,7 +162,7 @@ protected function showLtiErrorPage()
      * @throws LtiVariableMissingException
      * @throws \common_Exception
      */
-    protected function renderLtiErrorPage(LtiException $error, $returnLink = true)
+    protected function renderLtiErrorPage(LtiException $error, $returnLink = true): string
     {
         // In regard of the IMS LTI standard, we have to show a back button that refer to the
         // launch_presentation_return_url url param. So we have to retrieve this parameter before trying to start
@@ -245,4 +247,12 @@ private function ltiRedirect($url, $statusCode = 302)
         header(HTTPToolkit::statusCodeHeader($statusCode));
         header(HTTPToolkit::locationHeader($url));
     }
+
+    private function responseWithCode(int $statusCode, string $data, string $contentType = 'text/html'): void
+    {
+        $this->setHttpCode($statusCode);
+        $this->contentType = $contentType;
+        $this->sendHeaders();
+        echo $data;
+    }
 }