mirror of
https://github.com/go-gitea/gitea.git
synced 2024-12-04 14:46:57 -05:00
Fix LDAP sync when Username Attribute is empty (#25278)
Fix #21072 ![image](https://github.com/go-gitea/gitea/assets/15528715/96b30beb-7f88-4a60-baae-2e5ad8049555) Username Attribute is not a required item when creating an authentication source. If Username Attribute is empty, the username value of LDAP user cannot be read, so all users from LDAP will be marked as inactive by mistake when synchronizing external users. This PR improves the sync logic, if username is empty, the email address will be used to find user.
This commit is contained in:
parent
1a5b7c86a3
commit
33cd74ad70
@ -6,7 +6,6 @@ package ldap
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
asymkey_model "code.gitea.io/gitea/models/asymkey"
|
||||
@ -24,7 +23,6 @@ import (
|
||||
func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
log.Trace("Doing: SyncExternalUsers[%s]", source.authSource.Name)
|
||||
|
||||
var existingUsers []int
|
||||
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
|
||||
var sshKeysNeedUpdate bool
|
||||
|
||||
@ -41,9 +39,14 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
default:
|
||||
}
|
||||
|
||||
sort.Slice(users, func(i, j int) bool {
|
||||
return users[i].LowerName < users[j].LowerName
|
||||
})
|
||||
usernameUsers := make(map[string]*user_model.User, len(users))
|
||||
mailUsers := make(map[string]*user_model.User, len(users))
|
||||
keepActiveUsers := make(map[int64]struct{})
|
||||
|
||||
for _, u := range users {
|
||||
usernameUsers[u.LowerName] = u
|
||||
mailUsers[strings.ToLower(u.Email)] = u
|
||||
}
|
||||
|
||||
sr, err := source.SearchEntries()
|
||||
if err != nil {
|
||||
@ -59,11 +62,6 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
log.Warn("LDAP search found no entries but did not report an error. All users will be deactivated as per settings")
|
||||
}
|
||||
|
||||
sort.Slice(sr, func(i, j int) bool {
|
||||
return sr[i].LowerName < sr[j].LowerName
|
||||
})
|
||||
|
||||
userPos := 0
|
||||
orgCache := make(map[string]*organization.Organization)
|
||||
teamCache := make(map[string]*organization.Team)
|
||||
|
||||
@ -86,7 +84,22 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
return db.ErrCancelledf("During update of %s before completed update of users", source.authSource.Name)
|
||||
default:
|
||||
}
|
||||
if len(su.Username) == 0 {
|
||||
if len(su.Username) == 0 && len(su.Mail) == 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
var usr *user_model.User
|
||||
if len(su.Username) > 0 {
|
||||
usr = usernameUsers[su.LowerName]
|
||||
}
|
||||
if usr == nil && len(su.Mail) > 0 {
|
||||
usr = mailUsers[strings.ToLower(su.Mail)]
|
||||
}
|
||||
|
||||
if usr != nil {
|
||||
keepActiveUsers[usr.ID] = struct{}{}
|
||||
} else if len(su.Username) == 0 {
|
||||
// we cannot create the user if su.Username is empty
|
||||
continue
|
||||
}
|
||||
|
||||
@ -94,15 +107,6 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
su.Mail = fmt.Sprintf("%s@localhost", su.Username)
|
||||
}
|
||||
|
||||
var usr *user_model.User
|
||||
for userPos < len(users) && users[userPos].LowerName < su.LowerName {
|
||||
userPos++
|
||||
}
|
||||
if userPos < len(users) && users[userPos].LowerName == su.LowerName {
|
||||
usr = users[userPos]
|
||||
existingUsers = append(existingUsers, userPos)
|
||||
}
|
||||
|
||||
fullName := composeFullName(su.Name, su.Surname, su.Username)
|
||||
// If no existing user found, create one
|
||||
if usr == nil {
|
||||
@ -203,19 +207,17 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
|
||||
|
||||
// Deactivate users not present in LDAP
|
||||
if updateExisting {
|
||||
existPos := 0
|
||||
for i, usr := range users {
|
||||
for existPos < len(existingUsers) && i > existingUsers[existPos] {
|
||||
existPos++
|
||||
for _, usr := range users {
|
||||
if _, ok := keepActiveUsers[usr.ID]; ok {
|
||||
continue
|
||||
}
|
||||
if usr.IsActive && (existPos >= len(existingUsers) || i < existingUsers[existPos]) {
|
||||
log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name)
|
||||
|
||||
usr.IsActive = false
|
||||
err = user_model.UpdateUserCols(ctx, usr, "is_active")
|
||||
if err != nil {
|
||||
log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err)
|
||||
}
|
||||
log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name)
|
||||
|
||||
usr.IsActive = false
|
||||
err = user_model.UpdateUserCols(ctx, usr, "is_active")
|
||||
if err != nil {
|
||||
log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -268,6 +268,57 @@ func TestLDAPUserSync(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
|
||||
if skipLDAPTests() {
|
||||
t.Skip()
|
||||
return
|
||||
}
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
session := loginUser(t, "user1")
|
||||
csrf := GetCSRF(t, session, "/admin/auths/new")
|
||||
payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "")
|
||||
payload["attribute_username"] = ""
|
||||
req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload)
|
||||
session.MakeRequest(t, req, http.StatusSeeOther)
|
||||
|
||||
for _, u := range gitLDAPUsers {
|
||||
req := NewRequest(t, "GET", "/admin/users?q="+u.UserName)
|
||||
resp := session.MakeRequest(t, req, http.StatusOK)
|
||||
|
||||
htmlDoc := NewHTMLParser(t, resp.Body)
|
||||
|
||||
tr := htmlDoc.doc.Find("table.table tbody tr")
|
||||
assert.True(t, tr.Length() == 0)
|
||||
}
|
||||
|
||||
for _, u := range gitLDAPUsers {
|
||||
req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{
|
||||
"_csrf": csrf,
|
||||
"user_name": u.UserName,
|
||||
"password": u.Password,
|
||||
})
|
||||
MakeRequest(t, req, http.StatusSeeOther)
|
||||
}
|
||||
|
||||
auth.SyncExternalUsers(context.Background(), true)
|
||||
|
||||
authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{
|
||||
Name: payload["name"],
|
||||
})
|
||||
unittest.AssertCount(t, &user_model.User{
|
||||
LoginType: auth_model.LDAP,
|
||||
LoginSource: authSource.ID,
|
||||
}, len(gitLDAPUsers))
|
||||
|
||||
for _, u := range gitLDAPUsers {
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
|
||||
Name: u.UserName,
|
||||
})
|
||||
assert.True(t, user.IsActive)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
|
||||
if skipLDAPTests() {
|
||||
t.Skip()
|
||||
|
Loading…
Reference in New Issue
Block a user