Re-open control connection after it closes, and add timeouts (#2613)

* Re-open control connection after it closes, and add timeouts

Add much shorter LDAP timeouts to the control connection so users do not
need to wait for minutes to fail to login.

The control connection may close at any time, because TCP.  When it does
re-open it on the *next* connection attempt.  (The first attempted login
after being disconnected may fail.  I consider this a feature because it
will rarely happen, the user will try again to login, and if the attempt
just made is what caused the server to fail then we should give the user
some feedback.)

Fixes #2606.

* Name LDAP timeouts

Not on configuration because of impossibility to set default values on
optional subobjects when using viper.  Defer on this until a real user
requirement for this shows up.

cmd/lakefs/cmd/run.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
 	"os"
@@ -47,6 +48,10 @@ type Shutter interface {
 }
 
 func newLDAPAuthenticator(cfg *config.LDAP, service auth.Service) *auth.LDAPAuthenticator {
+	const (
+		connectionTimeout = 15 * time.Second
+		requestTimeout    = 7 * time.Second
+	)
 	group := cfg.DefaultUserGroup
 	if group == "" {
 		group = auth.ViewersGroup
@@ -58,10 +63,14 @@ func newLDAPAuthenticator(cfg *config.LDAP, service auth.Service) *auth.LDAPAuth
 		DefaultUserGroup:  group,
 		UsernameAttribute: cfg.UsernameAttribute,
 		MakeLDAPConn: func(_ context.Context) (*ldap.Conn, error) {
-			c, err := ldap.DialURL(cfg.ServerEndpoint)
+			c, err := ldap.DialURL(
+				cfg.ServerEndpoint,
+				ldap.DialWithDialer(&net.Dialer{Timeout: connectionTimeout}),
+			)
 			if err != nil {
 				return nil, fmt.Errorf("dial %s: %w", cfg.ServerEndpoint, err)
 			}
+			c.SetTimeout(requestTimeout)
 			// TODO(ariels): Support StartTLS (& other TLS configuration).
 			return c, nil
 		},

pkg/auth/authenticator.go

@@ -5,6 +5,7 @@ import (
 	"crypto/subtle"
 	"errors"
 	"fmt"
+	"os"
 	"strings"
 	"time"
 
@@ -91,12 +92,12 @@ type LDAPAuthenticator struct {
 
 	// control is bound to the operator (BindDN) and is used to query
 	// LDAP about users.
-	// TODO(ariels): Should auto-reopen.
 	control *ldap.Conn
 }
 
 func (la *LDAPAuthenticator) getControlConnection(ctx context.Context) (*ldap.Conn, error) {
-	if la.control != nil {
+	// LDAP connections are "closing" even after they've closed.
+	if la.control != nil && !la.control.IsClosing() {
 		return la.control, nil
 	}
 	control, err := la.MakeLDAPConn(ctx)
@@ -115,6 +116,11 @@ func (la *LDAPAuthenticator) getControlConnection(ctx context.Context) (*ldap.Co
 	return la.control, nil
 }
 
+func (la *LDAPAuthenticator) resetControlConnection() {
+	go la.control.Close() // Don't wait for dead connection to shut itself down
+	la.control = nil
+}
+
 // inBrackets returns filter (which should already be properly escaped)
 // enclosed in brackets if it does not start with open brackets.
 func inBrackets(filter string) string {
@@ -143,6 +149,9 @@ func (la *LDAPAuthenticator) AuthenticateUser(ctx context.Context, username, pas
 	res, err := controlConn.SearchWithPaging(&searchRequest, 2)
 	if err != nil {
 		logger.WithError(err).Error("Failed to search for DN by username")
+		if errors.Is(err, os.ErrDeadlineExceeded) {
+			la.resetControlConnection()
+		}
 		return InvalidUserID, fmt.Errorf("LDAP find user %s: %w", username, err)
 	}
 	if logger.IsTracing() {