diff --git a/.env.example.complete b/.env.example.complete index e44644f08..bc6b644aa 100644 --- a/.env.example.complete +++ b/.env.example.complete @@ -121,7 +121,7 @@ STORAGE_S3_ENDPOINT=https://my-custom-s3-compatible.service.com:8001 STORAGE_URL=false # Authentication method to use -# Can be 'standard' or 'ldap' +# Can be 'standard', 'ldap' or 'saml2' AUTH_METHOD=standard # Social authentication configuration @@ -205,8 +205,6 @@ LDAP_REMOVE_FROM_GROUPS=false # SAML authentication configuration # Refer to https://www.bookstackapp.com/docs/admin/saml2-auth/ SAML2_NAME=SSO -SAML2_ENABLED=false -SAML2_AUTO_REGISTER=true SAML2_EMAIL_ATTRIBUTE=email SAML2_DISPLAY_NAME_ATTRIBUTES=username SAML2_EXTERNAL_ID_ATTRIBUTE=null diff --git a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php index 3022b7f8e..d1fb0b606 100644 --- a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php @@ -2,6 +2,10 @@ namespace BookStack\Auth\Access\Guards; +use BookStack\Auth\User; +use BookStack\Auth\UserRepo; +use BookStack\Exceptions\LoginAttemptEmailNeededException; +use BookStack\Exceptions\LoginAttemptException; use Illuminate\Auth\GuardHelpers; use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract; use Illuminate\Contracts\Auth\StatefulGuard; @@ -51,21 +55,24 @@ class ExternalBaseSessionGuard implements StatefulGuard */ protected $loggedOut = false; + /** + * Repository to perform user-specific actions. + * + * @var UserRepo + */ + protected $userRepo; + /** * 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) + public function __construct(string $name, UserProvider $provider, Session $session, UserRepo $userRepo) { $this->name = $name; $this->session = $session; $this->provider = $provider; + $this->userRepo = $userRepo; } /** diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php index 223088d05..6c416bf30 100644 --- a/app/Auth/Access/Guards/LdapSessionGuard.php +++ b/app/Auth/Access/Guards/LdapSessionGuard.php @@ -15,7 +15,6 @@ class LdapSessionGuard extends ExternalBaseSessionGuard { protected $ldapService; - protected $userRepo; /** * LdapSessionGuard constructor. @@ -28,8 +27,7 @@ class LdapSessionGuard extends ExternalBaseSessionGuard ) { $this->ldapService = $ldapService; - $this->userRepo = $userRepo; - parent::__construct($name, $provider, $session); + parent::__construct($name, $provider, $session, $userRepo); } /** diff --git a/app/Auth/Access/Guards/Saml2SessionGuard.php b/app/Auth/Access/Guards/Saml2SessionGuard.php index 1bdb59d51..4023913ed 100644 --- a/app/Auth/Access/Guards/Saml2SessionGuard.php +++ b/app/Auth/Access/Guards/Saml2SessionGuard.php @@ -2,49 +2,27 @@ 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 +/** + * Saml2 Session Guard + * + * The saml2 login process is async in nature meaning it does not fit very well + * into the default laravel 'Guard' auth flow. Instead most of the logic is done + * via the Saml2 controller & Saml2Service. This class provides a safer, thin + * version of SessionGuard. + * + * @package BookStack\Auth\Access\Guards + */ +class Saml2SessionGuard extends ExternalBaseSessionGuard { - - protected $ldapService; - - /** - * LdapSessionGuard constructor. - */ - public function __construct($name, - UserProvider $provider, - Session $session, - LdapService $ldapService, - UserRepo $userRepo - ) - { - $this->ldapService = $ldapService; - parent::__construct($name, $provider, $session, $userRepo); - } - /** * 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']); + return false; } /** @@ -53,51 +31,10 @@ class LdapSessionGuard extends ExternalBaseSessionGuard * @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); - } - - $this->checkForUserEmail($user, $credentials['email'] ?? ''); - $this->saveIfNew($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; + return false; } } diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 7e8c7d5f5..74142301a 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -20,14 +20,15 @@ class RegistrationService $this->emailConfirmationService = $emailConfirmationService; } - /** * Check whether or not registrations are allowed in the app settings. * @throws UserRegistrationException */ public function checkRegistrationAllowed() { - if (!setting('registration-enabled') || config('auth.method') === 'ldap') { + $authMethod = config('auth.method'); + $authMethodsWithRegistration = ['standard']; + if (!setting('registration-enabled') || !in_array($authMethod, $authMethodsWithRegistration)) { throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login'); } } diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index c1038e730..c52dc3a39 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -20,7 +20,6 @@ class Saml2Service extends ExternalAuthService protected $config; protected $userRepo; protected $user; - protected $enabled; /** * Saml2Service constructor. @@ -30,7 +29,6 @@ class Saml2Service extends ExternalAuthService $this->config = config('saml2'); $this->userRepo = $userRepo; $this->user = $user; - $this->enabled = config('saml2.enabled') === true; } /** @@ -204,7 +202,7 @@ class Saml2Service extends ExternalAuthService */ protected function shouldSyncGroups(): bool { - return $this->enabled && $this->config['user_to_groups'] !== false; + return $this->config['user_to_groups'] !== false; } /** @@ -248,7 +246,7 @@ class Saml2Service extends ExternalAuthService /** * Extract the details of a user from a SAML response. */ - public function getUserDetails(string $samlID, $samlAttributes): array + protected function getUserDetails(string $samlID, $samlAttributes): array { $emailAttr = $this->config['email_attribute']; $externalId = $this->getExternalId($samlAttributes, $samlID); @@ -329,7 +327,7 @@ class Saml2Service extends ExternalAuthService throw new SamlException(trans('errors.saml_email_exists', ['email' => $userDetails['email']])); } - $user = $this->user->forceCreate($userData); + $user = $this->user->newQuery()->forceCreate($userData); $this->userRepo->attachDefaultRole($user); $this->userRepo->downloadAndAssignUserAvatar($user); return $user; @@ -337,15 +335,15 @@ class Saml2Service extends ExternalAuthService /** * Get the user from the database for the specified details. + * @throws SamlException */ protected function getOrRegisterUser(array $userDetails): ?User { - $isRegisterEnabled = $this->config['auto_register'] === true; - $user = $this->user - ->where('external_auth_id', $userDetails['external_id']) + $user = $this->user->newQuery() + ->where('external_auth_id', '=', $userDetails['external_id']) ->first(); - if ($user === null && $isRegisterEnabled) { + if (is_null($user)) { $user = $this->registerUser($userDetails); } diff --git a/app/Config/auth.php b/app/Config/auth.php index 0be5aeee8..2afb10ec2 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -34,7 +34,11 @@ return [ ], 'ldap' => [ 'driver' => 'ldap-session', - 'provider' => 'external' + 'provider' => 'external', + ], + 'saml2' => [ + 'driver' => 'saml2-session', + 'provider' => 'external', ], 'api' => [ 'driver' => 'api-token', diff --git a/app/Config/saml2.php b/app/Config/saml2.php index 2f2ad14f1..5f2c1395b 100644 --- a/app/Config/saml2.php +++ b/app/Config/saml2.php @@ -4,10 +4,6 @@ return [ // Display name, shown to users, for SAML2 option 'name' => env('SAML2_NAME', 'SSO'), - // Toggle whether the SAML2 option is active - 'enabled' => env('SAML2_ENABLED', false), - // Enable registration via SAML2 authentication - 'auto_register' => env('SAML2_AUTO_REGISTER', true), // Dump user details after a login request for debugging purposes 'dump_user_details' => env('SAML2_DUMP_USER_DETAILS', false), diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 2302937cb..8116288ad 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -65,7 +65,6 @@ class LoginController extends Controller { $socialDrivers = $this->socialAuthService->getActiveDrivers(); $authMethod = config('auth.method'); - $samlEnabled = config('saml2.enabled') === true; if ($request->has('email')) { session()->flashInput([ @@ -77,7 +76,6 @@ class LoginController extends Controller return view('auth.login', [ 'socialDrivers' => $socialDrivers, 'authMethod' => $authMethod, - 'samlEnabled' => $samlEnabled, ]); } @@ -129,28 +127,16 @@ class LoginController extends Controller */ protected function validateLogin(Request $request) { - $rules = []; + $rules = ['password' => 'required|string']; $authMethod = config('auth.method'); if ($authMethod === 'standard') { - $rules = [ - 'email' => 'required|string|email', - 'password' => 'required|string' - ]; + $rules['email'] = 'required|email'; } if ($authMethod === 'ldap') { - $rules = [ - 'username' => 'required|string', - 'password' => 'required|string', - 'email' => 'email', - ]; - } - - if ($authMethod === 'saml2') { - $rules = [ - 'email' => 'email', - ]; + $rules['username'] = 'required|string'; + $rules['email'] = 'email'; } $request->validate($rules); @@ -178,10 +164,6 @@ class LoginController extends Controller */ public function logout(Request $request) { - if (config('saml2.enabled') && session()->get('last_login_type') === 'saml2') { - return redirect('/saml2/logout'); - } - $this->guard()->logout(); $request->session()->invalidate(); diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index c9c0c3ec5..8fb13d536 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -75,10 +75,8 @@ class RegisterController extends Controller { $this->registrationService->checkRegistrationAllowed(); $socialDrivers = $this->socialAuthService->getActiveDrivers(); - $samlEnabled = (config('saml2.enabled') === true) && (config('saml2.auto_register') === true); return view('auth.register', [ 'socialDrivers' => $socialDrivers, - 'samlEnabled' => $samlEnabled, ]); } diff --git a/app/Http/Controllers/Auth/Saml2Controller.php b/app/Http/Controllers/Auth/Saml2Controller.php index 863894128..72cf0e019 100644 --- a/app/Http/Controllers/Auth/Saml2Controller.php +++ b/app/Http/Controllers/Auth/Saml2Controller.php @@ -20,7 +20,8 @@ class Saml2Controller extends Controller // SAML2 access middleware $this->middleware(function ($request, $next) { - if (!config('saml2.enabled')) { + + if (config('auth.method') !== 'saml2') { $this->showPermissionError(); } diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index 0b299551a..a885628f3 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -6,6 +6,7 @@ use Auth; use BookStack\Api\ApiTokenGuard; use BookStack\Auth\Access\ExternalBaseUserProvider; use BookStack\Auth\Access\Guards\LdapSessionGuard; +use BookStack\Auth\Access\Guards\Saml2SessionGuard; use BookStack\Auth\Access\LdapService; use BookStack\Auth\UserRepo; use Illuminate\Support\ServiceProvider; @@ -33,6 +34,16 @@ class AuthServiceProvider extends ServiceProvider $app[UserRepo::class] ); }); + + Auth::extend('saml2-session', function ($app, $name, array $config) { + $provider = Auth::createUserProvider($config['provider']); + return new Saml2SessionGuard( + $name, + $provider, + $this->app['session.store'], + $app[UserRepo::class] + ); + }); } /** diff --git a/resources/views/auth/forms/login/ldap.blade.php b/resources/views/auth/forms/login/ldap.blade.php index 2699fda00..12592d492 100644 --- a/resources/views/auth/forms/login/ldap.blade.php +++ b/resources/views/auth/forms/login/ldap.blade.php @@ -1,19 +1,30 @@ -
- - @include('form.text', ['name' => 'username', 'autofocus' => true]) -
+
+ {!! csrf_field() !!} -@if(session('request-email', false) === true) -
- - @include('form.text', ['name' => 'email']) - - {{ trans('auth.ldap_email_hint') }} - +
+
+ + @include('form.text', ['name' => 'username', 'autofocus' => true]) +
+ + @if(session('request-email', false) === true) +
+ + @include('form.text', ['name' => 'email']) + {{ trans('auth.ldap_email_hint') }} +
+ @endif + +
+ + @include('form.password', ['name' => 'password']) +
-@endif -
- - @include('form.password', ['name' => 'password']) -
\ No newline at end of file +
+
+ +
+
+ + \ No newline at end of file diff --git a/resources/views/auth/forms/login/saml2.blade.php b/resources/views/auth/forms/login/saml2.blade.php index 12592d492..aa1913e31 100644 --- a/resources/views/auth/forms/login/saml2.blade.php +++ b/resources/views/auth/forms/login/saml2.blade.php @@ -1,30 +1,11 @@ -
+ {!! csrf_field() !!} -
-
- - @include('form.text', ['name' => 'username', 'autofocus' => true]) -
- - @if(session('request-email', false) === true) -
- - @include('form.text', ['name' => 'email']) - {{ trans('auth.ldap_email_hint') }} -
- @endif - -
- - @include('form.password', ['name' => 'password']) -
-
- -
-
- -
+
+
\ No newline at end of file diff --git a/resources/views/auth/forms/login/standard.blade.php b/resources/views/auth/forms/login/standard.blade.php index 52fae3750..87603e2cb 100644 --- a/resources/views/auth/forms/login/standard.blade.php +++ b/resources/views/auth/forms/login/standard.blade.php @@ -1,12 +1,36 @@ -
- - @include('form.text', ['name' => 'email', 'autofocus' => true]) -
+
+ {!! csrf_field() !!} + +
+
+ + @include('form.text', ['name' => 'email', 'autofocus' => true]) +
+ +
+ + @include('form.password', ['name' => 'password']) +
+ {{ trans('auth.forgot_password') }} +
+
+
+ +
+
+ @include('components.custom-checkbox', [ + 'name' => 'remember', + 'checked' => false, + 'value' => 'on', + 'label' => trans('auth.remember_me'), + ]) +
+ +
+ +
+
+ +
+ -
- - @include('form.password', ['name' => 'password']) - - {{ trans('auth.forgot_password') }} - -
diff --git a/resources/views/auth/login.blade.php b/resources/views/auth/login.blade.php index 098ce2100..0a21a0f62 100644 --- a/resources/views/auth/login.blade.php +++ b/resources/views/auth/login.blade.php @@ -9,29 +9,7 @@

