The user has no write access to the company AD, so tests 1, 3, 4 and 8 cannot
be run there, and building a throwaway domain controller for them was judged
disproportionate now that the one substantive defect at this spot is found and
fixed (UAT test 2 -> quick task 260811-f9i).
Recorded rather than hidden: A1 (objectGUID survives a rename) now rests on
Microsoft's documentation, not on our own measurement. The Tessera-side rename
and delete logic stays covered by unit tests against fixtures. If a rename ever
fails to propagate in production, 16-VERIFICATION.md names that as the starting
point.
Phase 16 marked complete in STATE.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
The existence sweep in syncBoundGroupsForTenant() built its filter by
interpolating a byte-wise \xx escape of the stored objectGUID into a filter
string. ldapts parses that string before encoding it and does not turn the
escape sequences back into the bytes they stand for, so the assertion value
that reached the directory was a different value and matched nothing.
Measured read-only against a real Active Directory on 2026-08-11, probing a
group whose GUID had just been read from that same directory:
(objectGUID=\1e\4b...) 0 hits
(objectGUID=\1E\4B...) 0 hits
EqualityFilter{attribute, value: <16 bytes>} 1 hit, correct DN
(cn=Domain Admins) [control] 1 hit
Both the narrow base-DN sweep and the wider WR-03 move-detection sweep shared
that filter, so neither could ever hit: every AD-bound group looked deleted and
would have been removed together with its GroupMembership and ModuleGrant rows
on the first real sync, after handing off the default-group marker.
Build the filter as an EqualityFilter over the raw Buffer instead, and drop
escapeLdapFilterBuffer() -- it has no remaining caller and is the trap the code
walked into. escapeLdapFilterValue() is untouched: escaping STRING values into
a filter is correct and still in use.
The existing spec mocks matched on the escaped string, which is how the broken
shape passed review. They now match on the filter object's Buffer value, and
two added tests fail if a stringly-typed objectGUID filter ever comes back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ran the Phase 16 browser walkthrough against alpha.tessera.ctl.de with the
real AD (balios.ctl.local) behind it.
- Test 5 (AD group import section): passed. 236 discovered entries, none of
them OUs; selection count, disabled-at-zero button, already-imported badge,
result block and the discovery error state all behave as specified.
- Test 6 (group dialog, three states): passed. Includes the WR-02 case — a
409 name collision stays visible in the dialog with the input preserved.
- Test 7: partial. Error path of the sync report and the grants-matrix column
search under both internal and AD name pass; the number rows of a successful
sync run are still open because a real sync was skipped by request (no
group/OU filter set, so it would import every person under DC=ctl,DC=local).
Tests 1-4 and 8 remain open — they need AD write access or are backstop
assertions. CN=Claude_VT stays imported on alpha because tests 3 and 4 need it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks all four Warning findings from 16-REVIEW.md as resolved with their
fix commit hashes; the two Info findings (IN-01/IN-02) remain open and
out of scope for this fix pass. Appends a note to 16-03-SUMMARY.md
recording the WR-02/WR-03/WR-04 behavior change in
syncBoundGroupsForTenant(), since that summary still described the
pre-fix behavior.
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.
syncBoundGroupsForTenant() compared cn/dn for byte equality, so any
casing difference AD returns between two runs (e.g. after a
domain-controller switch) would look like a rename and re-write
name/ldapDn every single sync — violating the 'sync twice over an
unchanged AD state = no-op' idempotency guarantee. The comparison used
to DECIDE 'is this a rename' is now case-insensitive; the value written
on an actual rename is still stored byte-for-byte as the directory
reports it, per D-03.
syncBoundGroupsForTenant()'s rename write updates name and ldapDn in one
call, so a P2002 there can come from either @@unique([tenantId, name])
or @@unique([tenantId, ldapDn]). The catch previously reported every
P2002 as a name collision unconditionally; it now inspects
err.meta.target the same way importGroupsByDn() already does for its
own create() call, so a non-name unique violation is no longer
mislabelled and sent the admin down the wrong troubleshooting path.
A legacy binding from plan 15-06 has ldapDn set but ldapObjectGuid stays
null until the first syncBoundGroupsForTenant() run backfills it. Until
then, GroupsService.update() let a direct PATCH rename through even
though GroupFormModal.tsx already treats the same group as AD-bound
(isImported = ldapDn != null) — the D-03 name lock was only a UI
convention for that window, not the backend invariant the 16-02 summary
claimed.
- 16-05-SUMMARY.md documents the two-task plan (sync-report wiring,
grants-matrix internal-name fallback).
- PERM-02 marked complete in REQUIREMENTS.md: all five Phase-16 success
criteria are code-complete across plans 16-01..16-03; this plan
delivered the last missing visibility layer (D-05/D-06) and the
third D-04 display site.
- WINDOWS.md #6: this plan's own manual browser walkthrough (sync
report three-line render, amber default-marker line, grants-matrix
two-name search) not executed — no browser tool in this session.
- Local Group interface gains internalName?: string | null — GET
/module-grants/matrix already returns the field (no select on the
groups query), only the frontend type was missing it.
- Column header text and title tooltip now use internalName ?? name
(nullish, not truthy) — same fallback pattern as admin/groups and
UserAccessModal. No layout change: same truncate-with-tooltip cell.
- Search filter now matches both the display name and the stored AD
name, so neither the internal nor the original AD name search goes
empty after the header text changed.
- SyncResult interface grows additively: groupMembershipsAdded/Removed
(D-21 backend gap, existed since Phase 15 but never wired into the
frontend) plus groupsAdopted/Renamed/Deleted and defaultMarkerMoved
(Plan 16-03).
- New syncRequestError state: a failed sync request (network error or
!res.ok) now renders a visible text-sm text-destructive line instead
of silently reporting a three-zero result as a successful no-op.
- Sync report container gains three new lines: group-membership counts,
AD-group counts (always visible, even at zero), and a conditional
amber "default marker reassigned" line shown only when
defaultMarkerMoved > 0.
- Four new i18n keys under admin.ldap.sync in de.json/en.json.
Documents the GroupFormModal three-state rebuild (D-03/D-04/D-07), the
groups-list internalName fallback, and the ldapBind i18n cleanup for
Phase 16 Plan 4.
Delete the admin.groups.ldapBind.* subtree (hint/bound/unbind/
searchPlaceholder/discoverError/noResults) from de.json and en.json —
fully orphaned since GroupFormModal.tsx no longer has an AD-binding
codepath. admin.groups.ldapBinding (column header) and every other
admin.groups.* key are untouched; key sets stay in parity across both
languages.
Test file's translation stub loses the same dead ldapBind block and
gains the seven Task 1 keys. Two new cases lock down D-07: opening the
create dialog issues no /ldap/groups request, and editing an imported
group renders a disabled name input plus the internal-name field
instead of any AD-search UI.
Name column renders group.internalName ?? group.name (nullish, not
truthiness, since the backend already normalizes blank values to null)
inside a span carrying title={group.name} so the AD name stays
discoverable on hover once an internal name is set. Badge column is
untouched — it remains the sole imported-vs-local marker per D-07.
Adds two component-test cases covering the fallback and its title
attribute (D-04).
- Remove AD radio-selection block, discovery effect/state, and the
two-step create-then-PATCH-bind flow from GroupFormModal.tsx (D-07):
a local group can no longer be bound to an AD group from this dialog.
- Add locked name field with provenance hint + AD-DN read-only line and
an editable internalName field for imported groups (D-03/D-04); create
state gets a hint linking to the LDAP import area.
- Visible save-error line (never a silent catch{}) that distinguishes a
409 name collision from a generic failure; dialog stays open, inputs
are preserved.
- Add Group.internalName to the page.tsx interface now (Rule 3 — the
modal cannot typecheck without it; Task 2 adds the display-cell usage).
- Add seven new admin.groups i18n keys to de.json/en.json (orphaned
ldapBind.* keys removed in Task 3).
- syncUsersForTenant() now calls syncBoundGroupsForTenant() (5a) BEFORE
syncGroupMembershipsForTenant() (5b) — the central correctness ordering
of Phase 16 (RESEARCH.md Pitfall 1): a rename detected in the same run
must be written back before the memberOf filter is built, or the
membership sync would misreport a rename as a membership wipeout
- Add observable ordering test (call-order spies), a no-op-guard test, and
a regression test proving a memberOf search never uses the stale
pre-rename DN
- Update the D-21 membership-sync test fixtures to resolve step 5a as a
deterministic no-op (DN-derived identity GUID), since the wiring now
runs 5a ahead of every syncUsersForTenant() call those tests exercise
A1 (objectGUID survives an AD rename) and A2 (binary filter escape syntax)
remain unverified against a real directory — no reachable AD in this
sandbox. Documented as an outstanding live verification in the plan
SUMMARY, not silently skipped.
- New private LdapService.syncBoundGroupsForTenant(): rename detection
(SC-3), disappearance deletion with default-marker handoff before delete
(SC-4/D-05/D-06), legacy ldapDn-only binding GUID backfill (D-07), and a
32-hex-char guard before any objectGUID filter interpolation (T-16-01)
- LdapSyncResult grows additively: groupsAdopted, groupsRenamed,
groupsDeleted, defaultMarkerMoved
- LdapService constructor takes GroupsService; LdapModule imports
GroupsModule (no cycle)
- 14 new test cases covering the full behavior matrix plus idempotency
- ModuleGrantsService.getUserAccess() now selects internalName on the
membership query's group projection (already present via `include:
{ group: true }` on the grant query)
- Both display points (viaGroups names, membership chips' name field)
use internalName ?? name; groups[] sorting now runs over the
displayed name as a result, distinct from GroupsService.listForTenant()
which still sorts by the raw name column
- 3 new test cases: fallback set/unset, sort-by-displayed-name
- No apps/web/ changes (verified via git diff --name-only)
- GroupsService.update() rejects `name` with BadRequestException when the
loaded group carries a set ldapObjectGuid (imported groups) — a real
backend invariant, not a UI-only disable
- internalName is settable/clearable on any group; empty/whitespace-only
values normalize to null instead of an empty display name
- listForTenant() now projects internalName alongside name
- UpdateGroupDto drops ldapDn (D-07: no more codepath binds a local group
to AD via this route) and gains internalName?: string | null
- 9 new test cases in groups.service.spec.ts (name lock, internalName
set/clear/idempotent/local-group/unicode, listForTenant projection);
stale ldapDn update() test removed (behavior intentionally deleted)
- DEFAULT_GROUP_NAME extracted as shared constant between
ensureDefaultGroup() and the new reassignDefaultBeforeDelete()
- reassignDefaultBeforeDelete(tenantId, groupId) moves the default
marker deterministically (DEFAULT_GROUP_NAME first, else oldest
other group by createdAt asc), never deletes, never throws
- 6 test cases covering handoff, fallback ordering, no-other-group,
non-default no-op, cross-tenant no-op, and P2002 race
- Migration 20260806133916_add_group_internal_name_and_object_guid applied
against the local Postgres container (baselined 24 prior migrations first
— _prisma_migrations was missing, unrelated to this task's DDL)
- New describe block in migration-sql.spec.ts pins internalName,
ldapObjectGuid, and the (tenantId, ldapObjectGuid) unique index
- Full API test suite green (40 files, 526 tests)
Task 1 checkpoint resolved: approve-both, granted 2026-08-06 by the
project owner (D-04 one-way schema extension: Group.internalName +
Group.ldapObjectGuid, both nullable, one versioned migration).
Adds the Phase 16 tracer slice through every layer:
- Prisma schema: Group.internalName, Group.ldapObjectGuid,
@@unique([tenantId, ldapObjectGuid]) (Prisma client regenerated;
the versioned migration itself is Task 3, separately blocking).
- LdapService: listGroups() now reads objectGUID via
explicitBufferAttributes and flags alreadyImported per tenant;
new importGroupsByDn() creates a Group per checked DN with
name/ldapDn/ldapObjectGuid, reject-with-report on name collision
(P2002 on name -> nameCollisions, P2002 on ldapObjectGuid ->
skipped), never aborts the batch on one DN's error; new static
escapeLdapFilterBuffer() for Plan 16-03's later existence sweep.
- DTO/controller: ImportGroupsDto, POST /ldap/groups/import
(ADMIN/SUPER_ADMIN), listGroups route now tenant-scoped.
- Frontend: new "AD-Gruppen importieren" section in /admin/ldap,
own discovery/import handlers with a visible error state
(Owner decision 2026-08-06 — no silent catch{} for these two
handlers), i18n keys in de.json/en.json.
- Tests: 8 new cases covering the full <behavior> list plus
listGroups sort order and alreadyImported.
Flagged assumption (RESEARCH.md A1/A2): objectGUID rename-stability
and the binary filter syntax are unverified against a real AD —
this plan only WRITES the GUID, Plan 16-03 reads it back live.
- TenantService.create ruft nach prisma.tenant.create ensureDefaultGroup
auf; Fehler werden protokolliert, nicht propagiert (Muster aus
UserService.create)
- tenant.module.ts importiert GroupsModule (keine Zirkularitaet, wie
UserModule bereits vormacht)
- AdminSeedService.onApplicationBootstrap besteht jetzt aus zwei
sequenziellen await-Schritten: seedAdmin() (bisheriger Rumpf, plus
ensureDefaultGroup nach dem Tenant-Upsert und VOR user.create), dann
ensureDefaultGroupsForAllTenants() als abschliessende Reparatur ueber
ALLE Mandanten — laeuft unabhaengig von seedAdmin()s fruehen
Rueckkehrpfaden (fehlende ENV / Admin existiert bereits) und ist je
Mandant sowie insgesamt try/catch-gekapselt, blockiert den API-Start nie
- Reparatur sitzt bewusst NICHT als eigener onApplicationBootstrap-Hook
in GroupsModule (Ordering-Falle aus tender-scheduler.service.ts)
- tenant.service.spec.ts, admin-seed.service.spec.ts (neu): Reihenfolge,
beide fruehen Rueckkehrpfade, Fehlerisolation je Mandant, Idempotenz
ueber zwei Bootstrap-Laeufe
- Neue Methode ensureDefaultGroup: legt fuer einen Mandanten ohne jede
Gruppe die Standardgruppe 'Alle Benutzer' (isDefault:true) an, nimmt
alle Bestandsbenutzer als MANUAL-Mitglieder auf und erzeugt Grants
fuer alle aktiven Module — derselbe Endzustand wie die drei
Backfill-INSERTs der Migration 20260804130130
- Waechter prueft ausschliesslich group.count === 0, niemals die
fehlende isDefault-Markierung (D-13)
- P2002 aus dem partiellen Index Group_one_default_per_tenant wird
abgefangen und liefert null statt zu werfen (Race-Sicherheit)
- groups.service.spec.ts: Fake erweitert um group.count,
tenantModuleActivation, moduleGrant.findMany/createMany,
$transaction mit Callback-Form, plus voller ensureDefaultGroup-Testblock
- Reuses admin.groups.members.sourceManual/.sourceLdap -- no new i18n key
- Badge markup copied verbatim from GroupMembersModal.tsx (blue for LDAP,
neutral gray otherwise), same visual language on both surfaces
- Chips come from the response's groups (actual GroupMembership rows),
not the union of row.viaGroups -- closes the reproduced defect where
revoking a group's last grant hid an otherwise-unchanged membership
- React key is the group id, not the name
- Test file: moved the LDAP/MANUAL badge assertion out of this commit,
it belongs to Task 2 which reuses admin.groups.members