Change email confirmation from own middle to trait

Email confirmation middleware caused more mess than good, As caused
priority issues and it depended on auth actions. Instead its now a trai
used on auth middlewares.

Also used 'EncryptCookies' middleware on API instead of custom
decryption in custom middleware since we'd need to do replicate all the
same actions anyway. Shouldn't have too much effect since it only
actions over cookies that exist, of which none should be there for most
API requests.

Also split out some large guard functions to be a little more readable
and appease codeclimate.

app/Api/ApiTokenGuard.php

@@ -33,8 +33,7 @@ class ApiTokenGuard implements Guard
     {
         $this->request = $request;
     }
-
-
+    
     /**
      * @inheritDoc
      */
@@ -84,6 +83,24 @@ class ApiTokenGuard implements Guard
     protected function getAuthorisedUserFromRequest(): Authenticatable
     {
         $authToken = trim($this->request->headers->get('Authorization', ''));
+        $this->validateTokenHeaderValue($authToken);
+
+        [$id, $secret] = explode(':', str_replace('Token ', '', $authToken));
+        $token = ApiToken::query()
+            ->where('token_id', '=', $id)
+            ->with(['user'])->first();
+
+        $this->validateToken($token, $secret);
+
+        return $token->user;
+    }
+
+    /**
+     * Validate the format of the token header value string.
+     * @throws ApiAuthException
+     */
+    protected function validateTokenHeaderValue(string $authToken): void
+    {
         if (empty($authToken)) {
             throw new ApiAuthException(trans('errors.api_no_authorization_found'));
         }
@@ -91,12 +108,15 @@ class ApiTokenGuard implements Guard
         if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) {
             throw new ApiAuthException(trans('errors.api_bad_authorization_format'));
         }
+    }
 
-        [$id, $secret] = explode(':', str_replace('Token ', '', $authToken));
-        $token = ApiToken::query()
-            ->where('token_id', '=', $id)
-            ->with(['user'])->first();
-
+    /**
+     * Validate the given secret against the given token and ensure the token
+     * currently has access to the instance API.
+     * @throws ApiAuthException
+     */
+    protected function validateToken(?ApiToken $token, string $secret): void
+    {
         if ($token === null) {
             throw new ApiAuthException(trans('errors.api_user_token_not_found'));
         }
@@ -108,8 +128,6 @@ class ApiTokenGuard implements Guard
         if (!$token->user->can('access-api')) {
             throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403);
         }
