From f2e7823db98f59e5aa0033be0624d51f5b10bfaf Mon Sep 17 00:00:00 2001 From: Vitaliy Dmitriev Date: Fri, 3 Jan 2020 10:40:08 +0100 Subject: [PATCH 1/2] connector/ldap: add multiple user to group mapping Add an ability to fetch user's membership from groups of a different type by specifying multiple group attribute to user attribute value matchers in the Dex config: userMatchers: - userAttr: uid groupAttr: memberUid - userAttr: DN groupAttr: member In other words the user's groups can be fetched now from ldap structure similar to the following: dn: cn=john,ou=People,dc=example,dc=org objectClass: person objectClass: inetOrgPerson sn: doe cn: john uid: johndoe mail: johndoe@example.com userpassword: bar dn: cn=qa,ou=Groups,ou=Portland,dc=example,dc=org objectClass: groupOfNames cn: qa member: cn=john,ou=People,dc=example,dc=org dn: cn=logger,ou=UnixGroups,ou=Portland,dc=example,dc=org objectClass: posixGroup gidNumber: 1000 cn: logger memberUid: johndoe Signed-off-by: Vitaliy Dmitriev --- Documentation/connectors/ldap.md | 45 ++++++--- connector/ldap/ldap.go | 99 +++++++++++-------- connector/ldap/ldap_test.go | 154 ++++++++++++++++++++++++++++-- examples/config-ad-kubelogin.yaml | 5 +- examples/config-ldap.yaml | 9 +- 5 files changed, 247 insertions(+), 65 deletions(-) diff --git a/Documentation/connectors/ldap.md b/Documentation/connectors/ldap.md index 4bd540b5..e69c3005 100644 --- a/Documentation/connectors/ldap.md +++ b/Documentation/connectors/ldap.md @@ -123,11 +123,12 @@ connectors: # Optional filter to apply when searching the directory. filter: "(objectClass=group)" - # Following two fields are used to match a user to a group. It adds an additional + # Following list contains field pairs that are used to match a user to a group. It adds an additional # requirement to the filter that an attribute in the group must match the user's # attribute value. - userAttr: uid - groupAttr: member + userMatchers: + - userAttr: uid + groupAttr: member # Represents group name. nameAttr: name @@ -215,8 +216,9 @@ groupSearch: # The group search needs to match the "uid" attribute on # the user with the "memberUid" attribute on the group. - userAttr: uid - groupAttr: memberUid + userMatchers: + - userAttr: uid + groupAttr: memberUid # Unique name of the group. nameAttr: cn @@ -242,8 +244,27 @@ groupSearch: # Optional filter to apply when searching the directory. filter: "(objectClass=group)" - userAttr: DN # Use "DN" here not "uid" - groupAttr: member + userMatchers: + - userAttr: DN # Use "DN" here not "uid" + groupAttr: member + + nameAttr: name +``` + +There are cases when different types (objectClass) of groups use different attributes to keep a list of members. Below is an example of group query for such case: + +```yaml +groupSearch: + baseDN: cn=groups,cn=compat,dc=example,dc=com + # Optional filter to search for different group types + filter: "(|(objectClass=posixGroup)(objectClass=group))" + + # Use multiple user matchers so Dex will know which attribute names should be used to search for group members + userMatchers: + - userAttr: uid + groupAttr: memberUid + - userAttr: DN + groupAttr: member nameAttr: name ``` @@ -275,8 +296,9 @@ connectors: # Would translate to the query "(&(objectClass=group)(member=))". baseDN: cn=groups,dc=freeipa,dc=example,dc=com filter: "(objectClass=group)" - userAttr: uid - groupAttr: member + userMatchers: + - userAttr: uid + groupAttr: member nameAttr: name ``` @@ -315,8 +337,9 @@ connectors: groupSearch: baseDN: cn=Users,dc=example,dc=com filter: "(objectClass=group)" - userAttr: DN - groupAttr: member + userMatchers: + - userAttr: DN + groupAttr: member nameAttr: cn ``` diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index cac3539f..963aaa3d 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -41,16 +41,26 @@ import ( // nameAttr: name // preferredUsernameAttr: uid // groupSearch: -// # Would translate to the query "(&(objectClass=group)(member=))" +// # Would translate to the separate query per user matcher pair and aggregate results into a single group list: +// # "(&(|(objectClass=posixGroup)(objectClass=groupOfNames))(memberUid=))" +// # "(&(|(objectClass=posixGroup)(objectClass=groupOfNames))(member=))" // baseDN: cn=groups,dc=example,dc=com -// filter: "(objectClass=group)" -// userAttr: uid -// # Use if full DN is needed and not available as any other attribute -// # Will only work if "DN" attribute does not exist in the record -// # userAttr: DN -// groupAttr: member +// filter: "(|(objectClass=posixGroup)(objectClass=groupOfNames))" +// userMatchers: +// - userAttr: uid +// groupAttr: memberUid +// # Use if full DN is needed and not available as any other attribute +// # Will only work if "DN" attribute does not exist in the record: +// - userAttr: DN +// groupAttr: member // nameAttr: name // + +type UserMatcher struct { + UserAttr string `json:"userAttr"` + GroupAttr string `json:"groupAttr"` +} + type Config struct { // The host and optional port of the LDAP server. If port isn't supplied, it will be // guessed based on the TLS configuration. 389 or 636. @@ -124,16 +134,16 @@ type Config struct { Scope string `json:"scope"` // Defaults to "sub" - // These two fields are use to match a user to a group. + // Array of the field pairs used to match a user to a group. + // See the "UserMatcher" struct for the exact field names // - // It adds an additional requirement to the filter that an attribute in the group + // Each pair adds an additional requirement to the filter that an attribute in the group // match the user's attribute value. For example that the "members" attribute of // a group matches the "uid" of the user. The exact filter being added is: // - // (=) + // (userMatchers[n].=userMatchers[n].) // - UserAttr string `json:"userAttr"` - GroupAttr string `json:"groupAttr"` + UserMatchers []UserMatcher `json:"userMatchers"` // The attribute of the group that represents its name. NameAttr string `json:"nameAttr"` @@ -378,11 +388,14 @@ func (c *ldapConnector) userEntry(conn *ldap.Conn, username string) (user ldap.E Attributes: []string{ c.UserSearch.IDAttr, c.UserSearch.EmailAttr, - c.GroupSearch.UserAttr, // TODO(ericchiang): what if this contains duplicate values? }, } + for _, matcher := range c.GroupSearch.UserMatchers { + req.Attributes = append(req.Attributes, matcher.UserAttr) + } + if c.UserSearch.NameAttr != "" { req.Attributes = append(req.Attributes, c.UserSearch.NameAttr) } @@ -536,36 +549,38 @@ func (c *ldapConnector) groups(ctx context.Context, user ldap.Entry) ([]string, } var groups []*ldap.Entry - for _, attr := range getAttrs(user, c.GroupSearch.UserAttr) { - filter := fmt.Sprintf("(%s=%s)", c.GroupSearch.GroupAttr, ldap.EscapeFilter(attr)) - if c.GroupSearch.Filter != "" { - filter = fmt.Sprintf("(&%s%s)", c.GroupSearch.Filter, filter) - } - - req := &ldap.SearchRequest{ - BaseDN: c.GroupSearch.BaseDN, - Filter: filter, - Scope: c.groupSearchScope, - Attributes: []string{c.GroupSearch.NameAttr}, - } - - gotGroups := false - if err := c.do(ctx, func(conn *ldap.Conn) error { - c.logger.Infof("performing ldap search %s %s %s", - req.BaseDN, scopeString(req.Scope), req.Filter) - resp, err := conn.Search(req) - if err != nil { - return fmt.Errorf("ldap: search failed: %v", err) + for _, matcher := range c.GroupSearch.UserMatchers { + for _, attr := range getAttrs(user, matcher.UserAttr) { + filter := fmt.Sprintf("(%s=%s)", matcher.GroupAttr, ldap.EscapeFilter(attr)) + if c.GroupSearch.Filter != "" { + filter = fmt.Sprintf("(&%s%s)", c.GroupSearch.Filter, filter) + } + + req := &ldap.SearchRequest{ + BaseDN: c.GroupSearch.BaseDN, + Filter: filter, + Scope: c.groupSearchScope, + Attributes: []string{c.GroupSearch.NameAttr}, + } + + gotGroups := false + if err := c.do(ctx, func(conn *ldap.Conn) error { + c.logger.Infof("performing ldap search %s %s %s", + req.BaseDN, scopeString(req.Scope), req.Filter) + resp, err := conn.Search(req) + if err != nil { + return fmt.Errorf("ldap: search failed: %v", err) + } + gotGroups = len(resp.Entries) != 0 + groups = append(groups, resp.Entries...) + return nil + }); err != nil { + return nil, err + } + if !gotGroups { + // TODO(ericchiang): Is this going to spam the logs? + c.logger.Errorf("ldap: groups search with filter %q returned no groups", filter) } - gotGroups = len(resp.Entries) != 0 - groups = append(groups, resp.Entries...) - return nil - }); err != nil { - return nil, err - } - if !gotGroups { - // TODO(ericchiang): Is this going to spam the logs? - c.logger.Errorf("ldap: groups search with filter %q returned no groups", filter) } } diff --git a/connector/ldap/ldap_test.go b/connector/ldap/ldap_test.go index 1ed2f878..4cb6611a 100644 --- a/connector/ldap/ldap_test.go +++ b/connector/ldap/ldap_test.go @@ -307,8 +307,12 @@ member: cn=jane,ou=People,dc=example,dc=org c.UserSearch.IDAttr = "DN" c.UserSearch.Username = "cn" c.GroupSearch.BaseDN = "ou=Groups,dc=example,dc=org" - c.GroupSearch.UserAttr = "DN" - c.GroupSearch.GroupAttr = "member" + c.GroupSearch.UserMatchers = []UserMatcher{ + { + UserAttr: "DN", + GroupAttr: "member", + }, + } c.GroupSearch.NameAttr = "cn" tests := []subtest{ @@ -400,8 +404,12 @@ gidNumber: 1002 c.UserSearch.IDAttr = "DN" c.UserSearch.Username = "cn" c.GroupSearch.BaseDN = "ou=Groups,dc=example,dc=org" - c.GroupSearch.UserAttr = "departmentNumber" - c.GroupSearch.GroupAttr = "gidNumber" + c.GroupSearch.UserMatchers = []UserMatcher{ + { + UserAttr: "departmentNumber", + GroupAttr: "gidNumber", + }, + } c.GroupSearch.NameAttr = "cn" tests := []subtest{ { @@ -497,8 +505,12 @@ member: cn=jane,ou=People,dc=example,dc=org c.UserSearch.IDAttr = "DN" c.UserSearch.Username = "cn" c.GroupSearch.BaseDN = "dc=example,dc=org" - c.GroupSearch.UserAttr = "DN" - c.GroupSearch.GroupAttr = "member" + c.GroupSearch.UserMatchers = []UserMatcher{ + { + UserAttr: "DN", + GroupAttr: "member", + }, + } c.GroupSearch.NameAttr = "cn" c.GroupSearch.Filter = "(ou:dn:=Seattle)" // ignore other groups @@ -534,6 +546,136 @@ member: cn=jane,ou=People,dc=example,dc=org runTests(t, schema, connectLDAP, c, tests) } +func TestGroupToUserMatchers(t *testing.T) { + schema := ` +dn: ou=People,dc=example,dc=org +objectClass: organizationalUnit +ou: People + +dn: cn=jane,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: jane +uid: janedoe +mail: janedoe@example.com +userpassword: foo + +dn: cn=john,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: john +uid: johndoe +mail: johndoe@example.com +userpassword: bar + +# Group definitions. + +dn: ou=Seattle,dc=example,dc=org +objectClass: organizationalUnit +ou: Seattle + +dn: ou=Portland,dc=example,dc=org +objectClass: organizationalUnit +ou: Portland + +dn: ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: organizationalUnit +ou: Groups + +dn: ou=UnixGroups,ou=Seattle,dc=example,dc=org +objectClass: organizationalUnit +ou: UnixGroups + +dn: ou=Groups,ou=Portland,dc=example,dc=org +objectClass: organizationalUnit +ou: Groups + +dn: ou=UnixGroups,ou=Portland,dc=example,dc=org +objectClass: organizationalUnit +ou: UnixGroups + +dn: cn=qa,ou=Groups,ou=Portland,dc=example,dc=org +objectClass: groupOfNames +cn: qa +member: cn=john,ou=People,dc=example,dc=org + +dn: cn=logger,ou=UnixGroups,ou=Portland,dc=example,dc=org +objectClass: posixGroup +gidNumber: 1000 +cn: logger +memberUid: johndoe + +dn: cn=admins,ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: groupOfNames +cn: admins +member: cn=john,ou=People,dc=example,dc=org +member: cn=jane,ou=People,dc=example,dc=org + +dn: cn=developers,ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: groupOfNames +cn: developers +member: cn=jane,ou=People,dc=example,dc=org + +dn: cn=frontend,ou=UnixGroups,ou=Seattle,dc=example,dc=org +objectClass: posixGroup +gidNumber: 1001 +cn: frontend +memberUid: janedoe +` + c := &Config{} + c.UserSearch.BaseDN = "ou=People,dc=example,dc=org" + c.UserSearch.NameAttr = "cn" + c.UserSearch.EmailAttr = "mail" + c.UserSearch.IDAttr = "DN" + c.UserSearch.Username = "cn" + c.GroupSearch.BaseDN = "dc=example,dc=org" + c.GroupSearch.UserMatchers = []UserMatcher{ + { + UserAttr: "DN", + GroupAttr: "member", + }, + { + UserAttr: "uid", + GroupAttr: "memberUid", + }, + } + c.GroupSearch.NameAttr = "cn" + c.GroupSearch.Filter = "(|(objectClass=posixGroup)(objectClass=groupOfNames))" // search all group types + + tests := []subtest{ + { + name: "validpassword", + username: "jane", + password: "foo", + groups: true, + want: connector.Identity{ + UserID: "cn=jane,ou=People,dc=example,dc=org", + Username: "jane", + Email: "janedoe@example.com", + EmailVerified: true, + Groups: []string{"admins", "developers", "frontend"}, + }, + }, + { + name: "validpassword2", + username: "john", + password: "bar", + groups: true, + want: connector.Identity{ + UserID: "cn=john,ou=People,dc=example,dc=org", + Username: "john", + Email: "johndoe@example.com", + EmailVerified: true, + Groups: []string{"qa", "admins", "logger"}, + }, + }, + } + + runTests(t, schema, connectLDAP, c, tests) +} + func TestStartTLS(t *testing.T) { schema := ` dn: ou=People,dc=example,dc=org diff --git a/examples/config-ad-kubelogin.yaml b/examples/config-ad-kubelogin.yaml index 20bb9bd8..559ccae9 100644 --- a/examples/config-ad-kubelogin.yaml +++ b/examples/config-ad-kubelogin.yaml @@ -41,10 +41,11 @@ connectors: baseDN: cn=Users,dc=example,dc=com filter: "(objectClass=group)" + userMatchers: # A user is a member of a group when their DN matches # the value of a "member" attribute on the group entity. - userAttr: DN - groupAttr: member + - userAttr: DN + groupAttr: member # The group name should be the "cn" value. nameAttr: cn diff --git a/examples/config-ldap.yaml b/examples/config-ldap.yaml index 21fb052c..05265b4b 100644 --- a/examples/config-ldap.yaml +++ b/examples/config-ldap.yaml @@ -37,10 +37,11 @@ connectors: baseDN: ou=Groups,dc=example,dc=org filter: "(objectClass=groupOfNames)" - # A user is a member of a group when their DN matches - # the value of a "member" attribute on the group entity. - userAttr: DN - groupAttr: member + userMatchers: + # A user is a member of a group when their DN matches + # the value of a "member" attribute on the group entity. + - userAttr: DN + groupAttr: member # The group name should be the "cn" value. nameAttr: cn From e20a795a2a4cdccfc9da9051f36e765ef8ee5d0e Mon Sep 17 00:00:00 2001 From: Vitaliy Dmitriev Date: Tue, 7 Jan 2020 14:55:13 +0100 Subject: [PATCH 2/2] connector/ldap: backward compatibility with single user to group mapping Signed-off-by: Vitaliy Dmitriev --- connector/ldap/ldap.go | 27 +++++++++- connector/ldap/ldap_test.go | 103 ++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index 963aaa3d..53f9062f 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -134,6 +134,12 @@ type Config struct { Scope string `json:"scope"` // Defaults to "sub" + // DEPRECATED config options. Those are left for backward compatibility. + // See "UserMatchers" below for the current group to user matching implementation + // TODO: should be eventually removed from the code + UserAttr string `json:"userAttr"` + GroupAttr string `json:"groupAttr"` + // Array of the field pairs used to match a user to a group. // See the "UserMatcher" struct for the exact field names // @@ -175,6 +181,23 @@ func parseScope(s string) (int, bool) { return 0, false } +// Build a list of group attr name to user attr value matchers. +// Function exists here to allow backward compatibility between old and new +// group to user matching implementations. +// See "Config.GroupSearch.UserMatchers" comments for the details +func (c *ldapConnector) userMatchers() []UserMatcher { + if len(c.GroupSearch.UserMatchers) > 0 && c.GroupSearch.UserMatchers[0].UserAttr != "" { + return c.GroupSearch.UserMatchers[:] + } else { + return []UserMatcher{ + { + UserAttr: c.GroupSearch.UserAttr, + GroupAttr: c.GroupSearch.GroupAttr, + }, + } + } +} + // Open returns an authentication strategy using LDAP. func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { conn, err := c.OpenConnector(logger) @@ -392,7 +415,7 @@ func (c *ldapConnector) userEntry(conn *ldap.Conn, username string) (user ldap.E }, } - for _, matcher := range c.GroupSearch.UserMatchers { + for _, matcher := range c.userMatchers() { req.Attributes = append(req.Attributes, matcher.UserAttr) } @@ -549,7 +572,7 @@ func (c *ldapConnector) groups(ctx context.Context, user ldap.Entry) ([]string, } var groups []*ldap.Entry - for _, matcher := range c.GroupSearch.UserMatchers { + for _, matcher := range c.userMatchers() { for _, attr := range getAttrs(user, matcher.UserAttr) { filter := fmt.Sprintf("(%s=%s)", matcher.GroupAttr, ldap.EscapeFilter(attr)) if c.GroupSearch.Filter != "" { diff --git a/connector/ldap/ldap_test.go b/connector/ldap/ldap_test.go index 4cb6611a..1dc26afd 100644 --- a/connector/ldap/ldap_test.go +++ b/connector/ldap/ldap_test.go @@ -676,6 +676,109 @@ memberUid: janedoe runTests(t, schema, connectLDAP, c, tests) } +// Test deprecated group to user matching implementation +// which was left for backward compatibility. +// See "Config.GroupSearch.UserMatchers" comments for the details +func TestDeprecatedGroupToUserMatcher(t *testing.T) { + schema := ` +dn: ou=People,dc=example,dc=org +objectClass: organizationalUnit +ou: People + +dn: cn=jane,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: jane +mail: janedoe@example.com +userpassword: foo + +dn: cn=john,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: john +mail: johndoe@example.com +userpassword: bar + +# Group definitions. + +dn: ou=Seattle,dc=example,dc=org +objectClass: organizationalUnit +ou: Seattle + +dn: ou=Portland,dc=example,dc=org +objectClass: organizationalUnit +ou: Portland + +dn: ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: organizationalUnit +ou: Groups + +dn: ou=Groups,ou=Portland,dc=example,dc=org +objectClass: organizationalUnit +ou: Groups + +dn: cn=qa,ou=Groups,ou=Portland,dc=example,dc=org +objectClass: groupOfNames +cn: qa +member: cn=john,ou=People,dc=example,dc=org + +dn: cn=admins,ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: groupOfNames +cn: admins +member: cn=john,ou=People,dc=example,dc=org +member: cn=jane,ou=People,dc=example,dc=org + +dn: cn=developers,ou=Groups,ou=Seattle,dc=example,dc=org +objectClass: groupOfNames +cn: developers +member: cn=jane,ou=People,dc=example,dc=org +` + c := &Config{} + c.UserSearch.BaseDN = "ou=People,dc=example,dc=org" + c.UserSearch.NameAttr = "cn" + c.UserSearch.EmailAttr = "mail" + c.UserSearch.IDAttr = "DN" + c.UserSearch.Username = "cn" + c.GroupSearch.BaseDN = "dc=example,dc=org" + c.GroupSearch.UserAttr = "DN" + c.GroupSearch.GroupAttr = "member" + c.GroupSearch.NameAttr = "cn" + c.GroupSearch.Filter = "(ou:dn:=Seattle)" // ignore other groups + + tests := []subtest{ + { + name: "validpassword", + username: "jane", + password: "foo", + groups: true, + want: connector.Identity{ + UserID: "cn=jane,ou=People,dc=example,dc=org", + Username: "jane", + Email: "janedoe@example.com", + EmailVerified: true, + Groups: []string{"admins", "developers"}, + }, + }, + { + name: "validpassword2", + username: "john", + password: "bar", + groups: true, + want: connector.Identity{ + UserID: "cn=john,ou=People,dc=example,dc=org", + Username: "john", + Email: "johndoe@example.com", + EmailVerified: true, + Groups: []string{"admins"}, + }, + }, + } + + runTests(t, schema, connectLDAP, c, tests) +} + func TestStartTLS(t *testing.T) { schema := ` dn: ou=People,dc=example,dc=org