From f0c763d3e2e9ed07dcd0f9b3666293a62210b5bd Mon Sep 17 00:00:00 2001 From: Schalli Date: Tue, 11 Aug 2026 11:06:27 +0200 Subject: [PATCH] docs(16): record the objectGUID sweep defect found by UAT test 2 Quick task 260811-f9i: plan and summary of the fix, STATE.md row, and the UAT test 2 result. Test 2 was the read-only A2 check against the real directory -- it turned assumption A2 from "unverified" into "false as implemented" and surfaced a defect that would have deleted every AD-bound group on the first real sync. Also records why the defect survived review: the spec mocks built their expected filter with the same escape helper the production code used, so the test asserted self-consistency rather than directory behaviour. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/STATE.md | 1 + .../16-ad-gruppen-synchronisation/16-UAT.md | 29 ++++++- .../PLAN.md | 78 +++++++++++++++++++ .../SUMMARY.md | 76 ++++++++++++++++++ 4 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 .planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/PLAN.md create mode 100644 .planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/SUMMARY.md diff --git a/.planning/STATE.md b/.planning/STATE.md index aa4bbab..45a5bec 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -304,6 +304,7 @@ None yet. | 260728-lih | LDAP: Sync strikt selektiv (leere Auswahl = No-Op statt Voll-Import) + Auto-Sync-Default aus (syncIntervalMin 60→0) | 2026-07-28 | c54e424,57bc7f9,63a07ab | [260728-lih-ldap-sync-selektiv-und-auto-sync-default](.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/) | | 260729-d3k | LDAP: Multi-Base-DN und Base-DN als Sync-Scope (statt leerer Gruppenfilter = No-Op) | 2026-07-29 | 5cbd530,96be7e1 | [260729-d3k-ldap-multi-base-dn-und-base-dn-als-scope](.planning/quick/260729-d3k-ldap-multi-base-dn-und-base-dn-als-scope/) | | 260805-d0r | Benutzer-Detail zeigte Gruppenmitgliedschaften aus Freigaben statt aus Mitgliedschaften — GET /module-grants/users/:userId liefert jetzt { groups, modules }, Chips mit Herkunfts-Badge; im Browser gegengeprüft: Mitgliedschaft bleibt sichtbar, auch wenn die Gruppe kein Modul freigibt | 2026-08-05 | ecadf69,f8ff74b,8ce3748 | [260805-d0r-benutzer-detail-zeigt-gruppenmitgliedsch](.planning/quick/260805-d0r-benutzer-detail-zeigt-gruppenmitgliedsch/) | +| 260811-f9i | KRITISCH: objectGUID-Existenzpruefung fand nie etwas — der Filter wurde als `\xx`-escapter String gebaut, ldapts wandelt das nicht in Rohbytes; beide Suchen (Base-DNs und WR-03-Fallback) teilten ihn, also haette der erste echte Sync JEDE AD-gebundene Gruppe samt Mitgliedschaften und Modulfreigaben geloescht. Jetzt EqualityFilter ueber den rohen Buffer, `escapeLdapFilterBuffer()` entfernt. Read-only am echten AD gemessen (escapter String 0 Treffer, EqualityFilter 1 korrekter Treffer); Regressionstests gegengeprueft (alter Code = 8 rote Tests) | 2026-08-11 | d2019dc | [260811-f9i-fix-objectguid-existence-sweep-to-use-eq](.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/) | | 260805-fok | Standardgruppe bei Mandanten-Anlage + Startup-Reparatur — GroupsService.ensureDefaultGroup(tenantId) mit D-13-Waechter (null Gruppen, nicht fehlende Markierung), verdrahtet in TenantService.create und AdminSeedService.ensureDefaultGroupsForAllTenants; schliesst die Migrations-Backfill-Luecke auf frischen Installationen (Testserver: tenants=1 users=4 groups=0) | 2026-08-05 | 9d1254c,0d7d8a5 | [260805-fok-standardgruppe-bei-mandanten-anlage-und-](.planning/quick/260805-fok-standardgruppe-bei-mandanten-anlage-und-/) | ## Deferred Items diff --git a/.planning/phases/16-ad-gruppen-synchronisation/16-UAT.md b/.planning/phases/16-ad-gruppen-synchronisation/16-UAT.md index f000573..cc5ab39 100644 --- a/.planning/phases/16-ad-gruppen-synchronisation/16-UAT.md +++ b/.planning/phases/16-ad-gruppen-synchronisation/16-UAT.md @@ -27,7 +27,25 @@ result: [pending] ### 2. Binaerer Existenz-Filter liefert korrekte Treffer (Annahme A2) expected: Gegen dasselbe AD, read-only — eine Suche mit dem byteweise escapten (objectGUID=...)-Binaerfilter absetzen. Eine weiterhin existierende Gruppe muss einen Treffer liefern, eine tatsaechlich geloeschte keinen. Ein negatives Ergebnis bei Test 1 ODER 2 ist ein Stopp-Grund fuer die Loeschsemantik aus D-05. -result: [pending] +result: failed -> fixed +tested: 2026-08-11 read-only gegen balios.ctl.local, Sonde CN=Domain Admins,CN=Users,DC=ctl,DC=local +notes: | + A2 war FALSCH — nicht am Domain Controller, sondern an der Umsetzung. Der als + String interpolierte Filter (objectGUID=\1e\4b...) lieferte 0 Treffer fuer ein + Objekt, dessen GUID unmittelbar zuvor aus demselben Verzeichnis gelesen wurde. + Die Grossbuchstaben-Variante ebenfalls 0. Ein ldapts EqualityFilter mit dem + rohen 16-Byte-Buffer lieferte genau einen Treffer mit korrekter DN; die + Kontrollsuche (cn=Domain Admins) lieferte ebenfalls einen Treffer. + + Auswirkung im damaligen Stand: beide Suchen der Existenzpruefung — die schmale + ueber die Base-DNs und die weite WR-03-Absicherung — teilten sich diesen Filter. + Der erste echte Sync-Lauf haette daher JEDE AD-gebundene Gruppe als geloescht + eingestuft und samt GroupMembership und ModuleGrant entfernt. + + Behoben in Quick-Task 260811-f9i (Commit d2019dc): EqualityFilter ueber den + rohen Buffer, escapeLdapFilterBuffer() entfernt, zwei Regressionstests, die den + String-Filter ausschliessen. Der negative Fall (unbekannte GUID liefert nichts) + war in beiden Varianten erfuellt. ### 3. Umbenennung im AD zieht in Tessera nach (SC-3, end-to-end) expected: Gruppe im echten AD umbenennen, Sync laufen lassen. Group.name und Group.ldapDn ziehen nach, der Zaehler groupsRenamed steigt, es findet KEINE Loesch-und-Neuanlage statt, Mitgliedschaften und Modulfreigaben bleiben erhalten, ein gesetzter interner Name bleibt unveraendert. @@ -95,10 +113,15 @@ result: [pending] total: 8 passed: 2 partial: 1 -issues: 0 -pending: 5 +issues: 1 +pending: 4 skipped: 0 blocked: 0 +issues_detail: | + Test 2 hat einen kritischen Fehler aufgedeckt (Existenzpruefung fand nie etwas, + Loeschung aller gebundenen Gruppen beim ersten Sync). Behoben in Quick-Task + 260811-f9i, Commit d2019dc. Der Test selbst gilt damit als bestanden fuer die + Filtermechanik; der Sync-Lauf als Ganzes bleibt ueber Test 3/4 offen. ## Gaps diff --git a/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/PLAN.md b/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/PLAN.md new file mode 100644 index 0000000..e010de9 --- /dev/null +++ b/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/PLAN.md @@ -0,0 +1,78 @@ +--- +quick_id: 260811-f9i +slug: fix-objectguid-existence-sweep-to-use-eq +date: 2026-08-11 +status: planned +relates_to: 16-ad-gruppen-synchronisation +severity: critical +--- + +# Quick Task: objectGUID-Existenzprüfung auf EqualityFilter mit Rohbytes umstellen + +## Problem + +`syncBoundGroupsForTenant()` (`apps/api/src/ldap/ldap.service.ts:1318`) baut den +Filter der Existenzprüfung als **String**: + +```ts +const filter = `(objectGUID=${LdapService.escapeLdapFilterBuffer(guidBuffer)})`; +``` + +`escapeLdapFilterBuffer()` erzeugt `\1e\4b\4d…`. ldapts wandelt diese Sequenzen +beim Parsen des Filter-Strings nicht in Rohbytes zurück, also steht auf dem Draht +ein anderer Vergleichswert als gemeint. Die Suche trifft nie. + +Am 2026-08-11 read-only gegen das echte AD (`balios.ctl.local`) gemessen, Sonde +`CN=Domain Admins,CN=Users,DC=ctl,DC=local`, GUID `1e4b4df0b3caeb4c9b49fddf05227e10`: + +| Variante | Treffer | +|---|---| +| `(objectGUID=\1e\4b…)` — heutiger Code | 0 | +| `(objectGUID=\1E\4B…)` — Großbuchstaben | 0 | +| `new EqualityFilter({ attribute: 'objectGUID', value: })` | 1, korrekte DN | +| `EqualityFilter` mit escaptem String als Wert | 0 | +| Kontrolle `(cn=Domain Admins)` | 1 | + +Der Domain Controller ist also in Ordnung; die Annahme A2 aus `16-RESEARCH.md` +ist für die gewählte Umsetzung falsch. + +## Auswirkung + +Der Ablauf in `syncBoundGroupsForTenant()` ist: schmale Suche über die +konfigurierten Base-DNs → kein Treffer → weite Suche über `domainRoots` +(WR-03-Absicherung) → kein Treffer → Gruppe gilt als im AD gelöscht +(`ldap.service.ts:1432`). **Beide** Suchen verwenden denselben kaputten Filter. + +Folge: der erste echte Sync-Lauf hätte jede AD-gebundene Gruppe gelöscht, samt +`GroupMembership` und `ModuleGrant` per Kaskade, und vorher die +Standardgruppen-Markierung weitergereicht (D-06). Der Bericht hätte das als +reguläre Löschung ausgewiesen. Auf alpha ist nichts passiert — dort lief nie ein +Sync. + +## Tasks + +1. `EqualityFilter` aus `ldapts` importieren; in `syncBoundGroupsForTenant()` den + String-Filter durch ein `EqualityFilter`-Objekt mit dem rohen 16-Byte-Buffer + ersetzen. Beide Schleifen (Base-DNs und `domainRoots`) teilen sich weiterhin + denselben Filterwert. +2. `escapeLdapFilterBuffer()` entfernen — nach Task 1 ohne Aufrufer, und die + Methode ist genau die Falle, in die der Code gelaufen ist. `escapeLdapFilterValue()` + (String-Werte, RFC 4515) bleibt unberührt, die ist korrekt und wird weiter genutzt. +3. Die Mocks in `ldap.service.spec.ts`, die auf dem escapten String matchen + (Zeilen 579/586, 1060, 1587), auf den Buffer-Wert des Filterobjekts umstellen. +4. Regressionstests ergänzen: die Existenzprüfung übergibt ein Filterobjekt mit + `attribute: 'objectGUID'` und einem `Buffer`-Wert, der byteweise dem + gespeicherten `ldapObjectGuid` entspricht — und **kein** String. Ein Test hält + ausdrücklich fest, dass ein String-Filter der Form `(objectGUID=…)` nicht mehr + vorkommt. + +## Erfolgskriterium + +`pnpm --filter @tessera/api test` läuft grün, und ein Test schlägt fehl, sobald +jemand wieder einen escapten String als GUID-Filter übergibt. + +## Offen nach diesem Fix + +Der Fix belegt die Filtermechanik, nicht das Verhalten des Sync-Laufs. UAT 1, 3 +und 4 (Umbenennen und Löschen im AD) bleiben offen und sollen gegen einen +Wegwerf-Domänencontroller nachgezogen werden. diff --git a/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/SUMMARY.md b/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/SUMMARY.md new file mode 100644 index 0000000..8e19ee0 --- /dev/null +++ b/.planning/quick/260811-f9i-fix-objectguid-existence-sweep-to-use-eq/SUMMARY.md @@ -0,0 +1,76 @@ +--- +quick_id: 260811-f9i +slug: fix-objectguid-existence-sweep-to-use-eq +date: 2026-08-11 +status: complete +relates_to: 16-ad-gruppen-synchronisation +severity: critical +commits: + - d2019dc fix(ldap): search objectGUID by raw bytes, not an escaped filter string +--- + +# Summary: objectGUID-Existenzprüfung auf Rohbytes umgestellt + +## Was gemacht wurde + +`syncBoundGroupsForTenant()` baut den Filter der Existenzprüfung jetzt als +`EqualityFilter` über den rohen 16-Byte-Buffer statt als interpolierten +`(objectGUID=\xx…)`-String. Beide Suchen — die schmale über die konfigurierten +Base-DNs und die weite über die `domainRoots` (WR-03-Absicherung) — teilen sich +denselben Filterwert, damit die Absicherung nicht erneut einen kaputten Filter +erben kann. + +`escapeLdapFilterBuffer()` ist entfernt. An seiner Stelle steht ein Kommentar, +der erklärt, warum die Methode nicht zurückkommen darf. +`escapeLdapFilterValue()` (String-Werte nach RFC 4515) bleibt unverändert und +weiter in Gebrauch. + +## Wie der Fehler gefunden wurde + +Über UAT-Test 2 aus Phase 16 — read-only gegen das echte AD (`balios.ctl.local`), +Sonde `CN=Domain Admins,CN=Users,DC=ctl,DC=local`, GUID +`1e4b4df0b3caeb4c9b49fddf05227e10`: + +| Variante | Treffer | +|---|---| +| `(objectGUID=\1e\4b…)` — bisheriger Code | 0 | +| `(objectGUID=\1E\4B…)` — Großbuchstaben | 0 | +| `EqualityFilter` mit rohem Buffer | 1, korrekte DN | +| `EqualityFilter` mit escaptem String als Wert | 0 | +| Kontrolle `(cn=Domain Admins)` | 1 | + +Der Domain Controller war also nie das Problem. Annahme A2 aus `16-RESEARCH.md` +galt für die gewählte Umsetzung nicht. + +## Warum es durch Review und Tests gekommen ist + +Die Mocks in `ldap.service.spec.ts` haben den erwarteten Filter mit **derselben** +`escapeLdapFilterBuffer()`-Funktion gebaut, die der Produktionscode benutzt hat. +Damit war der Test tautologisch: er hat bestätigt, dass die Funktion sich selbst +gegenüber konsistent ist, nicht dass ein Verzeichnis den Filter versteht. Der +Code-Kommentar hat die Annahme sogar ausdrücklich als `[ASSUMED — nicht gegen ein +echtes AD verifiziert]` markiert. + +Die neuen Tests prüfen jetzt die Form des Aufrufs statt seines Inhalts: der +Filter muss ein Objekt mit `attribute: 'objectGUID'` und einem `Buffer`-Wert +sein, und kein Suchaufruf des Laufs darf einen String-Filter mit `objectGUID=` +tragen. + +## Verifikation + +- `pnpm --filter @tessera/api test` — 568 Tests grün (40 Dateien) +- `npx tsc --noEmit` — sauber +- Gegenprobe: der alte String-Filter kurzzeitig wiederhergestellt → 8 Tests rot, + darunter beide neuen. Die Regressionstests haben also Zähne. + +## Was offen bleibt + +Der Fix belegt die Filtermechanik gegen ein echtes AD, nicht das Verhalten eines +vollständigen Sync-Laufs. Offen bleiben aus `16-UAT.md`: + +- Test 1 (A1: GUID überlebt eine Umbenennung) — braucht Schreibrechte im AD +- Test 3 (Umbenennung end-to-end) und Test 4 (Löschung end-to-end) +- Der Teil von Test 7, der die Zahlenzeilen eines erfolgreichen Sync-Laufs prüft + +Nächster Schritt laut Absprache: ein Wegwerf-Domänencontroller im Container, gegen +den Umbenennen und Löschen vollständig durchgespielt werden können.