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

Fetch and buffer all entries from LDAP search #19002

Merged
merged 1 commit into from Dec 8, 2022

Conversation

LKozlowski
Copy link
Contributor

Fixes #17797.

Before it would throw an Size Limit Exceeded error when there were more search results than the search size limit.

Now, it will make as many calls as needed to fetch all the results without throwing an error.

In some cases, if there is a lot of entries, the search might slow down a bit as there will be more requests made to fetch all entries.

lib/auth/windows/ldap.go Outdated Show resolved Hide resolved
lib/auth/windows/ldap.go Outdated Show resolved Hide resolved
@LKozlowski LKozlowski force-pushed the lkozlowski/17797-ldap-size-limit-exceeded branch from 3aebd1a to 13af8ac Compare December 5, 2022 18:03
@LKozlowski LKozlowski requested a review from zmb3 December 5, 2022 18:03
@LKozlowski LKozlowski force-pushed the lkozlowski/17797-ldap-size-limit-exceeded branch from 13af8ac to 82ea6b4 Compare December 5, 2022 18:45
@github-actions github-actions bot removed the request for review from kimlisa December 5, 2022 18:54
@LKozlowski LKozlowski force-pushed the lkozlowski/17797-ldap-size-limit-exceeded branch 3 times, most recently from 476ea01 to 7c65b43 Compare December 7, 2022 13:38
@LKozlowski LKozlowski force-pushed the lkozlowski/17797-ldap-size-limit-exceeded branch from 7c65b43 to 8fde7a5 Compare December 8, 2022 10:44
@LKozlowski LKozlowski merged commit 033ade2 into master Dec 8, 2022
@zmb3 zmb3 deleted the lkozlowski/17797-ldap-size-limit-exceeded branch December 12, 2022 19:52
@zmb3
Copy link
Collaborator

zmb3 commented Dec 15, 2022

@LKozlowski this should be backported to all supported versions (9, 10, and 11)

@LKozlowski
Copy link
Contributor Author

@LKozlowski this should be backported to all supported versions (9, 10, and 11)

Are we going to also backport #18021 to all versions? If so then I'll wait for it as this PR changed a lot of the code & file structure so my changes can't be easily applied by cherry-picking. If not, I'll just backport it manually as my change wasn't that big anyway. Just don't want to make a mess if we're going to backport it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LDAP search failed: Result Code 4 "Size Limit Exceeded"
3 participants