From c79f884588f073e423fd9a7b13e877b7fb6505cf Mon Sep 17 00:00:00 2001 From: jolheiser Date: Sat, 28 Mar 2020 14:56:56 -0500 Subject: [PATCH 1/6] Port Gogs LDAP group verification Signed-off-by: jolheiser --- modules/auth/auth_form.go | 5 ++ modules/auth/ldap/README.md | 18 +++++++ modules/auth/ldap/ldap.go | 77 +++++++++++++++++++++++++-- routers/admin/auths.go | 5 ++ templates/admin/auth/edit.tmpl | 22 ++++++++ templates/admin/auth/source/ldap.tmpl | 22 ++++++++ 6 files changed, 146 insertions(+), 3 deletions(-) diff --git a/modules/auth/auth_form.go b/modules/auth/auth_form.go index 7fc62607e5215..1d02c7acf3647 100644 --- a/modules/auth/auth_form.go +++ b/modules/auth/auth_form.go @@ -30,6 +30,11 @@ type AuthenticationForm struct { SearchPageSize int Filter string AdminFilter string + GroupsEnabled bool + GroupDN string + GroupFilter string + GroupMemberUID string + UserUID string RestrictedFilter string AllowDeactivateAll bool IsActive bool diff --git a/modules/auth/ldap/README.md b/modules/auth/ldap/README.md index 4f7961da6b0de..226acac80ceb9 100644 --- a/modules/auth/ldap/README.md +++ b/modules/auth/ldap/README.md @@ -103,3 +103,21 @@ share the following fields: matching parameter will be substituted with the user's username. * Example: (&(objectClass=posixAccount)(cn=%s)) * Example: (&(objectClass=posixAccount)(uid=%s)) + +**Verify group membership in LDAP** uses the following fields: + +* Group Search Base (optional) + * The LDAP DN used for groups. + * Example: ou=group,dc=mydomain,dc=com + +* Group Name Filter (optional) + * An LDAP filter declaring how to find valid groups in the above DN. + * Example: (|(cn=gogs_users)(cn=admins)) + +* User Attribute in Group (optional) + * Which user LDAP attribute is listed in the group. + * Example: uid + +* Group Attribute for User (optional) + * Which group LDAP attribute contains an array above user attribute names. + * Example: memberUid diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 66676f2829d56..5ee0623a74e1c 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -1,4 +1,5 @@ // Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2020 The Gitea Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -13,7 +14,7 @@ import ( "code.gitea.io/gitea/modules/log" - ldap "gopkg.in/ldap.v3" + "gopkg.in/ldap.v3" ) // SecurityProtocol protocol type @@ -49,6 +50,11 @@ type Source struct { RestrictedFilter string // Query filter to check if user is restricted Enabled bool // if this source is disabled AllowDeactivateAll bool // Allow an empty search response to deactivate all users from this source + GroupsEnabled bool // if the group checking is enabled + GroupDN string // Group Search Base + GroupFilter string // Group Name Filter + GroupMemberUID string // Group Attribute containing array of UserUID + UserUID string // User Attribute listed in Group } // SearchResult : user data @@ -84,6 +90,28 @@ func (ls *Source) sanitizedUserDN(username string) (string, bool) { return fmt.Sprintf(ls.UserDN, username), true } +func (ls *Source) sanitizedGroupFilter(group string) (string, bool) { + // See http://tools.ietf.org/search/rfc4515 + badCharacters := "\x00*\\" + if strings.ContainsAny(group, badCharacters) { + log.Trace("Group filter invalid query characters: %s", group) + return "", false + } + + return group, true +} + +func (ls *Source) sanitizedGroupDN(groupDn string) (string, bool) { + // See http://tools.ietf.org/search/rfc4514: "special characters" + badCharacters := "\x00()*\\'\"#+;<>" + if strings.ContainsAny(groupDn, badCharacters) || strings.HasPrefix(groupDn, " ") || strings.HasSuffix(groupDn, " ") { + log.Trace("Group DN contains invalid query characters: %s", groupDn) + return "", false + } + + return groupDn, true +} + func (ls *Source) findUserDN(l *ldap.Conn, name string) (string, bool) { log.Trace("Search for LDAP user: %s", name) @@ -278,12 +306,12 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul var isAttributeSSHPublicKeySet = len(strings.TrimSpace(ls.AttributeSSHPublicKey)) > 0 - attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail} + attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.UserUID} if isAttributeSSHPublicKeySet { attribs = append(attribs, ls.AttributeSSHPublicKey) } - log.Trace("Fetching attributes '%v', '%v', '%v', '%v', '%v' with filter %s and base %s", ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.AttributeSSHPublicKey, userFilter, userDN) + log.Trace("Fetching attributes '%v', '%v', '%v', '%v', '%v' with filter '%s' and base '%s'", ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.UserUID, userFilter, userDN) search := ldap.NewSearchRequest( userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, userFilter, attribs, nil) @@ -308,6 +336,49 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul firstname := sr.Entries[0].GetAttributeValue(ls.AttributeName) surname := sr.Entries[0].GetAttributeValue(ls.AttributeSurname) mail := sr.Entries[0].GetAttributeValue(ls.AttributeMail) + uid := sr.Entries[0].GetAttributeValue(ls.UserUID) + + // Check group membership + if ls.GroupsEnabled { + groupFilter, ok := ls.sanitizedGroupFilter(ls.GroupFilter) + if !ok { + return nil + } + groupDN, ok := ls.sanitizedGroupDN(ls.GroupDN) + if !ok { + return nil + } + + log.Trace("Fetching groups '%v' with filter '%s' and base '%s'", ls.GroupMemberUid, groupFilter, groupDN) + groupSearch := ldap.NewSearchRequest( + groupDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, + []string{ls.GroupMemberUid}, + nil) + + srg, err := l.Search(groupSearch) + if err != nil { + log.Error("LDAP group search failed: %v", err) + return nil + } else if len(sr.Entries) < 1 { + log.Error("LDAP group search failed: 0 entries") + return nil + } + + isMember := false + for _, group := range srg.Entries { + for _, member := range group.GetAttributeValues(ls.GroupMemberUid) { + if member == uid { + isMember = true + } + } + } + + if !isMember { + log.Error("LDAP group membership test failed") + return nil + } + } + if isAttributeSSHPublicKeySet { sshPublicKey = sr.Entries[0].GetAttributeValues(ls.AttributeSSHPublicKey) } diff --git a/routers/admin/auths.go b/routers/admin/auths.go index a4fd5290b74ae..c6d6db73cd106 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -129,6 +129,11 @@ func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { AttributeSSHPublicKey: form.AttributeSSHPublicKey, SearchPageSize: pageSize, Filter: form.Filter, + GroupsEnabled: form.GroupsEnabled, + GroupDN: form.GroupDN, + GroupFilter: form.GroupFilter, + GroupMemberUid: form.GroupMemberUid, + UserUID: form.UserUID, AdminFilter: form.AdminFilter, RestrictedFilter: form.RestrictedFilter, AllowDeactivateAll: form.AllowDeactivateAll, diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 15ab2b227bd79..df9e18a881f9c 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -99,6 +99,28 @@ +
+
+ + +
+
+
+ + +
+
+ + +
+
+ + +
+
+ + +
{{if .Source.IsLDAP}}
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index f5806c829c54d..1a245baf56c45 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -71,6 +71,28 @@
+
+
+ + +
+
+
+ + +
+
+ + +
+
+ + +
+
+ + +
From 97c0e8fe3ae42cc2e67f07eb3f2e631edcf0a963 Mon Sep 17 00:00:00 2001 From: jolheiser Date: Sat, 28 Mar 2020 15:29:27 -0500 Subject: [PATCH 2/6] Appease linter and add some extra polish Signed-off-by: jolheiser --- modules/auth/ldap/ldap.go | 6 ++--- options/locale/locale_en-US.ini | 5 ++++ routers/admin/auths.go | 2 +- templates/admin/auth/edit.tmpl | 35 +++++++++++++------------ templates/admin/auth/source/ldap.tmpl | 37 +++++++++++++++------------ web_src/js/index.js | 13 ++++++++++ 6 files changed, 61 insertions(+), 37 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 5ee0623a74e1c..bfe6e7a1850dd 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -349,10 +349,10 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul return nil } - log.Trace("Fetching groups '%v' with filter '%s' and base '%s'", ls.GroupMemberUid, groupFilter, groupDN) + log.Trace("Fetching groups '%v' with filter '%s' and base '%s'", ls.GroupMemberUID, groupFilter, groupDN) groupSearch := ldap.NewSearchRequest( groupDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, - []string{ls.GroupMemberUid}, + []string{ls.GroupMemberUID}, nil) srg, err := l.Search(groupSearch) @@ -366,7 +366,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul isMember := false for _, group := range srg.Entries { - for _, member := range group.GetAttributeValues(ls.GroupMemberUid) { + for _, member := range group.GetAttributeValues(ls.GroupMemberUID) { if member == uid { isMember = true } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5aeffc758024e..a7800514c5da1 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2087,6 +2087,11 @@ auths.filter = User Filter auths.admin_filter = Admin Filter auths.restricted_filter = Restricted Filter auths.restricted_filter_helper = Leave empty to not set any users as restricted. Use an asterisk ('*') to set all users that do not match Admin Filter as restricted. +auths.verify_group_membership = Verify group membership in LDAP +auths.group_search_base = Group Search Base DN +auths.valid_groups_filter = Valid Groups Filter +auths.group_attribute_list_users = Group Attribute Containing List Of Users +auths.user_attribute_in_group = User Attribute Listed In Group auths.ms_ad_sa = MS AD Search Attributes auths.smtp_auth = SMTP Authentication Type auths.smtphost = SMTP Host diff --git a/routers/admin/auths.go b/routers/admin/auths.go index c6d6db73cd106..98f6e25b1f938 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -132,7 +132,7 @@ func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { GroupsEnabled: form.GroupsEnabled, GroupDN: form.GroupDN, GroupFilter: form.GroupFilter, - GroupMemberUid: form.GroupMemberUid, + GroupMemberUID: form.GroupMemberUID, UserUID: form.UserUID, AdminFilter: form.AdminFilter, RestrictedFilter: form.RestrictedFilter, diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index df9e18a881f9c..e6c5cf2578364 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -101,25 +101,28 @@
- +
-
- - -
-
- - -
-
- - -
-
- - +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
{{if .Source.IsLDAP}}
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 1a245baf56c45..651877b1f7c49 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -73,25 +73,28 @@
- - + +
-
- - -
-
- - -
-
- - -
-
- - +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
diff --git a/web_src/js/index.js b/web_src/js/index.js index 810493a7d6378..9ae8d537a535b 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -1798,6 +1798,15 @@ function initAdmin() { } } + function onVerifyGroupMembershipChange() { + const $groupsEnabled = $('#groups_enabled'); + if ($groupsEnabled.is(':checked')) { + $('#groups_enabled_change').show(); + } else { + $('#groups_enabled_change').hide(); + } + } + // New authentication if ($('.admin.new.authentication').length > 0) { $('#auth_type').on('change', function () { @@ -1838,6 +1847,7 @@ function initAdmin() { } if (authType === '2' || authType === '5') { onSecurityProtocolChange(); + onVerifyGroupMembershipChange(); } if (authType === '2') { onUsePagedSearchChange(); @@ -1848,12 +1858,15 @@ function initAdmin() { $('#use_paged_search').on('change', onUsePagedSearchChange); $('#oauth2_provider').on('change', onOAuth2Change); $('#oauth2_use_custom_url').on('change', onOAuth2UseCustomURLChange); + $('#groups_enabled').on('change', onVerifyGroupMembershipChange); } // Edit authentication if ($('.admin.edit.authentication').length > 0) { const authType = $('#auth_type').val(); if (authType === '2' || authType === '5') { $('#security_protocol').on('change', onSecurityProtocolChange); + $('#groups_enabled').on('change', onVerifyGroupMembershipChange); + onVerifyGroupMembershipChange(); if (authType === '2') { $('#use_paged_search').on('change', onUsePagedSearchChange); } From 2e2d19eed9a1e09aff889b982b9aa5a450eb2563 Mon Sep 17 00:00:00 2001 From: jolheiser Date: Sat, 28 Mar 2020 15:36:22 -0500 Subject: [PATCH 3/6] No reason for JS const. Signed-off-by: jolheiser --- web_src/js/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/web_src/js/index.js b/web_src/js/index.js index 9ae8d537a535b..a184578bcc3f4 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -1799,8 +1799,7 @@ function initAdmin() { } function onVerifyGroupMembershipChange() { - const $groupsEnabled = $('#groups_enabled'); - if ($groupsEnabled.is(':checked')) { + if ($('#groups_enabled').is(':checked')) { $('#groups_enabled_change').show(); } else { $('#groups_enabled_change').hide(); From 85dfd0e21eebb8262b5c1360b22d196378ccf05f Mon Sep 17 00:00:00 2001 From: jolheiser Date: Sat, 28 Mar 2020 23:20:23 -0500 Subject: [PATCH 4/6] Add bug-fixes Signed-off-by: jolheiser --- modules/auth/ldap/ldap.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index bfe6e7a1850dd..1a96450fd7cfa 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -359,16 +359,18 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul if err != nil { log.Error("LDAP group search failed: %v", err) return nil - } else if len(sr.Entries) < 1 { + } else if len(srg.Entries) < 1 { log.Error("LDAP group search failed: 0 entries") return nil } isMember := false + Entries: for _, group := range srg.Entries { for _, member := range group.GetAttributeValues(ls.GroupMemberUID) { - if member == uid { + if (ls.UserUID == "dn" && member == sr.Entries[0].DN) || member == uid { isMember = true + break Entries } } } From 6f6741cc51eae02f9c2748eead3d7a8655a92d1e Mon Sep 17 00:00:00 2001 From: John Olheiser Date: Sun, 29 Mar 2020 16:08:31 -0500 Subject: [PATCH 5/6] Apply suggestions from code review Co-Authored-By: Lauris BH Co-Authored-By: zeripath --- modules/auth/ldap/README.md | 2 +- modules/auth/ldap/ldap.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/auth/ldap/README.md b/modules/auth/ldap/README.md index 226acac80ceb9..76841f44aefd4 100644 --- a/modules/auth/ldap/README.md +++ b/modules/auth/ldap/README.md @@ -112,7 +112,7 @@ share the following fields: * Group Name Filter (optional) * An LDAP filter declaring how to find valid groups in the above DN. - * Example: (|(cn=gogs_users)(cn=admins)) + * Example: (|(cn=gitea_users)(cn=admins)) * User Attribute in Group (optional) * Which user LDAP attribute is listed in the group. diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 1a96450fd7cfa..2ac77df35c296 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -306,7 +306,10 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul var isAttributeSSHPublicKeySet = len(strings.TrimSpace(ls.AttributeSSHPublicKey)) > 0 - attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.UserUID} + attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail} + if len(strings.TrimSpace(ls.UserUID)) > 0 { + attribs = append(attribs, ls.UserUID) + } if isAttributeSSHPublicKeySet { attribs = append(attribs, ls.AttributeSSHPublicKey) } From 3f2c93954147ed2373676e5cb789c3f6b9e8ae76 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 29 Aug 2020 13:14:05 +0100 Subject: [PATCH 6/6] Update modules/auth/ldap/ldap.go --- modules/auth/ldap/ldap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 2ac77df35c296..7649639d36195 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -314,7 +314,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul attribs = append(attribs, ls.AttributeSSHPublicKey) } - log.Trace("Fetching attributes '%v', '%v', '%v', '%v', '%v' with filter '%s' and base '%s'", ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.UserUID, userFilter, userDN) + log.Trace("Fetching attributes '%v', '%v', '%v', '%v', '%v', '%v' with filter '%s' and base '%s'", ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.AttributeSSHPublicKey, ls.UserUID, userFilter, userDN) search := ldap.NewSearchRequest( userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, userFilter, attribs, nil)