Started alignment of auth services

- Removed LDAP specific logic from login controller, placed in Guard.
- Created safer base user provider for ldap login, to be used for SAML
soon.
- Moved LDAP auth work from user provider to guard.

app/Auth/Access/ExternalBaseUserProvider.php

@@ -1,12 +1,11 @@
 <?php
 
-namespace BookStack\Providers;
+namespace BookStack\Auth\Access;
 
-use BookStack\Auth\Access\LdapService;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Contracts\Auth\UserProvider;
 
-class LdapUserProvider implements UserProvider
+class ExternalBaseUserProvider implements UserProvider
 {
 
     /**
@@ -16,21 +15,13 @@ class LdapUserProvider implements UserProvider
      */
     protected $model;
 
-    /**
-     * @var \BookStack\Auth\LdapService
-     */
-    protected $ldapService;
-
-
     /**
      * LdapUserProvider constructor.
      * @param             $model
-     * @param \BookStack\Auth\LdapService $ldapService
      */
-    public function __construct($model, LdapService $ldapService)
+    public function __construct(string $model)
     {
         $this->model = $model;
-        $this->ldapService = $ldapService;
     }
 
     /**
@@ -44,7 +35,6 @@ class LdapUserProvider implements UserProvider
         return new $class;
     }
 
-
     /**
      * Retrieve a user by their unique identifier.
      *
@@ -65,12 +55,7 @@ class LdapUserProvider implements UserProvider
      */
     public function retrieveByToken($identifier, $token)
     {
-        $model = $this->createModel();
-
-        return $model->newQuery()
-            ->where($model->getAuthIdentifierName(), $identifier)
-            ->where($model->getRememberTokenName(), $token)
-            ->first();
+        return null;
     }
 
 
@@ -83,10 +68,7 @@ class LdapUserProvider implements UserProvider
      */
     public function updateRememberToken(Authenticatable $user, $token)
     {
-        if ($user->exists) {
-            $user->setRememberToken($token);
-            $user->save();
-        }
+        //
     }
 
     /**
@@ -97,27 +79,11 @@ class LdapUserProvider implements UserProvider
      */
     public function retrieveByCredentials(array $credentials)
     {
-        // Get user via LDAP
-        $userDetails = $this->ldapService->getUserDetails($credentials['username']);
-        if ($userDetails === null) {
-            return null;
-        }
-
         // Search current user base by looking up a uid
         $model = $this->createModel();
-        $currentUser = $model->newQuery()
-            ->where('external_auth_id', $userDetails['uid'])
+        return $model->newQuery()
+            ->where('external_auth_id', $credentials['external_auth_id'])
             ->first();
-
-        if ($currentUser !== null) {
-            return $currentUser;
-        }
-
-        $model->name = $userDetails['name'];
-        $model->external_auth_id = $userDetails['uid'];
-        $model->email = $userDetails['email'];
-        $model->email_confirmed = false;
-        return $model;
     }
 
     /**
@@ -129,6 +95,7 @@ class LdapUserProvider implements UserProvider
      */
     public function validateCredentials(Authenticatable $user, array $credentials)
     {
-        return $this->ldapService->validateUserCredentials($user, $credentials['username'], $credentials['password']);
+        // Should be done in the guard.
+        return false;
     }
 }

app/Auth/Access/Guards/ExternalBaseSessionGuard.php

