fix(16): WR-03 do not delete a bound group merely unobserved under a narrower base DN

syncBoundGroupsForTenant()'s existence sweep only searched the configured
base DNs, so an AD group MOVED to an OU outside that subtree (still
present in the directory) was indistinguishable from a genuine
disappearance and got deleted along with its memberships/module grants —
a silent access loss from a non-destructive AD operation, and a bigger
blast radius than D-05 ("group genuinely gone") was accepted for.

Before concluding disappearance, a second (objectGUID=...) sweep now runs
against each base DN's own domain root (skipped when a base DN already IS
its domain root — the common case, nothing wider to search). A hit there
is reported as an error line and the group is left untouched; only when
the wide sweep also finds nothing is deletion (SC-4/D-05/D-06) actually
established — mirroring the existing conservative stance already taken
for a legacy binding whose DN no longer resolves. Deleting on uncertainty
was the failure mode; this closes it without widening it.
This commit is contained in:
2026-08-06 17:00:54 +02:00
parent 2779d42e6c
commit 00de6a6f9c
2 changed files with 227 additions and 2 deletions
+114 -2
View File
@@ -1188,7 +1188,25 @@ export class LdapService {
* resulting P2002 (the new name collides with an existing local group)
* is caught, reported as an error line, and the group is left
* unchanged — the run continues with the remaining candidates.
* 4. No hit is a disappearance (SC-4/D-05): the default-marker handoff
* 4. No hit under any configured base DN is NOT automatically treated as a
* disappearance (WR-03, 16-REVIEW.md). The base-DN sweep only proves
* "not found under these specific base DNs" — an AD group that was
* MOVED to an OU outside the configured subtree (still present in the
* directory) would otherwise be misread as gone and deleted along with
* its memberships/module grants, which is a bigger blast radius than
* D-05 ("group genuinely disappeared") was accepted for. Before
* concluding disappearance, a second, wider (objectGUID=...) sweep runs
* against each base DN's own domain root (the trailing DC=... RDN
* chain, e.g. "ou=Sales,dc=example,dc=com" -> "dc=example,dc=com"),
* skipping any root already covered by a configured base DN (the
* common case where baseDn already IS the domain root — nothing wider
* to search). A hit there means the group still exists, just outside
* the configured subtree: reported as an error line, NOT deleted — the
* same conservative "absence must be established, never merely
* unobserved" stance already taken for a legacy binding whose DN no
* longer resolves (case 1 above). Only when the wide sweep ALSO finds
* nothing (or there is no wider root left to search) is disappearance
* (SC-4/D-05) actually established: the default-marker handoff
* (reassignDefaultBeforeDelete) runs BEFORE the delete — this ordering
* is the actual correctness guarantee of D-06, not a style choice. A
* resulting P2025 (already gone, e.g. a concurrent manual delete) is
@@ -1236,6 +1254,21 @@ export class LdapService {
const baseDns = this.parseBaseDns(config.baseDn);
let anyDeleted = false;
// WR-03 (16-REVIEW.md): domain root(s) NOT already covered by a
// configured base DN — the fallback anchor for the wide existence sweep
// below, so a group moved outside every configured base DN is not
// misread as deleted. Computed once per run (identical for every
// candidate), not per-group.
const domainRoots = Array.from(
new Set(
baseDns
.map((dn) => LdapService.domainRootOf(dn))
.filter(
(dn): dn is string => dn !== null && !baseDns.includes(dn),
),
),
);
for (const group of candidates) {
try {
let ldapObjectGuid = group.ldapObjectGuid;
@@ -1358,7 +1391,35 @@ export class LdapService {
}
}
} else {
// 4. Disappearance (SC-4/D-05/D-06) — handoff BEFORE delete.
// WR-03 (16-REVIEW.md): before concluding disappearance, rule out
// a directory-internal move by widening the SAME (objectGUID=...)
// filter to each base DN's own domain root — outside the
// configured base-DN restriction. A hit means the group still
// exists; report and move on WITHOUT deleting or writing anything
// (deliberately no auto-rename to the found location either — a
// move outside the configured scope is an admin configuration
// signal, not something this sync silently absorbs).
let wideHit: Entry | null = null;
for (const root of domainRoots) {
const { searchEntries } = await client.search(root, {
filter,
attributes: ['cn', 'dn'],
scope: 'sub',
});
if (searchEntries.length > 0) {
wideHit = searchEntries[0];
break;
}
}
if (wideHit) {
result.errors.push(
`Gruppe ${group.name}: nicht mehr unter den konfigurierten Base-DNs gefunden, existiert aber weiterhin unter '${wideHit.dn}' — vermutlich im Verzeichnis verschoben, Base-DN-Konfiguration pruefen. Nicht geloescht.`,
);
continue;
}
// 4. Disappearance (SC-4/D-05/D-06) established — handoff BEFORE
// delete.
const movedDefault =
await this.groupsService.reassignDefaultBeforeDelete(
tenantId,
@@ -1456,4 +1517,55 @@ export class LdapService {
.map((b) => '\\' + b.toString(16).padStart(2, '0'))
.join('');
}
/**
* Split a DN into its individual RDN components, respecting a
* backslash-escaped comma inside an RDN value (RFC 4514) so a value like
* "cn=Sales\, Inc.,dc=example,dc=com" is not split in the wrong place.
* Used only by domainRootOf() below — never for LDAP filter construction
* (T-16-01 stays in force there; this is pure DN string bookkeeping).
*/
private static splitDnComponents(dn: string): string[] {
const parts: string[] = [];
let current = '';
let escaped = false;
for (const ch of dn) {
if (escaped) {
current += ch;
escaped = false;
continue;
}
if (ch === '\\') {
current += ch;
escaped = true;
continue;
}
if (ch === ',') {
parts.push(current);
current = '';
continue;
}
current += ch;
}
parts.push(current);
return parts.map((p) => p.trim()).filter((p) => p.length > 0);
}
/**
* Derive the domain root DN — the trailing DC=... component chain — from a
* configured base DN, e.g. "ou=Sales,dc=example,dc=com" ->
* "dc=example,dc=com". WR-03 (16-REVIEW.md) fallback search anchor for
* syncBoundGroupsForTenant()'s existence sweep: widening a "not found"
* result to the domain root before concluding a bound group has actually
* disappeared, rather than merely moved outside a narrower configured
* base DN. Returns null when the DN carries no DC= component at all
* (nothing to widen the search to — e.g. a non-AD directory using a
* different root naming scheme).
*/
private static domainRootOf(dn: string): string | null {
const dcParts = LdapService.splitDnComponents(dn).filter((rdn) =>
/^dc=/i.test(rdn),
);
return dcParts.length > 0 ? dcParts.join(',') : null;
}
}