Skip to content

Commit f48c013

Browse files
raucaowxiaoguang
andauthored
Fix/improve avatar sync from LDAP (#34573)
This fixes 3 issues I encountered when debugging problems with our LDAP sync: 1. The comparison of the hashed image data in `IsUploadAvatarChanged` is wrong. It seems to be from before avatar hashing was changed and unified in #22289. This results in the function always returning `true` for any avatars, even if they weren't changed. 2. Even if there's no avatar to upload (i.e. no avatar available for the LDAP entry), the upload function would still be called for every single user, only to then fail, because the data isn't valid. This is unnecessary. 3. Another small issue is that the comparison function (and thus hashing of data) is called for every user, even if there is no avatar attribute configured at all for the LDAP source. Thus, I switched the condition nesting, so that no cycles are wasted when avatar sync isn't configured in the first place. I also added a trace log for when there is actually a new avatar being uploaded for an existing user, which is now only shown when that is actually the case. --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent e8d8984 commit f48c013

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

models/user/avatar.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package user
55

66
import (
77
"context"
8-
"crypto/md5"
98
"fmt"
109
"image/png"
1110
"io"
@@ -106,7 +105,7 @@ func (u *User) IsUploadAvatarChanged(data []byte) bool {
106105
if !u.UseCustomAvatar || len(u.Avatar) == 0 {
107106
return true
108107
}
109-
avatarID := fmt.Sprintf("%x", md5.Sum([]byte(fmt.Sprintf("%d-%x", u.ID, md5.Sum(data)))))
108+
avatarID := avatar.HashAvatar(u.ID, data)
110109
return u.Avatar != avatarID
111110
}
112111

services/auth/source/ldap/source_sync.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
178178
}
179179
}
180180

181-
if usr.IsUploadAvatarChanged(su.Avatar) {
182-
if err == nil && source.AttributeAvatar != "" {
181+
if source.AttributeAvatar != "" {
182+
if len(su.Avatar) > 0 && usr.IsUploadAvatarChanged(su.Avatar) {
183+
log.Trace("SyncExternalUsers[%s]: Uploading new avatar for %s", source.AuthSource.Name, usr.Name)
183184
_ = user_service.UploadAvatar(ctx, usr, su.Avatar)
184185
}
185186
}

0 commit comments

Comments
 (0)