Skip to content

Commit 289f9a7

Browse files
committed
fix(cache): DN-aware immediate-child + OU-parent reconstruction
- addOUChildren: parse DNs and compare RDN components instead of string-matching. Fixes false-negative on immediate children whose CN contains an escaped comma (e.g. "cn=Last\, First,ou=..."). - immediateOUFromDN: use go-ldap's DN serializer so escaped/multi- valued RDNs round-trip correctly instead of manual concat. - Regression test TestBuildGraph_OUFocus_EscapedCommaChild pins the escaped-comma case. Mutation-tested: temporarily reverting the DN-parsing check makes the test fail as expected. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 749dea1 commit 289f9a7

2 files changed

Lines changed: 71 additions & 34 deletions

File tree

internal/ldap_cache/graph.go

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -386,29 +386,11 @@ func immediateOUFromDN(dn string) string {
386386
}
387387

388388
tail := parsed.RDNs[1:]
389-
390-
var buf strings.Builder
391-
392-
for i, r := range tail {
393-
if i > 0 {
394-
buf.WriteString(",")
395-
}
396-
397-
for _, a := range r.Attributes {
398-
buf.WriteString(a.Type)
399-
buf.WriteString("=")
400-
buf.WriteString(a.Value)
401-
}
402-
}
403-
404-
out := buf.String()
405-
406-
// Only return if first RDN is ou=
407-
if len(tail) > 0 && strings.EqualFold(tail[0].Attributes[0].Type, "ou") {
408-
return out
389+
if len(tail[0].Attributes) == 0 || !strings.EqualFold(tail[0].Attributes[0].Type, "ou") {
390+
return ""
409391
}
410392

411-
return ""
393+
return (&goldap.DN{RDNs: tail}).String()
412394
}
413395

414396
func labelForOU(dn string) string {
@@ -467,16 +449,29 @@ func addOU(data *GraphData, seen map[string]int, ouDN string, ring int, edge Edg
467449
}
468450

469451
func (m *Manager) addOUChildren(data *GraphData, seen map[string]int, ouDN string, ring int) {
470-
suffix := "," + ouDN
452+
ouParsed, err := goldap.ParseDN(ouDN)
453+
if err != nil {
454+
return
455+
}
471456

472-
for _, u := range m.Users.Get() {
473-
if !strings.HasSuffix(u.DN(), suffix) {
474-
continue
457+
isImmediateChild := func(childDN string) bool {
458+
p, err := goldap.ParseDN(childDN)
459+
if err != nil || len(p.RDNs) != len(ouParsed.RDNs)+1 {
460+
return false
475461
}
476462

477-
rel := strings.TrimSuffix(u.DN(), suffix)
478-
if strings.Contains(rel, ",") {
479-
continue // not an immediate child
463+
for i, rdn := range ouParsed.RDNs {
464+
if !rdn.Equal(p.RDNs[i+1]) {
465+
return false
466+
}
467+
}
468+
469+
return true
470+
}
471+
472+
for _, u := range m.Users.Get() {
473+
if !isImmediateChild(u.DN()) {
474+
continue
480475
}
481476

482477
if _, dup := seen[u.DN()]; dup {
@@ -489,12 +484,7 @@ func (m *Manager) addOUChildren(data *GraphData, seen map[string]int, ouDN strin
489484
}
490485

491486
for _, c := range m.Computers.Get() {
492-
if !strings.HasSuffix(c.DN(), suffix) {
493-
continue
494-
}
495-
496-
rel := strings.TrimSuffix(c.DN(), suffix)
497-
if strings.Contains(rel, ",") {
487+
if !isImmediateChild(c.DN()) {
498488
continue
499489
}
500490

internal/ldap_cache/graph_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,53 @@ func TestBuildGraph_CycleSafe(t *testing.T) {
353353
}
354354
}
355355

356+
func TestBuildGraph_OUFocus_EscapedCommaChild(t *testing.T) {
357+
manager := New(&mockLDAPClient{})
358+
// "Last, First" with the comma escaped per RFC 4514.
359+
childDN := `cn=Last\, First,ou=Engineering,dc=ex,dc=com`
360+
manager.Users.setAll([]ldap.User{
361+
newUserWithDN(childDN, "Last, First", "lastf", true, nil),
362+
})
363+
364+
ouDN := "ou=Engineering,dc=ex,dc=com"
365+
data, err := manager.BuildGraph(ouDN, 1)
366+
if err != nil {
367+
t.Fatalf("BuildGraph: %v", err)
368+
}
369+
370+
foundNode := false
371+
372+
for _, n := range data.Nodes {
373+
if n.DN == childDN {
374+
foundNode = true
375+
376+
break
377+
}
378+
}
379+
380+
if !foundNode {
381+
t.Fatalf("escaped-comma immediate child %q must appear in depth-1 nodes for %q", childDN, ouDN)
382+
}
383+
384+
foundEdge := false
385+
386+
for _, e := range data.Edges {
387+
if e.Source == ouDN && e.Target == childDN {
388+
if e.Kind != EdgeContains {
389+
t.Errorf("edge kind for escaped-comma child: got %q, want %q", e.Kind, EdgeContains)
390+
}
391+
392+
foundEdge = true
393+
394+
break
395+
}
396+
}
397+
398+
if !foundEdge {
399+
t.Errorf("escaped-comma immediate child %q must have contains edge from %q", childDN, ouDN)
400+
}
401+
}
402+
356403
func TestAssignConcentric_EvenDistribution(t *testing.T) {
357404
m := graphFixture(t)
358405
data, _ := m.BuildGraph("cn=bob,ou=Engineering,dc=ex,dc=com", 1)

0 commit comments

Comments
 (0)