Enabled session in 404 responses

Fixes #634

diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php
index 12792e15184dfb0cef7bafaa8c28958e0ae5e9f8..a979072e23822f5fcc54b633696ef4115a86c5bf 100644 (file)
--- a/app/Exceptions/Handler.php
+++ b/app/Exceptions/Handler.php
@@ -4,11 +4,14 @@ namespace BookStack\Exceptions;
 
 use Exception;
 use Illuminate\Auth\AuthenticationException;
+use Illuminate\Http\Request;
+use Illuminate\Pipeline\Pipeline;
 use Illuminate\Validation\ValidationException;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
 use Illuminate\Auth\Access\AuthorizationException;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 class Handler extends ExceptionHandler
 {
@@ -60,9 +63,32 @@ class Handler extends ExceptionHandler
             return response()->view('errors/' . $code, ['message' => $message], $code);
         }
 
+        // Handle 404 errors with a loaded session to enable showing user-specific information
+        if ($this->isExceptionType($e, NotFoundHttpException::class)) {
+            return $this->loadErrorMiddleware($request, function ($request) use ($e) {
+                $message = $e->getMessage() ?: trans('errors.404_page_not_found');
+                return response()->view('errors/404', ['message' => $message], 404);
+            });
+        }
+
         return parent::render($request, $e);
     }
 
+    /**
+     * Load the middleware required to show state/session-enabled error pages.
+     * @param Request $request
+     * @param $callback
+     * @return mixed
+     */
+    protected function loadErrorMiddleware(Request $request, $callback)
+    {
+        $middleware = (\Route::getMiddlewareGroups()['web_errors']);
+        return (new Pipeline($this->container))
+            ->send($request)
+            ->through($middleware)
+            ->then($callback);
+    }
+
     /**
      * Check the exception chain to compare against the original exception type.
      * @param Exception $e

diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php
index 13e9284650e4f98e725570d43cfe9b4303c2e34b..9dc7d6401f97c07cd8915f802371ebc9bf395021 100644 (file)
--- a/app/Http/Controllers/PageController.php
+++ b/app/Http/Controllers/PageController.php
@@ -145,6 +145,7 @@ class PageController extends Controller
      * @param string $bookSlug
      * @param string $pageSlug
      * @return Response
+     * @throws NotFoundException
      */
     public function show($bookSlug, $pageSlug)
     {
@@ -152,7 +153,7 @@ class PageController extends Controller
             $page = $this->entityRepo->getBySlug('page', $pageSlug, $bookSlug);
         } catch (NotFoundException $e) {
             $page = $this->entityRepo->getPageByOldSlug($pageSlug, $bookSlug);
-            if ($page === null) abort(404);
+            if ($page === null) throw $e;
             return redirect($page->getUrl());
         }
 

diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index cd894de95340471f87e73c876cac8e2a4e49a657..9d2871bbeb828dd4c64ed63f78a35eb63c8806e5 100644 (file)
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -33,6 +33,14 @@ class Kernel extends HttpKernel
             \Illuminate\Routing\Middleware\SubstituteBindings::class,
             \BookStack\Http\Middleware\Localization::class
         ],
+        'web_errors' => [
+            \BookStack\Http\Middleware\EncryptCookies::class,
+            \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
+            \Illuminate\Session\Middleware\StartSession::class,
+            \Illuminate\View\Middleware\ShareErrorsFromSession::class,
+            \BookStack\Http\Middleware\VerifyCsrfToken::class,
+            \BookStack\Http\Middleware\Localization::class
+        ],
         'api' => [
             'throttle:60,1',
             'bindings',

diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php
index f6ef850afb2c2375c04109f6c6427ddc02b393d4..7cc67a6776bea41bf451bf86902ebee3f220a18d 100644 (file)
--- a/resources/views/errors/404.blade.php
+++ b/resources/views/errors/404.blade.php
@@ -1,8 +1,6 @@
 @extends('simple-layout')
 
 @section('content')
-
-
 <div class="container">
 
     <p>&nbsp;</p>
@@ -16,7 +14,6 @@
     </div>
 
     @if (setting('app-public') || !user()->isDefault())
-
         <div class="row">
             <div class="col-md-4">
                 <div class="card">

diff --git a/resources/views/errors/500.blade.php b/resources/views/errors/500.blade.php
index 71fb78a350242909420e9e6abbfbc26912e92044..a01234d811703a4d069a1fd915c23208d2a837b4 100644 (file)
--- a/resources/views/errors/500.blade.php
+++ b/resources/views/errors/500.blade.php
@@ -6,7 +6,7 @@
         <div class="card">
             <h3 class="text-muted">{{ trans('errors.error_occurred') }}</h3>
             <div class="body">
-                <h5>{{ $message }}</h5>
+                <h5>{{ $message or 'An unknown error occurred' }}</h5>
                 <p><a href="{{ baseUrl('/') }}" class="button outline">{{ trans('errors.return_home') }}</a></p>
             </div>
         </div>

diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php
new file mode 100644 (file)
index 0000000..c9b5a01
--- /dev/null
+++ b/tests/ErrorTest.php
@@ -0,0 +1,18 @@
+<?php namespace Tests;
+
+class ErrorTest extends TestCase
+{
+
+    public function test_404_page_does_not_show_login()
+    {
+        // Due to middleware being handled differently this will not fail
+        // if our custom, middleware-loaded handler fails but this is here
+        // as a reminder and as a general check in the event of other issues.
+        $editor = $this->getEditor();
+        $this->actingAs($editor);
+        $notFound = $this->get('/fgfdngldfnotfound');
+        $notFound->assertStatus(404);
+        $notFound->assertDontSeeText('Log in');
+        $notFound->assertSeeText($editor->getShortName(9));
+    }
+}
\ No newline at end of file