Add error messages, fix LDAP error

app/Auth/Access/ExternalAuthService.php

@@ -2,6 +2,7 @@
 
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
+use Illuminate\Database\Eloquent\Builder;
 
 class ExternalAuthService
 {
@@ -57,19 +58,19 @@ class ExternalAuthService
     /**
      * Sync the groups to the user roles for the current user
      * @param \BookStack\Auth\User $user
-     * @param array $samlAttributes
+     * @param array $userGroups
      */
     public function syncWithGroups(User $user, array $userGroups)
     {
         // Get the ids for the roles from the names
-        $samlGroupsAsRoles = $this->matchGroupsToSystemsRoles($userSamlGroups);
+        $groupsAsRoles = $this->matchGroupsToSystemsRoles($userGroups);
 
         // Sync groups
         if ($this->config['remove_from_groups']) {
-            $user->roles()->sync($samlGroupsAsRoles);
+            $user->roles()->sync($groupsAsRoles);
             $this->userRepo->attachDefaultRole($user);
         } else {
-            $user->roles()->syncWithoutDetaching($samlGroupsAsRoles);
+            $user->roles()->syncWithoutDetaching($groupsAsRoles);
         }
     }
 }

app/Auth/Access/LdapService.php

@@ -5,7 +5,6 @@ use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\LdapException;
 use Illuminate\Contracts\Auth\Authenticatable;
-use Illuminate\Database\Eloquent\Builder;
 
 /**
  * Class LdapService

app/Auth/Access/Saml2Service.php

@@ -5,8 +5,6 @@ use BookStack\Auth\User;
 use BookStack\Auth\UserRepo;
 use BookStack\Exceptions\SamlException;
 use Illuminate\Contracts\Auth\Authenticatable;
-use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Support\Facades\Log;
 
 
 /**
@@ -117,6 +115,27 @@ class Saml2Service extends Access\ExternalAuthService
         return $userGroups;
     }
 
+    /**
+     *  For an array of strings, return a default for an empty array,
+     *  a string for an array with one element and the full array for
+     *  more than one element.
+     *
+     *  @param array $data
+     *  @param $defaultValue
+     *  @return string
+     */
+    protected function simplifyValue(array $data, $defaultValue) {
+        switch (count($data)) {
+            case 0:
+                $data = $defaultValue;
+                break;
+            case 1:
+                $data = $data[0];
+                break;
+        }
+        return $data;
+    }
+
     /**
      * Get a property from an SAML response.
      * Handles properties potentially being an array.
@@ -128,16 +147,9 @@ class Saml2Service extends Access\ExternalAuthService
     protected function getSamlResponseAttribute(array $samlAttributes, string $propertyKey, $defaultValue)
     {
         if (isset($samlAttributes[$propertyKey])) {
-            $data = $samlAttributes[$propertyKey];
-            if (is_array($data)) {
-              if (count($data) == 0) {
-                $data = $defaultValue;
-              } else if (count($data) == 1) {
-                $data = $data[0];
-              }
-            }
+            $data = $this->simplifyValue($samlAttributes[$propertyKey], $defaultValue);
         } else {
-          $data = $defaultValue;
+            $data = $defaultValue;
         }
 
         return $data;
@@ -190,6 +202,7 @@ class Saml2Service extends Access\ExternalAuthService
      *  they exist, optionally registering them automatically.
      *  @param string $samlID
      *  @param array $samlAttributes
+     *  @throws SamlException
      */
     public function processLoginCallback($samlID, $samlAttributes)
     {
@@ -197,12 +210,14 @@ class Saml2Service extends Access\ExternalAuthService
         $isLoggedIn = auth()->check();
 
         if ($isLoggedIn) {
-            logger()->error("Already logged in");
+            throw new SamlException(trans('errors.saml_already_logged_in'), '/login');
         } else {
             $user = $this->getOrRegisterUser($userDetails);
             if ($user === null) {
-                logger()->error("User does not exist");
+                throw new SamlException(trans('errors.saml_user_not_registered', ['name' => $userDetails['uid']]), '/login');
             } else {
+                $groups = $this->getUserGroups($samlAttributes);
+                $this->syncWithGroups($user, $groups);
                 auth()->login($user);
             }
         }

app/Config/services.php

@@ -150,12 +150,14 @@ return [
     ],
 
     'saml' => [
+        'name' => env('SAML_NAME', 'SSO'),
         'enabled' => env('SAML2_ENABLED', false),
         'auto_register' => env('SAML_AUTO_REGISTER', false),
         'email_attribute' => env('SAML_EMAIL_ATTRIBUTE', 'email'),
         'display_name_attribute' => explode('|', env('SAML_DISPLAY_NAME_ATTRIBUTE', 'username')),
         'user_name_attribute' => env('SAML_USER_NAME_ATTRIBUTE', null),
         'group_attribute' => env('SAML_GROUP_ATTRIBUTE', 'group'),
+        'remove_from_groups' => env('SAML_REMOVE_FROM_GROUPS',false),
         'user_to_groups' => env('SAML_USER_TO_GROUPS', false),
         'id_is_user_name' => env('SAML_ID_IS_USER_NAME', true),
     ]

app/Exceptions/SamlException.php

@@ -1,6 +1,6 @@
 <?php namespace BookStack\Exceptions;
 
-class SamlException extends PrettyException
+class SamlException extends NotifyException
 {
 
 }