Add extra LDAP configuration validation (#14535)

- The result now contains suggestions on fixing common configuration issues.
- These suggestions will subsequently be exposed in console/mc

docs/sts/ldap.md

@@ -110,7 +110,7 @@ export MINIO_IDENTITY_LDAP_SERVER_ADDR=myldapserver.com:636
 export MINIO_IDENTITY_LDAP_LOOKUP_BIND_DN='cn=admin,dc=min,dc=io'
 export MINIO_IDENTITY_LDAP_LOOKUP_BIND_PASSWORD=admin
 export MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN='ou=hwengg,dc=min,dc=io'
-export MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER='(uid=%s,cn=accounts,dc=min,dc=io)'
+export MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER='(uid=%s)'
 export MINIO_IDENTITY_LDAP_TLS_SKIP_VERIFY=on
 ```
 

internal/config/identity/ldap/config-validator.go

@@ -0,0 +1,271 @@
+// Copyright (c) 2015-2022 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package ldap
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Result - type for high-level names for the validation status of the config.
+type Result string
+
+// Constant values for Result type.
+const (
+	ConfigOk                       Result = "Config OK"
+	ConnectivityError              Result = "LDAP Server Connection Error"
+	LookupBindError                Result = "LDAP Lookup Bind Error"
+	UserSearchParamsMisconfigured  Result = "User Search Parameters Misconfigured"
+	GroupSearchParamsMisconfigured Result = "Group Search Parameters Misconfigured"
+	UserDNLookupError              Result = "User DN Lookup Error"
+	GroupMembershipsLookupError    Result = "Group Memberships Lookup Error"
+)
+
+// Validation returns feedback on the configuration. The `Suggestion` field
+// needs to be "printed" for friendly display (it can contain escaped newlines
+// `\n`).
+type Validation struct {
+	Result     Result
+	Detail     string
+	Suggestion string
+	ErrCause   error
+}
+
+// Error instance for Validation.
+func (v Validation) Error() string {
+	if v.Result == ConfigOk {
+		return ""
+	}
+	return fmt.Sprintf("%s: %s", string(v.Result), v.Detail)
+}
+
+// IsOk - returns if the validation succeeded.
+func (v Validation) IsOk() bool {
+	return v.Result == ConfigOk
+}
+
+// UserLookupResult returns the DN found for the test user and their group
+// memberships.
+type UserLookupResult struct {
+	DN                 string
+	GroupDNMemberships []string
+}
+
+// Validate validates the LDAP configuration. It can be called with any subset
+// of configuration parameters provided by the user - it will return
+// information on what needs to be done to fix the problem if any.
+//
+// This function updates the UserDNSearchBaseDistNames and
+// GroupSearchBaseDistNames fields of the Config - however this an idempotent
+// operation. This is done to support configuration validation in Console/mc and
+// for tests.
+func (l *Config) Validate() Validation {
+	if !l.Enabled {
+		return Validation{Result: ConfigOk, Detail: "Config is not enabled"}
+	}
+
+	if l.ServerAddr == "" {
+		return Validation{
+			Result:     ConnectivityError,
+			Detail:     "Address is empty",
+			Suggestion: "Set a server address.",
+		}
+	}
+
+	conn, err := l.Connect()
+	if err != nil {
+		return Validation{
+			Result:   ConnectivityError,
+			Detail:   fmt.Sprintf("Could not connect to LDAP server: %v", err),
+			ErrCause: err,
+			Suggestion: `Check:
+    (1) server address
+    (2) TLS parameters, and
+    (3) LDAP server's TLS certificate is trusted by MinIO (when using TLS - highly recommended)`,
+		}
+	}
+	defer conn.Close()
+
+	if l.LookupBindDN == "" {
+		return Validation{
+			Result:     LookupBindError,
+			Detail:     "Lookup Bind UserDN not specified",
+			Suggestion: "Specify LDAP service account credentials for performing lookups.",
+		}
+	}
+	if err := l.lookupBind(conn); err != nil {
+		return Validation{
+			Result:     LookupBindError,
+			ErrCause:   err,
+			Detail:     fmt.Sprintf("Error connecting as LDAP Lookup Bind user: %v", err),
+			Suggestion: "Check LDAP Lookup Bind user credentials and if user is allowed to login",
+		}
+	}
+
+	// Validate User Lookup parameters
+	if l.UserDNSearchBaseDistName == "" {
+		return Validation{
+			Result:     UserSearchParamsMisconfigured,
+			Detail:     "UserDN search base is empty",
+			Suggestion: "Set the UserDN search base to the DN of the directory subtree where users are present",
+		}
+	}
+	l.UserDNSearchBaseDistNames = strings.Split(l.UserDNSearchBaseDistName, dnDelimiter)
+
+	if l.UserDNSearchFilter == "" {
+		return Validation{
+			Result: UserSearchParamsMisconfigured,
+			Detail: "UserDN search filter is empty",
+			Suggestion: `Set the UserDN search filter template:
+    Use "%s" - it will be replaced by the login user name and sent to the LDAP server.
+    For example: "(uid=%s)"`,
+		}
+	}
+	if strings.Contains(l.UserDNSearchFilter, "%d") {
+		return Validation{
+			Result: UserSearchParamsMisconfigured,
+			Detail: "User DN search filter contains `%d`",
+			Suggestion: `User DN search filter is a template where "%s" is replaced by the login username.
+    "%d" is not supported here.
+    Please provide a search filter containing "%s"`,
+		}
+	}
+	if !strings.Contains(l.UserDNSearchFilter, "%s") {
+		return Validation{
+			Result: UserSearchParamsMisconfigured,
+			Detail: "User DN search filter does not contain `%s`",
+			Suggestion: `During login, the user's DN is looked up using the search filter template:
+    "%s" gets replaced by the given username - it must be used.
+    Enter an LDAP search filter containing "%s"`,
+		}
+	}
+
+	// If group lookup is not configured, it's ok.
+	if l.GroupSearchBaseDistName != "" || l.GroupSearchFilter != "" {
+
+		// Validate Group Search parameters as they are given.
+		if l.GroupSearchBaseDistName == "" {
+			return Validation{
+				Result: GroupSearchParamsMisconfigured,
+				Detail: "Group Search Base DN is required.",
+				Suggestion: `Since you entered a value for the Group Search Filter - enter a value for the Group Search Base DN too:
+    Enter this value as the DN of the subtree where groups will be found.`,
+			}
+		}
+		l.GroupSearchBaseDistNames = strings.Split(l.GroupSearchBaseDistName, dnDelimiter)
+
+		if l.GroupSearchFilter == "" {
+			return Validation{
+				Result: GroupSearchParamsMisconfigured,
+				Detail: "Group Search Filter is required.",
+				Suggestion: `Since you entered a value for the Group Search Base DN - enter a value for the Group Search Filter too. This is a template where, before the query is sent to the server:
+    "%s" is replaced with the login username;
+    "%d" is replaced with the DN of the login user.
+    For example: "(&(objectclass=groupOfNames)(memberUid=%s))"`,
+			}
+		}
+
+		if !strings.Contains(l.GroupSearchFilter, "%d") && !strings.Contains(l.GroupSearchFilter, "%s") {
+			return Validation{
+				Result: GroupSearchParamsMisconfigured,
+				Detail: `GroupSearchFilter must contain at least one of "%s" or "%d"`,
+				Suggestion: `During group membership lookup the group search filter template is used:
+    "%s" gets replaced by the given username, and
+    "%d" gets replaced by the user's DN.
+    Either one is needed to find only groups that the user is a member of.
+    Enter an LDAP search filter template using at least one of these.`,
+			}
+		}
+	}
+
+	return Validation{
+		Result: ConfigOk,
+	}
+}
+
+// ValidateLookup takes a test username and performs user and group lookup (if
+// configured) and returns the result. It is to validate the LDAP configuration.
+// The lookup is performed without requiring the password for the test user -
+// and so can be used to test any LDAP user intending to use MinIO.
+func (l *Config) ValidateLookup(testUsername string) (*UserLookupResult, Validation) {
+	if testUsername == "" {
+		return nil, Validation{
+			Result: UserDNLookupError,
+			Detail: "Provided username is empty",
+		}
+	}
+
+	if r := l.Validate(); !r.IsOk() {
+		return nil, r
+	}
+
+	conn, err := l.Connect()
+	if err != nil {
+		return nil, Validation{
+			Result:   ConnectivityError,
+			Detail:   fmt.Sprintf("Could not connect to LDAP server: %v", err),
+			ErrCause: err,
+			Suggestion: `Check:
+    (1) server address
+    (2) TLS parameters, and
+    (3) LDAP server's TLS certificate is trusted by MinIO (when using TLS - highly recommended)`,
+		}
+	}
+	defer conn.Close()
+
+	if err := l.lookupBind(conn); err != nil {
+		return nil, Validation{
+			Result:     LookupBindError,
+			ErrCause:   err,
+			Detail:     fmt.Sprintf("Error connecting as LDAP Lookup Bind user: %v", err),
+			Suggestion: "Check LDAP Lookup Bind user credentials and if user is allowed to login",
+		}
+	}
+
+	// Lookup the given username.
+	dn, err := l.lookupUserDN(conn, testUsername)
+	if err != nil {
+		return nil, Validation{
+			Result:   UserDNLookupError,
+			Detail:   fmt.Sprintf("Got an error when looking up user (%s) DN: %v", testUsername, err),
+			ErrCause: err,
+			Suggestion: `Check if this is a temporary error and try again.
+    Perhaps there is an error in the user search filter or user search base DN.`,
+		}
+	}
+
+	// Lookup groups.
+	groups, err := l.searchForUserGroups(conn, testUsername, dn)
+	if err != nil {
+		return nil, Validation{
+			Result:   GroupMembershipsLookupError,
+			Detail:   fmt.Sprintf("Got an error when looking up groups for user(=>%s, dn=>%s): %v", testUsername, dn, err),
+			ErrCause: err,
+			Suggestion: `Check if this is a temporary error and try again.
+    Perhaps there is an error in the group search filter or group search base DN.`,
+		}
+	}
+
+	return &UserLookupResult{
+			DN:                 dn,
+			GroupDNMemberships: groups,
+		}, Validation{
+			Result: ConfigOk,
+			Detail: "User lookup done.",
+		}
+}

