Weird check for Ldap Account active

[thoro]

The code in LdapSync.php from #3640 uses this check:

                if( array_key_exists('useraccountcontrol', $results[$i]) ) {
                    $enabled_accounts = [
                        '512', '544', '66048', '66080', '262656', '262688', '328192', '328224'
                    ];
                    $item['activated'] = ( in_array($results[$i]['useraccountcontrol'][0], $enabled_accounts) ) ? 1 : 0;
                } else {
                    $item['activated'] = 0;
                }

But AD actually defines pretty well that ACCOUNTDISABLE is 0x2, is there any specific reason to have the non-exhaustive list?

Wouldn't a check like this be better:

                if( array_key_exists('useraccountcontrol', $results[$i]) ) {
                    $item['activated'] = ($results[$i]['useraccountcontrol'][0] & 0x2) === 0x2 ? 0 : 1;
                } else {
                    $item['activated'] = 0;
                }

Additionally in the documentation https://snipe-it.readme.io/v4.6.3/docs/ldap-sync-login there is this ldap filter:

&(sAMAccountType=805306368)(!(userAccountControl:1.2.840.113556.1.4.803:=2))

which will exclude any deactivated users and break the disabling user account feature

[snipe]

Wouldn't a check like this be better:

I don't think there was a specific reason it was limited.

which will exclude any deactivated users and break the disabling user account feature

Yes, not everyone wants to import AD-deactivated users at all. There are a lot of different ways people use the syncing features and for many, they'll have thousands of deactivated users (schools, for example), that they don't want to import at all, not just import and leave as deactivated.

[thoro]

So, we could adapt the check in the sync. (I can give a PR)

And amend the documentation with that mention.

[elkbullwinkle]

This change broke our sync with freeipa now all accounts are disabled after the sync.

[elkbullwinkle]

else { $item['activated'] = 0; }
This fallback change disables all users account imported from Ldap which don't have this Active directory specific field.

[thoro]

Pretty sure you mean the change in #3640 , how are accounts disabled in FreeIPA?

In this case I would add another check where the corresponding field can be set in the settings.

[elkbullwinkle]

@thoro yes that's what I mean, the logic is backwards in the change the checked field just doesn't exist in freeIPA so all the users are always deactivate, plus I was able to track down another related issue. When the system attempts to login a freeIpa user there is a condition added where('activated', 1) and if no users found it attempts to recreate it again and records it in the logs. Anyhow it became unusable after #3640 introduced as it requires some manual fiddling with activated in the database after each sync. In the meantime we have commented out this check. But that's a blocker for ldap servers users without this field. I mean there is already a checkbox in settings whether AD is used, why wouldn't make this check depending on that checkbox if it's AD only.

Regards.

[stale]

Is this still relevant? We haven't heard from anyone in a bit. If so, please comment with any updates or additional detail.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Don't take it personally, we just need to keep a handle on things. Thank you for your contributions!

[stale]

This issue has been automatically closed because it has not had recent activity. If you believe this is still an issue, please confirm that this issue is still happening in the most recent version of Snipe-IT and reply to this thread to re-open it.