138 lines
12 KiB
Markdown
138 lines
12 KiB
Markdown
---
|
|
phase: quick-260909-ipc
|
|
verified: 2026-09-09T14:20:00Z
|
|
status: passed
|
|
score: 7/7 must-haves verified
|
|
covered_files:
|
|
- .planning/quick/260909-ipc-mandantentrennung-etappe-2-bereich-ldap-/260909-ipc-PLAN.md
|
|
- .planning/quick/260909-ipc-mandantentrennung-etappe-2-bereich-ldap-/260909-ipc-SUMMARY.md
|
|
- apps/api/scripts/rls-scratch-check.mjs
|
|
- apps/api/src/ldap/ldap-config.service.spec.ts
|
|
- apps/api/src/ldap/ldap-config.service.ts
|
|
- apps/api/src/ldap/ldap.controller.ts
|
|
- apps/api/src/ldap/ldap.service.spec.ts
|
|
- apps/api/src/ldap/ldap.service.ts
|
|
- apps/api/src/prisma/rls-access-inventory.spec.ts
|
|
- docs/mandantentrennung-etappe2-fehlerrichtung.md
|
|
- docs/mandantentrennung-zugriffsklassifikation.md
|
|
covered_digest: "v1:sha256:2d8c27e76a2953a31a1eaca490d972fac12725f11f4d2e2f3ff67c2b3229e3dd"
|
|
behavior_unverified: 0
|
|
overrides_applied: 0
|
|
---
|
|
|
|
# Quick Task 260909-ipc: Mandantentrennung Etappe 2, Bereich ldap — Verification Report
|
|
|
|
**Task Goal:** Convert the 21 classified database access sites in the `ldap` area
|
|
(`ldap-config.service.ts`, `ldap.service.ts`) to `forTenant()`, keep the classification
|
|
document and its machine guard in sync with the code.
|
|
|
|
**Verified:** 2026-09-09
|
|
**Status:** passed
|
|
**Re-verification:** No — initial verification
|
|
|
|
## Goal Achievement
|
|
|
|
### Observable Truths
|
|
|
|
| # | Truth | Status | Evidence |
|
|
|---|-------|--------|----------|
|
|
| 1 | Every tenant-bound DB access in `ldap` runs through `forTenant()` with the caller's known tenant | ✓ VERIFIED | Post-change grep of `this.prisma.` in `ldap-config.service.ts` yields exactly 3 hits (lines 66, 78 in `onApplicationBootstrap`, line 309 in `getAllActiveConfigs`) and in `ldap.service.ts` exactly 1 hit (line 439, `resolveEmailForWrite`) — the three documented exceptions, nothing more |
|
|
| 2 | The two deliberate exceptions stay unbound and carry a written reason in the code | ✓ VERIFIED | `resolveEmailForWrite` (ldap.service.ts:417-432) and `getAllActiveConfigs`/`onApplicationBootstrap` (ldap-config.service.ts) each carry a multi-paragraph German comment explaining the platform-wide `@unique` constraint / cross-tenant scheduler read, read in full above |
|
|
| 3 | A written critique names, per path, the concrete signal a too-few result would produce, and names the code that reads emptiness as absence | ✓ VERIFIED | `docs/mandantentrennung-etappe2-fehlerrichtung.md` (164 lines) has a per-path signal table (section c, 8 rows) and a dedicated "Welcher Code deutet Leere als Abwesenheit" section (d) naming 4 specific methods with line/behavior detail — not generic prose |
|
|
| 4 | `LdapFieldMapping` visibility via the `LdapConfig` join is MEASURED under a role without BYPASSRLS, not asserted | ✓ VERIFIED | Independently re-ran `TESSERA_SCRATCH_ADMIN_URL=... node apps/api/scripts/rls-scratch-check.mjs` myself — all 13 checks passed, including `fieldmapping-folgt-join-auf-ldapconfig: bestanden` and `fieldmapping-schreiben-fremde-konfiguration-abgelehnt: bestanden — ... ERROR: new row violates row-level security policy`. Policy SQL is extracted verbatim from `20260618112133_rls_policies/migration.sql` (confirmed by reading both files), not retyped |
|
|
| 5 | Deleting a foreign tenant's field mapping by id no longer succeeds (T-IPC-01) | ✓ VERIFIED | `ldap.controller.ts` `removeFieldMapping` now derives `tenantId` from `req.tenantId` (session) and passes it to the service; `ldap-config.service.ts` `removeFieldMapping(tenantId, mappingId)` does a `tenantPrisma.ldapFieldMapping.findUnique` first and returns `null` (→ 404) when invisible under that tenant. Test `removeFieldMapping() liefert null, wenn die Zuordnung unter diesem Mandanten nicht sichtbar ist (T-IPC-01)` exists and is part of the 719 green tests |
|
|
| 6 | Classification doc and machine guard reflect the new state; a green run with a stale doc is impossible | ✓ VERIFIED | `rls-access-inventory.spec.ts` strips comments before scanning, detects `const X = forTenant(` + `X.<model>` bound sites in addition to `this.prisma.<model>` unbound sites, computes a `Stand` per (file, model) pair and asserts it against the doc's new 4th column; ran as part of the full suite (9 tests, all green) |
|
|
| 7 | 701+ tests and type-check are green; DATABASE_URL, compose, .env, schema.prisma unchanged | ✓ VERIFIED | Independently ran `npm --prefix apps/api run test` → 719/719 passed (53 files); `npm --prefix apps/api run type-check` → exit 0; `git diff --stat b34500b..HEAD` (11 files changed) contains no schema/migration/compose/.env entries |
|
|
|
|
**Score:** 7/7 truths verified (0 present, behavior-unverified)
|
|
|
|
### Required Artifacts
|
|
|
|
| Artifact | Expected | Status | Details |
|
|
|----------|----------|--------|---------|
|
|
| `docs/mandantentrennung-etappe2-fehlerrichtung.md` | New critique doc, substantive | ✓ VERIFIED | 164 lines, per-path signal table, named "reads emptiness as absence" section, measured (not assumed) values quoted verbatim |
|
|
| `apps/api/scripts/rls-scratch-check.mjs` | 5 new LDAP checks, policies extracted from migration | ✓ VERIFIED | `runLdapAreaChecks` present; independently executed, 13/13 checks pass; policy SQL sliced out of the shipped migration with a hard-fail guard (`ldap-policies-aus-migration-gefunden`) if extraction fails |
|
|
| `apps/api/src/ldap/ldap-config.service.ts` | 5 methods bound, 2 stay cross-tenant with reason | ✓ VERIFIED | `getConfig`/`createConfig`/`updateConfig`/`addFieldMapping`/`removeFieldMapping` all create `forTenant(this.prisma, tenantId)`; `getAllActiveConfigs`/`onApplicationBootstrap` unchanged and commented |
|
|
| `apps/api/src/ldap/ldap.service.ts` | 11 queries in 6 methods bound, 1 stays cross-tenant | ✓ VERIFIED | Only remaining `this.prisma.` hit is `resolveEmailForWrite` (line 439); all others route through a per-method `tenantPrisma` |
|
|
| `apps/api/src/ldap/ldap.controller.ts` | tenant sourced from session for delete | ✓ VERIFIED | `removeFieldMapping(@Req() req, @Param('id') id)` reads `req.tenantId`, 400s if absent, passes to service |
|
|
| `apps/api/src/prisma/rls-access-inventory.spec.ts` | detects bound + unbound sites, Stand column check | ✓ VERIFIED | Full implementation read; 9 tests, all green in the full suite run |
|
|
| `docs/mandantentrennung-zugriffsklassifikation.md` | new Stand column, corrected class, 2 new pairs | ✓ VERIFIED | All `ldap` rows carry a `Stand` value consistent with source; `(ldap-config.service.ts, ldapConfig)` corrected to `beides`; `auth.service.ts/passwordResetToken` and `ldap.service.ts/groupMembership` present as newly-surfaced pairs with explanatory text |
|
|
|
|
### Key Link Verification
|
|
|
|
| From | To | Via | Status | Details |
|
|
|------|-----|-----|--------|---------|
|
|
| `forTenant()` | `tenant_isolation_policy` on `LdapConfig`/`LdapFieldMapping` | scratch-database measurement against real migration SQL | ✓ WIRED | Independently re-run, all 13 checks green including the two evidentiary lines quoted above |
|
|
| `LdapFieldMapping` | `LdapConfig` | join-based RLS policy (read AND write measured) | ✓ WIRED | `fieldmapping-folgt-join-auf-ldapconfig` (read) and `fieldmapping-schreiben-fremde-konfiguration-abgelehnt` (write, rejected) both measured and passed |
|
|
| `ldap.controller.ts req.tenantId` | `LdapConfigService.removeFieldMapping(tenantId, ...)` | session-derived tenant parameter | ✓ WIRED | Confirmed by reading the controller source; closes the T-IPC-01 gap |
|
|
| `ldap.service.ts` delete branch | `groups.service.ts` (reassignDefaultBeforeDelete/ensureDefaultGroup) | documented handoff, NOT part of this conversion | ✓ CONFIRMED OUT OF SCOPE | `grep` of `groups.service.ts` shows it is entirely `this.prisma.*`-based, unconverted, exactly as the plan/critique doc describes as a deferred Etappe-4 ordering condition |
|
|
| `rls-access-inventory.spec.ts` | Bestandsaufnahme table incl. Stand column | mechanical cross-check | ✓ WIRED | Comment-stripped regex scan of both `this.prisma.<model>` and `<boundVar>.<model>`; asserts doc rows match measured Stand; ran green |
|
|
|
|
### Behavioral Spot-Checks
|
|
|
|
| Behavior | Command | Result | Status |
|
|
|----------|---------|--------|--------|
|
|
| Full test suite | `npm --prefix apps/api run test` | 719/719 passed, 53 files | ✓ PASS |
|
|
| Type-check | `npm --prefix apps/api run type-check` | exit 0 | ✓ PASS |
|
|
| Scratch RLS probe (all 13, incl. 5 new ldap checks) | `TESSERA_SCRATCH_ADMIN_URL=... node apps/api/scripts/rls-scratch-check.mjs` | "Alle 13 Pruefungen bestanden." | ✓ PASS |
|
|
| Debt-marker scan of all 9 touched code/doc files | `grep -nE "TBD\|FIXME\|XXX\|TODO\|HACK\|PLACEHOLDER"` | 0 hits across all 9 files | ✓ PASS |
|
|
|
|
### Requirements Coverage
|
|
|
|
| Requirement | Source Plan | Description | Status | Evidence |
|
|
|-------------|------------|-------------|--------|----------|
|
|
| WINDOWS-20 | 260909-ipc-PLAN.md | Mandantentrennung Etappe 2, ldap area | ✓ SATISFIED | 21 sites converted/justified, T-IPC-01 closed, doc + guard in sync |
|
|
| ETAPPE-2-LDAP | 260909-ipc-PLAN.md | ldap area of Etappe 2 | ✓ SATISFIED | Same evidence as above |
|
|
|
|
### Anti-Patterns Found
|
|
|
|
None. No TBD/FIXME/XXX/TODO/HACK/PLACEHOLDER markers in any of the 9 touched implementation/doc files. No stub returns, no hardcoded empty arrays feeding rendered/consumed output.
|
|
|
|
### Test Honesty Check (Item 4 of the verification brief)
|
|
|
|
`ldap.service.spec.ts` still carries a top-level identity mock for `forTenant`
|
|
(`forTenant: vi.fn((p) => p)`), used by the pre-existing describe blocks — this
|
|
mock alone genuinely cannot detect a binding regression, matching Befund F's
|
|
own diagnosis. A dedicated new describe block ("Bindungsnachweis mit
|
|
unterscheidbaren Clients", ~140 lines) overrides `forTenant`'s mock
|
|
implementation to return a second, structurally distinct object
|
|
(`boundPrisma`, whose `user`/`group`/`groupMembership` sub-objects expose
|
|
different methods than `unboundPrisma`). Reasoning through failure modes: if
|
|
`resolveEmailForWrite` were changed to query the bound client, or if any of
|
|
the six converted methods were changed back to query `this.prisma` directly,
|
|
the assertions (`expect(unboundPrisma.user.findUnique).toHaveBeenCalledWith(...)`
|
|
/ `expect(boundPrisma.user.findFirst).toHaveBeenCalled()`) would fail —
|
|
either because the wrong spy recorded the call, or because the mismatched
|
|
mock object lacks the method being called and throws. This is a real,
|
|
falsifiable regression test, not a rebranded identity mock.
|
|
|
|
`ldap-config.service.spec.ts` keeps the identity mock throughout (per the
|
|
plan's own, weaker, behavior spec — it only asserts `forTenant` was called
|
|
with the right tenant id, not which object received the query). This leaves
|
|
a narrower gap than `ldap.service.ts`, but it is closed by the independent,
|
|
textual `rls-access-inventory.spec.ts` guard, which inspects the literal
|
|
source for `tenantPrisma.<model>` vs `this.prisma.<model>` regardless of what
|
|
any mock returns.
|
|
|
|
### Human Verification Required
|
|
|
|
None. All must-haves are verifiable from the codebase and confirmed by
|
|
independently re-running the test suite, the type-check, and the scratch RLS
|
|
probe (not merely trusting SUMMARY.md's reported numbers).
|
|
|
|
### Gaps Summary
|
|
|
|
No gaps. All 7 must-have truths hold, all artifacts are substantive and
|
|
wired, the two deliberate cross-tenant exceptions are justified in code and
|
|
tested, the T-IPC-01 deletion gap is closed and tested, the classification
|
|
document and its machine guard are in sync (9/9 inventory tests green,
|
|
Stand column present and consistent), and the mandated critique document is
|
|
substantive with named per-path signals rather than generalities. No schema,
|
|
migration, compose, or `.env` changes were made; the cutover switch remains
|
|
untouched by this task's diff.
|
|
|
|
---
|
|
|
|
_Verified: 2026-09-09_
|
|
_Verifier: Claude (gsd-verifier)_
|