ldap: default domain name (#3414)

When the ldap synchronizer is look for an email address and fails at
finding one, it falls back at creating one using "localhost.local"
domain.

This new field makes this domain name configurable.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3414
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Baptiste Daroussin <bapt@FreeBSD.org>
Co-committed-by: Baptiste Daroussin <bapt@FreeBSD.org>

options/locale/locale_en-US.ini

@@ -3100,6 +3100,7 @@ auths.attribute_mail = Email attribute
 auths.attribute_ssh_public_key = Public SSH key attribute
 auths.attribute_avatar = Avatar attribute
 auths.attributes_in_bind = Fetch attributes in bind DN context
+auths.default_domain_name = Default domain name used for the email address
 auths.allow_deactivate_all = Allow an empty search result to deactivate all users
 auths.use_paged_search = Use paged search
 auths.search_page_size = Page size

release-notes/8.0.0/3414.md

@@ -0,0 +1 @@
+Allow to customize the domain name used as a fallback when synchronizing sources from ldap [`ldap: default domain name`](https://codeberg.org/forgejo/forgejo/pulls/3414)

routers/web/admin/auths.go

@@ -129,6 +129,7 @@ func parseLDAPConfig(form forms.AuthenticationForm) *ldap.Source {
 		UserDN:                form.UserDN,
 		BindPassword:          form.BindPassword,
 		UserBase:              form.UserBase,
+		DefaultDomainName:     form.DefaultDomainName,
 		AttributeUsername:     form.AttributeUsername,
 		AttributeName:         form.AttributeName,
 		AttributeSurname:      form.AttributeSurname,

services/auth/source/ldap/source.go

@@ -34,6 +34,7 @@ type Source struct {
 	BindPassword          string // Bind DN password
 	UserBase              string // Base search path for users
 	UserDN                string // Template for the DN of the user for simple auth
+	DefaultDomainName     string // DomainName used if none are in the field, default "localhost.local"
 	AttributeUsername     string // Username attribute
 	AttributeName         string // First name attribute
 	AttributeSurname      string // Surname attribute

services/auth/source/ldap/source_sync.go

@@ -105,7 +105,11 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
 		}
 
 		if len(su.Mail) == 0 {
-			su.Mail = fmt.Sprintf("%s@localhost.local", su.Username)
+			domainName := source.DefaultDomainName
+			if len(domainName) == 0 {
+				domainName = "localhost.local"
+			}
+			su.Mail = fmt.Sprintf("%s@%s", su.Username, domainName)
 		}
 
 		fullName := composeFullName(su.Name, su.Surname, su.Username)

services/forms/auth_form.go

@@ -26,6 +26,7 @@ type AuthenticationForm struct {
 	AttributeUsername             string
 	AttributeName                 string
 	AttributeSurname              string
+	DefaultDomainName             string
 	AttributeMail                 string
 	AttributeSSHPublicKey         string
 	AttributeAvatar               string

templates/admin/auth/edit.tmpl

@@ -97,6 +97,10 @@
 						<label for="attribute_mail">{{ctx.Locale.Tr "admin.auths.attribute_mail"}}</label>
 						<input id="attribute_mail" name="attribute_mail" value="{{$cfg.AttributeMail}}" placeholder="mail" required>
 					</div>
+					<div class="field">
+						<label for="default_domain_name">{{ctx.Locale.Tr "admin.auths.default_domain_name"}}</label>
+						<input id="default_domain_name" name="default_domain_name" value="{{$cfg.DefaultDomainName}}" placeholder="localhost.local" >
+					</div>
 					<div class="field">
 						<label for="attribute_ssh_public_key">{{ctx.Locale.Tr "admin.auths.attribute_ssh_public_key"}}</label>
 						<input id="attribute_ssh_public_key" name="attribute_ssh_public_key" value="{{$cfg.AttributeSSHPublicKey}}" placeholder="SshPublicKey">

templates/admin/auth/source/ldap.tmpl

@@ -71,6 +71,10 @@
 		<label for="attribute_mail">{{ctx.Locale.Tr "admin.auths.attribute_mail"}}</label>
 		<input id="attribute_mail" name="attribute_mail" value="{{.attribute_mail}}" placeholder="mail">
 	</div>
+	<div class="field">
+		<label for="default_domain_name">{{ctx.Locale.Tr "admin.auths.default_domain_name"}}</label>
+		<input id="default_domain_name" name="default_domain_name" value="{{.default_domain_name}}" placeholder="localhost.local">
+	</div>
 	<div class="field">
 		<label for="attribute_ssh_public_key">{{ctx.Locale.Tr "admin.auths.attribute_ssh_public_key"}}</label>
 		<input id="attribute_ssh_public_key" name="attribute_ssh_public_key" value="{{.attribute_ssh_public_key}}" placeholder="SshPublicKey">

tests/integration/auth_ldap_test.go

@@ -112,13 +112,17 @@ func getLDAPServerPort() string {
 	return port
 }
 
-func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval string) map[string]string {
+func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, mailKeyAttribute, defaultDomainName, groupFilter, groupTeamMap, groupTeamMapRemoval string) map[string]string {
 	// Modify user filter to test group filter explicitly
 	userFilter := "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))"
 	if groupFilter != "" {
 		userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))"
 	}
 
+	if len(mailKeyAttribute) == 0 {
+		mailKeyAttribute = "mail"
+	}
+
 	return map[string]string{
 		"_csrf":                    csrf,
 		"type":                     "2",
@@ -134,8 +138,9 @@ func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap
 		"attribute_username":       "uid",
 		"attribute_name":           "givenName",
 		"attribute_surname":        "sn",
-		"attribute_mail":           "mail",
+		"attribute_mail":           mailKeyAttribute,
 		"attribute_ssh_public_key": sshKeyAttribute,
+		"default_domain_name":      defaultDomainName,
 		"is_sync_enabled":          "on",
 		"is_active":                "on",
 		"groups_enabled":           "on",
@@ -148,7 +153,7 @@ func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap
 	}
 }
 
-func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupMapParams ...string) {
+func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, mailKeyAttribute, defaultDomainName, groupFilter string, groupMapParams ...string) {
 	groupTeamMapRemoval := "off"
 	groupTeamMap := ""
 	if len(groupMapParams) == 2 {
@@ -157,7 +162,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupM
 	}
 	session := loginUser(t, "user1")
 	csrf := GetCSRF(t, session, "/admin/auths/new")
-	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval))
+	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, mailKeyAttribute, defaultDomainName, groupFilter, groupTeamMap, groupTeamMapRemoval))
 	session.MakeRequest(t, req, http.StatusSeeOther)
 }
 
