Checked over and aligned registration option behavior across all auth options

- Added tests to cover

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);
         }

app/Auth/Access/Guards/ExternalBaseSessionGuard.php

@@ -2,10 +2,7 @@
 
 namespace BookStack\Auth\Access\Guards;
 
-use BookStack\Auth\User;
+use BookStack\Auth\Access\RegistrationService;
-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;
@@ -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;
     }
 
     /**

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.
+     * Create a new user from the given ldap credentials and login credentials
-     * @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.
      * @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
+        $email = trim($ldapUserDetails['email'] ?: ($credentials['email'] ?? ''));
-        if (is_null($user->email) && !$providedEmail) {
+
+        if (empty($email)) {
             throw new LoginAttemptEmailNeededException();
         }
 
-        // Add email to model if non-existing and email provided in request
+        $details = [
-        if (!$user->exists && is_null($user->email) && $providedEmail) {
+            'name' => $ldapUserDetails['name'],
-            $user->email = $providedEmail;
+            'email' => $ldapUserDetails['email'] ?: $credentials['email'],
-        }
+            'external_auth_id' => $ldapUserDetails['uid'],
-    }
+            'password' => Str::random(32),
+        ];
 
-    /**
+        return $this->registrationService->registerUser($details, null, false);
-     * 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

@@ -1,10 +1,8 @@
 <?php namespace BookStack\Auth\Access;
 
 use BookStack\Auth\User;
-use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\LdapException;
 use ErrorException;
-use Illuminate\Contracts\Auth\Authenticatable;
 
 /**
  * Class LdapService
@@ -16,17 +14,15 @@ class LdapService extends ExternalAuthService
     protected $ldap;
     protected $ldapConnection;
     protected $config;
-    protected $userRepo;
     protected $enabled;
 
     /**
      * LdapService constructor.
      */
-    public function __construct(Ldap $ldap, UserRepo $userRepo)
+    public function __construct(Ldap $ldap)
     {
         $this->ldap = $ldap;
         $this->config = config('services.ldap');
-        $this->userRepo = $userRepo;
         $this->enabled = config('auth.method') === 'ldap';
     }
 

app/Auth/Access/RegistrationService.php

@@ -1,6 +1,7 @@
 <?php namespace BookStack\Auth\Access;
 
 use BookStack\Auth\SocialAccount;
+use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\UserRegistrationException;
 use Exception;
@@ -24,38 +25,51 @@ class RegistrationService
      * Check whether or not registrations are allowed in the app settings.
      * @throws UserRegistrationException
      */
-    public function checkRegistrationAllowed()
+    public function ensureRegistrationAllowed()
+    {
+        if (!$this->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)) {
+        return in_array($authMethod, $authMethodsWithRegistration) && setting('registration-enabled');
-            throw new UserRegistrationException(trans('auth.registrations_disabled'), '/login');
-        }
     }
 
     /**
      * 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) {
+        // Email restriction
-            $restrictedEmailDomains = explode(',', str_replace(' ', '', $registrationRestrict));
+        $this->ensureEmailDomainAllowed($userEmail);
-            $userEmailDomain = $domain = mb_substr(mb_strrchr($userData['email'], "@"), 1);
+
-            if (!in_array($userEmailDomain, $restrictedEmailDomains)) {
+        // Ensure user does not already exist
-                throw new UserRegistrationException(trans('auth.registration_email_domain_invalid'), '/register');
+        $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);
     }
 
 }

app/Auth/Access/Saml2Service.php

@@ -1,9 +1,9 @@
 <?php namespace BookStack\Auth\Access;
 
 use BookStack\Auth\User;
-use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\JsonDebugException;
 use BookStack\Exceptions\SamlException;
+use BookStack\Exceptions\UserRegistrationException;
 use Exception;
 use Illuminate\Support\Str;
 use OneLogin\Saml2\Auth;
@@ -18,16 +18,16 @@ use OneLogin\Saml2\ValidationError;
 class Saml2Service extends ExternalAuthService
 {
     protected $config;
-    protected $userRepo;
+    protected $registrationService;
     protected $user;
 
     /**
      * Saml2Service constructor.
      */
-    public function __construct(UserRepo $userRepo, User $user)
+    public function __construct(RegistrationService $registrationService, User $user)
     {
         $this->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
     {

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]);
         }
         

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);
+        $this->roles()->attach($role->id);
-    }
-
-    /**
-     * Attach a role id to this user.
-     * @param $id
-     */
-    public function attachRoleId($id)
-    {
-        $this->roles()->attach($id);
     }
 
     /**

app/Auth/UserRepo.php

@@ -29,10 +29,9 @@ class UserRepo
     }
 
     /**
-     * @param string $email
+     * Get a user by their email address.
-     * @return User|null
      */
-    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);
+        $user = $this->create($data, $emailConfirmed);
-        $this->attachDefaultRole($user);
+        $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'] ?? '',
         ]);
     }
 

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);
         }
 

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;

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());

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();
     }
 

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 {
+        $user = $this->registrationService->registerUser($userData, $socialAccount, $emailVerified);
-            $this->registrationService->registerUser($userData, $socialAccount, $emailVerified);
+        auth()->login($user);
-        } catch (UserRegistrationException $exception) {
-            if ($exception->getMessage()) {
-                $this->showErrorNotification($exception->getMessage());
-            }
-            return redirect($exception->redirectLocation);
-        }
 
         $this->showSuccessNotification(trans('auth.register_success'));
         return redirect('/');

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]
             );
         });
     }

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.',

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.',

resources/views/settings/index.blade.php

@@ -219,8 +219,8 @@
                                 'label' => trans('settings.reg_enable_toggle')
                             ])
 
-                            @if(config('auth.method') === 'ldap')
+                            @if(in_array(config('auth.method'), ['ldap', 'saml2']))
-                                <div class="text-warn text-small mb-l">{{ trans('settings.reg_enable_ldap_warning') }}</div>
+                                <div class="text-warn text-small mb-l">{{ trans('settings.reg_enable_external_warning') }}</div>
                             @endif
 
                             <label for="setting-registration-role">{{ trans('settings.reg_default_role') }}</label>

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);

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);