From b54e7dc3eee866056bce694901e446d5642efc40 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 19 Jul 2023 17:44:40 +0100 Subject: [PATCH 1/5] Fixed #13336 - Save unhashed password if no password provided Signed-off-by: snipe --- app/Console/Commands/LdapSync.php | 6 +----- app/Http/Controllers/Api/UsersController.php | 8 +++++++- app/Http/Controllers/Auth/LoginController.php | 2 ++ app/Models/Ldap.php | 7 ++----- app/Models/SCIMUser.php | 3 +-- app/Models/User.php | 16 ++++++++++++++++ 6 files changed, 29 insertions(+), 13 deletions(-) diff --git a/app/Console/Commands/LdapSync.php b/app/Console/Commands/LdapSync.php index f77f5f8c4851..58b421403268 100755 --- 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 index 7b22f3af4bad..8962f67cca61 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -362,7 +362,13 @@ public function store(SaveUserRequest $request) $user->permissions = $permissions_array; } - $tmp_pass = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 40); + // + if ($request->filled('password')) { + $user->password = bcrypt($request->get('password')); + } else { + $user->password = $user->noPassword(); + } + $user->password = bcrypt($request->get('password', $tmp_pass)); 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 index 011184881c98..319ebd041819 100644 --- 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 index 4eb496a2ab1e..164b6cd5392e 100644 --- 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->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 index 71bd9169aec8..efb38f37654b 100644 --- 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; parent::__construct($attributes); + $this->noPassword(); } } \ No newline at end of file diff --git a/app/Models/User.php b/app/Models/User.php index 98a3ec346ba2..007489f9009d 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -465,6 +465,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 + * @since [v6.2.0] + * @return string + */ + public function noPassword() + { + return "*** NO PASSWORD ***"; + } + /** * Query builder scope to return NOT-deleted users From 611db4c0d2dac22df49f428a3660d84395a3c8fc Mon Sep 17 00:00:00 2001 From: snipe Date: Mon, 28 Aug 2023 20:46:45 +0100 Subject: [PATCH 2/5] Removed stray line Signed-off-by: snipe --- app/Http/Controllers/Api/UsersController.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 8962f67cca61..663f0aad73be 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -368,9 +368,7 @@ public function store(SaveUserRequest $request) } else { $user->password = $user->noPassword(); } - - $user->password = bcrypt($request->get('password', $tmp_pass)); - + app('App\Http\Requests\ImageUploadRequest')->handleImages($user, 600, 'image', 'avatars', 'avatar'); if ($user->save()) { From baffcbad7102101b1e8ae0ed4daea0d3d7b9d2d6 Mon Sep 17 00:00:00 2001 From: snipe Date: Mon, 28 Aug 2023 20:47:56 +0100 Subject: [PATCH 3/5] Set password property properly Signed-off-by: snipe --- app/Http/Controllers/Api/UsersController.php | 2 +- app/Models/Ldap.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 663f0aad73be..074e12a6d5eb 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -368,7 +368,7 @@ public function store(SaveUserRequest $request) } else { $user->password = $user->noPassword(); } - + app('App\Http\Requests\ImageUploadRequest')->handleImages($user, 600, 'image', 'avatars', 'avatar'); if ($user->save()) { diff --git a/app/Models/Ldap.php b/app/Models/Ldap.php index 164b6cd5392e..ae1f163dda3f 100644 --- a/app/Models/Ldap.php +++ b/app/Models/Ldap.php @@ -252,7 +252,7 @@ public static function createUserFromLdap($ldapatttibutes, $password) $user->last_name = $item['lastname']; $user->username = $item['username']; $user->email = $item['email']; - $user->noPassword(); + $user->password = $user->noPassword(); if (Setting::getSettings()->ldap_pw_sync == '1') { $user->password = bcrypt($password); From 18ff810d7e92f317f17096d174534ef16f9abfd8 Mon Sep 17 00:00:00 2001 From: snipe Date: Mon, 28 Aug 2023 20:51:52 +0100 Subject: [PATCH 4/5] Reverse orderof parent Signed-off-by: snipe --- app/Models/SCIMUser.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Models/SCIMUser.php b/app/Models/SCIMUser.php index efb38f37654b..97258a6e93a4 100644 --- a/app/Models/SCIMUser.php +++ b/app/Models/SCIMUser.php @@ -9,7 +9,8 @@ class SCIMUser extends User protected $throwValidationExceptions = true; // we want model-level validation to fully THROW, not just return false public function __construct(array $attributes = []) { - parent::__construct($attributes); $this->noPassword(); + parent::__construct($attributes); + } } \ No newline at end of file From dcae5503c86ac7072a8affc8d058be2430798aae Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 14 Sep 2023 13:52:57 +0100 Subject: [PATCH 5/5] Use $attributes array Signed-off-by: snipe --- app/Models/SCIMUser.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Models/SCIMUser.php b/app/Models/SCIMUser.php index 97258a6e93a4..fcf34c0d5d08 100644 --- 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 = []) { - $this->noPassword(); + $attributes['password'] = $this->noPassword(); parent::__construct($attributes); - } } \ No newline at end of file