From b01b0b99a596ff6f879b460aef9a115a2cd811a5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 15 Dec 2024 19:59:18 +0800 Subject: [PATCH] Refactor some LDAP code (#32849) --- services/auth/source/ldap/source.go | 5 +- .../auth/source/ldap/source_authenticate.go | 12 +- services/auth/source/ldap/source_search.go | 33 +- services/auth/source/ldap/source_sync.go | 20 +- services/auth/source/ldap/util.go | 6 +- tests/integration/auth_ldap_test.go | 366 +++++++++++------- 6 files changed, 264 insertions(+), 178 deletions(-) diff --git a/services/auth/source/ldap/source.go b/services/auth/source/ldap/source.go index dc4cb2c9403..963cdba7c21 100644 --- a/services/auth/source/ldap/source.go +++ b/services/auth/source/ldap/source.go @@ -56,8 +56,7 @@ type Source struct { UserUID string // User Attribute listed in Group SkipLocalTwoFA bool `json:",omitempty"` // Skip Local 2fa for users authenticated with this source - // reference to the authSource - authSource *auth.Source + authSource *auth.Source // reference to the authSource } // FromDB fills up a LDAPConfig from serialized format. @@ -107,7 +106,7 @@ func (source *Source) UseTLS() bool { // ProvidesSSHKeys returns if this source provides SSH Keys func (source *Source) ProvidesSSHKeys() bool { - return len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + return strings.TrimSpace(source.AttributeSSHPublicKey) != "" } // SetAuthSource sets the related AuthSource diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index 01cb7437205..020e5784dcf 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -31,13 +31,13 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u return nil, user_model.ErrUserNotExist{Name: loginName} } // Fallback. - if len(sr.Username) == 0 { + if sr.Username == "" { sr.Username = userName } - if len(sr.Mail) == 0 { + if sr.Mail == "" { sr.Mail = fmt.Sprintf("%s@localhost.local", sr.Username) } - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" // Update User admin flag if exist 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 } if user != nil && !user.ProhibitLogin { opts := &user_service.UpdateOptions{} - if len(source.AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { + if source.AdminFilter != "" && user.IsAdmin != sr.IsAdmin { // Change existing admin flag only if AdminFilter option is set opts.IsAdmin = optional.Some(sr.IsAdmin) } - if !sr.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { + if !sr.IsAdmin && source.RestrictedFilter != "" && user.IsRestricted != sr.IsRestricted { // Change existing restricted flag only if RestrictedFilter option is set opts.IsRestricted = optional.Some(sr.IsRestricted) } @@ -99,7 +99,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u return user, err } } - if len(source.AttributeAvatar) > 0 { + if source.AttributeAvatar != "" { if err := user_service.UploadAvatar(ctx, user, sr.Avatar); err != nil { return user, err } diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go index b20c90e791d..fa2c45ce4ad 100644 --- a/services/auth/source/ldap/source_search.go +++ b/services/auth/source/ldap/source_search.go @@ -147,7 +147,7 @@ func bindUser(l *ldap.Conn, userDN, passwd string) error { } func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool { - if len(ls.AdminFilter) == 0 { + if ls.AdminFilter == "" { return false } 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 { } func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool { - if len(ls.RestrictedFilter) == 0 { + if ls.RestrictedFilter == "" { return false } if ls.RestrictedFilter == "*" { @@ -250,8 +250,17 @@ func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string { // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult { + if MockedSearchEntry != nil { + return MockedSearchEntry(source, name, passwd, directBind) + } + return realSearchEntry(source, name, passwd, directBind) +} + +var MockedSearchEntry func(source *Source, name, passwd string, directBind bool) *SearchResult + +func realSearchEntry(source *Source, name, passwd string, directBind bool) *SearchResult { // See https://tools.ietf.org/search/rfc4513#section-5.1.2 - if len(passwd) == 0 { + if passwd == "" { log.Debug("Auth. failed for %s, password cannot be empty", name) return nil } @@ -323,17 +332,17 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR return nil } - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 - isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" + isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != "" attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail} - if len(strings.TrimSpace(source.UserUID)) > 0 { + if strings.TrimSpace(source.UserUID) != "" { attribs = append(attribs, source.UserUID) } if isAttributeSSHPublicKeySet { attribs = append(attribs, source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { attribs = append(attribs, source.AttributeAvatar) } @@ -375,7 +384,7 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR isRestricted = checkRestricted(l, source, userDN) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { Avatar = sr.Entries[0].GetRawAttributeValue(source.AttributeAvatar) } @@ -440,14 +449,14 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { userFilter := fmt.Sprintf(source.Filter, "*") - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 - isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" + isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != "" attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail, source.UserUID} if isAttributeSSHPublicKeySet { attribs = append(attribs, source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { attribs = append(attribs, source.AttributeAvatar) } @@ -503,7 +512,7 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { user.SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { user.Avatar = v.GetRawAttributeValue(source.AttributeAvatar) } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index a6d6d2a0f2f..e817bf1fa93 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -25,7 +25,7 @@ import ( func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("Doing: SyncExternalUsers[%s]", source.authSource.Name) - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" var sshKeysNeedUpdate bool // 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 { return db.ErrCancelledf("During update of %s before completed update of users", source.authSource.Name) default: } - if len(su.Username) == 0 && len(su.Mail) == 0 { + if su.Username == "" && su.Mail == "" { continue } var usr *user_model.User - if len(su.Username) > 0 { + if su.Username != "" { usr = usernameUsers[su.LowerName] } - if usr == nil && len(su.Mail) > 0 { + if usr == nil && su.Mail != "" { usr = mailUsers[strings.ToLower(su.Mail)] } if usr != nil { keepActiveUsers.Add(usr.ID) - } else if len(su.Username) == 0 { + } else if su.Username == "" { // we cannot create the user if su.Username is empty continue } - if len(su.Mail) == 0 { + if su.Mail == "" { su.Mail = fmt.Sprintf("%s@localhost.local", su.Username) } @@ -141,7 +141,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } } - if err == nil && len(source.AttributeAvatar) > 0 { + if err == nil && source.AttributeAvatar != "" { _ = user_service.UploadAvatar(ctx, usr, su.Avatar) } } else if updateExisting { @@ -151,8 +151,8 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } // Check if user data has changed - if (len(source.AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) || - (len(source.RestrictedFilter) > 0 && usr.IsRestricted != su.IsRestricted) || + if (source.AdminFilter != "" && usr.IsAdmin != su.IsAdmin) || + (source.RestrictedFilter != "" && usr.IsRestricted != su.IsRestricted) || !strings.EqualFold(usr.Email, su.Mail) || usr.FullName != fullName || !usr.IsActive { @@ -180,7 +180,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } if usr.IsUploadAvatarChanged(su.Avatar) { - if err == nil && len(source.AttributeAvatar) > 0 { + if err == nil && source.AttributeAvatar != "" { _ = user_service.UploadAvatar(ctx, usr, su.Avatar) } } diff --git a/services/auth/source/ldap/util.go b/services/auth/source/ldap/util.go index bd11e2d1193..05ae32c0fd2 100644 --- a/services/auth/source/ldap/util.go +++ b/services/auth/source/ldap/util.go @@ -6,11 +6,11 @@ package ldap // composeFullName composes a firstname surname or username func composeFullName(firstname, surname, username string) string { switch { - case len(firstname) == 0 && len(surname) == 0: + case firstname == "" && surname == "": return username - case len(firstname) == 0: + case firstname == "": return surname - case len(surname) == 0: + case surname == "": return firstname default: return firstname + " " + surname diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 1837e827956..5d372443310 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -15,13 +15,17 @@ import ( "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/auth/source/ldap" org_service "code.gitea.io/gitea/services/org" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type ldapUser struct { @@ -35,87 +39,97 @@ type ldapUser struct { SSHKeys []string } -var gitLDAPUsers = []ldapUser{ - { - UserName: "professor", - Password: "professor", - FullName: "Hubert Farnsworth", - Email: "professor@planetexpress.com", - OtherEmails: []string{"hubert@planetexpress.com"}, - IsAdmin: true, - }, - { - UserName: "hermes", - Password: "hermes", - FullName: "Conrad Hermes", - Email: "hermes@planetexpress.com", - SSHKeys: []string{ - "SHA256:qLY06smKfHoW/92yXySpnxFR10QFrLdRjf/GNPvwcW8", - "SHA256:QlVTuM5OssDatqidn2ffY+Lc4YA5Fs78U+0KOHI51jQ", - "SHA256:DXdeUKYOJCSSmClZuwrb60hUq7367j4fA+udNC3FdRI", +type ldapTestEnv struct { + gitLDAPUsers []ldapUser + otherLDAPUsers []ldapUser + serverHost string + serverPort string +} + +func prepareLdapTestEnv(t *testing.T) *ldapTestEnv { + if os.Getenv("TEST_LDAP") != "1" { + t.Skip() + return nil + } + + gitLDAPUsers := []ldapUser{ + { + UserName: "professor", + Password: "professor", + FullName: "Hubert Farnsworth", + Email: "professor@planetexpress.com", + OtherEmails: []string{"hubert@planetexpress.com"}, + IsAdmin: true, + }, + { + UserName: "hermes", + Password: "hermes", + FullName: "Conrad Hermes", + Email: "hermes@planetexpress.com", + SSHKeys: []string{ + "SHA256:qLY06smKfHoW/92yXySpnxFR10QFrLdRjf/GNPvwcW8", + "SHA256:QlVTuM5OssDatqidn2ffY+Lc4YA5Fs78U+0KOHI51jQ", + "SHA256:DXdeUKYOJCSSmClZuwrb60hUq7367j4fA+udNC3FdRI", + }, + IsAdmin: true, + }, + { + UserName: "fry", + Password: "fry", + FullName: "Philip Fry", + Email: "fry@planetexpress.com", + }, + { + UserName: "leela", + Password: "leela", + FullName: "Leela Turanga", + Email: "leela@planetexpress.com", + IsRestricted: true, + }, + { + UserName: "bender", + Password: "bender", + FullName: "Bender Rodríguez", + Email: "bender@planetexpress.com", }, - IsAdmin: true, - }, - { - UserName: "fry", - Password: "fry", - FullName: "Philip Fry", - Email: "fry@planetexpress.com", - }, - { - UserName: "leela", - Password: "leela", - FullName: "Leela Turanga", - Email: "leela@planetexpress.com", - IsRestricted: true, - }, - { - UserName: "bender", - Password: "bender", - FullName: "Bender Rodríguez", - Email: "bender@planetexpress.com", - }, -} - -var otherLDAPUsers = []ldapUser{ - { - UserName: "zoidberg", - Password: "zoidberg", - FullName: "John Zoidberg", - Email: "zoidberg@planetexpress.com", - }, - { - UserName: "amy", - Password: "amy", - FullName: "Amy Kroker", - Email: "amy@planetexpress.com", - }, -} - -func skipLDAPTests() bool { - return os.Getenv("TEST_LDAP") != "1" -} - -func getLDAPServerHost() string { - host := os.Getenv("TEST_LDAP_HOST") - if len(host) == 0 { - host = "ldap" } - return host -} -func getLDAPServerPort() string { - port := os.Getenv("TEST_LDAP_PORT") - if len(port) == 0 { - port = "389" + otherLDAPUsers := []ldapUser{ + { + UserName: "zoidberg", + Password: "zoidberg", + FullName: "John Zoidberg", + Email: "zoidberg@planetexpress.com", + }, + { + UserName: "amy", + Password: "amy", + FullName: "Amy Kroker", + Email: "amy@planetexpress.com", + }, + } + + return &ldapTestEnv{ + gitLDAPUsers: gitLDAPUsers, + otherLDAPUsers: otherLDAPUsers, + serverHost: util.IfZero(os.Getenv("TEST_LDAP_HOST"), "ldap"), + serverPort: util.IfZero(os.Getenv("TEST_LDAP_PORT"), "389"), } - return port } -func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval string) map[string]string { +type ldapAuthOptions struct { + attributeUID optional.Option[string] // defaults to "uid" + attributeSSHPublicKey string + groupFilter string + groupTeamMap string + groupTeamMapRemoval string +} + +func (te *ldapTestEnv) buildAuthSourcePayload(csrf string, opts ...ldapAuthOptions) map[string]string { + opt := util.OptionalArg(opts) // Modify user filter to test group filter explicitly userFilter := "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))" - if groupFilter != "" { + if opt.groupFilter != "" { userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))" } @@ -123,53 +137,47 @@ func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap "_csrf": csrf, "type": "2", "name": "ldap", - "host": getLDAPServerHost(), - "port": getLDAPServerPort(), + "host": te.serverHost, + "port": te.serverPort, "bind_dn": "uid=gitea,ou=service,dc=planetexpress,dc=com", "bind_password": "password", "user_base": "ou=people,dc=planetexpress,dc=com", "filter": userFilter, "admin_filter": "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)", "restricted_filter": "(uid=leela)", - "attribute_username": "uid", + "attribute_username": util.Iif(opt.attributeUID.Has(), opt.attributeUID.Value(), "uid"), "attribute_name": "givenName", "attribute_surname": "sn", "attribute_mail": "mail", - "attribute_ssh_public_key": sshKeyAttribute, + "attribute_ssh_public_key": opt.attributeSSHPublicKey, "is_sync_enabled": "on", "is_active": "on", "groups_enabled": "on", "group_dn": "ou=people,dc=planetexpress,dc=com", "group_member_uid": "member", - "group_filter": groupFilter, - "group_team_map": groupTeamMap, - "group_team_map_removal": groupTeamMapRemoval, + "group_filter": opt.groupFilter, + "group_team_map": opt.groupTeamMap, + "group_team_map_removal": opt.groupTeamMapRemoval, "user_uid": "DN", } } -func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupMapParams ...string) { - groupTeamMapRemoval := "off" - groupTeamMap := "" - if len(groupMapParams) == 2 { - groupTeamMapRemoval = groupMapParams[0] - groupTeamMap = groupMapParams[1] - } +func (te *ldapTestEnv) addAuthSource(t *testing.T, opts ...ldapAuthOptions) { session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval)) + req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", te.buildAuthSourcePayload(csrf, opts...)) session.MakeRequest(t, req, http.StatusSeeOther) } func TestLDAPUserSignin(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) - u := gitLDAPUsers[0] + u := te.gitLDAPUsers[0] session := loginUserWithPassword(t, u.UserName, u.Password) req := NewRequest(t, "GET", "/user/settings") @@ -183,8 +191,13 @@ func TestLDAPUserSignin(t *testing.T) { } func TestLDAPAuthChange(t *testing.T) { + te := prepareLdapTestEnv(t) + if te == nil { + return + } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) session := loginUser(t, "user1") req := NewRequest(t, "GET", "/-/admin/auths") @@ -201,34 +214,35 @@ func TestLDAPAuthChange(t *testing.T) { doc = NewHTMLParser(t, resp.Body) csrf := doc.GetCSRF() host, _ := doc.Find(`input[name="host"]`).Attr("value") - assert.Equal(t, host, getLDAPServerHost()) + assert.Equal(t, te.serverHost, host) binddn, _ := doc.Find(`input[name="bind_dn"]`).Attr("value") assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn) - req = NewRequestWithValues(t, "POST", href, buildAuthSourceLDAPPayload(csrf, "", "", "", "off")) + req = NewRequestWithValues(t, "POST", href, te.buildAuthSourcePayload(csrf, ldapAuthOptions{groupTeamMapRemoval: "off"})) session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", href) resp = session.MakeRequest(t, req, http.StatusOK) doc = NewHTMLParser(t, resp.Body) host, _ = doc.Find(`input[name="host"]`).Attr("value") - assert.Equal(t, host, getLDAPServerHost()) + assert.Equal(t, te.serverHost, host) binddn, _ = doc.Find(`input[name="bind_dn"]`).Attr("value") assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn) } func TestLDAPUserSync(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) err := auth.SyncExternalUsers(context.Background(), true) assert.NoError(t, err) // Check if users exists - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { dbUser, err := user_model.GetUserByName(db.DefaultContext, gitLDAPUser.UserName) assert.NoError(t, err) assert.Equal(t, gitLDAPUser.UserName, dbUser.Name) @@ -238,27 +252,28 @@ func TestLDAPUserSync(t *testing.T) { } // Check if no users exist - for _, otherLDAPUser := range otherLDAPUsers { + for _, otherLDAPUser := range te.otherLDAPUsers { _, err := user_model.GetUserByName(db.DefaultContext, otherLDAPUser.UserName) assert.True(t, user_model.IsErrUserNotExist(err)) } } func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "") + payload := te.buildAuthSourcePayload(csrf) payload["attribute_username"] = "" req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload) session.MakeRequest(t, req, http.StatusSeeOther) - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { req := NewRequest(t, "GET", "/-/admin/users?q="+u.UserName) resp := session.MakeRequest(t, req, http.StatusOK) @@ -268,7 +283,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { assert.Equal(t, 0, tr.Length()) } - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{ "_csrf": csrf, "user_name": u.UserName, @@ -277,7 +292,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { MakeRequest(t, req, http.StatusSeeOther) } - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{ Name: payload["name"], @@ -285,9 +300,9 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { unittest.AssertCount(t, &user_model.User{ LoginType: auth_model.LDAP, LoginSource: authSource.ID, - }, len(gitLDAPUsers)) + }, len(te.gitLDAPUsers)) - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ Name: u.UserName, }) @@ -296,12 +311,13 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { } func TestLDAPUserSyncWithGroupFilter(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "(cn=git)") + te.addAuthSource(t, ldapAuthOptions{groupFilter: "(cn=git)"}) // Assert a user not a member of the LDAP group "cn=git" cannot login // This test may look like TestLDAPUserSigninFailed but it is not. @@ -309,20 +325,20 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { // This test is for the case when LDAP user records may not be linked with // all groups the user is a member of, the user filter is modified accordingly inside // the addAuthSourceLDAP based on the value of the groupFilter - u := otherLDAPUsers[0] + u := te.otherLDAPUsers[0] testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect")) - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) // Assert members of LDAP group "cn=git" are added - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { unittest.BeanExists(t, &user_model.User{ Name: gitLDAPUser.UserName, }) } // Assert everyone else is not added - for _, gitLDAPUser := range otherLDAPUsers { + for _, gitLDAPUser := range te.otherLDAPUsers { unittest.AssertNotExistsBean(t, &user_model.User{ Name: gitLDAPUser.UserName, }) @@ -333,11 +349,11 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { }) ldapConfig := ldapSource.Cfg.(*ldap.Source) ldapConfig.GroupFilter = "(cn=ship_crew)" - auth_model.UpdateSource(db.DefaultContext, ldapSource) + require.NoError(t, auth_model.UpdateSource(db.DefaultContext, ldapSource)) - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { if gitLDAPUser.UserName == "fry" || gitLDAPUser.UserName == "leela" || gitLDAPUser.UserName == "bender" { // Assert members of the LDAP group "cn-ship_crew" are still active user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ @@ -355,29 +371,31 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { } func TestLDAPUserSigninFailed(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } - defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") - u := otherLDAPUsers[0] + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t) + + u := te.otherLDAPUsers[0] testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect")) } func TestLDAPUserSSHKeySync(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } - defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "sshPublicKey", "") - auth.SyncExternalUsers(context.Background(), true) + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t, ldapAuthOptions{attributeSSHPublicKey: "sshPublicKey"}) + + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) // Check if users has SSH keys synced - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { if len(u.SSHKeys) == 0 { continue } @@ -400,18 +418,22 @@ func TestLDAPUserSSHKeySync(t *testing.T) { } func TestLDAPGroupTeamSyncAddMember(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "", "on", `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`) + te.addAuthSource(t, ldapAuthOptions{ + groupTeamMap: `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`, + groupTeamMapRemoval: "on", + }) org, err := organization.GetOrgByName(db.DefaultContext, "org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") assert.NoError(t, err) - auth.SyncExternalUsers(context.Background(), true) - for _, gitLDAPUser := range gitLDAPUsers { + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) + for _, gitLDAPUser := range te.gitLDAPUsers { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ Name: gitLDAPUser.UserName, }) @@ -445,19 +467,22 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) { } func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`) + te.addAuthSource(t, ldapAuthOptions{ + groupTeamMap: `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`, + groupTeamMapRemoval: "on", + }) org, err := organization.GetOrgByName(db.DefaultContext, "org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") assert.NoError(t, err) - loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password) + loginUserWithPassword(t, te.gitLDAPUsers[0].UserName, te.gitLDAPUsers[0].Password) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ - Name: gitLDAPUsers[0].UserName, + Name: te.gitLDAPUsers[0].UserName, }) err = organization.AddOrgUser(db.DefaultContext, org.ID, user.ID) assert.NoError(t, err) @@ -470,7 +495,7 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { assert.NoError(t, err) assert.True(t, isMember, "User should be member of this team") // assert team member "professor" gets removed from org26 team11 - loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password) + loginUserWithPassword(t, te.gitLDAPUsers[0].UserName, te.gitLDAPUsers[0].Password) isMember, err = organization.IsOrganizationMember(db.DefaultContext, org.ID, user.ID) assert.NoError(t, err) assert.False(t, isMember, "User membership should have been removed from organization") @@ -480,14 +505,67 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { } func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off")) + payload := te.buildAuthSourcePayload(csrf, ldapAuthOptions{groupTeamMap: `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, groupTeamMapRemoval: "off"}) + req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload) session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok } + +func TestLDAPEmailSignin(t *testing.T) { + te := ldapTestEnv{ + gitLDAPUsers: []ldapUser{ + { + UserName: "u1", + Password: "xx", + FullName: "user 1", + Email: "u1@gitea.com", + }, + }, + serverHost: "mock-host", + serverPort: "mock-port", + } + defer test.MockVariableValue(&ldap.MockedSearchEntry, func(source *ldap.Source, name, passwd string, directBind bool) *ldap.SearchResult { + var u *ldapUser + for _, user := range te.gitLDAPUsers { + if user.Email == name && user.Password == passwd { + u = &user + break + } + } + if u == nil { + return nil + } + result := &ldap.SearchResult{ + Username: u.UserName, + Mail: u.Email, + LowerName: strings.ToLower(u.UserName), + } + nameFields := strings.Split(u.FullName, " ") + result.Name = nameFields[0] + if len(nameFields) > 1 { + result.Surname = nameFields[1] + } + return result + })() + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t) + + u := te.gitLDAPUsers[0] + + session := loginUserWithPassword(t, u.Email, u.Password) + req := NewRequest(t, "GET", "/user/settings") + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + + assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name")) + assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name")) + assert.Equal(t, u.Email, htmlDoc.Find("#signed-user-email").Text()) +}