diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go
index d54fa9ace64efbf6f6de125d7d2c06cb4235c934..01c8f9226bdc8210994b77e7d98e05662501c450 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)
- }
+ 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},
- }
+ 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)
+ 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 43c86ddae5f41ebbd456f97cffed8f8ee7bafb8f..9e78c34618d76da0c3c92a8f83513fc6ae62bb3a 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.
//