Extracted API auth into guard

Also implemented more elegant solution to allowing session auth for API
routes; A new 'StartSessionIfCookieExists' middleware, which wraps the
default 'StartSession' middleware will run for API routes which only
sets up the session if a session cookie is found on the request. Also
decrypts only the session cookie.

Also cleaned some TokenController codeclimate warnings.

app/Api/ApiToken.php

@@ -3,6 +3,7 @@
 use BookStack\Auth\User;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Support\Carbon;
 
 class ApiToken extends Model
 {
@@ -18,4 +19,13 @@ class ApiToken extends Model
     {
         return $this->belongsTo(User::class);
     }
+
+    /**
+     * Get the default expiry value for an API token.
+     * Set to 100 years from now.
+     */
+    public static function defaultExpiry(): string
+    {
+        return Carbon::now()->addYears(100)->format('Y-m-d');
+    }
 }

app/Api/ApiTokenGuard.php

@@ -0,0 +1,135 @@
+<?php
+
+namespace BookStack\Api;
+
+use BookStack\Exceptions\ApiAuthException;
+use Illuminate\Auth\GuardHelpers;
+use Illuminate\Contracts\Auth\Authenticatable;
+use Illuminate\Contracts\Auth\Guard;
+use Illuminate\Support\Facades\Hash;
+use Symfony\Component\HttpFoundation\Request;
+
+class ApiTokenGuard implements Guard
+{
+
+    use GuardHelpers;
+
+    /**
+     * The request instance.
+     */
+    protected $request;
+
+
+    /**
+     * The last auth exception thrown in this request.
+     * @var ApiAuthException
+     */
+    protected $lastAuthException;
+
+    /**
+     * ApiTokenGuard constructor.
+     */
+    public function __construct(Request $request)
+    {
+        $this->request = $request;
+    }
+
+
+    /**
+     * @inheritDoc
+     */
+    public function user()
+    {
+        // Return the user if we've already retrieved them.
+        // Effectively a request-instance cache for this method.
+        if (!is_null($this->user)) {
+            return $this->user;
+        }
+
+        $user = null;
+        try {
+            $user = $this->getAuthorisedUserFromRequest();
+        } catch (ApiAuthException $exception) {
+            $this->lastAuthException = $exception;
+        }
+
+        $this->user = $user;
+        return $user;
+    }
+
+    /**
+     * Determine if current user is authenticated. If not, throw an exception.
+     *
+     * @return \Illuminate\Contracts\Auth\Authenticatable
+     *
+     * @throws ApiAuthException
+     */
+    public function authenticate()
+    {
+        if (! is_null($user = $this->user())) {
+            return $user;
+        }
+
+        if ($this->lastAuthException) {
+            throw $this->lastAuthException;
+        }
+
+        throw new ApiAuthException('Unauthorized');
+    }
+
+    /**
+     * Check the API token in the request and fetch a valid authorised user.
+     * @throws ApiAuthException
+     */
+    protected function getAuthorisedUserFromRequest(): Authenticatable
+    {
+        $authToken = trim($this->request->headers->get('Authorization', ''));
+        if (empty($authToken)) {
+            throw new ApiAuthException(trans('errors.api_no_authorization_found'));
+        }
+
+        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();
+
+        if ($token === null) {
+            throw new ApiAuthException(trans('errors.api_user_token_not_found'));
+        }
+
+        if (!Hash::check($secret, $token->secret)) {
+            throw new ApiAuthException(trans('errors.api_incorrect_token_secret'));
+        }
+
+        if (!$token->user->can('access-api')) {
+            throw new ApiAuthException(trans('errors.api_user_no_api_permission'), 403);
+        }
+
+        return $token->user;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function validate(array $credentials = [])
+    {
+        if (empty($credentials['id']) || empty($credentials['secret'])) {
+            return false;
+        }
+
+        $token = ApiToken::query()
+            ->where('token_id', '=', $credentials['id'])
+            ->with(['user'])->first();
+
+        if ($token === null) {
+            return false;
+        }
+
+        return Hash::check($credentials['secret'], $token->secret);
+    }
+
+}

app/Config/auth.php

@@ -34,9 +34,7 @@ return [
         ],
 
         'api' => [
-            'driver' => 'token',
-            'provider' => 'users',
-            'hash' => false,
+            'driver' => 'api-token',
         ],
     ],
 

app/Exceptions/ApiAuthException.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace BookStack\Exceptions;
+
+use Exception;
+
+class ApiAuthException extends Exception
+{
+
+    /**
+     * ApiAuthException constructor.
+     */
+    public function __construct($message, $code = 401)
+    {
+        parent::__construct($message, $code);
+    }
+}

app/Http/Controllers/UserApiTokenController.php

@@ -41,17 +41,12 @@ class UserApiTokenController extends Controller
         $user = User::query()->findOrFail($userId);
         $secret = Str::random(32);
 
-        $expiry = $request->get('expires_at', null);
-        if (empty($expiry)) {
-            $expiry = Carbon::now()->addYears(100)->format('Y-m-d');
-        }
-
         $token = (new ApiToken())->forceFill([
             'name' => $request->get('name'),
             'token_id' => Str::random(32),
             'secret' => Hash::make($secret),
             'user_id' => $user->id,
-            'expires_at' => $expiry
+            'expires_at' => $request->get('expires_at') ?: ApiToken::defaultExpiry(),
         ]);
 
         while (ApiToken::query()->where('token_id', '=', $token->token_id)->exists()) {
@@ -59,7 +54,6 @@ class UserApiTokenController extends Controller
         }
 
         $token->save();
