Refactor LDAP tests (#37274)

Not really fix #37263, just make things better, and easy to catch more
clues if it would fail again.

modules/testlogger/testlogger.go

@@ -90,8 +90,8 @@ func (w *testLoggerWriterCloser) Reset() {
 	w.Unlock()
 }
 
-// Printf takes a format and args and prints the string to os.Stdout
+// stdoutPrintf takes a format and args and prints the string to os.Stdout
-func Printf(format string, args ...any) {
+func stdoutPrintf(format string, args ...any) {
 	if !log.CanColorStdout {
 		for i := range args {
 			if c, ok := args[i].(*log.ColoredValue); ok {
@@ -118,20 +118,20 @@ func PrintCurrentTest(t testing.TB, skip ...int) func() {
 	deferHasRun := false
 	t.Cleanup(func() {
 		if !deferHasRun {
-			Printf("!!! %s defer function hasn't been run but Cleanup is called, usually caused by panic", t.Name())
+			stdoutPrintf("!!! %s defer function hasn't been run but Cleanup is called, usually caused by panic\n", t.Name())
 		}
 	})
-	Printf("=== %s (%s:%d)\n", log.NewColoredValue(t.Name()), strings.TrimPrefix(filename, prefix), line)
+	stdoutPrintf("=== %s (%s:%d)\n", log.NewColoredValue(t.Name()), strings.TrimPrefix(filename, prefix), line)
 
 	WriterCloser.pushT(t)
 	timeoutChecker := time.AfterFunc(TestTimeout, func() {
-		Printf("!!! %s ... timeout: %v ... stacktrace:\n%s\n\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestTimeout, getRuntimeStackAll())
+		stdoutPrintf("!!! %s ... timeout: %v ... stacktrace:\n%s\n\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestTimeout, getRuntimeStackAll())
 	})
 	return func() {
 		deferHasRun = true
 		flushStart := time.Now()
 		slowFlushChecker := time.AfterFunc(TestSlowFlush, func() {
-			Printf("+++ %s ... still flushing after %v ...\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestSlowFlush)
+			stdoutPrintf("+++ %s ... still flushing after %v ...\n", log.NewColoredValue(t.Name(), log.Bold, log.FgRed), TestSlowFlush)
 		})
 		if err := queue.GetManager().FlushAll(t.Context(), -1); err != nil {
 			// if panic occurs, then the t.Context() is also cancelled ahead, so here it shows "context canceled" error.
@@ -143,7 +143,7 @@ func PrintCurrentTest(t testing.TB, skip ...int) func() {
 		runDuration := time.Since(runStart)
 		flushDuration := time.Since(flushStart)
 		if runDuration > TestSlowRun {
-			Printf("+++ %s is a slow test (run: %v, flush: %v)\n", log.NewColoredValue(t.Name(), log.Bold, log.FgYellow), runDuration, flushDuration)
+			stdoutPrintf("+++ %s is a slow test (run: %v, flush: %v)\n", log.NewColoredValue(t.Name(), log.Bold, log.FgYellow), runDuration, flushDuration)
 		}
 		WriterCloser.popT()
 	}

routers/web/admin/auths.go

@@ -440,7 +440,7 @@ func EditAuthSourcePost(ctx *context.Context) {
 	log.Trace("Authentication changed by admin(%s): %d", ctx.Doer.Name, source.ID)
 
 	ctx.Flash.Success(ctx.Tr("admin.auths.update_success"))
-	ctx.Redirect(setting.AppSubURL + "/-/admin/auths/" + strconv.FormatInt(form.ID, 10))
+	ctx.Redirect(setting.AppSubURL + "/-/admin/auths/" + strconv.FormatInt(source.ID, 10))
 }
 
 // DeleteAuthSource response for deleting an auth source

services/forms/auth_form.go

@@ -14,7 +14,6 @@ import (
 
 // AuthenticationForm form for authentication
 type AuthenticationForm struct {
-	ID              int64
 	Type            int    `binding:"Range(2,7)"`
 	Name            string `binding:"Required;MaxSize(30)"`
 	TwoFactorPolicy string

tests/integration/auth_ldap_test.go

@@ -4,8 +4,10 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
 	"os"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -14,7 +16,6 @@ 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/structs"
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/translation"
@@ -46,12 +47,29 @@ type ldapTestEnv struct {
 	serverPort     string
 }
 
-func prepareLdapTestEnv(t *testing.T) *ldapTestEnv {
+func TestAuthLDAP(t *testing.T) {
-	if os.Getenv("TEST_LDAP") != "1" {
+	// To test it locally:
-		t.Skip()
+	// $ docker run --rm gitea/test-openldap:latest -p 389:389
-		return nil
+	// $ TEST_LDAP=1 TEST_LDAP_HOST=localhost make "test-sqlite#TestAuthLDAP"
-	}
+	defer tests.PrepareTestEnv(t)()
+	t.Run("PreventInvalidGroupTeamMap", testLDAPPreventInvalidGroupTeamMap)
+	t.Run("AuthChange", testLDAPAuthChange)
+	t.Run("EmailSignin", testLDAPEmailSignin)
 
+	hasRealServer, _ := strconv.ParseBool(os.Getenv("TEST_LDAP"))
+	if hasRealServer {
+		t.Run("UserSignin", testLDAPUserSignin)
+		t.Run("UserSyncWithAttributeUsername", testLDAPUserSyncWithAttributeUsername)
+		t.Run("UserSyncWithoutAttributeUsername", testLDAPUserSyncWithoutAttributeUsername)
+		t.Run("UserSyncSSHKeys", testLDAPUserSyncSSHKeys)
+		t.Run("UserSyncWithGroupFilter", testLDAPUserSyncWithGroupFilter)
+
+		t.Run("GroupTeamSyncAddMember", testLDAPGroupTeamSyncAddMember)
+		t.Run("GroupTeamSyncRemoveMember", testLDAPGroupTeamSyncRemoveMember)
+	}
+}
+
+func prepareLdapTestServerEnv() *ldapTestEnv {
 	gitLDAPUsers := []ldapUser{
 		{
 			UserName:    "professor",
@@ -117,23 +135,8 @@ func prepareLdapTestEnv(t *testing.T) *ldapTestEnv {
 	}
 }
 
-type ldapAuthOptions struct {
+func (te *ldapTestEnv) buildAuthSourcePayload(m map[string]string) map[string]string {
-	attributeUID          optional.Option[string] // defaults to "uid"
+	ret := map[string]string{
-	attributeSSHPublicKey string
-	groupFilter           string
-	groupTeamMap          string
-	groupTeamMapRemoval   string
-}
-
-func (te *ldapTestEnv) buildAuthSourcePayload(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 opt.groupFilter != "" {
-		userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))"
-	}
-
-	return map[string]string{
 		"type":                     "2",
 		"name":                     "ldap",
 		"host":                     te.serverHost,
@@ -141,107 +144,119 @@ func (te *ldapTestEnv) buildAuthSourcePayload(opts ...ldapAuthOptions) map[strin
 		"bind_dn":                  "uid=gitea,ou=service,dc=planetexpress,dc=com",
 		"bind_password":            "password",
 		"user_base":                "ou=people,dc=planetexpress,dc=com",
-		"filter":                   userFilter,
+		"filter":                   "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))",
 		"admin_filter":             "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)",
 		"restricted_filter":        "(uid=leela)",
-		"attribute_username":       util.Iif(opt.attributeUID.Has(), opt.attributeUID.Value(), "uid"),
+		"attribute_username":       "uid",
 		"attribute_name":           "givenName",
 		"attribute_surname":        "sn",
 		"attribute_mail":           "mail",
-		"attribute_ssh_public_key": opt.attributeSSHPublicKey,
+		"attribute_ssh_public_key": "",
 		"is_sync_enabled":          "on",
 		"is_active":                "on",
 		"groups_enabled":           "on",
 		"group_dn":                 "ou=people,dc=planetexpress,dc=com",
 		"group_member_uid":         "member",
-		"group_filter":             opt.groupFilter,
+		"group_filter":             "",
-		"group_team_map":           opt.groupTeamMap,
+		"group_team_map":           "",
-		"group_team_map_removal":   opt.groupTeamMapRemoval,
+		"group_team_map_removal":   "",
 		"user_uid":                 "DN",
 	}
+	for k, v := range m {
+		if _, ok := ret[k]; !ok {
+			panic("invalid key: " + k)
+		}
+		ret[k] = v
+	}
+	return ret
 }
 
-func (te *ldapTestEnv) addAuthSource(t *testing.T, opts ...ldapAuthOptions) {
+func (te *ldapTestEnv) setupAuthSource(t *testing.T, params map[string]string) {
 	session := loginUser(t, "user1")
-	req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", te.buildAuthSourcePayload(opts...))
+	existing := &auth_model.Source{Name: params["name"]}
-	session.MakeRequest(t, req, http.StatusSeeOther)
+	if ok, _ := db.GetEngine(t.Context()).Get(existing); ok {
+		req := NewRequestWithValues(t, "POST", fmt.Sprintf("/-/admin/auths/%d", existing.ID), params)
+		session.MakeRequest(t, req, http.StatusSeeOther)
+	} else {
+		req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", params)
+		session.MakeRequest(t, req, http.StatusSeeOther)
+	}
 }
 
-func TestLDAPUserSignin(t *testing.T) {
+func testLDAPUserSignin(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	te.setupAuthSource(t, te.buildAuthSourcePayload(nil))
-	}
-	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t)
 
-	u := te.gitLDAPUsers[0]
+	t.Run("Success", func(t *testing.T) {
-
+		u := te.gitLDAPUsers[0]
-	session := loginUserWithPassword(t, u.UserName, u.Password)
+		session := loginUserWithPassword(t, u.UserName, u.Password)
-	req := NewRequest(t, "GET", "/user/settings")
+		req := NewRequest(t, "GET", "/user/settings")
-	resp := session.MakeRequest(t, req, http.StatusOK)
+		resp := session.MakeRequest(t, req, http.StatusOK)
-
+		htmlDoc := NewHTMLParser(t, resp.Body)
-	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.UserName, htmlDoc.GetInputValueByName("name"))
+		assert.Equal(t, u.Email, htmlDoc.Find("#signed-user-email").Text())
-	assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name"))
+	})
-	assert.Equal(t, u.Email, htmlDoc.Find("#signed-user-email").Text())
+	t.Run("Failed", func(t *testing.T) {
+		u := te.otherLDAPUsers[0]
+		testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect"))
+	})
 }
 
-func TestLDAPAuthChange(t *testing.T) {
+func testLDAPAuthChange(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	te.setupAuthSource(t, te.buildAuthSourcePayload(nil))
-	}
-
-	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t)
 
 	session := loginUser(t, "user1")
 	req := NewRequest(t, "GET", "/-/admin/auths")
 	resp := session.MakeRequest(t, req, http.StatusOK)
-	doc := NewHTMLParser(t, resp.Body)
+	respStr := resp.Body.String()
-	href, exists := doc.Find("table.table td a").Attr("href")
+	doc := NewHTMLParser(t, strings.NewReader(respStr))
-	if !exists {
+	hrefAuthSource, exists := doc.Find("table.table td a").Attr("href")
-		assert.True(t, exists, "No authentication source found")
+	if !assert.True(t, exists, "No authentication source found") {
+		t.Logf("response: %s", respStr)
 		return
 	}
 
-	req = NewRequest(t, "GET", href)
+	req = NewRequest(t, "GET", hrefAuthSource)
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	doc = NewHTMLParser(t, resp.Body)
 	host, _ := doc.Find(`input[name="host"]`).Attr("value")
 	assert.Equal(t, te.serverHost, host)
-	binddn, _ := doc.Find(`input[name="bind_dn"]`).Attr("value")
+	bindDN, _ := doc.Find(`input[name="bind_dn"]`).Attr("value")
-	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn)
+	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", bindDN)
 
-	req = NewRequestWithValues(t, "POST", href, te.buildAuthSourcePayload(ldapAuthOptions{groupTeamMapRemoval: "off"}))
+	req = NewRequestWithValues(t, "POST", hrefAuthSource, te.buildAuthSourcePayload(map[string]string{"group_team_map_removal": "off"}))
 	session.MakeRequest(t, req, http.StatusSeeOther)
 
-	req = NewRequest(t, "GET", href)
+	req = NewRequest(t, "GET", hrefAuthSource)
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	doc = NewHTMLParser(t, resp.Body)
 	host, _ = doc.Find(`input[name="host"]`).Attr("value")
 	assert.Equal(t, te.serverHost, host)
-	binddn, _ = doc.Find(`input[name="bind_dn"]`).Attr("value")
+	bindDN, _ = doc.Find(`input[name="bind_dn"]`).Attr("value")
-	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn)
+	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", bindDN)
 }
 
-func TestLDAPUserSync(t *testing.T) {
+func testLDAPUserSyncWithAttributeUsername(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	te.setupAuthSource(t, te.buildAuthSourcePayload(map[string]string{"attribute_username": "uid"}))
-	}
+
+	// reset user and email table
+	_, _ = db.GetEngine(t.Context()).Where("`name` != 'user1'").Delete(&user_model.User{})
+	_ = db.TruncateBeans(t.Context(), &user_model.EmailAddress{})
+	unittest.AssertCount(t, &user_model.User{}, 1)
 
-	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t)
 	err := auth.SyncExternalUsers(t.Context(), true)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	// Check if users exists
 	for _, gitLDAPUser := range te.gitLDAPUsers {
 		dbUser, err := user_model.GetUserByName(t.Context(), gitLDAPUser.UserName)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, gitLDAPUser.UserName, dbUser.Name)
 		assert.Equal(t, gitLDAPUser.Email, dbUser.Email)
 		assert.Equal(t, gitLDAPUser.IsAdmin, dbUser.IsAdmin)
@@ -255,23 +270,21 @@ func TestLDAPUserSync(t *testing.T) {
 	}
 }
 
-func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
+func testLDAPUserSyncWithoutAttributeUsername(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	authParams := te.buildAuthSourcePayload(map[string]string{"attribute_username": ""})
-	}
+	te.setupAuthSource(t, authParams)
 
-	defer tests.PrepareTestEnv(t)()
+	// reset user and email table
-
+	_, _ = db.GetEngine(t.Context()).Where("`name` != 'user1'").Delete(&user_model.User{})
-	session := loginUser(t, "user1")
+	_ = db.TruncateBeans(t.Context(), &user_model.EmailAddress{})
-	payload := te.buildAuthSourcePayload()
+	unittest.AssertCount(t, &user_model.User{}, 1)
-	payload["attribute_username"] = ""
-	req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload)
-	session.MakeRequest(t, req, http.StatusSeeOther)
 
+	adminSession := loginUser(t, "user1")
 	for _, u := range te.gitLDAPUsers {
 		req := NewRequest(t, "GET", "/-/admin/users?q="+u.UserName)
-		resp := session.MakeRequest(t, req, http.StatusOK)
+		resp := adminSession.MakeRequest(t, req, http.StatusOK)
 
 		htmlDoc := NewHTMLParser(t, resp.Body)
 
@@ -289,9 +302,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
 
 	require.NoError(t, auth.SyncExternalUsers(t.Context(), true))
 
-	authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{
+	authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{Name: authParams["name"]})
-		Name: payload["name"],
-	})
 	unittest.AssertCount(t, &user_model.User{
 		LoginType:   auth_model.LDAP,
 		LoginSource: authSource.ID,
@@ -305,14 +316,13 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
 	}
 }
 
-func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
+func testLDAPUserSyncWithGroupFilter(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	te.setupAuthSource(t, te.buildAuthSourcePayload(map[string]string{
-	}
+		"filter":       "(&(objectClass=inetOrgPerson)(uid=%s))",
-
+		"group_filter": "(cn=git)",
-	defer tests.PrepareTestEnv(t)()
+	}))
-	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.
@@ -365,64 +375,43 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
 	}
 }
 
-func TestLDAPUserSigninFailed(t *testing.T) {
+func testLDAPUserSyncSSHKeys(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
+	te.setupAuthSource(t, te.buildAuthSourcePayload(map[string]string{"attribute_ssh_public_key": "sshPublicKey"}))
-	}
-
-	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) {
-	te := prepareLdapTestEnv(t)
-	if te == nil {
-		return
-	}
-
-	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t, ldapAuthOptions{attributeSSHPublicKey: "sshPublicKey"})
 
 	require.NoError(t, auth.SyncExternalUsers(t.Context(), true))
 
 	// Check if users has SSH keys synced
+	count := 0
 	for _, u := range te.gitLDAPUsers {
 		if len(u.SSHKeys) == 0 {
 			continue
 		}
-		session := loginUserWithPassword(t, u.UserName, u.Password)
+		count++
 
+		session := loginUserWithPassword(t, u.UserName, u.Password)
 		req := NewRequest(t, "GET", "/user/settings/keys")
 		resp := session.MakeRequest(t, req, http.StatusOK)
-
 		htmlDoc := NewHTMLParser(t, resp.Body)
 
 		divs := htmlDoc.doc.Find("#keys-ssh .flex-item .flex-item-body:not(:last-child)")
-
 		syncedKeys := make([]string, divs.Length())
 		for i := 0; i < divs.Length(); i++ {
 			syncedKeys[i] = strings.TrimSpace(divs.Eq(i).Text())
 		}
-
 		assert.ElementsMatch(t, u.SSHKeys, syncedKeys, "Unequal number of keys synchronized for user: %s", u.UserName)
 	}
+	assert.NotZero(t, count)
 }
 
-func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
+func testLDAPGroupTeamSyncAddMember(t *testing.T) {
-	te := prepareLdapTestEnv(t)
-	if te == nil {
-		return
-	}
-
 	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t, ldapAuthOptions{
+	te := prepareLdapTestServerEnv()
-		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"]}}`,
+	te.setupAuthSource(t, te.buildAuthSourcePayload(map[string]string{
-		groupTeamMapRemoval: "on",
+		"group_team_map":         `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`,
-	})
+		"group_team_map_removal": "on",
+	}))
 	org, err := organization.GetOrgByName(t.Context(), "org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(t.Context(), org.ID, "team11")
@@ -461,16 +450,14 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
 	}
 }
 
-func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) {
+func testLDAPGroupTeamSyncRemoveMember(t *testing.T) {
-	te := prepareLdapTestEnv(t)
-	if te == nil {
-		return
-	}
 	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t, ldapAuthOptions{
+	te := prepareLdapTestServerEnv()
-		groupTeamMap:        `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`,
+	te.setupAuthSource(t, te.buildAuthSourcePayload(map[string]string{
-		groupTeamMapRemoval: "on",
+		"group_team_map":         `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`,
-	})
+		"group_team_map_removal": "on",
+	}))
+
 	org, err := organization.GetOrgByName(t.Context(), "org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(t.Context(), org.ID, "team11")
@@ -499,20 +486,18 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) {
 	assert.False(t, isMember, "User membership should have been removed from team")
 }
 
-func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) {
+func testLDAPPreventInvalidGroupTeamMap(t *testing.T) {
-	te := prepareLdapTestEnv(t)
+	defer tests.PrintCurrentTest(t)()
-	if te == nil {
+	te := prepareLdapTestServerEnv()
-		return
-	}
-	defer tests.PrepareTestEnv(t)()
 
 	session := loginUser(t, "user1")
-	payload := te.buildAuthSourcePayload(ldapAuthOptions{groupTeamMap: `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, groupTeamMapRemoval: "off"})
+	payload := te.buildAuthSourcePayload(map[string]string{"group_team_map": `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "group_team_map_removal": "off"})
 	req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload)
 	session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok
 }
 
-func TestLDAPEmailSignin(t *testing.T) {
+func testLDAPEmailSignin(t *testing.T) {
+	defer tests.PrintCurrentTest(t)()
 	te := ldapTestEnv{
 		gitLDAPUsers: []ldapUser{
 			{
@@ -549,7 +534,7 @@ func TestLDAPEmailSignin(t *testing.T) {
 		return result
 	})()
 	defer tests.PrepareTestEnv(t)()
-	te.addAuthSource(t)
+	te.setupAuthSource(t, te.buildAuthSourcePayload(nil))
 
 	u := te.gitLDAPUsers[0]
 

tests/integration/html_helper.go

@@ -4,7 +4,7 @@
 package integration
 
 import (
-	"bytes"
+	"io"
 	"testing"
 
 	"github.com/PuerkitoBio/goquery"
@@ -17,7 +17,7 @@ type HTMLDoc struct {
 }
 
 // NewHTMLParser parse html file
-func NewHTMLParser(t testing.TB, body *bytes.Buffer) *HTMLDoc {
+func NewHTMLParser(t testing.TB, body io.Reader) *HTMLDoc {
 	t.Helper()
 	doc, err := goquery.NewDocumentFromReader(body)
 	assert.NoError(t, err)

tests/integration/integration_test.go

@@ -404,7 +404,7 @@ func logUnexpectedResponse(t testing.TB, recorder *httptest.ResponseRecorder) {
 	if err != nil {
 		return // probably a non-HTML response
 	}
-	errMsg := htmlDoc.Find(".ui.negative.message").Text()
+	errMsg := htmlDoc.Find(".ui.negative.message:not(.tw-hidden)").Text()
 	if len(errMsg) > 0 {
 		t.Log("A flash error message was found:", errMsg)
 	}