@@ -167,7 +172,7 @@ func TestLDAPUserSignin(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "")
+	addAuthSourceLDAP(t, "", "", "", "")
 
 	u := gitLDAPUsers[0]
 
@@ -184,7 +189,7 @@ func TestLDAPUserSignin(t *testing.T) {
 
 func TestLDAPAuthChange(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "")
+	addAuthSourceLDAP(t, "", "", "", "")
 
 	session := loginUser(t, "user1")
 	req := NewRequest(t, "GET", "/admin/auths")
@@ -205,7 +210,7 @@ func TestLDAPAuthChange(t *testing.T) {
 	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, buildAuthSourceLDAPPayload(csrf, "", "", "", "", "", "off"))
 	session.MakeRequest(t, req, http.StatusSeeOther)
 
 	req = NewRequest(t, "GET", href)
@@ -215,6 +220,21 @@ func TestLDAPAuthChange(t *testing.T) {
 	assert.Equal(t, host, getLDAPServerHost())
 	binddn, _ = doc.Find(`input[name="bind_dn"]`).Attr("value")
 	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn)
+	domainname, _ := doc.Find(`input[name="default_domain_name"]`).Attr("value")
+	assert.Equal(t, "", domainname)
+
+	req = NewRequestWithValues(t, "POST", href, buildAuthSourceLDAPPayload(csrf, "", "", "test.org", "", "", "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())
+	binddn, _ = doc.Find(`input[name="bind_dn"]`).Attr("value")
+	assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn)
+	domainname, _ = doc.Find(`input[name="default_domain_name"]`).Attr("value")
+	assert.Equal(t, "test.org", domainname)
 }
 
 func TestLDAPUserSync(t *testing.T) {
@@ -223,7 +243,7 @@ func TestLDAPUserSync(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "")
+	addAuthSourceLDAP(t, "", "", "", "")
 	auth.SyncExternalUsers(context.Background(), true)
 
 	// Check if users exists
@@ -252,7 +272,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
 
 	session := loginUser(t, "user1")
 	csrf := GetCSRF(t, session, "/admin/auths/new")
-	payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "")
+	payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "", "", "")
 	payload["attribute_username"] = ""
 	req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload)
 	session.MakeRequest(t, req, http.StatusSeeOther)
