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/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/Providers/LdapUserProvider.php b/app/Auth/Access/ExternalBaseUserProvider.php similarity index 61% rename from app/Providers/LdapUserProvider.php rename to app/Auth/Access/ExternalBaseUserProvider.php index 9c91def2f..69295ee4e 100644 --- a/app/Providers/LdapUserProvider.php +++ b/app/Auth/Access/ExternalBaseUserProvider.php @@ -1,12 +1,11 @@ model = $model; - $this->ldapService = $ldapService; } /** @@ -44,7 +35,6 @@ class LdapUserProvider implements UserProvider return new $class; } - /** * Retrieve a user by their unique identifier. * @@ -65,12 +55,7 @@ class LdapUserProvider implements UserProvider */ public function retrieveByToken($identifier, $token) { - $model = $this->createModel(); - - return $model->newQuery() - ->where($model->getAuthIdentifierName(), $identifier) - ->where($model->getRememberTokenName(), $token) - ->first(); + return null; } @@ -83,10 +68,7 @@ class LdapUserProvider implements UserProvider */ public function updateRememberToken(Authenticatable $user, $token) { - if ($user->exists) { - $user->setRememberToken($token); - $user->save(); - } + // } /** @@ -97,27 +79,11 @@ class LdapUserProvider implements UserProvider */ public function retrieveByCredentials(array $credentials) { - // Get user via LDAP - $userDetails = $this->ldapService->getUserDetails($credentials['username']); - if ($userDetails === null) { - return null; - } - // Search current user base by looking up a uid $model = $this->createModel(); - $currentUser = $model->newQuery() - ->where('external_auth_id', $userDetails['uid']) + return $model->newQuery() + ->where('external_auth_id', $credentials['external_auth_id']) ->first(); - - if ($currentUser !== null) { - return $currentUser; - } - - $model->name = $userDetails['name']; - $model->external_auth_id = $userDetails['uid']; - $model->email = $userDetails['email']; - $model->email_confirmed = false; - return $model; } /** @@ -129,6 +95,7 @@ class LdapUserProvider implements UserProvider */ public function validateCredentials(Authenticatable $user, array $credentials) { - return $this->ldapService->validateUserCredentials($user, $credentials['username'], $credentials['password']); + // Should be done in the guard. + return false; } } diff --git a/app/Auth/Access/Guards/ExternalBaseSessionGuard.php b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php new file mode 100644 index 000000000..f3d05366d --- /dev/null +++ b/app/Auth/Access/Guards/ExternalBaseSessionGuard.php @@ -0,0 +1,305 @@ +name = $name; + $this->session = $session; + $this->provider = $provider; + $this->registrationService = $registrationService; + } + + /** + * Get the currently authenticated user. + * + * @return \Illuminate\Contracts\Auth\Authenticatable|null + */ + public function user() + { + if ($this->loggedOut) { + return; + } + + // If we've already retrieved the user for the current request we can just + // return it back immediately. We do not want to fetch the user data on + // every call to this method because that would be tremendously slow. + if (! is_null($this->user)) { + return $this->user; + } + + $id = $this->session->get($this->getName()); + + // First we will try to load the user using the + // identifier in the session if one exists. + if (! is_null($id)) { + $this->user = $this->provider->retrieveById($id); + } + + return $this->user; + } + + /** + * Get the ID for the currently authenticated user. + * + * @return int|null + */ + public function id() + { + if ($this->loggedOut) { + return; + } + + return $this->user() + ? $this->user()->getAuthIdentifier() + : $this->session->get($this->getName()); + } + + /** + * Log a user into the application without sessions or cookies. + * + * @param array $credentials + * @return bool + */ + public function once(array $credentials = []) + { + if ($this->validate($credentials)) { + $this->setUser($this->lastAttempted); + + return true; + } + + return false; + } + + /** + * Log the given user ID into the application without sessions or cookies. + * + * @param mixed $id + * @return \Illuminate\Contracts\Auth\Authenticatable|false + */ + public function onceUsingId($id) + { + if (! is_null($user = $this->provider->retrieveById($id))) { + $this->setUser($user); + + return $user; + } + + return false; + } + + /** + * Validate a user's credentials. + * + * @param array $credentials + * @return bool + */ + public function validate(array $credentials = []) + { + return false; + } + + + /** + * Attempt to authenticate a user using the given credentials. + * + * @param array $credentials + * @param bool $remember + * @return bool + */ + public function attempt(array $credentials = [], $remember = false) + { + return false; + } + + /** + * Log the given user ID into the application. + * + * @param mixed $id + * @param bool $remember + * @return \Illuminate\Contracts\Auth\Authenticatable|false + */ + public function loginUsingId($id, $remember = false) + { + if (! is_null($user = $this->provider->retrieveById($id))) { + $this->login($user, $remember); + + return $user; + } + + return false; + } + + /** + * Log a user into the application. + * + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @param bool $remember + * @return void + */ + public function login(AuthenticatableContract $user, $remember = false) + { + $this->updateSession($user->getAuthIdentifier()); + + $this->setUser($user); + } + + /** + * Update the session with the given ID. + * + * @param string $id + * @return void + */ + protected function updateSession($id) + { + $this->session->put($this->getName(), $id); + + $this->session->migrate(true); + } + + /** + * Log the user out of the application. + * + * @return void + */ + public function logout() + { + $this->clearUserDataFromStorage(); + + // Now we will clear the users out of memory so they are no longer available + // as the user is no longer considered as being signed into this + // application and should not be available here. + $this->user = null; + + $this->loggedOut = true; + } + + /** + * Remove the user data from the session and cookies. + * + * @return void + */ + protected function clearUserDataFromStorage() + { + $this->session->remove($this->getName()); + } + + /** + * Get the last user we attempted to authenticate. + * + * @return \Illuminate\Contracts\Auth\Authenticatable + */ + public function getLastAttempted() + { + return $this->lastAttempted; + } + + /** + * Get a unique identifier for the auth session value. + * + * @return string + */ + public function getName() + { + return 'login_'.$this->name.'_'.sha1(static::class); + } + + /** + * Determine if the user was authenticated via "remember me" cookie. + * + * @return bool + */ + public function viaRemember() + { + return false; + } + + /** + * Return the currently cached user. + * + * @return \Illuminate\Contracts\Auth\Authenticatable|null + */ + public function getUser() + { + return $this->user; + } + + /** + * Set the current user. + * + * @param \Illuminate\Contracts\Auth\Authenticatable $user + * @return $this + */ + public function setUser(AuthenticatableContract $user) + { + $this->user = $user; + + $this->loggedOut = false; + + return $this; + } + +} diff --git a/app/Auth/Access/Guards/LdapSessionGuard.php b/app/Auth/Access/Guards/LdapSessionGuard.php new file mode 100644 index 000000000..3c98140f6 --- /dev/null +++ b/app/Auth/Access/Guards/LdapSessionGuard.php @@ -0,0 +1,114 @@ +ldapService = $ldapService; + parent::__construct($name, $provider, $session, $registrationService); + } + + /** + * Validate a user's credentials. + * + * @param array $credentials + * @return bool + * @throws LdapException + */ + public function validate(array $credentials = []) + { + $userDetails = $this->ldapService->getUserDetails($credentials['username']); + $this->lastAttempted = $this->provider->retrieveByCredentials([ + 'external_auth_id' => $userDetails['uid'] + ]); + + return $this->ldapService->validateUserCredentials($userDetails, $credentials['username'], $credentials['password']); + } + + /** + * Attempt to authenticate a user using the given credentials. + * + * @param array $credentials + * @param bool $remember + * @return bool + * @throws LoginAttemptEmailNeededException + * @throws LoginAttemptException + * @throws LdapException + * @throws UserRegistrationException + */ + 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->createNewFromLdapAndCreds($userDetails, $credentials); + } + + // Sync LDAP groups if required + if ($this->ldapService->shouldSyncGroups()) { + $this->ldapService->syncGroups($user, $username); + } + + $this->login($user, $remember); + return true; + } + + /** + * Create a new user from the given ldap credentials and login credentials + * @throws LoginAttemptEmailNeededException + * @throws LoginAttemptException + * @throws UserRegistrationException + */ + protected function createNewFromLdapAndCreds(array $ldapUserDetails, array $credentials): User + { + $email = trim($ldapUserDetails['email'] ?: ($credentials['email'] ?? '')); + + if (empty($email)) { + throw new LoginAttemptEmailNeededException(); + } + + $details = [ + 'name' => $ldapUserDetails['name'], + 'email' => $ldapUserDetails['email'] ?: $credentials['email'], + 'external_auth_id' => $ldapUserDetails['uid'], + 'password' => Str::random(32), + ]; + + return $this->registrationService->registerUser($details, null, false); + } + +} diff --git a/app/Auth/Access/Guards/Saml2SessionGuard.php b/app/Auth/Access/Guards/Saml2SessionGuard.php new file mode 100644 index 000000000..4023913ed --- /dev/null +++ b/app/Auth/Access/Guards/Saml2SessionGuard.php @@ -0,0 +1,40 @@ +ldap = $ldap; $this->config = config('services.ldap'); - $this->userRepo = $userRepo; $this->enabled = config('auth.method') === 'ldap'; } @@ -106,20 +102,15 @@ class LdapService extends ExternalAuthService * Check if the given credentials are valid for the given user. * @throws LdapException */ - public function validateUserCredentials(Authenticatable $user, string $username, string $password): bool + public function validateUserCredentials(array $ldapUserDetails, string $username, string $password): bool { - $ldapUser = $this->getUserDetails($username); - if ($ldapUser === null) { - return false; - } - - if ($ldapUser['uid'] !== $user->external_auth_id) { + if ($ldapUserDetails === null) { return false; } $ldapConnection = $this->getConnection(); try { - $ldapBind = $this->ldap->bind($ldapConnection, $ldapUser['dn'], $password); + $ldapBind = $this->ldap->bind($ldapConnection, $ldapUserDetails['dn'], $password); } catch (ErrorException $e) { $ldapBind = false; } diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 7e8c7d5f5..8cf76013a 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -1,6 +1,7 @@ emailConfirmationService = $emailConfirmationService; } - /** * Check whether or not registrations are allowed in the app settings. * @throws UserRegistrationException */ - public function checkRegistrationAllowed() + public function ensureRegistrationAllowed() { - if (!setting('registration-enabled') || config('auth.method') === 'ldap') { + 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']; + 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 = ''; @@ -67,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 c1038e730..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; - $this->enabled = config('saml2.enabled') === true; } /** @@ -80,6 +78,7 @@ class Saml2Service extends ExternalAuthService * @throws SamlException * @throws ValidationError * @throws JsonDebugException + * @throws UserRegistrationException */ public function processAcsResponse(?string $requestId): ?User { @@ -204,7 +203,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 +247,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); @@ -310,43 +309,26 @@ 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->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 { - $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) { - $user = $this->registerUser($userDetails); + if (is_null($user)) { + $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; @@ -357,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/Config/auth.php b/app/Config/auth.php index b3e22c7e1..51b152ff1 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -11,14 +11,14 @@ return [ // Method of authentication to use - // Options: standard, ldap + // Options: standard, ldap, saml2 'method' => env('AUTH_METHOD', 'standard'), // Authentication Defaults // This option controls the default authentication "guard" and password // reset options for your application. 'defaults' => [ - 'guard' => 'web', + 'guard' => env('AUTH_METHOD', 'standard'), 'passwords' => 'users', ], @@ -26,13 +26,20 @@ return [ // All authentication drivers have a user provider. This defines how the // users are actually retrieved out of your database or other storage // mechanisms used by this application to persist your user's data. - // Supported: "session", "token" + // Supported drivers: "session", "api-token", "ldap-session" 'guards' => [ - 'web' => [ + 'standard' => [ 'driver' => 'session', 'provider' => 'users', ], - + 'ldap' => [ + 'driver' => 'ldap-session', + 'provider' => 'external', + ], + 'saml2' => [ + 'driver' => 'saml2-session', + 'provider' => 'external', + ], 'api' => [ 'driver' => 'api-token', ], @@ -42,17 +49,15 @@ return [ // All authentication drivers have a user provider. This defines how the // users are actually retrieved out of your database or other storage // mechanisms used by this application to persist your user's data. - // Supported: database, eloquent, ldap 'providers' => [ 'users' => [ - 'driver' => env('AUTH_METHOD', 'standard') === 'standard' ? 'eloquent' : env('AUTH_METHOD'), + 'driver' => 'eloquent', + 'model' => \BookStack\Auth\User::class, + ], + 'external' => [ + 'driver' => 'external-users', 'model' => \BookStack\Auth\User::class, ], - - // 'users' => [ - // 'driver' => 'database', - // 'table' => 'users', - // ], ], // Resetting Passwords 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/Exceptions/AuthException.php b/app/Exceptions/AuthException.php deleted file mode 100644 index 2ab7d4616..000000000 --- a/app/Exceptions/AuthException.php +++ /dev/null @@ -1,6 +0,0 @@ -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/Exceptions/LoginAttemptEmailNeededException.php b/app/Exceptions/LoginAttemptEmailNeededException.php new file mode 100644 index 000000000..7ffb0c065 --- /dev/null +++ b/app/Exceptions/LoginAttemptEmailNeededException.php @@ -0,0 +1,6 @@ +middleware('guest'); + $this->middleware('guard:standard'); parent::__construct(); } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index a74e53617..ea584a3b6 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -2,12 +2,11 @@ namespace BookStack\Http\Controllers\Auth; -use BookStack\Auth\Access\LdapService; use BookStack\Auth\Access\SocialAuthService; -use BookStack\Auth\UserRepo; -use BookStack\Exceptions\AuthException; +use BookStack\Exceptions\LoginAttemptEmailNeededException; +use BookStack\Exceptions\LoginAttemptException; +use BookStack\Exceptions\UserRegistrationException; use BookStack\Http\Controllers\Controller; -use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Foundation\Auth\AuthenticatesUsers; use Illuminate\Http\Request; @@ -27,32 +26,23 @@ class LoginController extends Controller use AuthenticatesUsers; /** - * Where to redirect users after login. - * - * @var string + * Redirection paths */ protected $redirectTo = '/'; - protected $redirectPath = '/'; protected $redirectAfterLogout = '/login'; protected $socialAuthService; - protected $ldapService; - protected $userRepo; /** * Create a new controller instance. - * - * @param \BookStack\Auth\\BookStack\Auth\Access\SocialAuthService $socialAuthService - * @param LdapService $ldapService - * @param \BookStack\Auth\UserRepo $userRepo */ - public function __construct(SocialAuthService $socialAuthService, LdapService $ldapService, UserRepo $userRepo) + public function __construct(SocialAuthService $socialAuthService) { - $this->middleware('guest', ['only' => ['getLogin', 'postLogin']]); + $this->middleware('guest', ['only' => ['getLogin', 'login']]); + $this->middleware('guard:standard,ldap', ['only' => ['login', 'logout']]); + $this->socialAuthService = $socialAuthService; - $this->ldapService = $ldapService; - $this->userRepo = $userRepo; $this->redirectPath = url('/'); $this->redirectAfterLogout = url('/login'); parent::__construct(); @@ -64,47 +54,11 @@ class LoginController extends Controller } /** - * Overrides the action when a user is authenticated. - * If the user authenticated but does not exist in the user table we create them. - * @throws AuthException - * @throws \BookStack\Exceptions\LdapException + * Get the needed authorization credentials from the request. */ - protected function authenticated(Request $request, Authenticatable $user) + protected function credentials(Request $request) { - // Explicitly log them out for now if they do no exist. - if (!$user->exists) { - auth()->logout($user); - } - - if (!$user->exists && $user->email === null && !$request->filled('email')) { - $request->flash(); - session()->flash('request-email', true); - return redirect('/login'); - } - - if (!$user->exists && $user->email === null && $request->filled('email')) { - $user->email = $request->get('email'); - } - - if (!$user->exists) { - // Check for users with same email already - $alreadyUser = $user->newQuery()->where('email', '=', $user->email)->count() > 0; - if ($alreadyUser) { - throw new AuthException(trans('errors.error_user_exists_different_creds', ['email' => $user->email])); - } - - $user->save(); - $this->userRepo->attachDefaultRole($user); - $this->userRepo->downloadAndAssignUserAvatar($user); - auth()->login($user); - } - - // Sync LDAP groups if required - if ($this->ldapService->shouldSyncGroups()) { - $this->ldapService->syncGroups($user, $request->get($this->username())); - } - - return redirect()->intended('/'); + return $request->only('username', 'email', 'password'); } /** @@ -114,7 +68,6 @@ class LoginController extends Controller { $socialDrivers = $this->socialAuthService->getActiveDrivers(); $authMethod = config('auth.method'); - $samlEnabled = config('saml2.enabled') === true; if ($request->has('email')) { session()->flashInput([ @@ -126,22 +79,87 @@ class LoginController extends Controller return view('auth.login', [ 'socialDrivers' => $socialDrivers, 'authMethod' => $authMethod, - 'samlEnabled' => $samlEnabled, ]); } /** - * Log the user out of the application. + * Handle a login request to the application. + * + * @param \Illuminate\Http\Request $request + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\Response|\Illuminate\Http\JsonResponse + * + * @throws \Illuminate\Validation\ValidationException */ - public function logout(Request $request) + public function login(Request $request) { - if (config('saml2.enabled') && session()->get('last_login_type') === 'saml2') { - return redirect('/saml2/logout'); + $this->validateLogin($request); + + // If the class is using the ThrottlesLogins trait, we can automatically throttle + // the login attempts for this application. We'll key this by the username and + // the IP address of the client making these requests into this application. + if (method_exists($this, 'hasTooManyLoginAttempts') && + $this->hasTooManyLoginAttempts($request)) { + $this->fireLockoutEvent($request); + + return $this->sendLockoutResponse($request); } - $this->guard()->logout(); - $request->session()->invalidate(); + try { + if ($this->attemptLogin($request)) { + return $this->sendLoginResponse($request); + } + } catch (LoginAttemptException $exception) { + return $this->sendLoginAttemptExceptionResponse($exception, $request); + } - return $this->loggedOut($request) ?: redirect('/'); + // If the login attempt was unsuccessful we will increment the number of attempts + // to login and redirect the user back to the login form. Of course, when this + // user surpasses their maximum number of attempts they will get locked out. + $this->incrementLoginAttempts($request); + + return $this->sendFailedLoginResponse($request); } + + /** + * Validate the user login request. + * + * @param \Illuminate\Http\Request $request + * @return void + * + * @throws \Illuminate\Validation\ValidationException + */ + protected function validateLogin(Request $request) + { + $rules = ['password' => 'required|string']; + $authMethod = config('auth.method'); + + if ($authMethod === 'standard') { + $rules['email'] = 'required|email'; + } + + if ($authMethod === 'ldap') { + $rules['username'] = 'required|string'; + $rules['email'] = 'email'; + } + + $request->validate($rules); + } + + /** + * Send a response when a login attempt exception occurs. + */ + protected function sendLoginAttemptExceptionResponse(LoginAttemptException $exception, Request $request) + { + if ($exception instanceof LoginAttemptEmailNeededException) { + $request->flash(); + session()->flash('request-email', true); + } + + if ($message = $exception->getMessage()) { + $this->showWarningNotification($message); + } + + return redirect('/login'); + } + } diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index c9c0c3ec5..0bdeef9e6 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -43,7 +43,8 @@ class RegisterController extends Controller */ public function __construct(SocialAuthService $socialAuthService, RegistrationService $registrationService) { - $this->middleware('guest')->only(['getRegister', 'postRegister']); + $this->middleware('guest'); + $this->middleware('guard:standard'); $this->socialAuthService = $socialAuthService; $this->registrationService = $registrationService; @@ -73,12 +74,10 @@ class RegisterController extends Controller */ public function getRegister() { - $this->registrationService->checkRegistrationAllowed(); + $this->registrationService->ensureRegistrationAllowed(); $socialDrivers = $this->socialAuthService->getActiveDrivers(); - $samlEnabled = (config('saml2.enabled') === true) && (config('saml2.auto_register') === true); return view('auth.register', [ 'socialDrivers' => $socialDrivers, - 'samlEnabled' => $samlEnabled, ]); } @@ -88,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/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index 4d98eca59..214647cd5 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -31,6 +31,7 @@ class ResetPasswordController extends Controller public function __construct() { $this->middleware('guest'); + $this->middleware('guard:standard'); parent::__construct(); } diff --git a/app/Http/Controllers/Auth/Saml2Controller.php b/app/Http/Controllers/Auth/Saml2Controller.php index 863894128..7ffcc572b 100644 --- a/app/Http/Controllers/Auth/Saml2Controller.php +++ b/app/Http/Controllers/Auth/Saml2Controller.php @@ -17,15 +17,7 @@ class Saml2Controller extends Controller { parent::__construct(); $this->samlService = $samlService; - - // SAML2 access middleware - $this->middleware(function ($request, $next) { - if (!config('saml2.enabled')) { - $this->showPermissionError(); - } - - return $next($request); - }); + $this->middleware('guard:saml2'); } /** @@ -89,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/Http/Controllers/Auth/UserInviteController.php b/app/Http/Controllers/Auth/UserInviteController.php index c361b0a9b..c61b1c42b 100644 --- a/app/Http/Controllers/Auth/UserInviteController.php +++ b/app/Http/Controllers/Auth/UserInviteController.php @@ -8,11 +8,9 @@ use BookStack\Exceptions\UserTokenExpiredException; use BookStack\Exceptions\UserTokenNotFoundException; use BookStack\Http\Controllers\Controller; use Exception; -use Illuminate\Contracts\View\Factory; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Routing\Redirector; -use Illuminate\View\View; class UserInviteController extends Controller { @@ -21,22 +19,20 @@ class UserInviteController extends Controller /** * Create a new controller instance. - * - * @param UserInviteService $inviteService - * @param UserRepo $userRepo */ public function __construct(UserInviteService $inviteService, UserRepo $userRepo) { + $this->middleware('guest'); + $this->middleware('guard:standard'); + $this->inviteService = $inviteService; $this->userRepo = $userRepo; - $this->middleware('guest'); + parent::__construct(); } /** * Show the page for the user to set the password for their account. - * @param string $token - * @return Factory|View|RedirectResponse * @throws Exception */ public function showSetPassword(string $token) @@ -54,9 +50,6 @@ class UserInviteController extends Controller /** * Sets the password for an invited user and then grants them access. - * @param Request $request - * @param string $token - * @return RedirectResponse|Redirector * @throws Exception */ public function setPassword(Request $request, string $token) @@ -85,7 +78,6 @@ class UserInviteController extends Controller /** * Check and validate the exception thrown when checking an invite token. - * @param Exception $exception * @return RedirectResponse|Redirector * @throws Exception */ diff --git a/app/Http/Controllers/SettingController.php b/app/Http/Controllers/SettingController.php index f0a078300..892b2d9cf 100644 --- a/app/Http/Controllers/SettingController.php +++ b/app/Http/Controllers/SettingController.php @@ -5,8 +5,6 @@ use BookStack\Notifications\TestEmail; use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageService; use Illuminate\Http\Request; -use Illuminate\Http\Response; -use Setting; class SettingController extends Controller { @@ -14,7 +12,6 @@ class SettingController extends Controller /** * SettingController constructor. - * @param $imageRepo */ public function __construct(ImageRepo $imageRepo) { @@ -22,10 +19,8 @@ class SettingController extends Controller parent::__construct(); } - /** * Display a listing of the settings. - * @return Response */ public function index() { @@ -43,8 +38,6 @@ class SettingController extends Controller /** * Update the specified settings in storage. - * @param Request $request - * @return Response */ public function update(Request $request) { @@ -78,12 +71,12 @@ class SettingController extends Controller } $this->showSuccessNotification(trans('settings.settings_save_success')); - return redirect('/settings'); + $redirectLocation = '/settings#' . $request->get('section', ''); + return redirect(rtrim($redirectLocation, '#')); } /** * Show the page for application maintenance. - * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View */ public function showMaintenance() { @@ -98,9 +91,6 @@ class SettingController extends Controller /** * Action to clean-up images in the system. - * @param Request $request - * @param ImageService $imageService - * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector */ public function cleanupImages(Request $request, ImageService $imageService) { @@ -127,11 +117,8 @@ class SettingController extends Controller /** * Action to send a test e-mail to the current user. - * @param Request $request - * @param User $user - * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector */ - public function sendTestEmail(Request $request) + public function sendTestEmail() { $this->checkPermission('settings-manage'); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index c2016281a..4517deb90 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -48,7 +48,8 @@ class Kernel extends HttpKernel 'auth' => \BookStack\Http\Middleware\Authenticate::class, 'can' => \Illuminate\Auth\Middleware\Authorize::class, 'guest' => \BookStack\Http\Middleware\RedirectIfAuthenticated::class, - 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, - 'perm' => \BookStack\Http\Middleware\PermissionMiddleware::class + 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, + 'perm' => \BookStack\Http\Middleware\PermissionMiddleware::class, + 'guard' => \BookStack\Http\Middleware\CheckGuard::class, ]; } diff --git a/app/Http/Middleware/CheckGuard.php b/app/Http/Middleware/CheckGuard.php new file mode 100644 index 000000000..cc73ea68d --- /dev/null +++ b/app/Http/Middleware/CheckGuard.php @@ -0,0 +1,27 @@ +flash('error', trans('errors.permission')); + return redirect('/'); + } + + return $next($request); + } +} diff --git a/app/Http/Middleware/PermissionMiddleware.php b/app/Http/Middleware/PermissionMiddleware.php index 28ffff643..d0bb4f79e 100644 --- a/app/Http/Middleware/PermissionMiddleware.php +++ b/app/Http/Middleware/PermissionMiddleware.php @@ -19,7 +19,7 @@ class PermissionMiddleware { if (!$request->user() || !$request->user()->can($permission)) { - Session::flash('error', trans('errors.permission')); + session()->flash('error', trans('errors.permission')); return redirect()->back(); } diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index ab7dd5195..fe52df168 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -4,7 +4,12 @@ namespace BookStack\Providers; 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\Access\RegistrationService; +use BookStack\Auth\UserRepo; use Illuminate\Support\ServiceProvider; class AuthServiceProvider extends ServiceProvider @@ -19,6 +24,27 @@ class AuthServiceProvider extends ServiceProvider Auth::extend('api-token', function ($app, $name, array $config) { return new ApiTokenGuard($app['request']); }); + + Auth::extend('ldap-session', function ($app, $name, array $config) { + $provider = Auth::createUserProvider($config['provider']); + return new LdapSessionGuard( + $name, + $provider, + $this->app['session.store'], + $app[LdapService::class], + $app[RegistrationService::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[RegistrationService::class] + ); + }); } /** @@ -28,8 +54,8 @@ class AuthServiceProvider extends ServiceProvider */ public function register() { - Auth::provider('ldap', function ($app, array $config) { - return new LdapUserProvider($config['model'], $app[LdapService::class]); + Auth::provider('external-users', function ($app, array $config) { + return new ExternalBaseUserProvider($config['model']); }); } } 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 b36fdda7a..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.', @@ -131,7 +131,7 @@ return [ 'users_send_invite_text' => 'You can choose to send this user an invitation email which allows them to set their own password otherwise you can set their password yourself.', 'users_send_invite_option' => 'Send user invite email', 'users_external_auth_id' => 'External Authentication ID', - 'users_external_auth_id_desc' => 'This is the ID used to match this user when communicating with your LDAP system.', + 'users_external_auth_id_desc' => 'This is the ID used to match this user when communicating with your external authentication system.', 'users_password_warning' => 'Only fill the below if you would like to change your password.', 'users_system_public' => 'This user represents any guest users that visit your instance. It cannot be used to log in but is assigned automatically.', 'users_delete' => 'Delete User', diff --git a/resources/views/auth/forms/login/ldap.blade.php b/resources/views/auth/forms/login/ldap.blade.php index 2699fda00..92eba80e8 100644 --- a/resources/views/auth/forms/login/ldap.blade.php +++ b/resources/views/auth/forms/login/ldap.blade.php @@ -1,19 +1,28 @@ -
- - @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 new file mode 100644 index 000000000..aa1913e31 --- /dev/null +++ b/resources/views/auth/forms/login/saml2.blade.php @@ -0,0 +1,11 @@ +
+ {!! csrf_field() !!} + +
+ +
+ +
\ 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']) + +
+
+ +
+
+ @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) -
-
- - @icon('saml2') - {{ trans('auth.log_in_with', ['socialDriver' => config('saml2.name')]) }} - -
- @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) -
-
- - @icon('saml2') - {{ trans('auth.log_in_with', ['socialDriver' => config('saml2.name')]) }} - -
- @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/index.blade.php b/resources/views/settings/index.blade.php index 94605da6f..b3a11955d 100644 --- a/resources/views/settings/index.blade.php +++ b/resources/views/settings/index.blade.php @@ -15,9 +15,10 @@
    -

    {{ trans('settings.app_features_security') }}

    +

    {{ trans('settings.app_features_security') }}

    {!! csrf_field() !!} +
    @@ -79,9 +80,10 @@
    -

    {{ trans('settings.app_customization') }}

    +

    {{ trans('settings.app_customization') }}

    {!! csrf_field() !!} +
    @@ -202,9 +204,10 @@
    -

    {{ trans('settings.reg_settings') }}

    +

    {{ trans('settings.reg_settings') }}

    {!! csrf_field() !!} +
    @@ -219,8 +222,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/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..90261e1ac 100644 --- a/routes/web.php +++ b/routes/web.php @@ -210,7 +210,9 @@ Route::group(['middleware' => 'auth'], function () { // Social auth routes Route::get('/login/service/{socialDriver}', 'Auth\SocialController@getSocialLogin'); Route::get('/login/service/{socialDriver}/callback', 'Auth\SocialController@socialCallback'); -Route::get('/login/service/{socialDriver}/detach', 'Auth\SocialController@detachSocialAccount'); +Route::group(['middleware' => 'auth'], function () { + Route::get('/login/service/{socialDriver}/detach', 'Auth\SocialController@detachSocialAccount'); +}); Route::get('/register/service/{socialDriver}', 'Auth\SocialController@socialRegister'); // Login/Logout routes @@ -225,7 +227,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'); diff --git a/tests/Api/ApiAuthTest.php b/tests/Api/ApiAuthTest.php index 2ab81814b..1f283753a 100644 --- a/tests/Api/ApiAuthTest.php +++ b/tests/Api/ApiAuthTest.php @@ -20,7 +20,7 @@ class ApiAuthTest extends TestCase $resp = $this->get($this->endpoint); $resp->assertStatus(401); - $this->actingAs($viewer, 'web'); + $this->actingAs($viewer, 'standard'); $resp = $this->get($this->endpoint); $resp->assertStatus(200); @@ -72,11 +72,11 @@ class ApiAuthTest extends TestCase public function test_api_access_permission_required_to_access_api_with_session_auth() { $editor = $this->getEditor(); - $this->actingAs($editor, 'web'); + $this->actingAs($editor, 'standard'); $resp = $this->get($this->endpoint); $resp->assertStatus(200); - auth('web')->logout(); + auth('standard')->logout(); $accessApiPermission = RolePermission::getByName('access-api'); $editorRole = $this->getEditor()->roles()->first(); @@ -84,7 +84,7 @@ class ApiAuthTest extends TestCase $editor = User::query()->where('id', '=', $editor->id)->first(); - $this->actingAs($editor, 'web'); + $this->actingAs($editor, 'standard'); $resp = $this->get($this->endpoint); $resp->assertStatus(403); $resp->assertJson($this->errorResponse("The owner of the used API token does not have permission to make API calls", 403)); diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 5d7bbfb1e..cb1194e22 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -1,4 +1,5 @@ set([ 'auth.method' => 'ldap', + 'auth.defaults.guard' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'services.ldap.email_attribute' => 'mail', 'services.ldap.display_name_attribute' => 'cn', 'services.ldap.id_attribute' => 'uid', 'services.ldap.user_to_groups' => false, - 'auth.providers.users.driver' => 'ldap', + 'services.ldap.version' => '3', + 'services.ldap.user_filter' => '(&(uid=${user}))', + 'services.ldap.follow_referrals' => false, + 'services.ldap.tls_insecure' => false, ]); $this->mockLdap = \Mockery::mock(Ldap::class); $this->app[Ldap::class] = $this->mockLdap; @@ -60,16 +65,16 @@ class LdapTest extends BrowserKitTest { $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $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(6)->andReturn(true); - $this->mockEscapes(4); + $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.'); @@ -81,21 +86,53 @@ 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); $this->mockLdap->shouldReceive('setVersion')->once(); $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn'); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + $this->mockLdap->shouldReceive('setOption')->times(1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'cn' => [$this->mockUser->name], 'dn' => $ldapDn, 'mail' => [$this->mockUser->email] ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/') @@ -109,8 +146,8 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn'); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2) + $this->mockLdap->shouldReceive('setOption')->times(1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'cn' => [$this->mockUser->name], @@ -120,8 +157,8 @@ class LdapTest extends BrowserKitTest ]]); - $this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/') @@ -133,16 +170,16 @@ class LdapTest extends BrowserKitTest { $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) + $this->mockLdap->shouldReceive('setOption')->times(1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1) ->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(3)->andReturn(true, true, false); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true, false); + $this->mockEscapes(1); $this->mockUserLogin() ->seePageIs('/login')->see('These credentials do not match our records.') @@ -201,10 +238,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => false, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(5); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(5) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(4); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], @@ -217,8 +254,8 @@ class LdapTest extends BrowserKitTest 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(5); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + $this->mockEscapes(4); $this->mockExplodes(6); $this->mockUserLogin()->seePageIs('/'); @@ -250,10 +287,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(3); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], @@ -265,8 +302,8 @@ class LdapTest extends BrowserKitTest 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(3); $this->mockExplodes(2); $this->mockUserLogin()->seePageIs('/'); @@ -299,10 +336,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(3); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], @@ -314,8 +351,8 @@ class LdapTest extends BrowserKitTest 0 => "cn=ex-auth-a,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); - $this->mockEscapes(4); + $this->mockLdap->shouldReceive('bind')->times(4)->andReturn(true); + $this->mockEscapes(3); $this->mockExplodes(2); $this->mockUserLogin()->seePageIs('/'); @@ -344,10 +381,10 @@ class LdapTest extends BrowserKitTest 'services.ldap.group_attribute' => 'memberOf', 'services.ldap.remove_from_groups' => true, ]); - $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); - $this->mockLdap->shouldReceive('setVersion')->times(2); - $this->mockLdap->shouldReceive('setOption')->times(5); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(5) + $this->mockLdap->shouldReceive('connect')->times(1)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(1); + $this->mockLdap->shouldReceive('setOption')->times(4); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array')) ->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], @@ -360,8 +397,8 @@ class LdapTest extends BrowserKitTest 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", ] ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(5); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + $this->mockEscapes(4); $this->mockExplodes(6); $this->mockUserLogin()->seePageIs('/'); @@ -385,8 +422,8 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $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], @@ -394,8 +431,8 @@ class LdapTest extends BrowserKitTest 'dn' => ['dc=test' . config('services.ldap.base_dn')], 'displayname' => 'displayNameAttribute' ]]); - $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); - $this->mockEscapes(4); + $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.'); @@ -415,16 +452,16 @@ class LdapTest extends BrowserKitTest $this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId); $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(4); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + $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(6)->andReturn(true); - $this->mockEscapes(4); + $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.'); @@ -444,14 +481,14 @@ class LdapTest extends BrowserKitTest // Standard mocks $this->mockLdap->shouldReceive('setVersion')->once(); - $this->mockLdap->shouldReceive('setOption')->times(2); - $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)->andReturn(['count' => 1, 0 => [ + $this->mockLdap->shouldReceive('setOption')->times(1); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->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(3)->andReturn(true); - $this->mockEscapes(2); + $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true); + $this->mockEscapes(1); $this->mockLdap->shouldReceive('connect')->once() ->with($expectedHost, $expectedPort)->andReturn($this->resourceId); @@ -473,4 +510,37 @@ class LdapTest extends BrowserKitTest { $this->checkLdapReceivesCorrectDetails('ldap.bookstack.com', 'ldap.bookstack.com', 389); } + + public function test_forgot_password_routes_inaccessible() + { + $resp = $this->get('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->get('/password/reset/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/reset'); + $this->assertPermissionError($resp); + } + + public function test_user_invite_routes_inaccessible() + { + $resp = $this->get('/register/invite/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register/invite/abc123'); + $this->assertPermissionError($resp); + } + + public function test_user_register_routes_inaccessible() + { + $resp = $this->get('/register'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register'); + $this->assertPermissionError($resp); + } } diff --git a/tests/Auth/Saml2Test.php b/tests/Auth/Saml2Test.php index 45b6efa07..9a3d6d8ec 100644 --- a/tests/Auth/Saml2Test.php +++ b/tests/Auth/Saml2Test.php @@ -11,9 +11,9 @@ class Saml2Test extends TestCase parent::setUp(); // Set default config for SAML2 config()->set([ + 'auth.method' => 'saml2', + 'auth.defaults.guard' => 'saml2', 'saml2.name' => 'SingleSignOn-Testing', - 'saml2.enabled' => true, - 'saml2.auto_register' => true, 'saml2.email_attribute' => 'email', 'saml2.display_name_attributes' => ['first_name', 'last_name'], 'saml2.external_id_attribute' => 'uid', @@ -53,27 +53,12 @@ class Saml2Test extends TestCase { $req = $this->get('/login'); $req->assertSeeText('SingleSignOn-Testing'); - $req->assertElementExists('a[href$="/saml2/login"]'); - } - - public function test_login_option_shows_on_register_page_only_when_auto_register_enabled() - { - $this->setSettings(['app-public' => 'true', 'registration-enabled' => 'true']); - - $req = $this->get('/register'); - $req->assertSeeText('SingleSignOn-Testing'); - $req->assertElementExists('a[href$="/saml2/login"]'); - - config()->set(['saml2.auto_register' => false]); - - $req = $this->get('/register'); - $req->assertDontSeeText('SingleSignOn-Testing'); - $req->assertElementNotExists('a[href$="/saml2/login"]'); + $req->assertElementExists('form[action$="/saml2/login"][method=POST] button'); } public function test_login() { - $req = $this->get('/saml2/login'); + $req = $this->post('/saml2/login'); $redirect = $req->headers->get('location'); $this->assertStringStartsWith('http://saml.local/saml2/idp/SSOService.php', $redirect, 'Login redirects to SSO location'); @@ -88,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' ]); @@ -138,20 +123,15 @@ class Saml2Test extends TestCase }); } - public function test_logout_redirects_to_saml_logout_when_active_saml_session() + public function test_logout_link_directs_to_saml_path() { config()->set([ 'saml2.onelogin.strict' => false, ]); - $this->withPost(['SAMLResponse' => $this->acsPostData], function () { - $acsPost = $this->post('/saml2/acs'); - $lastLoginType = session()->get('last_login_type'); - $this->assertEquals('saml2', $lastLoginType); - - $req = $this->get('/logout'); - $req->assertRedirect('/saml2/logout'); - }); + $resp = $this->actingAs($this->getEditor())->get('/'); + $resp->assertElementExists('a[href$="/saml2/logout"]'); + $resp->assertElementContains('a[href$="/saml2/logout"]', 'Logout'); } public function test_logout_sls_flow() @@ -229,32 +209,86 @@ 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); }); } public function test_saml_routes_are_only_active_if_saml_enabled() { - config()->set(['saml2.enabled' => false]); - $getRoutes = ['/login', '/logout', '/metadata', '/sls']; + config()->set(['auth.method' => 'standard']); + $getRoutes = ['/logout', '/metadata', '/sls']; foreach ($getRoutes as $route) { $req = $this->get('/saml2' . $route); - $req->assertRedirect('/'); - $error = session()->get('error'); - $this->assertStringStartsWith('You do not have permission to access', $error); - session()->flush(); + $this->assertPermissionError($req); } - $postRoutes = ['/acs']; + $postRoutes = ['/login', '/acs']; foreach ($postRoutes as $route) { $req = $this->post('/saml2' . $route); - $req->assertRedirect('/'); - $error = session()->get('error'); - $this->assertStringStartsWith('You do not have permission to access', $error); - session()->flush(); + $this->assertPermissionError($req); } } + public function test_forgot_password_routes_inaccessible() + { + $resp = $this->get('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/email'); + $this->assertPermissionError($resp); + + $resp = $this->get('/password/reset/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/password/reset'); + $this->assertPermissionError($resp); + } + + public function test_standard_login_routes_inaccessible() + { + $resp = $this->post('/login'); + $this->assertPermissionError($resp); + + $resp = $this->get('/logout'); + $this->assertPermissionError($resp); + } + + public function test_user_invite_routes_inaccessible() + { + $resp = $this->get('/register/invite/abc123'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register/invite/abc123'); + $this->assertPermissionError($resp); + } + + public function test_user_register_routes_inaccessible() + { + $resp = $this->get('/register'); + $this->assertPermissionError($resp); + + $resp = $this->post('/register'); + $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); diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php index 3433f3b83..f7b7d5edf 100644 --- a/tests/SharedTestHelpers.php +++ b/tests/SharedTestHelpers.php @@ -262,4 +262,19 @@ trait SharedTestHelpers self::assertThat($passed, self::isTrue(), "Failed asserting that given map:\n\n{$toCheckStr}\n\nincludes:\n\n{$toIncludeStr}"); } + /** + * Assert a permission error has occurred. + */ + protected function assertPermissionError($response) + { + if ($response instanceof BrowserKitTest) { + $response = \Illuminate\Foundation\Testing\TestResponse::fromBaseResponse($response->response); + } + + $response->assertRedirect('/'); + $this->assertSessionHas('error'); + $error = session()->pull('error'); + $this->assertStringStartsWith('You do not have permission to access', $error); + } + } \ No newline at end of file diff --git a/tests/TestCase.php b/tests/TestCase.php index f20b20fd8..1f1d5ece7 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -16,19 +16,6 @@ abstract class TestCase extends BaseTestCase */ protected $baseUrl = 'http://localhost'; - /** - * Assert a permission error has occurred. - * @param TestResponse $response - * @return TestCase - */ - protected function assertPermissionError(TestResponse $response) - { - $response->assertRedirect('/'); - $this->assertSessionHas('error'); - session()->remove('error'); - return $this; - } - /** * Assert the session contains a specific entry. * @param string $key