From ad4ded292153fafb5e6735aa7e76481ddedf3f33 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 12 Jan 2022 20:16:39 +0100 Subject: [PATCH] Adjust "groupfilter" to be able to search by member name (#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. --- .../ldap-usergroupfilter-template.md | 15 ++++++++++++++ pkg/user/manager/ldap/ldap.go | 20 ++++--------------- .../drone/ldap-users.toml | 2 +- .../local-mesh/ldap-users.toml | 2 +- .../local/ldap-users.toml | 2 +- 5 files changed, 22 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/ldap-usergroupfilter-template.md diff --git a/changelog/unreleased/ldap-usergroupfilter-template.md b/changelog/unreleased/ldap-usergroupfilter-template.md new file mode 100644 index 0000000000..52b2e6b2af --- /dev/null +++ b/changelog/unreleased/ldap-usergroupfilter-template.md @@ -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 diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index 420d67bec0..282b35e638 100644 --- a/pkg/user/manager/ldap/ldap.go +++ b/pkg/user/manager/ldap/ldap.go @@ -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 { @@ -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 @@ -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 } @@ -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)) } diff --git a/tests/oc-integration-tests/drone/ldap-users.toml b/tests/oc-integration-tests/drone/ldap-users.toml index 45b693961b..8040f6f5ee 100644 --- a/tests/oc-integration-tests/drone/ldap-users.toml +++ b/tests/oc-integration-tests/drone/ldap-users.toml @@ -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" diff --git a/tests/oc-integration-tests/local-mesh/ldap-users.toml b/tests/oc-integration-tests/local-mesh/ldap-users.toml index ad3fb33106..a3f77b313e 100644 --- a/tests/oc-integration-tests/local-mesh/ldap-users.toml +++ b/tests/oc-integration-tests/local-mesh/ldap-users.toml @@ -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" diff --git a/tests/oc-integration-tests/local/ldap-users.toml b/tests/oc-integration-tests/local/ldap-users.toml index 0489855b88..65e2d00f6a 100644 --- a/tests/oc-integration-tests/local/ldap-users.toml +++ b/tests/oc-integration-tests/local/ldap-users.toml @@ -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"