LDAP manager attribute not being synced correctly in AD (was: LDAP Sync fails after adding manager attribue on 6.0 stable (Error 500))

[jed214]

Debug mode

• I have enabled debug mode
• I have read checked the Common Issues page

Describe the bug

Whenever I run ldap sync I get error 500. Login and everything else works just fine.

It works though in the ldap settings page but that is probably because it does not lookup the manager attribute.
I am running Active Directory on the other side of the ldap. I tested ldap and ldaps.
When running the ldap sync from cli I get this

Reproduction steps

1.Upgrade to php 7.4
2.Upgrade to snipe 6.0
3.Configure LDAP sync with manager attribue set (manager for AD)
4. Go to people tab or run command for ldap sync
5. See the error 500 or the one in the screenshot.
6. Leave manager field in ldap settings blank and it all works again
...

Expected behavior

Users sync and existing ones update with their manager field (other user object) from active directory.

Screenshots

Snipe-IT Version

v6.0.0 build 6860 (g722e88a47)

Operating System

Debian

Web Server

Apache

PHP Version

7.4

Operating System

Windows

Browser

Tested on Chrome and Firefox

Version

newest

Device

No response

Operating System

No response

Browser

No response

Version

No response

Error messages

No response

Additional context

No response

[welcome]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[uberbrady]

@Godmartinz - I think you donked around with some bits here - any help you can give?

[jed214]

Should I maybe rollback the update and run it with php 8.1 installed instead of 7.4? Which one is better tested?

[uberbrady]

I would stick with php 7.4

As for whether or not you roll back, that’s kinda your call. I’d love to track down the error in the sync script if we can and get it fixed before the end of the weekend, but I can’t guarantee that

[uberbrady]

And another question - if you remove the Manager attribute from the settings, do the LDAP syncs work out ok?

[jed214]

I would stick with php 7.4

As for whether or not you roll back, that’s kinda your call. I’d love to track down the error in the sync script if we can and get it fixed before the end of the weekend, but I can’t guarantee that

Well even if I upgrade php to 8.1 I dont think the error will be resolved, not really so you will still have your option to look into it.

And another question - if you remove the Manager attribute from the settings, do the LDAP syncs work out ok?

Yup, removing the manager attribute from settings makes the sync work again without an issue. I posted this in reproducing steps.

It might also help that I am running a lxc container from turnkeylinux.org but the update process I done manually since its pure debian under.

[uberbrady]

ok, I can replicate the bug and I'm working on a fix. I think it looks like it should be pretty easy?

[uberbrady]

As soon as we can get this merged, I'd love for you to give it another whirl if you can.

[jed214]

Sure, give me a ping in this thread to check on my end the problem is resolved.

[uberbrady]

Ok @jed214 - @snipe has merged my PR against master, can you pull down the latest and see if it fixes your problem?

[jed214]

Well yes it now syncs, but the Manager field is empty though after the sync even if its populated in Active Directory with a user that is also being synced to Snipe

[uberbrady]

Hrm. What setting are you using for the "LDAP Manager" field? I'm using manager there and it is correctly syncing AD managers - only if they actually exist in Snipe-IT though.

[jed214]

I am also using manager. It syncs but even on a user I created just now the field is empty when on AD its populated by my user that is in also synced to snipe

[uberbrady]

Hrm. What are you using for your Username field?

[jed214]

[jed214]

Before you ask I tried case sensitive

But then it does not work for and sync fails

[jed214]

Strange enough in the query field it works in the case sensitive form

[uberbrady]

Yeah, it's weird like that - in the query field, it wants the case-sensitive version, but in the username field, it wants all lower-case.

[uberbrady]

If the user actually exists in Snipe-IT, the manager should get set to that user. Are there any users that can get correctly set as manager? Like, maybe one in the Base DN level? Maybe one in a different OU? It's possible that that manager-setting logic needs to be improved.

[uberbrady]

Does the manager exist within that LDAP Filter?

[jed214]

Unfortunately none get the manager set from ldap sync. We have users in various ous with various managers but none got through for us into snipe. niedz., 15 maj 2022, 19:39 użytkownik Brady Wetherington < ***@***.***> napisał:

