From 08f5a25d3b8104c6e73b7af32e4c0d23813fb539 Mon Sep 17 00:00:00 2001 From: Baptiste Daroussin Date: Fri, 26 Apr 2024 22:38:58 +0000 Subject: [PATCH] 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 Co-authored-by: Baptiste Daroussin Co-committed-by: Baptiste Daroussin --- options/locale/locale_en-US.ini | 1 + release-notes/8.0.0/3414.md | 1 + routers/web/admin/auths.go | 1 + services/auth/source/ldap/source.go | 1 + services/auth/source/ldap/source_sync.go | 6 +- services/forms/auth_form.go | 1 + templates/admin/auth/edit.tmpl | 4 + templates/admin/auth/source/ldap.tmpl | 4 + tests/integration/auth_ldap_test.go | 102 +++++++++++++++++++---- 9 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 release-notes/8.0.0/3414.md diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d7680f3545..7125d97d9b 100644 --- a/options/locale/locale_en-US.ini +++ b/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 diff --git a/release-notes/8.0.0/3414.md b/release-notes/8.0.0/3414.md new file mode 100644 index 0000000000..2e10483e23 --- /dev/null +++ b/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) diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index ba487d1045..799b7e8a84 100644 --- a/routers/web/admin/auths.go +++ b/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, diff --git a/services/auth/source/ldap/source.go b/services/auth/source/ldap/source.go index dc4cb2c940..ba407b351a 100644 --- a/services/auth/source/ldap/source.go +++ b/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 diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 62f052d68c..7c5d3da595 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/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) diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index c9f3182b3a..a3eca9473b 100644 --- a/services/forms/auth_form.go +++ b/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 diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 687a277a84..8a8bd61148 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -97,6 +97,10 @@ +
+ + +
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 680849c8ea..6cb6643f26 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -71,6 +71,10 @@
+
+ + +
diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 3a5fdb97a6..06677287c0 100644 --- a/tests/integration/auth_ldap_test.go +++ b/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)) + } +}