Skip to content

Commit 37c3db7

Browse files
lafrikslafriks
authored
Add restricted user filter to LDAP authentication (#10600)
* Add restricted user filter to LDAP authentification * Fix unit test cases
1 parent be544e8 commit 37c3db7

File tree

12 files changed

+146
-52
lines changed

12 files changed

+146
-52
lines changed

‎cmd/admin_auth_ldap.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ var (
6161
Name: "admin-filter",
6262
Usage: "An LDAP filter specifying if a user should be given administrator privileges.",
6363
},
64+
cli.StringFlag{
65+
Name: "restricted-filter",
66+
Usage: "An LDAP filter specifying if a user should be given restricted status.",
67+
},
6468
cli.BoolFlag{
6569
Name: "allow-deactivate-all",
6670
Usage: "Allow empty search results to deactivate all users.",
@@ -235,6 +239,9 @@ func parseLdapConfig(c *cli.Context, config *models.LDAPConfig) error {
235239
if c.IsSet("admin-filter") {
236240
config.Source.AdminFilter = c.String("admin-filter")
237241
}
242+
if c.IsSet("restricted-filter") {
243+
config.Source.RestrictedFilter = c.String("restricted-filter")
244+
}
238245
if c.IsSet("allow-deactivate-all") {
239246
config.Source.AllowDeactivateAll = c.Bool("allow-deactivate-all")
240247
}

‎cmd/admin_auth_ldap_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ func TestAddLdapBindDn(t *testing.T) {
3939
"--user-search-base", "ou=Users,dc=full-domain-bind,dc=org",
4040
"--user-filter", "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)",
4141
"--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)",
42+
"--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)",
4243
"--username-attribute", "uid-bind full",
4344
"--firstname-attribute", "givenName-bind full",
4445
"--surname-attribute", "sn-bind full",
@@ -74,6 +75,7 @@ func TestAddLdapBindDn(t *testing.T) {
7475
SearchPageSize: 99,
7576
Filter: "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)",
7677
AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)",
78+
RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)",
7779
Enabled: true,
7880
},
7981
},
@@ -265,6 +267,7 @@ func TestAddLdapSimpleAuth(t *testing.T) {
265267
"--user-search-base", "ou=Users,dc=full-domain-simple,dc=org",
266268
"--user-filter", "(&(objectClass=posixAccount)(full-simple-cn=%s))",
267269
"--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)",
270+
"--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)",
268271
"--username-attribute", "uid-simple full",
269272
"--firstname-attribute", "givenName-simple full",
270273
"--surname-attribute", "sn-simple full",
@@ -292,6 +295,7 @@ func TestAddLdapSimpleAuth(t *testing.T) {
292295
AttributeSSHPublicKey: "publickey-simple full",
293296
Filter: "(&(objectClass=posixAccount)(full-simple-cn=%s))",
294297
AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)",
298+
RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)",
295299
Enabled: true,
296300
},
297301
},
@@ -499,6 +503,7 @@ func TestUpdateLdapBindDn(t *testing.T) {
499503
"--user-search-base", "ou=Users,dc=full-domain-bind,dc=org",
500504
"--user-filter", "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)",
501505
"--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)",
506+
"--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)",
502507
"--username-attribute", "uid-bind full",
503508
"--firstname-attribute", "givenName-bind full",
504509
"--surname-attribute", "sn-bind full",
@@ -543,6 +548,7 @@ func TestUpdateLdapBindDn(t *testing.T) {
543548
SearchPageSize: 99,
544549
Filter: "(memberOf=cn=user-group,ou=example,dc=full-domain-bind,dc=org)",
545550
AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-bind,dc=org)",
551+
RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-bind,dc=org)",
546552
Enabled: true,
547553
},
548554
},
@@ -978,6 +984,7 @@ func TestUpdateLdapSimpleAuth(t *testing.T) {
978984
"--user-search-base", "ou=Users,dc=full-domain-simple,dc=org",
979985
"--user-filter", "(&(objectClass=posixAccount)(full-simple-cn=%s))",
980986
"--admin-filter", "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)",
987+
"--restricted-filter", "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)",
981988
"--username-attribute", "uid-simple full",
982989
"--firstname-attribute", "givenName-simple full",
983990
"--surname-attribute", "sn-simple full",
@@ -1006,6 +1013,7 @@ func TestUpdateLdapSimpleAuth(t *testing.T) {
10061013
AttributeSSHPublicKey: "publickey-simple full",
10071014
Filter: "(&(objectClass=posixAccount)(full-simple-cn=%s))",
10081015
AdminFilter: "(memberOf=cn=admin-group,ou=example,dc=full-domain-simple,dc=org)",
1016+
RestrictedFilter: "(memberOf=cn=restricted-group,ou=example,dc=full-domain-simple,dc=org)",
10091017
},
10101018
},
10111019
},

‎docs/content/doc/usage/command-line.en-us.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ Admin operations:
134134
- `--user-search-base value`: The LDAP base at which user accounts will be searched for. Required.
135135
- `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. Required.
136136
- `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges.
137+
- `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status.
137138
- `--username-attribute value`: The attribute of the user’s LDAP record containing the user name.
138139
- `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name.
139140
- `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname.
@@ -158,6 +159,7 @@ Admin operations:
158159
- `--user-search-base value`: The LDAP base at which user accounts will be searched for.
159160
- `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate.
160161
- `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges.
162+
- `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status.
161163
- `--username-attribute value`: The attribute of the user’s LDAP record containing the user name.
162164
- `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name.
163165
- `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname.
@@ -182,6 +184,7 @@ Admin operations:
182184
- `--user-search-base value`: The LDAP base at which user accounts will be searched for.
183185
- `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate. Required.
184186
- `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges.
187+
- `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status.
185188
- `--username-attribute value`: The attribute of the user’s LDAP record containing the user name.
186189
- `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name.
187190
- `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname.
@@ -202,6 +205,7 @@ Admin operations:
202205
- `--user-search-base value`: The LDAP base at which user accounts will be searched for.
203206
- `--user-filter value`: An LDAP filter declaring how to find the user record that is attempting to authenticate.
204207
- `--admin-filter value`: An LDAP filter specifying if a user should be given administrator privileges.
208+
- `--restricted-filter value`: An LDAP filter specifying if a user should be given restricted status.
205209
- `--username-attribute value`: The attribute of the user’s LDAP record containing the user name.
206210
- `--firstname-attribute value`: The attribute of the user’s LDAP record containing the user’s first name.
207211
- `--surname-attribute value`: The attribute of the user’s LDAP record containing the user’s surname.
@@ -313,4 +317,4 @@ var checklist = []check{
313317
}
314318
```
315319

316-
This function will receive a command line context and return a list of details about the problems or error.
320+
This function will receive a command line context and return a list of details about the problems or error.

‎integrations/auth_ldap_test.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ import (
1818
)
1919

2020
type ldapUser struct {
21-
UserName string
22-
Password string
23-
FullName string
24-
Email string
25-
OtherEmails []string
26-
IsAdmin bool
27-
SSHKeys []string
21+
UserName string
22+
Password string
23+
FullName string
24+
Email string
25+
OtherEmails []string
26+
IsAdmin bool
27+
IsRestricted bool
28+
SSHKeys []string
2829
}
2930

3031
var gitLDAPUsers = []ldapUser{
@@ -55,10 +56,11 @@ var gitLDAPUsers = []ldapUser{
5556
Email: "[email protected]",
5657
},
5758
{
58-
UserName: "leela",
59-
Password: "leela",
60-
FullName: "Leela Turanga",
61-
Email: "[email protected]",
59+
UserName: "leela",
60+
Password: "leela",
61+
FullName: "Leela Turanga",
62+
Email: "[email protected]",
63+
IsRestricted: true,
6264
},
6365
{
6466
UserName: "bender",
@@ -109,6 +111,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) {
109111
"user_base": "ou=people,dc=planetexpress,dc=com",
110112
"filter": "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))",
111113
"admin_filter": "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)",
114+
"restricted_filter": "(uid=leela)",
112115
"attribute_username": "uid",
113116
"attribute_name": "givenName",
114117
"attribute_surname": "sn",
@@ -173,6 +176,11 @@ func TestLDAPUserSync(t *testing.T) {
173176
} else {
174177
assert.True(t, tds.Find("td:nth-child(5) i").HasClass("fa-square-o"))
175178
}
179+
if u.IsRestricted {
180+
assert.True(t, tds.Find("td:nth-child(6) i").HasClass("fa-check-square-o"))
181+
} else {
182+
assert.True(t, tds.Find("td:nth-child(6) i").HasClass("fa-square-o"))
183+
}
176184
}
177185

178186
// Check if no users exist

‎models/login_source.go

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -475,13 +475,23 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use
475475
return nil, err
476476
}
477477
}
478-
if user != nil &&
479-
!user.ProhibitLogin && len(source.LDAP().AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin {
480-
// Change existing admin flag only if AdminFilter option is set
481-
user.IsAdmin = sr.IsAdmin
482-
err = UpdateUserCols(user, "is_admin")
483-
if err != nil {
484-
return nil, err
478+
if user != nil && !user.ProhibitLogin {
479+
cols := make([]string, 0)
480+
if len(source.LDAP().AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin {
481+
// Change existing admin flag only if AdminFilter option is set
482+
user.IsAdmin = sr.IsAdmin
483+
cols = append(cols, "is_admin")
484+
}
485+
if !user.IsAdmin && len(source.LDAP().RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted {
486+
// Change existing restricted flag only if RestrictedFilter option is set
487+
user.IsRestricted = sr.IsRestricted
488+
cols = append(cols, "is_restricted")
489+
}
490+
if len(cols) > 0 {
491+
err = UpdateUserCols(user, cols...)
492+
if err != nil {
493+
return nil, err
494+
}
485495
}
486496
}
487497
}
@@ -504,15 +514,16 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use
504514
}
505515

506516
user = &User{
507-
LowerName: strings.ToLower(sr.Username),
508-
Name: sr.Username,
509-
FullName: composeFullName(sr.Name, sr.Surname, sr.Username),
510-
Email: sr.Mail,
511-
LoginType: source.Type,
512-
LoginSource: source.ID,
513-
LoginName: login,
514-
IsActive: true,
515-
IsAdmin: sr.IsAdmin,
517+
LowerName: strings.ToLower(sr.Username),
518+
Name: sr.Username,
519+
FullName: composeFullName(sr.Name, sr.Surname, sr.Username),
520+
Email: sr.Mail,
521+
LoginType: source.Type,
522+
LoginSource: source.ID,
523+
LoginName: login,
524+
IsActive: true,
525+
IsAdmin: sr.IsAdmin,
526+
IsRestricted: sr.IsRestricted,
516527
}
517528

518529
err := CreateUser(user)

‎models/user.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1875,15 +1875,16 @@ func SyncExternalUsers(ctx context.Context) {
18751875
log.Trace("SyncExternalUsers[%s]: Creating user %s", s.Name, su.Username)
18761876

18771877
usr = &User{
1878-
LowerName: strings.ToLower(su.Username),
1879-
Name: su.Username,
1880-
FullName: fullName,
1881-
LoginType: s.Type,
1882-
LoginSource: s.ID,
1883-
LoginName: su.Username,
1884-
Email: su.Mail,
1885-
IsAdmin: su.IsAdmin,
1886-
IsActive: true,
1878+
LowerName: strings.ToLower(su.Username),
1879+
Name: su.Username,
1880+
FullName: fullName,
1881+
LoginType: s.Type,
1882+
LoginSource: s.ID,
1883+
LoginName: su.Username,
1884+
Email: su.Mail,
1885+
IsAdmin: su.IsAdmin,
1886+
IsRestricted: su.IsRestricted,
1887+
IsActive: true,
18871888
}
18881889

18891890
err = CreateUser(usr)
@@ -1906,6 +1907,7 @@ func SyncExternalUsers(ctx context.Context) {
19061907

19071908
// Check if user data has changed
19081909
if (len(s.LDAP().AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) ||
1910+
(len(s.LDAP().RestrictedFilter) > 0 && usr.IsRestricted != su.IsRestricted) ||
19091911
!strings.EqualFold(usr.Email, su.Mail) ||
19101912
usr.FullName != fullName ||
19111913
!usr.IsActive {
@@ -1918,9 +1920,13 @@ func SyncExternalUsers(ctx context.Context) {
19181920
if len(s.LDAP().AdminFilter) > 0 {
19191921
usr.IsAdmin = su.IsAdmin
19201922
}
1923+
// Change existing restricted flag only if RestrictedFilter option is set
1924+
if !usr.IsAdmin && len(s.LDAP().RestrictedFilter) > 0 {
1925+
usr.IsRestricted = su.IsRestricted
1926+
}
19211927
usr.IsActive = true
19221928

1923-
err = UpdateUserCols(usr, "full_name", "email", "is_admin", "is_active")
1929+
err = UpdateUserCols(usr, "full_name", "email", "is_admin", "is_restricted", "is_active")
19241930
if err != nil {
19251931
log.Error("SyncExternalUsers[%s]: Error updating user %s: %v", s.Name, usr.Name, err)
19261932
}

‎modules/auth/auth_form.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type AuthenticationForm struct {
3030
SearchPageSize int
3131
Filter string
3232
AdminFilter string
33+
RestrictedFilter string
3334
AllowDeactivateAll bool
3435
IsActive bool
3536
IsSyncEnabled bool

0 commit comments

Comments
 (0)