From 9249addb5c458d30a5b06ff1ccc3a4e1b5d29acb Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 17 Jul 2021 17:45:00 +0100 Subject: [PATCH] Updated all login events to route through single service --- .../Guards/ExternalBaseSessionGuard.php | 8 +-- app/Auth/Access/LoginService.php | 50 +++++++++++++++++++ app/Auth/Access/Saml2Service.php | 13 ++--- app/Auth/Access/SocialAuthService.php | 17 +++---- .../Auth/ConfirmEmailController.php | 13 +++-- app/Http/Controllers/Auth/LoginController.php | 33 ++++++------ .../Controllers/Auth/RegisterController.php | 16 +++--- .../Controllers/Auth/SocialController.php | 17 ++++--- .../Controllers/Auth/UserInviteController.php | 9 ++-- .../Middleware/EnforceMfaRequirements.php | 2 +- app/Providers/AppServiceProvider.php | 3 +- 11 files changed, 118 insertions(+), 63 deletions(-) create mode 100644 app/Auth/Access/LoginService.php diff --git a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php index 9c3b47e97..99bfd2e79 100644 --- a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php @@ -186,12 +186,8 @@ class ExternalBaseSessionGuard implements StatefulGuard */ public function loginUsingId($id, $remember = false) { - if (!is_null($user = $this->provider->retrieveById($id))) { - $this->login($user, $remember); - - return $user; - } - + // Always return false as to disable this method, + // Logins should route through LoginService. return false; } diff --git a/app/Auth/Access/LoginService.php b/app/Auth/Access/LoginService.php new file mode 100644 index 000000000..2bdef34a2 --- /dev/null +++ b/app/Auth/Access/LoginService.php @@ -0,0 +1,50 @@ +login($user); + Activity::add(ActivityType::AUTH_LOGIN, "{$method}; {$user->logDescriptor()}"); + Theme::dispatch(ThemeEvents::AUTH_LOGIN, $method, $user); + + // Authenticate on all session guards if a likely admin + if ($user->can('users-manage') && $user->can('user-roles-manage')) { + $guards = ['standard', 'ldap', 'saml2']; + foreach ($guards as $guard) { + auth($guard)->login($user); + } + } + } + + + /** + * Attempt the login of a user using the given credentials. + * Meant to mirror laravel's default guard 'attempt' method + * but in a manner that always routes through our login system. + */ + public function attempt(array $credentials, string $method, bool $remember = false): bool + { + $result = auth()->attempt($credentials, $remember); + if ($result) { + $user = auth()->user(); + auth()->logout(); + $this->login($user, $method); + } + + return $result; + } + +} \ No newline at end of file diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 28d4d4030..d0b6a655b 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -25,16 +25,16 @@ class Saml2Service extends ExternalAuthService { protected $config; protected $registrationService; - protected $user; + protected $loginService; /** * Saml2Service constructor. */ - public function __construct(RegistrationService $registrationService, User $user) + public function __construct(RegistrationService $registrationService, LoginService $loginService) { $this->config = config('saml2'); $this->registrationService = $registrationService; - $this->user = $user; + $this->loginService = $loginService; } /** @@ -332,7 +332,7 @@ class Saml2Service extends ExternalAuthService */ protected function getOrRegisterUser(array $userDetails): ?User { - $user = $this->user->newQuery() + $user = User::query() ->where('external_auth_id', '=', $userDetails['external_id']) ->first(); @@ -389,10 +389,7 @@ class Saml2Service extends ExternalAuthService $this->syncWithGroups($user, $groups); } - auth()->login($user); - Activity::add(ActivityType::AUTH_LOGIN, "saml2; {$user->logDescriptor()}"); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, 'saml2', $user); - + $this->loginService->login($user, 'saml2'); return $user; } } diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index 2f1a6876a..70f3fc3d3 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -2,15 +2,11 @@ namespace BookStack\Auth\Access; -use BookStack\Actions\ActivityType; use BookStack\Auth\SocialAccount; use BookStack\Auth\User; use BookStack\Exceptions\SocialDriverNotConfigured; use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Activity; -use BookStack\Facades\Theme; -use BookStack\Theming\ThemeEvents; use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\Factory as Socialite; @@ -28,6 +24,11 @@ class SocialAuthService */ protected $socialite; + /** + * @var LoginService + */ + protected $loginService; + /** * The default built-in social drivers we support. * @@ -59,9 +60,10 @@ class SocialAuthService /** * SocialAuthService constructor. */ - public function __construct(Socialite $socialite) + public function __construct(Socialite $socialite, LoginService $loginService) { $this->socialite = $socialite; + $this->loginService = $loginService; } /** @@ -139,10 +141,7 @@ class SocialAuthService // When a user is not logged in and a matching SocialAccount exists, // Simply log the user into the application. if (!$isLoggedIn && $socialAccount !== null) { - auth()->login($socialAccount->user); - Activity::add(ActivityType::AUTH_LOGIN, $socialAccount); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, $socialDriver, $socialAccount->user); - + $this->loginService->login($socialAccount->user, $socialAccount); return redirect()->intended('/'); } diff --git a/app/Http/Controllers/Auth/ConfirmEmailController.php b/app/Http/Controllers/Auth/ConfirmEmailController.php index 63f220a68..90e5fb76d 100644 --- a/app/Http/Controllers/Auth/ConfirmEmailController.php +++ b/app/Http/Controllers/Auth/ConfirmEmailController.php @@ -4,6 +4,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Actions\ActivityType; use BookStack\Auth\Access\EmailConfirmationService; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\UserRepo; use BookStack\Exceptions\ConfirmationEmailException; use BookStack\Exceptions\UserTokenExpiredException; @@ -20,14 +21,20 @@ use Illuminate\View\View; class ConfirmEmailController extends Controller { protected $emailConfirmationService; + protected $loginService; protected $userRepo; /** * Create a new controller instance. */ - public function __construct(EmailConfirmationService $emailConfirmationService, UserRepo $userRepo) + public function __construct( + EmailConfirmationService $emailConfirmationService, + LoginService $loginService, + UserRepo $userRepo + ) { $this->emailConfirmationService = $emailConfirmationService; + $this->loginService = $loginService; $this->userRepo = $userRepo; } @@ -87,9 +94,7 @@ class ConfirmEmailController extends Controller $user->email_confirmed = true; $user->save(); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); + $this->loginService->login($user, auth()->getDefaultDriver()); $this->showSuccessNotification(trans('auth.email_confirm_success')); $this->emailConfirmationService->deleteByUser($user); diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 5154f7e97..174770bf9 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -3,13 +3,11 @@ namespace BookStack\Http\Controllers\Auth; use Activity; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -37,16 +35,19 @@ class LoginController extends Controller protected $redirectAfterLogout = '/login'; protected $socialAuthService; + protected $loginService; /** * Create a new controller instance. */ - public function __construct(SocialAuthService $socialAuthService) + public function __construct(SocialAuthService $socialAuthService, LoginService $loginService) { $this->middleware('guest', ['only' => ['getLogin', 'login']]); $this->middleware('guard:standard,ldap', ['only' => ['login', 'logout']]); $this->socialAuthService = $socialAuthService; + $this->loginService = $loginService; + $this->redirectPath = url('/'); $this->redirectAfterLogout = url('/login'); } @@ -140,6 +141,19 @@ class LoginController extends Controller return $this->sendFailedLoginResponse($request); } + /** + * Attempt to log the user into the application. + * + * @param \Illuminate\Http\Request $request + * @return bool + */ + protected function attemptLogin(Request $request) + { + return $this->loginService->attempt( + $this->credentials($request), auth()->getDefaultDriver(), $request->filled('remember') + ); + } + /** * The user has been authenticated. * @@ -150,17 +164,6 @@ class LoginController extends Controller */ protected function authenticated(Request $request, $user) { - // Authenticate on all session guards if a likely admin - if ($user->can('users-manage') && $user->can('user-roles-manage')) { - $guards = ['standard', 'ldap', 'saml2']; - foreach ($guards as $guard) { - auth($guard)->login($user); - } - } - - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); - return redirect()->intended($this->redirectPath()); } diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 1728ece32..4008439bc 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -2,14 +2,12 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Auth\User; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Foundation\Auth\RegistersUsers; use Illuminate\Http\Request; use Illuminate\Support\Facades\Hash; @@ -32,6 +30,7 @@ class RegisterController extends Controller protected $socialAuthService; protected $registrationService; + protected $loginService; /** * Where to redirect users after login / registration. @@ -44,13 +43,18 @@ class RegisterController extends Controller /** * Create a new controller instance. */ - public function __construct(SocialAuthService $socialAuthService, RegistrationService $registrationService) + public function __construct( + SocialAuthService $socialAuthService, + RegistrationService $registrationService, + LoginService $loginService + ) { $this->middleware('guest'); $this->middleware('guard:standard'); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; + $this->loginService = $loginService; $this->redirectTo = url('/'); $this->redirectPath = url('/'); @@ -98,9 +102,7 @@ class RegisterController extends Controller try { $user = $this->registrationService->registerUser($userData); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); + $this->loginService->login($user, auth()->getDefaultDriver()); } catch (UserRegistrationException $exception) { if ($exception->getMessage()) { $this->showErrorNotification($exception->getMessage()); diff --git a/app/Http/Controllers/Auth/SocialController.php b/app/Http/Controllers/Auth/SocialController.php index 1d1468698..dd567fe18 100644 --- a/app/Http/Controllers/Auth/SocialController.php +++ b/app/Http/Controllers/Auth/SocialController.php @@ -2,16 +2,14 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\SocialDriverNotConfigured; use BookStack\Exceptions\SocialSignInAccountNotUsed; use BookStack\Exceptions\SocialSignInException; use BookStack\Exceptions\UserRegistrationException; -use BookStack\Facades\Theme; use BookStack\Http\Controllers\Controller; -use BookStack\Theming\ThemeEvents; use Illuminate\Http\Request; use Illuminate\Support\Str; use Laravel\Socialite\Contracts\User as SocialUser; @@ -20,15 +18,21 @@ class SocialController extends Controller { protected $socialAuthService; protected $registrationService; + protected $loginService; /** * SocialController constructor. */ - public function __construct(SocialAuthService $socialAuthService, RegistrationService $registrationService) + public function __construct( + SocialAuthService $socialAuthService, + RegistrationService $registrationService, + LoginService $loginService + ) { $this->middleware('guest')->only(['getRegister', 'postRegister']); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; + $this->loginService = $loginService; } /** @@ -136,12 +140,9 @@ class SocialController extends Controller } $user = $this->registrationService->registerUser($userData, $socialAccount, $emailVerified); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, $socialDriver, $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); + $this->loginService->login($user, $socialDriver); $this->showSuccessNotification(trans('auth.register_success')); - return redirect('/'); } } diff --git a/app/Http/Controllers/Auth/UserInviteController.php b/app/Http/Controllers/Auth/UserInviteController.php index 253e25507..1cc59d6ba 100644 --- a/app/Http/Controllers/Auth/UserInviteController.php +++ b/app/Http/Controllers/Auth/UserInviteController.php @@ -3,6 +3,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Actions\ActivityType; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\UserInviteService; use BookStack\Auth\UserRepo; use BookStack\Exceptions\UserTokenExpiredException; @@ -18,17 +19,19 @@ use Illuminate\Routing\Redirector; class UserInviteController extends Controller { protected $inviteService; + protected $loginService; protected $userRepo; /** * Create a new controller instance. */ - public function __construct(UserInviteService $inviteService, UserRepo $userRepo) + public function __construct(UserInviteService $inviteService, LoginService $loginService, UserRepo $userRepo) { $this->middleware('guest'); $this->middleware('guard:standard'); $this->inviteService = $inviteService; + $this->loginService = $loginService; $this->userRepo = $userRepo; } @@ -72,9 +75,7 @@ class UserInviteController extends Controller $user->email_confirmed = true; $user->save(); - auth()->login($user); - Theme::dispatch(ThemeEvents::AUTH_LOGIN, auth()->getDefaultDriver(), $user); - $this->logActivity(ActivityType::AUTH_LOGIN, $user); + $this->loginService->login($user, auth()->getDefaultDriver()); $this->showSuccessNotification(trans('auth.user_invite_success', ['appName' => setting('app-name')])); $this->inviteService->deleteByUser($user); diff --git a/app/Http/Middleware/EnforceMfaRequirements.php b/app/Http/Middleware/EnforceMfaRequirements.php index ac3c9609b..1877e5672 100644 --- a/app/Http/Middleware/EnforceMfaRequirements.php +++ b/app/Http/Middleware/EnforceMfaRequirements.php @@ -31,7 +31,7 @@ class EnforceMfaRequirements && !$request->is('mfa/verify*', 'uploads/images/user/*') && $this->mfaSession->requiredForCurrentUser() ) { - return redirect('/mfa/verify'); +// return redirect('/mfa/verify'); } // TODO - URI wildcard exceptions above allow access to the 404 page of this user diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index abee6bcda..f7c8fe492 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -3,6 +3,7 @@ namespace BookStack\Providers; use Blade; +use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\SocialAuthService; use BookStack\Entities\BreadcrumbsViewComposer; use BookStack\Entities\Models\Book; @@ -68,7 +69,7 @@ class AppServiceProvider extends ServiceProvider }); $this->app->singleton(SocialAuthService::class, function ($app) { - return new SocialAuthService($app->make(SocialiteFactory::class)); + return new SocialAuthService($app->make(SocialiteFactory::class), $app->make(LoginService::class)); }); } }