Merge pull request #13343 from snipe/fixes/re-scramble_password_if_ld…

…ap_pw_sync_not_enabled Fixed #13336 - Save unhashed password if no password provided
snipe · Sep 14, 2023 · 2166d66 · 2166d66
2 parents d916e20 + dcae550
commit 2166d66
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 14 deletions.
diff --git a/app/Console/Commands/LdapSync.php b/app/Console/Commands/LdapSync.php
@@ -180,10 +180,6 @@ public function handle()
             }
         }
 
-        /* Create user account entries in Snipe-IT */
-        $tmp_pass = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 20);
-        $pass = bcrypt($tmp_pass);
-
         $manager_cache = [];
 
         if($ldap_default_group != null) {
@@ -229,7 +225,7 @@ public function handle()
                 } else {
                     // Creating a new user.
                     $user = new User;
-                    $user->password = $pass;
+                    $user->password = $user->noPassword();
                     $user->activated = 1; // newly created users can log in by default, unless AD's UAC is in use, or an active flag is set (below)
                     $item['createorupdate'] = 'created';
                 }

diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php
@@ -363,8 +363,12 @@ public function store(SaveUserRequest $request)
             $user->permissions = $permissions_array;
         }
 
-        $tmp_pass = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 40);
-        $user->password = bcrypt($request->get('password', $tmp_pass));
+        // 
+        if ($request->filled('password')) {
+            $user->password = bcrypt($request->get('password'));
+        } else {
+            $user->password = $user->noPassword();
+        }
 
         app('App\Http\Requests\ImageUploadRequest')->handleImages($user, 600, 'image', 'avatars', 'avatar');
 

diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
@@ -191,9 +191,11 @@ private function loginViaLdap(Request $request): User
 
              $ldap_attr = Ldap::parseAndMapLdapAttributes($ldap_user);
 
+            $user->password = $user->noPassword();
             if (Setting::getSettings()->ldap_pw_sync=='1') {
                 $user->password = bcrypt($request->input('password'));
             }
+
             $user->email = $ldap_attr['email'];
             $user->first_name = $ldap_attr['firstname'];
             $user->last_name = $ldap_attr['lastname']; //FIXME (or TODO?) - do we need to map additional fields that we now support? E.g. country, phone, etc.

diff --git a/app/Models/Ldap.php b/app/Models/Ldap.php
@@ -252,13 +252,10 @@ public static function createUserFromLdap($ldapatttibutes, $password)
             $user->last_name = $item['lastname'];
             $user->username = $item['username'];
             $user->email = $item['email'];
+            $user->password = $user->noPassword();
 
             if (Setting::getSettings()->ldap_pw_sync == '1') {
-
                 $user->password = bcrypt($password);
-            } else {
-                $pass = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 25);
-                $user->password = bcrypt($pass);
             }
 
             $user->activated = 1;
@@ -268,7 +265,7 @@ public static function createUserFromLdap($ldapatttibutes, $password)
             if ($user->save()) {
                 return $user;
             } else {
-                LOG::debug('Could not create user.'.$user->getErrors());
+                \Log::debug('Could not create user.'.$user->getErrors());
                 throw new Exception('Could not create user: '.$user->getErrors());
             }
         }

diff --git a/app/Models/SCIMUser.php b/app/Models/SCIMUser.php
@@ -9,8 +9,7 @@ class SCIMUser extends User
     protected $throwValidationExceptions = true; // we want model-level validation to fully THROW, not just return false
 
     public function __construct(array $attributes = []) {
-        $attributes['password'] = "*NO PASSWORD*";
-        // $attributes['activated'] = 1;
+        $attributes['password'] = $this->noPassword();
         parent::__construct($attributes);
     }
 }
diff --git a/app/Models/User.php b/app/Models/User.php
@@ -456,6 +456,22 @@ public function checkoutRequests()
         return $this->belongsToMany(Asset::class, 'checkout_requests', 'user_id', 'requestable_id')->whereNull('canceled_at');
     }
 
+    /**
+     * Set a common string when the user has been imported/synced from:
+     *
+     * - LDAP without password syncing
+     * - SCIM
+     * - CSV import where no password was provided
+     *
+     * @author A. Gianotto <[email protected]>
+     * @since [v6.2.0]
+     * @return string
+     */
+    public function noPassword()
+    {
+        return "*** NO PASSWORD ***";
+    }
+
 
     /**
      * Query builder scope to return NOT-deleted users