Merge pull request #26 from oat-sa/develop

Develop
oat-sa · Sep 23, 2021 · 0737187 · 0737187
2 parents dbdd5cc + 1e97208
commit 0737187
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.1.1
+-----
+
+* Fixed wrong response http status code to return a bad request when the provided operation xml is invalid
+
 4.1.0
 -----
 

diff --git a/src/Service/Server/Handler/BasicOutcomeServiceServerRequestHandler.php b/src/Service/Server/Handler/BasicOutcomeServiceServerRequestHandler.php
@@ -38,6 +38,9 @@
 use OAT\Library\Lti1p3Core\Service\Server\Handler\LtiServiceServerRequestHandlerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use Throwable;
 
 class BasicOutcomeServiceServerRequestHandler implements LtiServiceServerRequestHandlerInterface, BasicOutcomeServiceInterface
 {
@@ -56,18 +59,23 @@ class BasicOutcomeServiceServerRequestHandler implements LtiServiceServerRequest
     /** @var ResponseFactory */
     private $httpResponseFactory;
 
+    /** @var LoggerInterface */
+    private $logger;
+
     public function __construct(
         BasicOutcomeServiceServerProcessorInterface $processor,
         ?BasicOutcomeRequestSerializerInterface $basicOutcomeRequestSerializer = null,
         ?BasicOutcomeResponseSerializerInterface $basicOutcomeResponseSerializer = null,
         ?BasicOutcomeResponseFactoryInterface $basicOutcomeResponseFactory = null,
-        ?ResponseFactory $httpResponseFactory = null
+        ?ResponseFactory $httpResponseFactory = null,
+        ?LoggerInterface $logger = null
     ) {
         $this->processor = $processor;
         $this->basicOutcomeRequestSerializer = $basicOutcomeRequestSerializer ?? new BasicOutcomeRequestSerializer();
         $this->basicOutcomeResponseSerializer = $basicOutcomeResponseSerializer ?? new BasicOutcomeResponseSerializer();
         $this->basicOutcomeResponseFactory = $basicOutcomeResponseFactory ?? new BasicOutcomeResponseFactory();
         $this->httpResponseFactory = $httpResponseFactory ?? new HttplugFactory();
+        $this->logger = $logger ?? new NullLogger();
     }
 
     public function getServiceName(): string
@@ -100,7 +108,14 @@ public function handleValidatedServiceRequest(
         array $options = []
     ): ResponseInterface {
         $registration = $validationResult->getRegistration();
-        $basicOutcomeRequest = $this->basicOutcomeRequestSerializer->deserialize((string)$request->getBody());
+
+        try {
+            $basicOutcomeRequest = $this->basicOutcomeRequestSerializer->deserialize((string)$request->getBody());
+        } catch (Throwable $exception) {
+            $this->logger->error($exception->getMessage());
+
+            return $this->httpResponseFactory->createResponse(400, null, [], $exception->getMessage());
+        }
 
         switch ($basicOutcomeRequest->getType()) {
             case BasicOutcomeMessageInterface::TYPE_REPLACE_RESULT:

diff --git a/tests/Integration/Service/Server/Handler/BasicOutcomeServiceServerRequestHandlerTest.php b/tests/Integration/Service/Server/Handler/BasicOutcomeServiceServerRequestHandlerTest.php
@@ -83,7 +83,9 @@ protected function setUp(): void
             $this->processorMock,
             null,
             null,
-            new BasicOutcomeResponseFactory($this->generatorMock)
+            new BasicOutcomeResponseFactory($this->generatorMock),
+            null,
+            $this->logger
         );
 
         $this->server = new LtiServiceServer(
@@ -434,4 +436,44 @@ public function testHandlingFailureOnInvalidRequestContentType(): void
             )
         );
     }
+
+    public function testHandlingFailureOnInvalidRequestXml(): void
+    {
+        $registration = $this->createTestRegistration();
+        $validationResult = new RequestAccessTokenValidationResult($registration);
+
+        $request = new ServerRequest(
+            'POST',
+            'http://platform.com/basic-outcome',
+            [
+                'Content-Type' => BasicOutcomeServiceInterface::CONTENT_TYPE_BASIC_OUTCOME
+            ],
+            'invalid'
+        );
+
+        $this->validatorMock
+            ->expects($this->once())
+            ->method('validate')
+            ->with($request)
+            ->willReturn($validationResult);
+
+        $this->generatorMock
+            ->expects($this->never())
+            ->method('generate');
+
+        $response = $this->server->handle($request);
+
+        $this->assertEquals(400, $response->getStatusCode());
+        $this->assertEquals(
+            'Cannot deserialize basic outcome request: The current node list is empty.',
+            (string)$response->getBody()
+        );
+
+        $this->assertTrue(
+            $this->logger->hasLog(
+                'error',
+                'Cannot deserialize basic outcome request: The current node list is empty.'
+            )
+        );
+    }
 }