internal/config/identity/ldap/config-validator_test.go

@@ -0,0 +1,189 @@
+// Copyright (c) 2015-2022 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package ldap
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/minio/minio-go/v7/pkg/set"
+)
+
+const (
+	EnvTestLDAPServer = "LDAP_TEST_SERVER"
+)
+
+func TestConfigValidator(t *testing.T) {
+	ldapServer := os.Getenv(EnvTestLDAPServer)
+	if ldapServer == "" {
+		t.Skip()
+	}
+	testCases := []struct {
+		cfg            Config
+		expectedResult Result
+	}{
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				return v
+			}(),
+			expectedResult: ConnectivityError,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				return v
+			}(),
+			expectedResult: ConnectivityError,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				return v
+			}(),
+			expectedResult: LookupBindError,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin1"
+				return v
+			}(),
+			expectedResult: LookupBindError,
+		},
+		{ // Case 4
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				return v
+			}(),
+			expectedResult: UserSearchParamsMisconfigured,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				v.UserDNSearchFilter = "(uid=x)"
+				v.UserDNSearchBaseDistName = "dc=min,dc=io"
+				return v
+			}(),
+			expectedResult: UserSearchParamsMisconfigured,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				v.UserDNSearchFilter = "(uid=%s)"
+				v.UserDNSearchBaseDistName = "dc=min,dc=io"
+				return v
+			}(),
+			expectedResult: ConfigOk,
+		},
+		{ // Case 7
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				v.UserDNSearchFilter = "(uid=%s)"
+				v.UserDNSearchBaseDistName = "dc=min,dc=io"
+				v.GroupSearchBaseDistName = "ou=swengg,dc=min,dc=io"
+				v.GroupSearchFilter = "(&(objectclass=groupofnames)(member=x))"
+				return v
+			}(),
+			expectedResult: GroupSearchParamsMisconfigured,
+		},
+		{
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				v.UserDNSearchFilter = "(uid=%s)"
+				v.UserDNSearchBaseDistName = "dc=min,dc=io"
+				v.GroupSearchFilter = "(&(objectclass=groupofnames)(member=x))"
+				return v
+			}(),
+			expectedResult: GroupSearchParamsMisconfigured,
+		},
+		{ // Case 9
+			cfg: func() Config {
+				v := Config{Enabled: true}
+				v.ServerAddr = ldapServer
+				v.serverInsecure = true
+				v.LookupBindDN = "cn=admin,dc=min,dc=io"
+				v.LookupBindPassword = "admin"
+				v.UserDNSearchFilter = "(uid=%s)"
+				v.UserDNSearchBaseDistName = "dc=min,dc=io"
+				v.GroupSearchBaseDistName = "ou=swengg,dc=min,dc=io"
+				v.GroupSearchFilter = "(&(objectclass=groupofnames)(member=%d))"
+				return v
+			}(),
+			expectedResult: ConfigOk,
+		},
+	}
+
+	expectedDN := "uid=dillon,ou=people,ou=swengg,dc=min,dc=io"
+	expectedGroups := set.CreateStringSet(
+		"cn=projecta,ou=groups,ou=swengg,dc=min,dc=io",
+		"cn=projectb,ou=groups,ou=swengg,dc=min,dc=io",
+	)
+
+	for i, test := range testCases {
+		result := test.cfg.Validate()
+		if result.Result != test.expectedResult {
+			fmt.Printf("Result: %#v\n", result)
+			t.Fatalf("Case %d: Got `%s` expected `%s`", i, result.Result, string(test.expectedResult))
+		}
+		if result.IsOk() {
+			lookupResult, validationResult := test.cfg.ValidateLookup("dillon")
+			if !validationResult.IsOk() {
+				t.Fatalf("Case %d: Got unexpected validation failure: %#v\n", i, validationResult)
+			}
+			if lookupResult.DN != expectedDN {
+				t.Fatalf("Case %d: Got unexpected DN: %v", i, lookupResult.DN)
+			}
+
+			if test.cfg.GroupSearchFilter == "" {
+				continue
+			}
+
+			if !set.CreateStringSet(lookupResult.GroupDNMemberships...).Equals(expectedGroups) {
+				t.Fatalf("Case %d: Got unexpected groups: %v", i, lookupResult.GroupDNMemberships)
+			}
+		}
+	}
+}

