From b54e7dc3eee866056bce694901e446d5642efc40 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 19 Jul 2023 17:44:40 +0100 Subject: [PATCH] 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 f77f5f8c4..58b421403 100755 --- a/app/Console/Commands/LdapSync.php +++ b/app/Console/Commands/LdapSync.php @@ -180,10 +180,6 @@ class LdapSync extends Command } } - /* 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 @@ class LdapSync extends Command } 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 7b22f3af4..8962f67cc 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -362,7 +362,13 @@ class UsersController extends Controller $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 011184881..319ebd041 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -191,9 +191,11 @@ class LoginController extends Controller $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 4eb496a2a..164b6cd53 100644 --- a/app/Models/Ldap.php +++ b/app/Models/Ldap.php @@ -252,13 +252,10 @@ class Ldap extends Model $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 @@ class Ldap extends Model 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 71bd9169a..efb38f376 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 98a3ec346..007489f90 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -465,6 +465,22 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo 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