@@ -300,7 +320,7 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "(cn=git)")
+	addAuthSourceLDAP(t, "", "", "", "(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.
@@ -359,7 +379,7 @@ func TestLDAPUserSigninFailed(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "")
+	addAuthSourceLDAP(t, "", "", "", "")
 
 	u := otherLDAPUsers[0]
 	testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect"))
@@ -371,7 +391,7 @@ func TestLDAPUserSSHKeySync(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "sshPublicKey", "")
+	addAuthSourceLDAP(t, "sshPublicKey", "", "", "")
 
 	auth.SyncExternalUsers(context.Background(), true)
 
@@ -404,7 +424,7 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
 		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"]}}`)
+	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"]}}`)
 	org, err := organization.GetOrgByName(db.DefaultContext, "org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11")
@@ -449,7 +469,7 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`)
+	addAuthSourceLDAP(t, "", "", "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`)
 	org, err := organization.GetOrgByName(db.DefaultContext, "org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11")
@@ -487,6 +507,58 @@ func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) {
 
 	session := loginUser(t, "user1")
 	csrf := GetCSRF(t, session, "/admin/auths/new")
-	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off"))
+	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off"))
 	session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok
 }
+
+func TestLDAPUserSyncInvalidMail(t *testing.T) {
+	if skipLDAPTests() {
+		t.Skip()
+		return
+	}
+	defer tests.PrepareTestEnv(t)()
+	addAuthSourceLDAP(t, "", "nonexisting", "", "")
+	auth.SyncExternalUsers(context.Background(), true)
+
+	// Check if users exists
+	for _, gitLDAPUser := range gitLDAPUsers {
+		dbUser, err := user_model.GetUserByName(db.DefaultContext, gitLDAPUser.UserName)
+		assert.NoError(t, err)
+		assert.Equal(t, gitLDAPUser.UserName, dbUser.Name)
+		assert.Equal(t, gitLDAPUser.UserName+"@localhost.local", dbUser.Email)
+		assert.Equal(t, gitLDAPUser.IsAdmin, dbUser.IsAdmin)
+		assert.Equal(t, gitLDAPUser.IsRestricted, dbUser.IsRestricted)
+	}
+
+	// Check if no users exist
+	for _, otherLDAPUser := range otherLDAPUsers {
+		_, err := user_model.GetUserByName(db.DefaultContext, otherLDAPUser.UserName)
+		assert.True(t, user_model.IsErrUserNotExist(err))
+	}
+}
+
+func TestLDAPUserSyncInvalidMailDefaultDomain(t *testing.T) {
+	if skipLDAPTests() {
+		t.Skip()
+		return
+	}
+	defer tests.PrepareTestEnv(t)()
+	addAuthSourceLDAP(t, "", "nonexisting", "test.org", "")
+	auth.SyncExternalUsers(context.Background(), true)
+
+	// Check if users exists
+	for _, gitLDAPUser := range gitLDAPUsers {
+		dbUser, err := user_model.GetUserByName(db.DefaultContext, gitLDAPUser.UserName)
+		assert.NoError(t, err)
+		assert.Equal(t, gitLDAPUser.UserName, dbUser.Name)
+		assert.Equal(t, gitLDAPUser.UserName+"@test.org", dbUser.Email)
+		assert.Equal(t, gitLDAPUser.IsAdmin, dbUser.IsAdmin)
+		assert.Equal(t, gitLDAPUser.IsRestricted, dbUser.IsRestricted)
+	}
+
+	// Check if no users exist
+	for _, otherLDAPUser := range otherLDAPUsers {
+		_, err := user_model.GetUserByName(db.DefaultContext, otherLDAPUser.UserName)
+		assert.True(t, user_model.IsErrUserNotExist(err))
+	}
+}