diff --git a/changelog/unreleased/split-ldap-filters.md b/changelog/unreleased/split-ldap-filters.md index 11e101d0ea6..78303312e6b 100644 --- a/changelog/unreleased/split-ldap-filters.md +++ b/changelog/unreleased/split-ldap-filters.md @@ -1,11 +1,25 @@ 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 +The current LDAP user and auth filters only allow a single `%s` to be replaced with the relevant string. +The current `userfilter` the userfilter` is used to lookup a single user, search for share recipients and for login. To make each use case more flexible we split this in three and introduced templates. + +For the `userfilter` we moved to filter templates that can use the CS3 user id properties `{{.OpaqueId}}` and `{{.Idp}}`: ``` userfilter = "(&(objectclass=posixAccount)(|(ownclouduuid={{.OpaqueId}})(cn={{.OpaqueId}})))" +``` + +We introduced a new `findfilter` that is used when searching for users. Use it like this: +``` 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. + +Furthermore, we also introduced a dedicated login filter for the LDAP auth manager: +``` +loginfilter = "(&(objectclass=posixAccount)(|(cn={{login}})(mail={{login}})))" +``` + +These filter changes are backward compatible: `findfilter` and `loginfilter` will be derived from the `userfilter` by replacing `%s` with `{{query}}` and `{{login}}` respectively. The `userfilter` replaces `%s` with `{{.OpaqueId}}` + +Finally, we changed the default attribute for the immutable uid of a user to `ms-DS-ConsistencyGuid`. See https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts for the background. You can fall back to `objectguid` or even `samaccountname` but you will run into trouble when user names change. You have been warned. https://github.com/cs3org/reva/pull/996 diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go index 28cedd9db69..ce742106c3f 100644 --- a/pkg/auth/manager/ldap/ldap.go +++ b/pkg/auth/manager/ldap/ldap.go @@ -22,12 +22,14 @@ import ( "context" "crypto/tls" "fmt" + "strings" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/auth" "github.com/cs3org/reva/pkg/auth/manager/registry" "github.com/cs3org/reva/pkg/errtypes" + "github.com/cs3org/reva/pkg/logger" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "gopkg.in/ldap.v2" @@ -46,6 +48,7 @@ type config struct { Port int `mapstructure:"port"` BaseDN string `mapstructure:"base_dn"` UserFilter string `mapstructure:"userfilter"` + LoginFilter string `mapstructure:"loginfilter"` BindUsername string `mapstructure:"bind_username"` BindPassword string `mapstructure:"bind_password"` Idp string `mapstructure:"idp"` @@ -53,18 +56,25 @@ type config struct { } type attributes struct { - DN string `mapstructure:"dn"` - UID string `mapstructure:"uid"` - Mail string `mapstructure:"mail"` + // DN is the distinguished name in ldap, e.g. `cn=einstein,ou=users,dc=example,dc=org` + DN string `mapstructure:"dn"` + // UID is an immutable user id, see https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts + UID string `mapstructure:"uid"` + // CN is the username, typically `cn`, `uid` or `samaccountname` + CN string `mapstructure:"cn"` + // Mail is the email address of a user + Mail string `mapstructure:"mail"` + // Displayname is the Human readable name, e.g. `Albert Einstein` DisplayName string `mapstructure:"displayName"` } // Default attributes (Active Directory) var ldapDefaults = attributes{ + DN: "dn", + 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. + CN: "cn", Mail: "mail", - UID: "objectGUID", DisplayName: "displayName", - DN: "dn", } func parseConfig(m map[string]interface{}) (*config, error) { @@ -85,6 +95,15 @@ func New(m map[string]interface{}) (auth.Manager, error) { return nil, err } + // backwards compatibility + if c.UserFilter != "" { + logger.New().Warn().Msg("userfilter is deprecated, use a loginfilter like `(&(objectclass=posixAccount)(|(cn={{login}}))(mail={{login}}))`") + } + if c.LoginFilter == "" { + c.LoginFilter = c.UserFilter + c.LoginFilter = strings.ReplaceAll(c.LoginFilter, "%s", "{{login}}") + } + return &mgr{ c: c, }, nil @@ -109,9 +128,8 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) searchRequest := ldap.NewSearchRequest( am.c.BaseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, - fmt.Sprintf(am.c.UserFilter, clientID), - // TODO(jfd): objectguid, entryuuid etc ... make configurable - []string{am.c.Schema.DN, am.c.Schema.UID, am.c.Schema.Mail, am.c.Schema.DisplayName}, + am.getLoginFilter(clientID), + []string{am.c.Schema.DN, am.c.Schema.UID, am.c.Schema.CN, am.c.Schema.Mail, am.c.Schema.DisplayName}, nil, ) @@ -140,7 +158,7 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) OpaqueId: sr.Entries[0].GetAttributeValue(am.c.Schema.UID), }, // TODO add more claims from the StandardClaims, eg EmailVerified - Username: sr.Entries[0].GetAttributeValue(am.c.Schema.UID), + Username: sr.Entries[0].GetAttributeValue(am.c.Schema.CN), // TODO groups Groups: []string{}, Mail: sr.Entries[0].GetAttributeValue(am.c.Schema.Mail), @@ -150,3 +168,7 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) return u, nil } + +func (am *mgr) getLoginFilter(login string) string { + return strings.ReplaceAll(am.c.LoginFilter, "{{login}}", login) +} diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index 11740ccc00f..6bf161f220e 100644 --- a/pkg/user/manager/ldap/ldap.go +++ b/pkg/user/manager/ldap/ldap.go @@ -42,16 +42,9 @@ func init() { } type manager struct { - hostname string - port int - baseDN string - userfilter *template.Template - findfilter string - groupfilter *template.Template - bindUsername string - bindPassword string - idp string - schema attributes + c *config + userfilter *template.Template + groupfilter *template.Template } type config struct { @@ -68,21 +61,25 @@ type config struct { } type attributes struct { - Mail string `mapstructure:"mail"` - UID string `mapstructure:"uid"` + // DN is the distinguished name in ldap, e.g. `cn=einstein,ou=users,dc=example,dc=org` + DN string `mapstructure:"dn"` + // UID is an immutable user id, see https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts + UID string `mapstructure:"uid"` + // CN is the username, typically `cn`, `uid` or `samaccountname` + CN string `mapstructure:"cn"` + // Mail is the email address of a user + Mail string `mapstructure:"mail"` + // Displayname is the Human readable name, e.g. `Albert Einstein` DisplayName string `mapstructure:"displayName"` - DN string `mapstructure:"dn"` - CN string `mapstructure:"cn"` } // Default attributes (Active Directory) var ldapDefaults = attributes{ - 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", + 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. CN: "cn", + Mail: "mail", + DisplayName: "displayName", } func parseConfig(m map[string]interface{}) (*config, error) { @@ -112,14 +109,7 @@ func New(m map[string]interface{}) (user.Manager, error) { c.GroupFilter = strings.ReplaceAll(c.GroupFilter, "%s", "{{.OpaqueId}}") mgr := &manager{ - hostname: c.Hostname, - port: c.Port, - baseDN: c.BaseDN, - findfilter: c.FindFilter, - bindUsername: c.BindUsername, - bindPassword: c.BindPassword, - idp: c.Idp, - schema: c.Schema, + c: c, } mgr.userfilter, err = template.New("uf").Funcs(sprig.TxtFuncMap()).Parse(c.UserFilter) @@ -138,24 +128,24 @@ func New(m map[string]interface{}) (user.Manager, error) { func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) { log := appctx.GetLogger(ctx) - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.hostname, m.port), &tls.Config{InsecureSkipVerify: true}) + l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) if err != nil { return nil, err } defer l.Close() // First bind with a read only user - err = l.Bind(m.bindUsername, m.bindPassword) + err = l.Bind(m.c.BindUsername, m.c.BindPassword) if err != nil { return nil, err } // Search for the given clientID searchRequest := ldap.NewSearchRequest( - m.baseDN, + m.c.BaseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, m.getUserFilter(uid), - []string{m.schema.DN, m.schema.CN, m.schema.UID, m.schema.Mail, m.schema.DisplayName}, + []string{m.c.Schema.DN, m.c.Schema.UID, m.c.Schema.CN, m.c.Schema.Mail, m.c.Schema.DisplayName}, nil, ) @@ -171,8 +161,8 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User log.Debug().Interface("entries", sr.Entries).Msg("entries") id := &userpb.UserId{ - Idp: m.idp, - OpaqueId: sr.Entries[0].GetAttributeValue(m.schema.UID), + Idp: m.c.Idp, + OpaqueId: sr.Entries[0].GetAttributeValue(m.c.Schema.UID), } groups, err := m.GetUserGroups(ctx, id) if err != nil { @@ -180,34 +170,34 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User } u := &userpb.User{ Id: id, - Username: sr.Entries[0].GetAttributeValue(m.schema.CN), + Username: sr.Entries[0].GetAttributeValue(m.c.Schema.CN), Groups: groups, - Mail: sr.Entries[0].GetAttributeValue(m.schema.Mail), - DisplayName: sr.Entries[0].GetAttributeValue(m.schema.DisplayName), + Mail: sr.Entries[0].GetAttributeValue(m.c.Schema.Mail), + DisplayName: sr.Entries[0].GetAttributeValue(m.c.Schema.DisplayName), } return u, nil } func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.hostname, m.port), &tls.Config{InsecureSkipVerify: true}) + l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) if err != nil { return nil, err } defer l.Close() // First bind with a read only user - err = l.Bind(m.bindUsername, m.bindPassword) + err = l.Bind(m.c.BindUsername, m.c.BindPassword) if err != nil { return nil, err } // Search for the given clientID searchRequest := ldap.NewSearchRequest( - m.baseDN, + m.c.BaseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, m.getFindFilter(query), - []string{m.schema.DN, m.schema.CN, m.schema.UID, m.schema.Mail, m.schema.DisplayName}, + []string{m.c.Schema.DN, m.c.Schema.UID, m.c.Schema.CN, m.c.Schema.Mail, m.c.Schema.DisplayName}, nil, ) @@ -220,8 +210,8 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, for _, entry := range sr.Entries { id := &userpb.UserId{ - Idp: m.idp, - OpaqueId: entry.GetAttributeValue(m.schema.UID), + Idp: m.c.Idp, + OpaqueId: entry.GetAttributeValue(m.c.Schema.UID), } groups, err := m.GetUserGroups(ctx, id) if err != nil { @@ -229,10 +219,10 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, } user := &userpb.User{ Id: id, - Username: entry.GetAttributeValue(m.schema.CN), + Username: entry.GetAttributeValue(m.c.Schema.CN), Groups: groups, - Mail: entry.GetAttributeValue(m.schema.Mail), - DisplayName: entry.GetAttributeValue(m.schema.DisplayName), + Mail: entry.GetAttributeValue(m.c.Schema.Mail), + DisplayName: entry.GetAttributeValue(m.c.Schema.DisplayName), } users = append(users, user) } @@ -241,24 +231,24 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, } func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]string, error) { - l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.hostname, m.port), &tls.Config{InsecureSkipVerify: true}) + l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", m.c.Hostname, m.c.Port), &tls.Config{InsecureSkipVerify: true}) if err != nil { return []string{}, err } defer l.Close() // First bind with a read only user - err = l.Bind(m.bindUsername, m.bindPassword) + err = l.Bind(m.c.BindUsername, m.c.BindPassword) if err != nil { return []string{}, err } // Search for the given clientID searchRequest := ldap.NewSearchRequest( - m.baseDN, + m.c.BaseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, m.getGroupFilter(uid), - []string{m.schema.CN}, // TODO use DN to look up group id + []string{m.c.Schema.CN}, // TODO use DN to look up group id nil, ) @@ -273,7 +263,7 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri // 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 - groups = append(groups, entry.GetAttributeValue(m.schema.CN)) + groups = append(groups, entry.GetAttributeValue(m.c.Schema.CN)) } return groups, nil @@ -305,7 +295,7 @@ func (m *manager) getUserFilter(uid *userpb.UserId) string { } func (m *manager) getFindFilter(query string) string { - return strings.ReplaceAll(m.findfilter, "{{query}}", query) + return strings.ReplaceAll(m.c.FindFilter, "{{query}}", query) } func (m *manager) getGroupFilter(uid *userpb.UserId) string {