From 74c099f0b3884940e22ba0be70150a6235b6bb94 Mon Sep 17 00:00:00 2001 From: Steffen Date: Tue, 15 Jan 2019 23:05:47 +0100 Subject: [PATCH] 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 | 21 +++++++---- app/Helpers/Helper.php | 32 ++++++++++++++++- app/Services/LdapAd.php | 52 ++++++++++++++++------------ app/Services/LdapAdConfiguration.php | 2 +- app/Traits/UserTrait.php | 36 ------------------- 5 files changed, 75 insertions(+), 68 deletions(-) delete mode 100644 app/Traits/UserTrait.php diff --git a/app/Console/Commands/LdapSync.php b/app/Console/Commands/LdapSync.php index dac3032513..c8a0fecd65 100755 --- a/app/Console/Commands/LdapSync.php +++ b/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); } diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index 59f7e24a66..5b4ca1fde0 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -670,7 +670,37 @@ class Helper return false; } + /** + * Generate a random encrypted password. + * + * @author Wes Hulette + * + * @since 5.0.0 + * + * @return string + */ + public static function generateEncyrptedPassword(): string + { + return bcrypt(Helper::generateUnencryptedPassword()); + } + /** + * Get a random unencrypted password. + * + * @author Steffen Buehl + * + * @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; + } } diff --git a/app/Services/LdapAd.php b/app/Services/LdapAd.php index d5df03fdb2..8afbb578f0 100644 --- a/app/Services/LdapAd.php +++ b/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 { - Log::debug('Attempting to find user in LDAP directory'); - $record = $this->ldap->search()->findBy($this->ldapSettings['ldap_username_field'], $username); - } catch (ModelNotFoundException $e) { - Log::error($e->getMessage()); - throw new Exception('Unable to find user in LDAP directory!'); + try { + Log::debug('Attempting to find user in LDAP directory'); + $record = $this->ldap->search()->findBy($this->ldapSettings['ldap_username_field'], $username); + + if($record) { + if ($this->isLdapSync($record)) { + $this->syncUserLdapLogin($record, $password); + } } - - $this->syncUserLdapLogin($record, $password); + else { + 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)) { - $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); - - return $this->setUserModel($snipeUser); + if(!$user) { + return null; } + $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 null; + return $this->setUserModel($snipeUser); } /** @@ -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', ]; diff --git a/app/Services/LdapAdConfiguration.php b/app/Services/LdapAdConfiguration.php index e6267a97db..d25679faaa 100644 --- a/app/Services/LdapAdConfiguration.php +++ b/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. diff --git a/app/Traits/UserTrait.php b/app/Traits/UserTrait.php deleted file mode 100644 index 9bc6d92fc2..0000000000 --- a/app/Traits/UserTrait.php +++ /dev/null @@ -1,36 +0,0 @@ - - * - * @since 5.0.0 - * - * @return string - */ - public function generateEncyrptedPassword(): string - { - return bcrypt($this->generateUnencryptedPassword()); - } - - /** - * Get a random unencrypted password. - * - * @author Wes Hulette - * - * @since 5.0.0 - * - * @return string - */ - public function generateUnencryptedPassword(): string - { - return substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 20); - } -}