Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feedback Wanted] Potential improvements for LDAP for future LDAP implementations #8741

Closed
uberbrady opened this issue Nov 13, 2020 · 14 comments
Assignees
Labels
🤙 open discussion Your opinion wanted! ldap

Comments

@uberbrady
Copy link
Collaborator

uberbrady commented Nov 13, 2020

I'm just trying to document the various LDAP issues we have all around and trying to collect together the main things I've been seeing into one central location. Feel free to chime in if you don't see something you want, or have had to deal with, listed here.

  1. the AdLdap2 library that we use is in maintenance mode; the author recommends moving to its successor, LdapRecord. AdLdap2 has some problems; including its implementation of paging, which causes memory issues.
  2. Users want the ability to have multiple AD servers, perhaps mapping each to a sub-company?
  3. Every field in the Users table in Snipe-IT should have the ability to be mapped to an LDAP attribute. This would solve a real-world problem one of our users encountered: [Feature request] LDAP Sync - Add Merge/Replace choice when doing a sync #8729
  4. Login via LDAP should potentially be enabled/disabled at the per-OU level?
  5. Filters in addition to our already existing per-OU mapping to map into groups/Locations?
  6. The settings are extremely confusing and trip people up all the time. One recent example: user forgot to put ldap:// in front of their LDAP server URL, and the system didn't warn them or nudge them at all, it just silently failed. Bummer.
  7. Client-side TLS certs may give us the ability to integrate with Google Apps LDAP.
  8. Permissions on LDAP-created users? or permission groups?
  9. People who don't use LDAP at all are still required to install php-ldap module. The ServiceProvider seems to get initialized whether it's used or not. (Potential fix without rewrite: switching to "Deferred Service Providers")
  10. PHP v7.4 has made some....questionable....choices on deprecating certain LDAP methods, which can show up as an Exception unless we suppress it or catch it.
  11. We conflate "activated/deactivated" - which means to us "can log in/can't log in" - with "User is deactivated in AD", which is more analogous to a 'soft delete'. There are bits of code where we use the wrong 'sense' - e.g., choosing not to sync 'deactivated' users - which would be wrong (if we do that, which I'm not sure of).
  12. Login and Sync might need different LDAP settings - perhaps one OU which isn't parent-or-child of another might need to log in, and a completely different OU might be where we need to sync from.
  13. We never 'clean up' properly-deleted users from Snipe-IT. E.g., they're originally from LDAP, we sync with the server, and their record isn't there anymore. Shouldn't we soft-delete them on our side too? Or at least block their login? (Note: users who have assets checked out to them cannot be deleted. So this adds some excitement to this problem).
  14. Users who are 'valid' somehow in AD (or LDAP) might not be 'valid' in Snipe-IT - one customer example I saw was a customer who had a few users with spaces in their email addresses. Snipe-IT's validations wouldn't allow that, so any users with that problem couldn't be sync'ed. We should at least give a better error indication for that.
  15. When working on LDAP settings, the "Save-before-test" flow is counter-intuitive and not helpful when you want to test that your settings will work before you save them.
  16. I don't think anyone correctly modifies LDAP schema to add an Active flag to mark whether or not people can log in to Snipe-IT. And I don't think we would want to ask them to; it's far too scary of a request.
@uberbrady uberbrady added the 🤙 open discussion Your opinion wanted! label Nov 13, 2020
@uberbrady uberbrady self-assigned this Nov 13, 2020
@snipe snipe changed the title [Feedback Wanted] Potential improvements for LDAP for "Snipe-IT LDAP version 3" [Feedback Wanted] Potential improvements for LDAP for future LDAP implementations Nov 13, 2020
@sabait
Copy link

sabait commented Nov 20, 2020

Really want to see the sync functionality mentioned on 3. implemented.

@jgarcesres
Copy link

  • Be able to assign permission to run ldap sync outside of the "super user" scope. Before v5 "Admin" role was able to
  • Schedule recurring ldap syncs from the webUI
  • Define default OU to drop users in if their OU is not mapped to a Locations

@snipe snipe added the ldap label Nov 25, 2020
@snipe snipe pinned this issue Nov 25, 2020
@benwa
Copy link
Contributor

benwa commented Dec 1, 2020

Our setup is pretty simplified: Active Directory domain, one company, simple hierarchy both with locations and chain of command (manager fields all filled out so organizational tree is correct).

1.
2. Could be extremely useful for sub-domains, separate domains, or separate forests
3. I very much want this, specifically for managers to be filled and hopefully automatically tally checked out devices to their chain
4.
5. Depending on how other directories are mapped, it might be useful to have the physicalDeliveryOffice be the location. Though there is also streetAddress and other attributes.
6.
7.
8. Permissions on groups is always a nice feature to have.
9.
10.
11. We will deactivate a user, and then delete them after a time. With AD, the recycle bin allows a restore with all* of their attributes retained.
12.
13. Soft-delete might be good. There is a case where John.Smith retires, and a new John.Smith is hired a year later and given the same email address.
14. An email address shouldn't prevent an account from being created. We could have users without email addresses but are assigned assets. A warning for an invalid RFC 5322 email address would be good in any case.
15. Agreed
16. Super agreed

@kno005
Copy link

kno005 commented Dec 2, 2020

I would love to see the AD sync based on the SID instead of the samaccountname. When a user changes their last name they get a new username. This causes a second user record to be created in Snipe with assets checked out to the original name. This requires a manual database edit to correct.

Also, a flag in the API that identifies that the user was imported from LDAP/AD would be very useful. I have a script that compares AD and Snipe and deletes Snipe users when they are deleted from AD. If any deleted users have assets assigned then it sends an email alert. I have to manually exclude built-in users in my script which is not the cleanest solution.

@ChicagoJay
Copy link

All of these sound like great ideas.

16 is tripping us up right now. On MY AD account! I get deactivated every time LDAP syncs.

@ghost
Copy link

ghost commented Dec 12, 2020

How about SAML and SCIM?

@snipe
Copy link
Owner

snipe commented Dec 12, 2020

@ChicagoJay That was fixed a few releases ago, I thought.... We've tested it and I don't think we can reproduce, can we, @uberbrady?

@trch15 SAML already exists in Snipe-IT. SCIM is outside the scope of the LDAP discussion (as is SAML), IMHO

@ChicagoJay
Copy link

ChicagoJay commented Dec 14, 2020

@ChicagoJay That was fixed a few releases ago, I thought.... We've tested it and I don't think we can reproduce, can we, @uberbrady?

@trch15 SAML already exists in Snipe-IT. SCIM is outside the scope of the LDAP discussion (as is SAML), IMHO

I'm happy to help track it down - tell me what you need. My other LDAP users are all fine, AFAICT.

We are running Snipe-IT v5.0.11 build 5695 (gfd4ee6027) with Laravel 6.18.10 and PHP 7.3.24-3+ubuntu18.04.1+deb.sury.org+1.

Interestingly, I am running this on Ubuntu 20.04.1 but the PHP version seems to think I am on ubuntu 18.04.1. When we installed Snipe-It originally, we were on 18.04.1 but I did an in-place upgrade a few months ago. I don't use the software often enough to tell if that is when it started disabling my account (and ONLY my account) during AD sync.

@nghia-dang
Copy link

nghia-dang commented Feb 16, 2021

  1. Can we please also have our own created custom user attributes that can also be mapped to ldap attributes as well ?
  2. On a separate note, can we also have optional custom field types that work in a simillar way to the location LDAP search OU, but also works with an LDAP filter to search through a CN for user existence ? We would like to also map conditional custom attributes based on if a user exists in a particular CN

@raphaeljoie
Copy link

Hello, I don't know if it falls into that scope, but I'd like to integrate snipe it with a third party app.
Do you plan to provide an oauth flow to generate user access tokens?

The workaround I have now, is to ask my people to create an access token from their profile, then to store it in the second app. But it's very not secure to store access tokens.

@taylor1123
Copy link

Would love to see number 5 seems that people have asked for years for this

@dgw
Copy link

dgw commented Mar 10, 2021

Is there an existing standalone issue for item 9 that I'm too blind to see in search?

We're blocked from updating Snipe-IT to v5 because our hosting doesn't provide php-ldap and it's just not worth us paying for a dedicated server only to update Snipe-IT when the blocker is a dependency for a feature we won't even use.

@00shorty
Copy link

00shorty commented Mar 25, 2021

  • I would love to see that the AD sync can be used to set a default permission group for all new users that are imported with the sync.
  • It would be also nice if we could define a mapping between AD-Groups and Snipe-IT permission groups to control rights for users within AD

@exoup
Copy link

exoup commented Apr 6, 2021

The two primary features I'd be interested in in:

  • The ability to exclude certain OUs from a directory synchronization (we have a subset of users in our AD that will never be receiving or checking out assets and thus don't need to be in the system)

  • And the ability to provide more than one LDAP Search OU for a location. I.e. people in the red and blue OU could both be added to the purple group location in Snipe-IT.

@snipe snipe closed this as completed Apr 14, 2021
Repository owner locked and limited conversation to collaborators Apr 14, 2021
@snipe snipe unpinned this issue Apr 14, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
🤙 open discussion Your opinion wanted! ldap
Projects
None yet
Development

No branches or pull requests