…

If the user actually exists in Snipe-IT, the manager should get set to that user. Are there *any* users that can get correctly set as manager? Like, maybe one in the Base DN level? Maybe one in a different OU? It's possible that that manager-setting logic needs to be improved. — Reply to this email directly, view it on GitHub <#11086 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXHSB5YW65W6F3SKQ76OX4LVKEZEVANCNFSM5V6G5IRA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jed214]

Yes it does. As you can see on the ss the filter is the employee group wich consists of every employee user in the domain niedz., 15 maj 2022, 19:41 użytkownik Brady Wetherington < ***@***.***> napisał:

…

Does the manager exist within that LDAP Filter? — Reply to this email directly, view it on GitHub <#11086 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXHSB5ZBBWIARKHPU6ESIADVKEZMJANCNFSM5V6G5IRA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[uberbrady]

I'm not able to reproduce this on my AD server. I've even tried to set the manager to a user in a different OU (but an OU which does sync, via per-location OU settings), and it still syncs correctly. Any other ideas as to what might be 'weird' in your Active Directory @jed214 ? I'd love to fix this for you (and others!) but I can't unless I can somehow reproduce it...

You might want to try throwing some \Log::error() statements into the LDAP sync script itself to see if you can drill down to find out what's going wrong, maybe?

[jed214]

Yeah first things first my locstions dont have a ou specified in snipe, is that required? niedz., 15 maj 2022, 21:07 użytkownik Brady Wetherington < ***@***.***> napisał:

…

I'm not able to reproduce this on my AD server. I've even tried to set the manager to a user in a different OU (but an OU which *does* sync, via per-location OU settings), and it still syncs correctly. Any other ideas as to what might be 'weird' in your Active Directory @jed214 <https://github.com/jed214> ? I'd love to fix this for you (and others!) but I can't unless I can somehow reproduce it... You might want to try throwing some \Log::error() statements into the LDAP sync script itself to see if you can drill down to find out what's going wrong, maybe? — Reply to this email directly, view it on GitHub <#11086 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXHSB57HVOIBGDBYDUN7XHLVKFDP7ANCNFSM5V6G5IRA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jed214]

Ok I have tried to set the ldap ou to one of the locations and the location field has updated for all users in that ou, but it did not help with the manager field.

You might want to try throwing some \Log::error() statements into the LDAP sync script itself to see if you can drill down to find out what's going wrong, maybe?

You need to tell me how to exactly do that, as I am unsure if I understand what you want me to do.

[crakttremblay]

I have th esame error. I had the Error 500 when using the manager field and syncing the LDAP users. Disabling it would solved the problem. Did an update a few minutes ago, I can set the manager field now and sync correctly but the manager field doesn't populate. The ones that have managers are the one that I manually set in snipe-it.

[uberbrady]

So in the file - app/Console/Commands/LdapSync.php - around line 218, right after it says //Captures only the Canonical Name - make these changes to add this stuff:

                if($item['manager'] != null) {
                    //Captures only the Canonical Name
                    \Log::error("MANAGER IS: ".$item['manager']);
                    $item['manager'] = ltrim($item['manager'], "CN=");
                    \Log::error("After ltrim is: ".$item['manager']);
                    $item['manager'] = substr($item['manager'],0, strpos($item['manager'], ','));
                    \Log::error("Final is: ".$item['manager']);
                    $ldap_manager = User::where('username', $item['manager'])->first();
                    if ( $ldap_manager && isset($ldap_manager->id) ) {
                        $user->manager_id = $ldap_manager->id;
                    }
                }

Then, if you can let me know what you see in your logs after that, maybe we can track it down?

Thank you so much for your help, everybody! Hopefully we can get this properly nailed down soon.

[abakerwwsb]

I also am not getting managers to sync either. Following uberbrady's advice for adding the Log::error I get the following, so it seems that the trim isn't handled properly?

