26 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, estimate, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | estimate | must_haves | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 16-ad-gruppen-synchronisation | 02 | execute | 2 |
|
|
true |
|
|
|
Purpose: Alle Gruppen-seitigen Bausteine liegen fertig vor, bevor der Sync in 16-03 sie orchestriert. Der Sync soll keine eigene Lösch-, Handoff- oder Namenslogik erfinden.
Output: DEFAULT_GROUP_NAME, reassignDefaultBeforeDelete(), Namenssperre und internalName-Unterstuetzung in update()/listForTenant(), angepasstes UpdateGroupDto, Anzeige-Fallback in module-grants.service.ts, Tests.
<artifacts_this_phase_produces> Neu entstehende Symbole in diesem Plan:
| Art | Symbol |
|---|---|
| Exportierte Konstante | DEFAULT_GROUP_NAME (apps/api/src/groups/groups.service.ts) |
| Service-Methode | GroupsService.reassignDefaultBeforeDelete(tenantId, groupId): Promise<boolean> |
| DTO-Feld | UpdateGroupDto.internalName?: string | null |
| Entferntes DTO-Feld | UpdateGroupDto.ldapDn (D-07) |
| Rueckgabefeld | internalName in der Projektion von GroupsService.listForTenant() |
Aus Plan 16-01 uebernommen und hier vorausgesetzt: Group.internalName, Group.ldapObjectGuid.
</artifacts_this_phase_produces>
<execution_context> @$HOME/.claude/gsd-core/workflows/execute-plan.md @$HOME/.claude/gsd-core/templates/summary.md </execution_context>
@.planning/PROJECT.md @.planning/ROADMAP.md @.planning/STATE.md @.planning/phases/16-ad-gruppen-synchronisation/16-CONTEXT.md @.planning/phases/16-ad-gruppen-synchronisation/16-PATTERNS.md @.planning/phases/16-ad-gruppen-synchronisation/16-UI-SPEC.md @.planning/phases/16-ad-gruppen-synchronisation/16-01-SUMMARY.md Task 1: Standardgruppen-Handoff als eigener Baustein (D-06) apps/api/src/groups/groups.service.ts, apps/api/src/groups/groups.service.spec.ts - `apps/api/src/groups/groups.service.ts` — `findOwned()` (Zeilen 78-86), `update()` mit der Standardmarkierungs-Transaktion (99-154), `ensureDefaultGroup()` (277-328, enthaelt heute das Literal des Standardgruppennamens und das P2002-Muster) - `apps/api/src/groups/groups.service.spec.ts` — bestehende Mock-Struktur fuer `update`/`findOwned` - `.planning/phases/16-ad-gruppen-synchronisation/16-PATTERNS.md` — Abschnitt `groups.service.ts`, insbesondere die Default-Marker-Transaktion und der `ensureDefaultGroup`-Auszug - `.planning/phases/16-ad-gruppen-synchronisation/16-CONTEXT.md` — D-06 im Wortlaut - Gruppe traegt die Standardmarkierung, und es existiert eine andere Gruppe mit dem Namen aus `DEFAULT_GROUP_NAME` → Markierung wandert dorthin, Rueckgabe `true` - Gruppe traegt die Markierung, `DEFAULT_GROUP_NAME` existiert nicht, aber andere Gruppen → Markierung wandert auf die aelteste andere Gruppe (`createdAt` aufsteigend), Rueckgabe `true` - Gruppe traegt die Markierung, es gibt keine andere Gruppe → Rueckgabe `false`, kein Wurf - Gruppe traegt die Markierung nicht → No-Op, Rueckgabe `false` - Gruppen-ID gehoert zu einem fremden Mandanten → No-Op, Rueckgabe `false` (kein Wurf, da der Aufrufer ein Batch-Lauf ist) - Die Transaktion laeuft in einen `P2002` → Rueckgabe `false` statt Wurf ("hat sich schon jemand anderes gekuemmert") Ziehe das heute in `ensureDefaultGroup()` hart stehende Literal des Standardgruppennamens in eine exportierte Konstante `DEFAULT_GROUP_NAME` am Dateikopf von `apps/api/src/groups/groups.service.ts` und nutze sie an der bestehenden Stelle weiter. Damit teilen sich beide Methoden eine einzige Wahrheit; zwei getrennte Literale wuerden bei einer spaeteren Umbenennung auseinanderlaufen.Neue oeffentliche Methode async reassignDefaultBeforeDelete(tenantId: string, groupId: string): Promise<boolean>. Sie loescht nichts — sie verschiebt ausschliesslich die Markierung und meldet per Rueckgabewert, ob sie es getan hat. Ablauf:
- Gruppe per
findFirst({ where: { id: groupId, tenantId } })laden. Kein Treffer oderisDefault === false→falsezurueckgeben. Bewusst NICHTfindOwned()verwenden: dessenNotFoundExceptionist fuer HTTP gebaut und wuerde einen Batch-Lauf abbrechen. - Zielgruppe bestimmen — zuerst
findFirst({ where: { tenantId, id: { not: groupId }, name: DEFAULT_GROUP_NAME } }), sonstfindFirst({ where: { tenantId, id: { not: groupId } }, orderBy: { createdAt: 'asc' } }). Die aufsteigende Sortierung ist der Determinismus-Garant: zwei Laeufe ueber denselben Bestand waehlen dieselbe Gruppe. - Kein Ziel →
falsezurueckgeben. Der Aufrufer (Plan 16-03) ruft nach der LoeschungensureDefaultGroup(tenantId)und baut damit den Bestand neu auf. - Sonst dieselbe Zwei-Schritt-Transaktionsform wie in
update():$transaction([group.updateMany({ where: { tenantId, isDefault: true }, data: { isDefault: false } }), group.update({ where: { id: <ziel> }, data: { isDefault: true } })]).truezurueckgeben. - Fehler mit
code === 'P2002'abfangen undfalsezurueckgeben — der partielle Unique-IndexGroup_one_default_per_tenantist der eigentliche Durchsetzungspunkt, das Muster ist ausensureDefaultGroup()uebernommen. Andere Fehler weiterwerfen.
Ergaenze in apps/api/src/groups/groups.service.spec.ts einen describe-Block GroupsService.reassignDefaultBeforeDelete (D-06) mit je einem Fall pro Zeile der <behavior>-Liste, aufgebaut auf der bestehenden Prisma-Mock-Struktur der Datei. Achte darauf, dass der gemockte findFirst mehrere unterschiedliche Aufrufmuster bedienen muss (Gruppe laden, Namensziel, Fallback-Ziel) — verwende mockResolvedValueOnce-Ketten statt eines einzelnen mockResolvedValue, damit die drei Aufrufe unterscheidbar bleiben.
cd apps/api && npx vitest run src/groups/groups.service.spec.ts -t "reassignDefaultBeforeDelete"
cd apps/api && npx tsc --noEmit
<acceptance_criteria>
- grep -c "export const DEFAULT_GROUP_NAME" apps/api/src/groups/groups.service.ts ergibt 1
- Der Standardgruppenname steht nur noch an dieser einen Stelle als Literal: grep -c "'Alle Benutzer'" apps/api/src/groups/groups.service.ts ergibt 1
- grep -q "async reassignDefaultBeforeDelete" apps/api/src/groups/groups.service.ts trifft
- Die Methode enthaelt keinen Aufruf von group.delete: awk '/async reassignDefaultBeforeDelete/,/^ }$/' apps/api/src/groups/groups.service.ts | grep -c 'group.delete' ergibt 0
- awk '/async reassignDefaultBeforeDelete/,/^ }$/' apps/api/src/groups/groups.service.ts | grep -c "orderBy" ist >= 1 (deterministische Zielauswahl)
- cd apps/api && npx vitest run src/groups/groups.service.spec.ts -t "reassignDefaultBeforeDelete" ist gruen mit mindestens 6 Faellen
</acceptance_criteria>
reassignDefaultBeforeDelete verschiebt die Standardmarkierung deterministisch, meldet den Ausgang per Rueckgabewert, wirft in keinem der sechs Faelle und ist getestet.
Service. In GroupsService.update() die Signatur auf { name?: string; isDefault?: boolean; internalName?: string | null } umstellen und den ldapDn-Zweig ersatzlos streichen. Der Rueckgabewert von findOwned(tenantId, id) wird ab jetzt gebraucht — bisher wurde er verworfen. Unmittelbar vor dem bestehenden if (data.name !== undefined)-Block eine Pruefung einziehen: ist data.name !== undefined und traegt die geladene Gruppe einen gesetzten ldapObjectGuid, dann BadRequestException mit dem Hinweis, dass der Name einer aus dem Verzeichnis uebernommenen Gruppe dort gepflegt wird. Reihenfolge ist wichtig: die Sperre greift vor der Leerstring-Pruefung, damit ein gesperrter Name nicht mit der falschen Fehlermeldung antwortet.
Danach den internalName-Zweig: bei undefined nichts tun; bei null updateData.internalName = null; bei einem String den getrimmten Wert nehmen und, wenn er leer ist, ebenfalls null schreiben. Das ist der Grund, warum "gesetzt" spaeter ueber Inhalt statt ueber Truthiness eines Strings mit Leerzeichen entschieden werden kann — leere und nur aus Leerzeichen bestehende Werte erreichen die Datenbank nie.
Der bestehende P2002-Uebersetzungsblock bleibt unveraendert und bezieht sich weiterhin auf updateData.name; internalName traegt keinen Unique-Index und kann dort nicht auftreten.
In listForTenant() die Projektion um internalName: g.internalName erweitern. Die Sortierung bleibt bewusst auf name — die Konsequenz (Anzeige-Reihenfolge weicht bei gesetzten internen Namen von der alphabetischen Ordnung der Anzeigenamen ab) ist als backstop in must_haves festgehalten und wird nicht in diesem Plan aufgeloest.
Tests. In apps/api/src/groups/groups.service.spec.ts einen describe-Block GroupsService.update — Namenssperre und interner Name (D-03/D-04) ergaenzen, mit je einem Fall pro Zeile der <behavior>-Liste. Der Mock fuer findFirst muss dabei die geladene Gruppe samt ldapObjectGuid liefern — beim Erweitern der bestehenden Mocks darauf achten, dass findOwned weiterhin genau ein findFirst-Aufrufmuster erwartet und kein zweiter, inkompatibler Mock fuer dieselbe Methode entsteht.
cd apps/api && npx vitest run src/groups/groups.service.spec.ts
cd apps/api && npx tsc --noEmit
<acceptance_criteria>
- grep -c 'ldapDn' apps/api/src/groups/dto/update-group.dto.ts ergibt 0
- grep -q 'internalName' apps/api/src/groups/dto/update-group.dto.ts trifft
- awk '/async update\(/,/^ }$/' apps/api/src/groups/groups.service.ts | grep -c 'ldapObjectGuid' ist >= 1 (die Sperre liest die Spalte)
- awk '/async update\(/,/^ }$/' apps/api/src/groups/groups.service.ts | grep -c 'updateData.ldapDn' ergibt 0
- awk '/async listForTenant/,/^ }$/' apps/api/src/groups/groups.service.ts | grep -c 'internalName' ist >= 1
- cd apps/api && npx vitest run src/groups/groups.service.spec.ts ist gruen und der neue describe-Block enthaelt mindestens 8 it(-Faelle
- Ein Testfall belegt HTTP-400-Semantik: der Block referenziert BadRequestException mindestens einmal
</acceptance_criteria>
Der Name einer importierten Gruppe ist serverseitig gesperrt, internalName ist setz- und loeschbar mit Leerstring-Normalisierung auf null, listForTenant liefert das Feld aus, und UpdateGroupDto bietet keinen Weg mehr, eine AD-Bindung zu setzen.
- Die Mitgliedschafts-Query selektiert heute ausdruecklich nur
{ id: true, name: true }fuer die eingebundene Gruppe — ergaenzeinternalName: true. Ohne diese Ergaenzung ist das Feld zur Laufzeitundefinedund der Fallback greift stillschweigend immer aufname. DiegroupGrants-Query nutztinclude: { group: true }und liefert die Spalte bereits mit; hier ist keine Aenderung noetig, aber pruefe das beim Lesen gegen. - An der Projektionsstelle fuer die
viaGroups-Namen den gepushten Wert auf den internen Namen mit Rueckfall auf den gespeicherten Namen umstellen. - An der Projektionsstelle fuer die Mitgliedschafts-Chips dasselbe fuer das
name-Feld des zurueckgegebenen Objekts. Da die anschliessende Sortierung ueber genau dieses Feld laeuft, sortiert sie damit automatisch ueber den angezeigten Namen — das ist gewollt und der Unterschied zuGroupsService.listForTenant(), das weiterhin ueber die Datenbankspalte sortiert. - Verwende an beiden Stellen die Nullish-Variante, nicht die Oder-Variante — ein leerer String soll hier nicht stillschweigend zu
namezurueckfallen, sondern gar nicht erst ankommen (Task 2 normalisiert ihn zunull). Wuerde diese Schicht auf Truthiness pruefen, verstuende sie die Normalisierung nachtraeglich um.
Ergaenze in apps/api/src/groups/module-grants.service.spec.ts einen describe-Block ModuleGrantsService.getUserAccess — Anzeigename mit Fallback (D-04) mit je einem Fall fuer gesetzten und nicht gesetzten internen Namen, der beide Projektionen (groups[].name und modules[].viaGroups) in derselben Antwort prueft, sowie einem Fall, der die Sortierung ueber den angezeigten Namen belegt.
cd apps/api && npx vitest run src/groups/module-grants.service.spec.ts
cd apps/api && npx vitest run
<acceptance_criteria>
- grep -c 'internalName' apps/api/src/groups/module-grants.service.ts ist >= 3 (Select plus beide Projektionsstellen)
- awk '/async getUserAccess/,/^ }$/' apps/api/src/groups/module-grants.service.ts | grep -c '??' ist >= 2
- Keine Truthiness-Variante an den beiden Stellen: awk '/async getUserAccess/,/^ }$/' apps/api/src/groups/module-grants.service.ts | grep -c 'group.internalName ||' ergibt 0
- cd apps/api && npx vitest run src/groups/module-grants.service.spec.ts ist gruen mit mindestens 3 neuen Faellen
- cd apps/api && npx vitest run ist vollstaendig gruen
- Keine Datei unter apps/web/ wurde in diesem Task geaendert (UI-SPEC Surface Contract 6): git diff --name-only HEAD -- apps/web | wc -l ergibt 0
</acceptance_criteria>
Die Gruppen-Chips und die viaGroups-Namen im Benutzer-Detail zeigen den internen Namen, sobald einer gesetzt ist, und sonst den gespeicherten Namen — ohne eine einzige Zeile Frontend-Aenderung.
<threat_model>
Trust Boundaries
| Boundary | Description |
|---|---|
Browser → API (PATCH /groups/:id) |
Admin-gelieferter Body mit name, internalName, isDefault; die Gruppen-ID stammt aus dem Pfad |
| API → PostgreSQL | Schreibpfade auf Group unter RLS und unter dem partiellen Unique-Index Group_one_default_per_tenant |
STRIDE Threat Register
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|---|---|---|---|---|---|
| T-16-06 | Elevation of Privilege | GroupsService.update, reassignDefaultBeforeDelete |
high | mitigate | Jede Gruppen-Lookup-Query filtert zusaetzlich auf tenantId (bestehendes findOwned-Muster); reassignDefaultBeforeDelete nutzt bewusst eine eigene, nicht werfende Variante desselben Filters, verzichtet aber nicht auf den tenantId-Teil |
| T-16-07 | Tampering | Namenssperre in update() |
high | mitigate | Die Sperre ist eine Backend-Invariante (BadRequestException), nicht eine deaktivierte Eingabe im Browser — ein direkter API-Aufruf kommt an ihr nicht vorbei (RESEARCH.md Open Question 1) |
| T-16-04 | Tampering | Standardmarkierungs-Transaktion | medium | mitigate | Partieller Unique-Index Group_one_default_per_tenant als DB-seitiges zweites Netz; ein daraus resultierender P2002 wird als "hat sich schon jemand anderes gekuemmert" behandelt statt geworfen (Muster aus ensureDefaultGroup()) |
| T-16-08 | Information Disclosure | Ausgabe von internalName in listForTenant/getUserAccess |
low | accept | internalName ist ein admin-gepflegtes Anzeigefeld ohne Geheimnischarakter; es verlaesst den Mandanten nicht, weil beide Methoden bereits mandantengefiltert abfragen |
| T-16-SC | Tampering | Paketinstallation | low | accept | Keine neuen Pakete in diesem Plan |
| </threat_model> |
<success_criteria>
- Der Name einer importierten Gruppe kann ueber die API nicht geaendert werden (D-03)
internalNameist setz-, aenderbar und loeschbar, wird aufnullnormalisiert statt leer gespeichert (D-04)listForTenantliefertinternalName;getUserAccessliefert den Anzeigenamen mit FallbackreassignDefaultBeforeDeletesteht als deterministischer, nicht werfender Baustein fuer Plan 16-03 bereit (D-06)- Kein Codepfad setzt mehr eine AD-Bindung auf eine bestehende Gruppe (D-07, Backend-Haelfte) </success_criteria>