-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
LDAP Sync disabled login by default #10563
Comments
That sounds like a filter issue perhaps. active does mean "able to login" |
No filter issue. I'm importing only the users I want to import, and they are all active in AD. But even if they are active in AD, I still do not want them to be able to login to Snipe-IT as we only want the IT-dep to be able to login to Snipe-IT. Please help me as to how I get the desired effect. ... My only other option is to mass-edit users and disable login after each LDAP sync. Which one day (in the heat of things) will be forgotten, and the users will then be able to log in. |
Again, the active flag in your Snipe-IT LDAP settings should do what you're asking for. Without introspection into your AD/LDAP settings, this normally just works as advertised. That said, if login/not-login is an issue, uncheck the "sync LDAP passwords" settings and/or (un)check the option in settings to link back to the Snipe-IT install. The active flag should handle this already, but if accounts get made via sync and no one knows urls, it usually doesn't matter. |
If I enter value "active" into the field LDAP Active Flag, all imported users are able to login (Login Enabled = Yes). I do not know what I should put in the field LDAP Active Flag to disable Snipe-IT login for imported users, and I do not have the time to guess the right value. Is there any values other than "active" which the field LDAP Active Flag accepts? It is the only value described in the documentation. Unchecking the "sync LDAP passwords" will not solve anything, as LDAP will query AD directly if the passwords are not synced. We are security aware, which means that not disclosing the URL will not be sufficient. |
Do you have anything on the above? (other accepted values for the field) Any and all help is much appreciated =) |
Following up on this. |
This is one for @uberbrady, since he was the last to meddle with that LDAP code. He's afk for now, but I'm sure he'll look into it soon. |
@snipe - thanks mate |
@avielc thanks for the additional debugging info - we're looking into this |
just for the meantime, are there any workaround you are aware of perhaps ? a way to permanently disable login to users? Thanks again! |
If you delete the active flag from your ldap settings, then manuall
ydisable login for users, then sync again, do they still get toggled to
“login enabled”?
…On Wed, Feb 2, 2022 at 2:28 PM avielc ***@***.***> wrote:
just for the meantime, are there any workaround you are aware of perhaps ?
a way to permanently disable login to users?
Thanks again!
—
Reply to this email directly, view it on GitHub
<#10563 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAI333JFNIBIZHSDSXGR23UZGVXTANCNFSM5M2PPAQQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I'll repeat what you said, to make sure we are on the same page @uberbrady
|
Dammit :(
Well another workaround is to set up SAML SSO, enable the app for your it
staff only, and check “require SAML as primary login” and maybe even enable
the `.env` var that forces no local login override.
…On Wed, Feb 2, 2022 at 2:39 PM avielc ***@***.***> wrote:
I'll repeat what you said, to make sure we are on the same page @uberbrady
<https://github.com/uberbrady>
- I've removed the "inactive" flag I added on the ldap settings.
- edited some users and disabled their login to snipeit (active
directory users)
- pressed ldap sync
- All those users are marked that they are able to login.
—
Reply to this email directly, view it on GitHub
<#10563 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAI3325ZCB5W6YILI7GZR3UZGXB7ANCNFSM5M2PPAQQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I'll have to look into it, but it sounds good (didn't mess with SAML much, so it'll probably take some time for me to figure it out) |
I feel like this issue is always haunted. Every time we fix it for one set of users, it breaks something else for another set. It's always tricky to try to accommodate the differences in LDAP, AD, and all of the new cloud-hosted solutions out there. We'll get to the bottom of it for once and for all though. I'd love to put this issue to bed forever. |
or to the grave :) |
Where it gets tricky is that people are absolutely going to ask for some groups of people to be able to login, but others not. Hm. Maybe we set that as a group permission instead of on the user itself.... I'd have to think through the implications of that, but that could be a nice "have your cake and eat it too" solution, on top of fixing the active flag issue. 🤔🧐 One doesn't depend on the other though - the flag fix needs to happen first, but the group thing could be something useful to add for v6. |
Sounds awesome man! I see what you mean with implications here, |
Just an alternative viewpoint here: Fundamentally, if all users are allowed by default, wouldn't the simplest solution be to set the default permission as deny action for everything for standard users, and have a separate group for the IT admins with the relevant permissions needed to do their tasks ? |
@nghia-dang Not everyone uses the system the same way though. Some people DO definitely want their users to be able to login and see what they have assigned to them, some definitely do not. Users being able to login or not in some ways isn’t that important, since an unprivileged user would only be able to see what has been checked out to them and nothing else, but we certainly understand people not wanting them to login at all if that’s what’s require for their setup. To further complicate things, some people don’t use permission groups at all, so this becomes tricky to make sure the solutions we come up with don’t change behavior in ways we’re not expecting for each different use case. |
Okay, so @uberbrady and I have chatted a bit… This is the current plan:
In v6, we will find an LDAP filter or attribute that maps to a permission group (and/or set a default permission group in case users do not have any LDAP attributes for what group they should be in). This will hopefully handle the existing use cases, without breaking anything for v6 users. Hopefully. :D |
@avielc Can you confirm that you actually have an |
Actually, I'm a liar-faced liar. It is looking for whatever field you've set as
But it looks like it's looking for a value specifically of snipe-it/app/Console/Commands/LdapSync.php Line 173 in 8add477
It also seems weird and maybe a bug. We're checking it out now. |
I've got what might be a fix up in a PR right now, but @uberbrady and I are afk at the moment, so we can't test it against our LDAP-enabled test setup right this second. We'll make sure it gets tested first thing tomorrow though. |
Wow, this issue gained a lot of traction overnight (Europe time). This is great news! I'll just pitch in and say that I can confirm the results @avielc is getting, for our environment as well. Now this is not suprising if I understand you correctly and the LDAP Active Flag Field is actually looking for a value of TRUE on the AD user object, where you (we) are supposed to define what field on the AD user object Snipe-IT should look in to find said TRUE value. While I beleive AD has fileds toggling TRUE and FALSE based on the user-state and even some fields called "extension attributes" that could be used to enter the value manually, I don't think either is the way to go. And as you said, the current situation may be a bug. Might I suggest adding users based on AD User Group membership, instead of Base Bind DN? One group for users who should be disabled in Snipe-IT, and another for users who should be enabled in Snipe-IT. Users who are not member of either group will not be synced to Snipe-IT. You could extend this even further in coming versions, by linking AD User Groups to Snipe-IT permission sets. |
Yea
Regarding this big of info, this looks like a great spot to be able and change that default flag from TRUE to FALSE - which if I understand logically means I can simply decide if the sync will disable login by default or enable it. (basically a solution if you can add the ability to choose that in the UI. let me know what you think of this idea too :) |
Okay, now I'm testing the PR, and I had to make a few fixes (which I'll of course push up). Our test AD Server is, obviously, Active Directory. So one seriously janky thing I had to do (and I apologize in advance for this) is jam in a condition to force the code to not take the AD branch - when it does that, the active flag is completely ignored. But this means that if someone did want to use a custom LDAP flag in AD to determine whether or not someone can log in, they can't. I will obviously pull my force condition out before pushing up my changes. We can discuss whether or not we want to do this going forward, of course (and I do do that below). So now, I did my test again - and now all of my AD users can no longer log in, except for syncuser123, who can. So it works! But there is a caveat (there's always a caveat) - Every single customer instance or self-hosted instance I have seen that has this flag set, has it set wrong. If we push this fix out, as-is, it's going to suddenly start respecting that wrongly-set flag, and Github and Discord and every single other place on the planet is going to light up like a Christmas tree, because LDAP users suddently won't be able to log in. So, there are a few proposed fixes to that problem I can think of:
There's at least a slight bright spot here - if we do option 1, we can also make it so that AD people can get a better can-log-in mapping system. We can continue to do the |
My preference is for the first option. (I know we discussed it separately, but I just wanted to make it clear in public as well.) |
And it sounds like you like the idea of letting AD people skip the UAC check by setting an active flag? And another thing we can do here - we can make that flag a little more user-friendly by allowing it to be anything truth-ey? E.g. "true" "TRUE" "1" "active" "bblah blah" - (false-ey things being: blank, "", "0", "false", "0.0") |
Perfect |
One thing we haven't addressed here is 'leaving can-login flag alone' - e.g., if an administrator manually marks a Snipe-IT user account as "cannot-log-in", then does an LDAP sync, the flag will be toggled back. I think we prolly oughtta fix that too, eh @snipe ? Unfortunately, that directly conflicts with our desire to leave the UAC control in-use for AD configs :( |
Balls. Yeah, this is the tricksy part I mentioned earlier. Maybe an additional setting in the AD/LDAP settings page? "Override individual settings with Directory Settings" type of thing? Ugh. |
Not sure I completelyu followed up (read it over email) OR.. or...(creativity strikes as you spit ball) (and then maybe adding similar flags to group permissions |
@avielc I agree that this absolutely this needs an overhaul going forward, but I'm just trying to get this one little thing fixed for our current users right now. |
Understood and completely agree with you here. |
So here's my thinking - the only time I hear from AD users about user-login-ability is when we end up missing one of the UAC settings that we check to enable login. Other than that, I don't hear from them about this issue. So my thinking is we do this:
This means that AD users who want to manually toggle can-log-in are left out in the cold. While this does suck, it's exactly the situation they are in, right now - so effectively, nothing changes for them. @snipe how's that sound? |
I'm missing out here on what |
@avielc It's totally fine, no disrespect taken! But to answer your question, for the solution that I'm proposing here, if you can find any LDAP attribute that you don't care about (I used 'Notes' which translates into LDAP as |
I hate it, but I kinda can't think of a better solution :( |
Yay! Another compromise solution that is terrible, except for every other way of doing the thing in the first place. Whee! |
Thanks for explaining (Though I'm not sure I follow UAC term here still...) If i could just specify specific OU that will be where the users there can login |
I think specifying the OU -> permission group connection would be for v6. Right now, we're just trying to make it unbroken. |
Fixes #10563 - Rework the LDAP sync command to better handle the active flag
I'll have to agree with @avielc here. Setting, maintaining and keeping up with a true/false flag for each user object is a lot of work in large AD environments. Forget to set that flag on a new user, and you have yourself an issue. Copy a user where you forgot to take this flag into consideration, same issue. "Right now, we're just trying to make it unbroken." Excellent, but might I suggest that you make it the way that I and presumably many others (mis)read the documentation (Wiki)? In the Snipe-IT LDAP settings page, have a field/toggle-switch/dropdown-menu that decides the login ability for ALL synced users (Admin users should be local users in Snipe-IT, for now). I really thought this was how the LDAP Active Flag Field in the settings page worked. That if I wrote "active" or left it blank then all synced users would be login enabled. And if I wrote "inactive" in that same field, all synced users would be login disabled. If you have the time/means then yes, you should also create a flag to determine if the synced user login ability has been manually modified. If yes, then ignore whatever the next sync says. Or even better, give us 3 choices in the edit-user page 1:Login-enabled, 2:Login-disabled, 3:Respect LDAP. |
Well said @FG-Lars . |
I had this issue when I setup my SnipeIT instance at the school I work at several years ago. I only wanted all AD users in there for the purpose of assigning assets. Only myself and a select few would be able to login. I ended up writing a quick and dirty python script to poll two AD groups. One for all users, and one for all users who should be able to login to snipe. This is then compared to existing snipe users via the API. Users that don't exist in snipe are created. Those that exist in snipe, but no longer in AD are moved to a different group with a note added to their account stating disabled on x date. Any config changes IMHO should be on the snipeIT end. AD for me is locked down as it is shared as part of the larger school district. I do not have the ability to add/edit attributes for any users, whereas I can create groups galore. Having the ability in Snipe to select whether users are created with login enabled or not, and then a separate admin group setting that would always enable login regardless would be the perfect config options in my eyes. |
Yeah, this solution that I've cobbled together (above) is probably not what we want going forward, and probably wouldn't have helped you much. LDAP is a major pain point for customers, so getting some fixes in there is a high priority for me. Another option of course is if you have an 'unused' LDAP field that you can set only for your administrators (assuming there's a small enough handful of them), then you could at least set that - if you're allowed to simply update a user, which you might not have been. |
Glad to see this thread. Ideally I would just want LDAP to sync/merge users. Then I would go into snipeit to set who could login and assign them to a group with higher privileges' and enable login. If there is a LDAP value to set then great, but by default it should return false if not set. I would also expect that any manually created user that matched a LDAP sync'd user to merge and not create a second user. |
That is already the case. |
I had the same problem. All my users where getting "LOGIN ENABLED" upon LDAP sync, except for 2 of them. I found that the diference on them to the rest was on the AD, in the "userAccountControl" field. Both had the value "524832" (0x80220=( PASSWD_NOTREQD | NORMAL_ACCOUNT... ) instead of the value "512" ( NORMAL_ACCOUNT ). So it might also be a workaround for some people, changing that value on the AD. ps: I also thing that a flag stating "disable login" upon LDAP SYNC would be heaven! :) |
Hello new here, Is there any way to add new users without giving an email or password ? Thanks Amanda |
My situation ... to throw this into the mix ... (I have it working but it isn't as clean as I would wish):
Ideally, "login enabled" wouldn't be shown as true in Snipe-IT, but it works out OK because the LDAP server denies all authentication attempts "LDAP Password sync" is unchecked now, although it was initially set for the first sync. It would be nice to enable a few logins for IT people via LDAP ... but it seems the easiest way (given the way it works) is to set local passwords. These need to be separate accounts (e.g. username_local instead of [email protected]) in SnipeIT so that they don't get overwritten by future LDAP syncs and turned into LDAP users. |
Man, I hate resurrecting old threads, but I cannot find this issue anywhere else... We are new to Snipe-IT and have installed Version v6.3.0 - build 12490 on Windows Server 2019 with IIS. Thanks! |
Debug mode
Describe the bug
If I understand the documentation at https://snipe-it.readme.io/docs/ldap-sync-login correctly, I should be able to import AD users without giving them the ability to login to Snipe-IT using the LDAP Active Flag.
The only example value given to this field in the documentation is "active".
I have tried "inactive" but this had no effect.
Reproduction steps
Expected behavior
LDAP imported users should be:
Login Enabled = No
Screenshots
No response
Snipe-IT Version
5.3.7 build 6587 (gcf14a0222)
Operating System
Debian 11
Web Server
Apache2
PHP Version
7.4.25
Operating System
No response
Browser
No response
Version
No response
Device
No response
Operating System
No response
Browser
No response
Version
No response
Error messages
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: