Skip to content
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

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

Open
2 tasks done
maciej-poleszczyk opened this issue Jul 10, 2024 · 3 comments
Open
2 tasks done
Labels

Comments

@maciej-poleszczyk
Copy link
Contributor

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

Copy link

welcome bot commented Jul 10, 2024

👋 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
Copy link
Owner

snipe commented Jul 10, 2024

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

@snipe snipe added the ldap label Jul 10, 2024
@maciej-poleszczyk
Copy link
Contributor Author

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.

maciej-poleszczyk added a commit to maciej-poleszczyk/snipe-it that referenced this issue Sep 25, 2024
maciej-poleszczyk added a commit to maciej-poleszczyk/snipe-it that referenced this issue Sep 25, 2024
snipe added a commit that referenced this issue Nov 14, 2024
Fixing #15064 - to not fail ldap sync on single data issue with ldap …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants