From 97813ff4fc0049d87975032666236c7a641ba4aa Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Tue, 11 Apr 2017 09:48:48 -0700 Subject: [PATCH] connector/ldap: fix case where groups are listed on the user entity Support schemas that determine membership by having fields on the user entity, instead of listing users on a groups entity. E.g. the following schema is now supported when it wasn't previously: cn=eric,cn=user,dn=exapmle,dn=com objectClass=myPerson cn: eric uid: eric email: eric@example.com memberOf: foo memberOf: bar cn=foo,cn=group,dn=exapmle,dn=com objectClass=myGroup cn: foo cn=bar,cn=group,dn=exapmle,dn=com objectClass=myGroup cn: bar --- connector/ldap/ldap.go | 69 +++++++++++++----------- connector/ldap/ldap_test.go | 105 ++++++++++++++++++++++++++++++++++-- 2 files changed, 139 insertions(+), 35 deletions(-) diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index d54fa9ac..01c8f922 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -256,18 +256,22 @@ func (c *ldapConnector) do(ctx context.Context, f func(c *ldap.Conn) error) erro return f(conn) } -func getAttr(e ldap.Entry, name string) string { +func getAttrs(e ldap.Entry, name string) []string { for _, a := range e.Attributes { if a.Name != name { continue } - if len(a.Values) == 0 { - return "" - } - return a.Values[0] + return a.Values } if name == "DN" { - return e.DN + return []string{e.DN} + } + return nil +} + +func getAttr(e ldap.Entry, name string) string { + if a := getAttrs(e, name); len(a) > 0 { + return a[0] } return "" } @@ -454,36 +458,39 @@ func (c *ldapConnector) groups(ctx context.Context, user ldap.Entry) ([]string, return nil, nil } - filter := fmt.Sprintf("(%s=%s)", c.GroupSearch.GroupAttr, ldap.EscapeFilter(getAttr(user, c.GroupSearch.UserAttr))) - 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}, - } - var groups []*ldap.Entry - if err := c.do(ctx, func(conn *ldap.Conn) error { - resp, err := conn.Search(req) - if err != nil { - return fmt.Errorf("ldap: search failed: %v", err) + 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 { + 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) } - groups = resp.Entries - return nil - }); err != nil { - return nil, err - } - if len(groups) == 0 { - // TODO(ericchiang): Is this going to spam the logs? - c.logger.Errorf("ldap: groups search with filter %q returned no groups", filter) } var groupNames []string - for _, group := range groups { name := getAttr(*group, c.GroupSearch.NameAttr) if name == "" { diff --git a/connector/ldap/ldap_test.go b/connector/ldap/ldap_test.go index 43c86dda..9e78c346 100644 --- a/connector/ldap/ldap_test.go +++ b/connector/ldap/ldap_test.go @@ -52,7 +52,7 @@ ou: People dn: cn=jane,ou=People,dc=example,dc=org objectClass: person -objectClass: iNetOrgPerson +objectClass: inetOrgPerson sn: doe cn: jane mail: janedoe@example.com @@ -60,7 +60,7 @@ userpassword: foo dn: cn=john,ou=People,dc=example,dc=org objectClass: person -objectClass: iNetOrgPerson +objectClass: inetOrgPerson sn: doe cn: john mail: johndoe@example.com @@ -127,7 +127,7 @@ ou: People dn: cn=jane,ou=People,dc=example,dc=org objectClass: person -objectClass: iNetOrgPerson +objectClass: inetOrgPerson sn: doe cn: jane mail: janedoe@example.com @@ -135,7 +135,7 @@ userpassword: foo dn: cn=john,ou=People,dc=example,dc=org objectClass: person -objectClass: iNetOrgPerson +objectClass: inetOrgPerson sn: doe cn: john mail: johndoe@example.com @@ -201,6 +201,103 @@ member: cn=jane,ou=People,dc=example,dc=org runTests(t, schema, c, tests) } +func TestGroupsOnUserEntity(t *testing.T) { + schema := ` +dn: dc=example,dc=org +objectClass: dcObject +objectClass: organization +o: Example Company +dc: example + +dn: ou=People,dc=example,dc=org +objectClass: organizationalUnit +ou: People + +# Groups are enumerated as part of the user entity instead of the members being +# a list on the group entity. + +dn: cn=jane,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: jane +mail: janedoe@example.com +userpassword: foo +departmentNumber: 1000 +departmentNumber: 1001 + +dn: cn=john,ou=People,dc=example,dc=org +objectClass: person +objectClass: inetOrgPerson +sn: doe +cn: john +mail: johndoe@example.com +userpassword: bar +departmentNumber: 1000 +departmentNumber: 1002 + +# Group definitions. Notice that they don't have any "member" field. + +dn: ou=Groups,dc=example,dc=org +objectClass: organizationalUnit +ou: Groups + +dn: cn=admins,ou=Groups,dc=example,dc=org +objectClass: posixGroup +cn: admins +gidNumber: 1000 + +dn: cn=developers,ou=Groups,dc=example,dc=org +objectClass: posixGroup +cn: developers +gidNumber: 1001 + +dn: cn=designers,ou=Groups,dc=example,dc=org +objectClass: posixGroup +cn: designers +gidNumber: 1002 +` + 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 = "ou=Groups,dc=example,dc=org" + c.GroupSearch.UserAttr = "departmentNumber" + c.GroupSearch.GroupAttr = "gidNumber" + c.GroupSearch.NameAttr = "cn" + 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", "designers"}, + }, + }, + } + runTests(t, schema, c, tests) +} + // runTests runs a set of tests against an LDAP schema. It does this by // setting up an OpenLDAP server and injecting the provided scheme. //