From 5cdd48d86469666147cba7c629aa5f1691d58901 Mon Sep 17 00:00:00 2001 From: Schalli Date: Tue, 28 Jul 2026 15:47:22 +0200 Subject: [PATCH] docs(260728-lih): add plan + verification (passed 6/6) for LDAP selective sync Co-Authored-By: Claude Opus 4.8 (1M context) --- .../260728-lih-PLAN.md | 239 ++++++++++++++++++ .../260728-lih-VERIFICATION.md | 78 ++++++ 2 files changed, 317 insertions(+) create mode 100644 .planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-PLAN.md create mode 100644 .planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-VERIFICATION.md diff --git a/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-PLAN.md b/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-PLAN.md new file mode 100644 index 0000000..0b03e27 --- /dev/null +++ b/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-PLAN.md @@ -0,0 +1,239 @@ +--- +phase: quick-260728-lih +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/api/prisma/schema.prisma + - apps/api/prisma/migrations/_ldap_sync_interval_default_off/migration.sql + - apps/api/src/ldap/ldap.service.ts + - apps/api/src/ldap/ldap.service.spec.ts + - apps/web/src/app/(portal)/admin/ldap/page.tsx + - apps/web/src/messages/de.json + - apps/web/src/messages/en.json +autonomous: true +requirements: [quick-260728-lih] + +must_haves: + truths: + - "New LdapConfig rows default to syncIntervalMin=0 (auto-sync OFF); existing rows keep their stored value untouched" + - "isActive still defaults true — the LDAP login gate (auth.service.ts:55 `if (!config || !config.isActive) return null`) is unchanged" + - "A sync with empty/undefined groupFilterDns is a COMPLETE no-op: no LDAP search, created=0/updated=0/deactivated=0, and — critically — ZERO existing LDAP users deactivated" + - "A sync with a non-empty groupFilterDns still imports/updates only the selected groups/OUs + manually added DNs, and still deactivates LDAP users no longer in that selection (existing selective behavior preserved)" + - "DELIBERATE back-compat break: empty groupFilterDns previously meant 'import every user under baseDn'; it now means 'sync nothing' — both code semantics and admin UI copy (de+en) reflect this" + - "The scheduler is not touched — it already treats syncIntervalMin<=0 as disabled (ldap-sync.scheduler.ts:36), so the new default 0 means new configs never auto-sync" + artifacts: + - "apps/api/prisma/schema.prisma: `syncIntervalMin Int @default(0)`" + - "New migration dir apps/api/prisma/migrations/*_ldap_sync_interval_default_off/migration.sql with an ALTER COLUMN ... SET DEFAULT 0, APPLIED to the local tessera DB" + - "apps/api/src/ldap/ldap.service.ts: collectSearchEntries() returns [] on empty groupFilterDns; syncUsersForTenant() early-returns an empty successful result on empty groupFilterDns BEFORE any search/deactivation" + - "apps/api/src/ldap/ldap.service.spec.ts: updated exclude-list tests (now use a non-empty groupFilterDns) + a NEW no-op test proving empty selection = no search, no deactivation" + - "apps/web/src/app/(portal)/admin/ldap/page.tsx: formData default syncIntervalMin: 0" + - "apps/web/src/messages/de.json + en.json: groupFilter.description + groupFilter.emptyMeansAll reworded to 'no selection = nothing synced'" + key_links: + - "syncUsersForTenant early-return guard MUST run before the deactivation loop (ldap.service.ts ~631-649) — if placed after the search, an empty syncedDns deactivates every LDAP user (the exact bug this task prevents)" + - "schema @default(0) -> new migration SET DEFAULT 0 -> scheduler line 36 (syncIntervalMin<=0 skip): the chain that makes auto-sync off-by-default for new configs" + - "isActive @default(true) stays -> auth.service.ts:55 login gate keeps working for LDAP users" +--- + + +Make LDAP directory sync strictly selective and turn auto-sync OFF by default. +Five coordinated changes: (1) Prisma `LdapConfig.syncIntervalMin` default 60→0 + +migration applied locally + matching frontend form default; (2) `collectSearchEntries()` +returns nothing when no group/OU is selected (no longer scans the whole baseDn subtree); +(3) `syncUsersForTenant()` short-circuits to a clean empty result on an empty selection +so NOBODY is created AND NOBODY is deactivated; (4) de+en admin copy reworded from +"no selection imports all" to "no selection syncs nothing"; (5) tests updated to the new +semantics plus a new no-op proof test. + +Purpose: Prevent the whole directory from being pulled in (and, worse, prevent an empty +selection from mass-deactivating every existing LDAP user via the D-15 deactivation pass), +and stop new tenants from silently auto-syncing on an hourly cron they never opted into. + +Output: schema + migration, two focused edits in ldap.service.ts, updated + extended +ldap.service.spec.ts, one frontend default, two reworded i18n strings. + +## Semantic honesty (READ FIRST) + +- `isActive` MUST keep its `@default(true)` — it gates LDAP LOGIN + (auth.service.ts:55), NOT auto-sync. Only `syncIntervalMin` controls auto-sync. +- Do NOT touch ldap-sync.scheduler.ts — it already skips `syncIntervalMin <= 0` (line 36). +- The migration changes only the column DEFAULT. It MUST NOT rewrite existing rows. +- The early-return in `syncUsersForTenant` is the critical safety change: it must sit + ABOVE the deactivation loop. The `collectSearchEntries` []-return (change 2) is a + defensive belt-and-suspenders — with the early return in place the sync path never + reaches it for an empty selection, but the method's contract is corrected regardless. +- Chosen lastSyncAt policy on empty selection: LEAVE IT UNTOUCHED. A no-op leaves no + trace (the early return happens before any DB write). Documented here per the task brief. +- Scope guard: NO new Prisma fields, NO UI restructure, NO new endpoints. Only the + behavior changes above. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/STATE.md +@CLAUDE.md + +# The sync + selective-filter logic being changed +@apps/api/src/ldap/ldap.service.ts +# Scheduler — DO NOT EDIT, understand only (line 36 already skips interval<=0) +@apps/api/src/ldap/ldap-sync.scheduler.ts +# Why isActive must stay default true (login gate at line ~55) +@apps/api/src/auth/auth.service.ts +# Existing tests to update + extend +@apps/api/src/ldap/ldap.service.spec.ts +# Schema (LdapConfig model ~line 61) +@apps/api/prisma/schema.prisma +# Frontend form default + i18n usage +@apps/web/src/app/(portal)/admin/ldap/page.tsx + + + + + + Task 1: Schema default 60→0 + local migration (applied) + apps/api/prisma/schema.prisma, apps/api/prisma/migrations/<generated>_ldap_sync_interval_default_off/migration.sql + + In apps/api/prisma/schema.prisma, LdapConfig model (~line 70), change + `syncIntervalMin Int @default(60)` to `syncIntervalMin Int @default(0)`. Leave the + `isActive Boolean @default(true)` line exactly as-is — it gates LDAP login, not sync. + + Generate AND apply the migration against the LOCAL tessera Postgres. The db container + has NO host port; Prisma must reach it over the container network (project memory: + "Lokale DB-Migrationen"). The container is `tessera-ctl-db-1` (image postgres:16-alpine), + db name/user/password = tessera/tessera/tessera_dev. Steps the executor runs: + (a) `docker start tessera-ctl-db-1` (it is currently Exited); + (b) resolve its IP into a var, e.g. `DB_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' tessera-ctl-db-1)` (fallback `docker exec tessera-ctl-db-1 hostname -i`); + (c) from apps/api run `DATABASE_URL="postgresql://tessera:tessera_dev@${DB_IP}:5432/tessera" pnpm exec prisma migrate dev --name ldap_sync_interval_default_off`. + Prisma is v6 (`pnpm exec prisma`), and `migrate dev` also regenerates the client. + The emitted migration.sql must be a single `ALTER TABLE "LdapConfig" ALTER COLUMN + "syncIntervalMin" SET DEFAULT 0;` — it must NOT contain any UPDATE/data rewrite of + existing rows. If Prisma proposes anything beyond the SET DEFAULT (e.g. a drift-driven + reset), stop and report drift rather than applying. + + + grep -Eq 'syncIntervalMin[[:space:]]+Int[[:space:]]+@default\(0\)' apps/api/prisma/schema.prisma && test "$(docker exec tessera-ctl-db-1 psql -U tessera -d tessera -tAc "SELECT column_default FROM information_schema.columns WHERE table_name='LdapConfig' AND column_name='syncIntervalMin'" | tr -d '[:space:]')" = "0" + + Schema shows @default(0); a new *_ldap_sync_interval_default_off migration exists and is applied — the live LdapConfig.syncIntervalMin column_default is 0; existing rows unchanged; isActive default still true. + + + + Task 2: Selective-only sync semantics + tests + apps/api/src/ldap/ldap.service.ts, apps/api/src/ldap/ldap.service.spec.ts + + Two edits in ldap.service.ts: + + (A) `collectSearchEntries()` (~line 682): in the branch handling empty/undefined + `config.groupFilterDns`, STOP doing the full `client.search(config.baseDn, ...)` and + instead return an empty array (`return [];`). Update the method's doc comment so the + "Empty groupFilterDns" bullet says an empty selection yields no search and returns [] + (no longer "identical to pre-filter behavior / backward compatible"). Keep the + non-empty OU-base and group-memberOf branches exactly as they are. + + (B) `syncUsersForTenant()` (~line 526): add a guard as the VERY FIRST thing in the + method body, after constructing the empty `result` object and BEFORE `new Client(...)` + and BEFORE the deactivation loop. When `config.groupFilterDns` is missing or empty, + return the empty successful `result` immediately (created=0/updated=0/deactivated=0, + errors=[]). Add a short comment stating this returns before any search or deactivation + so an empty selection can never deactivate existing LDAP users, and that lastSyncAt is + intentionally left untouched. Do NOT write to ldapConfig.lastSyncAt on this path. + + Then update ldap.service.spec.ts to the new semantics: + + (1) In the `describe('LdapService.syncUsersForTenant — per-user exclude list', ...)` + block, change `baseConfig.groupFilterDns` from `[]` to a non-empty selection, e.g. + `['ou=people,dc=example,dc=com']`, so the search actually runs and the existing three + tests ('imports every user when the exclude list is empty', 'skips excluded usernames', + 'deactivates a previously-imported user once they are excluded') still exercise the + import/exclude/deactivation logic and keep their current expectations. (The shared + mockSearch returns the mocked entries for the OU-base search, so those tests pass + unchanged apart from the config field.) + + (2) ADD a new `describe('LdapService.syncUsersForTenant — empty selection no-op', ...)` + with a test that: sets `prisma.user.findMany` to return an existing LDAP user + (e.g. `[{ id: 'u-existing', ldapDn: 'cn=existing' }]`); calls + `service.syncUsersForTenant({ ...baseConfig, groupFilterDns: [] }, 't1')`; and asserts + ALL of: result equals `{ created: 0, updated: 0, deactivated: 0, errors: [] }`; + `mockSearch` was NOT called; `mockBind` was NOT called; `userService.create` was NOT + called; `prisma.user.update` was NOT called (no deactivation of the existing user); + and `prisma.ldapConfig.update` was NOT called (lastSyncAt untouched). This is the proof + that an empty selection creates nobody AND deactivates nobody. + + Do NOT change the importUsersByDn / searchUsers / testConnection / verifyUserCredentials + describe blocks — those paths are unaffected by the groupFilterDns semantics. + + + cd apps/api && pnpm vitest run src/ldap/ldap.service.spec.ts + + ldap.service spec green: exclude-list tests run against a non-empty selection; new no-op test proves empty groupFilterDns performs no search and deactivates zero existing users; created/updated/deactivated all 0 on empty selection. + + + + Task 3: Frontend form default 0 + de/en copy rewording + apps/web/src/app/(portal)/admin/ldap/page.tsx, apps/web/src/messages/de.json, apps/web/src/messages/en.json + + In apps/web/src/app/(portal)/admin/ldap/page.tsx, the `useState` `formData` initializer + (~line 87): change `syncIntervalMin: 60,` to `syncIntervalMin: 0,`. This governs the + default for a brand-new config (create form). Leave the `data.syncIntervalMin ?? 60` + fallback inside `fetchConfig` (~line 146) as-is: it only applies when editing an already + saved config, whose value comes from the non-null DB column, so the fallback never fires + for real data — changing it is out of scope for this task. + + In both apps/web/src/messages/de.json and en.json, under `admin.ldap.groupFilter`, + reword the two strings that currently claim an empty selection imports the whole + directory, so they instead say an empty selection syncs nothing. Write EXACTLY: + - de.json `description`: "Beschraenkt den Sync auf ausgewaehlte AD-Gruppen oder Organisationseinheiten. Ohne Auswahl wird nichts synchronisiert." + - de.json `emptyMeansAll`: "Keine Auswahl - es wird nichts synchronisiert." + - en.json `description`: "Restrict sync to selected AD groups or organizational units. With no selection, nothing is synced." + - en.json `emptyMeansAll`: "No selection - nothing is synced." + Keep the JSON keys (`description`, `emptyMeansAll`) unchanged so key-parity between the + two locales is preserved — only the values change. Do not touch any other key. + + + grep -Eq 'syncIntervalMin: 0,' 'apps/web/src/app/(portal)/admin/ldap/page.tsx' && node -e "JSON.parse(require('fs').readFileSync('apps/web/src/messages/de.json','utf8'));JSON.parse(require('fs').readFileSync('apps/web/src/messages/en.json','utf8'))" && test "$(grep -c 'nichts synchronisiert' apps/web/src/messages/de.json)" = "2" && test "$(grep -c 'nothing is synced' apps/web/src/messages/en.json)" = "2" + + Create-form default is 0; both message files are valid JSON with matching keys; the empty-selection copy in de+en says nothing is synced (2 occurrences each); the old "imports all under base DN" wording is gone. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| admin UI/scheduler → LdapService.syncUsersForTenant | trusted admin config drives which directory entries are pulled and which local users get deactivated | +| Prisma migrate → local tessera DB | schema default change applied to the live LdapConfig table | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-lih-01 | Denial of Service (availability) | syncUsersForTenant deactivation pass (~631-649) | high | mitigate | Early-return guard on empty groupFilterDns runs BEFORE the deactivation loop — an empty selection can no longer mass-deactivate every LDAP user (would otherwise lock all directory users out of login). Proven by the new no-op unit test. | +| T-lih-02 | Tampering | *_ldap_sync_interval_default_off migration | medium | mitigate | Migration is SET DEFAULT only — no UPDATE of existing rows; executor halts and reports if Prisma proposes any data rewrite/drift. Verified via live column_default = 0 and existing rows untouched. | +| T-lih-03 | Elevation of Privilege | isActive login gate (auth.service.ts:55) | medium | accept | isActive @default(true) is deliberately left unchanged; only syncIntervalMin default changes. No auth path is modified. | + + + +- API: `cd apps/api && pnpm vitest run src/ldap/ldap.service.spec.ts` green (updated exclude tests + new no-op test). +- Migration applied: live `information_schema.columns` shows LdapConfig.syncIntervalMin column_default = 0; schema shows @default(0). +- Web: page.tsx create-form default is 0; de.json + en.json parse and state "no selection = nothing synced" in both locales. +- Scheduler (ldap-sync.scheduler.ts) and auth.service.ts left untouched. +- No new Prisma fields, no new endpoints, no UI restructure. + + + +- New LdapConfig rows default syncIntervalMin=0 (auto-sync off); isActive stays true. +- Empty groupFilterDns = full no-op: no search, no create, no deactivation (unit-proven). +- Non-empty groupFilterDns still selectively imports + deactivates as before. +- de+en admin copy reworded to "no selection syncs nothing". +- Three atomic commits (Task 1 schema+migration, Task 2 service+tests, Task 3 frontend+i18n). No push, no docker deploy beyond starting the local db for the migration. + + + +Create `.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-SUMMARY.md` when done. + \ No newline at end of file diff --git a/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-VERIFICATION.md b/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-VERIFICATION.md new file mode 100644 index 0000000..104aed5 --- /dev/null +++ b/.planning/quick/260728-lih-ldap-sync-selektiv-und-auto-sync-default/260728-lih-VERIFICATION.md @@ -0,0 +1,78 @@ +--- +phase: quick-260728-lih +verified: 2026-07-28T15:46:00Z +status: passed +score: 6/6 must-haves verified +behavior_unverified: 0 +overrides_applied: 0 +--- + +# Quick Task 260728-lih: LDAP Sync Selektiv + Auto-Sync Default — Verification Report + +**Task Goal:** LDAP-Sync selektiv & Auto-Sync-off-by-default — (1) syncIntervalMin Prisma-Default 60→0 + Migration + Frontend-Default (isActive bleibt default true), (2) collectSearchEntries leere groupFilterDns ⇒ leeres Ergebnis, (3) KRITISCH syncUsersForTenant Early-Return bei leerer Auswahl = KEINE Suche + KEINE Deaktivierung, (4) i18n de+en umformuliert, (5) Tests angepasst + No-Op-Test. +**Verified:** 2026-07-28T15:46:00Z +**Status:** passed +**Re-verification:** No — initial verification + +## Goal Achievement + +### Observable Truths + +| # | Truth | Status | Evidence | +|---|-------|--------|----------| +| 1 | New LdapConfig rows default to `syncIntervalMin=0`; `isActive` still defaults `true` | ✓ VERIFIED | `apps/api/prisma/schema.prisma:70-71` — `syncIntervalMin Int @default(0)` / `isActive Boolean @default(true)` (read directly from file) | +| 2 | Migration only changes the column DEFAULT, no data rewrite; applied to local DB | ✓ VERIFIED | `apps/api/prisma/migrations/20260728134010_ldap_sync_interval_default_off/migration.sql` contains exactly `ALTER TABLE "LdapConfig" ALTER COLUMN "syncIntervalMin" SET DEFAULT 0;` (2 lines, no UPDATE). Live check: `docker exec tessera-ctl-db-1 psql ... SELECT column_default ...` returned `0`. | +| 3 | `collectSearchEntries()` returns `[]` on empty/undefined `groupFilterDns`, no search | ✓ VERIFIED | `ldap.service.ts:702-704`: `if (!config.groupFilterDns \|\| config.groupFilterDns.length === 0) { return []; }` — precedes any `client.search()` call in the method | +| 4 | `syncUsersForTenant()` early-returns BEFORE the deactivation loop (and before any search/bind) on empty selection — the critical safety guard | ✓ VERIFIED | `ldap.service.ts:544-546`: guard `if (!config.groupFilterDns \|\| config.groupFilterDns.length === 0) { return result; }` sits immediately after `result` construction (line 530), before `new Client(...)` (line 548) and far before the deactivation loop (line 642) | +| 5 | Scheduler (`ldap-sync.scheduler.ts`) and `auth.service.ts` are unchanged | ✓ VERIFIED | `git diff c54e424^ HEAD --stat -- apps/api/src/ldap/ldap-sync.scheduler.ts apps/api/src/auth/auth.service.ts` produced empty output. Scheduler line ~36 still `if (config.syncIntervalMin <= 0) continue;`. `auth.service.ts` line ~55 still `if (!config \|\| !config.isActive) return null;` | +| 6 | A no-op test exists proving empty selection = no search, no deactivation; i18n de+en reworded | ✓ VERIFIED | `ldap.service.spec.ts:121-168` new describe block asserts `result === {created:0,updated:0,deactivated:0,errors:[]}`, `mockSearch`/`mockBind`/`userService.create`/`prisma.user.update`/`prisma.ldapConfig.update` all not called, with an existing LDAP user pre-loaded in the mock. `de.json`/`en.json` `groupFilter.description`+`emptyMeansAll` read back exactly as specified in the plan. | + +**Score:** 6/6 truths verified (0 present-but-behavior-unverified) + +### Required Artifacts + +| Artifact | Expected | Status | Details | +|----------|----------|--------|---------| +| `apps/api/prisma/schema.prisma` | `syncIntervalMin Int @default(0)`, `isActive` unchanged | ✓ VERIFIED | Confirmed lines 70-71 | +| `apps/api/prisma/migrations/20260728134010_ldap_sync_interval_default_off/migration.sql` | SET DEFAULT only, applied | ✓ VERIFIED | File exists, content matches exactly; live `column_default` = 0 | +| `apps/api/src/ldap/ldap.service.ts` | `collectSearchEntries` returns `[]`; `syncUsersForTenant` early-return before deactivation | ✓ VERIFIED | Both edits present and correctly ordered | +| `apps/api/src/ldap/ldap.service.spec.ts` | Exclude-list tests use non-empty selection + new no-op test | ✓ VERIFIED | `baseConfig.groupFilterDns = ['ou=people,dc=example,dc=com']` (line 40); new no-op describe block (lines 121-168) | +| `apps/web/src/app/(portal)/admin/ldap/page.tsx` | `formData` default `syncIntervalMin: 0` | ✓ VERIFIED | Line 87: `syncIntervalMin: 0,` | +| `apps/web/src/messages/de.json` + `en.json` | groupFilter copy reworded | ✓ VERIFIED | Both files' `admin.ldap.groupFilter.description`/`emptyMeansAll` read back verbatim as specified in plan | + +### Key Link Verification + +| From | To | Via | Status | Details | +|------|-----|-----|--------|---------| +| `syncUsersForTenant` early-return guard | deactivation loop (~line 642) | guard placement | ✓ WIRED | Guard at line 544 returns before line 548 (`new Client`), long before line 642 (deactivation query) — physically impossible to reach deactivation on empty selection | +| `schema.prisma @default(0)` | live DB column default | Prisma migrate | ✓ WIRED | Live psql query confirms `column_default = 0` | +| `isActive @default(true)` | `auth.service.ts:55` login gate | unchanged code path | ✓ WIRED | Both the default and the gate code are unchanged and still consistent | + +### Behavioral Spot-Checks + +| Behavior | Command | Result | Status | +|----------|---------|--------|--------| +| ldap.service test suite green (updated exclude tests + new no-op test) | `cd apps/api && pnpm vitest run src/ldap/ldap.service.spec.ts` | 16/16 passed | ✓ PASS | +| API typecheck clean | `cd apps/api && pnpm exec tsc --noEmit` | no errors | ✓ PASS | +| Live migration applied | `docker exec tessera-ctl-db-1 psql ... column_default` | `0` | ✓ PASS | + +### Anti-Patterns Found + +None. Scanned all 7 modified/created files for `TBD|FIXME|XXX|TODO|HACK|PLACEHOLDER` — zero matches. + +### Requirements Coverage + +Quick task — no formal REQUIREMENTS.md entries beyond the task's own `must_haves`, all of which are verified above. + +### Human Verification Required + +None. All must-haves are code-level, statically verifiable, and were confirmed by direct file reads, a live DB query, and a passing automated test run — no UI/visual/runtime judgment call remains. + +### Gaps Summary + +No gaps. All 6 derived truths, all 6 required artifacts, and all 3 key links verified directly against the live codebase and running local DB (not just SUMMARY.md claims). The critical safety property — early-return before the deactivation loop — was confirmed by reading the exact line ordering in `ldap.service.ts`, and further confirmed behaviorally by running the new no-op unit test (which asserts zero deactivation calls). The three commits referenced in SUMMARY.md (`c54e424`, `57bc7f9`, `63a07ab`) all exist and touch exactly the files claimed. + +--- + +_Verified: 2026-07-28T15:46:00Z_ +_Verifier: Claude (gsd-verifier)_