Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split LDAP user filters #996

Merged
merged 2 commits into from
Jul 24, 2020
Merged

Split LDAP user filters #996

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed64c21
Split LDAP user filters
butonic Jul 23, 2020
66a7e9d
use config as member of manager, also update auth manager
butonic Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/unreleased/split-ldap-filters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Enhancement: Split LDAP user filters

The current LDAP user filters only allow a single `%s` to be replaced with the relevant string. We moved to filter templates that can use the CS3 user id properties `{{.OpaqueId}}` and `{{.Idp}}`. Furthermore,
we introduced a new find filter that is used when searching for users. An example config would be
```
userfilter = "(&(objectclass=posixAccount)(|(ownclouduuid={{.OpaqueId}})(cn={{.OpaqueId}})))"
findfilter = "(&(objectclass=posixAccount)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))"
```
As you can see the userfilter is used to lookup a single user, whereas the findfilter is for a broader search, e.g. used when searching for share recipients.

https://github.com/cs3org/reva/pull/996
84 changes: 68 additions & 16 deletions pkg/user/manager/ldap/ldap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
package ldap

import (
"bytes"
"context"
"crypto/tls"
"fmt"
"strings"
"text/template"

"github.com/Masterminds/sprig"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
Expand All @@ -41,8 +45,9 @@ type manager struct {
hostname string
port int
baseDN string
userfilter string
groupfilter string
userfilter *template.Template
findfilter string
groupfilter *template.Template
bindUsername string
bindPassword string
idp string
Expand All @@ -54,6 +59,7 @@ type config struct {
Port int `mapstructure:"port"`
BaseDN string `mapstructure:"base_dn"`
UserFilter string `mapstructure:"userfilter"`
FindFilter string `mapstructure:"findfilter"`
GroupFilter string `mapstructure:"groupfilter"`
BindUsername string `mapstructure:"bind_username"`
BindPassword string `mapstructure:"bind_password"`
Expand All @@ -71,8 +77,9 @@ type attributes struct {

// Default attributes (Active Directory)
var ldapDefaults = attributes{
Mail: "mail",
UID: "objectGUID",
Mail: "mail",
// An immutable user id, see https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts
UID: "ms-DS-ConsistencyGuid", // you can fall back to objectguid or even samaccountname but you will run into trouble when user names change. You have been warned.
DisplayName: "displayName",
DN: "dn",
CN: "cn",
Expand All @@ -97,17 +104,36 @@ func New(m map[string]interface{}) (user.Manager, error) {
return nil, err
}

return &manager{
// backwards compatibility
c.UserFilter = strings.ReplaceAll(c.UserFilter, "%s", "{{.OpaqueId}}")
if c.FindFilter == "" {
c.FindFilter = c.UserFilter
}
c.GroupFilter = strings.ReplaceAll(c.GroupFilter, "%s", "{{.OpaqueId}}")
ishank011 marked this conversation as resolved.
Show resolved Hide resolved

mgr := &manager{
hostname: c.Hostname,
port: c.Port,
baseDN: c.BaseDN,
userfilter: c.UserFilter,
groupfilter: c.GroupFilter,
findfilter: c.FindFilter,
bindUsername: c.BindUsername,
bindPassword: c.BindPassword,
idp: c.Idp,
schema: c.Schema,
}, nil
}

mgr.userfilter, err = template.New("uf").Funcs(sprig.TxtFuncMap()).Parse(c.UserFilter)
if err != nil {
err := errors.Wrap(err, fmt.Sprintf("error parsing userfilter tpl:%s", c.UserFilter))
panic(err)
}
mgr.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 mgr, nil
}

func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) {
Expand All @@ -128,8 +154,8 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User
searchRequest := ldap.NewSearchRequest(
m.baseDN,
ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
fmt.Sprintf(m.userfilter, uid.OpaqueId), // TODO this is screaming for errors if filter contains >1 %s
[]string{m.schema.DN, m.schema.UID, m.schema.Mail, m.schema.DisplayName},
m.getUserFilter(uid),
[]string{m.schema.DN, m.schema.CN, m.schema.UID, m.schema.Mail, m.schema.DisplayName},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we add the cn attribute so we can return a user with an id that is different from the username

nil,
)

Expand All @@ -154,7 +180,7 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User
}
u := &userpb.User{
Id: id,
Username: sr.Entries[0].GetAttributeValue(m.schema.UID),
Username: sr.Entries[0].GetAttributeValue(m.schema.CN),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we use the previously fetched cn as the username. they can be configured to the same attribute and the code will work as before

Groups: groups,
Mail: sr.Entries[0].GetAttributeValue(m.schema.Mail),
DisplayName: sr.Entries[0].GetAttributeValue(m.schema.DisplayName),
Expand All @@ -180,8 +206,8 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User,
searchRequest := ldap.NewSearchRequest(
m.baseDN,
ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
fmt.Sprintf(m.userfilter, query), // TODO this is screaming for errors if filter contains >1 %s
[]string{m.schema.DN, m.schema.UID, m.schema.Mail, m.schema.DisplayName},
m.getFindFilter(query),
[]string{m.schema.DN, m.schema.CN, m.schema.UID, m.schema.Mail, m.schema.DisplayName},
nil,
)

Expand All @@ -203,7 +229,7 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User,
}
user := &userpb.User{
Id: id,
Username: entry.GetAttributeValue(m.schema.UID),
Username: entry.GetAttributeValue(m.schema.CN),
Groups: groups,
Mail: entry.GetAttributeValue(m.schema.Mail),
DisplayName: entry.GetAttributeValue(m.schema.DisplayName),
Expand Down Expand Up @@ -231,8 +257,8 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri
searchRequest := ldap.NewSearchRequest(
m.baseDN,
ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false,
fmt.Sprintf(m.groupfilter, uid.OpaqueId), // TODO this is screaming for errors if filter contains >1 %s
[]string{m.schema.CN},
m.getGroupFilter(uid),
[]string{m.schema.CN}, // TODO use DN to look up group id
nil,
)

Expand All @@ -244,13 +270,17 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri
groups := []string{}

for _, entry := range sr.Entries {
// FIXME this makes the users groups use the cn, not an immutable id
// FIXME 1. use the memberof or members attribute of a user to get the groups
// FIXME 2. ook up the id for each group
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be cleaned up to actually fetch groups. AFAIU it will require the memberof overlay. In any case the member attribute needs to be made configurable as well. As part of a subsequent PR.

groups = append(groups, entry.GetAttributeValue(m.schema.CN))
}

return groups, nil
}

func (m *manager) IsInGroup(ctx context.Context, uid *userpb.UserId, group string) (bool, error) {
// TODO implement with dedicated ldap query
groups, err := m.GetUserGroups(ctx, uid)
if err != nil {
return false, err
Expand All @@ -264,3 +294,25 @@ func (m *manager) IsInGroup(ctx context.Context, uid *userpb.UserId, group strin

return false, nil
}

func (m *manager) getUserFilter(uid *userpb.UserId) string {
b := bytes.Buffer{}
if err := m.userfilter.Execute(&b, uid); err != nil {
err := errors.Wrap(err, fmt.Sprintf("error executing user template: userid:%+v", uid))
panic(err)
}
return b.String()
}

func (m *manager) getFindFilter(query string) string {
return strings.ReplaceAll(m.findfilter, "{{query}}", query)
}

func (m *manager) getGroupFilter(uid *userpb.UserId) 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()
}