mirrors
/
forgejo
mirror of
1
Fork 0
Code Issues Releases Activity

Improve LDAP synchronization efficiency (#16994) 

The current LDAP sync routine has order n^2 efficiency. This change reduces this
to order n.log n.

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2021-09-15 10:28:37 +01:00 committed by GitHub
parent 976db2a8b7
commit db6b7db06d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
services/auth/source/ldap/source_search.go
View File

@ -26,6 +26,7 @@ type SearchResult struct {
SSHPublicKey []string // SSH Public Key SSHPublicKey []string // SSH Public Key
IsAdmin bool // if user is administrator IsAdmin bool // if user is administrator
IsRestricted bool // if user is restricted IsRestricted bool // if user is restricted
LowerName string // Lowername
} }
func (ls *Source) sanitizedUserQuery(username string) (string, bool) { func (ls *Source) sanitizedUserQuery(username string) (string, bool) {
@ -363,6 +364,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul
} }
return &SearchResult{ return &SearchResult{
LowerName: strings.ToLower(username),
Username: username, Username: username,
Name: firstname, Name: firstname,
Surname: surname, Surname: surname,
@ -440,6 +442,8 @@ func (ls *Source) SearchEntries() ([]*SearchResult, error) {
if isAttributeSSHPublicKeySet { if isAttributeSSHPublicKeySet {
result[i].SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey) result[i].SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey)
} }
result[i].LowerName = strings.ToLower(result[i].Username)
} }
return result, nil return result, nil

42
services/auth/source/ldap/source_sync.go
View File

@ -7,6 +7,7 @@ package ldap
import ( import (
"context" "context"
"fmt" "fmt"
"sort"
"strings" "strings"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
@ -17,7 +18,7 @@ import (
func (source *Source) Sync(ctx context.Context, updateExisting bool) error { func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
log.Trace("Doing: SyncExternalUsers[%s]", source.loginSource.Name) log.Trace("Doing: SyncExternalUsers[%s]", source.loginSource.Name)
var existingUsers []int64 var existingUsers []int
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
var sshKeysNeedUpdate bool var sshKeysNeedUpdate bool
@ -34,6 +35,10 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
default: default:
} }
sort.Slice(users, func(i, j int) bool {
return users[i].LowerName < users[j].LowerName
})
sr, err := source.SearchEntries() sr, err := source.SearchEntries()
if err != nil { if err != nil {
log.Error("SyncExternalUsers LDAP source failure [%s], skipped", source.loginSource.Name) log.Error("SyncExternalUsers LDAP source failure [%s], skipped", source.loginSource.Name)
@ -48,6 +53,12 @@ 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") 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
for _, su := range sr { for _, su := range sr {
select { select {
case <-ctx.Done(): case <-ctx.Done():
@ -71,12 +82,12 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
} }
var usr *models.User var usr *models.User
// Search for existing user for userPos < len(users) && users[userPos].LowerName < su.LowerName {
for _, du := range users { userPos++
if du.LowerName == strings.ToLower(su.Username) { }
usr = du if userPos < len(users) && users[userPos].LowerName == su.LowerName {
break usr = users[userPos]
} existingUsers = append(existingUsers, userPos)
} }
fullName := composeFullName(su.Name, su.Surname, su.Username) fullName := composeFullName(su.Name, su.Surname, su.Username)
@ -85,7 +96,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
log.Trace("SyncExternalUsers[%s]: Creating user %s", source.loginSource.Name, su.Username) log.Trace("SyncExternalUsers[%s]: Creating user %s", source.loginSource.Name, su.Username)
usr = &models.User{ usr = &models.User{
LowerName: strings.ToLower(su.Username), LowerName: su.LowerName,
Name: su.Username, Name: su.Username,
FullName: fullName, FullName: fullName,
LoginType: source.loginSource.Type, LoginType: source.loginSource.Type,
@ -108,8 +119,6 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
} }
} }
} else if updateExisting { } else if updateExisting {
existingUsers = append(existingUsers, usr.ID)
// Synchronize SSH Public Key if that attribute is set // Synchronize SSH Public Key if that attribute is set
if isAttributeSSHPublicKeySet && models.SynchronizePublicKeys(usr, source.loginSource, su.SSHPublicKey) { if isAttributeSSHPublicKeySet && models.SynchronizePublicKeys(usr, source.loginSource, su.SSHPublicKey) {
sshKeysNeedUpdate = true sshKeysNeedUpdate = true
@ -161,15 +170,12 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
// Deactivate users not present in LDAP // Deactivate users not present in LDAP
if updateExisting { if updateExisting {
for _, usr := range users { existPos := 0
found := false for i, usr := range users {
for _, uid := range existingUsers { for existPos < len(existingUsers) && i > existingUsers[existPos] {
if usr.ID == uid { existPos++
found = true
break
}
} }
if !found { if usr.IsActive && (existPos >= len(existingUsers) || i < existingUsers[existPos]) {
log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.loginSource.Name, usr.Name) log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.loginSource.Name, usr.Name)
usr.IsActive = false usr.IsActive = false