Skip to content

Commit 16d2650

Browse files
HesterGHesterG
authored
Merge branch 'go-gitea:main' into lock-pin-fetch
2 parents e0dde35 + ef6f5f0 commit 16d2650

File tree

7 files changed

+89
-36
lines changed

7 files changed

+89
-36
lines changed

‎services/auth/source/ldap/source_sync.go

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package ldap
66
import (
77
"context"
88
"fmt"
9-
"sort"
109
"strings"
1110

1211
asymkey_model "code.gitea.io/gitea/models/asymkey"
@@ -24,7 +23,6 @@ import (
2423
func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
2524
log.Trace("Doing: SyncExternalUsers[%s]", source.authSource.Name)
2625

27-
var existingUsers []int
2826
isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0
2927
var sshKeysNeedUpdate bool
3028

@@ -41,9 +39,14 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
4139
default:
4240
}
4341

44-
sort.Slice(users, func(i, j int) bool {
45-
return users[i].LowerName < users[j].LowerName
46-
})
42+
usernameUsers := make(map[string]*user_model.User, len(users))
43+
mailUsers := make(map[string]*user_model.User, len(users))
44+
keepActiveUsers := make(map[int64]struct{})
45+
46+
for _, u := range users {
47+
usernameUsers[u.LowerName] = u
48+
mailUsers[strings.ToLower(u.Email)] = u
49+
}
4750

4851
sr, err := source.SearchEntries()
4952
if err != nil {
@@ -59,11 +62,6 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
5962
log.Warn("LDAP search found no entries but did not report an error. All users will be deactivated as per settings")
6063
}
6164

62-
sort.Slice(sr, func(i, j int) bool {
63-
return sr[i].LowerName < sr[j].LowerName
64-
})
65-
66-
userPos := 0
6765
orgCache := make(map[string]*organization.Organization)
6866
teamCache := make(map[string]*organization.Team)
6967

@@ -86,21 +84,27 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
8684
return db.ErrCancelledf("During update of %s before completed update of users", source.authSource.Name)
8785
default:
8886
}
89-
if len(su.Username) == 0 {
87+
if len(su.Username) == 0 && len(su.Mail) == 0 {
9088
continue
9189
}
9290

93-
if len(su.Mail) == 0 {
94-
su.Mail = fmt.Sprintf("%s@localhost", su.Username)
91+
var usr *user_model.User
92+
if len(su.Username) > 0 {
93+
usr = usernameUsers[su.LowerName]
94+
}
95+
if usr == nil && len(su.Mail) > 0 {
96+
usr = mailUsers[strings.ToLower(su.Mail)]
9597
}
9698

97-
var usr *user_model.User
98-
for userPos < len(users) && users[userPos].LowerName < su.LowerName {
99-
userPos++
99+
if usr != nil {
100+
keepActiveUsers[usr.ID] = struct{}{}
101+
} else if len(su.Username) == 0 {
102+
// we cannot create the user if su.Username is empty
103+
continue
100104
}
101-
if userPos < len(users) && users[userPos].LowerName == su.LowerName {
102-
usr = users[userPos]
103-
existingUsers = append(existingUsers, userPos)
105+
106+
if len(su.Mail) == 0 {
107+
su.Mail = fmt.Sprintf("%s@localhost", su.Username)
104108
}
105109

106110
fullName := composeFullName(su.Name, su.Surname, su.Username)
@@ -203,19 +207,17 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
203207

204208
// Deactivate users not present in LDAP
205209
if updateExisting {
206-
existPos := 0
207-
for i, usr := range users {
208-
for existPos < len(existingUsers) && i > existingUsers[existPos] {
209-
existPos++
210+
for _, usr := range users {
211+
if _, ok := keepActiveUsers[usr.ID]; ok {
212+
continue
210213
}
211-
if usr.IsActive && (existPos >= len(existingUsers) || i < existingUsers[existPos]) {
212-
log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name)
213214

214-
usr.IsActive = false
215-
err = user_model.UpdateUserCols(ctx, usr, "is_active")
216-
if err != nil {
217-
log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err)
218-
}
215+
log.Trace("SyncExternalUsers[%s]: Deactivating user %s", source.authSource.Name, usr.Name)
216+
217+
usr.IsActive = false
218+
err = user_model.UpdateUserCols(ctx, usr, "is_active")
219+
if err != nil {
220+
log.Error("SyncExternalUsers[%s]: Error deactivating user %s: %v", source.authSource.Name, usr.Name, err)
219221
}
220222
}
221223
}

‎templates/base/head_script.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
1717
notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
1818
enableTimeTracking: {{EnableTimetracking}},
1919
{{if or .Participants .Assignees .MentionableTeams}}
20-
tributeValues: Array.from(new Map([
20+
mentionValues: Array.from(new Map([
2121
{{- range .Participants -}}
2222
['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}', name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink $.Context}}'}],
2323
{{- end -}}

‎tests/integration/auth_ldap_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,57 @@ func TestLDAPUserSync(t *testing.T) {
268268
}
269269
}
270270

271+
func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
272+
if skipLDAPTests() {
273+
t.Skip()
274+
return
275+
}
276+
defer tests.PrepareTestEnv(t)()
277+
278+
session := loginUser(t, "user1")
279+
csrf := GetCSRF(t, session, "/admin/auths/new")
280+
payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "")
281+
payload["attribute_username"] = ""
282+
req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload)
283+
session.MakeRequest(t, req, http.StatusSeeOther)
284+
285+
for _, u := range gitLDAPUsers {
286+
req := NewRequest(t, "GET", "/admin/users?q="+u.UserName)
287+
resp := session.MakeRequest(t, req, http.StatusOK)
288+
289+
htmlDoc := NewHTMLParser(t, resp.Body)
290+
291+
tr := htmlDoc.doc.Find("table.table tbody tr")
292+
assert.True(t, tr.Length() == 0)
293+
}
294+
295+
for _, u := range gitLDAPUsers {
296+
req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{
297+
"_csrf": csrf,
298+
"user_name": u.UserName,
299+
"password": u.Password,
300+
})
301+
MakeRequest(t, req, http.StatusSeeOther)
302+
}
303+
304+
auth.SyncExternalUsers(context.Background(), true)
305+
306+
authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{
307+
Name: payload["name"],
308+
})
309+
unittest.AssertCount(t, &user_model.User{
310+
LoginType: auth_model.LDAP,
311+
LoginSource: authSource.ID,
312+
}, len(gitLDAPUsers))
313+
314+
for _, u := range gitLDAPUsers {
315+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
316+
Name: u.UserName,
317+
})
318+
assert.True(t, user.IsActive)
319+
}
320+
}
321+
271322
func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
272323
if skipLDAPTests() {
273324
t.Skip()

‎web_src/js/features/tribute.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ function makeCollections({mentions, emoji}) {
3131

3232
if (mentions) {
3333
collections.push({
34-
values: window.config.tributeValues,
34+
values: window.config.mentionValues,
3535
requireLeadingSpace: true,
3636
menuItemTemplate: (item) => {
3737
return `

‎web_src/js/test/setup.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ window.config = {
33
pageData: {},
44
i18n: {},
55
appSubUrl: '',
6-
tributeValues: [
6+
mentionValues: [
77
{key: 'user1 User 1', value: 'user1', name: 'user1', fullname: 'User 1', avatar: 'https://avatar1.com'},
88
{key: 'user2 User 2', value: 'user2', name: 'user2', fullname: 'User 2', avatar: 'https://avatar2.com'},
99
{key: 'user3 User 3', value: 'user3', name: 'user3', fullname: 'User 3', avatar: 'https://avatar3.com'},

‎web_src/js/utils/match.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function matchMention(queryText) {
3232

3333
// results is a map of weights, lower is better
3434
const results = new Map();
35-
for (const obj of window.config.tributeValues) {
35+
for (const obj of window.config.mentionValues) {
3636
const index = obj.key.toLowerCase().indexOf(query);
3737
if (index === -1) continue;
3838
const existing = results.get(obj);

‎web_src/js/utils/match.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,6 @@ test('matchEmoji', () => {
4242
});
4343

4444
test('matchMention', () => {
45-
expect(matchMention('')).toEqual(window.config.tributeValues.slice(0, 6));
46-
expect(matchMention('user4')).toEqual([window.config.tributeValues[3]]);
45+
expect(matchMention('')).toEqual(window.config.mentionValues.slice(0, 6));
46+
expect(matchMention('user4')).toEqual([window.config.mentionValues[3]]);
4747
});

0 commit comments

Comments
 (0)