From 6455734807f3614020522b3102652cbd4b8c6bc5 Mon Sep 17 00:00:00 2001 From: Yaodong Yu <2814461814@qq.com> Date: Thu, 18 May 2023 22:03:53 +0800 Subject: [PATCH] fix: fix incorrect LDAP sync status (#1859) --- controllers/ldap.go | 11 ++++++--- object/ldap_autosync.go | 10 +++++--- object/ldap_conn.go | 51 +++++++++++++++++---------------------- web/src/LdapSyncPage.js | 4 +-- web/src/auth/LoginPage.js | 1 + 5 files changed, 38 insertions(+), 39 deletions(-) diff --git a/controllers/ldap.go b/controllers/ldap.go index 1d54418a..52dc242e 100644 --- a/controllers/ldap.go +++ b/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, - Failed: *failed, + Exist: exist, + Failed: failed, }) } diff --git a/object/ldap_autosync.go b/object/ldap_autosync.go index 1a31ea01..04cc19f2 100644 --- a/object/ldap_autosync.go +++ b/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 { - logs.Warning(fmt.Sprintf("ldap autosync,%d new users,but %d user failed during :", len(users)-len(*existed)-len(*failed), len(*failed)), *failed) + + existed, failed, err := SyncLdapUsers(ldap.Owner, LdapUsersToLdapRespUsers(users), ldap.Id) + 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))) } } } diff --git a/object/ldap_conn.go b/object/ldap_conn.go index 184ba0c1..515860c2 100644 --- a/object/ldap_conn.go +++ b/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) { - var existUsers []LdapRespUser - var failedUsers []LdapRespUser +func SyncLdapUsers(owner string, syncUsers []LdapRespUser, ldapId string) (existUsers []LdapRespUser, failedUsers []LdapRespUser, err error) { 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 { - existUsers = append(existUsers, respUser) + if syncUser.Uuid == existUuid { + 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, - Phone: respUser.Phone, - Address: []string{respUser.Address}, + Email: syncUser.Email, + Phone: syncUser.Phone, + 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 } diff --git a/web/src/LdapSyncPage.js b/web/src/LdapSyncPage.js index a4db940a..f2582a5d 100644 --- a/web/src/LdapSyncPage.js +++ b/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 === "", }), }; diff --git a/web/src/auth/LoginPage.js b/web/src/auth/LoginPage.js index 901c99b3..d5853c83 100644 --- a/web/src/auth/LoginPage.js +++ b/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)) {