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.');
} }
@ -243,6 +244,7 @@ class LdapSync extends Command
$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']) {
@ -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){
@ -342,16 +344,16 @@ 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
@ -395,13 +397,16 @@ class LdapSync extends Command
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.
} }
} }
// 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; $location = null;
$user->ldap_import = 1; $user->ldap_import = 1;
@ -410,7 +415,7 @@ class LdapSync extends Command
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);
} }