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 <wxiaoguang@gmail.com>
pull/34553/head^2
Râu Cao 2025-06-02 19:05:47 +02:00 committed by GitHub
parent e8d8984f7c
commit f48c0135a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 4 additions and 4 deletions

View File

@ -5,7 +5,6 @@ package user
import ( import (
"context" "context"
"crypto/md5"
"fmt" "fmt"
"image/png" "image/png"
"io" "io"
@ -106,7 +105,7 @@ func (u *User) IsUploadAvatarChanged(data []byte) bool {
if !u.UseCustomAvatar || len(u.Avatar) == 0 { if !u.UseCustomAvatar || len(u.Avatar) == 0 {
return true return true
} }
avatarID := fmt.Sprintf("%x", md5.Sum([]byte(fmt.Sprintf("%d-%x", u.ID, md5.Sum(data))))) avatarID := avatar.HashAvatar(u.ID, data)
return u.Avatar != avatarID return u.Avatar != avatarID
} }

View File

@ -178,8 +178,9 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
} }
} }
if usr.IsUploadAvatarChanged(su.Avatar) { if source.AttributeAvatar != "" {
if err == nil && source.AttributeAvatar != "" { if len(su.Avatar) > 0 && usr.IsUploadAvatarChanged(su.Avatar) {
log.Trace("SyncExternalUsers[%s]: Uploading new avatar for %s", source.AuthSource.Name, usr.Name)
_ = user_service.UploadAvatar(ctx, usr, su.Avatar) _ = user_service.UploadAvatar(ctx, usr, su.Avatar)
} }
} }