From f7dd51b8d265bca85108d3129944cee83c4c4de3 Mon Sep 17 00:00:00 2001 From: tegwick Date: Mon, 7 Sep 2026 08:45:50 +0200 Subject: [PATCH] Make the LLDAP export report its own completeness The exporter discovered groups by walking each user's memberships, so a group nobody belongs to never reached the snapshot, and a failed lookup was skipped by a `continue` under a comment claiming it was recorded in the incompatibility report. The run then emitted `result: "success"`. Add an optional `domain.GroupLister` capability and implement `ListGroups` on the LLDAP adapter as a direct group-subtree search, kept off `UserRepository` because the OIDC layer never enumerates the directory. Record `groupEnumeration` on every result and a `Complete()` predicate over it; abort rather than write a smaller snapshot when the enumeration fails; report a failed per-user lookup on the fallback path; emit `partial` telemetry and name the mode from the CLI. Reading the adapter to write this surfaced a defect the assessment had not listed: `LookupGroups` never populated `Group.Members`, and the exporter built every membership from that field, so against a real directory the `memberships` block was always empty while the fixture-backed tests passed. Memberships on the fallback path now come from the user/group pair actually observed. Sort users, groups and memberships so an unchanged directory exports identically. Closes G05 of the scope/intent assessment. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012WAsfsfQmDu4vcBhiMcmQp Assistant: claude-code Assistant-Model: opus Assistant-Process: 867844@bnt-lap001 Assistant-Session: 3d45905e-0016-4b49-b828-231406881f7b --- SCOPE.md | 10 +- ...26-09-05-011726-scope-intent-assessment.md | 17 ++ src/cmd/lldap-export/main.go | 9 +- src/internal/adapters/lldap/adapter.go | 47 +++++ src/internal/adapters/lldap/adapter_test.go | 67 +++++++ src/internal/domain/repository.go | 12 ++ .../migration/lldapexport/exporter.go | 147 +++++++++++---- .../migration/lldapexport/exporter_test.go | 172 +++++++++++++++++- ...EY-WP-0018-export-completeness-evidence.md | 100 ++++++++++ 9 files changed, 538 insertions(+), 43 deletions(-) create mode 100644 workplans/KEY-WP-0018-export-completeness-evidence.md diff --git a/SCOPE.md b/SCOPE.md index 2e434e6..6d57d07 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -25,7 +25,7 @@ Keycloak interchangeability are not established. | Service authentication | Static confidential `client_credentials` clients authenticated with form-encoded `client_secret_basic`; configured subject, tenant, roles, scopes and per-client token lifetime. Secrets are resolved from environment references at startup. | | Tokens and identity | Locally signed RS256 JWTs; configurable access-token resource audience while ID tokens retain the client audience; human/service principal types, tenant, groups, roles, scope and assurance claims. UserInfo resolves canonical directory subjects and filters profile/email/groups by scope. | | Caller commands | `keycape login` for public-client browser PKCE login and `keycape service-token` for service exchange. HTTPS discovery/JWKS verification and private JSON token-file delivery outside Git; no token output on stdout. | -| Validation and migration | Canonical snapshot checks; LLDAP user/group/membership export; basic Keycloak realm JSON; LDIF generation for OpenLDAP, 389 Directory Server and AD targets. These generate artifacts rather than execute a complete migration. | +| Validation and migration | Canonical snapshot checks; deterministic LLDAP user/group/membership export that records whether it enumerated the whole directory; basic Keycloak realm JSON; LDIF generation for OpenLDAP, 389 Directory Server and AD targets. These generate artifacts rather than execute a complete migration. | | Diagnostics and packaging | Structured authentication/enforcement/migration events, a process health response, Go build/test/vet targets, a container containing the KeyCape binary, and development/CI scaffolding. | ## Material limits @@ -38,6 +38,14 @@ Keycloak interchangeability are not established. - The optional tenant-engine `tenant_roles` adapter and handler support exist, but the server entry point does not configure them. That claim is not an enabled capability of the stock executable. +- The LLDAP export enumerates the group subtree directly, so groups with no + members are present, and every snapshot carries a `groupEnumeration` field + saying whether that enumeration ran or the membership-derived fallback did + (KEY-WP-0018). A failed enumeration aborts the export rather than writing a + smaller snapshot, and read failures are reported rather than skipped. Users, + groups and memberships are sorted, so an unchanged directory exports + identically. Completeness beyond users, groups and memberships — passwords and + MFA credentials in particular — is still not covered. - The Keycloak CLI exports users/groups through the basic transformer and has no client-list input. Library-level client mapping does not preserve the full current service-identity, audience, tenant/role, MFA and lifetime contract. diff --git a/history/2026-09-05-011726-scope-intent-assessment.md b/history/2026-09-05-011726-scope-intent-assessment.md index 7952341..5eee689 100644 --- a/history/2026-09-05-011726-scope-intent-assessment.md +++ b/history/2026-09-05-011726-scope-intent-assessment.md @@ -184,6 +184,23 @@ unspecified. reads, define ordering, and test empty groups and backend failures. Preserve completeness evidence before claiming deterministic full snapshots. +**Status 2026-09-07 (KEY-WP-0018): closed.** `LDAPAdapter` gained a +`ListGroups` enumeration over the group subtree, offered to the exporter through +an optional `domain.GroupLister` rather than by widening `UserRepository`, so +groups with no members are in the snapshot and `Group.Members` is populated from +the directory. Reading the adapter to write it surfaced a defect the assessment +had not listed: `LookupGroups` never set `Members`, and the exporter built every +membership from that field, so a real export's `memberships` block was always +empty while the fixture-backed tests passed. Each result now carries +`groupEnumeration` (`directory` or `membership-derived`) and a `Complete()` +predicate; a failed enumeration aborts instead of writing a smaller snapshot, a +failed per-user lookup on the fallback path is reported rather than dropped, an +incomplete run emits `partial` telemetry, and the CLI names the mode. Users, +groups and memberships are sorted on stable keys. Tests cover the empty group, +the enumeration failure, the fallback lookup failure and repeat-run determinism. +This is completeness evidence for the user/group/membership surface only — +credential migration remains out of scope under G03. + ### G06 — The validator is narrower than schema enforcement **Priority: medium. Kind: implementation/claim gap.** diff --git a/src/cmd/lldap-export/main.go b/src/cmd/lldap-export/main.go index 76e5f8b..62fc616 100644 --- a/src/cmd/lldap-export/main.go +++ b/src/cmd/lldap-export/main.go @@ -52,8 +52,13 @@ func main() { os.Exit(1) } - fmt.Fprintf(os.Stdout, "Exported %d users, %d groups to %s\n", - len(result.Users), len(result.Groups), *output) + fmt.Fprintf(os.Stdout, "Exported %d users, %d groups to %s (group enumeration: %s)\n", + len(result.Users), len(result.Groups), *output, result.GroupEnumeration) + + if result.GroupEnumeration != lldapexport.EnumerationDirectory { + fmt.Fprintln(os.Stderr, + "lldap-export: groups were derived from user memberships; groups with no members are absent") + } if len(result.IncompatibilityReport) > 0 { fmt.Fprintln(os.Stderr, "Incompatibility report:") diff --git a/src/internal/adapters/lldap/adapter.go b/src/internal/adapters/lldap/adapter.go index 6121cca..623db8e 100644 --- a/src/internal/adapters/lldap/adapter.go +++ b/src/internal/adapters/lldap/adapter.go @@ -175,6 +175,47 @@ func (a *LDAPAdapter) LookupGroups(ctx context.Context, userDN string) ([]domain return groups, nil } +// ListGroups returns every group in the LLDAP group subtree with its members +// populated, independently of any user's membership. Export tooling needs this +// to enumerate empty and unreferenced groups, which a (member=DN) search by +// construction cannot see. +func (a *LDAPAdapter) ListGroups(ctx context.Context) ([]domain.Group, error) { + conn, err := a.dial() + if err != nil { + return nil, err + } + defer conn.Close() + + req := ldap.NewSearchRequest( + a.cfg.groupBaseDN(), + ldap.ScopeWholeSubtree, + ldap.NeverDerefAliases, + 0, 0, false, + "(|(objectClass=groupOfNames)(objectClass=groupOfUniqueNames))", + []string{"dn", "cn", "description", "member", "uniqueMember"}, + nil, + ) + result, err := conn.Search(req) + if err != nil { + return nil, fmt.Errorf("lldap: list groups search: %w", err) + } + + groups := make([]domain.Group, 0, len(result.Entries)) + for _, entry := range result.Entries { + members := entry.GetAttributeValues("member") + if len(members) == 0 { + members = entry.GetAttributeValues("uniqueMember") + } + groups = append(groups, domain.Group{ + ID: entry.DN, + Name: entry.GetAttributeValue("cn"), + Description: entry.GetAttributeValue("description"), + Members: members, + }) + } + return groups, nil +} + // ListUsers returns all user records from the LLDAP directory. // It performs an LDAP search with filter (objectClass=inetOrgPerson) to list every user, // then validates each against the canonical LDAP schema. @@ -388,3 +429,9 @@ func validationSummary(r validator.Report) string { } return strings.Join(msgs, "; ") } + +// Compile-time checks: the adapter satisfies the directory contracts it claims. +var ( + _ domain.UserRepository = (*LDAPAdapter)(nil) + _ domain.GroupLister = (*LDAPAdapter)(nil) +) diff --git a/src/internal/adapters/lldap/adapter_test.go b/src/internal/adapters/lldap/adapter_test.go index d94bd5f..e153314 100644 --- a/src/internal/adapters/lldap/adapter_test.go +++ b/src/internal/adapters/lldap/adapter_test.go @@ -3,6 +3,7 @@ package lldap_test import ( "context" "errors" + "strings" "testing" "github.com/go-ldap/ldap/v3" @@ -427,3 +428,69 @@ func TestValidatePassword_UserNotFound(t *testing.T) { t.Error("expected false for non-existent user") } } + +// --------------------------------------------------------------------------- +// ListGroups (KEY-WP-0018) +// --------------------------------------------------------------------------- + +// ListGroups must search the group subtree directly rather than by membership, +// so that groups nobody belongs to are returned, and it must populate Members — +// LookupGroups leaves that field empty, which silently emptied every export. +func TestLDAPAdapter_ListGroups_ReturnsAllGroupsWithMembers(t *testing.T) { + var gotFilter, gotBase string + conn := &mockConn{ + searchFn: func(req *ldap.SearchRequest) (*ldap.SearchResult, error) { + gotFilter, gotBase = req.Filter, req.BaseDN + return &ldap.SearchResult{Entries: []*ldap.Entry{ + { + DN: "cn=admins,ou=groups,dc=netkingdom,dc=local", + Attributes: []*ldap.EntryAttribute{ + {Name: "cn", Values: []string{"admins"}}, + {Name: "description", Values: []string{"Administrators"}}, + {Name: "member", Values: []string{"uid=alice,ou=people,dc=netkingdom,dc=local"}}, + }, + }, + { + DN: "cn=orphans,ou=groups,dc=netkingdom,dc=local", + Attributes: []*ldap.EntryAttribute{ + {Name: "cn", Values: []string{"orphans"}}, + }, + }, + }}, nil + }, + } + adapter := lldap.NewForTest(testConfig(), func(string) (lldap.LDAPConn, error) { return conn, nil }) + + groups, err := adapter.ListGroups(context.Background()) + if err != nil { + t.Fatalf("ListGroups returned error: %v", err) + } + if len(groups) != 2 { + t.Fatalf("want 2 groups, got %d: %+v", len(groups), groups) + } + if groups[0].Name != "admins" || len(groups[0].Members) != 1 { + t.Errorf("first group not mapped with members: %+v", groups[0]) + } + if len(groups[1].Members) != 0 { + t.Errorf("member-less group should have no members: %+v", groups[1]) + } + if strings.Contains(gotFilter, "member=") { + t.Errorf("ListGroups must not filter by membership, filter was %q", gotFilter) + } + if !strings.Contains(gotBase, "ou=groups") { + t.Errorf("ListGroups should search the group subtree, base was %q", gotBase) + } +} + +func TestLDAPAdapter_ListGroups_PropagatesSearchError(t *testing.T) { + conn := &mockConn{ + searchFn: func(*ldap.SearchRequest) (*ldap.SearchResult, error) { + return nil, errors.New("ldap: server unavailable") + }, + } + adapter := lldap.NewForTest(testConfig(), func(string) (lldap.LDAPConn, error) { return conn, nil }) + + if _, err := adapter.ListGroups(context.Background()); err == nil { + t.Fatal("expected an error when the group search fails, got nil") + } +} diff --git a/src/internal/domain/repository.go b/src/internal/domain/repository.go index a52bdcf..56252d8 100644 --- a/src/internal/domain/repository.go +++ b/src/internal/domain/repository.go @@ -29,3 +29,15 @@ const ErrUserNotFound = userNotFound("user not found") type userNotFound string func (e userNotFound) Error() string { return string(e) } + +// GroupLister is an optional capability of a UserRepository: enumerating every +// group in the directory independently of any user's membership. Migration and +// export tooling needs it to prove a snapshot is complete — a group nobody +// belongs to is invisible to LookupGroups. It is deliberately separate from +// UserRepository because the OIDC layer never enumerates the directory. +type GroupLister interface { + // ListGroups returns every group in the directory with its members + // populated. An error means the enumeration is incomplete; callers must + // not treat a partial result as a full snapshot. + ListGroups(ctx context.Context) ([]Group, error) +} diff --git a/src/internal/migration/lldapexport/exporter.go b/src/internal/migration/lldapexport/exporter.go index 2526e5e..2e5f442 100644 --- a/src/internal/migration/lldapexport/exporter.go +++ b/src/internal/migration/lldapexport/exporter.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "os" + "sort" "time" "gopkg.in/yaml.v3" @@ -16,14 +17,40 @@ import ( "keycape/internal/validator" ) +// Group enumeration modes recorded on an ExportResult. They are the export's +// own statement about how much of the directory it could see. +const ( + // EnumerationDirectory means every group was read from the directory, + // so groups nobody belongs to are present in the snapshot. + EnumerationDirectory = "directory" + + // EnumerationMembershipDerived means groups were discovered by walking + // each user's memberships. Empty and unreferenced groups are absent by + // construction; such a snapshot is not a complete directory export. + EnumerationMembershipDerived = "membership-derived" +) + // ExportResult is the structured output of a single export run. type ExportResult struct { - Users []domain.User `yaml:"users"` - Groups []domain.Group `yaml:"groups"` - Memberships []domain.Membership `yaml:"memberships"` - ExportedAt time.Time `yaml:"exportedAt"` - ProfileVersion string `yaml:"profileVersion"` - IncompatibilityReport []string `yaml:"incompatibilityReport,omitempty"` + Users []domain.User `yaml:"users"` + Groups []domain.Group `yaml:"groups"` + Memberships []domain.Membership `yaml:"memberships"` + ExportedAt time.Time `yaml:"exportedAt"` + + // GroupEnumeration is EnumerationDirectory or EnumerationMembershipDerived. + // It is written unconditionally: a consumer must not have to infer from an + // absent field whether the snapshot covers the whole directory. + GroupEnumeration string `yaml:"groupEnumeration"` + + ProfileVersion string `yaml:"profileVersion"` + IncompatibilityReport []string `yaml:"incompatibilityReport,omitempty"` +} + +// Complete reports whether the run enumerated the whole directory and read +// every entry it attempted. Only a complete run may be described as a full +// directory snapshot. +func (r *ExportResult) Complete() bool { + return r.GroupEnumeration == EnumerationDirectory && len(r.IncompatibilityReport) == 0 } // Exporter reads from a UserRepository, validates, and writes canonical-export.yaml. @@ -52,29 +79,16 @@ func (e *Exporter) Export(ctx context.Context, outputFile string) (*ExportResult return nil, fmt.Errorf("lldapexport: list users: %w", err) } - // 2. List all groups by looking up groups for each user's DN. - // Since UserRepository.LookupGroups takes a userDN, we collect groups - // from all users and deduplicate by group ID. - groupMap := make(map[string]domain.Group) - for _, u := range users { - userGroups, err := e.repo.LookupGroups(ctx, u.ID) - if err != nil { - // Non-fatal: log in incompatibility report. - continue - } - for _, g := range userGroups { - if _, seen := groupMap[g.ID]; !seen { - groupMap[g.ID] = g - } - } - } - groups := make([]domain.Group, 0, len(groupMap)) - for _, g := range groupMap { - groups = append(groups, g) + // 2. Enumerate groups. A directory-wide enumeration is the only way to + // see groups nobody belongs to, so prefer it and fall back to walking + // user memberships only when the repository cannot provide one. + groups, memberships, enumeration, readErrs, err := e.enumerateGroups(ctx, users) + if err != nil { + return nil, err } // 3. Validate each user against the canonical LDAP schema. - var incompatibilities []string + incompatibilities := readErrs validatedUsers := make([]domain.User, 0, len(users)) for _, u := range users { snap := validator.Snapshot{Users: []domain.User{u}} @@ -96,16 +110,16 @@ func (e *Exporter) Export(ctx context.Context, outputFile string) (*ExportResult validatedUsers = append(validatedUsers, u) } - // 4. Build memberships from group member lists. - var memberships []domain.Membership - for _, g := range groups { - for _, memberID := range g.Members { - memberships = append(memberships, domain.Membership{ - UserID: memberID, - GroupID: g.ID, - }) + // 4. Order every collection on a stable key so an unchanged directory + // exports byte-identically across runs. + sort.Slice(validatedUsers, func(i, j int) bool { return validatedUsers[i].ID < validatedUsers[j].ID }) + sort.Slice(groups, func(i, j int) bool { return groups[i].ID < groups[j].ID }) + sort.Slice(memberships, func(i, j int) bool { + if memberships[i].GroupID != memberships[j].GroupID { + return memberships[i].GroupID < memberships[j].GroupID } - } + return memberships[i].UserID < memberships[j].UserID + }) // 5. Build ExportResult. result := &ExportResult{ @@ -113,16 +127,23 @@ func (e *Exporter) Export(ctx context.Context, outputFile string) (*ExportResult Groups: groups, Memberships: memberships, ExportedAt: time.Now().UTC(), + GroupEnumeration: enumeration, ProfileVersion: "0.1", IncompatibilityReport: incompatibilities, } - // 6. Emit migration_event telemetry. + // 6. Emit migration_event telemetry. A snapshot that skipped an entry or + // could not enumerate the directory is reported as partial — calling it + // a success is what let an incomplete export pass unnoticed. + outcome := "partial" + if result.Complete() { + outcome = "success" + } e.emitter.Emit(ctx, telemetry.Event{ Timestamp: time.Now().UTC(), EventType: telemetry.EventMigration, Endpoint: "lldap-export", - Result: "success", + Result: outcome, }) // 7. Write YAML to output file. @@ -136,3 +157,55 @@ func (e *Exporter) Export(ctx context.Context, outputFile string) (*ExportResult return result, nil } + +// enumerateGroups returns the directory's groups and memberships, the +// enumeration mode that produced them, and any read failures to record in the +// incompatibility report. +// +// A failed directory enumeration is fatal: the caller asked for the whole +// directory and cannot be handed a silently smaller one. A failed per-user +// lookup in the fallback path is reported rather than fatal, because the +// fallback is already known to be incomplete and the report is where that is +// stated. +func (e *Exporter) enumerateGroups(ctx context.Context, users []domain.User) ( + groups []domain.Group, memberships []domain.Membership, enumeration string, readErrs []string, err error, +) { + if lister, ok := e.repo.(domain.GroupLister); ok { + groups, err = lister.ListGroups(ctx) + if err != nil { + return nil, nil, "", nil, fmt.Errorf("lldapexport: list groups: %w", err) + } + for _, g := range groups { + for _, memberID := range g.Members { + memberships = append(memberships, domain.Membership{UserID: memberID, GroupID: g.ID}) + } + } + return groups, memberships, EnumerationDirectory, nil, nil + } + + readErrs = append(readErrs, "export incomplete: the directory adapter cannot enumerate groups, "+ + "so groups with no members are absent from this snapshot") + + groupMap := make(map[string]domain.Group) + for _, u := range users { + userGroups, lookupErr := e.repo.LookupGroups(ctx, u.ID) + if lookupErr != nil { + readErrs = append(readErrs, + fmt.Sprintf("user %q group lookup failed, memberships omitted: %v", u.Username, lookupErr)) + continue + } + for _, g := range userGroups { + if _, seen := groupMap[g.ID]; !seen { + groupMap[g.ID] = g + } + // Derive the membership from the pair actually observed. The + // group's own Members list is not populated on this path. + memberships = append(memberships, domain.Membership{UserID: u.ID, GroupID: g.ID}) + } + } + groups = make([]domain.Group, 0, len(groupMap)) + for _, g := range groupMap { + groups = append(groups, g) + } + return groups, memberships, EnumerationMembershipDerived, readErrs, nil +} diff --git a/src/internal/migration/lldapexport/exporter_test.go b/src/internal/migration/lldapexport/exporter_test.go index 566d015..e9071d1 100644 --- a/src/internal/migration/lldapexport/exporter_test.go +++ b/src/internal/migration/lldapexport/exporter_test.go @@ -2,8 +2,10 @@ package lldapexport_test import ( "context" + "errors" "os" "path/filepath" + "strings" "testing" "keycape/internal/domain" @@ -74,10 +76,10 @@ func validUser() domain.User { func validGroup() domain.Group { return domain.Group{ - ID: "cn=admins,ou=groups,dc=example,dc=local", - Name: "admins", + ID: "cn=admins,ou=groups,dc=example,dc=local", + Name: "admins", Description: "Admin group", - Members: []string{"uid=alice,ou=users,dc=example,dc=local"}, + Members: []string{"uid=alice,ou=users,dc=example,dc=local"}, } } @@ -233,3 +235,167 @@ func TestExporter_Export_ProfileVersion(t *testing.T) { t.Errorf("expected ProfileVersion 0.1, got %q", result.ProfileVersion) } } + +// --------------------------------------------------------------------------- +// Completeness evidence (KEY-WP-0018) +// --------------------------------------------------------------------------- + +// listerRepo is a mockRepo that can also enumerate the directory's groups, +// like the real LLDAP adapter. listErr makes that enumeration fail. +type listerRepo struct { + mockRepo + allGroups []domain.Group + listErr error +} + +func (l *listerRepo) ListGroups(_ context.Context) ([]domain.Group, error) { + if l.listErr != nil { + return nil, l.listErr + } + return l.allGroups, nil +} + +var _ domain.GroupLister = (*listerRepo)(nil) + +// failingLookupRepo has no ListGroups, so the exporter falls back to walking +// memberships — and every lookup on that path fails. +type failingLookupRepo struct{ mockRepo } + +func (f *failingLookupRepo) LookupGroups(_ context.Context, _ string) ([]domain.Group, error) { + return nil, errors.New("ldap: connection reset") +} + +func exportWith(t *testing.T, repo domain.UserRepository) (*lldapexport.ExportResult, *capEmitter) { + t.Helper() + em := &capEmitter{} + outFile := filepath.Join(t.TempDir(), "export.yaml") + exp := lldapexport.New(repo, validator.ModeProvisioning, em) + result, err := exp.Export(context.Background(), outFile) + if err != nil { + t.Fatalf("Export returned error: %v", err) + } + return result, em +} + +// A group nobody belongs to is invisible to a membership walk, which is the +// completeness defect the directory enumeration exists to fix. +func TestExporter_Export_IncludesGroupWithNoMembers(t *testing.T) { + empty := domain.Group{ID: "cn=orphans,ou=groups,dc=example,dc=local", Name: "orphans"} + repo := &listerRepo{ + mockRepo: mockRepo{users: []domain.User{validUser()}}, + allGroups: []domain.Group{validGroup(), empty}, + } + + result, _ := exportWith(t, repo) + + if result.GroupEnumeration != lldapexport.EnumerationDirectory { + t.Fatalf("group enumeration: want %q, got %q", + lldapexport.EnumerationDirectory, result.GroupEnumeration) + } + found := false + for _, g := range result.Groups { + if g.ID == empty.ID { + found = true + } + } + if !found { + t.Errorf("empty group %q missing from export: %+v", empty.ID, result.Groups) + } + if !result.Complete() { + t.Errorf("expected a complete export, report: %v", result.IncompatibilityReport) + } +} + +// A directory enumeration that fails cannot be downgraded to a partial +// snapshot: the caller asked for the whole directory. +func TestExporter_Export_GroupEnumerationFailureIsFatal(t *testing.T) { + repo := &listerRepo{ + mockRepo: mockRepo{users: []domain.User{validUser()}}, + listErr: errors.New("ldap: search failed"), + } + + outFile := filepath.Join(t.TempDir(), "export.yaml") + exp := lldapexport.New(repo, validator.ModeProvisioning, &capEmitter{}) + if _, err := exp.Export(context.Background(), outFile); err == nil { + t.Fatal("expected an error when group enumeration fails, got nil") + } + if _, err := os.Stat(outFile); !os.IsNotExist(err) { + t.Error("no snapshot file should be written when the directory could not be enumerated") + } +} + +// The fallback path must say it is a fallback, and say which users it skipped. +func TestExporter_Export_FallbackReportsIncompleteness(t *testing.T) { + repo := &failingLookupRepo{mockRepo: mockRepo{users: []domain.User{validUser()}}} + + result, em := exportWith(t, repo) + + if result.GroupEnumeration != lldapexport.EnumerationMembershipDerived { + t.Errorf("group enumeration: want %q, got %q", + lldapexport.EnumerationMembershipDerived, result.GroupEnumeration) + } + if result.Complete() { + t.Error("a membership-derived export with a failed lookup must not report itself complete") + } + if !reportMentions(result.IncompatibilityReport, "group lookup failed") { + t.Errorf("failed lookup not reported: %v", result.IncompatibilityReport) + } + if !reportMentions(result.IncompatibilityReport, "export incomplete") { + t.Errorf("fallback enumeration not reported: %v", result.IncompatibilityReport) + } + for _, ev := range em.events { + if ev.EventType == telemetry.EventMigration && ev.Result != "partial" { + t.Errorf("telemetry result: want %q, got %q", "partial", ev.Result) + } + } +} + +// An unchanged directory must export byte-identically, whatever order the +// backend and Go's map iteration happen to produce. +func TestExporter_Export_IsDeterministic(t *testing.T) { + users := []domain.User{ + {ID: "uid=zoe,ou=users,dc=example,dc=local", Username: "zoe", DisplayName: "Zoe", Email: "zoe@example.com", Enabled: true}, + validUser(), + } + groups := []domain.Group{ + {ID: "cn=zeta,ou=groups,dc=example,dc=local", Name: "zeta", Members: []string{users[0].ID, users[1].ID}}, + {ID: "cn=alpha,ou=groups,dc=example,dc=local", Name: "alpha", Members: []string{users[0].ID}}, + } + + var first string + for run := 0; run < 3; run++ { + repo := &listerRepo{mockRepo: mockRepo{users: users}, allGroups: groups} + result, _ := exportWith(t, repo) + + var got []string + for _, g := range result.Groups { + got = append(got, "g:"+g.ID) + } + for _, m := range result.Memberships { + got = append(got, "m:"+m.GroupID+"/"+m.UserID) + } + for _, u := range result.Users { + got = append(got, "u:"+u.ID) + } + joined := strings.Join(got, "\n") + if run == 0 { + first = joined + continue + } + if joined != first { + t.Fatalf("export order differs between runs:\nfirst:\n%s\nrun %d:\n%s", first, run, joined) + } + } + if !strings.Contains(first, "g:cn=alpha,ou=groups,dc=example,dc=local\ng:cn=zeta") { + t.Errorf("groups are not sorted by ID:\n%s", first) + } +} + +func reportMentions(report []string, substr string) bool { + for _, entry := range report { + if strings.Contains(entry, substr) { + return true + } + } + return false +} diff --git a/workplans/KEY-WP-0018-export-completeness-evidence.md b/workplans/KEY-WP-0018-export-completeness-evidence.md new file mode 100644 index 0000000..e37e71b --- /dev/null +++ b/workplans/KEY-WP-0018-export-completeness-evidence.md @@ -0,0 +1,100 @@ +--- +id: KEY-WP-0018 +type: workplan +title: "Make the LLDAP export report its own completeness" +domain: infotech +repo: key-cape +status: finished +owner: claude +topic_slug: export-completeness-evidence +created: "2026-09-07" +updated: "2026-09-07" +--- + +Closes gap G05 of `history/2026-09-05-011726-scope-intent-assessment.md`. + +`src/internal/migration/lldapexport/exporter.go` discovers groups by walking each +user's memberships, so a group nobody belongs to is absent from the snapshot and +nothing says so. A `LookupGroups` failure is skipped by a `continue` under a +comment claiming it is recorded in the incompatibility report — it is not. The +run then emits `result: "success"` and the CLI reports a clean export. Group +order comes from map iteration, so two exports of an unchanged directory differ. + +A second defect falls out of reading the adapter: `LDAPAdapter.LookupGroups` +never populates `Group.Members`, and the exporter builds every membership from +that field. Against a real LLDAP directory the `memberships` block is therefore +always empty, while the fixture-based tests pass because the mock fills it in. + +The unit of work is the evidence, not the enumeration: an export that cannot +prove it saw the whole directory must say which one it is. + +## Enumerate groups independently of membership + +```task +id: KEY-WP-0018-T01 +status: done +priority: medium +``` + +Add an optional `domain.GroupLister` capability (`ListGroups`) and implement it +on `LDAPAdapter` with a direct group-subtree search that reads `member` and +`uniqueMember`, so `Group.Members` is populated from the directory rather than +left empty. Keep it optional rather than widening `UserRepository`: the OIDC +layer has no use for it, and every existing implementation and test double would +otherwise have to grow a method it never calls. + +Added `domain.GroupLister` and `LDAPAdapter.ListGroups`, searching the group +subtree for `groupOfNames`/`groupOfUniqueNames` and reading `member` with +`uniqueMember` as the fallback attribute. Compile-time assertions on the adapter +now state both contracts it satisfies. The adapter tests assert the filter is +*not* a membership filter and that a member-less group survives the mapping, +which is the property the whole task exists for. + +## Report or fail on incomplete reads + +```task +id: KEY-WP-0018-T02 +status: done +priority: medium +``` + +Record on the result which enumeration actually ran — a complete directory +enumeration or the membership-derived fallback — and treat a failed group +enumeration as fatal rather than as a silent gap. In the fallback path, record +each skipped user in the incompatibility report instead of discarding the error, +and derive memberships from the discovered user/group pairs. Emit `partial` +telemetry, and have the CLI say which enumeration produced the snapshot, so a +degraded export is legible without reading the YAML. + +`ExportResult` gained `groupEnumeration` — written unconditionally, so nobody has +to infer completeness from an absent field — and a `Complete()` predicate that +requires both a directory enumeration and an empty report. The fallback path +records its own incompleteness as a report entry before it starts, so the +snapshot says so even when every user lookup succeeds. + +The fallback also had to stop deriving memberships from `Group.Members`: the +LLDAP adapter never populated that field, so against a real directory the +`memberships` block was always empty while the fixture-backed tests passed. It +now derives each membership from the user/group pair actually observed. + +## Define ordering and test the failure modes + +```task +id: KEY-WP-0018-T03 +status: done +priority: medium +``` + +Sort users, groups and memberships on stable keys so an unchanged directory +exports byte-identically. Cover the cases the assessment names and the current +suite does not: an empty group, a group enumeration failure, a per-user lookup +failure in the fallback path, and repeat-run determinism. + +Sorted users and groups by ID and memberships by group then user. Four exporter +tests and two adapter tests cover the named cases; the determinism test runs the +export three times over deliberately unsorted input and compares the full +ordering, rather than asserting on a single sorted field. + +Recorded in `SCOPE.md` and in G05's status what this does and does not establish: +completeness evidence for the user/group/membership surface, not for credential +migration, which stays under G03.