155 lines
14 KiB
Markdown
155 lines
14 KiB
Markdown
---
|
|
phase: quick-260729-d3k
|
|
plan: 01
|
|
type: execute
|
|
wave: 1
|
|
depends_on: []
|
|
files_modified:
|
|
- 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:
|
|
- 260729-d3k
|
|
must_haves:
|
|
truths:
|
|
- "The Base-DN admin field accepts multiple DNs (one per line) and persists them as a single `\\n`-separated String — no Prisma schema change, no migration."
|
|
- "LDAP sync searches EVERY configured base DN and merges/dedupes results by entry `dn` across all three search paths (collectSearchEntries, listGroups, searchUsers)."
|
|
- "An empty groupFilterDns no longer blocks sync: with >=1 base DN, all users under the base DN(s) are synced with `sanitizedFilter` (no memberOf restriction)."
|
|
- "groupFilterDns is now an OPTIONAL extra restriction: `ou=` entries are additional search bases (plain filter); non-`ou=` (group) DNs become a memberOf constraint applied to the base-DN search."
|
|
- "CRITICAL SAFETY: the syncUsersForTenant early-return No-Op keys ONLY on the parsed base-DN list being EMPTY. An empty base-DN list is the sole condition that skips search + the deactivation loop; an empty groupFilterDns never triggers the No-Op and therefore never mass-deactivates users."
|
|
- "i18n (de + en) describes the Base-DN(s) as the sync scope and the group filter as an optional additional restriction; the old '...nichts synchronisiert' / '...nothing is synced' wording is gone."
|
|
artifacts:
|
|
- "apps/api/src/ldap/ldap.service.ts — parseBaseDns() helper + multi-base collectSearchEntries/listGroups/searchUsers + base-DN-list-keyed deactivation guard."
|
|
- "apps/api/src/ldap/ldap.service.spec.ts — empty-base-DN No-Op test (replaces the empty-groupFilterDns No-Op), multi-base merge/dedup test, adjusted exclude-list/groupFilter specs."
|
|
- "apps/web/src/app/(portal)/admin/ldap/page.tsx — multi-line <textarea> for baseDn + one-DN-per-line hint."
|
|
- "apps/web/src/messages/de.json + en.json — reworded groupFilter.description/emptyMeansAll + new baseDnHint key."
|
|
key_links:
|
|
- "syncUsersForTenant early-return guard <-> parseBaseDns(config.baseDn).length === 0 (mass-deactivation safety net for an unconfigured config)."
|
|
- "collectSearchEntries base-search filter <-> presence of group DNs (memberOf constraint when present, plain sanitizedFilter when absent)."
|
|
- "ldap.controller.ts still passes config.baseDn (unchanged String) into listGroups/searchUsers — the service splits it internally, so the controller needs no change."
|
|
---
|
|
|
|
<objective>
|
|
Make the LDAP Base-DN a multi-value field and the PRIMARY sync scope, replacing the
|
|
"empty group filter = sync nothing" model introduced in Quick 260728-lih (commit 57bc7f9)
|
|
with a clean "Base-DN(s) = scope" model.
|
|
|
|
Base-DN stays a single `String` column holding one DN per line (`\n`-separated) — NO schema
|
|
change, NO migration. The backend splits it and loops all three directory-search paths over
|
|
every base DN, merging/deduping by entry `dn`. groupFilterDns becomes an OPTIONAL extra
|
|
restriction (ou= = extra bases, group DN = memberOf constraint). The critical mass-deactivation
|
|
safety guard is re-keyed: the sync is a total No-Op ONLY when NO base DN is configured.
|
|
|
|
Purpose: Give admins multiple import roots and restore normal multi-base sync when no group
|
|
filter is set, while keeping the guard that stops an unconfigured config from mass-deactivating
|
|
every existing LDAP user.
|
|
Output: Updated ldap.service.ts + spec, multi-line Base-DN textarea, reworked de/en i18n.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
|
@$HOME/.claude/gsd-core/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/STATE.md
|
|
@CLAUDE.md
|
|
|
|
# The three commits this builds on are already on main: c54e424 (syncIntervalMin default 0),
|
|
# 57bc7f9 (empty groupFilterDns = no-op — being REPLACED here), 63a07ab (form default + i18n).
|
|
@apps/api/src/ldap/ldap.service.ts
|
|
@apps/api/src/ldap/ldap.service.spec.ts
|
|
@apps/web/src/app/(portal)/admin/ldap/page.tsx
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 1: Backend multi-base scope + base-DN-keyed deactivation guard + spec</name>
|
|
<files>apps/api/src/ldap/ldap.service.ts, apps/api/src/ldap/ldap.service.spec.ts</files>
|
|
<behavior>
|
|
- parseBaseDns('dc=a\ndc=b') returns ['dc=a','dc=b']; blank/whitespace-only lines dropped; '' returns [].
|
|
- syncUsersForTenant with an EMPTY parsed base-DN list (baseDn '' or whitespace) is a total No-Op:
|
|
no bind, no search, no user create/update, no deactivation, no ldapConfig.update — result is
|
|
{ created:0, updated:0, deactivated:0, errors:[] }.
|
|
- syncUsersForTenant with two base-DN lines and empty groupFilterDns searches BOTH bases and merges/
|
|
dedupes by dn: a user returned under both bases is created once; distinct users are all created.
|
|
- syncUsersForTenant with a non-empty baseDn and EMPTY groupFilterDns now DOES search (no early return)
|
|
and imports users — an empty group filter is no longer a no-op.
|
|
- Existing exclude-list specs stay green: excluded usernames are skipped/deactivated as before, now
|
|
driven by the base-DN scope instead of the removed empty-groupFilterDns short-circuit.
|
|
</behavior>
|
|
<action>
|
|
Add a private helper parseBaseDns(baseDn: string): string[] near mapEntry that splits config.baseDn on /\r?\n/, trims each line, and drops empty lines (returns [] for undefined/empty input).
|
|
|
|
In syncUsersForTenant() REPLACE the 57bc7f9 early-return guard (currently keyed on empty groupFilterDns) with one keyed on the parsed base-DN list: compute const baseDns = this.parseBaseDns(config.baseDn) at the top and, when baseDns.length === 0, return the empty result immediately BEFORE any bind/search and BEFORE the deactivation loop. This is the sole No-Op path and the only thing preventing an unconfigured config from mass-deactivating every LDAP user. Keep lastSyncAt untouched on this path. Rewrite the guard's explanatory comment to describe the base-DN-list condition (do not leave the old groupFilterDns rationale).
|
|
|
|
In collectSearchEntries() REMOVE the special case that returns [] for empty groupFilterDns. New logic: derive baseDns via parseBaseDns(config.baseDn); split groupFilterDns (?? []) into ouBases (/^ou=/i) and groupDns (the rest). Build a Map<string, Entry> keyed by entry.dn. Search EVERY base DN in baseDns with scope 'sub': when groupDns is non-empty apply the existing memberOf OR-clause as an AND with sanitizedFilter (reuse escapeLdapFilterValue exactly as today); when groupDns is empty use sanitizedFilter alone. Then, as ADDITIONAL bases, search each ouBase with the plain sanitizedFilter. Merge every result into the map by entry.dn and return Array.from(map.values()). The ou=/memberOf logic is unchanged in substance — only the search base is now the multi-base list rather than the single config.baseDn.
|
|
|
|
Apply the same multi-base loop to the other two read paths so the whole feature is consistent (per requirement item 2): in listGroups() loop parseBaseDns(config.baseDn), run the existing group/OU discovery search under each base, and merge entries by dn before mapping. In searchUsers() loop parseBaseDns(config.baseDn), run the existing person filter search under each base (keep the per-base sizeLimit), and merge the mapped entries by dn before the alreadyImported flagging query. The controller keeps passing config.baseDn as a String — no controller change.
|
|
|
|
In ldap.service.spec.ts: update the outdated baseConfig comment that claims empty groupFilterDns is a no-op (the exclude-list block now relies on the base-DN scope). REPLACE the entire "empty selection no-op" describe block: change its config to an empty baseDn ('' ) and assert the No-Op (no bind, no search, no create, no update, no ldapConfig.update, result all zeroes) — this now proves the base-DN-list guard, not the removed groupFilterDns guard. ADD a new describe block "multi base DN scope" with a config whose baseDn is 'dc=a,dc=com\ndc=b,dc=com' and empty groupFilterDns: program mockSearch with mockResolvedValueOnce per base returning overlapping+distinct dns, then assert both bases were searched (mockSearch called twice) and results were merged/deduped by dn (a shared dn creates one user, distinct dns each create). Keep all other specs (exclude-list, individual search/import, TLS, verifyUserCredentials) passing.
|
|
</action>
|
|
<verify>
|
|
<automated>pnpm --filter @tessera/api exec vitest run src/ldap/ldap.service.spec.ts</automated>
|
|
<automated>pnpm --filter @tessera/api run type-check</automated>
|
|
</verify>
|
|
<done>All ldap.service.spec.ts specs pass, including the empty-base-DN No-Op test and the new multi-base merge/dedup test; type-check is clean. An empty base-DN list is the only No-Op path; an empty groupFilterDns performs a normal multi-base search.</done>
|
|
</task>
|
|
|
|
<task type="auto">
|
|
<name>Task 2: Multi-line Base-DN textarea + reworked de/en i18n</name>
|
|
<files>apps/web/src/app/(portal)/admin/ldap/page.tsx, apps/web/src/messages/de.json, apps/web/src/messages/en.json</files>
|
|
<action>
|
|
In apps/web/src/app/(portal)/admin/ldap/page.tsx replace the single-line Base-DN control (the type="text" input bound to formData.baseDn, in the connection-settings grid) with a multi-line textarea: keep value={formData.baseDn} and onChange={(e) => setFormData({ ...formData, baseDn: e.target.value })} so the value stays ONE string with newline separators, add rows={3}, swap the fixed-height class for a multi-line class (e.g. min-h-20 instead of h-10, keep the rounded/border/bg/px/py/text-sm styling and required), and set a placeholder showing two example DNs on separate lines. Directly under the textarea render a hint paragraph: <p className="text-xs text-muted-foreground">{t('baseDnHint')}</p>. Do not touch fetchConfig/handleSave — they already carry baseDn as a plain string.
|
|
|
|
In apps/web/src/messages/de.json and en.json under admin.ldap: add a new baseDnHint key (de: one DN per line, the base DN(s) define the sync scope; en equivalent). Reword admin.ldap.groupFilter.description to present the group filter as an OPTIONAL additional restriction and state that with no selection all users under the Base-DN(s) are synced. Reword admin.ldap.groupFilter.emptyMeansAll to say that with no selection all users under the Base-DN(s) are synced. German must read "alle Benutzer unter den Basis-DN(s)" and English "all users under the base DN(s)"; keep both languages semantically identical and drop the previous "es wird nichts synchronisiert" / "nothing is synced" framing.
|
|
</action>
|
|
<verify>
|
|
<automated>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'));console.log('json-ok')"</automated>
|
|
<automated>grep -qF "Basis-DN(s)" apps/web/src/messages/de.json && grep -qF "base DN(s)" apps/web/src/messages/en.json && grep -qF "baseDnHint" apps/web/src/messages/de.json && grep -qF "baseDnHint" apps/web/src/messages/en.json && grep -q "textarea" "apps/web/src/app/(portal)/admin/ldap/page.tsx" && echo grep-ok</automated>
|
|
<automated>pnpm --filter @tessera/web run type-check</automated>
|
|
</verify>
|
|
<done>Both message files are valid JSON and contain the new baseDnHint key plus the "Basis-DN(s)" / "base DN(s)" scope wording; the admin LDAP page renders a multi-line textarea for baseDn with the one-DN-per-line hint; web type-check is clean.</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<threat_model>
|
|
## Trust Boundaries
|
|
|
|
| Boundary | Description |
|
|
|----------|-------------|
|
|
| admin browser -> /ldap/config API | Admin supplies base DN lines + group/exclude filters (trusted admin input, but still interpolated into directory searches) |
|
|
| API -> LDAP/AD directory | Base DNs used as search bases; group DNs interpolated into memberOf filter clauses |
|
|
|
|
## STRIDE Threat Register
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
| T-d3k-01 | Tampering | collectSearchEntries memberOf clause | medium | mitigate | Group DNs stay wrapped in the existing LdapService.escapeLdapFilterValue() (RFC 4515) before interpolation — unchanged from 57bc7f9; base DNs are passed as search-base arguments, not interpolated into filter strings. |
|
|
| T-d3k-02 | Denial of Service | syncUsersForTenant deactivation loop | high | mitigate | Re-keyed guard: the deactivation loop is skipped entirely when the parsed base-DN list is empty, preventing an unconfigured/blank config from mass-deactivating every existing LDAP user (login-gating field isActive). Covered by the empty-base-DN No-Op spec. |
|
|
| T-d3k-03 | Information Disclosure | multi-base merge | low | accept | Merging by entry dn only widens results to already-authorized base DNs an admin explicitly configured; no cross-tenant exposure (upsert stays tenant-scoped via existing tenantId + forTenant path). |
|
|
</threat_model>
|
|
|
|
<verification>
|
|
- `pnpm --filter @tessera/api exec vitest run src/ldap/ldap.service.spec.ts` — all specs green (empty-base-DN No-Op + multi-base merge/dedup + existing exclude/import/TLS specs).
|
|
- `pnpm --filter @tessera/api run type-check` and `pnpm --filter @tessera/web run type-check` — clean.
|
|
- de.json / en.json parse as JSON and carry the reworded scope wording + baseDnHint.
|
|
- No Prisma schema change, no migration, no new endpoint, no Dockerfile/infra change (git diff touches only the five listed files).
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
- Base-DN admin field is a multi-line textarea; value persists as one `\n`-separated String (no schema/migration change).
|
|
- All three directory-search paths loop every configured base DN and merge/dedupe by entry dn.
|
|
- Empty groupFilterDns performs a normal multi-base search (no memberOf restriction); it is NOT a No-Op.
|
|
- syncUsersForTenant is a total No-Op (no search, no deactivation) ONLY when the parsed base-DN list is empty.
|
|
- de + en i18n consistently describe Base-DN(s) as the scope and the group filter as an optional extra restriction.
|
|
</success_criteria>
|
|
|
|
<output>
|
|
Create `.planning/quick/260729-d3k-ldap-multi-base-dn-und-base-dn-als-scope/260729-d3k-SUMMARY.md` when done.
|
|
</output> |