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.

Sign up for GitHub

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

Jump to bottom

fix: concurrent calls not authorized and LDAP timeout #20

Merged
merged 8 commits into from
Sep 21, 2023
Merged

fix: concurrent calls not authorized and LDAP timeout #20

merged 8 commits into from
Sep 21, 2023

Conversation

kpetremann
Copy link
Contributor

@kpetremann kpetremann commented Aug 21, 2023 •
edited
Loading

Fix two LDAP issues:

  • unable to authenticate concurrent requests
  • no reconnect after the TCP/LDAP timed out
  • timeout if the LDAP is not responding to authentication requests

@kpetremann
fix: handle timeout for LDAP requests
12b4b19 
In some cases like network connectivity lost, the LDAP bind request is
waiting indefinetely.

To avoid that, we to a timeout on the LDAP request by closing the
LDAP connection.
SergeShpak
SergeShpak previously requested changes Aug 24, 2023
View reviewed changes
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
cmd/data-aggregation-api/main.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/basic_auth.go Outdated Show resolved Hide resolved
internal/config/settings.go Outdated Show resolved Hide resolved
@kpetremann kpetremann force-pushed the ldap_fix branch 2 times, most recently from 21d35dc to 668cd94 Compare August 24, 2023 15:47
@kpetremann
refactor: rework ldap implementation
7659001
@kpetremann
feat(ldap): configurable max connection lifetime
d78ab1f 
Max connection lifetime is the maximum time a LDAP connection will be
re-used for authentication requests. Default is 1 minute.

This is not a guarantee:
- the LDAP could close the connection
- the TCP connection could be closed (timeout if the max connection
  lifetime is too high, issues etc...)
@kpetremann
refactor(ldap): timeout via context
c8f487b
@kpetremann kpetremann requested review from a team September 21, 2023 06:54
fdomain
fdomain reviewed Sep 21, 2023
View reviewed changes
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
internal/api/auth/ldap.go Outdated Show resolved Hide resolved
@fdomain fdomain self-requested a review September 21, 2023 09:38
@kpetremann kpetremann merged commit b233912 into criteo:main Sep 21, 2023
2 checks passed
@kpetremann kpetremann deleted the ldap_fix branch September 21, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergeShpak SergeShpak SergeShpak left review comments

@fdomain fdomain fdomain approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kpetremann @SergeShpak @fdomain