-
-        return $token->user;
     }
 
     /**

app/Http/Kernel.php

@@ -13,26 +13,6 @@ class Kernel extends HttpKernel
         \Illuminate\Foundation\Http\Middleware\ValidatePostSize::class,
         \BookStack\Http\Middleware\TrimStrings::class,
         \BookStack\Http\Middleware\TrustProxies::class,
-
-    ];
-
-    /**
-     * The priority ordering of middleware.
-     */
-    protected $middlewarePriority = [
-        \BookStack\Http\Middleware\EncryptCookies::class,
-        \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
-        \Illuminate\Session\Middleware\StartSession::class,
-        \BookStack\Http\Middleware\StartSessionIfCookieExists::class,
-        \Illuminate\View\Middleware\ShareErrorsFromSession::class,
-        \Illuminate\Routing\Middleware\ThrottleRequests::class,
-        \BookStack\Http\Middleware\VerifyCsrfToken::class,
-        \Illuminate\Routing\Middleware\SubstituteBindings::class,
-        \BookStack\Http\Middleware\Localization::class,
-        \BookStack\Http\Middleware\GlobalViewData::class,
-        \BookStack\Http\Middleware\Authenticate::class,
-        \BookStack\Http\Middleware\ApiAuthenticate::class,
-        \BookStack\Http\Middleware\ConfirmEmails::class,
     ];
 
     /**
@@ -50,13 +30,12 @@ class Kernel extends HttpKernel
             \BookStack\Http\Middleware\VerifyCsrfToken::class,
             \BookStack\Http\Middleware\Localization::class,
             \BookStack\Http\Middleware\GlobalViewData::class,
-            \BookStack\Http\Middleware\ConfirmEmails::class,
         ],
         'api' => [
             'throttle:60,1',
+            \BookStack\Http\Middleware\EncryptCookies::class,
             \BookStack\Http\Middleware\StartSessionIfCookieExists::class,
             \BookStack\Http\Middleware\ApiAuthenticate::class,
-            \BookStack\Http\Middleware\ConfirmEmails::class,
         ],
     ];
 

app/Http/Middleware/ApiAuthenticate.php

@@ -3,11 +3,12 @@
 namespace BookStack\Http\Middleware;
 
 use BookStack\Exceptions\ApiAuthException;
-use BookStack\Http\Request;
 use Closure;
+use Illuminate\Http\Request;
 
 class ApiAuthenticate
 {
+    use ChecksForEmailConfirmation;
 
     /**
      * Handle an incoming request.
@@ -17,6 +18,9 @@ class ApiAuthenticate
         // Return if the user is already found to be signed in via session-based auth.
         // This is to make it easy to browser the API via browser after just logging into the system.
         if (signedInUser()) {
+            if ($this->awaitingEmailConfirmation()) {
+                return $this->emailConfirmationErrorResponse($request);
+            }
             return $next($request);
         }
 
@@ -30,6 +34,10 @@ class ApiAuthenticate
             return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode());
         }
 
+        if ($this->awaitingEmailConfirmation()) {
+            return $this->emailConfirmationErrorResponse($request);
+        }
+
         return $next($request);
     }
 

app/Http/Middleware/Authenticate.php

@@ -2,16 +2,22 @@
 
 namespace BookStack\Http\Middleware;
 
-use BookStack\Http\Request;
 use Closure;
+use Illuminate\Http\Request;
 
 class Authenticate
 {
+    use ChecksForEmailConfirmation;
+
     /**
      * Handle an incoming request.
      */
     public function handle(Request $request, Closure $next)
     {
+        if ($this->awaitingEmailConfirmation()) {
+            return $this->emailConfirmationErrorResponse($request);
+        }
+
         if (!hasAppAccess()) {
             if ($request->ajax()) {
                 return response('Unauthorized.', 401);

app/Http/Middleware/ChecksForEmailConfirmation.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use Illuminate\Http\Request;
+
+trait ChecksForEmailConfirmation
+{
+
+    /**
+     * Check if email confirmation is required and the current user is awaiting confirmation.
+     */
+    protected function awaitingEmailConfirmation(): bool
+    {
+        if (auth()->check()) {
+            $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict'));
+            if ($requireConfirmation && !auth()->user()->email_confirmed) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Provide an error response for when the current user's email is not confirmed
+     * in a system which requires it.
+     */
+    protected function emailConfirmationErrorResponse(Request $request)
+    {
+        if ($request->wantsJson()) {
+            return response()->json([
+                'error' => [
+                    'code' => 401,
+                    'message' => trans('errors.email_confirmation_awaiting')
+                ]
+            ], 401);
+        }
+
+        return redirect('/register/confirm/awaiting');
+    }
+}

app/Http/Middleware/ConfirmEmails.php

@@ -1,60 +0,0 @@
-<?php
-
-namespace BookStack\Http\Middleware;
-
-use BookStack\Http\Request;
-use Closure;
-use Illuminate\Contracts\Auth\Guard;
-
-/**
- * Confirms the current user's email address.
- * Must come after any middleware that may log users in.
- */
-class ConfirmEmails
-{
-    /**
-     * The Guard implementation.
-     */
-    protected $auth;
-
-    /**
-     * Create a new ConfirmEmails instance.
-     */
-    public function __construct(Guard $auth)
-    {
-        $this->auth = $auth;
-    }
-
-    /**
-     * Handle an incoming request.
-     */
-    public function handle(Request $request, Closure $next)
-    {
-        if ($this->auth->check()) {
-            $requireConfirmation = (setting('registration-confirmation') || setting('registration-restrict'));
-            if ($requireConfirmation && !$this->auth->user()->email_confirmed) {
-                return $this->errorResponse($request);
-            }
-        }
-
-        return $next($request);
-    }
-
-    /**
-     * Provide an error response for when the current user's email is not confirmed
-     * in a system which requires it.
-     */
-    protected function errorResponse(Request $request)
-    {
-        if ($request->wantsJson()) {
-            return response()->json([
-                'error' => [
-                    'code' => 401,
-                    'message' => trans('errors.email_confirmation_awaiting')
-                ]
-            ], 401);
-        }
-
-        return redirect('/register/confirm/awaiting');
-    }
-}

app/Http/Middleware/StartSessionIfCookieExists.php

@@ -2,9 +2,7 @@
 
 namespace BookStack\Http\Middleware;
 
-use BookStack\Http\Request;
 use Closure;
-use Exception;
 use Illuminate\Session\Middleware\StartSession as Middleware;
 
 class StartSessionIfCookieExists extends Middleware
@@ -16,24 +14,9 @@ class StartSessionIfCookieExists extends Middleware
     {
         $sessionCookieName = config('session.cookie');
         if ($request->cookies->has($sessionCookieName)) {
-            $this->decryptSessionCookie($request, $sessionCookieName);
             return parent::handle($request, $next);
         }
 
         return $next($request);
     }
-
-    /**
-     * Attempt decryption of the session cookie.
-     */
-    protected function decryptSessionCookie(Request $request, string $sessionCookieName)
-    {
-        try {
-            $sessionCookie = $request->cookies->get($sessionCookieName);
-            $sessionCookie = decrypt($sessionCookie, false);
-            $request->cookies->set($sessionCookieName, $sessionCookie);
-        } catch (Exception $e) {
-            //
-        }
-    }
 }