From 19bfc8ad379744e76b72c3c737018e8ce5fe4815 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 23 May 2020 11:26:48 +0100
Subject: [PATCH] Prevented entity "Not Found" events from being logged

- Added testing to cover, which was more hassle than thought
  since Laravel did not have built in log test helpers, so:
- Added Log testing helper.

Related to #2110
---
 app/Config/logging.php      |  7 +++++++
 app/Exceptions/Handler.php  |  2 +-
 phpcs.xml                   |  1 +
 tests/ErrorTest.php         | 20 ++++++++++++++++++++
 tests/SharedTestHelpers.php | 31 ++++++++++++++++++++++++++-----
 5 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/app/Config/logging.php b/app/Config/logging.php
index 0b55dc24d..375e84083 100644
--- a/app/Config/logging.php
+++ b/app/Config/logging.php
@@ -77,6 +77,13 @@ return [
             'driver' => 'monolog',
             'handler' => NullHandler::class,
         ],
+
+        // Testing channel
+        // Uses a shared testing instance during tests
+        // so that logs can be checked against.
+        'testing' => [
+            'driver' => 'testing',
+        ],
     ],
 
 ];
diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php
index a3bc1e8b9..57078522b 100644
--- a/app/Exceptions/Handler.php
+++ b/app/Exceptions/Handler.php
@@ -9,7 +9,6 @@ use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
-use Illuminate\Http\Response;
 use Illuminate\Validation\ValidationException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -26,6 +25,7 @@ class Handler extends ExceptionHandler
         HttpException::class,
         ModelNotFoundException::class,
         ValidationException::class,
+        NotFoundException::class,
     ];
 
     /**
diff --git a/phpcs.xml b/phpcs.xml
index 009791fc9..ccde28033 100644
--- a/phpcs.xml
+++ b/phpcs.xml
@@ -3,6 +3,7 @@
     <description>The coding standard for BookStack.</description>
     <file>app</file>
     <exclude-pattern>*/migrations/*</exclude-pattern>
+    <exclude-pattern>*/tests/*</exclude-pattern>
     <arg value="np"/>
     <rule ref="PSR2"/>
 </ruleset>
\ No newline at end of file
diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php
index a5e4a4a5e..8f6867cde 100644
--- a/tests/ErrorTest.php
+++ b/tests/ErrorTest.php
@@ -1,5 +1,8 @@
 <?php namespace Tests;
 
+use BookStack\Entities\Book;
+use Illuminate\Support\Facades\Log;
+
 class ErrorTest extends TestCase
 {
 
@@ -18,4 +21,21 @@ class ErrorTest extends TestCase
         $notFound->assertDontSeeText('Log in');
         $notFound->assertSeeText('tester');
     }
+
+    public function test_item_not_found_does_not_get_logged_to_file()
+    {
+        $this->actingAs($this->getViewer());
+        $handler = $this->withTestLogger();
+        $book = Book::query()->first();
+
+        // Ensure we're seeing errors
+        Log::error('cat');
+        $this->assertTrue($handler->hasErrorThatContains('cat'));
+
+        $this->get('/books/arandomnotfouindbook');
+        $this->get($book->getUrl('/chapter/arandomnotfouindchapter'));
+        $this->get($book->getUrl('/chapter/arandomnotfouindpages'));
+
+        $this->assertCount(1, $handler->getRecords());
+    }
 }
\ No newline at end of file
diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php
index f7b7d5edf..c7659a02d 100644
--- a/tests/SharedTestHelpers.php
+++ b/tests/SharedTestHelpers.php
@@ -16,7 +16,10 @@ use BookStack\Entities\Repos\PageRepo;
 use BookStack\Settings\SettingService;
 use BookStack\Uploads\HttpFetcher;
 use Illuminate\Support\Env;
+use Illuminate\Support\Facades\Log;
 use Mockery;
+use Monolog\Handler\TestHandler;
+use Monolog\Logger;
 use Throwable;
 
 trait SharedTestHelpers
@@ -69,14 +72,14 @@ trait SharedTestHelpers
     }
 
     /**
-     * Get an instance of a user with 'viewer' permissions
-     * @param $attributes
-     * @return mixed
+     * Get an instance of a user with 'viewer' permissions.
      */
-    protected function getViewer($attributes = [])
+    protected function getViewer(array $attributes = []): User
     {
         $user = Role::getRole('viewer')->users()->first();
-        if (!empty($attributes)) $user->forceFill($attributes)->save();
+        if (!empty($attributes)) {
+            $user->forceFill($attributes)->save();
+        }
         return $user;
     }
 
@@ -277,4 +280,22 @@ trait SharedTestHelpers
         $this->assertStringStartsWith('You do not have permission to access', $error);
     }
 
+    /**
+     * Set a test handler as the logging interface for the application.
+     * Allows capture of logs for checking against during tests.
+     */
+    protected function withTestLogger(): TestHandler
+    {
+        $monolog = new Logger('testing');
+        $testHandler = new TestHandler();
+        $monolog->pushHandler($testHandler);
+
+        Log::extend('testing', function() use ($monolog) {
+            return $monolog;
+        });
+        Log::setDefaultDriver('testing');
+
+        return $testHandler;
+    }
+
 }
\ No newline at end of file