From 37c3db7be6dd6fc5ee085979cc5f5dda09d978c3 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Thu, 5 Mar 2020 08:30:33 +0200 Subject: [PATCH] Add restricted user filter to LDAP authentication (#10600) * Add restricted user filter to LDAP authentification * Fix unit test cases --- cmd/admin_auth_ldap.go | 7 +++ cmd/admin_auth_ldap_test.go | 8 +++ docs/content/doc/usage/command-line.en-us.md | 6 +- integrations/auth_ldap_test.go | 30 +++++---- models/login_source.go | 43 ++++++++----- models/user.go | 26 +++++--- modules/auth/auth_form.go | 1 + modules/auth/ldap/ldap.go | 64 +++++++++++++++----- options/locale/locale_en-US.ini | 2 + routers/admin/auths.go | 1 + templates/admin/auth/edit.tmpl | 5 ++ templates/admin/auth/source/ldap.tmpl | 5 ++ 12 files changed, 146 insertions(+), 52 deletions(-) diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go index e869686cbd..5ab64ec7d5 100644 --- a/cmd/admin_auth_ldap.go +++ b/cmd/admin_auth_ldap.go @@ -61,6 +61,10 @@ var ( Name: "admin-filter", Usage: "An LDAP filter specifying if a user should be given administrator privileges.", }, + cli.StringFlag{ + Name: "restricted-filter", + Usage: "An LDAP filter specifying if a user should be given restricted status.", + }, cli.BoolFlag{ Name: "allow-deactivate-all", Usage: "Allow empty search results to deactivate all users.", @@ -235,6 +239,9 @@ func parseLdapConfig(c *cli.Context, config *models.LDAPConfig) error { if c.IsSet("admin-filter") { config.Source.AdminFilter = c.String("admin-filter") } + if c.IsSet("restricted-filter") { + config.Source.RestrictedFilter = c.String("restricted-filter") + } if c.IsSet("allow-deactivate-all") { config.Source.AllowDeactivateAll = c.Bool("allow-deactivate-all") } diff --git a/cmd/admin_auth_ldap_test.go b/cmd/admin_auth_ldap_test.go index 4af9f167c3..87f4f789ab 100644 --- a/cmd/admin_auth_ldap_test.go +++ b/cmd/admin_auth_ldap_test.go @@ -39,6 +39,7 @@ func TestAddLdapBindDn(t *testing.T) { "--user-search-base", "ou=Users,dc=full-domain-bind,dc=org", "--user-filter", "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)", "--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)", + "--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)", "--username-attribute", "uid-bind full", "--firstname-attribute", "givenName-bind full", "--surname-attribute", "sn-bind full", @@ -74,6 +75,7 @@ func TestAddLdapBindDn(t *testing.T) { SearchPageSize: 99, Filter: "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)", AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)", + RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)", Enabled: true, }, }, @@ -265,6 +267,7 @@ func TestAddLdapSimpleAuth(t *testing.T) { "--user-search-base", "ou=Users,dc=full-domain-simple,dc=org", "--user-filter", "(&(objectClass=posixAccount)(full-simple-cn=%s))", "--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)", + "--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)", "--username-attribute", "uid-simple full", "--firstname-attribute", "givenName-simple full", "--surname-attribute", "sn-simple full", @@ -292,6 +295,7 @@ func TestAddLdapSimpleAuth(t *testing.T) { AttributeSSHPublicKey: "publickey-simple full", Filter: "(&(objectClass=posixAccount)(full-simple-cn=%s))", AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)", + RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)", Enabled: true, }, }, @@ -499,6 +503,7 @@ func TestUpdateLdapBindDn(t *testing.T) { "--user-search-base", "ou=Users,dc=full-domain-bind,dc=org", "--user-filter", "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)", "--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)", + "--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)", "--username-attribute", "uid-bind full", "--firstname-attribute", "givenName-bind full", "--surname-attribute", "sn-bind full", @@ -543,6 +548,7 @@ func TestUpdateLdapBindDn(t *testing.T) { SearchPageSize: 99, Filter: "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)", AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)", + RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)", Enabled: true, }, }, @@ -978,6 +984,7 @@ func TestUpdateLdapSimpleAuth(t *testing.T) { "--user-search-base", "ou=Users,dc=full-domain-simple,dc=org", "--user-filter", "(&(objectClass=posixAccount)(full-simple-cn=%s))", "--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)", + "--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)", "--username-attribute", "uid-simple full", "--firstname-attribute", "givenName-simple full", "--surname-attribute", "sn-simple full", @@ -1006,6 +1013,7 @@ func TestUpdateLdapSimpleAuth(t *testing.T) { AttributeSSHPublicKey: "publickey-simple full", Filter: "(&(objectClass=posixAccount)(full-simple-cn=%s))", AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)", + RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)", }, }, }, diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index 60c2e26a7b..c0236f913d 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -134,6 +134,7 @@ Admin operations: - `--user-search-base value`: The LDAP base at which user accounts will be searched for. Required. - `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. Required. - `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges. + - `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status. - `--username-attribute value`: The attribute of the user’s LDAP record containing the user name. - `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name. - `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname. @@ -158,6 +159,7 @@ Admin operations: - `--user-search-base value`: The LDAP base at which user accounts will be searched for. - `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. - `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges. + - `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status. - `--username-attribute value`: The attribute of the user’s LDAP record containing the user name. - `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name. - `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname. @@ -182,6 +184,7 @@ Admin operations: - `--user-search-base value`: The LDAP base at which user accounts will be searched for. - `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. Required. - `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges. + - `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status. - `--username-attribute value`: The attribute of the user’s LDAP record containing the user name. - `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name. - `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname. @@ -202,6 +205,7 @@ Admin operations: - `--user-search-base value`: The LDAP base at which user accounts will be searched for. - `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. - `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges. + - `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status. - `--username-attribute value`: The attribute of the user’s LDAP record containing the user name. - `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name. - `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname. @@ -313,4 +317,4 @@ var checklist = []check{ } ``` -This function will receive a command line context and return a list of details about the problems or error. \ No newline at end of file +This function will receive a command line context and return a list of details about the problems or error. diff --git a/integrations/auth_ldap_test.go b/integrations/auth_ldap_test.go index 80286c09e6..6c6147f20e 100644 --- a/integrations/auth_ldap_test.go +++ b/integrations/auth_ldap_test.go @@ -18,13 +18,14 @@ import ( ) type ldapUser struct { - UserName string - Password string - FullName string - Email string - OtherEmails []string - IsAdmin bool - SSHKeys []string + UserName string + Password string + FullName string + Email string + OtherEmails []string + IsAdmin bool + IsRestricted bool + SSHKeys []string } var gitLDAPUsers = []ldapUser{ @@ -55,10 +56,11 @@ var gitLDAPUsers = []ldapUser{ Email: "fry@planetexpress.com", }, { - UserName: "leela", - Password: "leela", - FullName: "Leela Turanga", - Email: "leela@planetexpress.com", + UserName: "leela", + Password: "leela", + FullName: "Leela Turanga", + Email: "leela@planetexpress.com", + IsRestricted: true, }, { UserName: "bender", @@ -109,6 +111,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) { "user_base": "ou=people,dc=planetexpress,dc=com", "filter": "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))", "admin_filter": "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)", + "restricted_filter": "(uid=leela)", "attribute_username": "uid", "attribute_name": "givenName", "attribute_surname": "sn", @@ -173,6 +176,11 @@ func TestLDAPUserSync(t *testing.T) { } else { assert.True(t, tds.Find("td:nth-child(5) i").HasClass("fa-square-o")) } + if u.IsRestricted { + assert.True(t, tds.Find("td:nth-child(6) i").HasClass("fa-check-square-o")) + } else { + assert.True(t, tds.Find("td:nth-child(6) i").HasClass("fa-square-o")) + } } // Check if no users exist diff --git a/models/login_source.go b/models/login_source.go index 2774d6f80d..88028283e8 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -475,13 +475,23 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use return nil, err } } - if user != nil && - !user.ProhibitLogin && len(source.LDAP().AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { - // Change existing admin flag only if AdminFilter option is set - user.IsAdmin = sr.IsAdmin - err = UpdateUserCols(user, "is_admin") - if err != nil { - return nil, err + if user != nil && !user.ProhibitLogin { + cols := make([]string, 0) + if len(source.LDAP().AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { + // Change existing admin flag only if AdminFilter option is set + user.IsAdmin = sr.IsAdmin + cols = append(cols, "is_admin") + } + if !user.IsAdmin && len(source.LDAP().RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { + // Change existing restricted flag only if RestrictedFilter option is set + user.IsRestricted = sr.IsRestricted + cols = append(cols, "is_restricted") + } + if len(cols) > 0 { + err = UpdateUserCols(user, cols...) + if err != nil { + return nil, err + } } } } @@ -504,15 +514,16 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use } user = &User{ - LowerName: strings.ToLower(sr.Username), - Name: sr.Username, - FullName: composeFullName(sr.Name, sr.Surname, sr.Username), - Email: sr.Mail, - LoginType: source.Type, - LoginSource: source.ID, - LoginName: login, - IsActive: true, - IsAdmin: sr.IsAdmin, + LowerName: strings.ToLower(sr.Username), + Name: sr.Username, + FullName: composeFullName(sr.Name, sr.Surname, sr.Username), + Email: sr.Mail, + LoginType: source.Type, + LoginSource: source.ID, + LoginName: login, + IsActive: true, + IsAdmin: sr.IsAdmin, + IsRestricted: sr.IsRestricted, } err := CreateUser(user) diff --git a/models/user.go b/models/user.go index f91ffa7169..06f11c968c 100644 --- a/models/user.go +++ b/models/user.go @@ -1875,15 +1875,16 @@ func SyncExternalUsers(ctx context.Context) { log.Trace("SyncExternalUsers[%s]: Creating user %s", s.Name, su.Username) usr = &User{ - LowerName: strings.ToLower(su.Username), - Name: su.Username, - FullName: fullName, - LoginType: s.Type, - LoginSource: s.ID, - LoginName: su.Username, - Email: su.Mail, - IsAdmin: su.IsAdmin, - IsActive: true, + LowerName: strings.ToLower(su.Username), + Name: su.Username, + FullName: fullName, + LoginType: s.Type, + LoginSource: s.ID, + LoginName: su.Username, + Email: su.Mail, + IsAdmin: su.IsAdmin, + IsRestricted: su.IsRestricted, + IsActive: true, } err = CreateUser(usr) @@ -1906,6 +1907,7 @@ func SyncExternalUsers(ctx context.Context) { // Check if user data has changed if (len(s.LDAP().AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) || + (len(s.LDAP().RestrictedFilter) > 0 && usr.IsRestricted != su.IsRestricted) || !strings.EqualFold(usr.Email, su.Mail) || usr.FullName != fullName || !usr.IsActive { @@ -1918,9 +1920,13 @@ func SyncExternalUsers(ctx context.Context) { if len(s.LDAP().AdminFilter) > 0 { usr.IsAdmin = su.IsAdmin } + // Change existing restricted flag only if RestrictedFilter option is set + if !usr.IsAdmin && len(s.LDAP().RestrictedFilter) > 0 { + usr.IsRestricted = su.IsRestricted + } usr.IsActive = true - err = UpdateUserCols(usr, "full_name", "email", "is_admin", "is_active") + err = UpdateUserCols(usr, "full_name", "email", "is_admin", "is_restricted", "is_active") if err != nil { log.Error("SyncExternalUsers[%s]: Error updating user %s: %v", s.Name, usr.Name, err) } diff --git a/modules/auth/auth_form.go b/modules/auth/auth_form.go index a30ebb75eb..7fc62607e5 100644 --- a/modules/auth/auth_form.go +++ b/modules/auth/auth_form.go @@ -30,6 +30,7 @@ type AuthenticationForm struct { SearchPageSize int Filter string AdminFilter string + RestrictedFilter string AllowDeactivateAll bool IsActive bool IsSyncEnabled bool diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 7f0d2c93f3..66676f2829 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -46,6 +46,7 @@ type Source struct { SearchPageSize uint32 // Search with paging page size Filter string // Query filter to validate entry AdminFilter string // Query filter to check if user is admin + 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 } @@ -58,6 +59,7 @@ type SearchResult struct { Mail string // E-mail address SSHPublicKey []string // SSH Public Key IsAdmin bool // if user is administrator + IsRestricted bool // if user is restricted } func (ls *Source) sanitizedUserQuery(username string) (string, bool) { @@ -153,22 +155,48 @@ func bindUser(l *ldap.Conn, userDN, passwd string) error { } func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool { - if len(ls.AdminFilter) > 0 { - log.Trace("Checking admin with filter %s and base %s", ls.AdminFilter, userDN) - search := ldap.NewSearchRequest( - userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, ls.AdminFilter, - []string{ls.AttributeName}, - nil) + if len(ls.AdminFilter) == 0 { + return false + } + log.Trace("Checking admin with filter %s and base %s", ls.AdminFilter, userDN) + search := ldap.NewSearchRequest( + userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, ls.AdminFilter, + []string{ls.AttributeName}, + nil) - sr, err := l.Search(search) + sr, err := l.Search(search) - if err != nil { - log.Error("LDAP Admin Search failed unexpectedly! (%v)", err) - } else if len(sr.Entries) < 1 { - log.Trace("LDAP Admin Search found no matching entries.") - } else { - return true - } + if err != nil { + log.Error("LDAP Admin Search failed unexpectedly! (%v)", err) + } else if len(sr.Entries) < 1 { + log.Trace("LDAP Admin Search found no matching entries.") + } else { + return true + } + return false +} + +func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool { + if len(ls.RestrictedFilter) == 0 { + return false + } + if ls.RestrictedFilter == "*" { + return true + } + log.Trace("Checking restricted with filter %s and base %s", ls.RestrictedFilter, userDN) + search := ldap.NewSearchRequest( + userDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, ls.RestrictedFilter, + []string{ls.AttributeName}, + nil) + + sr, err := l.Search(search) + + if err != nil { + log.Error("LDAP Restrictred Search failed unexpectedly! (%v)", err) + } else if len(sr.Entries) < 1 { + log.Trace("LDAP Restricted Search found no matching entries.") + } else { + return true } return false } @@ -284,6 +312,10 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul sshPublicKey = sr.Entries[0].GetAttributeValues(ls.AttributeSSHPublicKey) } isAdmin := checkAdmin(l, ls, userDN) + var isRestricted bool + if !isAdmin { + isRestricted = checkRestricted(l, ls, userDN) + } if !directBind && ls.AttributesInBind { // binds user (checking password) after looking-up attributes in BindDN context @@ -300,6 +332,7 @@ func (ls *Source) SearchEntry(name, passwd string, directBind bool) *SearchResul Mail: mail, SSHPublicKey: sshPublicKey, IsAdmin: isAdmin, + IsRestricted: isRestricted, } } @@ -364,6 +397,9 @@ func (ls *Source) SearchEntries() ([]*SearchResult, error) { Mail: v.GetAttributeValue(ls.AttributeMail), IsAdmin: checkAdmin(l, ls, v.DN), } + if !result[i].IsAdmin { + result[i].IsRestricted = checkRestricted(l, ls, v.DN) + } if isAttributeSSHPublicKeySet { result[i].SSHPublicKey = v.GetAttributeValues(ls.AttributeSSHPublicKey) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b43fe51efc..375a2ec88d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1893,6 +1893,8 @@ auths.use_paged_search = Use Paged Search auths.search_page_size = Page Size 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.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 4fa76df44e..9b96f08031 100644 --- a/routers/admin/auths.go +++ b/routers/admin/auths.go @@ -130,6 +130,7 @@ func parseLDAPConfig(form auth.AuthenticationForm) *models.LDAPConfig { SearchPageSize: pageSize, Filter: form.Filter, AdminFilter: form.AdminFilter, + RestrictedFilter: form.RestrictedFilter, AllowDeactivateAll: form.AllowDeactivateAll, Enabled: true, }, diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 3f515bbf48..15ab2b227b 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -74,6 +74,11 @@ +
+ + +

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

+
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 3386884ff1..f5806c829c 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -46,6 +46,11 @@
+
+ + +

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

+