From 50002c68616080e1789918079614f0fd3b6022b1 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 26 Jun 2019 21:11:07 +0200 Subject: [PATCH 01/26] Removed function attribute sshKeyAttribute from LDAP test --- integrations/auth_ldap_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 9ea3184ae7927..6a0ff582c970c 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -94,7 +94,7 @@ func getLDAPServerHost() string { return host } -func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { +func addAuthSourceLDAP(t *testing.T) { session := loginUser(t, "user1") csrf := GetCSRF(t, session, "/admin/auths/new") req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ @@ -112,7 +112,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { "attribute_name": "givenName", "attribute_surname": "sn", "attribute_mail": "mail", - "attribute_ssh_public_key": sshKeyAttribute, + "attribute_ssh_public_key": "sshPublicKey", "is_sync_enabled": "on", "is_active": "on", }) @@ -125,7 +125,7 @@ func TestLDAPUserSignin(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t) u := gitLDAPUsers[0] @@ -146,7 +146,7 @@ func TestLDAPUserSync(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t) models.SyncExternalUsers() session := loginUser(t, "user1") @@ -192,7 +192,7 @@ func TestLDAPUserSigninFailed(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t, "") + addAuthSourceLDAP(t) u := otherLDAPUsers[0] @@ -205,7 +205,7 @@ func TestLDAPUserSSHKeySync(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t, "sshPublicKey") + addAuthSourceLDAP(t) models.SyncExternalUsers() // Check if users has SSH keys synced From ad80909ba5eb679373b90dd9895286a3d7d2c0fd Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 26 Jun 2019 22:44:23 +0200 Subject: [PATCH 02/26] Added config options for LDAP groups and filters --- modules/auth/auth_form.go | 3 +++ modules/auth/ldap/ldap.go | 3 +++ routers/admin/auths.go | 3 +++ templates/admin/auth/edit.tmpl | 12 ++++++++++++ templates/admin/auth/source/ldap.tmpl | 12 ++++++++++++ 5 files changed, 33 insertions(+) diff --git a/modules/auth/auth_form.go b/modules/auth/auth_form.go index 358472a3855b4..d2cc0c5cc3df5 100644 --- a/modules/auth/auth_form.go +++ b/modules/auth/auth_form.go @@ -20,6 +20,9 @@ type AuthenticationForm struct { BindPassword string UserBase string UserDN string + GroupSearchBase string + MemberGroupFilter string + AdminGroupFilter string AttributeUsername string AttributeName string AttributeSurname string diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index ed83a77e12f68..c50e93a5d76cd 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -37,6 +37,9 @@ 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 + GroupSearchBase string // Base search path for groups + MemberGroupFilter string // Query group filter to check if user is allowed to log in + AdminGroupFilter string // Query group filter to check if user is admin AttributeUsername string // Username attribute AttributeName string // First name attribute AttributeSurname string // Surname attribute diff --git a/routers/admin/auths.go b/routers/admin/auths.go index 8e0c27e2267de..6d911f2fb22ab 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -106,6 +106,9 @@ func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { UserDN: form.UserDN, BindPassword: form.BindPassword, UserBase: form.UserBase, + GroupSearchBase: form.GroupSearchBase, + MemberGroupFilter: form.MemberGroupFilter, + AdminGroupFilter: form.AdminGroupFilter, AttributeUsername: form.AttributeUsername, AttributeName: form.AttributeName, AttributeSurname: form.AttributeSurname, diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 6a176a43a84ee..cf44faf508b15 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -74,6 +74,18 @@ +
+ + +
+
+ + +
+
+ + +
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 3386884ff14a4..64be04f323ffe 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -46,6 +46,18 @@
+
+ + +
+
+ + +
+
+ + +
From bfdc279d3f933286eed16d1dbc2f8273c9b6e2e9 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Thu, 27 Jun 2019 23:46:41 +0200 Subject: [PATCH 03/26] Rudimentary group support - Find user groups by - Search base, like ou=people,dc=mycompany,dc=com - Group filter, like (&(objectClass=posixGroup)(member=%s)) - User attribute used by above group filter, like cn - Additional filter for - Membership in general (allowed to log in) - Admin role --- modules/auth/auth_form.go | 2 + modules/auth/ldap/ldap.go | 63 +++++++++++++++++++++++++-- routers/admin/auths.go | 2 + templates/admin/auth/edit.tmpl | 12 ++++- templates/admin/auth/source/ldap.tmpl | 12 ++++- 5 files changed, 84 insertions(+), 7 deletions(-) diff --git a/modules/auth/auth_form.go b/modules/auth/auth_form.go index d2cc0c5cc3df5..3e88c4ad0ced4 100644 --- a/modules/auth/auth_form.go +++ b/modules/auth/auth_form.go @@ -21,6 +21,8 @@ type AuthenticationForm struct { UserBase string UserDN string GroupSearchBase string + GroupSearchFilter string + UserAttributeInGroup string MemberGroupFilter string AdminGroupFilter string AttributeUsername string diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index c50e93a5d76cd..932f6aa9c6433 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -38,6 +38,8 @@ type Source struct { UserBase string // Base search path for users UserDN string // Template for the DN of the user for simple auth GroupSearchBase string // Base search path for groups + GroupSearchFilter string // + UserAttributeInGroup string // MemberGroupFilter string // Query group filter to check if user is allowed to log in AdminGroupFilter string // Query group filter to check if user is admin AttributeUsername string // Username attribute @@ -73,6 +75,17 @@ func (ls *Source) sanitizedUserQuery(username string) (string, bool) { return fmt.Sprintf(ls.Filter, username), true } +func (ls *Source) sanitizedGroupQuery(username string) (string, bool) { + // See http://tools.ietf.org/search/rfc4515 + badCharacters := "\x00()*\\" + if strings.ContainsAny(username, badCharacters) { + log.Debug("'%s' contains invalid query characters. Aborting.", username) + return "", false + } + + return fmt.Sprintf(ls.GroupSearchFilter, username), true +} + func (ls *Source) sanitizedUserDN(username string) (string, bool) { // See http://tools.ietf.org/search/rfc4514: "special characters" badCharacters := "\x00()*\\,='\"#+;<>" @@ -175,6 +188,18 @@ func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool { return false } +// CheckGroupFilter : +func (ls *Source) CheckGroupFilter(l *ldap.Conn, groupSR *ldap.SearchResult, filter string) bool { + for _, groupEntry := range groupSR.Entries { + search := ldap.NewSearchRequest(groupEntry.DN, ldap.ScopeBaseObject, ldap.NeverDerefAliases, 0, 0, false, filter, []string{}, nil) + sr, err := l.Search(search) + if (err == nil) && (len(sr.Entries) > 0) { + return true + } + } + return false +} + // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult { // See https://tools.ietf.org/search/rfc4513#section-5.1.2 @@ -252,12 +277,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.UserAttributeInGroup} 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', '%v' with filter %s and base %s", ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.AttributeSSHPublicKey, ls.UserAttributeInGroup, userFilter, userDN) search := ldap.NewSearchRequest( userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, userFilter, attribs, nil) @@ -285,7 +310,39 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul if isAttributeSSHPublicKeySet { sshPublicKey = sr.Entries[0].GetAttributeValues(ls.AttributeSSHPublicKey) } - isAdmin := checkAdmin(l, ls, userDN) + + var hasAdminGroup = false + if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 && len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { + groupUID := sr.Entries[0].GetAttributeValue(ls.UserAttributeInGroup) + groupFilter, ok := ls.sanitizedGroupQuery(groupUID) + if !ok { + return nil + } + + groupSearch := ldap.NewSearchRequest( + ls.GroupSearchBase, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, []string{}, nil) + + sr, err := l.Search(groupSearch) + if err != nil { + log.Error("LDAP Search failed unexpectedly! (%v)", err) + return nil + } + + log.Trace("LDAP group search found %i entries", len(sr.Entries)) + + if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { + if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { + log.Error("No group matched the required member group filter!") + return nil + } + } + + if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { + hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) + } + } + + isAdmin := hasAdminGroup || checkAdmin(l, ls, userDN) if !directBind && ls.AttributesInBind { // binds user (checking password) after looking-up attributes in BindDN context diff --git a/routers/admin/auths.go b/routers/admin/auths.go index 6d911f2fb22ab..4bc32812c3620 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -107,6 +107,8 @@ func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { BindPassword: form.BindPassword, UserBase: form.UserBase, GroupSearchBase: form.GroupSearchBase, + GroupSearchFilter: form.GroupSearchFilter, + UserAttributeInGroup: form.UserAttributeInGroup, MemberGroupFilter: form.MemberGroupFilter, AdminGroupFilter: form.AdminGroupFilter, AttributeUsername: form.AttributeUsername, diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index cf44faf508b15..7a6458e4f00e5 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -78,13 +78,21 @@
+
+ + +
+
+ + +
- +
- +
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 64be04f323ffe..b1959a80ee817 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -50,13 +50,21 @@
+
+ + +
+
+ + +
- +
- +
From d9673e50a9e5338109cdd54566e572a86a0a2ec3 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 3 Jul 2019 21:55:40 +0200 Subject: [PATCH 04/26] LDAP: Special case for using DN as user attribute in group --- modules/auth/ldap/ldap.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 932f6aa9c6433..5895208510484 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -197,6 +197,7 @@ func (ls *Source) CheckGroupFilter(l *ldap.Conn, groupSR *ldap.SearchResult, fil return true } } + log.Trace("LDAP group search with filter %v found no matching entries", filter) return false } @@ -312,8 +313,15 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul } var hasAdminGroup = false - if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 && len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { - groupUID := sr.Entries[0].GetAttributeValue(ls.UserAttributeInGroup) + if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { + var groupUID string + if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { + groupUID = sr.Entries[0].GetAttributeValue(ls.UserAttributeInGroup) + } else { + groupUID = sr.Entries[0].DN + } + log.Trace("User attribute used in LDAP group: %v", groupUID) + groupFilter, ok := ls.sanitizedGroupQuery(groupUID) if !ok { return nil @@ -324,12 +332,10 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul sr, err := l.Search(groupSearch) if err != nil { - log.Error("LDAP Search failed unexpectedly! (%v)", err) + log.Error("LDAP group search failed unexpectedly! (%v)", err) return nil } - log.Trace("LDAP group search found %i entries", len(sr.Entries)) - if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { log.Error("No group matched the required member group filter!") @@ -339,6 +345,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) + log.Info("LDAP user is in admin group!") } } From 27bfd7afcaf60399d795ac372beb43f37cd1371a Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 3 Jul 2019 21:56:14 +0200 Subject: [PATCH 05/26] LDAP: Small template improvements --- templates/admin/auth/edit.tmpl | 6 +++--- templates/admin/auth/source/ldap.tmpl | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 7a6458e4f00e5..0e01790e43207 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -76,7 +76,7 @@
- +
@@ -84,11 +84,11 @@
- +
- +
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index b1959a80ee817..5451d0ad22148 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -48,7 +48,7 @@
- +
@@ -56,11 +56,11 @@
- +
- +
From 1c59987330f1e62af2e1d1f9dda6e496a86003f1 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 3 Jul 2019 22:09:05 +0200 Subject: [PATCH 06/26] LDAP: Added text for the new configuration features --- options/locale/locale_de-DE.ini | 6 ++++++ options/locale/locale_en-US.ini | 6 ++++++ templates/admin/auth/edit.tmpl | 1 + templates/admin/auth/source/ldap.tmpl | 1 + 4 files changed, 14 insertions(+) diff --git a/options/locale/locale_de-DE.ini b/options/locale/locale_de-DE.ini index 4db25eaedbed1..bff3b9e2783c7 100644 --- a/options/locale/locale_de-DE.ini +++ b/options/locale/locale_de-DE.ini @@ -1757,6 +1757,12 @@ auths.use_paged_search=Seitensuche verwenden auths.search_page_size=Seitengröße auths.filter=Benutzerfilter auths.admin_filter=Admin-Filter +auths.group_search_base = Basis für Gruppensuche +auths.group_search_filter = Gruppenfilter +auths.user_attribute_in_group = Benutzerattribut für Gruppensuche +auths.user_attribute_in_group_helper = Der Wert dieses Attributs wird in die Gruppensuche eingesetzt. Um den DN zu verwenden, muss das Feld leer gelassen werden. +auths.member_group_filter = Gruppenfilter für Registrierung +auths.admin_group_filter = Gruppenfilter für Admin-Rechte auths.ms_ad_sa=MS-AD-Suchattribute auths.smtp_auth=SMTP-Authentifizierungstyp auths.smtphost=SMTP-Host diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index e3e0dba9fecda..2cd4bfd5bd890 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1776,6 +1776,12 @@ auths.use_paged_search = Use Paged Search auths.search_page_size = Page Size auths.filter = User Filter auths.admin_filter = Admin Filter +auths.group_search_base = Group Search Base +auths.group_search_filter = Group Search Filter +auths.user_attribute_in_group = User Attribute in Group +auths.user_attribute_in_group_helper = The value of this user attribute is inserted into the group search filter. To use the DN, leave this field empty. +auths.member_group_filter = Member Group Filter +auths.admin_group_filter = Admin Group Filter auths.ms_ad_sa = MS AD Search Attributes auths.smtp_auth = SMTP Authentication Type auths.smtphost = SMTP Host diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 0e01790e43207..fcff49adee237 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -85,6 +85,7 @@
+

{{.i18n.Tr "admin.auths.user_attribute_in_group_helper"}}

diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 5451d0ad22148..efecea3244c41 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -57,6 +57,7 @@
+

{{.i18n.Tr "admin.auths.user_attribute_in_group_helper"}}

From f71752f451b09fdbf7f659767fe5207a75d509fb Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 3 Jul 2019 22:17:55 +0200 Subject: [PATCH 07/26] LDAP: Added some comments --- modules/auth/ldap/ldap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 5895208510484..a67103a16aec5 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -38,8 +38,8 @@ type Source struct { UserBase string // Base search path for users UserDN string // Template for the DN of the user for simple auth GroupSearchBase string // Base search path for groups - GroupSearchFilter string // - UserAttributeInGroup string // + GroupSearchFilter string // Query group filter to validate entry + UserAttributeInGroup string // User attribute inserted into group filter MemberGroupFilter string // Query group filter to check if user is allowed to log in AdminGroupFilter string // Query group filter to check if user is admin AttributeUsername string // Username attribute From 53d8c1d5a97e939778b17552f27c4b4d35aec539 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Wed, 3 Jul 2019 22:43:47 +0200 Subject: [PATCH 08/26] LDAP: Added new group search to SearchEntries() --- modules/auth/ldap/ldap.go | 52 ++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index a67103a16aec5..21f086f6d6440 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -420,20 +420,60 @@ func (ls *Source) SearchEntries() ([]*SearchResult, error) { return nil, err } - result := make([]*SearchResult, len(sr.Entries)) + results := []*SearchResult{} + + for _, v := range sr.Entries { + + // TODO: Remove code duplication + var hasAdminGroup = false + if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { + var groupUID string + if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { + groupUID = v.GetAttributeValue(ls.UserAttributeInGroup) + } else { + groupUID = v.DN + } + log.Trace("User attribute used in LDAP group: %v", groupUID) + + groupFilter, ok := ls.sanitizedGroupQuery(groupUID) + if !ok { + continue + } + + groupSearch := ldap.NewSearchRequest( + ls.GroupSearchBase, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, []string{}, nil) + + sr, err := l.Search(groupSearch) + if err != nil { + log.Error("LDAP group search failed unexpectedly! (%v)", err) + continue + } + + if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { + if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { + log.Error("No group matched the required member group filter!") + continue + } + } + + if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { + hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) + log.Info("LDAP user is in admin group!") + } + } - for i, v := range sr.Entries { - result[i] = &SearchResult{ + result := &SearchResult{ Username: v.GetAttributeValue(ls.AttributeUsername), Name: v.GetAttributeValue(ls.AttributeName), Surname: v.GetAttributeValue(ls.AttributeSurname), Mail: v.GetAttributeValue(ls.AttributeMail), - IsAdmin: checkAdmin(l, ls, v.DN), + IsAdmin: hasAdminGroup || checkAdmin(l, ls, v.DN), } if isAttributeSSHPublicKeySet { - result[i].SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey) + result.SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey) } + results = append(results, result) } - return result, nil + return results, nil } From 7a122c06a3e2fe193e3734814e4adb9578fca932 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 7 Jul 2019 21:28:36 +0200 Subject: [PATCH 09/26] Revert "Removed function attribute sshKeyAttribute from LDAP test" This reverts commit 155751c73b9ee964a375e956ef965b367c3a7317. --- integrations/auth_ldap_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 6a0ff582c970c..9ea3184ae7927 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -94,7 +94,7 @@ func getLDAPServerHost() string { return host } -func addAuthSourceLDAP(t *testing.T) { +func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { session := loginUser(t, "user1") csrf := GetCSRF(t, session, "/admin/auths/new") req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ @@ -112,7 +112,7 @@ func addAuthSourceLDAP(t *testing.T) { "attribute_name": "givenName", "attribute_surname": "sn", "attribute_mail": "mail", - "attribute_ssh_public_key": "sshPublicKey", + "attribute_ssh_public_key": sshKeyAttribute, "is_sync_enabled": "on", "is_active": "on", }) @@ -125,7 +125,7 @@ func TestLDAPUserSignin(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t) + addAuthSourceLDAP(t, "") u := gitLDAPUsers[0] @@ -146,7 +146,7 @@ func TestLDAPUserSync(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t) + addAuthSourceLDAP(t, "") models.SyncExternalUsers() session := loginUser(t, "user1") @@ -192,7 +192,7 @@ func TestLDAPUserSigninFailed(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t) + addAuthSourceLDAP(t, "") u := otherLDAPUsers[0] @@ -205,7 +205,7 @@ func TestLDAPUserSSHKeySync(t *testing.T) { return } prepareTestEnv(t) - addAuthSourceLDAP(t) + addAuthSourceLDAP(t, "sshPublicKey") models.SyncExternalUsers() // Check if users has SSH keys synced From 89256fb1d36002025987cc0e0249933c8fb48c39 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 7 Jul 2019 21:31:33 +0200 Subject: [PATCH 10/26] Removed german translations for LDAP group configuration --- options/locale/locale_de-DE.ini | 6 ------ 1 file changed, 6 deletions(-) diff --git a/options/locale/locale_de-DE.ini b/options/locale/locale_de-DE.ini index bff3b9e2783c7..4db25eaedbed1 100644 --- a/options/locale/locale_de-DE.ini +++ b/options/locale/locale_de-DE.ini @@ -1757,12 +1757,6 @@ auths.use_paged_search=Seitensuche verwenden auths.search_page_size=Seitengröße auths.filter=Benutzerfilter auths.admin_filter=Admin-Filter -auths.group_search_base = Basis für Gruppensuche -auths.group_search_filter = Gruppenfilter -auths.user_attribute_in_group = Benutzerattribut für Gruppensuche -auths.user_attribute_in_group_helper = Der Wert dieses Attributs wird in die Gruppensuche eingesetzt. Um den DN zu verwenden, muss das Feld leer gelassen werden. -auths.member_group_filter = Gruppenfilter für Registrierung -auths.admin_group_filter = Gruppenfilter für Admin-Rechte auths.ms_ad_sa=MS-AD-Suchattribute auths.smtp_auth=SMTP-Authentifizierungstyp auths.smtphost=SMTP-Host From 66ccdd067c37728047a82c83f9bb8534b71cf075 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 14 Jul 2019 15:38:44 +0200 Subject: [PATCH 11/26] LDAP: Added tests for simple login --- integrations/auth_ldap_test.go | 61 ++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 9ea3184ae7927..420ebadb4c04a 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -119,6 +119,33 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { session.MakeRequest(t, req, http.StatusFound) } +func addAuthSourceLDAPWithGroups(t *testing.T) { + session := loginUser(t, "user1") + csrf := GetCSRF(t, session, "/admin/auths/new") + req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ + "_csrf": csrf, + "type": "2", + "name": "ldap", + "host": getLDAPServerHost(), + "port": "389", + "bind_dn": "uid=gitea,ou=service,dc=planetexpress,dc=com", + "bind_password": "password", + "user_base": "ou=people,dc=planetexpress,dc=com", + "filter": "(&(objectClass=inetOrgPerson)(uid=%s))", + "group_search_base": "ou=people,dc=planetexpress,dc=com", + "group_search_filter": "(&(objectClass=groupOfNames)(member=%s))", + "user_attribute_in_group": "", + "member_group_filter": "(cn=git)", + "admin_group_filter": "(cn=admin_staff)", + "attribute_username": "uid", + "attribute_name": "givenName", + "attribute_surname": "sn", + "attribute_mail": "mail", + "is_active": "on", + }) + session.MakeRequest(t, req, http.StatusFound) +} + func TestLDAPUserSignin(t *testing.T) { if skipLDAPTests() { t.Skip() @@ -140,6 +167,27 @@ func TestLDAPUserSignin(t *testing.T) { assert.Equal(t, u.Email, htmlDoc.GetInputValueByName("email")) } +func TestLDAPUserSigninWithGroups(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + prepareTestEnv(t) + addAuthSourceLDAPWithGroups(t) + + for _, u := range gitLDAPUsers { + session := loginUserWithPassword(t, u.UserName, 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.GetInputValueByName("email")) + } +} + func TestLDAPUserSync(t *testing.T) { if skipLDAPTests() { t.Skip() @@ -199,6 +247,19 @@ func TestLDAPUserSigninFailed(t *testing.T) { testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) } +func TestLDAPUserSigninFailedWithGroups(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + prepareTestEnv(t) + addAuthSourceLDAPWithGroups(t) + + for _, u := range otherLDAPUsers { + testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) + } +} + func TestLDAPUserSSHKeySync(t *testing.T) { if skipLDAPTests() { t.Skip() From 62f54f811922f822c5a0849bdee1b0682afc2533 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 14 Jul 2019 16:40:43 +0200 Subject: [PATCH 12/26] Fixed LDAP test for Hermes Conrad. --- integrations/auth_ldap_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 420ebadb4c04a..0e625fb26bf07 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -38,7 +38,7 @@ var gitLDAPUsers = []ldapUser{ { UserName: "hermes", Password: "hermes", - FullName: "Conrad Hermes", + FullName: "Hermes Conrad", Email: "hermes@planetexpress.com", SSHKeys: []string{ "SHA256:qLY06smKfHoW/92yXySpnxFR10QFrLdRjf/GNPvwcW8", From 1846719f5950fc66248487ff319ec49e02384653 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 14 Jul 2019 19:02:59 +0200 Subject: [PATCH 13/26] Added test for LDAP admin filter --- integrations/auth_ldap_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 0e625fb26bf07..77412e3a378ea 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -185,6 +185,15 @@ func TestLDAPUserSigninWithGroups(t *testing.T) { assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name")) assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name")) assert.Equal(t, u.Email, htmlDoc.GetInputValueByName("email")) + + reqAdmin := NewRequest(t, "GET", "/admin") + if u.IsAdmin { + adminResp := session.MakeRequest(t, reqAdmin, http.StatusOK) + assert.Equal(t, http.StatusOK, adminResp.Code) + } else { + adminResp := session.MakeRequest(t, reqAdmin, http.StatusForbidden) + assert.Equal(t, http.StatusForbidden, adminResp.Code) + } } } From 41f0eacacce52fd4e59f07f38188d91cd06c35a8 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 3 Nov 2019 17:12:47 +0100 Subject: [PATCH 14/26] LDAP: Added CLI arguments for LDAP groups (incl. tests) --- cmd/admin_auth_ldap.go | 35 +++++++ cmd/admin_auth_ldap_test.go | 200 ++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+) diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go index cce3aa894f697..e21f68c7f7d9f 100644 --- a/cmd/admin_auth_ldap.go +++ b/cmd/admin_auth_ldap.go @@ -53,6 +53,26 @@ var ( Name: "user-search-base", Usage: "The LDAP base at which user accounts will be searched for.", }, + cli.StringFlag{ + Name: "group-search-base", + Usage: "The LDAP base at which groups will be searched for.", + }, + cli.StringFlag{ + Name: "group-search-filter", + Usage: "An LDAP filter declaring how to find the groups a user is member of.", + }, + cli.StringFlag{ + Name: "user-attribute-in-group", + Usage: "The LDAP attribute of a user referenced by groups.", + }, + cli.StringFlag{ + Name: "member-group-filter", + Usage: "An LDAP filter specifying if a group should be given administrator privileges.", + }, + cli.StringFlag{ + Name: "admin-group-filter", + Usage: "An LDAP filter specifying if a group is allowed to log in.", + }, cli.StringFlag{ Name: "user-filter", Usage: "An LDAP filter declaring how to find the user record that is attempting to authenticate.", @@ -204,6 +224,21 @@ func parseLdapConfig(c *cli.Context, config *models.LDAPConfig) error { if c.IsSet("user-search-base") { config.Source.UserBase = c.String("user-search-base") } + if c.IsSet("group-search-base") { + config.Source.GroupSearchBase = c.String("group-search-base") + } + if c.IsSet("group-search-filter") { + config.Source.GroupSearchFilter = c.String("group-search-filter") + } + if c.IsSet("user-attribute-in-group") { + config.Source.UserAttributeInGroup = c.String("user-attribute-in-group") + } + if c.IsSet("member-group-filter") { + config.Source.MemberGroupFilter = c.String("member-group-filter") + } + if c.IsSet("admin-group-filter") { + config.Source.AdminGroupFilter = c.String("admin-group-filter") + } if c.IsSet("username-attribute") { config.Source.AttributeUsername = c.String("username-attribute") } diff --git a/cmd/admin_auth_ldap_test.go b/cmd/admin_auth_ldap_test.go index 4af9f167c3799..e89df7128a77d 100644 --- a/cmd/admin_auth_ldap_test.go +++ b/cmd/admin_auth_ldap_test.go @@ -49,6 +49,11 @@ func TestAddLdapBindDn(t *testing.T) { "--attributes-in-bind", "--synchronize-users", "--page-size", "99", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + "--user-attribute-in-group", "uid", + "--member-group-filter", "(cn=user-group)", + "--admin-group-filter", "(cn=admin-group)", }, loginSource: &models.LoginSource{ Type: models.LoginLDAP, @@ -65,6 +70,11 @@ func TestAddLdapBindDn(t *testing.T) { BindDN: "cn=readonly,dc=full-domain-bind,dc=org", BindPassword: "secret-bind-full", UserBase: "ou=Users,dc=full-domain-bind,dc=org", + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + UserAttributeInGroup: "uid", + MemberGroupFilter: "(cn=user-group)", + AdminGroupFilter: "(cn=admin-group)", AttributeUsername: "uid-bind full", AttributeName: "givenName-bind full", AttributeSurname: "sn-bind full", @@ -271,6 +281,11 @@ func TestAddLdapSimpleAuth(t *testing.T) { "--email-attribute", "mail-simple full", "--public-ssh-key-attribute", "publickey-simple full", "--user-dn", "cn=%s,ou=Users,dc=full-domain-simple,dc=org", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + "--user-attribute-in-group", "uid", + "--member-group-filter", "(cn=user-group)", + "--admin-group-filter", "(cn=admin-group)", }, loginSource: &models.LoginSource{ Type: models.LoginDLDAP, @@ -285,6 +300,11 @@ func TestAddLdapSimpleAuth(t *testing.T) { SkipVerify: true, UserDN: "cn=%s,ou=Users,dc=full-domain-simple,dc=org", UserBase: "ou=Users,dc=full-domain-simple,dc=org", + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + UserAttributeInGroup: "uid", + MemberGroupFilter: "(cn=user-group)", + AdminGroupFilter: "(cn=admin-group)", AttributeUsername: "uid-simple full", AttributeName: "givenName-simple full", AttributeSurname: "sn-simple full", @@ -508,6 +528,11 @@ func TestUpdateLdapBindDn(t *testing.T) { "--bind-password", "secret-bind-full", "--synchronize-users", "--page-size", "99", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + "--user-attribute-in-group", "uid", + "--member-group-filter", "(cn=user-group)", + "--admin-group-filter", "(cn=admin-group)", }, id: 23, existingLoginSource: &models.LoginSource{ @@ -534,6 +559,11 @@ func TestUpdateLdapBindDn(t *testing.T) { BindDN: "cn=readonly,dc=full-domain-bind,dc=org", BindPassword: "secret-bind-full", UserBase: "ou=Users,dc=full-domain-bind,dc=org", + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + UserAttributeInGroup: "uid", + MemberGroupFilter: "(cn=user-group)", + AdminGroupFilter: "(cn=admin-group)", AttributeUsername: "uid-bind full", AttributeName: "givenName-bind full", AttributeSurname: "sn-bind full", @@ -901,6 +931,86 @@ func TestUpdateLdapBindDn(t *testing.T) { }, errMsg: "Invalid authentication type. expected: LDAP (via BindDN), actual: OAuth2", }, + // case 24 + { + args: []string{ + "ldap-test", + "--id", "1", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + }, + loginSource: &models.LoginSource{ + Type: models.LoginLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + }, + }, + }, + }, + // case 25 + { + args: []string{ + "ldap-test", + "--id", "1", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + }, + loginSource: &models.LoginSource{ + Type: models.LoginLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + }, + }, + }, + }, + // case 26 + { + args: []string{ + "ldap-test", + "--id", "1", + "--user-attribute-in-group", "uid", + }, + loginSource: &models.LoginSource{ + Type: models.LoginLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + UserAttributeInGroup: "uid", + }, + }, + }, + }, + // case 27 + { + args: []string{ + "ldap-test", + "--id", "1", + "--member-group-filter", "(cn=user-group)", + }, + loginSource: &models.LoginSource{ + Type: models.LoginLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + MemberGroupFilter: "(cn=user-group)", + }, + }, + }, + }, + // case 28 + { + args: []string{ + "ldap-test", + "--id", "1", + "--admin-group-filter", "(cn=admin-group)", + }, + loginSource: &models.LoginSource{ + Type: models.LoginLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + AdminGroupFilter: "(cn=admin-group)", + }, + }, + }, + }, } for n, c := range cases { @@ -984,6 +1094,11 @@ func TestUpdateLdapSimpleAuth(t *testing.T) { "--email-attribute", "mail-simple full", "--public-ssh-key-attribute", "publickey-simple full", "--user-dn", "cn=%s,ou=Users,dc=full-domain-simple,dc=org", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + "--user-attribute-in-group", "uid", + "--member-group-filter", "(cn=user-group)", + "--admin-group-filter", "(cn=admin-group)", }, id: 7, loginSource: &models.LoginSource{ @@ -999,6 +1114,11 @@ func TestUpdateLdapSimpleAuth(t *testing.T) { SkipVerify: true, UserDN: "cn=%s,ou=Users,dc=full-domain-simple,dc=org", UserBase: "ou=Users,dc=full-domain-simple,dc=org", + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + UserAttributeInGroup: "uid", + MemberGroupFilter: "(cn=user-group)", + AdminGroupFilter: "(cn=admin-group)", AttributeUsername: "uid-simple full", AttributeName: "givenName-simple full", AttributeSurname: "sn-simple full", @@ -1300,6 +1420,86 @@ func TestUpdateLdapSimpleAuth(t *testing.T) { }, errMsg: "Invalid authentication type. expected: LDAP (simple auth), actual: PAM", }, + // case 20 + { + args: []string{ + "ldap-test", + "--id", "1", + "--group-search-base", "ou=Groups,dc=full-domain-bind,dc=org", + }, + loginSource: &models.LoginSource{ + Type: models.LoginDLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + GroupSearchBase: "ou=Groups,dc=full-domain-bind,dc=org", + }, + }, + }, + }, + // case 21 + { + args: []string{ + "ldap-test", + "--id", "1", + "--group-search-filter", "(&(objectClass=groupOfNames)(member=%s))", + }, + loginSource: &models.LoginSource{ + Type: models.LoginDLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + GroupSearchFilter: "(&(objectClass=groupOfNames)(member=%s))", + }, + }, + }, + }, + // case 22 + { + args: []string{ + "ldap-test", + "--id", "1", + "--user-attribute-in-group", "uid", + }, + loginSource: &models.LoginSource{ + Type: models.LoginDLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + UserAttributeInGroup: "uid", + }, + }, + }, + }, + // case 23 + { + args: []string{ + "ldap-test", + "--id", "1", + "--member-group-filter", "(cn=user-group)", + }, + loginSource: &models.LoginSource{ + Type: models.LoginDLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + MemberGroupFilter: "(cn=user-group)", + }, + }, + }, + }, + // case 24 + { + args: []string{ + "ldap-test", + "--id", "1", + "--admin-group-filter", "(cn=admin-group)", + }, + loginSource: &models.LoginSource{ + Type: models.LoginDLDAP, + Cfg: &models.LDAPConfig{ + Source: &ldap.Source{ + AdminGroupFilter: "(cn=admin-group)", + }, + }, + }, + }, } for n, c := range cases { From 2df2271823e4e37eed0ac9ec2b51506a0c594172 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 3 Nov 2019 21:47:58 +0100 Subject: [PATCH 15/26] LDAP: Reduced code duplication by indroducing ProcessUserEntry function --- modules/auth/ldap/ldap.go | 171 +++++++++++++++----------------------- 1 file changed, 66 insertions(+), 105 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 21f086f6d6440..aa81a08019c9e 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -201,6 +201,67 @@ func (ls *Source) CheckGroupFilter(l *ldap.Conn, groupSR *ldap.SearchResult, fil return false } +// ProcessUserEntry : +func (ls *Source) ProcessUserEntry(entry *ldap.Entry, l *ldap.Conn) *SearchResult { + username := entry.GetAttributeValue(ls.AttributeUsername) + firstname := entry.GetAttributeValue(ls.AttributeName) + surname := entry.GetAttributeValue(ls.AttributeSurname) + mail := entry.GetAttributeValue(ls.AttributeMail) + + var sshPublicKey []string + if len(strings.TrimSpace(ls.AttributeSSHPublicKey)) > 0 { + sshPublicKey = entry.GetAttributeValues(ls.AttributeSSHPublicKey) + } + + var hasAdminGroup = false + if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { + var groupUID string + if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { + groupUID = entry.GetAttributeValue(ls.UserAttributeInGroup) + } else { + groupUID = entry.DN + } + log.Trace("User attribute used in LDAP group: %v", groupUID) + + groupFilter, ok := ls.sanitizedGroupQuery(groupUID) + if !ok { + return nil + } + + groupSearch := ldap.NewSearchRequest( + ls.GroupSearchBase, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, []string{}, nil) + + sr, err := l.Search(groupSearch) + if err != nil { + log.Error("LDAP group search failed unexpectedly! (%v)", err) + return nil + } + + if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { + if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { + log.Error("No group matched the required member group filter!") + return nil + } + } + + if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { + hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) + log.Info("LDAP user is in admin group!") + } + } + + isAdmin := hasAdminGroup || checkAdmin(l, ls, entry.DN) + + return &SearchResult{ + Username: username, + Name: firstname, + Surname: surname, + Mail: mail, + SSHPublicKey: sshPublicKey, + IsAdmin: isAdmin, + } +} + // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult { // See https://tools.ietf.org/search/rfc4513#section-5.1.2 @@ -302,54 +363,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul return nil } - var sshPublicKey []string - - username := sr.Entries[0].GetAttributeValue(ls.AttributeUsername) - firstname := sr.Entries[0].GetAttributeValue(ls.AttributeName) - surname := sr.Entries[0].GetAttributeValue(ls.AttributeSurname) - mail := sr.Entries[0].GetAttributeValue(ls.AttributeMail) - if isAttributeSSHPublicKeySet { - sshPublicKey = sr.Entries[0].GetAttributeValues(ls.AttributeSSHPublicKey) - } - - var hasAdminGroup = false - if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { - var groupUID string - if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { - groupUID = sr.Entries[0].GetAttributeValue(ls.UserAttributeInGroup) - } else { - groupUID = sr.Entries[0].DN - } - log.Trace("User attribute used in LDAP group: %v", groupUID) - - groupFilter, ok := ls.sanitizedGroupQuery(groupUID) - if !ok { - return nil - } - - groupSearch := ldap.NewSearchRequest( - ls.GroupSearchBase, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, []string{}, nil) - - sr, err := l.Search(groupSearch) - if err != nil { - log.Error("LDAP group search failed unexpectedly! (%v)", err) - return nil - } - - if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { - if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { - log.Error("No group matched the required member group filter!") - return nil - } - } - - if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { - hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) - log.Info("LDAP user is in admin group!") - } - } - - isAdmin := hasAdminGroup || checkAdmin(l, ls, userDN) + result := ls.ProcessUserEntry(sr.Entries[0], l) if !directBind && ls.AttributesInBind { // binds user (checking password) after looking-up attributes in BindDN context @@ -359,14 +373,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul } } - return &SearchResult{ - Username: username, - Name: firstname, - Surname: surname, - Mail: mail, - SSHPublicKey: sshPublicKey, - IsAdmin: isAdmin, - } + return result } // UsePagedSearch returns if need to use paged search @@ -423,56 +430,10 @@ func (ls *Source) SearchEntries() ([]*SearchResult, error) { results := []*SearchResult{} for _, v := range sr.Entries { - - // TODO: Remove code duplication - var hasAdminGroup = false - if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { - var groupUID string - if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { - groupUID = v.GetAttributeValue(ls.UserAttributeInGroup) - } else { - groupUID = v.DN - } - log.Trace("User attribute used in LDAP group: %v", groupUID) - - groupFilter, ok := ls.sanitizedGroupQuery(groupUID) - if !ok { - continue - } - - groupSearch := ldap.NewSearchRequest( - ls.GroupSearchBase, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter, []string{}, nil) - - sr, err := l.Search(groupSearch) - if err != nil { - log.Error("LDAP group search failed unexpectedly! (%v)", err) - continue - } - - if len(strings.TrimSpace(ls.MemberGroupFilter)) > 0 { - if !ls.CheckGroupFilter(l, sr, ls.MemberGroupFilter) { - log.Error("No group matched the required member group filter!") - continue - } - } - - if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { - hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) - log.Info("LDAP user is in admin group!") - } - } - - result := &SearchResult{ - Username: v.GetAttributeValue(ls.AttributeUsername), - Name: v.GetAttributeValue(ls.AttributeName), - Surname: v.GetAttributeValue(ls.AttributeSurname), - Mail: v.GetAttributeValue(ls.AttributeMail), - IsAdmin: hasAdminGroup || checkAdmin(l, ls, v.DN), - } - if isAttributeSSHPublicKeySet { - result.SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey) + result := ls.ProcessUserEntry(v, l) + if result != nil { + results = append(results, result) } - results = append(results, result) } return results, nil From e5c7d5cc700e7363e5583fd528decaaa8f4eecd2 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Tue, 12 Nov 2019 21:50:43 +0100 Subject: [PATCH 16/26] LDAP: Added test for userAttributeInGroup='cn' --- integrations/auth_ldap_test.go | 107 ++++++++++++++++++++++++--------- 1 file changed, 77 insertions(+), 30 deletions(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 77412e3a378ea..17ee1c427f92c 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -119,7 +119,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { session.MakeRequest(t, req, http.StatusFound) } -func addAuthSourceLDAPWithGroups(t *testing.T) { +func addAuthSourceLDAPWithGroupsUsingDN(t *testing.T) { session := loginUser(t, "user1") csrf := GetCSRF(t, session, "/admin/auths/new") req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ @@ -146,16 +146,34 @@ func addAuthSourceLDAPWithGroups(t *testing.T) { session.MakeRequest(t, req, http.StatusFound) } -func TestLDAPUserSignin(t *testing.T) { - if skipLDAPTests() { - t.Skip() - return - } - prepareTestEnv(t) - addAuthSourceLDAP(t, "") - - u := gitLDAPUsers[0] +func addAuthSourceLDAPWithGroupsUsingCN(t *testing.T) { + session := loginUser(t, "user1") + csrf := GetCSRF(t, session, "/admin/auths/new") + req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{ + "_csrf": csrf, + "type": "2", + "name": "ldap", + "host": getLDAPServerHost(), + "port": "389", + "bind_dn": "uid=gitea,ou=service,dc=planetexpress,dc=com", + "bind_password": "password", + "user_base": "ou=people,dc=planetexpress,dc=com", + "filter": "(&(objectClass=inetOrgPerson)(uid=%s))", + "group_search_base": "ou=people,dc=planetexpress,dc=com", + "group_search_filter": "(&(objectClass=groupOfNames)(member=cn=%s,ou=people,dc=planetexpress,dc=com))", + "user_attribute_in_group": "cn", + "member_group_filter": "(cn=git)", + "admin_group_filter": "(cn=admin_staff)", + "attribute_username": "uid", + "attribute_name": "givenName", + "attribute_surname": "sn", + "attribute_mail": "mail", + "is_active": "on", + }) + session.MakeRequest(t, req, http.StatusFound) +} +func testSingleLDAPSignin(t *testing.T, u *ldapUser) { session := loginUserWithPassword(t, u.UserName, u.Password) req := NewRequest(t, "GET", "/user/settings") resp := session.MakeRequest(t, req, http.StatusOK) @@ -165,35 +183,51 @@ func TestLDAPUserSignin(t *testing.T) { assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name")) assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name")) assert.Equal(t, u.Email, htmlDoc.GetInputValueByName("email")) + + reqAdmin := NewRequest(t, "GET", "/admin") + if u.IsAdmin { + adminResp := session.MakeRequest(t, reqAdmin, http.StatusOK) + assert.Equal(t, http.StatusOK, adminResp.Code) + } else { + adminResp := session.MakeRequest(t, reqAdmin, http.StatusForbidden) + assert.Equal(t, http.StatusForbidden, adminResp.Code) + } } -func TestLDAPUserSigninWithGroups(t *testing.T) { +func TestLDAPUserSignin(t *testing.T) { if skipLDAPTests() { t.Skip() return } prepareTestEnv(t) - addAuthSourceLDAPWithGroups(t) + addAuthSourceLDAP(t, "") - for _, u := range gitLDAPUsers { - session := loginUserWithPassword(t, u.UserName, u.Password) - req := NewRequest(t, "GET", "/user/settings") - resp := session.MakeRequest(t, req, http.StatusOK) + testSingleLDAPSignin(t, &gitLDAPUsers[0]) +} - htmlDoc := NewHTMLParser(t, resp.Body) +func TestLDAPUserSigninWithGroupsUsingDN(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + prepareTestEnv(t) + addAuthSourceLDAPWithGroupsUsingDN(t) - assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name")) - assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name")) - assert.Equal(t, u.Email, htmlDoc.GetInputValueByName("email")) + for _, u := range gitLDAPUsers { + testSingleLDAPSignin(t, &u) + } +} - reqAdmin := NewRequest(t, "GET", "/admin") - if u.IsAdmin { - adminResp := session.MakeRequest(t, reqAdmin, http.StatusOK) - assert.Equal(t, http.StatusOK, adminResp.Code) - } else { - adminResp := session.MakeRequest(t, reqAdmin, http.StatusForbidden) - assert.Equal(t, http.StatusForbidden, adminResp.Code) - } +func TestLDAPUserSigninWithGroupsUsingCN(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + prepareTestEnv(t) + addAuthSourceLDAPWithGroupsUsingCN(t) + + for _, u := range gitLDAPUsers { + testSingleLDAPSignin(t, &u) } } @@ -256,13 +290,26 @@ func TestLDAPUserSigninFailed(t *testing.T) { testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) } -func TestLDAPUserSigninFailedWithGroups(t *testing.T) { +func TestLDAPUserSigninFailedWithGroupsUsignDN(t *testing.T) { + if skipLDAPTests() { + t.Skip() + return + } + prepareTestEnv(t) + addAuthSourceLDAPWithGroupsUsingDN(t) + + for _, u := range otherLDAPUsers { + testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) + } +} + +func TestLDAPUserSigninFailedWithGroupsUsignCN(t *testing.T) { if skipLDAPTests() { t.Skip() return } prepareTestEnv(t) - addAuthSourceLDAPWithGroups(t) + addAuthSourceLDAPWithGroupsUsingCN(t) for _, u := range otherLDAPUsers { testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) From 11acb53c6ef13b32c1e54ee0d69bd335d815ec5f Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Tue, 7 Jan 2020 09:45:08 +0100 Subject: [PATCH 17/26] LDAP: Preallocate results array (as suggested by zeripath) Co-Authored-By: zeripath --- 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 aa81a08019c9e..2ea4de23de2e2 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -427,7 +427,7 @@ func (ls *Source) SearchEntries() ([]*SearchResult, error) { return nil, err } - results := []*SearchResult{} + results := make([]*SearchResult, 0, len(sr.Entries)) for _, v := range sr.Entries { result := ls.ProcessUserEntry(v, l) From b8ecb988d676b4a432420fee74daa80d1d362617 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sun, 19 Jan 2020 12:57:45 +0100 Subject: [PATCH 18/26] LDAP: Apply suggestions from code review prepareTestEnv(t) --> defer prepareTestEnv(t)() Co-Authored-By: zeripath --- integrations/auth_ldap_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 1ff5ed084d747..71dea29433c4c 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -200,7 +200,7 @@ func TestLDAPUserSignin(t *testing.T) { t.Skip() return } - defer prepareTestEnv(t) + defer prepareTestEnv(t)() addAuthSourceLDAP(t, "") testSingleLDAPSignin(t, &gitLDAPUsers[0]) @@ -211,7 +211,7 @@ func TestLDAPUserSigninWithGroupsUsingDN(t *testing.T) { t.Skip() return } - prepareTestEnv(t) + defer prepareTestEnv(t)() addAuthSourceLDAPWithGroupsUsingDN(t) for _, u := range gitLDAPUsers { @@ -224,7 +224,7 @@ func TestLDAPUserSigninWithGroupsUsingCN(t *testing.T) { t.Skip() return } - prepareTestEnv(t) + defer prepareTestEnv(t)() addAuthSourceLDAPWithGroupsUsingCN(t) for _, u := range gitLDAPUsers { @@ -296,7 +296,7 @@ func TestLDAPUserSigninFailedWithGroupsUsignDN(t *testing.T) { t.Skip() return } - prepareTestEnv(t) + defer prepareTestEnv(t)() addAuthSourceLDAPWithGroupsUsingDN(t) for _, u := range otherLDAPUsers { @@ -309,7 +309,7 @@ func TestLDAPUserSigninFailedWithGroupsUsignCN(t *testing.T) { t.Skip() return } - prepareTestEnv(t) + defer prepareTestEnv(t)() addAuthSourceLDAPWithGroupsUsingCN(t) for _, u := range otherLDAPUsers { From 2bcd3ca175cef24906de5a53abf37023cb053295 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 30 Mar 2020 21:23:48 +0100 Subject: [PATCH 19/26] Only request ls.UserAttributeInGroup if it is set Signed-off-by: Andrew Thornton --- modules/auth/ldap/ldap.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 18f3bde4aead0..0aa6554efc97b 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -383,10 +383,11 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul return nil } - var isAttributeSSHPublicKeySet = len(strings.TrimSpace(ls.AttributeSSHPublicKey)) > 0 - - attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail, ls.UserAttributeInGroup} - if isAttributeSSHPublicKeySet { + attribs := []string{ls.AttributeUsername, ls.AttributeName, ls.AttributeSurname, ls.AttributeMail} + if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { + attribs = append(attribs, ls.UserAttributeInGroup) + } + if len(strings.TrimSpace(ls.AttributeSSHPublicKey)) > 0 { attribs = append(attribs, ls.AttributeSSHPublicKey) } From 1b2674ef83f517eabdc4199453b02687bb2c122c Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 30 Mar 2020 22:35:00 +0100 Subject: [PATCH 20/26] Update cmd/admin_auth_ldap.go --- cmd/admin_auth_ldap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go index 3d5cadc0efc06..68c1380851672 100644 --- a/cmd/admin_auth_ldap.go +++ b/cmd/admin_auth_ldap.go @@ -67,7 +67,7 @@ var ( }, cli.StringFlag{ Name: "member-group-filter", - Usage: "An LDAP filter specifying if a group should be given administrator privileges.", + Usage: "An LDAP filter specifying if a group should be allowed to login.", }, cli.StringFlag{ Name: "admin-group-filter", From 6c95d6a7f21c9735ff105fde9e809314c530fd5a Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 9 Apr 2020 22:13:44 +0100 Subject: [PATCH 21/26] fix misplaced divs Co-Authored-By: 6543 <6543@obermui.de> --- templates/admin/auth/edit.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 2ea7bd61b7d07..d0fa9ed06effd 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -75,10 +75,10 @@
-

{{.i18n.Tr "admin.auths.restricted_filter_helper"}}

+
From e958a5b1b660a0637d646f2cae0fe8bdecf4933a Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 12 Apr 2020 19:39:29 +0100 Subject: [PATCH 22/26] Apply suggestions from code review Thank you @vaab those are good catches Co-Authored-By: Valentin Lab --- integrations/auth_ldap_test.go | 2 +- modules/auth/ldap/ldap.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index fa9ee9a312d3e..cc7c3c6aef12a 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -299,7 +299,7 @@ func TestLDAPUserSigninFailed(t *testing.T) { testLoginFailed(t, u.UserName, u.Password, i18n.Tr("en", "form.username_password_incorrect")) } -func TestLDAPUserSigninFailedWithGroupsUsignDN(t *testing.T) { +func TestLDAPUserSigninFailedWithGroupsUsingDN(t *testing.T) { if skipLDAPTests() { t.Skip() return diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 0aa6554efc97b..838794b874ea2 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -283,7 +283,7 @@ func (ls *Source) ProcessUserEntry(entry *ldap.Entry, l *ldap.Conn) *SearchResul } if len(strings.TrimSpace(ls.RestrictedGroupFilter)) > 0 { - hasRestrictedGroup = ls.CheckGroupFilter(l, sr, ls.RestrictedFilter) + hasRestrictedGroup = ls.CheckGroupFilter(l, sr, ls.RestrictedGroupFilter) if hasRestrictedGroup && !hasAdminGroup { log.Info("LDAP user %s is in restricted group!", username) } From 6c8d75bcecf02a0f6193a15772fdcd997e009ea6 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 12 Apr 2020 19:42:32 +0100 Subject: [PATCH 23/26] Apply suggestions from code review Co-Authored-By: Valentin Lab --- integrations/auth_ldap_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index cc7c3c6aef12a..b756cf4b0c63b 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -312,7 +312,7 @@ func TestLDAPUserSigninFailedWithGroupsUsingDN(t *testing.T) { } } -func TestLDAPUserSigninFailedWithGroupsUsignCN(t *testing.T) { +func TestLDAPUserSigninFailedWithGroupsUsingCN(t *testing.T) { if skipLDAPTests() { t.Skip() return From 6e8e615d8c33675851b88522b100ec8615cf830f Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sat, 2 May 2020 13:35:59 +0200 Subject: [PATCH 24/26] Rework admin and restricted checks with LDAP groups - If both admin filter and admin group filter are set, the user must pass both tests (before, the conditions were ORed) - Same for restricted users - Log restricted group membership regardless of admin group membership (Maybe we should display a warning in that case) - Rename variables 'hasXxxGroup' to 'isInXxxGroup' --- modules/auth/ldap/ldap.go | 40 ++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 838794b874ea2..664064d9b9405 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -243,8 +243,13 @@ func (ls *Source) ProcessUserEntry(entry *ldap.Entry, l *ldap.Conn) *SearchResul sshPublicKey = entry.GetAttributeValues(ls.AttributeSSHPublicKey) } - var hasAdminGroup = false - var hasRestrictedGroup = false + adminFilterSet := len(strings.TrimSpace(ls.AdminFilter)) > 0 + adminGroupFilterSet := len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 + restrictedFilterSet := len(strings.TrimSpace(ls.RestrictedFilter)) > 0 + restrictedGroupFilterSet := len(strings.TrimSpace(ls.RestrictedGroupFilter)) > 0 + + var isInAdminGroup = false + var isInRestrictedGroup = false if len(strings.TrimSpace(ls.GroupSearchBase)) > 0 && len(strings.TrimSpace(ls.GroupSearchFilter)) > 0 { var groupUID string if len(strings.TrimSpace(ls.UserAttributeInGroup)) > 0 { @@ -275,26 +280,39 @@ func (ls *Source) ProcessUserEntry(entry *ldap.Entry, l *ldap.Conn) *SearchResul } } - if len(strings.TrimSpace(ls.AdminGroupFilter)) > 0 { - hasAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) - if hasAdminGroup { + if adminGroupFilterSet { + isInAdminGroup = ls.CheckGroupFilter(l, sr, ls.AdminGroupFilter) + if isInAdminGroup { log.Info("LDAP user %s is in admin group!", username) } } - if len(strings.TrimSpace(ls.RestrictedGroupFilter)) > 0 { - hasRestrictedGroup = ls.CheckGroupFilter(l, sr, ls.RestrictedGroupFilter) - if hasRestrictedGroup && !hasAdminGroup { + if restrictedGroupFilterSet { + isInRestrictedGroup = ls.CheckGroupFilter(l, sr, ls.RestrictedGroupFilter) + if isInRestrictedGroup { log.Info("LDAP user %s is in restricted group!", username) } } } - var isRestricted = false + var isAdmin = false; + if adminFilterSet && adminGroupFilterSet { + isAdmin = isInAdminGroup && checkAdmin(l, ls, entry.DN) + } else if adminFilterSet { + isAdmin = checkAdmin(l, ls, entry.DN) + } else if adminGroupFilterSet { + isAdmin = isInAdminGroup + } - isAdmin := hasAdminGroup || checkAdmin(l, ls, entry.DN) + var isRestricted = false if !isAdmin { - isRestricted = hasRestrictedGroup || checkRestricted(l, ls, entry.DN) + if restrictedFilterSet && restrictedGroupFilterSet { + isRestricted = isInRestrictedGroup && checkRestricted(l, ls, entry.DN) + } else if restrictedFilterSet { + isRestricted = checkRestricted(l, ls, entry.DN) + } else if restrictedGroupFilterSet { + isRestricted = isInRestrictedGroup + } } return &SearchResult{ From e8a4465ef5b2d00266814a0fe29e15e83d689169 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sat, 2 May 2020 14:21:49 +0200 Subject: [PATCH 25/26] Added some helping notes on LDAP groups for admins --- cmd/admin_auth_ldap.go | 12 ++++++------ options/locale/locale_en-US.ini | 5 ++++- templates/admin/auth/edit.tmpl | 5 +++++ templates/admin/auth/source/ldap.tmpl | 5 +++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go index 68c1380851672..c0e707af99ff8 100644 --- a/cmd/admin_auth_ldap.go +++ b/cmd/admin_auth_ldap.go @@ -67,27 +67,27 @@ var ( }, cli.StringFlag{ Name: "member-group-filter", - Usage: "An LDAP filter specifying if a group should be allowed to login.", + Usage: "An LDAP filter specifying if a group should be allowed to login. If both user filter and member group filter are set, the user must satisfy both conditions to log in.", }, cli.StringFlag{ Name: "admin-group-filter", - Usage: "An LDAP filter specifying an admin user group.", + Usage: "An LDAP filter specifying an admin user group. If both admin filter and admin group filter are set, the user must satisfy both conditions to get admin privileges.", }, cli.StringFlag{ Name: "restricted-group-filter", - Usage: "An LDAP filter specifying a restricted user group.", + Usage: "An LDAP filter specifying a restricted user group. If both restricted filter and restricted group filter are set, the user must satisfy both conditions to be a restricted user.", }, cli.StringFlag{ Name: "user-filter", - Usage: "An LDAP filter declaring how to find the user record that is attempting to authenticate.", + Usage: "An LDAP filter declaring how to find the user record that is attempting to authenticate. If both user filter and member group filter are set, the user must satisfy both conditions to log in.", }, cli.StringFlag{ Name: "admin-filter", - Usage: "An LDAP filter specifying if a user should be given administrator privileges.", + Usage: "An LDAP filter specifying if a user should be given administrator privileges. If both admin filter and admin group filter are set, the user must satisfy both conditions to get admin privileges.", }, cli.StringFlag{ Name: "restricted-filter", - Usage: "An LDAP filter specifying if a user should be given restricted status.", + Usage: "An LDAP filter specifying if a user should be given restricted status. Use an asterisk ('*') to set all users that do not match Admin Filter as restricted. If both restricted filter and restricted group filter are set, the user must satisfy both conditions to be a restricted user.", }, cli.BoolFlag{ Name: "allow-deactivate-all", diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index adaa619d3255a..0be24e7427eb6 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1977,9 +1977,11 @@ auths.allow_deactivate_all = Allow an empty search result to deactivate all user auths.use_paged_search = Use Paged Search auths.search_page_size = Page Size auths.filter = User Filter +auths.filter_helper = If both user filter and member group filter are set, the user must satisfy both conditions to log in. auths.admin_filter = Admin Filter +auths.admin_filter_helper = If both admin filter and admin group filter are set, the user must satisfy both conditions to get admin privileges. 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.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. If both restricted filter and restricted group filter are set, the user must satisfy both conditions to be a restricted user. auths.group_search_base = Group Search Base auths.group_search_filter = Group Search Filter auths.user_attribute_in_group = User Attribute in Group @@ -1987,6 +1989,7 @@ auths.user_attribute_in_group_helper = The value of this user attribute is inser auths.member_group_filter = Member Group Filter auths.admin_group_filter = Admin Group Filter auths.restricted_group_filter = Restricted Group Filter +auths.restricted_group_filter_helper = If both restricted filter and restricted group filter are set, the user must satisfy both conditions to be a restricted user. auths.ms_ad_sa = MS AD Search Attributes auths.smtp_auth = SMTP Authentication Type auths.smtphost = SMTP Host diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index d0fa9ed06effd..c4bc7b462c6c9 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -69,10 +69,12 @@
+

{{.i18n.Tr "admin.auths.filter_helper"}}

+

{{.i18n.Tr "admin.auths.admin_filter_helper"}}

@@ -95,14 +97,17 @@
+

{{.i18n.Tr "admin.auths.filter_helper"}}

+

{{.i18n.Tr "admin.auths.admin_filter_helper"}}

+

{{.i18n.Tr "admin.auths.restricted_group_filter_helper"}}

diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 11fde95891cb3..b33e6e7206686 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -41,10 +41,12 @@
+

{{.i18n.Tr "admin.auths.filter_helper"}}

+

{{.i18n.Tr "admin.auths.admin_filter_helper"}}

@@ -67,14 +69,17 @@
+

{{.i18n.Tr "admin.auths.filter_helper"}}

+

{{.i18n.Tr "admin.auths.admin_filter_helper"}}

+

{{.i18n.Tr "admin.auths.restricted_group_filter_helper"}}

From f3fe8827ec472665a8b7c3b81093d0b8cb5630a7 Mon Sep 17 00:00:00 2001 From: Hermann von Kleist Date: Sat, 2 May 2020 14:26:10 +0200 Subject: [PATCH 26/26] Fixed formatting in 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 664064d9b9405..4694d10c8263c 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -295,7 +295,7 @@ func (ls *Source) ProcessUserEntry(entry *ldap.Entry, l *ldap.Conn) *SearchResul } } - var isAdmin = false; + var isAdmin = false if adminFilterSet && adminGroupFilterSet { isAdmin = isInAdminGroup && checkAdmin(l, ls, entry.DN) } else if adminFilterSet {