Skip to content

Commit b01b0b9

Browse files
authored
Refactor some LDAP code (#32849)
1 parent 33e8e82 commit b01b0b9

File tree

6 files changed

+259
-173
lines changed

6 files changed

+259
-173
lines changed

Diff for: services/auth/source/ldap/source.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ type Source struct {
5656
UserUID string // User Attribute listed in Group
5757
SkipLocalTwoFA bool `json:",omitempty"` // Skip Local 2fa for users authenticated with this source
5858

59-
// reference to the authSource
60-
authSource *auth.Source
59+
authSource *auth.Source // reference to the authSource
6160
}
6261

6362
// FromDB fills up a LDAPConfig from serialized format.
@@ -107,7 +106,7 @@ func (source *Source) UseTLS() bool {
107106

108107
// ProvidesSSHKeys returns if this source provides SSH Keys
109108
func (source *Source) ProvidesSSHKeys() bool {
110-
return len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
109+
return strings.TrimSpace(source.AttributeSSHPublicKey) != ""
111110
}
112111

113112
// SetAuthSource sets the related AuthSource

Diff for: services/auth/source/ldap/source_authenticate.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
3131
return nil, user_model.ErrUserNotExist{Name: loginName}
3232
}
3333
// Fallback.
34-
if len(sr.Username) == 0 {
34+
if sr.Username == "" {
3535
sr.Username = userName
3636
}
37-
if len(sr.Mail) == 0 {
37+
if sr.Mail == "" {
3838
sr.Mail = fmt.Sprintf("%[email protected]", sr.Username)
3939
}
40-
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
40+
isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != ""
4141

4242
// Update User admin flag if exist
4343
if isExist, err := user_model.IsUserExist(ctx, 0, sr.Username); err != nil {
@@ -51,11 +51,11 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
5151
}
5252
if user != nil && !user.ProhibitLogin {
5353
opts := &user_service.UpdateOptions{}
54-
if len(source.AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin {
54+
if source.AdminFilter != "" && user.IsAdmin != sr.IsAdmin {
5555
// Change existing admin flag only if AdminFilter option is set
5656
opts.IsAdmin = optional.Some(sr.IsAdmin)
5757
}
58-
if !sr.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted {
58+
if !sr.IsAdmin && source.RestrictedFilter != "" && user.IsRestricted != sr.IsRestricted {
5959
// Change existing restricted flag only if RestrictedFilter option is set
6060
opts.IsRestricted = optional.Some(sr.IsRestricted)
6161
}
@@ -99,7 +99,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
9999
return user, err
100100
}
101101
}
102-
if len(source.AttributeAvatar) > 0 {
102+
if source.AttributeAvatar != "" {
103103
if err := user_service.UploadAvatar(ctx, user, sr.Avatar); err != nil {
104104
return user, err
105105
}

Diff for: services/auth/source/ldap/source_search.go

+21-12
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func bindUser(l *ldap.Conn, userDN, passwd string) error {
147147
}
148148

149149
func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool {
150-
if len(ls.AdminFilter) == 0 {
150+
if ls.AdminFilter == "" {
151151
return false
152152
}
153153
log.Trace("Checking admin with filter %s and base %s", ls.AdminFilter, userDN)
@@ -169,7 +169,7 @@ func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool {
169169
}
170170

171171
func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool {
172-
if len(ls.RestrictedFilter) == 0 {
172+
if ls.RestrictedFilter == "" {
173173
return false
174174
}
175175
if ls.RestrictedFilter == "*" {
@@ -250,8 +250,17 @@ func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string {
250250

251251
// SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter
252252
func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult {
253+
if MockedSearchEntry != nil {
254+
return MockedSearchEntry(source, name, passwd, directBind)
255+
}
256+
return realSearchEntry(source, name, passwd, directBind)
257+
}
258+
259+
var MockedSearchEntry func(source *Source, name, passwd string, directBind bool) *SearchResult
260+
261+
func realSearchEntry(source *Source, name, passwd string, directBind bool) *SearchResult {
253262
// See https://tools.ietf.org/search/rfc4513#section-5.1.2
254-
if len(passwd) == 0 {
263+
if passwd == "" {
255264
log.Debug("Auth. failed for %s, password cannot be empty", name)
256265
return nil
257266
}
@@ -323,17 +332,17 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR
323332
return nil
324333
}
325334

326-
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
327-
isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0
335+
isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != ""
336+
isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != ""
328337

329338
attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail}
330-
if len(strings.TrimSpace(source.UserUID)) > 0 {
339+
if strings.TrimSpace(source.UserUID) != "" {
331340
attribs = append(attribs, source.UserUID)
332341
}
333342
if isAttributeSSHPublicKeySet {
334343
attribs = append(attribs, source.AttributeSSHPublicKey)
335344
}
336-
if isAtributeAvatarSet {
345+
if isAttributeAvatarSet {
337346
attribs = append(attribs, source.AttributeAvatar)
338347
}
339348

@@ -375,7 +384,7 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR
375384
isRestricted = checkRestricted(l, source, userDN)
376385
}
377386

378-
if isAtributeAvatarSet {
387+
if isAttributeAvatarSet {
379388
Avatar = sr.Entries[0].GetRawAttributeValue(source.AttributeAvatar)
380389
}
381390

@@ -440,14 +449,14 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) {
440449

441450
userFilter := fmt.Sprintf(source.Filter, "*")
442451

443-
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
444-
isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0
452+
isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != ""
453+
isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != ""
445454

446455
attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail, source.UserUID}
447456
if isAttributeSSHPublicKeySet {
448457
attribs = append(attribs, source.AttributeSSHPublicKey)
449458
}
450-
if isAtributeAvatarSet {
459+
if isAttributeAvatarSet {
451460
attribs = append(attribs, source.AttributeAvatar)
452461
}
453462

@@ -503,7 +512,7 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) {
503512
user.SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey)
504513
}
505514

506-
if isAtributeAvatarSet {
515+
if isAttributeAvatarSet {
507516
user.Avatar = v.GetRawAttributeValue(source.AttributeAvatar)
508517
}
509518

Diff for: services/auth/source/ldap/source_sync.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
2626
log.Trace("Doing: SyncExternalUsers[%s]", source.authSource.Name)
2727

28-
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
28+
isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != ""
2929
var sshKeysNeedUpdate bool
3030

3131
// Find all users with this login type - FIXME: Should this be an iterator?
@@ -86,26 +86,26 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
8686
return db.ErrCancelledf("During update of %s before completed update of users", source.authSource.Name)
8787
default:
8888
}
89-
if len(su.Username) == 0 && len(su.Mail) == 0 {
89+
if su.Username == "" && su.Mail == "" {
9090
continue
9191
}
9292

9393
var usr *user_model.User
94-
if len(su.Username) > 0 {
94+
if su.Username != "" {
9595
usr = usernameUsers[su.LowerName]
9696
}
97-
if usr == nil && len(su.Mail) > 0 {
97+
if usr == nil && su.Mail != "" {
9898
usr = mailUsers[strings.ToLower(su.Mail)]
9999
}
100100

101101
if usr != nil {
102102
keepActiveUsers.Add(usr.ID)
103-
} else if len(su.Username) == 0 {
103+
} else if su.Username == "" {
104104
// we cannot create the user if su.Username is empty
105105
continue
106106
}
107107

108-
if len(su.Mail) == 0 {
108+
if su.Mail == "" {
109109
su.Mail = fmt.Sprintf("%[email protected]", su.Username)
110110
}
111111

@@ -141,7 +141,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
141141
}
142142
}
143143

144-
if err == nil && len(source.AttributeAvatar) > 0 {
144+
if err == nil && source.AttributeAvatar != "" {
145145
_ = user_service.UploadAvatar(ctx, usr, su.Avatar)
146146
}
147147
} else if updateExisting {
@@ -151,8 +151,8 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
151151
}
152152

153153
// Check if user data has changed
154-
if (len(source.AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) ||
155-
(len(source.RestrictedFilter) > 0 && usr.IsRestricted != su.IsRestricted) ||
154+
if (source.AdminFilter != "" && usr.IsAdmin != su.IsAdmin) ||
155+
(source.RestrictedFilter != "" && usr.IsRestricted != su.IsRestricted) ||
156156
!strings.EqualFold(usr.Email, su.Mail) ||
157157
usr.FullName != fullName ||
158158
!usr.IsActive {
@@ -180,7 +180,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
180180
}
181181

182182
if usr.IsUploadAvatarChanged(su.Avatar) {
183-
if err == nil && len(source.AttributeAvatar) > 0 {
183+
if err == nil && source.AttributeAvatar != "" {
184184
_ = user_service.UploadAvatar(ctx, usr, su.Avatar)
185185
}
186186
}

Diff for: services/auth/source/ldap/util.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ package ldap
66
// composeFullName composes a firstname surname or username
77
func composeFullName(firstname, surname, username string) string {
88
switch {
9-
case len(firstname) == 0 && len(surname) == 0:
9+
case firstname == "" && surname == "":
1010
return username
11-
case len(firstname) == 0:
11+
case firstname == "":
1212
return surname
13-
case len(surname) == 0:
13+
case surname == "":
1414
return firstname
1515
default:
1616
return firstname + " " + surname

0 commit comments

Comments
 (0)