@@ -0,0 +1,301 @@
+<?php
+
+namespace BookStack\Auth\Access\Guards;
+
+use Illuminate\Auth\GuardHelpers;
+use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract;
+use Illuminate\Contracts\Auth\StatefulGuard;
+use Illuminate\Contracts\Auth\UserProvider;
+use Illuminate\Contracts\Session\Session;
+
+/**
+ * Class BaseSessionGuard
+ * A base implementation of a session guard. Is a copy of the default Laravel
+ * guard with 'remember' functionality removed. Basic auth and event emission
+ * has also been removed to keep this simple. Designed to be extended by external
+ * Auth Guards.
+ *
+ * @package Illuminate\Auth
+ */
+class ExternalBaseSessionGuard implements StatefulGuard
+{
+    use GuardHelpers;
+
+    /**
+     * The name of the Guard. Typically "session".
+     *
+     * Corresponds to guard name in authentication configuration.
+     *
+     * @var string
+     */
+    protected $name;
+
+    /**
+     * The user we last attempted to retrieve.
+     *
+     * @var \Illuminate\Contracts\Auth\Authenticatable
+     */
+    protected $lastAttempted;
+
+    /**
+     * The session used by the guard.
+     *
+     * @var \Illuminate\Contracts\Session\Session
+     */
+    protected $session;
+
+    /**
+     * Indicates if the logout method has been called.
+     *
+     * @var bool
+     */
+    protected $loggedOut = false;
+
+    /**
+     * Create a new authentication guard.
+     *
+     * @param  string  $name
+     * @param  \Illuminate\Contracts\Auth\UserProvider  $provider
+     * @param  \Illuminate\Contracts\Session\Session  $session
+     * @return void
+     */
+    public function __construct($name,
+        UserProvider $provider,
+        Session $session)
+    {
+        $this->name = $name;
+        $this->session = $session;
+        $this->provider = $provider;
+    }
+
+    /**
+     * Get the currently authenticated user.
+     *
+     * @return \Illuminate\Contracts\Auth\Authenticatable|null
+     */
+    public function user()
+    {
+        if ($this->loggedOut) {
+            return;
+        }
+
+        // If we've already retrieved the user for the current request we can just
+        // return it back immediately. We do not want to fetch the user data on
+        // every call to this method because that would be tremendously slow.
+        if (! is_null($this->user)) {
+            return $this->user;
+        }
+
+        $id = $this->session->get($this->getName());
+
+        // First we will try to load the user using the
+        // identifier in the session if one exists.
+        if (! is_null($id)) {
+            $this->user = $this->provider->retrieveById($id);
+        }
+
+        return $this->user;
+    }
+
+    /**
+     * Get the ID for the currently authenticated user.
+     *
+     * @return int|null
+     */
+    public function id()
+    {
+        if ($this->loggedOut) {
+            return;
+        }
+
+        return $this->user()
+            ? $this->user()->getAuthIdentifier()
+            : $this->session->get($this->getName());
+    }
+
+    /**
+     * Log a user into the application without sessions or cookies.
+     *
+     * @param  array  $credentials
+     * @return bool
+     */
+    public function once(array $credentials = [])
+    {
+        if ($this->validate($credentials)) {
+            $this->setUser($this->lastAttempted);
+
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * Log the given user ID into the application without sessions or cookies.
+     *
+     * @param  mixed  $id
+     * @return \Illuminate\Contracts\Auth\Authenticatable|false
+     */
+    public function onceUsingId($id)
+    {
+        if (! is_null($user = $this->provider->retrieveById($id))) {
+            $this->setUser($user);
+
+            return $user;
+        }
+
+        return false;
+    }
+
+    /**
+     * Validate a user's credentials.
+     *
+     * @param  array  $credentials
+     * @return bool
+     */
+    public function validate(array $credentials = [])
+    {
+        return false;
+    }
+
+
+    /**
+     * Attempt to authenticate a user using the given credentials.
+     *
+     * @param  array  $credentials
+     * @param  bool  $remember
+     * @return bool
+     */
+    public function attempt(array $credentials = [], $remember = false)
+    {
+        return false;
+    }
+
+    /**
+     * Log the given user ID into the application.
+     *
+     * @param  mixed  $id
+     * @param  bool  $remember
+     * @return \Illuminate\Contracts\Auth\Authenticatable|false
+     */
+    public function loginUsingId($id, $remember = false)
+    {
+        if (! is_null($user = $this->provider->retrieveById($id))) {
+            $this->login($user, $remember);
+
+            return $user;
+        }
+
+        return false;
+    }
+
+    /**
+     * Log a user into the application.
+     *
+     * @param  \Illuminate\Contracts\Auth\Authenticatable  $user
+     * @param  bool  $remember
+     * @return void
+     */
+    public function login(AuthenticatableContract $user, $remember = false)
+    {
+        $this->updateSession($user->getAuthIdentifier());
+
+        $this->setUser($user);
+    }
+
+    /**
+     * Update the session with the given ID.
+     *
+     * @param  string  $id
+     * @return void
+     */
+    protected function updateSession($id)
+    {
+        $this->session->put($this->getName(), $id);
+
+        $this->session->migrate(true);
+    }
+
+    /**
+     * Log the user out of the application.
+     *
+     * @return void
+     */
+    public function logout()
+    {
+        $this->clearUserDataFromStorage();
+
+        // Now we will clear the users out of memory so they are no longer available
+        // as the user is no longer considered as being signed into this
+        // application and should not be available here.
+        $this->user = null;
+
+        $this->loggedOut = true;
+    }
+
+    /**
+     * Remove the user data from the session and cookies.
+     *
+     * @return void
+     */
+    protected function clearUserDataFromStorage()
+    {
+        $this->session->remove($this->getName());
+    }
+
+    /**
+     * Get the last user we attempted to authenticate.
+     *
+     * @return \Illuminate\Contracts\Auth\Authenticatable
+     */
+    public function getLastAttempted()
+    {
+        return $this->lastAttempted;
+    }
+
+    /**
+     * Get a unique identifier for the auth session value.
+     *
+     * @return string
+     */
+    public function getName()
+    {
+        return 'login_'.$this->name.'_'.sha1(static::class);
+    }
+
+    /**
+     * Determine if the user was authenticated via "remember me" cookie.
+     *
+     * @return bool
+     */
+    public function viaRemember()
+    {
+        return false;
+    }
+
+    /**
+     * Return the currently cached user.
+     *
+     * @return \Illuminate\Contracts\Auth\Authenticatable|null
+     */
+    public function getUser()
+    {
+        return $this->user;
+    }
+
+    /**
+     * Set the current user.
+     *
+     * @param  \Illuminate\Contracts\Auth\Authenticatable  $user
+     * @return $this
+     */
+    public function setUser(AuthenticatableContract $user)
+    {
+        $this->user = $user;
+
+        $this->loggedOut = false;
+
+        return $this;
+    }
+
+}

app/Auth/Access/Guards/LdapSessionGuard.php

@@ -0,0 +1,126 @@
+<?php
+
+namespace BookStack\Auth\Access\Guards;
+
+use BookStack\Auth\Access\LdapService;
+use BookStack\Auth\User;
+use BookStack\Auth\UserRepo;
+use BookStack\Exceptions\LdapException;
+use BookStack\Exceptions\LoginAttemptException;
+use BookStack\Exceptions\LoginAttemptEmailNeededException;
+use Illuminate\Contracts\Auth\UserProvider;
+use Illuminate\Contracts\Session\Session;
+
+class LdapSessionGuard extends ExternalBaseSessionGuard
+{
+
+    protected $ldapService;
+    protected $userRepo;
+
+    /**
+     * LdapSessionGuard constructor.
+     */
+    public function __construct($name,
+        UserProvider $provider,
+        Session $session,
+        LdapService $ldapService,
+        UserRepo $userRepo
+    )
+    {
+        $this->ldapService = $ldapService;
+        $this->userRepo = $userRepo;
+        parent::__construct($name, $provider, $session);
+    }
+
+    /**
+     * Validate a user's credentials.
+     *
+     * @param array $credentials
+     * @return bool
+     * @throws LdapException
+     */
+    public function validate(array $credentials = [])
+    {
+        $userDetails = $this->ldapService->getUserDetails($credentials['username']);
+        $this->lastAttempted = $this->provider->retrieveByCredentials([
+            'external_auth_id' => $userDetails['uid']
+        ]);
+
+        return $this->ldapService->validateUserCredentials($userDetails, $credentials['username'], $credentials['password']);
+    }
+
+    /**
+     * Attempt to authenticate a user using the given credentials.
+     *
+     * @param array $credentials
+     * @param bool $remember
+     * @return bool
+     * @throws LoginAttemptEmailNeededException
+     * @throws LoginAttemptException
+     * @throws LdapException
+     */
+    public function attempt(array $credentials = [], $remember = false)
+    {
+        $username = $credentials['username'];
+        $userDetails = $this->ldapService->getUserDetails($username);
+        $this->lastAttempted = $user = $this->provider->retrieveByCredentials([
+            'external_auth_id' => $userDetails['uid']
+        ]);
+
+        if (!$this->ldapService->validateUserCredentials($userDetails, $username, $credentials['password'])) {
+            return false;
+        }
+
+        if (is_null($user)) {
+            $user = $this->freshUserInstanceFromLdapUserDetails($userDetails);
+        }
+
+        $providedEmail = ($credentials['email'] ?? false);
+
+        // Request email if missing from LDAP and model and missing from request
+        if (is_null($user->email) && !$providedEmail) {
+            throw new LoginAttemptEmailNeededException();
+        }
+
+        // Add email to model if non-existing and email provided in request
+        if (!$user->exists && $user->email === null && $providedEmail) {
+            $user->email = $providedEmail;
+        }
+
+        if (!$user->exists) {
+            // Check for existing users with same email
+            $alreadyUser = $user->newQuery()->where('email', '=', $user->email)->count() > 0;
+            if ($alreadyUser) {
+                throw new LoginAttemptException(trans('errors.error_user_exists_different_creds', ['email' => $user->email]));
+            }
+
+            $user->save();
+            $this->userRepo->attachDefaultRole($user);
+            $this->userRepo->downloadAndAssignUserAvatar($user);
+        }
+
+        // Sync LDAP groups if required
+        if ($this->ldapService->shouldSyncGroups()) {
+            $this->ldapService->syncGroups($user, $username);
+        }
+
+        $this->login($user, $remember);
+        return true;
+    }
+
+    /**
+     * Create a fresh user instance from details provided by a LDAP lookup.
+     */
+    protected function freshUserInstanceFromLdapUserDetails(array $ldapUserDetails): User
+    {
+        $user = new User();
+
+        $user->name = $ldapUserDetails['name'];
+        $user->external_auth_id = $ldapUserDetails['uid'];
+        $user->email = $ldapUserDetails['email'];
+        $user->email_confirmed = false;
+
+        return $user;
+    }
+
+}

app/Auth/Access/LdapService.php

@@ -106,20 +106,15 @@ class LdapService extends ExternalAuthService
      * Check if the given credentials are valid for the given user.
      * @throws LdapException
      */
-    public function validateUserCredentials(Authenticatable $user, string $username, string $password): bool
+    public function validateUserCredentials(array $ldapUserDetails, string $username, string $password): bool
     {
-        $ldapUser = $this->getUserDetails($username);
-        if ($ldapUser === null) {
-            return false;
-        }
-
-        if ($ldapUser['uid'] !== $user->external_auth_id) {
+        if ($ldapUserDetails === null) {
             return false;
         }
 
         $ldapConnection = $this->getConnection();
         try {
-            $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password);
+            $ldapBind = $this->ldap->bind($ldapConnection, $ldapUserDetails['dn'], $password);
         } catch (ErrorException $e) {
             $ldapBind = false;
         }

app/Config/auth.php

@@ -18,7 +18,7 @@ return [
     // This option controls the default authentication "guard" and password
     // reset options for your application.
     'defaults' => [
-        'guard' => 'web',
+        'guard' => env('AUTH_METHOD', 'standard') === 'standard' ? 'web' : env('AUTH_METHOD'),
         'passwords' => 'users',
     ],
 
@@ -26,13 +26,16 @@ return [
     // All authentication drivers have a user provider. This defines how the
     // users are actually retrieved out of your database or other storage
     // mechanisms used by this application to persist your user's data.
-    // Supported: "session", "token"
+    // Supported drivers: "session", "api-token", "ldap-session"
     'guards' => [
         'web' => [
             'driver' => 'session',
             'provider' => 'users',
         ],
-
+        'ldap' => [
+            'driver' => 'ldap-session',
+            'provider' => 'external'
+        ],
         'api' => [
             'driver' => 'api-token',
         ],
@@ -42,17 +45,15 @@ return [
     // All authentication drivers have a user provider. This defines how the
     // users are actually retrieved out of your database or other storage
     // mechanisms used by this application to persist your user's data.
-    // Supported: database, eloquent, ldap
     'providers' => [
         'users' => [
             'driver' => env('AUTH_METHOD', 'standard') === 'standard' ? 'eloquent' : env('AUTH_METHOD'),
             'model' => \BookStack\Auth\User::class,
         ],
-
-        // 'users' => [
-        //     'driver' => 'database',
-        //     'table' => 'users',
-        // ],
+        'external' => [
+            'driver' => 'external-users',
+            'model' => \BookStack\Auth\User::class,
+        ],
     ],
 
     // Resetting Passwords

app/Exceptions/AuthException.php

@@ -1,6 +0,0 @@
-<?php namespace BookStack\Exceptions;
-
-class AuthException extends PrettyException
-{
-
-}

app/Exceptions/LoginAttemptEmailNeededException.php

@@ -0,0 +1,6 @@
+<?php namespace BookStack\Exceptions;
+
+class LoginAttemptEmailNeededException extends LoginAttemptException
+{
+
+}

app/Exceptions/LoginAttemptException.php

@@ -0,0 +1,6 @@
+<?php namespace BookStack\Exceptions;
+
+class LoginAttemptException extends \Exception
+{
+
+}

app/Http/Controllers/Auth/LoginController.php

@@ -2,12 +2,10 @@
 
 namespace BookStack\Http\Controllers\Auth;
 
-use BookStack\Auth\Access\LdapService;
 use BookStack\Auth\Access\SocialAuthService;
-use BookStack\Auth\UserRepo;
-use BookStack\Exceptions\AuthException;
+use BookStack\Exceptions\LoginAttemptEmailNeededException;
+use BookStack\Exceptions\LoginAttemptException;
 use BookStack\Http\Controllers\Controller;
-use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Foundation\Auth\AuthenticatesUsers;
 use Illuminate\Http\Request;
 
@@ -27,32 +25,21 @@ class LoginController extends Controller
     use AuthenticatesUsers;
 
     /**
-     * Where to redirect users after login.
-     *
-     * @var string
+     * Redirection paths
      */
     protected $redirectTo = '/';
-
     protected $redirectPath = '/';
     protected $redirectAfterLogout = '/login';
 
     protected $socialAuthService;
-    protected $ldapService;
-    protected $userRepo;
 
     /**
      * Create a new controller instance.
-     *
-     * @param \BookStack\Auth\\BookStack\Auth\Access\SocialAuthService $socialAuthService
-     * @param LdapService $ldapService
-     * @param \BookStack\Auth\UserRepo $userRepo
      */
-    public function __construct(SocialAuthService $socialAuthService, LdapService $ldapService, UserRepo $userRepo)
+    public function __construct(SocialAuthService $socialAuthService)
     {
         $this->middleware('guest', ['only' => ['getLogin', 'postLogin']]);
         $this->socialAuthService = $socialAuthService;
-        $this->ldapService = $ldapService;
-        $this->userRepo = $userRepo;
         $this->redirectPath = url('/');
         $this->redirectAfterLogout = url('/login');
         parent::__construct();
@@ -64,47 +51,11 @@ class LoginController extends Controller
     }
 
     /**
-     * Overrides the action when a user is authenticated.
-     * If the user authenticated but does not exist in the user table we create them.
-     * @throws AuthException
-     * @throws \BookStack\Exceptions\LdapException
+     * Get the needed authorization credentials from the request.
      */
-    protected function authenticated(Request $request, Authenticatable $user)
+    protected function credentials(Request $request)
     {
-        // Explicitly log them out for now if they do no exist.
-        if (!$user->exists) {
-            auth()->logout($user);
-        }
-
-        if (!$user->exists && $user->email === null && !$request->filled('email')) {
-            $request->flash();
-            session()->flash('request-email', true);
-            return redirect('/login');
-        }
-
-        if (!$user->exists && $user->email === null && $request->filled('email')) {
-            $user->email = $request->get('email');
-        }
-
-        if (!$user->exists) {
-            // Check for users with same email already
-            $alreadyUser = $user->newQuery()->where('email', '=', $user->email)->count() > 0;
-            if ($alreadyUser) {
-                throw new AuthException(trans('errors.error_user_exists_different_creds', ['email' => $user->email]));
-            }
-
-            $user->save();
-            $this->userRepo->attachDefaultRole($user);
-            $this->userRepo->downloadAndAssignUserAvatar($user);
-            auth()->login($user);
-        }
-
-        // Sync LDAP groups if required
-        if ($this->ldapService->shouldSyncGroups()) {
-            $this->ldapService->syncGroups($user, $request->get($this->username()));
-        }
-
-        return redirect()->intended('/');
+        return $request->only('username', 'email', 'password');
     }
 
     /**
@@ -130,6 +81,61 @@ class LoginController extends Controller
         ]);
     }
 
+    /**
+     * Handle a login request to the application.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\Response|\Illuminate\Http\JsonResponse
+     *
+     * @throws \Illuminate\Validation\ValidationException
+     */
+    public function login(Request $request)
+    {
+        $this->validateLogin($request);
+
+        // If the class is using the ThrottlesLogins trait, we can automatically throttle
+        // the login attempts for this application. We'll key this by the username and
+        // the IP address of the client making these requests into this application.
+        if (method_exists($this, 'hasTooManyLoginAttempts') &&
+            $this->hasTooManyLoginAttempts($request)) {
+            $this->fireLockoutEvent($request);
+
+            return $this->sendLockoutResponse($request);
+        }
+
+        try {
+            if ($this->attemptLogin($request)) {
+                return $this->sendLoginResponse($request);
+            }
+        } catch (LoginAttemptException $exception) {
+            return $this->sendLoginAttemptExceptionResponse($exception, $request);
+        }
+
+        // If the login attempt was unsuccessful we will increment the number of attempts
+        // to login and redirect the user back to the login form. Of course, when this
+        // user surpasses their maximum number of attempts they will get locked out.
+        $this->incrementLoginAttempts($request);
+
+        return $this->sendFailedLoginResponse($request);
+    }
+
+    /**
+     * Send a response when a login attempt exception occurs.
+     */
+    protected function sendLoginAttemptExceptionResponse(LoginAttemptException $exception, Request $request)
+    {
+        if ($exception instanceof LoginAttemptEmailNeededException) {
+            $request->flash();
+            session()->flash('request-email', true);
+        }
+
+        if ($message = $exception->getMessage()) {
+            $this->showWarningNotification($message);
+        }
+
+        return redirect('/login');
+    }
+
     /**
      * Log the user out of the application.
      */

app/Providers/AuthServiceProvider.php

@@ -4,7 +4,10 @@ namespace BookStack\Providers;
 
 use Auth;
 use BookStack\Api\ApiTokenGuard;
+use BookStack\Auth\Access\ExternalBaseUserProvider;
+use BookStack\Auth\Access\Guards\LdapSessionGuard;
 use BookStack\Auth\Access\LdapService;
+use BookStack\Auth\UserRepo;
 use Illuminate\Support\ServiceProvider;
 
 class AuthServiceProvider extends ServiceProvider
@@ -19,6 +22,17 @@ class AuthServiceProvider extends ServiceProvider
         Auth::extend('api-token', function ($app, $name, array $config) {
             return new ApiTokenGuard($app['request']);
         });
+
+        Auth::extend('ldap-session', function ($app, $name, array $config) {
+            $provider = Auth::createUserProvider($config['provider']);
+            return new LdapSessionGuard(
+                $name,
+                $provider,
+                $this->app['session.store'],
+                $app[LdapService::class],
+                $app[UserRepo::class]
+            );
+        });
     }
 
     /**
@@ -28,8 +42,8 @@ class AuthServiceProvider extends ServiceProvider
      */
     public function register()
     {
-        Auth::provider('ldap', function ($app, array $config) {
-            return new LdapUserProvider($config['model'], $app[LdapService::class]);
+        Auth::provider('external-users', function ($app, array $config) {
+            return new ExternalBaseUserProvider($config['model']);
         });
     }
 }