Skip to content

Commit

Permalink
Adjust "groupfilter" to be able to search by member name (#2436)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rhafer authored and butonic committed Feb 14, 2022
1 parent 626d28a commit 8a956f3
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 19 deletions.
15 changes: 15 additions & 0 deletions changelog/unreleased/ldap-usergroupfilter-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Change: Replace template in GroupFilter for UserProvider with a simple string

Previously the "groupfilter" configuration for the UserProvider expected a
go-template value (based of of an `userpb.UserId` as it's input). And 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).

This change removes the userpb.UserId template processing from the groupfilter
and replaces it with a single string (the username) to cleanup the config a
bit. Existing configs need to be update to replace the go template references
in `groupfilter` (e.g. `{{.}}` or `{{.OpaqueId}}`) with `{{query}}`.

https://github.com/cs3org/reva/pull/2436
20 changes: 4 additions & 16 deletions pkg/user/manager/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ func init() {
}

type manager struct {
c *config
userfilter *template.Template
groupfilter *template.Template
c *config
userfilter *template.Template
}

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

if c.Nobody == 0 {
c.Nobody = 99
Expand All @@ -136,11 +134,6 @@ func (m *manager) Configure(ml map[string]interface{}) error {
err := errors.Wrap(err, fmt.Sprintf("error parsing userfilter tpl:%s", c.UserFilter))
panic(err)
}
m.groupfilter, err = template.New("gf").Funcs(sprig.TxtFuncMap()).Parse(c.GroupFilter)
if err != nil {
err := errors.Wrap(err, fmt.Sprintf("error parsing groupfilter tpl:%s", c.GroupFilter))
panic(err)
}
return nil
}

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

func (m *manager) getGroupFilter(uid interface{}) string {
b := bytes.Buffer{}
if err := m.groupfilter.Execute(&b, uid); err != nil {
err := errors.Wrap(err, fmt.Sprintf("error executing group template: userid:%+v", uid))
panic(err)
}
return b.String()
func (m *manager) getGroupFilter(memberName string) string {
return strings.ReplaceAll(m.c.GroupFilter, "{{query}}", ldap.EscapeFilter(memberName))
}
2 changes: 1 addition & 1 deletion tests/oc-integration-tests/drone/ldap-users.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ base_dn="dc=owncloud,dc=com"
userfilter="(&(objectclass=posixAccount)(|(entryuuid={{.OpaqueId}})(cn={{.OpaqueId}})))"
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
bind_username="cn=admin,dc=owncloud,dc=com"
bind_password="admin"
idp="http://localhost:20080"
Expand Down
2 changes: 1 addition & 1 deletion tests/oc-integration-tests/local-mesh/ldap-users.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ base_dn="dc=owncloud,dc=com"
userfilter="(&(objectclass=posixAccount)(|(uid={{.OpaqueId}})(cn={{.OpaqueId}})))"
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.OpaqueId}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
bind_username="cn=admin,dc=owncloud,dc=com"
bind_password="admin"
idp="http://localhost:40080"
Expand Down
2 changes: 1 addition & 1 deletion tests/oc-integration-tests/local/ldap-users.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ base_dn="dc=owncloud,dc=com"
userfilter="(&(objectclass=posixAccount)(|(entryuuid={{.OpaqueId}})(cn={{.OpaqueId}})))"
findfilter="(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
attributefilter="(&(objectclass=posixAccount)({{attr}}={{value}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{.}}))"
groupfilter="(&(objectclass=posixGroup)(cn=*)(memberuid={{query}}))"
bind_username="cn=admin,dc=owncloud,dc=com"
bind_password="admin"
idp="http://localhost:20080"
Expand Down

0 comments on commit 8a956f3

Please sign in to comment.