From 3de55ee6454667e2d4b3cb866625a165ccb9aee3 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 30 Dec 2019 02:16:07 +0000
Subject: [PATCH] Linked new API token system into middleware
Base logic in place but needs review and refactor to see if can better
fit into Laravel using 'Guard' system. Currently has issues due to
cookies in use from active session on API.
---
app/Api/ApiToken.php | 10 ++++
app/Http/Kernel.php | 30 ++++++++--
app/Http/Middleware/ApiAuthenticate.php | 78 +++++++++++++++++++++++++
app/Http/Middleware/Authenticate.php | 29 +--------
app/Http/Middleware/ConfirmEmails.php | 60 +++++++++++++++++++
app/helpers.php | 5 --
resources/lang/en/errors.php | 8 +++
7 files changed, 183 insertions(+), 37 deletions(-)
create mode 100644 app/Http/Middleware/ApiAuthenticate.php
create mode 100644 app/Http/Middleware/ConfirmEmails.php
diff --git a/app/Api/ApiToken.php b/app/Api/ApiToken.php
index 4ea12888e..cdcb33a7b 100644
--- a/app/Api/ApiToken.php
+++ b/app/Api/ApiToken.php
@@ -1,6 +1,8 @@
<?php namespace BookStack\Api;
+use BookStack\Auth\User;
use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
class ApiToken extends Model
{
@@ -8,4 +10,12 @@ class ApiToken extends Model
protected $casts = [
'expires_at' => 'date:Y-m-d'
];
+
+ /**
+ * Get the user that this token belongs to.
+ */
+ public function user(): BelongsTo
+ {
+ return $this->belongsTo(User::class);
+ }
}
diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index cd3fc83ec..64782fedc 100644
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -1,21 +1,38 @@
<?php namespace BookStack\Http;
+use BookStack\Http\Middleware\ApiAuthenticate;
use Illuminate\Foundation\Http\Kernel as HttpKernel;
class Kernel extends HttpKernel
{
/**
* The application's global HTTP middleware stack.
- *
* These middleware are run during every request to your application.
- *
- * @var array
*/
protected $middleware = [
\BookStack\Http\Middleware\CheckForMaintenanceMode::class,
\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,
+ \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,
];
/**
@@ -31,12 +48,16 @@ class Kernel extends HttpKernel
\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\ConfirmEmails::class,
],
'api' => [
'throttle:60,1',
+ \BookStack\Http\Middleware\EncryptCookies::class,
+ \Illuminate\Session\Middleware\StartSession::class,
+ \BookStack\Http\Middleware\ApiAuthenticate::class,
+ \BookStack\Http\Middleware\ConfirmEmails::class,
],
];
@@ -47,7 +68,6 @@ class Kernel extends HttpKernel
*/
protected $routeMiddleware = [
'auth' => \BookStack\Http\Middleware\Authenticate::class,
- 'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
'can' => \Illuminate\Auth\Middleware\Authorize::class,
'guest' => \BookStack\Http\Middleware\RedirectIfAuthenticated::class,
'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
diff --git a/app/Http/Middleware/ApiAuthenticate.php b/app/Http/Middleware/ApiAuthenticate.php
new file mode 100644
index 000000000..3e68cb3ae
--- /dev/null
+++ b/app/Http/Middleware/ApiAuthenticate.php
@@ -0,0 +1,78 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use BookStack\Api\ApiToken;
+use BookStack\Http\Request;
+use Closure;
+use Hash;
+
+class ApiAuthenticate
+{
+
+ /**
+ * Handle an incoming request.
+ */
+ 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'));
+ }
+
+ 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)
+ {
+ return response()->json([
+ 'error' => [
+ 'code' => $code,
+ 'message' => $message,
+ ]
+ ], 401);
+ }
+}
diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php
index d840a9b2e..40acc254b 100644
--- a/app/Http/Middleware/Authenticate.php
+++ b/app/Http/Middleware/Authenticate.php
@@ -2,41 +2,16 @@
namespace BookStack\Http\Middleware;
+use BookStack\Http\Request;
use Closure;
-use Illuminate\Contracts\Auth\Guard;
class Authenticate
{
- /**
- * The Guard implementation.
- * @var Guard
- */
- protected $auth;
-
- /**
- * Create a new filter instance.
- * @param Guard $auth
- */
- public function __construct(Guard $auth)
- {
- $this->auth = $auth;
- }
-
/**
* Handle an incoming request.
- * @param \Illuminate\Http\Request $request
- * @param \Closure $next
- * @return mixed
*/
- public function handle($request, Closure $next)
+ 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 redirect('/register/confirm/awaiting');
- }
- }
-
if (!hasAppAccess()) {
if ($request->ajax()) {
return response('Unauthorized.', 401);
diff --git a/app/Http/Middleware/ConfirmEmails.php b/app/Http/Middleware/ConfirmEmails.php
new file mode 100644
index 000000000..3700e9973
--- /dev/null
+++ b/app/Http/Middleware/ConfirmEmails.php
@@ -0,0 +1,60 @@
+<?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');
+ }
+}
diff --git a/app/helpers.php b/app/helpers.php
index 6211f41be..65da1853b 100644
--- a/app/helpers.php
+++ b/app/helpers.php
@@ -42,7 +42,6 @@ function user(): User
/**
* Check if current user is a signed in user.
- * @return bool
*/
function signedInUser(): bool
{
@@ -51,7 +50,6 @@ function signedInUser(): bool
/**
* Check if the current user has general access.
- * @return bool
*/
function hasAppAccess(): bool
{
@@ -62,9 +60,6 @@ function hasAppAccess(): bool
* Check if the current user has a permission.
* If an ownable element is passed in the jointPermissions are checked against
* that particular item.
- * @param string $permission
- * @param Ownable $ownable
- * @return bool
*/
function userCan(string $permission, Ownable $ownable = null): bool
{
diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php
index a7c591c5d..85c498f48 100644
--- a/resources/lang/en/errors.php
+++ b/resources/lang/en/errors.php
@@ -13,6 +13,7 @@ return [
'email_already_confirmed' => 'Email has already been confirmed, Try logging in.',
'email_confirmation_invalid' => 'This confirmation token is not valid or has already been used, Please try registering again.',
'email_confirmation_expired' => 'The confirmation token has expired, A new confirmation email has been sent.',
+ 'email_confirmation_awaiting' => 'The email address for the account in use needs to be confirmed',
'ldap_fail_anonymous' => 'LDAP access failed using anonymous bind',
'ldap_fail_authed' => 'LDAP access failed using given dn & password details',
'ldap_extension_not_installed' => 'LDAP PHP extension not installed',
@@ -88,4 +89,11 @@ return [
'app_down' => ':appName is down right now',
'back_soon' => 'It will be back up soon.',
+ // API errors
+ 'api_no_authorization_found' => 'No authorization token found on the request',
+ 'api_bad_authorization_format' => 'An authorization token was found on the request but the format appeared incorrect',
+ 'api_user_token_not_found' => 'No matching API token was found for the provided authorization token',
+ 'api_incorrect_token_secret' => 'The secret provided for the given used API token is incorrect',
+ 'api_user_no_api_permission' => 'The owner of the used API token does not have permission to make API calls',
+
];