internal/config/identity/ldap/config.go

@@ -18,18 +18,9 @@
 package ldap
 
 import (
-	"crypto/tls"
 	"crypto/x509"
-	"errors"
-	"fmt"
-	"net"
-	"strconv"
-	"strings"
 	"time"
 
-	ldap "github.com/go-ldap/ldap/v3"
-	"github.com/minio/minio-go/v7/pkg/set"
-	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/config"
 	"github.com/minio/pkg/env"
 )
@@ -52,12 +43,12 @@ type Config struct {
 
 	// User DN search parameters
 	UserDNSearchBaseDistName  string   `json:"userDNSearchBaseDN"`
-	UserDNSearchBaseDistNames []string `json:"-"`
+	UserDNSearchBaseDistNames []string `json:"-"` // Generated field
 	UserDNSearchFilter        string   `json:"userDNSearchFilter"`
 
 	// Group search parameters
 	GroupSearchBaseDistName  string   `json:"groupSearchBaseDN"`
-	GroupSearchBaseDistNames []string `json:"-"`
+	GroupSearchBaseDistNames []string `json:"-"` // Generated field
 	GroupSearchFilter        string   `json:"groupSearchFilter"`
 
 	// Lookup bind LDAP service account
@@ -151,318 +142,6 @@ var (
 	}
 )
 
