Added back email confirmation check in middleware

During writing of the update notes, found that the upgrade path would be
tricky from a security point of view. If people were pending email
confirmation but had an active session, they could technically be
actively logged in after the next release.

Added middlware as an extra precaution for now.

diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index 1733d29b358bc930489491874d361c67a11ae108..4b8cdfba466365cf2c1e5753785149c88fb80f6c 100644 (file)
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -30,6 +30,7 @@ class Kernel extends HttpKernel
             \Illuminate\Session\Middleware\StartSession::class,
             \Illuminate\View\Middleware\ShareErrorsFromSession::class,
             \BookStack\Http\Middleware\VerifyCsrfToken::class,
+            \BookStack\Http\Middleware\CheckEmailConfirmed::class,
             \BookStack\Http\Middleware\RunThemeActions::class,
             \BookStack\Http\Middleware\Localization::class,
         ],
@@ -38,6 +39,7 @@ class Kernel extends HttpKernel
             \BookStack\Http\Middleware\EncryptCookies::class,
             \BookStack\Http\Middleware\StartSessionIfCookieExists::class,
             \BookStack\Http\Middleware\ApiAuthenticate::class,
+            \BookStack\Http\Middleware\CheckEmailConfirmed::class,
         ],
     ];
 

diff --git a/app/Http/Middleware/CheckEmailConfirmed.php b/app/Http/Middleware/CheckEmailConfirmed.php
new file mode 100644 (file)
index 0000000..b4843e7
--- /dev/null
+++ b/app/Http/Middleware/CheckEmailConfirmed.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use BookStack\Auth\Access\EmailConfirmationService;
+use BookStack\Auth\User;
+use Closure;
+
+/**
+ * Check that the user's email address is confirmed.
+ *
+ * As of v21.08 this is technically not required but kept as a prevention
+ * to log out any users that may be logged in but in an "awaiting confirmation" state.
+ * We'll keep this for a while until it'd be very unlikely for a user to be upgrading from
+ * a pre-v21.08 version.
+ *
+ * Ideally we'd simply invalidate all existing sessions upon update but that has
+ * proven to be a lot more difficult than expected.
+ */
+class CheckEmailConfirmed
+{
+    protected $confirmationService;
+
+    public function __construct(EmailConfirmationService $confirmationService)
+    {
+        $this->confirmationService = $confirmationService;
+    }
+
+
+    /**
+     * Handle an incoming request.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @param  \Closure  $next
+     * @return mixed
+     */
+    public function handle($request, Closure $next)
+    {
+        /** @var User $user */
+        $user = auth()->user();
+        if (auth()->check() && !$user->email_confirmed && $this->confirmationService->confirmationRequired()) {
+            auth()->logout();
+            return redirect()->to('/');
+        }
+
+        return $next($request);
+    }
+}

diff --git a/tests/Auth/AuthTest.php b/tests/Auth/AuthTest.php
index 657728c175e712e8d075ed2436483b1e6c2dacd9..718fb859d9b5c1b5b635b8e98c103ecd841f783b 100644 (file)
--- a/tests/Auth/AuthTest.php
+++ b/tests/Auth/AuthTest.php
@@ -459,6 +459,22 @@ class AuthTest extends BrowserKitTest
         $this->assertFalse($log->hasWarningThatContains('Failed login for [email protected]'));
     }
 
+    public function test_logged_in_user_with_unconfirmed_email_is_logged_out()
+    {
+        $this->setSettings(['registration-confirmation' => 'true']);
+        $user = $this->getEditor();
+        $user->email_confirmed = false;
+        $user->save();
+
+        auth()->login($user);
+        $this->assertTrue(auth()->check());
+
+        $this->get('/books');
+        $this->assertRedirectedTo("/");
+
+        $this->assertFalse(auth()->check());
+    }
+
     /**
      * Perform a login.
      */