-        $token->refresh();
 
         session()->flash('api-token-secret:' . $token->id, $secret);
         $this->showSuccessNotification(trans('settings.user_api_token_create_success'));
@@ -87,18 +81,17 @@ class UserApiTokenController extends Controller
      */
     public function update(Request $request, int $userId, int $tokenId)
     {
-        $requestData = $this->validate($request, [
+        $this->validate($request, [
             'name' => 'required|max:250',
             'expires_at' => 'date_format:Y-m-d',
         ]);
 
         [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId);
+        $token->fill([
+            'name' => $request->get('name'),
+            'expires_at' => $request->get('expires_at') ?: ApiToken::defaultExpiry(),
+        ])->save();
 
-        if (empty($requestData['expires_at'])) {
-            $requestData['expires_at'] = Carbon::now()->addYears(100)->format('Y-m-d');
-        }
-
-        $token->fill($requestData)->save();
         $this->showSuccessNotification(trans('settings.user_api_token_update_success'));
         return redirect($user->getEditUrl('/api-tokens/' . $token->id));
     }

app/Http/Kernel.php

@@ -1,6 +1,5 @@
 <?php namespace BookStack\Http;
 
-use BookStack\Http\Middleware\ApiAuthenticate;
 use Illuminate\Foundation\Http\Kernel as HttpKernel;
 
 class Kernel extends HttpKernel
@@ -24,6 +23,7 @@ class Kernel extends HttpKernel
         \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,
@@ -54,8 +54,7 @@ class Kernel extends HttpKernel
         ],
         'api' => [
             'throttle:60,1',
-            \BookStack\Http\Middleware\EncryptCookies::class,
-            \Illuminate\Session\Middleware\StartSession::class,
+            \BookStack\Http\Middleware\StartSessionIfCookieExists::class,
             \BookStack\Http\Middleware\ApiAuthenticate::class,
             \BookStack\Http\Middleware\ConfirmEmails::class,
         ],

app/Http/Middleware/ApiAuthenticate.php

@@ -2,10 +2,9 @@
 
 namespace BookStack\Http\Middleware;
 
-use BookStack\Api\ApiToken;
+use BookStack\Exceptions\ApiAuthException;
 use BookStack\Http\Request;
 use Closure;
-use Hash;
 
 class ApiAuthenticate
 {
@@ -15,58 +14,29 @@ class ApiAuthenticate
      */
     public function handle(Request $request, Closure $next)
     {
-        // TODO - Look to extract a lot of the logic here into a 'Guard'
-        // Ideally would like to be able to request API via browser without having to boot
-        // the session middleware (in Kernel).
-
-//        $sessionCookieName = config('session.cookie');
-//        if ($request->cookies->has($sessionCookieName)) {
-//            $sessionCookie = $request->cookies->get($sessionCookieName);
-//            $sessionCookie = decrypt($sessionCookie, false);
-//            dd($sessionCookie);
-//        }
-
         // 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()) {
             return $next($request);
         }
 
-        $authToken = trim($request->header('Authorization', ''));
-        if (empty($authToken)) {
-            return $this->unauthorisedResponse(trans('errors.api_no_authorization_found'));
+        // Set our api guard to be the default for this request lifecycle.
+        auth()->shouldUse('api');
+
+        // Validate the token and it's users API access
+        try {
+            auth()->authenticate();
+        } catch (ApiAuthException $exception) {
+            return $this->unauthorisedResponse($exception->getMessage(), $exception->getCode());
         }
 
-        if (strpos($authToken, ':') === false || strpos($authToken, 'Token ') !== 0) {
-            return $this->unauthorisedResponse(trans('errors.api_bad_authorization_format'));
-        }
-
-        [$id, $secret] = explode(':', str_replace('Token ', '', $authToken));
-        $token = ApiToken::query()
-            ->where('token_id', '=', $id)
-            ->with(['user'])->first();
-
-        if ($token === null) {
-            return $this->unauthorisedResponse(trans('errors.api_user_token_not_found'));
-        }
-
-        if (!Hash::check($secret, $token->secret)) {
-            return $this->unauthorisedResponse(trans('errors.api_incorrect_token_secret'));
-        }
-
-        if (!$token->user->can('access-api')) {
-            return $this->unauthorisedResponse(trans('errors.api_user_no_api_permission'), 403);
-        }
-
-        auth()->login($token->user);
-
         return $next($request);
     }
 
     /**
      * Provide a standard API unauthorised response.
      */
-    protected function unauthorisedResponse(string $message, int $code = 401)
+    protected function unauthorisedResponse(string $message, int $code)
     {
         return response()->json([
             'error' => [

app/Http/Middleware/StartSessionIfCookieExists.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use BookStack\Http\Request;
+use Closure;
+use Exception;
+use Illuminate\Session\Middleware\StartSession as Middleware;
+
+class StartSessionIfCookieExists extends Middleware
+{
+    /**
+     * Handle an incoming request.
+     */
+    public function handle($request, Closure $next)
+    {
+        $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) {
+            //
+        }
+    }
+}

app/Providers/AuthServiceProvider.php

@@ -3,6 +3,7 @@
 namespace BookStack\Providers;
 
 use Auth;
+use BookStack\Api\ApiTokenGuard;
 use BookStack\Auth\Access\LdapService;
 use Illuminate\Support\ServiceProvider;
 
@@ -15,7 +16,9 @@ class AuthServiceProvider extends ServiceProvider
      */
     public function boot()
     {
-        //
+        Auth::extend('api-token', function ($app, $name, array $config) {
+            return new ApiTokenGuard($app['request']);
+        });
     }
 
     /**