-func getGroups(conn *ldap.Conn, sreq *ldap.SearchRequest) ([]string, error) {
-	var groups []string
-	sres, err := conn.Search(sreq)
-	if err != nil {
-		// Check if there is no matching result and return empty slice.
-		// Ref: https://ldap.com/ldap-result-code-reference/
-		if ldap.IsErrorWithCode(err, 32) {
-			return nil, nil
-		}
-		return nil, err
-	}
-	for _, entry := range sres.Entries {
-		// We only queried one attribute,
-		// so we only look up the first one.
-		groups = append(groups, entry.DN)
-	}
-	return groups, nil
-}
-
-func (l *Config) lookupBind(conn *ldap.Conn) error {
-	var err error
-	if l.LookupBindPassword == "" {
-		err = conn.UnauthenticatedBind(l.LookupBindDN)
-	} else {
-		err = conn.Bind(l.LookupBindDN, l.LookupBindPassword)
-	}
-	if ldap.IsErrorWithCode(err, 49) {
-		return fmt.Errorf("LDAP Lookup Bind user invalid credentials error: %w", err)
-	}
-	return err
-}
-
-// lookupUserDN searches for the DN of the user given their username. conn is
-// assumed to be using the lookup bind service account. It is required that the
-// search result in at most one result.
-func (l *Config) lookupUserDN(conn *ldap.Conn, username string) (string, error) {
-	filter := strings.ReplaceAll(l.UserDNSearchFilter, "%s", ldap.EscapeFilter(username))
-	var foundDistNames []string
-	for _, userSearchBase := range l.UserDNSearchBaseDistNames {
-		searchRequest := ldap.NewSearchRequest(
-			userSearchBase,
-			ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
-			filter,
-			[]string{}, // only need DN, so no pass no attributes here
-			nil,
-		)
-
-		searchResult, err := conn.Search(searchRequest)
-		if err != nil {
-			return "", err
-		}
-
-		for _, entry := range searchResult.Entries {
-			foundDistNames = append(foundDistNames, entry.DN)
-		}
-	}
-	if len(foundDistNames) == 0 {
-		return "", fmt.Errorf("User DN for %s not found", username)
-	}
-	if len(foundDistNames) != 1 {
-		return "", fmt.Errorf("Multiple DNs for %s found - please fix the search filter", username)
-	}
-	return foundDistNames[0], nil
-}
-
-func (l *Config) searchForUserGroups(conn *ldap.Conn, username, bindDN string) ([]string, error) {
-	// User groups lookup.
-	var groups []string
-	if l.GroupSearchFilter != "" {
-		for _, groupSearchBase := range l.GroupSearchBaseDistNames {
-			filter := strings.ReplaceAll(l.GroupSearchFilter, "%s", ldap.EscapeFilter(username))
-			filter = strings.ReplaceAll(filter, "%d", ldap.EscapeFilter(bindDN))
-			searchRequest := ldap.NewSearchRequest(
-				groupSearchBase,
-				ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
-				filter,
-				nil,
-				nil,
-			)
-
-			var newGroups []string
-			newGroups, err := getGroups(conn, searchRequest)
-			if err != nil {
-				errRet := fmt.Errorf("Error finding groups of %s: %w", bindDN, err)
-				return nil, errRet
-			}
-
-			groups = append(groups, newGroups...)
-		}
-	}
-
-	return groups, nil
-}
-
-// LookupUserDN searches for the full DN and groups of a given username
-func (l *Config) LookupUserDN(username string) (string, []string, error) {
-	conn, err := l.Connect()
-	if err != nil {
-		return "", nil, err
-	}
-	defer conn.Close()
-
-	// Bind to the lookup user account
-	if err = l.lookupBind(conn); err != nil {
-		return "", nil, err
-	}
-
-	// Lookup user DN
-	bindDN, err := l.lookupUserDN(conn, username)
-	if err != nil {
-		errRet := fmt.Errorf("Unable to find user DN: %w", err)
-		return "", nil, errRet
-	}
-
-	groups, err := l.searchForUserGroups(conn, username, bindDN)
-	if err != nil {
-		return "", nil, err
-	}
-
-	return bindDN, groups, nil
-}
-
-// Bind - binds to ldap, searches LDAP and returns the distinguished name of the
-// user and the list of groups.
-func (l *Config) Bind(username, password string) (string, []string, error) {
-	conn, err := l.Connect()
-	if err != nil {
-		return "", nil, err
-	}
-	defer conn.Close()
-
-	var bindDN string
-	// Bind to the lookup user account
-	if err = l.lookupBind(conn); err != nil {
-		return "", nil, err
-	}
-
-	// Lookup user DN
-	bindDN, err = l.lookupUserDN(conn, username)
-	if err != nil {
-		errRet := fmt.Errorf("Unable to find user DN: %w", err)
-		return "", nil, errRet
-	}
-
-	// Authenticate the user credentials.
-	err = conn.Bind(bindDN, password)
-	if err != nil {
-		errRet := fmt.Errorf("LDAP auth failed for DN %s: %w", bindDN, err)
-		return "", nil, errRet
-	}
-
-	// Bind to the lookup user account again to perform group search.
-	if err = l.lookupBind(conn); err != nil {
-		return "", nil, err
-	}
-
-	// User groups lookup.
-	groups, err := l.searchForUserGroups(conn, username, bindDN)
-	if err != nil {
-		return "", nil, err
-	}
-
-	return bindDN, groups, nil
-}
-
-// Connect connect to ldap server.
-func (l *Config) Connect() (ldapConn *ldap.Conn, err error) {
-	if l == nil {
-		return nil, errors.New("LDAP is not configured")
-	}
-
-	_, _, err = net.SplitHostPort(l.ServerAddr)
-	if err != nil {
-		// User default LDAP port if none specified "636"
-		l.ServerAddr = net.JoinHostPort(l.ServerAddr, "636")
-	}
-
-	if l.serverInsecure {
-		return ldap.Dial("tcp", l.ServerAddr)
-	}
-
-	tlsConfig := &tls.Config{
-		InsecureSkipVerify: l.tlsSkipVerify,
-		RootCAs:            l.rootCAs,
-	}
-
-	if l.serverStartTLS {
-		conn, err := ldap.Dial("tcp", l.ServerAddr)
-		if err != nil {
-			return nil, err
-		}
-		err = conn.StartTLS(tlsConfig)
-		return conn, err
-	}
-
-	return ldap.DialTLS("tcp", l.ServerAddr, tlsConfig)
-}
-
-// GetExpiryDuration - return parsed expiry duration.
-func (l Config) GetExpiryDuration(dsecs string) (time.Duration, error) {
-	if dsecs == "" {
-		return l.stsExpiryDuration, nil
-	}
-
-	d, err := strconv.Atoi(dsecs)
-	if err != nil {
-		return 0, auth.ErrInvalidDuration
-	}
-
-	dur := time.Duration(d) * time.Second
-
-	if dur < minLDAPExpiry || dur > maxLDAPExpiry {
-		return 0, auth.ErrInvalidDuration
-	}
-	return dur, nil
-}
-
-func (l Config) testConnection() error {
-	conn, err := l.Connect()
-	if err != nil {
-		return fmt.Errorf("Error creating connection to LDAP server: %w", err)
-	}
-	defer conn.Close()
-
-	if err = l.lookupBind(conn); err != nil {
-		return fmt.Errorf("Error connecting as LDAP Lookup Bind user: %w", err)
-	}
-	return nil
-}
-
-// IsLDAPUserDN determines if the given string could be a user DN from LDAP.
-func (l Config) IsLDAPUserDN(user string) bool {
-	for _, baseDN := range l.UserDNSearchBaseDistNames {
-		if strings.HasSuffix(user, ","+baseDN) {
-			return true
-		}
-	}
-	return false
-}
-
-// GetNonEligibleUserDistNames - find user accounts (DNs) that are no longer
-// present in the LDAP server or do not meet filter criteria anymore
-func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string, error) {
-	conn, err := l.Connect()
-	if err != nil {
-		return nil, err
-	}
-	defer conn.Close()
-
-	// Bind to the lookup user account
-	if err = l.lookupBind(conn); err != nil {
-		return nil, err
-	}
-
-	// Evaluate the filter again with generic wildcard instead of  specific values
-	filter := strings.ReplaceAll(l.UserDNSearchFilter, "%s", "*")
-
-	nonExistentUsers := []string{}
-	for _, dn := range userDistNames {
-		searchRequest := ldap.NewSearchRequest(
-			dn,
-			ldap.ScopeBaseObject, ldap.NeverDerefAliases, 0, 0, false,
-			filter,
-			[]string{}, // only need DN, so pass no attributes here
-			nil,
-		)
-
-		searchResult, err := conn.Search(searchRequest)
-		if err != nil {
-			// Object does not exist error?
-			if ldap.IsErrorWithCode(err, 32) {
-				nonExistentUsers = append(nonExistentUsers, dn)
-				continue
-			}
-			return nil, err
-		}
-		if len(searchResult.Entries) == 0 {
-			// DN was not found - this means this user account is
-			// expired.
-			nonExistentUsers = append(nonExistentUsers, dn)
-		}
-	}
-	return nonExistentUsers, nil
-}
-
-// LookupGroupMemberships - for each DN finds the set of LDAP groups they are a
-// member of.
-func (l *Config) LookupGroupMemberships(userDistNames []string, userDNToUsernameMap map[string]string) (map[string]set.StringSet, error) {
-	conn, err := l.Connect()
-	if err != nil {
-		return nil, err
-	}
-	defer conn.Close()
-
-	// Bind to the lookup user account
-	if err = l.lookupBind(conn); err != nil {
-		return nil, err
-	}
-
-	res := make(map[string]set.StringSet, len(userDistNames))
-	for _, userDistName := range userDistNames {
-		username := userDNToUsernameMap[userDistName]
-		groups, err := l.searchForUserGroups(conn, username, userDistName)
-		if err != nil {
-			return nil, err
-		}
-		res[userDistName] = set.CreateStringSet(groups...)
-	}
-
-	return res, nil
-}
-
 // Enabled returns if LDAP config is enabled.
 func Enabled(kvs config.KVS) bool {
 	return kvs.Get(ServerAddr) != ""
@@ -480,6 +159,7 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 	if err = config.CheckValidKeys(config.IdentityLDAPSubSys, kvs, DefaultKVS); err != nil {
 		return l, err
 	}
+
 	ldapServer := env.Get(EnvServerAddr, kvs.Get(ServerAddr))
 	if ldapServer == "" {
 		return l, nil
@@ -510,44 +190,21 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 	}
 
 	// Lookup bind user configuration
-	lookupBindDN := env.Get(EnvLookupBindDN, kvs.Get(LookupBindDN))
+	l.LookupBindDN = env.Get(EnvLookupBindDN, kvs.Get(LookupBindDN))
-	if lookupBindDN == "" {
+	l.LookupBindPassword = env.Get(EnvLookupBindPassword, kvs.Get(LookupBindPassword))
-		return l, errors.New("Lookup Bind DN is required")
-	}
-	lookupBindPassword := env.Get(EnvLookupBindPassword, kvs.Get(LookupBindPassword))
-	if lookupBindDN != "" {
-		l.LookupBindDN = lookupBindDN
-		l.LookupBindPassword = lookupBindPassword
-	}
-
-	// Test connection to LDAP server.
-	if err := l.testConnection(); err != nil {
-		return l, fmt.Errorf("Connection test for LDAP server failed: %w", err)
-	}
 
 	// User DN search configuration
-	userDNSearchBaseDN := env.Get(EnvUserDNSearchBaseDN, kvs.Get(UserDNSearchBaseDN))
+	l.UserDNSearchFilter = env.Get(EnvUserDNSearchFilter, kvs.Get(UserDNSearchFilter))
-	userDNSearchFilter := env.Get(EnvUserDNSearchFilter, kvs.Get(UserDNSearchFilter))
+	l.UserDNSearchBaseDistName = env.Get(EnvUserDNSearchBaseDN, kvs.Get(UserDNSearchBaseDN))
-	if userDNSearchFilter == "" || userDNSearchBaseDN == "" {
-		return l, errors.New("UserDN search base DN and UserDN search filter are both required")
-	}
-	l.UserDNSearchBaseDistName = userDNSearchBaseDN
-	l.UserDNSearchBaseDistNames = strings.Split(userDNSearchBaseDN, dnDelimiter)
-	l.UserDNSearchFilter = userDNSearchFilter
 
 	// Group search params configuration
-	grpSearchFilter := env.Get(EnvGroupSearchFilter, kvs.Get(GroupSearchFilter))
+	l.GroupSearchFilter = env.Get(EnvGroupSearchFilter, kvs.Get(GroupSearchFilter))
-	grpSearchBaseDN := env.Get(EnvGroupSearchBaseDN, kvs.Get(GroupSearchBaseDN))
+	l.GroupSearchBaseDistName = env.Get(EnvGroupSearchBaseDN, kvs.Get(GroupSearchBaseDN))
 
-	// Either all group params must be set or none must be set.
+	// Validate and test configuration.
-	if (grpSearchFilter != "" && grpSearchBaseDN == "") || (grpSearchFilter == "" && grpSearchBaseDN != "") {
+	valResult := l.Validate()
-		return l, errors.New("All group related parameters must be set")
+	if !valResult.IsOk() {
-	}
+		return l, valResult
-
-	if grpSearchFilter != "" {
-		l.GroupSearchFilter = grpSearchFilter
-		l.GroupSearchBaseDistName = grpSearchBaseDN
-		l.GroupSearchBaseDistNames = strings.Split(l.GroupSearchBaseDistName, dnDelimiter)
 	}
 
 	return l, nil

internal/config/identity/ldap/ldap.go

@@ -0,0 +1,331 @@
+// Copyright (c) 2015-2022 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package ldap
+
+import (
+	"crypto/tls"
+	"errors"
+	"fmt"
+	"net"
+	"strconv"
+	"strings"
+	"time"
+
+	ldap "github.com/go-ldap/ldap/v3"
+	"github.com/minio/minio-go/v7/pkg/set"
+	"github.com/minio/minio/internal/auth"
+)
+
+func getGroups(conn *ldap.Conn, sreq *ldap.SearchRequest) ([]string, error) {
+	var groups []string
+	sres, err := conn.Search(sreq)
+	if err != nil {
+		// Check if there is no matching result and return empty slice.
+		// Ref: https://ldap.com/ldap-result-code-reference/
+		if ldap.IsErrorWithCode(err, 32) {
+			return nil, nil
+		}
+		return nil, err
+	}
+	for _, entry := range sres.Entries {
+		// We only queried one attribute,
+		// so we only look up the first one.
+		groups = append(groups, entry.DN)
+	}
+	return groups, nil
+}
+
+func (l *Config) lookupBind(conn *ldap.Conn) error {
+	var err error
+	if l.LookupBindPassword == "" {
+		err = conn.UnauthenticatedBind(l.LookupBindDN)
+	} else {
+		err = conn.Bind(l.LookupBindDN, l.LookupBindPassword)
+	}
+	if ldap.IsErrorWithCode(err, 49) {
+		return fmt.Errorf("LDAP Lookup Bind user invalid credentials error: %w", err)
+	}
+	return err
+}
+
+// lookupUserDN searches for the DN of the user given their username. conn is
+// assumed to be using the lookup bind service account. It is required that the
+// search result in at most one result.
+func (l *Config) lookupUserDN(conn *ldap.Conn, username string) (string, error) {
+	filter := strings.ReplaceAll(l.UserDNSearchFilter, "%s", ldap.EscapeFilter(username))
+	var foundDistNames []string
+	for _, userSearchBase := range l.UserDNSearchBaseDistNames {
+		searchRequest := ldap.NewSearchRequest(
+			userSearchBase,
+			ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
+			filter,
+			[]string{}, // only need DN, so no pass no attributes here
+			nil,
+		)
+
+		searchResult, err := conn.Search(searchRequest)
+		if err != nil {
+			return "", err
+		}
+
+		for _, entry := range searchResult.Entries {
+			foundDistNames = append(foundDistNames, entry.DN)
+		}
+	}
+	if len(foundDistNames) == 0 {
+		return "", fmt.Errorf("User DN for %s not found", username)
+	}
+	if len(foundDistNames) != 1 {
+		return "", fmt.Errorf("Multiple DNs for %s found - please fix the search filter", username)
+	}
+	return foundDistNames[0], nil
+}
+
+func (l *Config) searchForUserGroups(conn *ldap.Conn, username, bindDN string) ([]string, error) {
+	// User groups lookup.
+	var groups []string
+	if l.GroupSearchFilter != "" {
+		for _, groupSearchBase := range l.GroupSearchBaseDistNames {
+			filter := strings.ReplaceAll(l.GroupSearchFilter, "%s", ldap.EscapeFilter(username))
+			filter = strings.ReplaceAll(filter, "%d", ldap.EscapeFilter(bindDN))
+			searchRequest := ldap.NewSearchRequest(
+				groupSearchBase,
+				ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
+				filter,
+				nil,
+				nil,
+			)
+
+			var newGroups []string
+			newGroups, err := getGroups(conn, searchRequest)
+			if err != nil {
+				errRet := fmt.Errorf("Error finding groups of %s: %w", bindDN, err)
+				return nil, errRet
+			}
+
+			groups = append(groups, newGroups...)
+		}
+	}
+
+	return groups, nil
+}
+
+// LookupUserDN searches for the full DN and groups of a given username
+func (l *Config) LookupUserDN(username string) (string, []string, error) {
+	conn, err := l.Connect()
+	if err != nil {
+		return "", nil, err
+	}
+	defer conn.Close()
+
+	// Bind to the lookup user account
+	if err = l.lookupBind(conn); err != nil {
+		return "", nil, err
+	}
+
+	// Lookup user DN
+	bindDN, err := l.lookupUserDN(conn, username)
+	if err != nil {
+		errRet := fmt.Errorf("Unable to find user DN: %w", err)
+		return "", nil, errRet
+	}
+
+	groups, err := l.searchForUserGroups(conn, username, bindDN)
+	if err != nil {
+		return "", nil, err
+	}
+
+	return bindDN, groups, nil
+}
+
+// Bind - binds to ldap, searches LDAP and returns the distinguished name of the
+// user and the list of groups.
+func (l *Config) Bind(username, password string) (string, []string, error) {
+	conn, err := l.Connect()
+	if err != nil {
+		return "", nil, err
+	}
+	defer conn.Close()
+
+	var bindDN string
+	// Bind to the lookup user account
+	if err = l.lookupBind(conn); err != nil {
+		return "", nil, err
+	}
+
+	// Lookup user DN
+	bindDN, err = l.lookupUserDN(conn, username)
+	if err != nil {
+		errRet := fmt.Errorf("Unable to find user DN: %w", err)
+		return "", nil, errRet
+	}
+
+	// Authenticate the user credentials.
+	err = conn.Bind(bindDN, password)
+	if err != nil {
+		errRet := fmt.Errorf("LDAP auth failed for DN %s: %w", bindDN, err)
+		return "", nil, errRet
+	}
+
+	// Bind to the lookup user account again to perform group search.
+	if err = l.lookupBind(conn); err != nil {
+		return "", nil, err
+	}
+
+	// User groups lookup.
+	groups, err := l.searchForUserGroups(conn, username, bindDN)
+	if err != nil {
+		return "", nil, err
+	}
+
+	return bindDN, groups, nil
+}
+
+// Connect connect to ldap server.
+func (l *Config) Connect() (ldapConn *ldap.Conn, err error) {
+	if l == nil {
+		return nil, errors.New("LDAP is not configured")
+	}
+
+	_, _, err = net.SplitHostPort(l.ServerAddr)
+	if err != nil {
+		// User default LDAP port if none specified "636"
+		l.ServerAddr = net.JoinHostPort(l.ServerAddr, "636")
+	}
+
+	if l.serverInsecure {
+		return ldap.Dial("tcp", l.ServerAddr)
+	}
+
+	tlsConfig := &tls.Config{
+		InsecureSkipVerify: l.tlsSkipVerify,
+		RootCAs:            l.rootCAs,
+	}
+
+	if l.serverStartTLS {
+		conn, err := ldap.Dial("tcp", l.ServerAddr)
+		if err != nil {
+			return nil, err
+		}
+		err = conn.StartTLS(tlsConfig)
+		return conn, err
+	}
+
+	return ldap.DialTLS("tcp", l.ServerAddr, tlsConfig)
+}
+
+// GetExpiryDuration - return parsed expiry duration.
+func (l Config) GetExpiryDuration(dsecs string) (time.Duration, error) {
+	if dsecs == "" {
+		return l.stsExpiryDuration, nil
+	}
+
+	d, err := strconv.Atoi(dsecs)
+	if err != nil {
+		return 0, auth.ErrInvalidDuration
+	}
+
+	dur := time.Duration(d) * time.Second
+
+	if dur < minLDAPExpiry || dur > maxLDAPExpiry {
+		return 0, auth.ErrInvalidDuration
+	}
+	return dur, nil
+}
+
+// IsLDAPUserDN determines if the given string could be a user DN from LDAP.
+func (l Config) IsLDAPUserDN(user string) bool {
+	for _, baseDN := range l.UserDNSearchBaseDistNames {
+		if strings.HasSuffix(user, ","+baseDN) {
+			return true
+		}
+	}
+	return false
+}
+
+// GetNonEligibleUserDistNames - find user accounts (DNs) that are no longer
+// present in the LDAP server or do not meet filter criteria anymore
+func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string, error) {
+	conn, err := l.Connect()
+	if err != nil {
+		return nil, err
+	}
+	defer conn.Close()
+
+	// Bind to the lookup user account
+	if err = l.lookupBind(conn); err != nil {
+		return nil, err
+	}
+
+	// Evaluate the filter again with generic wildcard instead of  specific values
+	filter := strings.ReplaceAll(l.UserDNSearchFilter, "%s", "*")
+
+	nonExistentUsers := []string{}
+	for _, dn := range userDistNames {
+		searchRequest := ldap.NewSearchRequest(
+			dn,
+			ldap.ScopeBaseObject, ldap.NeverDerefAliases, 0, 0, false,
+			filter,
+			[]string{}, // only need DN, so pass no attributes here
+			nil,
+		)
+
+		searchResult, err := conn.Search(searchRequest)
+		if err != nil {
+			// Object does not exist error?
+			if ldap.IsErrorWithCode(err, 32) {
+				nonExistentUsers = append(nonExistentUsers, dn)
+				continue
+			}
+			return nil, err
+		}
+		if len(searchResult.Entries) == 0 {
+			// DN was not found - this means this user account is
+			// expired.
+			nonExistentUsers = append(nonExistentUsers, dn)
+		}
+	}
+	return nonExistentUsers, nil
+}
+
+// LookupGroupMemberships - for each DN finds the set of LDAP groups they are a
+// member of.
+func (l *Config) LookupGroupMemberships(userDistNames []string, userDNToUsernameMap map[string]string) (map[string]set.StringSet, error) {
+	conn, err := l.Connect()
+	if err != nil {
+		return nil, err
+	}
+	defer conn.Close()
+
+	// Bind to the lookup user account
+	if err = l.lookupBind(conn); err != nil {
+		return nil, err
+	}
+
+	res := make(map[string]set.StringSet, len(userDistNames))
+	for _, userDistName := range userDistNames {
+		username := userDNToUsernameMap[userDistName]
+		groups, err := l.searchForUserGroups(conn, username, userDistName)
+		if err != nil {
+			return nil, err
+		}
+		res[userDistName] = set.CreateStringSet(groups...)
+	}
+
+	return res, nil
+}