525f212e39
Plan, Zusammenfassung, Verifikation und STATE.md zum Quick-Vorgang 260921-fi3. Befund: auth.service legt mustChangePassword in den JWT, jwt.strategy liess das Feld beim Auspacken fallen. request.user.mustChangePassword war damit immer undefined und der global registrierte ForcePasswordChangeInterceptor hat seit seiner Einfuehrung nie blockiert. Durchgesetzt wurde der Zwangswechsel allein von der Web-Middleware; jeder Weg daran vorbei umging ihn. Gemessen: eine Sitzung mit mustChangePassword=true erhielt auf GET /users 200 samt vollstaendiger Benutzerliste. Behoben, und belegt bei gleicher Rolle und gleicher Route: GET /modules/active liefert 403 FORCE_PASSWORD_CHANGE mit Zwang und 200 ohne. Neue Spezifikationen gegen den alten Stand 6 von 12 rot, danach 12 von 12 gruen, vom Verifier unabhaengig nachgestellt. Der Browser-Ablauf wurde vollstaendig durchgespielt: niemand wird ausgesperrt, der Wechsel gelingt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPPB4ApQxzSU1rwV2Ffj9J
232 lines
16 KiB
Markdown
232 lines
16 KiB
Markdown
---
|
|
phase: quick-260921-fi3
|
|
plan: 01
|
|
subsystem: auth
|
|
tags: [nestjs, jwt, passport, interceptor, next-intl, react, vitest]
|
|
|
|
requires:
|
|
- phase: quick-260921-bi2
|
|
provides: Lint-Rueckstand abgebaut, Befunde 1-3 aus dieser Aufgabe entdeckt
|
|
provides:
|
|
- Erzwungener Passwortwechsel wird jetzt an der API selbst durchgesetzt (nicht nur im Browser)
|
|
- Erlaubnisliste des Abfangers vergleicht Methode UND Pfad exakt statt Teilstring
|
|
- VehicleTable: Loeschknopf gegen Doppelausloesung gesperrt
|
|
- VehicleTable: alle sichtbaren Texte und Vorlesehilfen ueber next-intl
|
|
affects: [auth, dkv-fleet]
|
|
|
|
actuals:
|
|
tokens: 6512
|
|
tasks: 3
|
|
commits: 2
|
|
plan_head_before: 116041b7fd455ffac6e43d36d5a08aa156a90690
|
|
|
|
tech-stack:
|
|
added: []
|
|
patterns:
|
|
- "Global-Interceptor-Erlaubnisliste als eingefrorenes Array von {method, path}-Objekten mit exaktem Abgleich statt Teilstring-Vergleich"
|
|
- "Reentrancy-Sperre fuer asynchrone Lösch-/Speicheraktionen im Zustand selbst (isDeleting), nicht nur ueber das disabled-Attribut in der Darstellung"
|
|
|
|
key-files:
|
|
created:
|
|
- apps/api/src/auth/strategies/jwt.strategy.spec.ts
|
|
- apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts
|
|
modified:
|
|
- apps/api/src/auth/strategies/jwt.strategy.ts
|
|
- apps/api/src/auth/interceptors/force-password-change.interceptor.ts
|
|
- apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.tsx
|
|
- apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.test.tsx
|
|
- apps/web/src/messages/de.json
|
|
- apps/web/src/messages/en.json
|
|
|
|
key-decisions:
|
|
- "JwtStrategy.validate liefert mustChangePassword jetzt durch (strenger Vergleich mit true); fehlender Anspruch (Alt-Sitzung) ergibt false, keine Aussperrwelle"
|
|
- "Erlaubnisliste des Abfangers auf exakten Abgleich von Methode UND normalisiertem Pfad umgestellt statt Teilstring-Vergleich (heute nicht ausnutzbar, Absicherung gegen kuenftige Routen)"
|
|
- "load() in VehicleTable behaelt bewusst leeres Abhaengigkeitsfeld ([]) statt [t] — ein instabiler Uebersetzer-Mock haette sonst einen Abruf-bei-jedem-Render-Zyklus ausgeloest"
|
|
- "vi.restoreAllMocks() im Testabbau von VehicleTable.test.tsx durch vi.clearAllMocks() ersetzt — restoreAllMocks leerte die Aufrufzaehlung reiner vi.fn()-Mocks nicht"
|
|
|
|
patterns-established:
|
|
- "Nahttests, die JwtStrategy.validate() und einen nachgelagerten Guard/Interceptor zusammenfuehren, um Naht-Regressionen wie das stille Verlieren eines Claims zu pinnen"
|
|
|
|
requirements-completed: [SEC-FORCE-PW, UI-DKV-DELETE, I18N-DKV]
|
|
|
|
coverage:
|
|
- id: D1
|
|
description: "Sitzung mit mustChangePassword=true erhaelt an der API auf GET /users und GET /modules/active 403 (statt vorher 200)"
|
|
requirement: SEC-FORCE-PW
|
|
verification:
|
|
- kind: unit
|
|
ref: "apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts — Nahttest + Erlaubnisliste + Teilstring-Falle"
|
|
status: pass
|
|
- kind: integration
|
|
ref: "curl gegen localhost:3001 nach docker compose up -d --build api: GET /users -> 403, GET /modules/active -> 403"
|
|
status: pass
|
|
human_judgment: false
|
|
- id: D2
|
|
description: "Dieselbe Sitzung erreicht GET /auth/me (200), POST /auth/logout (200) und POST /auth/change-password (401, Fachfehler statt 403) weiterhin"
|
|
requirement: SEC-FORCE-PW
|
|
verification:
|
|
- kind: integration
|
|
ref: "curl gegen localhost:3001: GET /auth/me -> 200, POST /auth/change-password -> 401, POST /auth/logout -> 200"
|
|
status: pass
|
|
human_judgment: false
|
|
- id: D3
|
|
description: "Nahttest scheitert nachweislich gegen den alten Quelltext (RED), gruen nach der Aenderung (GREEN)"
|
|
requirement: SEC-FORCE-PW
|
|
verification:
|
|
- kind: unit
|
|
ref: "vitest run vor der Aenderung: 6 von 12 Faellen rot (Nahttest, Falsche-Methode-Fall, Teilstring-Falle in force-password-change.interceptor.spec.ts + alle 3 in jwt.strategy.spec.ts); danach 12/12 gruen"
|
|
status: pass
|
|
human_judgment: false
|
|
- id: D4
|
|
description: "Zweiter Klick auf die Bestaetigungsschaltflaeche des Loeschdialogs loest kein zweites DELETE aus"
|
|
requirement: UI-DKV-DELETE
|
|
verification:
|
|
- kind: unit
|
|
ref: "VehicleTable.test.tsx#double-clicking the delete confirm button triggers exactly one deleteVehicle call"
|
|
status: pass
|
|
human_judgment: false
|
|
- id: D5
|
|
description: "Alle sichtbaren Texte und Vorlesehilfen der Fahrzeugtabelle stammen aus de.json/en.json, gleicher Schluesselsatz in beiden Sprachen"
|
|
requirement: I18N-DKV
|
|
verification:
|
|
- kind: unit
|
|
ref: "src/messages/umlaut-guard.spec.ts, src/messages/tenderRadar-parity.spec.ts (Muster fuer Schluesselgleichheit) + node-Einzeiler fuer dkvFleet-Schluesselgleichheit (87 Schluessel deckungsgleich)"
|
|
status: pass
|
|
- kind: other
|
|
ref: "grep-Strukturtore: 0 Fehlertext-Literale an Setzern, 0 aria-label-Literale in VehicleTable.tsx"
|
|
status: pass
|
|
human_judgment: false
|
|
|
|
duration: ~55min
|
|
completed: 2026-09-21
|
|
status: complete
|
|
---
|
|
|
|
# Quick 260921-fi3: Erzwungener Passwortwechsel wirklich durchgesetzt Summary
|
|
|
|
**JwtStrategy liess mustChangePassword auf dem Weg zum Interceptor fallen — der global registrierte ForcePasswordChangeInterceptor hat seither nie etwas blockiert; jetzt durchgesetzt und mit einem Nahttest gepinnt, der nachweislich gegen den alten Quelltext scheitert.**
|
|
|
|
## Performance
|
|
|
|
- **Dauer:** ca. 55 Minuten
|
|
- **Abgeschlossen:** 2026-09-21
|
|
- **Aufgaben:** 3/3
|
|
- **Geänderte Dateien:** 8
|
|
|
|
## Accomplishments
|
|
|
|
- Die eigentliche Ursache behoben: `JwtStrategy.validate` reicht `mustChangePassword` jetzt durch (strenger Vergleich mit `true`), statt dass der Interceptor auf ein Feld prüft, das nie ankam.
|
|
- Die Erlaubnisliste des Interceptors von Teilstring-Vergleich auf exakten Abgleich von Methode **und** Pfad umgestellt — schließt die Teilstring-Falle, ohne dass es heute eine ausnutzbare Route dafür gäbe.
|
|
- Am laufenden System nachgewiesen: eine Sitzung mit gesetzter Kennzeichnung bekommt jetzt `403` auf `/users` und `/modules/active` (vorher `200`), während `/auth/me`, `/auth/change-password` und `/auth/logout` — die drei Wege, die der Zwangswechsel im Browser tatsächlich braucht — unverändert erreichbar bleiben.
|
|
- `VehicleTable`: das bisher ungelesene Beschäftigt-Kennzeichen (`const [, setIsDeleting]`) wieder lesbar gemacht, Dialogschaltflächen währenddessen gesperrt, `confirmDelete` bricht bei bereits laufender Löschung selbst ab — ein Doppelklick löst nachweislich nur ein `DELETE` aus.
|
|
- Alle sieben fest verdrahteten Texte und sechs Vorlesehilfen der Fahrzeugtabelle jetzt über next-intl, sieben neue Schlüssel im Bereich `dkvFleet`, deckungsgleich in `de.json` und `en.json`.
|
|
|
|
## Task Commits
|
|
|
|
Jede Aufgabe wurde atomar committet:
|
|
|
|
1. **Aufgabe 1: Erzwungenen Passwortwechsel an der API wirklich durchsetzen** - `f7c02b7` (fix)
|
|
2. **Aufgabe 2: Fahrzeugtabelle — Doppelauslösung sperren, next-intl** - `e56cce4` (fix)
|
|
3. **Aufgabe 3: Gesamtabnahme** - keine eigene Quelldatei-Änderung (reine Abnahme), Ergebnisse unten dokumentiert; fließt in den Abschluss-Commit dieses Berichts.
|
|
|
|
_Beide Aufgaben mit Code-Änderung liefen TDD (`tdd="true"`): Testdateien zuerst, RED-Lauf gegen den alten Quelltext protokolliert, dann Quelltext angepasst bis GREEN._
|
|
|
|
## Files Created/Modified
|
|
|
|
- `apps/api/src/auth/strategies/jwt.strategy.ts` - `validate` reicht `mustChangePassword` jetzt durch (strenger Vergleich mit `true`)
|
|
- `apps/api/src/auth/strategies/jwt.strategy.spec.ts` - neu; pinnt Durchreichung, Alt-Sitzungs-Fallback, unveränderte Feldweitergabe
|
|
- `apps/api/src/auth/interceptors/force-password-change.interceptor.ts` - Erlaubnisliste auf exakten Methode+Pfad-Abgleich umgestellt, Kopfkommentar korrigiert
|
|
- `apps/api/src/auth/interceptors/force-password-change.interceptor.spec.ts` - neu; Nahttest, Erlaubnisliste, Teilstring-Falle, Kennzeichnung nicht gesetzt, öffentliche Route, kein `request.user`
|
|
- `apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.tsx` - Reentrancy-Sperre gegen Doppelklick, alle Texte/Vorlesehilfen über next-intl
|
|
- `apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.test.tsx` - drei neue Testfälle, Testabbau-Fehler behoben (`vi.clearAllMocks()`)
|
|
- `apps/web/src/messages/de.json` - sieben neue Schlüssel im Bereich `dkvFleet` (`form.saveRow`, `form.deleteConfirm`, fünf `errors.*`)
|
|
- `apps/web/src/messages/en.json` - dieselben sieben Schlüssel, englische Übersetzung
|
|
|
|
## Decisions Made
|
|
|
|
- `load()` in `VehicleTable` behält bewusst `[]` als Abhängigkeitsfeld statt `[t]`: der next-intl-Mock im Test erzeugt bei jedem Aufruf von `useTranslations` eine neue Funktionsidentität; hätte `load` von `t` abgehangen, wäre `load` bei jedem Render neu erzeugt worden und der `useEffect`, der `load` beim Mount aufruft, hätte bei jedem Render erneut ausgelöst — ein Abruf-Loop, der in der Praxis (echtes next-intl memoisiert `t`) nicht auftritt, aber im Test sofort sichtbar wurde, weil er den Mock-Warteschlangenzustand für spätere Testfälle leerte.
|
|
- `vi.restoreAllMocks()` im Testabbau von `VehicleTable.test.tsx` durch `vi.clearAllMocks()` ersetzt: `restoreAllMocks` leert die Aufrufzählung reiner `vi.fn()`-Mocks (ohne echtes Original) nicht zuverlässig, wodurch der neue Doppelklick-Test (`toHaveBeenCalledTimes(1)`) eine aus dem vorigen Testfall übertragene Zählung sah. Dieser Fund war vorher unsichtbar, weil bisher kein Test die Aufrufzahl prüfte, nur `toHaveBeenCalledWith(...)`.
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 1 - Bug] `load()` mit `t` als Abhängigkeit hätte einen Abruf-bei-jedem-Render-Zyklus ausgelöst**
|
|
- **Gefunden während:** Aufgabe 2, beim ersten vollen Testlauf der Datei (nicht isoliert)
|
|
- **Problem:** Beim Verdrahten der Ladefehlermeldung über `t('errors.loadVehiclesFailed')` wurde `t` naheliegend in `load`s Abhängigkeitsfeld aufgenommen (`[t]`). Da der next-intl-Mock im Test bei jedem Aufruf eine neue Funktion zurückgibt, wurde `load` bei jedem Render neu erzeugt, und der `useEffect`, der `load` beim Mount aufruft, löste dadurch bei jedem Render erneut aus — ein bestehender, vorher grüner Testfall ("clicking pencil icon...") schlug dadurch fehl, weil die Mock-Warteschlange eines späteren Tests durch die vielen zusätzlichen Aufrufe verzerrt wurde.
|
|
- **Fix:** Abhängigkeitsfeld auf `[]` zurückgesetzt (wie im Ausgangszustand), mit Kommentar, der begründet, warum `t` hier bewusst fehlt.
|
|
- **Dateien geändert:** `apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.tsx`
|
|
- **Verifikation:** Vollständiger Testlauf der Datei wieder grün (8/8)
|
|
- **Committed in:** `e56cce4` (Teil des Aufgabe-2-Commits)
|
|
|
|
**2. [Rule 1 - Bug] `vi.restoreAllMocks()` leerte die Aufrufzählung von `mockDeleteVehicle` nicht zwischen Testfällen**
|
|
- **Gefunden während:** Aufgabe 2, beim Verifizieren des neuen Doppelklick-Testfalls
|
|
- **Problem:** Der neue Testfall erwartete `mockDeleteVehicle` genau einmal aufgerufen; tatsächlich zeigte er zwei Aufrufe. Debugging (temporäres `console.log` in `confirmDelete`, per Backup-Datei wieder entfernt) zeigte: `confirmDelete` wurde in diesem Testfall nur einmal ausgeführt — der zweite gezählte Aufruf war aus dem vorigen Testfall ("clicking trash icon...") übrig geblieben, weil `vi.restoreAllMocks()` im Testabbau die Aufrufliste reiner `vi.fn()`-Mocks nicht zurücksetzt.
|
|
- **Fix:** `vi.restoreAllMocks()` durch `vi.clearAllMocks()` ersetzt, mit erklärendem Kommentar.
|
|
- **Dateien geändert:** `apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.test.tsx`
|
|
- **Verifikation:** Vollständiger Testlauf der Datei grün (8/8), Doppelklick-Test zählt jetzt korrekt genau einen Aufruf
|
|
- **Committed in:** `e56cce4` (Teil des Aufgabe-2-Commits)
|
|
|
|
---
|
|
|
|
**Gesamtzahl Abweichungen:** 2 automatisch behoben (beide Regel 1 — Fehlerkorrektur, beide beim Testen der eigenen neuen Testfälle in Aufgabe 2 gefunden, nicht im Ausgangscode)
|
|
**Auswirkung auf den Plan:** Beide Korrekturen waren nötig, damit die vom Plan geforderten neuen Testfälle tatsächlich das prüfen, was sie behaupten zu prüfen. Kein Umfang über den Plan hinaus.
|
|
|
|
## Issues Encountered
|
|
|
|
Keine offenen Probleme. Die beiden oben dokumentierten Deviations sind bereits gelöst.
|
|
|
|
## User Setup Required
|
|
|
|
Keine — keine externe Dienstkonfiguration nötig.
|
|
|
|
## Gesamtabnahme (Aufgabe 3)
|
|
|
|
### Fünf Zahlen vorher/nachher
|
|
|
|
| Kennzahl | Ausgangsstand | Nachher | Erwartung erfüllt? |
|
|
|---|---|---|---|
|
|
| `pnpm type-check` | 4/4 | 4/4 | ja |
|
|
| `pnpm test` apps/api | 69 Dateien / 1124 Tests | 71 Dateien / 1136 Tests | ja (+2 Dateien, +12 Tests aus Aufgabe 1) |
|
|
| `pnpm test` apps/web | 66 Dateien / 459 Tests | 66 Dateien / 462 Tests | ja (+3 Tests aus Aufgabe 2, keine neue Datei) |
|
|
| `pnpm lint` | 5/5, 0 Fehlerstufe | 5/5, 0 Fehlerstufe | ja |
|
|
| Warnungssumme | 464 (api 357, web 107) | 466 (api 358, web 108) | ja — genau an der erlaubten Obergrenze von 466, nicht darüber |
|
|
|
|
Die zwei zusätzlichen Warnungen sind identifiziert und bewusst belassen (keine bestehende Unterdrückungs-Konvention im Projekt — es gibt an keiner Stelle in apps/api oder apps/web einen `biome-ignore`-Kommentar, das Projekt führt Lint-Warnungen stattdessen als gezählten Rückstand):
|
|
- `apps/api/src/auth/interceptors/force-password-change.interceptor.ts:33` — `lint/suspicious/noExplicitAny` an der neuen `normalizePath(request: any)`-Hilfsfunktion. Das bestehende Muster derselben Datei (`intercept(...): Observable<any>`, unverändert) verwendet ebenfalls `any` für den Request/Response-Rahmen von NestJS-Interceptoren.
|
|
- `apps/web/src/app/(portal)/modules/dkv-fleet/settings/components/VehicleTable.tsx:182` — `lint/correctness/useExhaustiveDependencies` an `load`, weil `t` absichtlich aus dem Abhängigkeitsfeld ausgeschlossen ist (siehe Decisions Made oben — mit `t` in den Abhängigkeiten löst der next-intl-Mock im Test einen Abruf-Loop aus).
|
|
|
|
### HTTP-Messung aus Aufgabe 1
|
|
|
|
| Route | Methode | Ergebnis mit gesetzter Kennzeichnung | Bewertung |
|
|
|---|---|---|---|
|
|
| `/users` | GET | 403 | erwartet — vorher 200 |
|
|
| `/modules/active` | GET | 403 | erwartet — vorher ungeprüft, jetzt durchgesetzt |
|
|
| `/auth/me` | GET | 200 | erwartet — Erlaubnisliste greift |
|
|
| `/auth/change-password` | POST | 401 | erwartet — Fachfehler des Handlers (falsches aktuelles Passwort), NICHT 403; beweist, dass die Erlaubnisliste durchlässt und der Handler selbst entscheidet |
|
|
| `/auth/logout` | POST | 200 | erwartet — Erlaubnisliste greift |
|
|
|
|
Nach der Messung wurde `mustChangePassword` für `admin` in der Datenbank wieder auf `false` gesetzt.
|
|
|
|
### Befund 3 (SplitTab.tsx) — Begründung in einem Satz
|
|
|
|
`SplitTab.tsx` bleibt unangetastet: `downloadAllAsZip` liegt auf Modulebene und kann den next-intl-Hook nicht aufrufen, ein übersetzter Downloadname bringt echte Nachteile (Umlaute auf Windows-Freigaben, sprachabhängige Anhangsnamen) ohne Nutzen (ein Dateiname wird nie vorgelesen oder gesucht), und die Dateien im Archiv tragen ohnehin die vom Server gelieferten Zertifikatsnamen.
|
|
|
|
### Datenbank-Endstand
|
|
|
|
```
|
|
admin|f
|
|
nutzer2|f
|
|
nutzer1|f
|
|
```
|
|
|
|
Alle drei Nutzer stehen wieder auf dem Ausgangsstand — kein Nutzer trägt die Zwangswechsel-Kennzeichnung.
|
|
|
|
## Next Phase Readiness
|
|
|
|
Kein Folge-Plan in dieser Kette. Die drei Befunde aus der Lint-Rückstandsaufgabe 260921-bi2 sind damit vollständig abgearbeitet: Befund 1 (Sicherheitslücke) behoben und mit einem RED→GREEN-Nahttest gepinnt, Befund 2 (Doppelauslösung/next-intl) behoben, Befund 3 (SplitTab.tsx-Dateiname) bewusst nicht angefasst, begründet oben und im Plan.
|
|
|
|
## Self-Check: PASSED
|
|
|
|
Alle acht geänderten/neu erstellten Quelldateien sowie diese Zusammenfassung wurden auf Existenz geprüft (`FOUND` für jede). Beide Task-Commits (`f7c02b7`, `e56cce4`) wurden in `git log --oneline --all` gefunden.
|