Skip to content

Commit

Permalink
Adjust "groupfilter" to be able to search by member name
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 committed Jan 12, 2022
1 parent 07451f6 commit 7137306
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 7137306

Please sign in to comment.