Skip to content

Commit 7208747

Browse files
EthanDieterichEthanDieterich
authored
Add LDAP parent groups search, Active Directory Hierarchy (#4113)
This commit enables universal nested group search support across a variety of LDAP server implementations. It updates the code to allow recursive group membership discovery during user authentication and provides CI tests to validate the functionality. Based on @paroque’s original #1058 PR. - Removed `Recursive` boolean flag from config and logic - Made recursion behavior dependant on presence of `RecursionGroupAttr` - Updated log messages to reflect changes and follow `slog` structured format Signed-off-by: Ethan Dieterich <ethandieterich@gmail.com>
1 parent 56cca05 commit 7208747

3 files changed

Lines changed: 216 additions & 38 deletions

File tree

‎connector/ldap/ldap.go‎

Lines changed: 106 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ import (
6262
type UserMatcher struct {
6363
UserAttr string `json:"userAttr"`
6464
GroupAttr string `json:"groupAttr"`
65+
// Look for parent groups
66+
RecursionGroupAttr string `json:"recursionGroupAttr"`
6567
}
6668

6769
// Config holds configuration options for LDAP logins.
@@ -144,6 +146,8 @@ type Config struct {
144146
UserAttr string `json:"userAttr"`
145147
GroupAttr string `json:"groupAttr"`
146148

149+
RecursionGroupAttr string `json:"recursionGroupAttr"`
150+
147151
// Array of the field pairs used to match a user to a group.
148152
// See the "UserMatcher" struct for the exact field names
149153
//
@@ -197,8 +201,9 @@ func userMatchers(c *Config, logger *slog.Logger) []UserMatcher {
197201
logger.Warn(`use "groupSearch.userMatchers" option instead of "userAttr/groupAttr" fields`, "deprecated", true)
198202
return []UserMatcher{
199203
{
200-
UserAttr: c.GroupSearch.UserAttr,
201-
GroupAttr: c.GroupSearch.GroupAttr,
204+
UserAttr: c.GroupSearch.UserAttr,
205+
GroupAttr: c.GroupSearch.GroupAttr,
206+
RecursionGroupAttr: c.GroupSearch.RecursionGroupAttr,
202207
},
203208
}
204209
}
@@ -591,57 +596,120 @@ func (c *ldapConnector) groups(ctx context.Context, user ldap.Entry) ([]string,
591596
return nil, nil
592597
}
593598

594-
var groups []*ldap.Entry
599+
var groupNames []string
600+
595601
for _, matcher := range c.GroupSearch.UserMatchers {
602+
// Initial Search
603+
var groups []*ldap.Entry
596604
for _, attr := range c.getAttrs(user, matcher.UserAttr) {
597-
filter := fmt.Sprintf("(%s=%s)", matcher.GroupAttr, ldap.EscapeFilter(attr))
598-
if c.GroupSearch.Filter != "" {
599-
filter = fmt.Sprintf("(&%s%s)", c.GroupSearch.Filter, filter)
605+
obtained, filter, err := c.queryGroups(ctx, matcher.GroupAttr, attr)
606+
if err != nil {
607+
return nil, err
608+
}
609+
gotGroups := len(obtained) != 0
610+
if !gotGroups {
611+
// TODO(ericchiang): Is this going to spam the logs?
612+
c.logger.Error("ldap: groups search returned no groups", "filter", filter)
600613
}
614+
groups = append(groups, obtained...)
615+
}
601616

602-
req := &ldap.SearchRequest{
603-
BaseDN: c.GroupSearch.BaseDN,
604-
Filter: filter,
605-
Scope: c.groupSearchScope,
606-
Attributes: []string{c.GroupSearch.NameAttr},
617+
// If RecursionGroupAttr is not set, convert direct groups into names and return
618+
if matcher.RecursionGroupAttr == "" {
619+
for _, group := range groups {
620+
name := c.getAttr(*group, c.GroupSearch.NameAttr)
621+
if name == "" {
622+
return nil, fmt.Errorf(
623+
"ldap: group entity %q missing required attribute %q",
624+
group.DN, c.GroupSearch.NameAttr,
625+
)
626+
}
627+
groupNames = append(groupNames, name)
607628
}
629+
continue
630+
}
631+
632+
// Recursive Search
633+
c.logger.Info("Recursive group search enabled", "groupAttr", matcher.GroupAttr, "recursionAttr", matcher.RecursionGroupAttr)
634+
for {
635+
var nextLevel []*ldap.Entry
636+
for _, group := range groups {
637+
name := c.getAttr(*group, c.GroupSearch.NameAttr)
638+
if name == "" {
639+
return nil, fmt.Errorf("ldap: group entity %q missing required attribute %q",
640+
group.DN, c.GroupSearch.NameAttr)
641+
}
642+
643+
// Prevent duplicates and circular references.
644+
duplicate := false
645+
for _, existingName := range groupNames {
646+
if name == existingName {
647+
c.logger.Debug("Found duplicate group", "name", name)
648+
duplicate = true
649+
break
650+
}
651+
}
652+
if duplicate {
653+
continue
654+
}
655+
656+
groupNames = append(groupNames, name)
608657

609-
gotGroups := false
610-
if err := c.do(ctx, func(conn *ldap.Conn) error {
611-
c.logger.Info("performing ldap search",
612-
"base_dn", req.BaseDN, "scope", scopeString(req.Scope), "filter", req.Filter)
613-
resp, err := conn.Search(req)
658+
// Search for parent groups using the group's DN.
659+
parents, filter, err := c.queryGroups(ctx, matcher.RecursionGroupAttr, group.DN)
614660
if err != nil {
615-
return fmt.Errorf("ldap: search failed: %v", err)
661+
return nil, err
662+
}
663+
if len(parents) == 0 {
664+
c.logger.Debug("No parent groups found", "filter", filter)
665+
} else {
666+
nextLevel = append(nextLevel, parents...)
616667
}
617-
gotGroups = len(resp.Entries) != 0
618-
groups = append(groups, resp.Entries...)
619-
return nil
620-
}); err != nil {
621-
return nil, err
622668
}
623-
if !gotGroups {
624-
// TODO(ericchiang): Is this going to spam the logs?
625-
c.logger.Error("groups search returned no groups", "filter", filter)
669+
if len(nextLevel) == 0 {
670+
break
626671
}
672+
groups = nextLevel
627673
}
628674
}
675+
return groupNames, nil
676+
}
629677

630-
groupNames := make([]string, 0, len(groups))
631-
for _, group := range groups {
632-
name := c.getAttr(*group, c.GroupSearch.NameAttr)
633-
if name == "" {
634-
// Be obnoxious about missing attributes. If the group entry is
635-
// missing its name attribute, that indicates a misconfiguration.
636-
//
637-
// In the future we can add configuration options to just log these errors.
638-
return nil, fmt.Errorf("ldap: group entity %q missing required attribute %q",
639-
group.DN, c.GroupSearch.NameAttr)
640-
}
678+
func (c *ldapConnector) queryGroups(ctx context.Context, memberAttr, dn string) ([]*ldap.Entry, string, error) {
679+
filter := fmt.Sprintf("(%s=%s)", memberAttr, ldap.EscapeFilter(dn))
680+
if c.GroupSearch.Filter != "" {
681+
filter = fmt.Sprintf("(&%s%s)", c.GroupSearch.Filter, filter)
682+
}
641683

642-
groupNames = append(groupNames, name)
684+
req := &ldap.SearchRequest{
685+
BaseDN: c.GroupSearch.BaseDN,
686+
Filter: filter,
687+
Scope: c.groupSearchScope,
688+
Attributes: []string{c.GroupSearch.NameAttr},
689+
}
690+
691+
var entries []*ldap.Entry
692+
if err := c.do(ctx, func(conn *ldap.Conn) error {
693+
c.logger.Info(
694+
"performing ldap search",
695+
"base_dn", req.BaseDN,
696+
"scope", scopeString(req.Scope),
697+
"filter", req.Filter,
698+
)
699+
resp, err := conn.Search(req)
700+
if err != nil {
701+
if ldapErr, ok := err.(*ldap.Error); ok && ldapErr.ResultCode == ldap.LDAPResultNoSuchObject {
702+
c.logger.Info("LDAP search returned no groups", "filter", filter)
703+
return nil
704+
}
705+
return fmt.Errorf("ldap: search failed: %v", err)
706+
}
707+
entries = append(entries, resp.Entries...)
708+
return nil
709+
}); err != nil {
710+
return nil, filter, err
643711
}
644-
return groupNames, nil
712+
return entries, filter, nil
645713
}
646714

647715
func (c *ldapConnector) Prompt() string {

‎connector/ldap/ldap_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,56 @@ func TestUsernamePrompt(t *testing.T) {
525525
}
526526
}
527527

528+
func TestNestedGroups(t *testing.T) {
529+
c := &Config{}
530+
c.UserSearch.BaseDN = "ou=People,ou=TestNestedGroups,dc=example,dc=org"
531+
c.UserSearch.NameAttr = "cn"
532+
c.UserSearch.EmailAttr = "mail"
533+
c.UserSearch.IDAttr = "DN"
534+
c.UserSearch.Username = "cn"
535+
536+
c.GroupSearch.BaseDN = "ou=TestNestedGroups,dc=example,dc=org"
537+
c.GroupSearch.UserMatchers = []UserMatcher{
538+
{
539+
UserAttr: "DN",
540+
GroupAttr: "member",
541+
// Enable Recursive Search
542+
RecursionGroupAttr: "member",
543+
},
544+
}
545+
c.GroupSearch.NameAttr = "cn"
546+
547+
tests := []subtest{
548+
{
549+
name: "nestedgroups_jane",
550+
username: "jane",
551+
password: "foo",
552+
groups: true,
553+
want: connector.Identity{
554+
UserID: "cn=jane,ou=People,ou=TestNestedGroups,dc=example,dc=org",
555+
Username: "jane",
556+
Email: "janedoe@example.com",
557+
EmailVerified: true,
558+
Groups: []string{"childGroup", "circularGroup1", "intermediateGroup", "circularGroup2", "parentGroup"},
559+
},
560+
},
561+
{
562+
name: "nestedgroups_john",
563+
username: "john",
564+
password: "bar",
565+
groups: true,
566+
want: connector.Identity{
567+
UserID: "cn=john,ou=People,ou=TestNestedGroups,dc=example,dc=org",
568+
Username: "john",
569+
Email: "johndoe@example.com",
570+
EmailVerified: true,
571+
Groups: []string{"circularGroup2", "intermediateGroup", "circularGroup1", "parentGroup"},
572+
},
573+
},
574+
}
575+
runTests(t, connectLDAP, c, tests)
576+
}
577+
528578
func getenv(key, defaultVal string) string {
529579
if val := os.Getenv(key); val != "" {
530580
return val

‎connector/ldap/testdata/schema.ldif‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,3 +445,63 @@ sn: doe
445445
cn: jane
446446
mail: janedoe@example.com
447447
userpassword: foo
448+
449+
########################################################################
450+
451+
dn: ou=TestNestedGroups,dc=example,dc=org
452+
objectClass: organizationalUnit
453+
ou: TestNestedGroups
454+
455+
dn: ou=People,ou=TestNestedGroups,dc=example,dc=org
456+
objectClass: organizationalUnit
457+
ou: People
458+
459+
dn: cn=jane,ou=People,ou=TestNestedGroups,dc=example,dc=org
460+
objectClass: person
461+
objectClass: inetOrgPerson
462+
sn: doe
463+
cn: jane
464+
mail: janedoe@example.com
465+
userpassword: foo
466+
467+
dn: cn=john,ou=People,ou=TestNestedGroups,dc=example,dc=org
468+
objectClass: person
469+
objectClass: inetOrgPerson
470+
sn: doe
471+
cn: john
472+
mail: johndoe@example.com
473+
userpassword: bar
474+
475+
# Group definitions.
476+
477+
dn: ou=Groups,ou=TestNestedGroups,dc=example,dc=org
478+
objectClass: organizationalUnit
479+
ou: Groups
480+
481+
dn: cn=childGroup,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
482+
objectClass: groupOfNames
483+
cn: childGroup
484+
member: cn=jane,ou=People,ou=TestNestedGroups,dc=example,dc=org
485+
486+
dn: cn=intermediateGroup,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
487+
objectClass: groupOfNames
488+
cn: intermediateGroup
489+
member: cn=childGroup,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
490+
member: cn=john,ou=People,ou=TestNestedGroups,dc=example,dc=org
491+
492+
dn: cn=parentGroup,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
493+
objectClass: groupOfNames
494+
cn: parentGroup
495+
member: cn=intermediateGroup,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
496+
497+
dn: cn=circularGroup1,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
498+
objectClass: groupOfNames
499+
cn: circularGroup1
500+
member: cn=circularGroup2,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
501+
member: cn=jane,ou=People,ou=TestNestedGroups,dc=example,dc=org
502+
503+
dn: cn=circularGroup2,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
504+
objectClass: groupOfNames
505+
cn: circularGroup2
506+
member: cn=circularGroup1,ou=Groups,ou=TestNestedGroups,dc=example,dc=org
507+
member: cn=john,ou=People,ou=TestNestedGroups,dc=example,dc=org

0 commit comments

Comments
 (0)