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

Added support for multiple LDAP servers in a auth source (go-gitea#6898) #12480

Closed

Conversation

Rob61
Copy link

@Rob61 Rob61 commented Aug 13, 2020

Fixes Feature-Request #6898

Added support for multiple LDAP(S) servers in a auth source.
Servers are separated by space characters.

Before this pull request:
func dial gets a string containing the LDAP server and try to connect.

With this pull request:
func dial gets a string containing the LDAP server or multiple LDAP servers separated by space character.
It splits the string into a list of strings and try to connect to the first, but will try the second, if the first fails, etc.
Leading to the possibility to provide a backup LDAP servers if the main server is not reachable e.g. because of maintaince.

Best
~Rob

Added support for multiple LDAP(S) servers in auth source.
Servers are separated by space characters.
@silverwind
Copy link
Member

silverwind commented Aug 13, 2020

Suggestions:

  • Split on comma instead of space, it's more intuitive I think
  • Any reason not to parallelize the connections? If for example a server fails with a network timeout, all authorizations would need to wait for that timeout (or forever if there's not timeout). I could not find what the default timeout value for that module is on https://godoc.org/gopkg.in/ldap.v3 and it doesn't look we set one either.
  • Docs need to be updated probably (at least a hint in the UI)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 13, 2020
@lunny lunny added the type/enhancement An improvement of existing functionality label Aug 16, 2020
@techknowlogick techknowlogick added this to the 1.14.0 milestone Sep 5, 2020
@stale
Copy link

stale bot commented Nov 9, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Nov 9, 2020
@lunny lunny modified the milestones: 1.14.0, 1.15.0 Jan 27, 2021
@stale stale bot removed the issue/stale label Jan 27, 2021
@lunny
Copy link
Member

lunny commented Jan 27, 2021

We needs some tests for this PR.

@techknowlogick techknowlogick modified the milestones: 1.15.0, 1.16.0 Jun 15, 2021
@techknowlogick techknowlogick modified the milestones: 1.16.0, 1.17.0 Nov 23, 2021
@stale
Copy link

stale bot commented May 1, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label May 1, 2022
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label May 14, 2022
@stale stale bot removed the issue/stale label May 14, 2022
@lunny
Copy link
Member

lunny commented May 14, 2022

Please resolve the conflicts.

@lunny lunny modified the milestones: 1.17.0, 1.18.0 Jun 3, 2022
@lunny lunny removed this from the 1.18.0 milestone Oct 17, 2022
@wxiaoguang
Copy link
Contributor

It has been stale for a long time and I can't think of a way to handling it other than closing it. Feel free to reopen if there's any new progress and I could also help.

@wxiaoguang wxiaoguang closed this May 10, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants