fix: fix incorrect LDAP sync status (#1859)

controllers/ldap.go

@@ -86,7 +86,10 @@ func (c *ApiController) GetLdapUsers() {
 			Phone:       util.GetMaxLenStr(user.TelephoneNumber, user.Mobile, user.MobileTelephoneNumber),
 			Address:     util.GetMaxLenStr(user.RegisteredAddress, user.PostalAddress),
 		})
-		uuids = append(uuids, user.Uuid)
+
+		if user.Uuid != "" {
+			uuids = append(uuids, user.Uuid)
+		}
 	}
 
 	existUuids := object.GetExistUuids(ldapServer.Owner, uuids)
@@ -215,10 +218,10 @@ func (c *ApiController) SyncLdapUsers() {
 
 	object.UpdateLdapSyncTime(ldapId)
 
-	exist, failed := object.SyncLdapUsers(owner, users, ldapId)
+	exist, failed, _ := object.SyncLdapUsers(owner, users, ldapId)
 
 	c.ResponseOk(&LdapSyncResp{
-		Exist:  *exist,
+		Exist:  exist,
-		Failed: *failed,
+		Failed: failed,
 	})
 }

object/ldap_autosync.go

@@ -87,11 +87,13 @@ func (l *LdapAutoSynchronizer) syncRoutine(ldap *Ldap, stopChan chan struct{}) {
 			logs.Warning(fmt.Sprintf("autoSync failed for %s, error %s", ldap.Id, err))
 			continue
 		}
-		existed, failed := SyncLdapUsers(ldap.Owner, LdapUsersToLdapRespUsers(users), ldap.Id)
+
-		if len(*failed) != 0 {
+		existed, failed, err := SyncLdapUsers(ldap.Owner, LdapUsersToLdapRespUsers(users), ldap.Id)
-			logs.Warning(fmt.Sprintf("ldap autosync,%d new users,but %d user failed during :", len(users)-len(*existed)-len(*failed), len(*failed)), *failed)
+		if len(failed) != 0 {
+			logs.Warning(fmt.Sprintf("ldap autosync,%d new users,but %d user failed during :", len(users)-len(existed)-len(failed), len(failed)), failed)
+			logs.Warning(err.Error())
 		} else {
-			logs.Info(fmt.Sprintf("ldap autosync success, %d new users, %d existing users", len(users)-len(*existed), len(*existed)))
+			logs.Info(fmt.Sprintf("ldap autosync success, %d new users, %d existing users", len(users)-len(existed), len(existed)))
 		}
 	}
 }

object/ldap_conn.go

@@ -259,17 +259,13 @@ func LdapUsersToLdapRespUsers(users []ldapUser) []LdapRespUser {
 	return res
 }
 
-func SyncLdapUsers(owner string, respUsers []LdapRespUser, ldapId string) (*[]LdapRespUser, *[]LdapRespUser) {
+func SyncLdapUsers(owner string, syncUsers []LdapRespUser, ldapId string) (existUsers []LdapRespUser, failedUsers []LdapRespUser, err error) {
-	var existUsers []LdapRespUser
-	var failedUsers []LdapRespUser
 	var uuids []string
 
-	for _, user := range respUsers {
+	for _, user := range syncUsers {
 		uuids = append(uuids, user.Uuid)
 	}
 
-	existUuids := GetExistUuids(owner, uuids)
-
 	organization := getOrganization("admin", owner)
 	ldap := GetLdap(ldapId)
 
@@ -289,12 +285,19 @@ func SyncLdapUsers(owner string, respUsers []LdapRespUser, ldapId string) (*[]Ld
 	}
 	tag := strings.Join(ou, ".")
 
-	for _, respUser := range respUsers {
+	for _, syncUser := range syncUsers {
+		if syncUser.Uuid == "" {
+			failedUsers = append(failedUsers, syncUser)
+			err = errors.New("uuid of user being synced is empty")
+			continue
+		}
+
+		existUuids := GetExistUuids(owner, uuids)
 		found := false
 		if len(existUuids) > 0 {
 			for _, existUuid := range existUuids {
-				if respUser.Uuid == existUuid {
+				if syncUser.Uuid == existUuid {
-					existUsers = append(existUsers, respUser)
+					existUsers = append(existUsers, syncUser)
 					found = true
 				}
 			}
@@ -303,49 +306,39 @@ func SyncLdapUsers(owner string, respUsers []LdapRespUser, ldapId string) (*[]Ld
 		if !found {
 			newUser := &User{
 				Owner:       owner,
-				Name:        respUser.buildLdapUserName(),
+				Name:        syncUser.buildLdapUserName(),
 				CreatedTime: util.GetCurrentTime(),
-				DisplayName: respUser.buildLdapDisplayName(),
+				DisplayName: syncUser.buildLdapDisplayName(),
 				Avatar:      organization.DefaultAvatar,
-				Email:       respUser.Email,
+				Email:       syncUser.Email,
-				Phone:       respUser.Phone,
+				Phone:       syncUser.Phone,
-				Address:     []string{respUser.Address},
+				Address:     []string{syncUser.Address},
 				Affiliation: affiliation,
 				Tag:         tag,
 				Score:       beego.AppConfig.DefaultInt("initScore", 2000),
-				Ldap:        respUser.Uuid,
+				Ldap:        syncUser.Uuid,
 			}
 
 			affected := AddUser(newUser)
 			if !affected {
-				failedUsers = append(failedUsers, respUser)
+				failedUsers = append(failedUsers, syncUser)
 				continue
 			}
 		}
 	}
 
-	return &existUsers, &failedUsers
+	return existUsers, failedUsers, err
 }
 
 func GetExistUuids(owner string, uuids []string) []string {
-	var users []User
 	var existUuids []string
-	existUuidSet := make(map[string]struct{})
 
-	err := adapter.Engine.Where(fmt.Sprintf("ldap IN (%s) AND owner = ?", "'"+strings.Join(uuids, "','")+"'"), owner).Find(&users)
+	err := adapter.Engine.Table("user").Where("owner = ?", owner).Cols("ldap").
+		In("ldap", uuids).Select("DISTINCT ldap").Find(&existUuids)
 	if err != nil {
 		panic(err)
 	}
 
-	if len(users) > 0 {
-		for _, result := range users {
-			existUuidSet[result.Ldap] = struct{}{}
-		}
-	}
-
-	for uuid := range existUuidSet {
-		existUuids = append(existUuids, uuid)
-	}
 	return existUuids
 }
 

web/src/LdapSyncPage.js

@@ -94,7 +94,7 @@ class LdapSyncPage extends React.Component {
         if (res.status === "ok") {
           this.setState((prevState) => {
             prevState.users = res.data.users;
-            prevState.existUuids = res.data2?.length > 0 ? res.data2 : [];
+            prevState.existUuids = res.data2?.length > 0 ? res.data2.filter(uuid => uuid !== "") : [];
             return prevState;
           });
         } else {
@@ -210,7 +210,7 @@ class LdapSyncPage extends React.Component {
         });
       },
       getCheckboxProps: record => ({
-        disabled: this.state.existUuids.indexOf(record.uuid) !== -1,
+        disabled: this.state.existUuids.indexOf(record.uuid) !== -1 || record.uidNumber === "",
       }),
     };
 

web/src/auth/LoginPage.js

@@ -242,6 +242,7 @@ class LoginPage extends React.Component {
 
             if (resp.msg === RequiredMfa) {
               Setting.goToLink(`/prompt/${application.name}?redirectUri=${oAuthParams.redirectUri}&code=${code}&state=${oAuthParams.state}&promptType=mfa`);
+              return;
             }
 
             if (Setting.isPromptAnswered(account, application)) {