-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 active status not matching #5054
Comments
Update, If you via FreeIPA explicitly disable an account and re-enable it then ldapsearch -LLL -Y GSSAPI -h ipa.example.com -b cn=users,cn=accounts,dc=example,dc=com "(nsaccountlock=FALSE)" uid now works for that account, in other words this explicitly sets a false value. However despite this the account still syncs in to SnipeIT as disabled. :( Since all accounts keep getting disabled this is a complete road-block issue. :( |
This is technically a duplicate of #4998, but since this issue has more information, I'll see this one open and close the other. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions! |
@snipe This is a comment to keep this live. This issue is still broken in the current version. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions! |
Any likelihood this bug is going to be fixed? Do you need more information? |
It's happening the same to me when I sync users from my LDAP server. All accounts keep getting disabled, also if I enable by hand an account, when it syncs back it overrides this action and disables this user again |
I had to change the LdapSync.php file: My OpenLDAP implementation does not have a flag for active and deactivated accounts, we just move users from OU to one with disabled users. This script is expecting the UserAccountControl flag that is not present in our deployment. I think this is very restrictive to some specific LDAP implementations, maybe should be reviewed. Cheers |
@steineracedo My own LDAP server is FreeIPA and does support an active flag. What is so annoying is that I can use this flag in SnipeIt as a filter for importing user accounts and it correctly only imports active accounts. It then unfortunately marks them all as inactive! I use the LDAP Filter set as - &(cn=*)(!(nsaccountlock=TRUE)) This filters the import to only import active accounts, as might be obvious the LDAP field being used as the active flag is nsaccountlock |
@jelockwood the activated status seems to be modified only by the LDAP Active Flag Above is the piece of code of the LdapSync.php and (I'm not a Laravel or PHP expert) but AFAIK is trying to modify the "activated" status based on the UserAccountControl flag that you don't have in your FreeIPA and neither do I in our OpenLDAP server This implementation seems to be more AD oriented: In conclusion, no matter what filter you use this won't change the activated status @snipe can you please confirm this? |
btw.. in the image above what I did was to change the else condition from activated = 0 to activated = 1.. is not the best approach but it works for me... I'm syncing users with LDAP only to be able to assign assets but the auth actually is through SAML and I assign/remove SnipeIT access to users within our IdP |
I'm using an IBM Domino server as LDAP server. And we don't really us an active flag there. |
@snipe The default behaviour should be to keep the current activation status when the "LDAP Active Flag" is not set. I would like to manually activate some users and manage this state in snipe-it. if the user is no longer present in ldap, then the sync should deactivate the login. Another desirable option would be to allow login to a admin group, but not to all. |
@snipe Is this still an issue? I believe your fix on Aug 27 solved the issue. |
The LDAP import deactivates a lot of our users on every run. We're using MS Active Directory, so there should be some kind of active flag, but I haven't found out how to use it. Any advice welcome! |
@vogtmh The code checks the useraccountcontrol for these values '512', '544', '66048', '66080', '262656', '262688', '328192', '328224', if the user has a value that matches they are active, if not they are inactive. Your inactive users, what useraccountcontrol value do they have? |
@snipe I am now running Version v4.6.7 - build 3944 (master). I have also updated to PHP 7.2.14 and php7.2-ldap (If anything this issue is worse in v4.6.7) |
Would replacing this flag with a filter be a good option maybe? so you'd enter:
or FWIW, we have the same issue with snipe-it disabling accounts, and its blocking a wider roll-out. |
@thinkl33t I do use a similar filter for importing accounts i.e. I only import active accounts however they then get marked inactive. :( |
OK, for clarity, here is how i think this functionality should work:
When a user leaves (or before their start date), they should be disabled to prevent them from logging in etc, but should still exist so their assets can be checked in and history should still exist. snipe-it/app/Console/Commands/LdapSync.php Line 181 in 75032de
Having dug into the code, it appears that in the non-active-directory usecase, snipe-it is using the activated flag to see if the user should be imported, not to enable / disable their account. In my opinion this behaviour is incorrect; If a user shouldn't be imported at all they should be filtered out in the existing LDAP filter, and the contents of the activated / deactivated filter should be used to set the active / inactive flag. |
@thinkl33t I would suggest that if the field in SnipeIT is left empty then SnipeIT should not mark accounts enabled/disabled automatically and instead this would be done manually per account in SnipeIT. The initial process should be that an imported account which is new should be enabled i.e. the default is enabled. |
That would work for us, as 'inactive' users wont be able to authenticate anyway, the LDAP auth will fail. I'd probably suggest a config variable for the 'enable new ldap users by default' bit though? |
I have a similar issue and was wondering if there is any update. Would this be considered a seperate issue? |
@EnvistaGit This has certainly stopped accounts being repeatedly disabled although I do not believe it truly fixes it in actually respecting the behaviour of the LDAP flag. |
Same here. |
Ran into this same issue today. Using openldap and couldn't get anything in the "LDAP Active Flag" to make a difference. We have an attribute for account status that is set to I've applied 5b8cbe2 for the time being since that enables all the users we sync. |
When using none-AD ldap, users are automatically deactivated every LDAP sync. This commit changes the behaviour so that if the active flag isn't set, the users are enabled. Fixed snipe#5054, at least for 4.X
* Fixes #6204 - added email alerts and web/API access to assets due for audits (#6992) * Added upcoming audit report TODO: Fid diff/threshold math * Added route to list overdue / upcoming assets via API * Controller/API methods for due/overdue audits We could probably skip this and just handle it via view in the routes… * Added query scopes for due and overdue audits * Added audit due console command to kernel * Added ability to pass audit specs to main API asset search method * Added audit presenter * Added bootstrap-tables presenter formatter to display an audit button * Added gated sidenav items to left nav * Added audit due/overdue blades * Cleanup on audit due/overdue console command * Added language strings for audit views * Fixed :threshold placeholder * Removed unused setting variable * Fixed next audit date math * Added scope for both overdue and upcoming * Derp. Wrong version * Bumped version (I will release this version officially tomorrow) * Leave the activated state for users alone in normal LDAP synchronisation. (#6988) * Fixed #7003 - crash when warranty months or purchase date is null * Fixed #6956 - viewKeys policy inconsistent (#7009) * Fixed #6956 - Added additional gates show showing/hiding license keys * Modified gate to allow user to see licenses if they can create or edit the license as well * Added API middleware to API routes to enable throttling TODO: Figure out how to make this costumizable without touching the code * Import locations from CSV via command line (#7021) * Added import locations command * Small fixes to location importer * Added country, LDAP OU * Cleaned up comments, added more clarification to what the script does * Added ability to update groups via API Fixes [ch9139] * Bumped version * Fixed #6883 - remove escaping of fields on LDAP import * Fixed #6880 - correctly encrypt encrypted fields via the API * Fixes #5054: LDAP users deactivated for none-ad (#7032) When using none-AD ldap, users are automatically deactivated every LDAP sync. This commit changes the behaviour so that if the active flag isn't set, the users are enabled. Fixed #5054, at least for 4.X * Updated packages - Updating erusev/parsedown (v1.7.2 => 1.7.3): Downloading (100%) - Updating squizlabs/php_codesniffer (3.4.1 => 3.4.2): Downloading (100%) - Updating symfony/polyfill-mbstring (v1.10.0 => v1.11.0): Downloading (100%) - Updating symfony/var-dumper (v3.4.23 => v3.4.27): Downloading (100%) - Updating league/flysystem (1.0.50 => 1.0.51): Downloading (100%) - Updating symfony/translation (v3.4.23 => v3.4.27): Downloading (100%) - Updating nesbot/carbon (1.36.2 => 1.37.1): Downloading (100%) - Updating symfony/debug (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/console (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/finder (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/polyfill-ctype (v1.10.0 => v1.11.0): Downloading (100%) - Updating symfony/polyfill-php70 (v1.10.0 => v1.11.0): Downloading (100%) - Updating symfony/http-foundation (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/event-dispatcher (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/http-kernel (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/process (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/routing (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/polyfill-util (v1.10.0 => v1.11.0): Downloading (100%) - Updating symfony/polyfill-php56 (v1.10.0 => v1.11.0): Downloading (100%) - Updating symfony/psr-http-message-bridge (v1.1.1 => v1.1.2): Downloading (failed) Downloading (100%) - Updating rollbar/rollbar (v1.7.5 => v1.8.1): Downloading (100%) - Updating symfony/yaml (v3.4.23 => v3.4.27): Downloading (100%) - Updating symfony/browser-kit (v3.4.23 => v3.4.27): Downloading (100%) * Fixed #7044 - API update deleted custom fields if they are not re-presented * Fixed XSS vulnerability when creating a new categories, etc via modal on create Same fix as before, because of the weird select2 post-parsing ajax behavior * Updated email strings * Fixed #7046 - added user website url back into UI * Updated language strings * Bumped version * Updated packages * New backups config for spatie * Removed debugbar service provider (autodiscovery) * Use laravel v5.5 withCount manual aliases * Added spatie language files * Removed old laravel backups config This config file was renamed in a newer version of spatie laravel-backup * Set the serialization * Added the command loader to console kernel * Renamed fire() to handle() * Updated withCount to use manual naming * Updated backup path in backup admin * Updated travis with new php versions * Bumped laravel version in readme * Fixed custom field edit screen * Fixed baseUrl is undefined error I literally cannot figure out how this ever worked before. * Fix for included files in backup * Bumped version * Switch has() to filled() * Change ->has() to ->filled() * Removed cosole log * Bumped packages * Use getReader instead of fetchAssoc for CSV parser https://csv.thephpleague.com/9.0/upgrading/ * Handle JSON validation errors like 5.4 * Handle JSON validation errors like 5.4 * Handle JSON validation errors like 5.4 * Trying to fix ajax asset validation This I think gets us closer, but still not handling the validation on the asset properly. When I do a print_r of the validation in the other items, its looking for an error bag that looks something like this: ``` Illuminate\Support\MessageBag Object ( [messages:protected] => Array ( [name] => Array ( [0] => The name field is required. ) [seats] => Array ( [0] => The seats field is required. ) [category_id] => Array ( [0] => The category id field is required. ) ) [format:protected] => :message ) ``` Currently the Assets ajax returns: ``` [2019-05-24 06:52:06] develop.ERROR: array ( 'messages' => array ( 'model_id' => array ( 0 => 'The model id field is required.', ), 'status_id' => array ( 0 => 'The status id field is required.', ), 'asset_tag' => array ( 0 => 'The asset tag field is required.', ), ), ) ``` So not sure why it’s not working. * Fixed missing asset validation * Check that a model exists before trying to fiddle with fieldsets * Tidied up license check * Removed extra escaping on checkin * Updated importer to work with newer CSV Reader::getRecords() method * Fixed field mapping * Small fix for reordering fields Fixes Illuminate\Database\QueryException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'order' cannot be null (SQL: insert into `custom_field_custom_fieldset` (`custom_field_id`, `custom_fieldset_id`, `order`, `required`) values (12, 7, , 0)) [ch1151] This needs revisiting for a more solid fix, especially for data that was already entered bad. * Fixed bug where sorting by company name in Users API did not work Fixes [ch9200] * Removed custom fields from AssignedSearch to prevent confusing data in selectlist Fixes [ch9193] * Removed alert-danger from tests * Fixed missed consumables_count withCount() statement * Fixed Undefined variable user in $backto if checked out to a non-user Fixes [ch9194] * Check for valid model before attempting to access fieldsets Fixes [ch1249] * Only build the log upload destination path if there is a matching record Fixes [ch1232] * Fixed free_seats_count variable name (I forgot that Laravel switched camel case to snake case for their old 5.4 withCount variables) * Only gtry to delete the file if a record is found in the log * Only try to get fieldset if model is valid * Fixed more camel-casing -> snake-casing * Only display the file if the log record can be found * Fixed casing in sync command * Updated README * Derp - typo * Added link to Atlassian plugin * More Atlassian clarifications * Show accessory image on view page * Increased image size to 800px, added lightboxes * Fixed #7083 - Removed user_exists constraint on department save If the user has been deleted, this prevented the department from being successfully saved on edit * Updated branch in version file * Dockerfile update to bring us up to php v7.1 for Laravel 5.5 (#7084) * bump up to php7.1 & change deprecated MAINTAINER to a LABEL so it is visible with `docker inspect` * AND modapache >< * 2 updates required to get software-properties+ppa * Bumped version * Bumped release again :( * Missed one * Fixed #7098 - updated backup config for deleteFile() method * Fixed #7092 - handle weird port forwarding/port numbers for baseUrl * Bumped version * Fixed #7099 - set email to null by default for backup notifications * Removed old comments * Fixed #7100 - Check if $user isset on checkin * Increased throttle to 120 requests per minute * Added Filipino, corrected order for Spanish variations * Update language strings * Bumped hash * Changed has to filled to fix bulk asset editing * Bumped point version * Small fixes for phpleague CSB reader v9 * Improved error checking in locations importer * Fixed #7145 - rename groups table to permissions_group for mysql 8 reserved word compatibility * Reduce minimum group name length to 2 (from 3) eg: IT * Back in time fix FOR #7145 for new installs on MySQL 8+ * Fixed permission insert //TODO Handle this via model * Possible fix for reporting/admin migration back in time * Fixed #7164 - change table name to permission_groups * Fixed LDAP password blanking on save * fixing previous commit's actual wiping of password (#7183) replaced Input::fille('ldap_pword') with _filled_. Should be good to go. #7179 #7169 * Bumped version * Downgrading rollbar for Laravel 5.5 * Spelling Correction (#7206) Fixed Spelling for the word reqrite, to be rewrite. * Fix #6910: Add logic to manipulate the eloquent query. (#7006) * Added company_id to consumables_users table * Added logic to manage when a pivot table doesn't have the column company_id trough a join with users * Remove a migration that tries to fix this problem, but is not longer necessary * Addresses #7238 - add PWA code to layout Needs additional UX testing * Better log message for bad LDAP connection * Fixed #7186 - has vs filled in User’s API blanking out groups if no group_ids are passed * Comment clarification on #7186 * Check for valid seat on hardware view * Added space between footer and custom message * Cap warranty months to three characters Filles rollbar 209 * Cap warranty months to 3 on the frontend blade * Fixed countable() strings on user destroy * Check that the user has assets and that the aset model is valid * Bumped hash * Caps asset warranty to 20 years * Command to fix custom field unicode conversion differences between PHP versions (#7263) * Fixes #7252 form request changes (#7272) * Fixes for #7252 - custom fields not validating / no validaton messages in API w/form requests * Removed debug info * More fixes for #7252 This is mostly working as intended, if not yet the way Laravel wants us to do it. Right now, the API returns correctly, and the form UI will return highlighted errors, with the input filled in ~sometimes~. I’m not sure why it’s only sometimes yet, but this is potentially progress. * Removed experimental method * Check for digits_between:0,240 for warranty * Removed debug code * Apply fix from PR #7273 to master * Bumped hash * Fixed #7250 - permission issue for API fieldsets and fields endpoints This applies the change from #7294 to master * Add @mskrip as a contributor * Fixed #7270 - Checking-in Assets via API Removes the Item's Asset Name * CORS for api (#7292) * Added CORS support to API * Changed order so CORS will still work if throttle hit * Added APP_CORS_ALLOWED_ORIGINS env option * Fixed typo * Clarified header comments * More clarification * DIsable CORS allowed origins by default to replicate existing behavior * Change variable name to be clearer * Bumped version * Added condition to deal with fieldname 'rtd_location' which can be tried to be queried in some places and doesn't exist in database (#7317) * Added comments to the ByFilter query scope for clarity * Added accessories checkout/checkin API endpoint * Fixed CVE-2019-10742 https://nvd.nist.gov/vuln/detail/CVE-2019-10742 * Update README.md (#7334) Add reference to CSV importer. * Group related variables in .env * History importer fixes * Fixes to history importer
Expected Behavior (or desired behavior if a feature request)
I get the impression SnipeIT is supposed to be able to use an LDAP field defined as the 'LDAP Active Flag' to check if an LDAP account is active or not and should then set the status of the matching account in SnipeIT to the same status.
I am using FreeIPA as the LDAP server and I am able to successfully bind to it, to sync accounts from it and to use those to login to SnipeIT. What is not working is that all accounts are synced in to SnipeIT as disabled. I have defined the 'LDAP Active Flag' as being nsAccountLock which is the LDAP field FreeIPA uses for this purpose.
The following command works via the command line to find disabled accounts
ldapsearch -LLL -Y GSSAPI -h ipa.example.com -b cn=users,cn=accounts,dc=example,dc=com "(nsaccountlock=TRUE)" uid
The following command works via the command line to find enabled accounts
ldapsearch -LLL -Y GSSAPI -h ipa.example.com -b cn=users,cn=accounts,dc=examplei,dc=com '(!(nsaccountlock=TRUE))' uid
I have tried entries in the 'LDAP Active Flag' box of
(empty i.e. nothing in the box)
nsAccountLock
!(nsAccountLock=true)
(!(nsAccountLock=tue))
But none of these work. Every time a sync is done all accounts end up being listed in SnipeIT as disabled i.e. not activated.
I can say that whilst disabled accounts in LDAP i.e. in FreeIPA do have a value set for nsAccountLock i.e. nsAccountLock=true the majority of enabled accounts have no value set at all so it is not that they are nsAccountLock=false they are empty values which makes them more like nsAccountLock=
I can define an LDAP filter as &(cn=*)(!(nsaccountlock=TRUE)) and this only imports i.e. syncs active users only but they are still all marked as inactive.
So we need more flexibility over how the LDAP Active Flag is used. It seems it is looking specifically for nsAccountLock=false (or whatever field name you specify).
(what you expect to happen goes here)
I expect to be able to define an LDAP Active Flag that will work and for it to set the matching SnipeIT account status to active or not as appropriate.
Actual Behavior
Accounts always end up marked as inactive, even if you edit an account to mark it active a subsequent LDAP sync change it back to inactive.
(what actually happens goes here)
Please confirm you have done the following before posting your bug report:
Provide answers to these questions:
PHP 7.0.25-0ubuntu0.16.04.1 (cli) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
with Zend OPcache v7.0.25-0ubuntu0.16.04.1, Copyright (c) 1999-2017, by Zend Technologies
mysql Ver 15.1 Distrib 10.1.31-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2
Ubuntu 16.04
Server version: Apache/2.4.18 (Ubuntu)
Server built: 2017-09-18T15:09:02
install.sh
users aka people
storage/logs
and your webserver's logs.No
Please do not post an issue without answering the related questions above. If you have opened a different issue and already answered these questions, answer them again, once for every ticket. It will be next to impossible for us to help you.
https://snipe-it.readme.io/docs/getting-help
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
The text was updated successfully, but these errors were encountered: