Skip to content

Commit 8a956f3

Browse files
rhaferbutonic
authored andcommitted
Adjust "groupfilter" to be able to search by member name (cs3org#2436)
Previously the input for the LDAP Groupfilter to lookup all groups a specific user is member of was the userpb.UserId part of the User object. I.e. it assumed we could run a single LDAP query to get all groups a user is member of by specifying the userid. However most LDAP Servers store the GroupMembership by either username (e.g. in memberUID Attribute) or by the user's DN (e.g. in member/uniqueMember). The GetUserGroups method was already updated recently to do a two-staged lookup (first lookup the user's name by Id then search the Groups by username). This change just removes the userpb.UserId template processing from the GroupFilter and replaces it with a single string (the username) to get rid of the annoying `{{.}}` template values in the config. In the future we should add a config switch to also allow lookups by member DN.
1 parent 626d28a commit 8a956f3

File tree

5 files changed

+22
-19
lines changed

5 files changed

+22
-19
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Change: Replace template in GroupFilter for UserProvider with a simple string
2+
3+
Previously the "groupfilter" configuration for the UserProvider expected a
4+
go-template value (based of of an `userpb.UserId` as it's input). And it
5+
assumed we could run a single LDAP query to get all groups a user is member of
6+
by specifying the userid. However most LDAP Servers store the GroupMembership
7+
by either username (e.g. in memberUID Attribute) or by the user's DN (e.g. in
8+
member/uniqueMember).
9+
10+
This change removes the userpb.UserId template processing from the groupfilter
11+
and replaces it with a single string (the username) to cleanup the config a
12+
bit. Existing configs need to be update to replace the go template references
13+
in `groupfilter` (e.g. `{{.}}` or `{{.OpaqueId}}`) with `{{query}}`.
14+
15+
https://github.com/cs3org/reva/pull/2436

pkg/user/manager/ldap/ldap.go

+4-16
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,8 @@ func init() {
4343
}
4444

4545
type manager struct {
46-
c *config
47-
userfilter *template.Template
48-
groupfilter *template.Template
46+
c *config
47+
userfilter *template.Template
4948
}
5049

5150
type config struct {
@@ -124,7 +123,6 @@ func (m *manager) Configure(ml map[string]interface{}) error {
124123
if c.FindFilter == "" {
125124
c.FindFilter = c.UserFilter
126125
}
127-
c.GroupFilter = strings.ReplaceAll(c.GroupFilter, "%s", "{{.OpaqueId}}")
128126

129127
if c.Nobody == 0 {
130128
c.Nobody = 99
@@ -136,11 +134,6 @@ func (m *manager) Configure(ml map[string]interface{}) error {
136134
err := errors.Wrap(err, fmt.Sprintf("error parsing userfilter tpl:%s", c.UserFilter))
137135
panic(err)
138136
}
139-
m.groupfilter, err = template.New("gf").Funcs(sprig.TxtFuncMap()).Parse(c.GroupFilter)
140-
if err != nil {
141-
err := errors.Wrap(err, fmt.Sprintf("error parsing groupfilter tpl:%s", c.GroupFilter))
142-
panic(err)
143-
}
144137
return nil
145138
}
146139

@@ -433,11 +426,6 @@ func (m *manager) getFindFilter(query string) string {
433426
return strings.ReplaceAll(m.c.FindFilter, "{{query}}", ldap.EscapeFilter(query))
434427
}
435428

436-
func (m *manager) getGroupFilter(uid interface{}) string {
437-
b := bytes.Buffer{}
438-
if err := m.groupfilter.Execute(&b, uid); err != nil {
439-
err := errors.Wrap(err, fmt.Sprintf("error executing group template: userid:%+v", uid))
440-
panic(err)
441-
}
442-
return b.String()
429+
func (m *manager) getGroupFilter(memberName string) string {
430+
return strings.ReplaceAll(m.c.GroupFilter, "{{query}}", ldap.EscapeFilter(memberName))
443431
}

tests/oc-integration-tests/drone/ldap-users.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ base_dn="dc=owncloud,dc=com"
3737
userfilter="(&(objectclass=posixAccount)(|(entryuuid={{.OpaqueId}})(cn={{.OpaqueId}})))"
3838
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
3939
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
40-
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.}}))"
40+
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
4141
bind_username="cn=admin,dc=owncloud,dc=com"
4242
bind_password="admin"
4343
idp="http://localhost:20080"

tests/oc-integration-tests/local-mesh/ldap-users.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ base_dn="dc=owncloud,dc=com"
3838
userfilter="(&(objectclass=posixAccount)(|(uid={{.OpaqueId}})(cn={{.OpaqueId}})))"
3939
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
4040
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
41-
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.OpaqueId}}))"
41+
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
4242
bind_username="cn=admin,dc=owncloud,dc=com"
4343
bind_password="admin"
4444
idp="http://localhost:40080"

tests/oc-integration-tests/local/ldap-users.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ base_dn="dc=owncloud,dc=com"
4141
userfilter="(&(objectclass=posixAccount)(|(entryuuid={{.OpaqueId}})(cn={{.OpaqueId}})))"
4242
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
4343
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
44-
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.}}))"
44+
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
4545
bind_username="cn=admin,dc=owncloud,dc=com"
4646
bind_password="admin"
4747
idp="http://localhost:20080"

0 commit comments

Comments
 (0)