feat(16-03): wire group reconciliation before membership sync (5a)
- syncUsersForTenant() now calls syncBoundGroupsForTenant() (5a) BEFORE syncGroupMembershipsForTenant() (5b) — the central correctness ordering of Phase 16 (RESEARCH.md Pitfall 1): a rename detected in the same run must be written back before the memberOf filter is built, or the membership sync would misreport a rename as a membership wipeout - Add observable ordering test (call-order spies), a no-op-guard test, and a regression test proving a memberOf search never uses the stale pre-rename DN - Update the D-21 membership-sync test fixtures to resolve step 5a as a deterministic no-op (DN-derived identity GUID), since the wiring now runs 5a ahead of every syncUsersForTenant() call those tests exercise A1 (objectGUID survives an AD rename) and A2 (binary filter escape syntax) remain unverified against a real directory — no reachable AD in this sandbox. Documented as an outstanding live verification in the plan SUMMARY, not silently skipped.
This commit is contained in:
@@ -906,9 +906,24 @@ export class LdapService {
|
||||
}
|
||||
}
|
||||
|
||||
// 5a. Plan 16-03 (SC-3/SC-4/SC-5, D-05/D-06/D-07): reconcile every
|
||||
// AD-bound Group's name/ldapDn/existence BEFORE the membership
|
||||
// reconciliation below. This ordering is the central correctness
|
||||
// condition of Phase 16, not a style choice (RESEARCH.md Pitfall 1):
|
||||
// step 5b reads Group.ldapDn from the DB to build its memberOf
|
||||
// filter. If a rename detected in THIS SAME run hadn't already been
|
||||
// written back by the time 5b runs, the memberOf filter would still
|
||||
// use the stale pre-rename DN, AD would return zero hits for it, and
|
||||
// every LDAP membership of the renamed group would be misreported as
|
||||
// removed — a rename would look like a membership wipeout that never
|
||||
// happened in the directory.
|
||||
await this.syncBoundGroupsForTenant(client, config, tenantId, result);
|
||||
|
||||
// 5b. D-21: reconcile GroupMembership rows for every AD-bound Group in
|
||||
// this same run — no separate sync job, no second button. Runs behind
|
||||
// the Base-DN no-op guard above, exactly like the rest of this method.
|
||||
// Relies on 5a above having already written back any rename in this
|
||||
// run (see 5a's comment).
|
||||
await this.syncGroupMembershipsForTenant(
|
||||
client,
|
||||
config,
|
||||
|
||||
Reference in New Issue
Block a user