diff --git a/app/Auth/Access/ExternalAuthService.php b/app/Auth/Access/ExternalAuthService.php index 4bd8f8680..db8bd2dfb 100644 --- a/app/Auth/Access/ExternalAuthService.php +++ b/app/Auth/Access/ExternalAuthService.php @@ -64,10 +64,8 @@ class ExternalAuthService /** * Sync the groups to the user roles for the current user - * @param \BookStack\Auth\User $user - * @param array $userGroups */ - public function syncWithGroups(User $user, array $userGroups) + public function syncWithGroups(User $user, array $userGroups): void { // Get the ids for the roles from the names $groupsAsRoles = $this->matchGroupsToSystemsRoles($userGroups); @@ -75,7 +73,7 @@ class ExternalAuthService // Sync groups if ($this->config['remove_from_groups']) { $user->roles()->sync($groupsAsRoles); - $this->userRepo->attachDefaultRole($user); + $user->attachDefaultRole(); } else { $user->roles()->syncWithoutDetaching($groupsAsRoles); } diff --git a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php index d1fb0b606..f3d05366d 100644 --- a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php +++ b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php @@ -2,10 +2,7 @@ namespace BookStack\Auth\Access\Guards; -use BookStack\Auth\User; -use BookStack\Auth\UserRepo; -use BookStack\Exceptions\LoginAttemptEmailNeededException; -use BookStack\Exceptions\LoginAttemptException; +use BookStack\Auth\Access\RegistrationService; use Illuminate\Auth\GuardHelpers; use Illuminate\Contracts\Auth\Authenticatable as AuthenticatableContract; use Illuminate\Contracts\Auth\StatefulGuard; @@ -56,23 +53,23 @@ class ExternalBaseSessionGuard implements StatefulGuard protected $loggedOut = false; /** - * Repository to perform user-specific actions. + * Service to handle common registration actions. * - * @var UserRepo + * @var RegistrationService */ - protected $userRepo; + protected $registrationService; /** * Create a new authentication guard. * * @return void */ - public function __construct(string $name, UserProvider $provider, Session $session, UserRepo $userRepo) + public function __construct(string $name, UserProvider $provider, Session $session, RegistrationService $registrationService) { $this->name = $name; $this->session = $session; $this->provider = $provider; - $this->userRepo = $userRepo; + $this->registrationService = $registrationService; } /** diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php index 6c416bf30..3c98140f6 100644 --- a/app/Auth/Access/Guards/LdapSessionGuard.php +++ b/app/Auth/Access/Guards/LdapSessionGuard.php @@ -3,13 +3,17 @@ namespace BookStack\Auth\Access\Guards; use BookStack\Auth\Access\LdapService; +use BookStack\Auth\Access\RegistrationService; use BookStack\Auth\User; use BookStack\Auth\UserRepo; use BookStack\Exceptions\LdapException; use BookStack\Exceptions\LoginAttemptException; use BookStack\Exceptions\LoginAttemptEmailNeededException; +use BookStack\Exceptions\UserRegistrationException; use Illuminate\Contracts\Auth\UserProvider; use Illuminate\Contracts\Session\Session; +use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Str; class LdapSessionGuard extends ExternalBaseSessionGuard { @@ -23,11 +27,11 @@ class LdapSessionGuard extends ExternalBaseSessionGuard UserProvider $provider, Session $session, LdapService $ldapService, - UserRepo $userRepo + RegistrationService $registrationService ) { $this->ldapService = $ldapService; - parent::__construct($name, $provider, $session, $userRepo); + parent::__construct($name, $provider, $session, $registrationService); } /** @@ -56,6 +60,7 @@ class LdapSessionGuard extends ExternalBaseSessionGuard * @throws LoginAttemptEmailNeededException * @throws LoginAttemptException * @throws LdapException + * @throws UserRegistrationException */ public function attempt(array $credentials = [], $remember = false) { @@ -70,12 +75,9 @@ class LdapSessionGuard extends ExternalBaseSessionGuard } if (is_null($user)) { - $user = $this->freshUserInstanceFromLdapUserDetails($userDetails); + $user = $this->createNewFromLdapAndCreds($userDetails, $credentials); } - $this->checkForUserEmail($user, $credentials['email'] ?? ''); - $this->saveIfNew($user); - // Sync LDAP groups if required if ($this->ldapService->shouldSyncGroups()) { $this->ldapService->syncGroups($user, $username); @@ -86,58 +88,27 @@ class LdapSessionGuard extends ExternalBaseSessionGuard } /** - * Save the give user if they don't yet existing in the system. - * @throws LoginAttemptException - */ - protected function saveIfNew(User $user) - { - if ($user->exists) { - return; - } - - // 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); - } - - /** - * Ensure the given user has an email. - * Takes the provided email in the request if a value is provided - * and the user does not have an existing email. + * Create a new user from the given ldap credentials and login credentials * @throws LoginAttemptEmailNeededException + * @throws LoginAttemptException + * @throws UserRegistrationException */ - protected function checkForUserEmail(User $user, string $providedEmail) + protected function createNewFromLdapAndCreds(array $ldapUserDetails, array $credentials): User { - // Request email if missing from user and missing from request - if (is_null($user->email) && !$providedEmail) { + $email = trim($ldapUserDetails['email'] ?: ($credentials['email'] ?? '')); + + if (empty($email)) { throw new LoginAttemptEmailNeededException(); } - // Add email to model if non-existing and email provided in request - if (!$user->exists && is_null($user->email) && $providedEmail) { - $user->email = $providedEmail; - } - } + $details = [ + 'name' => $ldapUserDetails['name'], + 'email' => $ldapUserDetails['email'] ?: $credentials['email'], + 'external_auth_id' => $ldapUserDetails['uid'], + 'password' => Str::random(32), + ]; - /** - * 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 $this->registrationService->registerUser($details, null, false); } } diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index cc2890817..07e9f7b64 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -1,10 +1,8 @@ ldap = $ldap; $this->config = config('services.ldap'); - $this->userRepo = $userRepo; $this->enabled = config('auth.method') === 'ldap'; } diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 74142301a..8cf76013a 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -1,6 +1,7 @@ registrationAllowed()) { + throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login'); + } + } + + /** + * Check if standard BookStack User registrations are currently allowed. + * Does not prevent external-auth based registration. + */ + protected function registrationAllowed(): bool { $authMethod = config('auth.method'); $authMethodsWithRegistration = ['standard']; - if (!setting('registration-enabled') || !in_array($authMethod, $authMethodsWithRegistration)) { - throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login'); - } + return in_array($authMethod, $authMethodsWithRegistration) && setting('registration-enabled'); } /** * The registrations flow for all users. * @throws UserRegistrationException */ - public function registerUser(array $userData, ?SocialAccount $socialAccount = null, bool $emailVerified = false) + public function registerUser(array $userData, ?SocialAccount $socialAccount = null, bool $emailConfirmed = false): User { - $registrationRestrict = setting('registration-restrict'); + $userEmail = $userData['email']; - if ($registrationRestrict) { - $restrictedEmailDomains = explode(',', str_replace(' ', '', $registrationRestrict)); - $userEmailDomain = $domain = mb_substr(mb_strrchr($userData['email'], "@"), 1); - if (!in_array($userEmailDomain, $restrictedEmailDomains)) { - throw new UserRegistrationException(trans('auth.registration_email_domain_invalid'), '/register'); - } + // Email restriction + $this->ensureEmailDomainAllowed($userEmail); + + // Ensure user does not already exist + $alreadyUser = !is_null($this->userRepo->getByEmail($userEmail)); + if ($alreadyUser) { + throw new UserRegistrationException(trans('errors.error_user_exists_different_creds', ['email' => $userEmail])); } - $newUser = $this->userRepo->registerNew($userData, $emailVerified); + // Create the user + $newUser = $this->userRepo->registerNew($userData, $emailConfirmed); + // Assign social account if given if ($socialAccount) { $newUser->socialAccounts()->save($socialAccount); } - if ($this->emailConfirmationService->confirmationRequired() && !$emailVerified) { + // Start email confirmation flow if required + if ($this->emailConfirmationService->confirmationRequired() && !$emailConfirmed) { $newUser->save(); $message = ''; @@ -68,7 +82,37 @@ class RegistrationService throw new UserRegistrationException($message, '/register/confirm'); } - auth()->login($newUser); + return $newUser; + } + + /** + * Ensure that the given email meets any active email domain registration restrictions. + * Throws if restrictions are active and the email does not match an allowed domain. + * @throws UserRegistrationException + */ + protected function ensureEmailDomainAllowed(string $userEmail): void + { + $registrationRestrict = setting('registration-restrict'); + + if (!$registrationRestrict) { + return; + } + + $restrictedEmailDomains = explode(',', str_replace(' ', '', $registrationRestrict)); + $userEmailDomain = $domain = mb_substr(mb_strrchr($userEmail, "@"), 1); + if (!in_array($userEmailDomain, $restrictedEmailDomains)) { + $redirect = $this->registrationAllowed() ? '/register' : '/login'; + throw new UserRegistrationException(trans('auth.registration_email_domain_invalid'), $redirect); + } + } + + /** + * Alias to the UserRepo method of the same name. + * Attaches the default system role, if configured, to the given user. + */ + public function attachDefaultRole(User $user): void + { + $this->userRepo->attachDefaultRole($user); } } \ No newline at end of file diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index c52dc3a39..8f9a24cde 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -1,9 +1,9 @@ config = config('saml2'); - $this->userRepo = $userRepo; + $this->registrationService = $registrationService; $this->user = $user; } @@ -78,6 +78,7 @@ class Saml2Service extends ExternalAuthService * @throws SamlException * @throws ValidationError * @throws JsonDebugException + * @throws UserRegistrationException */ public function processAcsResponse(?string $requestId): ?User { @@ -308,34 +309,10 @@ class Saml2Service extends ExternalAuthService return $defaultValue; } - /** - * Register a user that is authenticated but not already registered. - */ - protected function registerUser(array $userDetails): User - { - // Create an array of the user data to create a new user instance - $userData = [ - 'name' => $userDetails['name'], - 'email' => $userDetails['email'], - 'password' => Str::random(32), - 'external_auth_id' => $userDetails['external_id'], - 'email_confirmed' => true, - ]; - - $existingUser = $this->user->newQuery()->where('email', '=', $userDetails['email'])->first(); - if ($existingUser) { - throw new SamlException(trans('errors.saml_email_exists', ['email' => $userDetails['email']])); - } - - $user = $this->user->newQuery()->forceCreate($userData); - $this->userRepo->attachDefaultRole($user); - $this->userRepo->downloadAndAssignUserAvatar($user); - return $user; - } - /** * Get the user from the database for the specified details. * @throws SamlException + * @throws UserRegistrationException */ protected function getOrRegisterUser(array $userDetails): ?User { @@ -344,7 +321,14 @@ class Saml2Service extends ExternalAuthService ->first(); if (is_null($user)) { - $user = $this->registerUser($userDetails); + $userData = [ + 'name' => $userDetails['name'], + 'email' => $userDetails['email'], + 'password' => Str::random(32), + 'external_auth_id' => $userDetails['external_id'], + ]; + + $user = $this->registrationService->registerUser($userData, null, false); } return $user; @@ -355,6 +339,7 @@ class Saml2Service extends ExternalAuthService * they exist, optionally registering them automatically. * @throws SamlException * @throws JsonDebugException + * @throws UserRegistrationException */ public function processLoginCallback(string $samlID, array $samlAttributes): User { diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index cf836618a..16815a8e1 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -64,7 +64,7 @@ class SocialAuthService if ($this->userRepo->getByEmail($socialUser->getEmail())) { $email = $socialUser->getEmail(); - throw new UserRegistrationException(trans('errors.social_account_in_use', ['socialAccount'=>$socialDriver, 'email' => $email]), '/login'); + throw new UserRegistrationException(trans('errors.error_user_exists_different_creds', ['email' => $email]), '/login'); } return $socialUser; @@ -124,7 +124,7 @@ class SocialAuthService // Otherwise let the user know this social account is not used by anyone. $message = trans('errors.social_account_not_used', ['socialAccount' => $titleCaseDriver]); - if (setting('registration-enabled') && config('auth.method') !== 'ldap') { + if (setting('registration-enabled') && config('auth.method') !== 'ldap' && config('auth.method') !== 'saml2') { $message .= trans('errors.social_account_register_instructions', ['socialAccount' => $titleCaseDriver]); } diff --git a/app/Auth/User.php b/app/Auth/User.php index 35b3cd54f..28fb9c7fc 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -116,6 +116,17 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon return $this->roles->pluck('system_name')->contains($role); } + /** + * Attach the default system role to this user. + */ + public function attachDefaultRole(): void + { + $roleId = setting('registration-role'); + if ($roleId && $this->roles()->where('id', '=', $roleId)->count() === 0) { + $this->roles()->attach($roleId); + } + } + /** * Get all permissions belonging to a the current user. * @param bool $cache @@ -153,16 +164,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function attachRole(Role $role) { - $this->attachRoleId($role->id); - } - - /** - * Attach a role id to this user. - * @param $id - */ - public function attachRoleId($id) - { - $this->roles()->attach($id); + $this->roles()->attach($role->id); } /** diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php index e082b2dd5..cfa7bfce1 100644 --- a/app/Auth/UserRepo.php +++ b/app/Auth/UserRepo.php @@ -29,10 +29,9 @@ class UserRepo } /** - * @param string $email - * @return User|null + * Get a user by their email address. */ - public function getByEmail($email) + public function getByEmail(string $email): ?User { return $this->user->where('email', '=', $email)->first(); } @@ -78,31 +77,16 @@ class UserRepo /** * Creates a new user and attaches a role to them. - * @param array $data - * @param boolean $verifyEmail - * @return User */ - public function registerNew(array $data, $verifyEmail = false) + public function registerNew(array $data, bool $emailConfirmed = false): User { - $user = $this->create($data, $verifyEmail); - $this->attachDefaultRole($user); + $user = $this->create($data, $emailConfirmed); + $user->attachDefaultRole(); $this->downloadAndAssignUserAvatar($user); return $user; } - /** - * Give a user the default role. Used when creating a new user. - * @param User $user - */ - public function attachDefaultRole(User $user) - { - $roleId = setting('registration-role'); - if ($roleId !== false && $user->roles()->where('id', '=', $roleId)->count() === 0) { - $user->attachRoleId($roleId); - } - } - /** * Assign a user to a system-level role. * @param User $user @@ -172,17 +156,15 @@ class UserRepo /** * Create a new basic instance of user. - * @param array $data - * @param boolean $verifyEmail - * @return User */ - public function create(array $data, $verifyEmail = false) + public function create(array $data, bool $emailConfirmed = false): User { return $this->user->forceCreate([ 'name' => $data['name'], 'email' => $data['email'], 'password' => bcrypt($data['password']), - 'email_confirmed' => $verifyEmail + 'email_confirmed' => $emailConfirmed, + 'external_auth_id' => $data['external_auth_id'] ?? '', ]); } diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 284d731d7..a3bc1e8b9 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -57,7 +57,10 @@ class Handler extends ExceptionHandler // Handle notify exceptions which will redirect to the // specified location then show a notification message. if ($this->isExceptionType($e, NotifyException::class)) { - session()->flash('error', $this->getOriginalMessage($e)); + $message = $this->getOriginalMessage($e); + if (!empty($message)) { + session()->flash('error', $message); + } return redirect($e->redirectLocation); } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 4292ad6dd..ea584a3b6 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -5,6 +5,7 @@ namespace BookStack\Http\Controllers\Auth; use BookStack\Auth\Access\SocialAuthService; use BookStack\Exceptions\LoginAttemptEmailNeededException; use BookStack\Exceptions\LoginAttemptException; +use BookStack\Exceptions\UserRegistrationException; use BookStack\Http\Controllers\Controller; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\Request; diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index 56ec66bff..0bdeef9e6 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -74,7 +74,7 @@ class RegisterController extends Controller */ public function getRegister() { - $this->registrationService->checkRegistrationAllowed(); + $this->registrationService->ensureRegistrationAllowed(); $socialDrivers = $this->socialAuthService->getActiveDrivers(); return view('auth.register', [ 'socialDrivers' => $socialDrivers, @@ -87,12 +87,13 @@ class RegisterController extends Controller */ public function postRegister(Request $request) { - $this->registrationService->checkRegistrationAllowed(); + $this->registrationService->ensureRegistrationAllowed(); $this->validator($request->all())->validate(); $userData = $request->all(); try { - $this->registrationService->registerUser($userData); + $user = $this->registrationService->registerUser($userData); + auth()->login($user); } catch (UserRegistrationException $exception) { if ($exception->getMessage()) { $this->showErrorNotification($exception->getMessage()); diff --git a/app/Http/Controllers/Auth/Saml2Controller.php b/app/Http/Controllers/Auth/Saml2Controller.php index 8c0cb21d2..7ffcc572b 100644 --- a/app/Http/Controllers/Auth/Saml2Controller.php +++ b/app/Http/Controllers/Auth/Saml2Controller.php @@ -81,7 +81,6 @@ class Saml2Controller extends Controller return redirect('/login'); } - session()->put('last_login_type', 'saml2'); return redirect()->intended(); } diff --git a/app/Http/Controllers/Auth/SocialController.php b/app/Http/Controllers/Auth/SocialController.php index bcd82a9c0..ae7a1bc06 100644 --- a/app/Http/Controllers/Auth/SocialController.php +++ b/app/Http/Controllers/Auth/SocialController.php @@ -49,7 +49,7 @@ class SocialController extends Controller */ public function socialRegister(string $socialDriver) { - $this->registrationService->checkRegistrationAllowed(); + $this->registrationService->ensureRegistrationAllowed(); session()->put('social-callback', 'register'); return $this->socialAuthService->startRegister($socialDriver); } @@ -78,7 +78,7 @@ class SocialController extends Controller // Attempt login or fall-back to register if allowed. $socialUser = $this->socialAuthService->getSocialUser($socialDriver); - if ($action == 'login') { + if ($action === 'login') { try { return $this->socialAuthService->handleLoginCallback($socialDriver, $socialUser); } catch (SocialSignInAccountNotUsed $exception) { @@ -89,7 +89,7 @@ class SocialController extends Controller } } - if ($action == 'register') { + if ($action === 'register') { return $this->socialRegisterCallback($socialDriver, $socialUser); } @@ -108,7 +108,6 @@ class SocialController extends Controller /** * Register a new user after a registration callback. - * @return RedirectResponse|Redirector * @throws UserRegistrationException */ protected function socialRegisterCallback(string $socialDriver, SocialUser $socialUser) @@ -121,17 +120,11 @@ class SocialController extends Controller $userData = [ 'name' => $socialUser->getName(), 'email' => $socialUser->getEmail(), - 'password' => Str::random(30) + 'password' => Str::random(32) ]; - try { - $this->registrationService->registerUser($userData, $socialAccount, $emailVerified); - } catch (UserRegistrationException $exception) { - if ($exception->getMessage()) { - $this->showErrorNotification($exception->getMessage()); - } - return redirect($exception->redirectLocation); - } + $user = $this->registrationService->registerUser($userData, $socialAccount, $emailVerified); + auth()->login($user); $this->showSuccessNotification(trans('auth.register_success')); return redirect('/'); diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index a885628f3..fe52df168 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -8,6 +8,7 @@ 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\Access\RegistrationService; use BookStack\Auth\UserRepo; use Illuminate\Support\ServiceProvider; @@ -31,7 +32,7 @@ class AuthServiceProvider extends ServiceProvider $provider, $this->app['session.store'], $app[LdapService::class], - $app[UserRepo::class] + $app[RegistrationService::class] ); }); @@ -41,7 +42,7 @@ class AuthServiceProvider extends ServiceProvider $name, $provider, $this->app['session.store'], - $app[UserRepo::class] + $app[RegistrationService::class] ); }); } diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index bb7b6148c..4752d8b0c 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -23,7 +23,6 @@ return [ 'saml_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', 'saml_invalid_response_id' => 'The request from the external authentication system is not recognised by a process started by this application. Navigating back after a login could cause this issue.', 'saml_fail_authed' => 'Login using :system failed, system did not provide successful authorization', - 'saml_email_exists' => 'Registration unsuccessful since a user already exists with email address ":email"', 'social_no_action_defined' => 'No action defined', 'social_login_bad_response' => "Error received during :socialAccount login: \n:error", 'social_account_in_use' => 'This :socialAccount account is already in use, Try logging in via the :socialAccount option.', diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index 8c9675f09..692489afd 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -56,7 +56,7 @@ return [ 'reg_enable_toggle' => 'Enable registration', 'reg_enable_desc' => 'When registration is enabled user will be able to sign themselves up as an application user. Upon registration they are given a single, default user role.', 'reg_default_role' => 'Default user role after registration', - 'reg_enable_ldap_warning' => 'The option above is not used while LDAP authentication is active. User accounts for non-existing members will be auto-created if authentication, against the LDAP system in use, is successful.', + 'reg_enable_external_warning' => 'The option above is ignore while external LDAP or SAML authentication is active. User accounts for non-existing members will be auto-created if authentication, against the external system in use, is successful.', 'reg_email_confirmation' => 'Email Confirmation', 'reg_email_confirmation_toggle' => 'Require email confirmation', 'reg_confirm_email_desc' => 'If domain restriction is used then email confirmation will be required and this option will be ignored.', diff --git a/resources/views/settings/index.blade.php b/resources/views/settings/index.blade.php index 94605da6f..2585ec5e3 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -219,8 +219,8 @@ 'label' => trans('settings.reg_enable_toggle') ]) - @if(config('auth.method') === 'ldap') -
{{ trans('settings.reg_enable_ldap_warning') }}
+ @if(in_array(config('auth.method'), ['ldap', 'saml2'])) +
{{ trans('settings.reg_enable_external_warning') }}
@endif diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 92ff4a829..cb1194e22 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -86,6 +86,38 @@ class LdapTest extends BrowserKitTest ->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name]); } + public function test_email_domain_restriction_active_on_new_ldap_login() + { + $this->setSettings([ + 'registration-restrict' => 'testing.com' + ]); + + $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->once(); + $this->mockLdap->shouldReceive('setOption')->times(2); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) + ->andReturn(['count' => 1, 0 => [ + 'uid' => [$this->mockUser->name], + 'cn' => [$this->mockUser->name], + 'dn' => ['dc=test' . config('services.ldap.base_dn')] + ]]); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(2); + + $this->mockUserLogin() + ->seePageIs('/login') + ->see('Please enter an email to use for this account.'); + + $email = 'tester@invaliddomain.com'; + + $this->type($email, '#email') + ->press('Log In') + ->seePageIs('/login') + ->see('That email domain does not have access to this application') + ->dontSeeInDatabase('users', ['email' => $email]); + } + public function test_login_works_when_no_uid_provided_by_ldap_server() { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php index b3e6bd41b..9a3d6d8ec 100644 --- a/tests/Auth/Saml2Test.php +++ b/tests/Auth/Saml2Test.php @@ -73,7 +73,7 @@ class Saml2Test extends TestCase $this->assertDatabaseHas('users', [ 'email' => 'user@example.com', 'external_auth_id' => 'user', - 'email_confirmed' => true, + 'email_confirmed' => false, 'name' => 'Barry Scott' ]); @@ -209,7 +209,7 @@ class Saml2Test extends TestCase $acsPost = $this->post('/saml2/acs'); $acsPost->assertRedirect('/'); $errorMessage = session()->get('error'); - $this->assertEquals('Registration unsuccessful since a user already exists with email address "user@example.com"', $errorMessage); + $this->assertEquals('A user with the email user@example.com already exists but with different credentials.', $errorMessage); }); } @@ -271,6 +271,24 @@ class Saml2Test extends TestCase $this->assertPermissionError($resp); } + public function test_email_domain_restriction_active_on_new_saml_login() + { + $this->setSettings([ + 'registration-restrict' => 'testing.com' + ]); + config()->set([ + 'saml2.onelogin.strict' => false, + ]); + + $this->withPost(['SAMLResponse' => $this->acsPostData], function () { + $acsPost = $this->post('/saml2/acs'); + $acsPost->assertRedirect('/login'); + $errorMessage = session()->get('error'); + $this->assertStringContainsString('That email domain does not have access to this application', $errorMessage); + $this->assertDatabaseMissing('users', ['email' => 'user@example.com']); + }); + } + protected function withGet(array $options, callable $callback) { return $this->withGlobal($_GET, $options, $callback);