From 947c42382428827229395407ec5d4b4988d88938 Mon Sep 17 00:00:00 2001
From: Aditya Manthramurthy <donatello@users.noreply.github.com>
Date: Thu, 4 Nov 2021 13:11:20 -0700
Subject: [PATCH] fix: user DN filtering that causes some unnecessary logs
 (#13584)

Additionally, remove the unnecessary `isUsingLookupBind` field in the LDAP struct
---
 cmd/iam.go                              |  4 ++--
 internal/config/identity/ldap/config.go | 31 ++++---------------------
 2 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/cmd/iam.go b/cmd/iam.go
index 69351c1c9..c61ea9280 100644
--- a/cmd/iam.go
+++ b/cmd/iam.go
@@ -286,7 +286,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc
 				}
 			}
 		}()
-	case globalLDAPConfig.EnabledWithLookupBind():
+	case globalLDAPConfig.Enabled:
 		go func() {
 			ticker := time.NewTicker(sys.iamRefreshInterval)
 			defer ticker.Stop()
@@ -843,7 +843,7 @@ func (sys *IAMSys) purgeExpiredCredentialsForLDAP(ctx context.Context) {
 		allDistNames = append(allDistNames, parentUser)
 	}
 
-	expiredUsers, err := globalLDAPConfig.GetNonEligibleUserDistNames(parentUsers)
+	expiredUsers, err := globalLDAPConfig.GetNonEligibleUserDistNames(allDistNames)
 	if err != nil {
 		// Log and return on error - perhaps it'll work the next time.
 		logger.LogIf(GlobalContext, err)
diff --git a/internal/config/identity/ldap/config.go b/internal/config/identity/ldap/config.go
index 1df6c8f7f..c7409d08e 100644
--- a/internal/config/identity/ldap/config.go
+++ b/internal/config/identity/ldap/config.go
@@ -67,7 +67,6 @@ type Config struct {
 	tlsSkipVerify     bool          // allows skipping TLS verification
 	serverInsecure    bool          // allows plain text connection to LDAP server
 	serverStartTLS    bool          // allows using StartTLS connection to LDAP server
-	isUsingLookupBind bool
 	rootCAs           *x509.CertPool
 }
 
@@ -240,10 +239,6 @@ func (l *Config) searchForUserGroups(conn *ldap.Conn, username, bindDN string) (
 
 // LookupUserDN searches for the full DN and groups of a given username
 func (l *Config) LookupUserDN(username string) (string, []string, error) {
-	if !l.isUsingLookupBind {
-		return "", nil, errors.New("current lookup mode does not support searching for User DN")
-	}
-
 	conn, err := l.Connect()
 	if err != nil {
 		return "", nil, err
@@ -386,10 +381,6 @@ func (l Config) IsLDAPUserDN(user string) bool {
 // 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) {
-	if !l.isUsingLookupBind {
-		return nil, errors.New("current LDAP configuration does not permit looking for expired user accounts")
-	}
-
 	conn, err := l.Connect()
 	if err != nil {
 		return nil, err
@@ -410,7 +401,7 @@ func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string,
 			dn,
 			ldap.ScopeBaseObject, ldap.NeverDerefAliases, 0, 0, false,
 			filter,
-			[]string{}, // only need DN, so no pass no attributes here
+			[]string{}, // only need DN, so pass no attributes here
 			nil,
 		)
 
@@ -435,10 +426,6 @@ func (l *Config) GetNonEligibleUserDistNames(userDistNames []string) ([]string,
 // 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) {
-	if !l.isUsingLookupBind {
-		return nil, errors.New("current LDAP configuration does not permit this lookup")
-	}
-
 	conn, err := l.Connect()
 	if err != nil {
 		return nil, err
@@ -463,12 +450,7 @@ func (l *Config) LookupGroupMemberships(userDistNames []string, userDNToUsername
 	return res, nil
 }
 
-// EnabledWithLookupBind - checks if ldap IDP is enabled in lookup bind mode.
-func (l Config) EnabledWithLookupBind() bool {
-	return l.Enabled && l.isUsingLookupBind
-}
-
-// Enabled returns if jwks is enabled.
+// Enabled returns if LDAP config is enabled.
 func Enabled(kvs config.KVS) bool {
 	return kvs.Get(ServerAddr) != ""
 }
@@ -516,11 +498,13 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 
 	// Lookup bind user configuration
 	lookupBindDN := env.Get(EnvLookupBindDN, kvs.Get(LookupBindDN))
+	if lookupBindDN == "" {
+		return l, errors.New("Lookup Bind DN is required")
+	}
 	lookupBindPassword := env.Get(EnvLookupBindPassword, kvs.Get(LookupBindPassword))
 	if lookupBindDN != "" {
 		l.LookupBindDN = lookupBindDN
 		l.LookupBindPassword = lookupBindPassword
-		l.isUsingLookupBind = true
 
 		// User DN search configuration
 		userDNSearchBaseDN := env.Get(EnvUserDNSearchBaseDN, kvs.Get(UserDNSearchBaseDN))
@@ -532,11 +516,6 @@ func Lookup(kvs config.KVS, rootCAs *x509.CertPool) (l Config, err error) {
 		l.UserDNSearchFilter = userDNSearchFilter
 	}
 
-	// Lookup bind mode is mandatory
-	if !l.isUsingLookupBind {
-		return l, errors.New("Lookup Bind mode is required")
-	}
-
 	// Test connection to LDAP server.
 	if err := l.testConnection(); err != nil {
 		return l, fmt.Errorf("Connection test for LDAP server failed: %w", err)