Clean up how we use the '$location' in LDAP sync command

This commit is contained in:
Brady Wetherington 2024-10-15 17:26:31 +01:00
parent 16c8264e76
commit e4e1d0d50a

View file

@ -137,23 +137,24 @@ class LdapSync extends Command
} }
/* Determine which location to assign users to by default. */ /* Determine which location to assign users to by default. */
$location = null; // TODO - this would be better called "$default_location", which is more explicit about its purpose $default_location = null;
if ($this->option('location') != '') { if ($this->option('location') != '') {
if ($location = Location::where('name', '=', $this->option('location'))->first()) { if ($default_location = Location::where('name', '=', $this->option('location'))->first()) {
Log::debug('Location name ' . $this->option('location') . ' passed'); Log::debug('Location name ' . $this->option('location') . ' passed');
Log::debug('Importing to ' . $location->name . ' (' . $location->id . ')'); Log::debug('Importing to '.$default_location->name.' ('.$default_location->id.')');
} }
} elseif ($this->option('location_id')) { } elseif ($this->option('location_id')) {
//TODO - figure out how or why this is an array?
foreach($this->option('location_id') as $location_id) { foreach($this->option('location_id') as $location_id) {
if ($location = Location::where('id', '=', $location_id)->first()) { if ($default_location = Location::where('id', '=', $location_id)->first()) {
Log::debug('Location ID ' . $location_id . ' passed'); Log::debug('Location ID ' . $location_id . ' passed');
Log::debug('Importing to ' . $location->name . ' (' . $location->id . ')'); Log::debug('Importing to '.$default_location->name.' ('.$default_location->id.')');
} }
} }
} }
if (! isset($location)) { if (!isset($default_location)) {
Log::debug('That location is invalid or a location was not provided, so no location will be assigned by default.'); Log::debug('That location is invalid or a location was not provided, so no location will be assigned by default.');
} }
@ -229,43 +230,44 @@ class LdapSync extends Command
for ($i = 0; $i < $results['count']; $i++) { for ($i = 0; $i < $results['count']; $i++) {
$item = []; $item = [];
$item['username'] = $results[$i][$ldap_map["username"]][0] ?? ''; $item['username'] = $results[$i][$ldap_map["username"]][0] ?? '';
$item['employee_number'] = $results[$i][$ldap_map["emp_num"]][0] ?? ''; $item['employee_number'] = $results[$i][$ldap_map["emp_num"]][0] ?? '';
$item['lastname'] = $results[$i][$ldap_map["last_name"]][0] ?? ''; $item['lastname'] = $results[$i][$ldap_map["last_name"]][0] ?? '';
$item['firstname'] = $results[$i][$ldap_map["first_name"]][0] ?? ''; $item['firstname'] = $results[$i][$ldap_map["first_name"]][0] ?? '';
$item['email'] = $results[$i][$ldap_map["email"]][0] ?? ''; $item['email'] = $results[$i][$ldap_map["email"]][0] ?? '';
$item['ldap_location_override'] = $results[$i]['ldap_location_override'] ?? ''; $item['ldap_location_override'] = $results[$i]['ldap_location_override'] ?? '';
$item['location_id'] = $results[$i]['location_id'] ?? ''; $item['location_id'] = $results[$i]['location_id'] ?? '';
$item['telephone'] = $results[$i][$ldap_map["phone"]][0] ?? ''; $item['telephone'] = $results[$i][$ldap_map["phone"]][0] ?? '';
$item['jobtitle'] = $results[$i][$ldap_map["jobtitle"]][0] ?? ''; $item['jobtitle'] = $results[$i][$ldap_map["jobtitle"]][0] ?? '';
$item['country'] = $results[$i][$ldap_map["country"]][0] ?? ''; $item['country'] = $results[$i][$ldap_map["country"]][0] ?? '';
$item['department'] = $results[$i][$ldap_map["dept"]][0] ?? ''; $item['department'] = $results[$i][$ldap_map["dept"]][0] ?? '';
$item['manager'] = $results[$i][$ldap_map["manager"]][0] ?? ''; $item['manager'] = $results[$i][$ldap_map["manager"]][0] ?? '';
$item['location'] = $results[$i][$ldap_map["location"]][0] ?? ''; $item['location'] = $results[$i][$ldap_map["location"]][0] ?? '';
$location = $default_location; //initially, set '$location' to the default_location (which may just be `null`)
// ONLY if you are using the "ldap_location" option *AND* you have an actual result // ONLY if you are using the "ldap_location" option *AND* you have an actual result
if ($ldap_map["location"] && $item['location']) { if ($ldap_map["location"] && $item['location']) {
$location = Location::firstOrCreate([ $location = Location::firstOrCreate([
'name' => $item['location'], 'name' => $item['location'],
]);
}
$department = Department::firstOrCreate([
'name' => $item['department'],
]); ]);
}
$department = Department::firstOrCreate([
'name' => $item['department'],
]);
$user = User::where('username', $item['username'])->first(); $user = User::where('username', $item['username'])->first();
if ($user) { if ($user) {
// Updating an existing user. // Updating an existing user.
$item['createorupdate'] = 'updated'; $item['createorupdate'] = 'updated';
} else { } else {
// Creating a new user. // Creating a new user.
$user = new User; $user = new User;
$user->password = $user->noPassword(); $user->password = $user->noPassword();
$user->locale = app()->getLocale(); $user->locale = app()->getLocale();
$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) $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'; $item['createorupdate'] = 'created';
} }
//If a sync option is not filled in on the LDAP settings don't populate the user field //If a sync option is not filled in on the LDAP settings don't populate the user field
if($ldap_map["username"] != null){ if($ldap_map["username"] != null){
@ -296,7 +298,7 @@ class LdapSync extends Command
$user->department_id = $department->id; $user->department_id = $department->id;
} }
if($ldap_map["location"] != null){ if($ldap_map["location"] != null){
$user->location_id = $location ? $location->id : null; $user->location_id = $location?->id;
} }
if($ldap_map["manager"] != null){ if($ldap_map["manager"] != null){
@ -341,38 +343,38 @@ class LdapSync extends Command
} }
} }
// Sync activated state for Active Directory. // Sync activated state for Active Directory.
if ( !empty($ldap_map["active_flag"])) { // IF we have an 'active' flag set.... if (!empty($ldap_map["active_flag"])) { // IF we have an 'active' flag set....
// ....then *most* things that are truthy will activate the user. Anything falsey will deactivate them. // ....then *most* things that are truthy will activate the user. Anything falsey will deactivate them.
// (Specifically, we don't handle a value of '0.0' correctly) // (Specifically, we don't handle a value of '0.0' correctly)
$raw_value = @$results[$i][$ldap_map["active_flag"]][0]; $raw_value = @$results[$i][$ldap_map["active_flag"]][0];
$filter_var = filter_var($raw_value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); $filter_var = filter_var($raw_value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
$boolean_cast = (bool)$raw_value; $boolean_cast = (bool) $raw_value;
$user->activated = $filter_var ?? $boolean_cast; // if filter_var() was true or false, use that. If it's null, use the $boolean_cast $user->activated = $filter_var ?? $boolean_cast; // if filter_var() was true or false, use that. If it's null, use the $boolean_cast
} elseif (array_key_exists('useraccountcontrol', $results[$i]) ) { } elseif (array_key_exists('useraccountcontrol', $results[$i])) {
// ....otherwise, (ie if no 'active' LDAP flag is defined), IF the UAC setting exists, // ....otherwise, (ie if no 'active' LDAP flag is defined), IF the UAC setting exists,
// ....then use the UAC setting on the account to determine can-log-in vs. cannot-log-in // ....then use the UAC setting on the account to determine can-log-in vs. cannot-log-in
/* The following is _probably_ the correct logic, but we can't use it because /* The following is _probably_ the correct logic, but we can't use it because
some users may have been dependent upon the previous behavior, and this some users may have been dependent upon the previous behavior, and this
could cause additional access to be available to users they don't want could cause additional access to be available to users they don't want
to allow to log in. to allow to log in.
$useraccountcontrol = $results[$i]['useraccountcontrol'][0]; $useraccountcontrol = $results[$i]['useraccountcontrol'][0];
if( if(
// based on MS docs at: https://support.microsoft.com/en-us/help/305144/how-to-use-useraccountcontrol-to-manipulate-user-account-properties // based on MS docs at: https://support.microsoft.com/en-us/help/305144/how-to-use-useraccountcontrol-to-manipulate-user-account-properties
($useraccountcontrol & 0x200) && // is a NORMAL_ACCOUNT ($useraccountcontrol & 0x200) && // is a NORMAL_ACCOUNT
!($useraccountcontrol & 0x02) && // *and* _not_ ACCOUNTDISABLE !($useraccountcontrol & 0x02) && // *and* _not_ ACCOUNTDISABLE
!($useraccountcontrol & 0x10) // *and* _not_ LOCKOUT !($useraccountcontrol & 0x10) // *and* _not_ LOCKOUT
) { ) {
$user->activated = 1; $user->activated = 1;
} else { } else {
$user->activated = 0; $user->activated = 0;
} */ } */
$enabled_accounts = [ $enabled_accounts = [
'512', // 0x200 NORMAL_ACCOUNT '512', // 0x200 NORMAL_ACCOUNT
'544', // 0x220 NORMAL_ACCOUNT, PASSWD_NOTREQD '544', // 0x220 NORMAL_ACCOUNT, PASSWD_NOTREQD
'66048', // 0x10200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD '66048', // 0x10200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD
@ -385,44 +387,47 @@ class LdapSync extends Command
'4260352', // 0x410200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD, DONT_REQ_PREAUTH '4260352', // 0x410200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD, DONT_REQ_PREAUTH
'1049088', // 0x100200 NORMAL_ACCOUNT, NOT_DELEGATED '1049088', // 0x100200 NORMAL_ACCOUNT, NOT_DELEGATED
'1114624', // 0x110200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD, NOT_DELEGATED, '1114624', // 0x110200 NORMAL_ACCOUNT, DONT_EXPIRE_PASSWORD, NOT_DELEGATED,
]; ];
$user->activated = (in_array($results[$i]['useraccountcontrol'][0], $enabled_accounts)) ? 1 : 0; $user->activated = (in_array($results[$i]['useraccountcontrol'][0], $enabled_accounts)) ? 1 : 0;
// If we're not using AD, and there isn't an activated flag set, activate all users // If we're not using AD, and there isn't an activated flag set, activate all users
} /* implied 'else' here - leave the $user->activated flag alone. Newly-created accounts will be active. } /* implied 'else' here - leave the $user->activated flag alone. Newly-created accounts will be active.
already-existing accounts will be however the administrator has set them */ already-existing accounts will be however the administrator has set them */
if ($item['ldap_location_override'] == true) { if ($item['ldap_location_override'] == true) {
$user->location_id = $item['location_id']; $user->location_id = $item['location_id'];
} elseif ((isset($location)) && (! empty($location))) { } elseif ((isset($location)) && (!empty($location))) {
if ((is_array($location)) && (array_key_exists('id', $location))) { if ((is_array($location)) && (array_key_exists('id', $location))) {
$user->location_id = $location['id']; $user->location_id = $location['id'];
} elseif (is_object($location)) { } elseif (is_object($location)) {
$user->location_id = $location->id; $user->location_id = $location->id; //THIS is the magic line, this should do it.
}
} }
$location = null; }
$user->ldap_import = 1; // TODO - should we be NULLING locations if $location is really `null`, and that's what we came up with?
// will that conflict with any overriding setting that the user set? Like, if they moved someone from
// the 'null' location to somewhere, we wouldn't want to try to override that, right?
$location = null;
$user->ldap_import = 1;
$errors = ''; $errors = '';
if ($user->save()) { if ($user->save()) {
$item['note'] = $item['createorupdate']; $item['note'] = $item['createorupdate'];
$item['status'] = 'success'; $item['status'] = 'success';
if ( $item['createorupdate'] === 'created' && $ldap_default_group) { if ($item['createorupdate'] === 'created' && $ldap_default_group) {
$user->groups()->attach($ldap_default_group); $user->groups()->attach($ldap_default_group);
}
} else {
foreach ($user->getErrors()->getMessages() as $key => $err) {
$errors .= $err[0];
}
$item['note'] = $errors;
$item['status'] = 'error';
} }
array_push($summary, $item); } else {
foreach ($user->getErrors()->getMessages() as $key => $err) {
$errors .= $err[0];
}
$item['note'] = $errors;
$item['status'] = 'error';
}
array_push($summary, $item);
} }
if ($this->option('summary')) { if ($this->option('summary')) {