search

snipe / snipe-it

A free open source IT asset/license management system
https://snipeitapp.com
GNU Affero General Public License v3.0
10.86k stars 3.14k forks source link

LdapSync.php - sync process failing and aborting on single data issue #15064

Open maciej-poleszczyk opened 2 months ago

maciej-poleszczyk commented 2 months ago

Debug mode

Describe the bug

If I configure ldap manager field but for some reason any of retrieved managers doesn't have attributed pointed in Username Field ... the whole ldap sync process fails on Undefined array key

https://github.com/snipe/snipe-it/blob/3ae16ff8ca0d5e82c2588c730203bcfc815984f7/app/Console/Commands/LdapSync.php#L316C70-L316C90

Reproduction steps

  1. Configure LDAP including LDAP Manager field
  2. Have in LDAP manager object that would be missing attribute used for Username Field
  3. Run ldap sync

Expected behavior

It might be disputable but sync process shouldn't fail as a whole. The preferred approach might be to catch exception in https://github.com/snipe/snipe-it/blob/3ae16ff8ca0d5e82c2588c730203bcfc815984f7/app/Console/Commands/LdapSync.php#L316C70-L316C90 log warning, continue the sync process.

Screenshots

No response

Snipe-IT Version

6.x

Operating System

Linux

Web Server

n/a

PHP Version

n/a

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

welcome[bot] commented 2 months ago

👋 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.

snipe commented 2 months ago

You should probably consider upgrading - v7 has been out for a bit now and has a ton of fixes in it.

maciej-poleszczyk commented 2 months ago

I checked that part of code (as I did modified it locally) and it didn't change much since 6.x. So it would still fail on single misconfigured manager.