fix LDAP/AD sync: function calls for password creation (#6581)

* - change generatePassword to be more secure (allow duplicate chars)
- move generatePassword from trait to helper
- fix summary output for sync command

* - Don't treat ldap_active_flag as boolean - fixes sync not working at all when ldap field is set
- Sync non activated users (But set activated=0)

* - Read user first before checking against user settings

* Fix failed logins to not throw exceptions

app/Console/Commands/LdapSync.php

@@ -142,14 +142,15 @@ class LdapSync extends Command
      * @return string
      */
     private function getSummary(): string
-    {
+    {        
-        if ($this->option('summary') && null === $this->dryrun) {
+        if ($this->option('summary') && !$this->dryrun) {
             $this->summary->each(function ($item) {
-                $this->info('USER: '.$item['note']);
-
                 if ('ERROR' === $item['status']) {
                     $this->error('ERROR: '.$item['note']);
                 }
+                else {
+                    $this->info('USER: '.$item['note']);
+                }
             });
         } elseif ($this->option('json_summary')) {
             $json_summary = [
@@ -175,6 +176,12 @@ class LdapSync extends Command
     private function updateCreateUser(AdldapUser $snipeUser): void
     {
         $user = $this->ldap->processUser($snipeUser, $this->defaultLocation, $this->mappedLocations);
+        if(!$user) {
+            $summary['note']   = sprintf("'%s' was not imported. REASON: User inactive or not found", $snipeUser->ou);
+            $summary['status'] = 'ERROR';
+            $this->summary->push($summary);
+            return;
+        }
         $summary = [
             'firstname'       => $user->first_name,
             'lastname'        => $user->last_name,
@@ -186,19 +193,19 @@ class LdapSync extends Command
         // Only update the database if is not a dry run
         if (!$this->dryrun) {
             if ($user->save()) {
-                $summary['note']   = ($user->wasRecentlyCreated ? 'CREATED' : 'UPDATED');
+                $summary['note']   = sprintf("'%s' %s", $user->username, ($user->wasRecentlyCreated ? 'CREATED' : 'UPDATED'));
                 $summary['status'] = 'SUCCESS';
             } else {
                 $errors = '';
                 foreach ($user->getErrors()->getMessages() as  $error) {
                     $errors .= $error[0];
                 }
-                $summary['note']   = $userMsg.' was not imported. REASON: '.$errors;
+                $summary['note']   = sprintf("'%s' was not imported. REASON: %s", $user->username, $errors);
                 $summary['status'] = 'ERROR';
             }
         }
 
-        $summary['note'] = ($user->getOriginal('username') ? 'UPDATED' : 'CREATED');
+        //$summary['note'] = ($user->getOriginal('username') ? 'UPDATED' : 'CREATED');
         $this->summary->push($summary);
     }
 

app/Helpers/Helper.php

@@ -670,7 +670,37 @@ class Helper
         return false;
     }
 
+    /**
+     * Generate a random encrypted password.
+     *
+     * @author Wes Hulette <jwhulette@gmail.com>
+     *
+     * @since 5.0.0
+     *
+     * @return string
+     */
+    public static function generateEncyrptedPassword(): string
+    {
+        return bcrypt(Helper::generateUnencryptedPassword());
+    }
 
+    /**
+     * Get a random unencrypted password.
+     *
+     * @author Steffen Buehl <sb@sbuehl.com>
+     *
+     * @since 5.0.0
+     *
+     * @return string
+     */
+    public static function generateUnencryptedPassword(): string
+    {
+        $chars = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
 
-
+        $password = '';
+        for ( $i = 0; $i < 20; $i++ ) {
+            $password .= substr( $chars, random_int( 0, strlen( $chars ) - 1 ), 1 );
+        }
+        return $password;
+    }
 }

app/Services/LdapAd.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace App\Services;
 
 use App\Models\User;
+use App\Helpers\Helper;
 use Exception;
 use Adldap\Adldap;
 use Adldap\Query\Paginator;
@@ -93,16 +94,22 @@ class LdapAd extends LdapAdConfiguration
         }
 
         // Should we sync the logged in user
-        if ($this->isLdapSync($record)) {
+        try {
-            try {
+            Log::debug('Attempting to find user in LDAP directory');
-                Log::debug('Attempting to find user in LDAP directory');
+            $record = $this->ldap->search()->findBy($this->ldapSettings['ldap_username_field'], $username);
-                $record = $this->ldap->search()->findBy($this->ldapSettings['ldap_username_field'], $username);
+            
-            } catch (ModelNotFoundException $e) {
+            if($record) {
-                Log::error($e->getMessage());
+                if ($this->isLdapSync($record)) {
-                throw new Exception('Unable to find user in LDAP directory!');
+                    $this->syncUserLdapLogin($record, $password);
+                }
             }
-
+            else {
-            $this->syncUserLdapLogin($record, $password);
+                Log::error($e->getMessage());
+                throw new Exception('Unable to find user in LDAP directory!');    
+            }            
+        } catch (ModelNotFoundException $e) {
+            Log::error($e->getMessage());
+            throw new Exception('Unable to find user in LDAP directory!');
         }
 
         return User::where('username', $username)
@@ -126,21 +133,19 @@ class LdapAd extends LdapAdConfiguration
     public function processUser(AdldapUser $user, ?Collection $defaultLocation=null, ?Collection $mappedLocations=null): ?User
     {
         // Only sync active users
-        if ($this->isLdapSync($user)) {
+        if(!$user) {
-            $snipeUser = [];
+            return null;
-            $snipeUser['username']        = $user->{$this->ldapSettings['ldap_username_field']}[0] ?? '';
-            $snipeUser['employee_number'] = $user->{$this->ldapSettings['ldap_emp_num']}[0] ?? '';
-            $snipeUser['lastname']        = $user->{$this->ldapSettings['ldap_lname_field']}[0] ?? '';
-            $snipeUser['firstname']       = $user->{$this->ldapSettings['ldap_fname_field']}[0] ?? '';
-            $snipeUser['email']           = $user->{$this->ldapSettings['ldap_email']}[0] ?? '';
-            $snipeUser['location_id']     = $this->getLocationId($user, $defaultLocation, $mappedLocations);
-            $snipeUser['activated']       = $this->getActiveStatus($user);
-
-            return $this->setUserModel($snipeUser);
         }
+        $snipeUser = [];
+        $snipeUser['username']        = $user->{$this->ldapSettings['ldap_username_field']}[0] ?? '';
+        $snipeUser['employee_number'] = $user->{$this->ldapSettings['ldap_emp_num']}[0] ?? '';
+        $snipeUser['lastname']        = $user->{$this->ldapSettings['ldap_lname_field']}[0] ?? '';
+        $snipeUser['firstname']       = $user->{$this->ldapSettings['ldap_fname_field']}[0] ?? '';
+        $snipeUser['email']           = $user->{$this->ldapSettings['ldap_email']}[0] ?? '';
+        $snipeUser['location_id']     = $this->getLocationId($user, $defaultLocation, $mappedLocations);
+        $snipeUser['activated']       = $this->getActiveStatus($user);
 
-        // We are not syncing user info
+        return $this->setUserModel($snipeUser);
-        return null;
     }
 
     /**
@@ -161,7 +166,7 @@ class LdapAd extends LdapAdConfiguration
                     'username' => $userInfo['username'],
                 ]);
         $user->username     = $user->username ?? trim($userInfo['username']);
-        $user->password     = $user->password ?? $this->generateEncyrptedPassword();
+        $user->password     = $user->password ?? Helper::generateEncyrptedPassword();
         $user->first_name   = trim($userInfo['firstname']);
         $user->last_name    = trim($userInfo['lastname']);
         $user->email        = trim($userInfo['email']);
@@ -349,6 +354,7 @@ class LdapAd extends LdapAdConfiguration
             $this->ldapSettings['ldap_lname_field'],
             $this->ldapSettings['ldap_email'],
             $this->ldapSettings['ldap_emp_num'],
+            $this->ldapSettings['ldap_active_flag'],
             'memberOf',
             'useraccountcontrol',
         ];

app/Services/LdapAdConfiguration.php

@@ -22,7 +22,7 @@ class LdapAdConfiguration
     const LDAP_PORT             = 389;
     const CONNECTION_TIMEOUT    = 5;
     const DEFAULT_LDAP_VERSION  = 3;
-    const LDAP_BOOLEAN_SETTINGS = ['ldap_enabled', 'ldap_server_cert_ignore', 'ldap_active_flag', 'ldap_tls', 'ldap_tls', 'ldap_pw_sync', 'is_ad'];
+    const LDAP_BOOLEAN_SETTINGS = ['ldap_enabled', 'ldap_server_cert_ignore', 'ldap_tls', 'ldap_tls', 'ldap_pw_sync', 'is_ad'];
 
     /**
      * Ldap Settings.

app/Traits/UserTrait.php

@@ -1,36 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace App\Traits;
-
-trait UserTrait
-{
-    /**
-     * Generate a random encrypted password.
-     *
-     * @author Wes Hulette <jwhulette@gmail.com>
-     *
-     * @since 5.0.0
-     *
-     * @return string
-     */
-    public function generateEncyrptedPassword(): string
-    {
-        return bcrypt($this->generateUnencryptedPassword());
-    }
-
-    /**
-     * Get a random unencrypted password.
-     *
-     * @author Wes Hulette <jwhulette@gmail.com>
-     *
-     * @since 5.0.0
-     *
-     * @return string
-     */
-    public function generateUnencryptedPassword(): string
-    {
-        return substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 20);
-    }
-}