Fix/improve avatar sync from LDAP (#34573) (#34587)

Backport #34573 by @raucao

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: Râu Cao <842+raucao@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
pull/34571/head^2
Giteabot 2025-06-03 05:19:55 +08:00 committed by GitHub
parent 7baa6fa47c
commit 5d6c5ce71a
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 (
"context"
"crypto/md5"
"fmt"
"image/png"
"io"
@ -106,7 +105,7 @@ func (u *User) IsUploadAvatarChanged(data []byte) bool {
if !u.UseCustomAvatar || len(u.Avatar) == 0 {
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
}

View File

@ -178,8 +178,9 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
}
}
if usr.IsUploadAvatarChanged(su.Avatar) {
if err == nil && source.AttributeAvatar != "" {
if 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)
}
}