{{ Str::title(trans('auth.log_in')) }}

-
- {!! csrf_field() !!} - -
- @include('auth.forms.login.' . $authMethod) -
- -
-
- @include('components.custom-checkbox', [ - 'name' => 'remember', - 'checked' => false, - 'value' => 'on', - 'label' => trans('auth.remember_me'), - ]) -
- -
- -
-
- -
+ @include('auth.forms.login.' . $authMethod) @if(count($socialDrivers) > 0)
@@ -45,17 +23,7 @@ @endforeach @endif - @if($samlEnabled) -
- - @endif - - @if(setting('registration-enabled') && config('auth.method') !== 'ldap') + @if(setting('registration-enabled') && config('auth.method') === 'standard')
{{ trans('auth.dont_have_account') }} diff --git a/resources/views/auth/register.blade.php b/resources/views/auth/register.blade.php index 8dd6592c1..1625ebc4c 100644 --- a/resources/views/auth/register.blade.php +++ b/resources/views/auth/register.blade.php @@ -50,15 +50,6 @@ @endforeach @endif - @if($samlEnabled) -
- - @endif
@stop diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index b06036031..7c837ec29 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -42,7 +42,7 @@ @endif @if(!signedInUser()) - @if(setting('registration-enabled') && config('auth.method') !== 'ldap') + @if(setting('registration-enabled') && config('auth.method') === 'standard') @icon('new-user') {{ trans('auth.sign_up') }} @endif @icon('login') {{ trans('auth.log_in') }} @@ -64,7 +64,11 @@ id}") }}">@icon('edit'){{ trans('common.edit_profile') }}
  • - @icon('logout'){{ trans('auth.logout') }} + @if(config('auth.method') === 'saml2') + @icon('logout'){{ trans('auth.logout') }} + @else + @icon('logout'){{ trans('auth.logout') }} + @endif
    • diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php index 1fbc80b1f..ed57ad944 100644 --- a/resources/views/settings/roles/form.blade.php +++ b/resources/views/settings/roles/form.blade.php @@ -19,7 +19,7 @@ @include('form.text', ['name' => 'description'])
    - @if(config('auth.method') === 'ldap' || config('saml2.enabled') === true) + @if(config('auth.method') === 'ldap' || config('auth.method') === 'saml2')
    @include('form.text', ['name' => 'external_auth_id']) diff --git a/resources/views/users/form.blade.php b/resources/views/users/form.blade.php index 6eafd43bc..df3d06c2f 100644 --- a/resources/views/users/form.blade.php +++ b/resources/views/users/form.blade.php @@ -25,7 +25,7 @@
    -@if(($authMethod === 'ldap' || config('saml2.enabled') === true) && userCan('users-manage')) +@if(($authMethod === 'ldap' || $authMethod === 'saml2') && userCan('users-manage'))
    diff --git a/routes/web.php b/routes/web.php index eafad6489..3aa95dd68 100644 --- a/routes/web.php +++ b/routes/web.php @@ -225,7 +225,7 @@ Route::get('/register/confirm/{token}', 'Auth\ConfirmEmailController@confirm'); Route::post('/register', 'Auth\RegisterController@postRegister'); // SAML routes -Route::get('/saml2/login', 'Auth\Saml2Controller@login'); +Route::post('/saml2/login', 'Auth\Saml2Controller@login'); Route::get('/saml2/logout', 'Auth\Saml2Controller@logout'); Route::get('/saml2/metadata', 'Auth\Saml2Controller@metadata'); Route::get('/saml2/sls', 'Auth\Saml2Controller@sls');