(changed name and DC and couple OU names, but format remains the same

[2022-05-16 22:10:25] development.ERROR: MANAGER IS: CN=Smith\, John,OU=Department,OU=Users,OU=Location,DC=contoso,DC=com  
[2022-05-16 22:10:25] development.ERROR: After ltrim is: Smith\, John,OU=Department,OU=Users,OU=Location,DC=contoso,DC=com  
[2022-05-16 22:10:25] development.ERROR: Final is: Smith\  

[uberbrady]

Yeah, that seems like the problem. It seems like "LastName, FirstName" isn't getting handled properly by our code that tries to extract the Common Name (CN) from the attribute. We're instead truncating after that comma. That at least looks like something that's fixable.

[uberbrady]

So what does "John Smith"'s username look like in your system? Is it "Smith\, John" or "Smith, John" or something else? What are you using for your LDAP Username Attribute, uid? Something else?

Just want to make sure that when we push a fix for this we don't break it for someone else.

[abakerwwsb]

My snipe-it ldap settings page looks exactly like jed214's screenshot above, except my LDAP Filter has a user filter at the beginning "&(objectClass=user)(memberOf..." (the rest is the same). So I am using "samaccountname" in the "Username Field" box.

The sAMAccountName attribute is actually firstname.lastname, and that is how users login, so his is "john.smith", but active directory seems to store the users "distinguishedName" as the manager name, which is CN=Smith\, John,OU=Department,OU=Users,OU=Location,DC=contoso,DC=com

[uberbrady]

Yeah, I think I'm going to have to actually throw in a lookup on DN, and then extract whatever we're calling the 'username' attribute from that - then do the DB lookup. Maybe with some sort of cache - under the assumption that the subset of users who are managers is much smaller than the regular set of users. Maybe we don't have to do that? We'll see.

[jed214]

Hi,
Yes, for me its also name.surename format for samaccountname attribute that is being used for login.

[jed214]

So in the file - app/Console/Commands/LdapSync.php - around line 218, right after it says //Captures only the Canonical Name - make these changes to add this stuff:

                if($item['manager'] != null) {
                    //Captures only the Canonical Name
                    \Log::error("MANAGER IS: ".$item['manager']);
                    $item['manager'] = ltrim($item['manager'], "CN=");
                    \Log::error("After ltrim is: ".$item['manager']);
                    $item['manager'] = substr($item['manager'],0, strpos($item['manager'], ','));
                    \Log::error("Final is: ".$item['manager']);
                    $ldap_manager = User::where('username', $item['manager'])->first();
                    if ( $ldap_manager && isset($ldap_manager->id) ) {
                        $user->manager_id = $ldap_manager->id;
                    }
                }

Then, if you can let me know what you see in your logs after that, maybe we can track it down?

Thank you so much for your help, everybody! Hopefully we can get this properly nailed down soon.

Do you mean you want this log?
This is after changing the code for what you have sent me. I had to hide the names though as its from a production server.

[jed214]

Hi guys, is there any update on this?

[snipe]

If there was, we'd have posted an update here. We've been dealing with a Google LDAP issue.

[takuy]

Yeah, I think I'm going to have to actually throw in a lookup on DN, and then extract whatever we're calling the 'username' attribute from that - then do the DB lookup. Maybe with some sort of cache - under the assumption that the subset of users who are managers is much smaller than the regular set of users. Maybe we don't have to do that? We'll see.

I was just going to post an issue about this. The manager attribute is always a proper DN. You shouldn't extrapolate information from it. You need to use it to look up the user in LDAP. It does add a bit of work, and a per-sync run cache would help for sure. There's no other way of handling it unless you only want to support a subset of however people choose to design their directories.

Our LDAP (which I have no control over) includes no names or usernames in the DN at all, and they're subject to change anyways, so syncing the manager just fails.

[mattcarras]

In my PowerShell implementation of LDAP syncing (utilizing the REST API due to security concerns), I do the lookup first, store it, then replace all references of a manager with a reference to the manager's object from cache, matching on distinguishedname (which is what the manager attribute contains). The way I see it, since only users in the main query will be synced it doesn't matter if the manager doesn't exist in the cache, as they shouldn't be created anyway.