gitea icon indicating copy to clipboard operation
gitea copied to clipboard

Support multiple LDAP servers in a auth source

Open silverwind opened this issue 6 years ago • 13 comments
trafficstars

I have a LDAP auth source which has multiple redundant servers, I think it would be useful if a LDAP auth source would allow to specify more than one server here:

Maybe accept a comma-separated list. Servers should be tried in the order they are defined, or possibly randomly to even the load. All servers should be tried and auth should only fail if it fails on all servers.

silverwind avatar May 10 '19 09:05 silverwind

This issue 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 weeks. Thank you for your contributions.

stale[bot] avatar Jul 09 '19 10:07 stale[bot]

Still want to do this, just haven't gotten around to it yet.

silverwind avatar Jul 09 '19 15:07 silverwind

Hello,

Any update about this PR? That is a feature I am looking for. In resilient environments, having at least two LDAP servers is common (in case one server is not working properly for example), so I would like to put 2 URLs for my LDAP source.

I see content of modules/auth/ldap/ldap.go file was moved to services/auth/source/ldap/source_search.go, so this PR must be updated.

I would also be very interested in this feature in order to support higher availability.

palto42 avatar May 27 '24 11:05 palto42

4 years after, there's still no changes for an issue related to security (availability is a part of security for me) ???? Are you sure you are right on your "What is Gitea?" page mention :

"Gitea places a strong emphasis on security, offering features such as user permission management, access control lists, and more to ensure the security of code and data."

I think this issue would have a better priority to be resolved in less than 4 years, no ? As many others, I would also be very interested in this feature.

Thank you !

luCL21 avatar Jul 16 '24 14:07 luCL21

@luCL21 We merge >400 PRs a month, and have thousands of support requests a month through forum, issues reports, emails, chat messages, and more. So while we wish to get to all of the feature requests sometimes it takes longer to get to everything. Part of our long-term strategic roadmap includes improving the high-availability of the project, and this would certainly fit in there.

That being said, we always welcome non-maintainer contributions, so I'll put up a bounty to perhaps usher the progress on this ticket along.

/bounty $200

techknowlogick avatar Jul 16 '24 15:07 techknowlogick

@techknowlogick Can i get this assigned?

abhishek818 avatar Jul 16 '24 15:07 abhishek818

@abhishek818 yup. If you wish to attempt it for the bounty please ensure you follow the steps from algora posted above.

techknowlogick avatar Jul 16 '24 15:07 techknowlogick

/attempt #6898

Algora profile Completed bounties Tech Active attempts Options
@abhishek818 13 bounties from 6 projects
JavaScript, TypeScript
Cancel attempt

abhishek818 avatar Jul 16 '24 15:07 abhishek818

💡 @abhishek818 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

algora-pbc[bot] avatar Jul 18 '24 07:07 algora-pbc[bot]

Here are some steps and pointers to help you get started on resolving this issue:

  1. Modify the Source struct:

    • Update the Host field to accept a comma-separated list of servers.
    • Add a method to parse and return the list of servers.
  2. Update the Authenticate method:

    • Modify the Authenticate method to iterate over the list of servers and attempt authentication with each one until a successful authentication or all servers fail.
  3. Update the configuration parsing:

    • Ensure that the configuration parsing logic can handle the new format for the Host field.

Step-by-Step Implementation

1. Modify the Source struct

In source.go, update the Source struct and add a method to parse the list of servers:

type Source struct {
    Name                  string // canonical name (ie. corporate.ad)
    Hosts                 string // Comma-separated list of LDAP hosts
    Port                  int    // port number
    // ... other fields ...
}

// GetHosts returns the list of LDAP hosts.
func (source *Source) GetHosts() []string {
    return strings.Split(source.Hosts, ",")
}

2. Update the Authenticate method

In source_authenticate.go, update the Authenticate method to try each server in the list:

func (source *Source) Authenticate(ctx context.Context, user *user_model.User, userName, password string) (*user_model.User, error) {
    loginName := userName
    if user != nil {
        loginName = user.LoginName
    }

    var lastErr error
    for _, host := range source.GetHosts() {
        source.Host = host
        sr := source.SearchEntry(loginName, password, source.authSource.Type == auth.DLDAP)
        if sr != nil {
            // Successful authentication
            // ... existing logic ...
            return user, nil
        }
        lastErr = user_model.ErrUserNotExist{Name: loginName}
    }

    // All servers failed
    return nil, lastErr
}

3. Update the configuration parsing

In admin_auth_ldap.go, ensure that the Hosts field is correctly parsed:

func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
    if c.IsSet("name") {
        config.Name = c.String("name")
    }
    if c.IsSet("host") {
        config.Hosts = c.String("host")
    }
    // ... other fields ...
    return nil
}

Potential Implications

  1. Security: Ensure that the connection to each LDAP server is secure, especially if using different security protocols for different servers.
  2. Stability: The new logic should handle cases where some servers are down or unreachable without causing significant delays or failures in the authentication process.
  3. Potential Bugs: Thoroughly test the new feature to ensure that it correctly falls back to the next server in case of failure and that it does not introduce any regressions in the existing authentication logic.

Relevant Files

algora-pbc[bot] avatar Jul 19 '24 12:07 algora-pbc[bot]

The "steps" proposed by algora-pbc seem to be easy but not quite right.

https://github.com/go-gitea/gitea/pull/31649#issuecomment-2505555133

It needs to completely refactor the existing LDAP config related code, the key points (in my mind) could be:

  1. use full LDAP URLs for servers, eg: ldap://server:port/, ldaps://server:port/, ldap+starttls://server:port/
  2. keep backwards compatibility (eg: avoid changing unnecessary CLI arguments, could re-use/migrate existing LDAP server configs in database)

wxiaoguang avatar Nov 28 '24 08:11 wxiaoguang

Given that this issue is far more complicated than we initially expected, it is not feasible to address it through the bounty program. Consequently, we have decided to remove it from the bounty list.

lunny avatar Dec 12 '24 00:12 lunny