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

Fixed default ldap version if not specified #11264

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

snipe
Copy link
Owner

@snipe snipe commented Jun 7, 2022

This PR should hopefully solve some of the LDAP configs that some users have been having issues with. This should prevent the ldap_search(): Search: Partial results and referral received error that could occur if the LDAP version field was left blank in the LDAP settings.

Previous versions of Snipe-IT automatically assumed v3 of LDAP (because in 9 years of running this project I have never seen any other version used), but somehow when we had to forklift the old LDAP functionality from v4 to un-do the AdLdap package stuff, that default got dropped in the shuffle.

Next iteration we will likely remove that field in the LDAP settings screen altogether, since LDAP v2 has been EOL forever and LDAP v4 is unlikely. It's pretty much always going to be v3.

snipe added 2 commits June 6, 2022 20:57
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
@snipe snipe requested a review from uberbrady June 7, 2022 04:09
@snipe snipe added the ldap label Jun 7, 2022
Copy link
Collaborator

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great and simple! Let me test it on my local setup then I can fully sign off on it.

@snipe
Copy link
Owner Author

snipe commented Jun 7, 2022

Awesome, thanks @uberbrady - I'd love to sneak this into v6.0.3 if we can.

@snipe snipe merged commit 0153a37 into develop Jun 14, 2022
@snipe snipe deleted the fixes/set_default_ldap_version branch June 22, 2022 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants