Skip to content

LDAP: refactor to a GenServer

feld requested to merge ldap-tls into develop

The current LDAP authentication flow requires creating a new TCP connection to the LDAP server for every authentication request. As part of that process it also reads the CA certs and parses them for each request.

During additional testing I also discovered that the :eldap module does not supervise its own connections which can leak processes if a network failure happens between the connection opening and simple bind.

Moving the work to a GenServer gives us some benefits:

  • prevents auth spam from directly spawning TCP connections
  • keeps the connection to the LDAP server open for reuse
  • we can capture the {:gen_tcp_error, :closed} error and gracefully reconnect and re-attempt authentication again without the enduser knowing
  • the process/PID leak should be impossible
  • auto reconnect after 5 seconds if the initial connection attempt fails for any reason
  • the CA certs are parsed only once per connection instead of once per login

Additional improvements include detecting configuration problems at startup:

  • if :ldap is enabled, but the Auth is not set to LDAPAuthenticator we log a warning
  • if LDAPAuthenticator is enabled but :ldap is disabled we raise an exception. This will make it impossible to start up Pleroma with a configuration that will never allow authentication to work.
Edited by feld

Merge request reports

Loading