diff --git a/CHANGELOG.md b/CHANGELOG.md index 238027b..5ff8691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,44 @@ und dieses Projekt folgt [Semantic Versioning](https://semver.org/lang/de/). ### Behoben +- **Der Link in der Mail „Verzeichnis-Abgleich fehlgeschlagen“ zeigte ins Leere.** Gebaut wurde + `/verwaltung/verzeichnis`; `verwaltung` ist aber der Name des Rechts, die Seite liegt unter + `/admin/verzeichnis`. Das ist die eine Meldung, die an alle Administratoren geht, und zwar im + Moment eines Ausfalls — der Empfänger klickte und landete auf der Nicht-gefunden-Seite. Ein Test + prüft jetzt jeden Pfad einer Meldung gegen die vorhandenen Routen (#036) +- **Ein offener Ereignisstrom überlebte Abmelden, Deaktivieren und Sitzungswiderruf.** Das Recht + wurde genau einmal geprüft, beim Aufbau des Stroms. Ein Administrator deaktivierte ein Konto, und + eine offen gelassene Lasche bekam weiter live Status und Schrittzustände dieses einen Laufs, bis + jemand sie schloss. Der Herzschlag prüft die Sitzung jetzt alle 25 Sekunden nach; eine + Herabstufung während des Stroms engt ihn ein, statt ihn zu beenden (#036) +- **Der Export eines Entwurfs mit Baustein lieferte einen leeren Abschnitt.** Ein Baustein-Abschnitt + bekommt seine Schritte erst beim Veröffentlichen; wer eine noch unveröffentlichte Vorlage in eine + zweite Installation brachte, fand den geteilten Abschnitt dort unter dem Namen „wird ersetzt“ und + ohne einen einzigen Schritt. Beim Export werden Bausteine jetzt aufgelöst (#036) +- **`docs/betrieb/sicherung.md` nannte für den nativen Weg einen Anhänge-Pfad, den es nirgends + gibt** — als Rückfallwert in einem Skript, in dem ein `rm -rf` steht. Auf einer Installation ohne + `ANHAENGE_PFAD` in der Env-Datei wäre in ein Verzeichnis zurückgespielt worden, das die Anwendung + nie liest: „Rücksicherung gelaufen“, und danach fehlt jede Datei. Jetzt bricht der Befehl ab, + statt zu raten (#036) +- **`README.md` kannte die Rolle `MITARBEITER` nicht** und behauptete weiter, unbekannte Benutzer + würden beim ersten Login angelegt. Seit #035 wird jemand ohne Rollengruppe über seinen UPN + zugeordnet oder abgewiesen. Wer auf v0.2.0 hebt und nur das README liest, hätte genau diese Leute + ausgesperrt (#036) +- **Der Protokolleintrag „Anmeldung abgewiesen: UPN mehrdeutig“ nannte immer zwei Treffer**, auch bei + sieben. Wer das Protokoll liest, um den Fall aufzulösen, braucht die wirkliche Zahl (#036) +- **`anzahlSchritte` im Protokoll eines neuen Laufs zählte die Schritte der Version**, nicht die des + Laufs — bei einer Vorlage mit abteilungsgebundenen Abschnitten also zu viele (#036) +- Die Attributzuordnung einer Verzeichnisquelle wird jetzt **im Dienst** geprüft und nicht nur im + Formular. Ein Attributname mit Komma und `$filter` kam zuvor durch und schrieb einen zweiten + `$filter` in die Graph-Abfrage (#036) +- Der Graph-Seitenabruf folgt `@odata.nextLink` nur noch, wenn die Adresse auf denselben Host zeigt + wie `GRAPH_API_URL`. Jede Seite wird mit einem mandantenweit gültigen Token geholt (#036) +- **Die Ende-zu-Ende-Abläufe räumen auf, was sie anlegen** — zentral, vor und nach dem Lauf. In der + Entwicklungsdatenbank standen 105 Vorlagen aus früheren Durchgängen. Ein struktureller Test + schlägt an, wenn ein neuer Ablauf das Präfix nicht benutzt (#027) +- Vier Ende-zu-Ende-Abläufe waren seit #020, #030, #031 beziehungsweise #033 rot, ohne dass es + auffiel: `pnpm test:e2e` steht in keinem Tor. Alle vier behoben, die Lücke ist als #038 benannt + (#027) - Der Verbindungstest einer Verzeichnisquelle unterschied „der Filter findet niemanden“ nicht mehr von „gelesen, aber nichts zuordenbar“ — zwei verschiedene Ursachen, die den Administrator an verschiedene Stellen schicken. Die Unterscheidung war in #028 eingebaut worden, aber durch eine diff --git a/README.md b/README.md index 5b0f130..ec702ab 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ Checklisten für wiederkehrende IT-Arbeiten. Versionierte Vorlagen, daraus gesta Team abgearbeitet, abgeschlossen und unveränderlich archiviert — mit Historie je Gerät und je Mitarbeiter. Einstieg über QR-Aufkleber: Code scannen, Geräteseite öffnen, loslegen. -> **Stand:** in Entwicklung auf `v0.1.0` (MVP). Der Fortschritt steht in -> [`docs/issues/README.md`](docs/issues/README.md), Entscheidungen im +> **Stand:** `v0.1.0` ist veröffentlicht, Phase 2 liegt auf `main` und wird `v0.2.0`. Der +> Fortschritt steht in [`docs/issues/README.md`](docs/issues/README.md), Entscheidungen im > [Entwicklungstagebuch](docs/devlog.md). ## Zwei Betriebsmodi @@ -158,6 +158,19 @@ zugeordnet. Unbekannte Benutzer werden beim ersten Login angelegt. | `REDAKTEUR` | Vorlagen anlegen, bearbeiten, veröffentlichen; alles was `TECHNIKER` darf | | `TECHNIKER` | Läufe starten, abarbeiten, abschließen, archivieren; Geräte pflegen | | `LESER` | nur lesen | +| `MITARBEITER` | **nur die eigenen Geräte und deren Läufe** — ohne Schritte, Notizen und Kommentare | + +**`MITARBEITER` hat keine eigene Gruppe, und das ist Absicht** (seit #035). Wer sich anmeldet und in +keiner der vier abgebildeten Gruppen steht, bekommt **nicht mehr** `LESER` — das hieße Lesezugriff +auf alle Läufe des Hauses. Stattdessen versucht Laufzettel, ihn über seinen UPN einer Person im +Verzeichnis zuzuordnen: klappt das, bekommt er die Selbstauskunft; klappt es nicht, wird die +Anmeldung mit einer Begründung abgewiesen. + +Das greift **nur, wenn Rollengruppen konfiguriert sind** — ohne `OIDC_GROUP_*` bleibt alles wie +vorher, denn ohne abgebildete Gruppen kann die Anwendung nicht unterscheiden, ob jemand Techniker +oder Mitarbeiter ist. Wer das alte Verhalten für bestimmte Leute braucht, trägt sie in die +Leser-Gruppe ein. Einzelheiten in +[`docs/betrieb/authentik.md`](docs/betrieb/authentik.md). Die Anbindung an authentik ist in [`docs/betrieb/authentik.md`](docs/betrieb/authentik.md) Schritt für Schritt beschrieben. diff --git a/docs/betrieb/sicherung.md b/docs/betrieb/sicherung.md index 0a3e38d..4ce65a5 100644 --- a/docs/betrieb/sicherung.md +++ b/docs/betrieb/sicherung.md @@ -17,7 +17,7 @@ Drei Dinge, und nur diese drei: |---|---|---| | **1** | **Die Datenbank.** Läufe, Vorlagen, Schrittergebnisse, Kommentare, Geräte, Personen, Tag-Codes, Protokoll. | `pg_dump`, siehe unten | | **2** | **Die Geheimnisse.** `APP_SECRET` und das Datenbank-Passwort. | Compose: die `.env` · Nativ: `/etc/laufzettel/laufzettel.env` | -| **3** | **Die Anhänge.** Die Dateien und Fotos an den Schritten. | Volume `laufzettel_anhaenge` · Nativ: `ANHAENGE_PFAD` | +| **3** | **Die Anhänge.** Die Dateien und Fotos an den Schritten. | Volume `laufzettel_anhaenge` · Nativ: `ANHAENGE_PFAD`, von `install.sh` auf `/var/lib/laufzettel/anhaenge` gesetzt | > **Die Anhänge sind seit #032 dabei, und sie sind die Falle.** In der Datenbank stehen nur ihre > **Metadaten** — Name, Typ, Größe, Prüfsumme. Die Dateien selbst liegen im Dateisystem. Eine @@ -157,11 +157,23 @@ sudo -u postgres pg_dump -Fc laufzettel > /var/backups/laufzettel-$(date +%F).du Und die Anhänge dazu — gewöhnliche Dateien, also `tar`: ```bash -# Der Pfad steht in ANHAENGE_PFAD; ohne Angabe liegt er unter /opt/laufzettel/daten/anhaenge. -ANHAENGE=$(sudo sh -c '. /etc/laufzettel/laufzettel.env; echo "${ANHAENGE_PFAD:-/opt/laufzettel/daten/anhaenge}"') +# Der Pfad steht in ANHAENGE_PFAD; `deploy/install.sh` setzt ihn auf +# /var/lib/laufzettel/anhaenge. **Kein Rückfallwert** – siehe den Kasten darunter. +ANHAENGE=$(sudo sh -c '. /etc/laufzettel/laufzettel.env; echo "${ANHAENGE_PFAD:?ANHAENGE_PFAD fehlt in /etc/laufzettel/laufzettel.env}"') sudo tar czf /var/backups/laufzettel-anhaenge-$(date +%F).tar.gz -C "$ANHAENGE" . ``` +> **Warum hier kein `${ANHAENGE_PFAD:-…}` steht.** Es stand einmal eines, und es war falsch: +> `/opt/laufzettel/daten/anhaenge` passt zu keiner Installation. `deploy/install.sh` setzt +> `/var/lib/laufzettel/anhaenge`, und ohne die Variable nimmt die Anwendung `./daten/anhaenge` +> **relativ zu ihrem Arbeitsverzeichnis** `/opt/laufzettel/.next/standalone` — also noch etwas +> anderes. +> +> Ein geratener Pfad ist beim Sichern eine leere Datei und beim Zurückspielen ein Schaden: dort +> steht ein `rm -rf` davor. Fehlt die Variable, soll der Befehl **abbrechen** und nicht raten. +> Deshalb `:?` und nicht `:-`. Gefunden bei der Durchsicht von Phase 2, nachdem derselbe Fehler in +> dieser Datei schon einmal Daten gekostet hätte (#032). + Und das Geheimnis, einmalig und in den Passwortmanager: ```bash @@ -187,7 +199,10 @@ sudo -u postgres pg_restore -d laufzettel --no-owner /var/backups/laufzettel-202 # 4. Die Anhänge zurück – **vorher leeren**, sonst bleiben Dateien liegen, auf # die keine Zeile mehr zeigt -ANHAENGE=$(sudo sh -c '. /etc/laufzettel/laufzettel.env; echo "${ANHAENGE_PFAD:-/opt/laufzettel/daten/anhaenge}"') +# Ohne ANHAENGE_PFAD bricht das hier ab – und zwar **vor** dem rm -rf. Ein +# geratener Pfad hiesse: das echte Verzeichnis bleibt leer, die Anwendung meldet +# „Die Datei zu … fehlt im Dateisystem“, und niemand weiss, warum. +ANHAENGE=$(sudo sh -c '. /etc/laufzettel/laufzettel.env; echo "${ANHAENGE_PFAD:?ANHAENGE_PFAD fehlt in /etc/laufzettel/laufzettel.env}"') sudo rm -rf "$ANHAENGE" sudo install -d -o laufzettel -g laufzettel -m 0750 "$ANHAENGE" sudo tar xzf /var/backups/laufzettel-anhaenge-2026-09-24.tar.gz -C "$ANHAENGE" @@ -247,7 +262,9 @@ runuser -u postgres -- pg_dump -Fc laufzettel > "/var/backups/laufzettel-$tag.du # shellcheck source=/dev/null . /etc/laufzettel/laufzettel.env -anhaenge="${ANHAENGE_PFAD:-/opt/laufzettel/daten/anhaenge}" +# Kein Rueckfallwert: ein geratener Pfad ergibt eine leere Sicherung, und die +# faellt erst beim Zuruckspielen auf. +anhaenge="${ANHAENGE_PFAD:?ANHAENGE_PFAD fehlt in /etc/laufzettel/laufzettel.env}" if [ -d "$anhaenge" ]; then tar czf "/var/backups/laufzettel-anhaenge-$tag.tar.gz" -C "$anhaenge" . fi diff --git a/docs/devlog.md b/docs/devlog.md index 485e382..536021e 100644 --- a/docs/devlog.md +++ b/docs/devlog.md @@ -36,12 +36,14 @@ als Unterpunkte mit Aufgabe und Dauer. | #035 | M22 – Mitarbeiter-Selbstauskunft | 2 h 50 min | | #028 | M15 – Quellenarten und Microsoft-Graph-Quelle | 4 h 10 min | | #029 | M16 – SCIM-Endpunkt als Alternative zum Abgleich | 4 h 05 min | -| | **Summe** | **64 h 40 min** | +| #036 | Reviewbefunde aus der Durchsicht von Phase 2 | 3 h 40 min | +| #027 | Die Editor-Abläufe lassen ihre Vorlagen stehen | 2 h 10 min | +| | **Summe** | **70 h 30 min** | | Meilenstein | Tatsächlich | Status | |---|---|---| | v0.1.0 | 35 h 20 min | veröffentlicht | -| v0.2.0 | 29 h 20 min | in Arbeit | +| v0.2.0 | 35 h 10 min | in Arbeit | --- @@ -1531,3 +1533,81 @@ stehen in `docs/betrieb/scim.md`. fremden Blick auf `main`, und diese Sitzung hat wieder zwei Dinge gezeigt, die eine grüne Suite nicht sieht: einen Schalter, der eine Änderung verschluckte, und einen Zweig, der nie erreicht wurde. Beide Befunde kamen aus Gegenproben, nicht aus Tests. + + +--- + +## 2026-10-11 · Sitzung 23 · 21:50–01:30 + +**Issues:** #036 (Reviewbefunde Phase 2), #027 (E2E-Reste) +**Version:** 0.2.0 (unveröffentlicht) + +### Erledigt + +- **Der unabhängige Review ist gelaufen** — acht Meilensteine (M15–M22) lagen ohne fremden Blick auf + `main`, und das stand seit Sitzung 21 als „überfällig“ im Devlog. Ein Agent mit eigener Datenbank, + der den Code nicht geschrieben hat, gegen die Akzeptanzkriterien und gegen `CLAUDE.md`. +- **Dreizehn Befunde, zwölf behoben**, zwei als #037 und #038 benannt. Alle lagen am Rand: zwei + Route Handler, ein Export-Pfad, Betriebsdokumentation, Kommentare. **Die Kernregeln halten** — + fünf von fünf Gegenproben gegen Versionierung, Mandantengrenze, Abschlussregel und + Selbstauskunft wurden rot. +- **#027 abgeschlossen**: die 105 Altreste sind weg, geräumt wird jetzt zentral vor und nach dem + Lauf, und ein struktureller Test bewacht es. +- 1454 Vitest-Tests (vorher 1432), 113 E2E-Abläufe grün. + +### Entscheidungen + +- **Eine Liste, die nur in einer Richtung geprüft wird, läuft dem Code hinterher.** `/api/scim` kam + mit #029 in die Proxy-Allowlist und stand in keinem Test; die Zeile ließ sich entfernen, ohne + dass etwas rot wurde. `OEFFENTLICH` ist jetzt exportiert, und der Test vergleicht **beide** + Richtungen: wer die Allowlist erweitert, muss es aufschreiben. Dasselbe Muster wie die Zahlen in + `CLAUDE.md`, die dreimal der Wirklichkeit nachliefen. +- **Ein Rückspielskript rät nicht.** In `sicherung.md` stand `${ANHAENGE_PFAD:-/opt/…}` — ein + Rückfallwert, der zu keiner Installation passte, in einem Skript mit `rm -rf`. Jetzt `:?`: fehlt + die Variable, bricht es ab. Ein geratener Pfad ist beim Sichern eine leere Datei und beim + Zurückspielen ein Schaden. +- **Die Zugweite eines Sortiertests wird gemessen, nicht gesetzt.** Eine feste Pixelzahl ist eine + Zusage über das Layout, die niemand pflegt — und sie war falsch, seit #030 und #031 jeden + Abschnitt höher gemacht haben. +- **Protokoll-Waisen werden nicht erraten.** Eine Zeile, deren Gegenstand fort ist, über den Namen + im Json zu greifen wäre naheliegend und falsch: derselbe Name steht in echter Historie einer noch + bestehenden Zeile. Dass eine Protokollzeile ihren Gegenstand überlebt, ist der Grund, warum + `entitaetId` kein Fremdschlüssel ist. + +### Probleme + +- **Die drei wertvollsten Befunde waren Gegenproben, die grün blieben.** Das ist inzwischen das + wiederkehrende Muster dieses Projekts: nicht der falsche Code fällt auf, sondern der Code, der + *aussieht*, als prüfte er etwas. + - Der **Link in der Abgleich-Meldung** war ein 404 (`/verwaltung/verzeichnis` statt + `/admin/verzeichnis`) — die eine Meldung, die an alle Administratoren geht, im Moment eines + Ausfalls. Der bestehende Test prüfte, dass der **übergebene** Link im Rumpf steht. Das ist + richtig und sagt nichts darüber, ob der Pfad existiert. + - Der **Anhang-Handler** hatte keinen einzigen Test. Die ganze Rechteprüfung ließ sich durch einen + hartcodierten ADMIN ersetzen, Suite grün. Das ist die einzige Stelle, die rohe Dateibytes + ausliefert. Bei SCIM war das Muster schon da — hier wurde es nicht angewandt. + - **`/api/scim`** in der Allowlist, siehe oben. +- **Ein offener Ereignisstrom überlebte das Deaktivieren eines Kontos.** Gefunden durch Lesen, nicht + durch einen Test. Dabei fiel auf, warum: die Attrappe der Sitzung war `{}` — ohne `benutzerId` + ließe sich eine Nachprüfung nicht einmal zeigen. **Eine Attrappe, die nicht die Form der + Wirklichkeit hat, prüft etwas anderes als die Wirklichkeit.** Dieselbe Lehre wie beim + Graph-Doppelgänger in #028, nur von der anderen Seite. +- **Die E2E-Suite war seit Monaten rot**, vier Ursachen, keine ein Produktfehler: ein Test, der die + Felder des Zustands aufzählt und `anzahlAnhaenge` nicht kannte (#033); ein Selektor, der seit #020 + zweimal traf; eine feste Zugweite von 300 px bei 532 px Abstand (#030/#031); und + Entwicklungsdaten, die vom Seed abwichen. Niemand hat es gemerkt, weil `pnpm test:e2e` in keinem + Tor steht — das ist jetzt #038 und keine Fußnote. +- **Ein Befund des Reviews war falsch:** der Zustands-Handler habe keinen Vitest-Test. Er hat acht + direkte Aufrufe. Nachgeprüft, bevor etwas geändert wurde — ein Agentenbericht ist eine Behauptung, + keine Feststellung. +- **Ein `DELETE` von 105 Zeilen wurde abgelehnt**, und das war am Ende richtig: der im Issue + vorgesehene Weg erledigte es von selbst, weil das Aufräumen über das Präfix beim ersten Lauf auch + die Altlast mitnimmt. Eine Sicherung der Entwicklungsdatenbank lag vorher. + +### Nächster Schritt + +- Offen bleiben **#024** (Proxmox-Einzeiler auf echtem PVE-Host), **#037** (zwei Nachzügler) und + **#038** (E2E in ein Tor). +- Danach ist v0.2.0 reif für einen Tag. Vorher gehört #038 entschieden: ein Release, dessen + E2E-Suite nur gelegentlich läuft, ist ein Release mit ungeprüften Zusagen — und genau das hat + diese Sitzung gezeigt. diff --git a/docs/issues/027-e2e-reste-vorlagen.md b/docs/issues/027-e2e-reste-vorlagen.md index c4387b3..b93e47d 100644 --- a/docs/issues/027-e2e-reste-vorlagen.md +++ b/docs/issues/027-e2e-reste-vorlagen.md @@ -1,10 +1,10 @@ # #027 Die Editor-Abläufe lassen ihre Vorlagen stehen -**Status:** offen +**Status:** erledigt **Im Tracker:** [#46](https://git.wilde-server.de/florian.wilde/Laufzettel/issues/46) **Meilenstein:** v0.2.0 **Labels:** phase-2, test -**Geschätzt:** 1 h **Tatsächlich:** – +**Geschätzt:** 1 h **Tatsächlich:** 2 h 10 min ## Ziel @@ -22,14 +22,69 @@ Vorlagenliste in der Oberfläche, Screenshots, und jede Abfrage von Hand. Bei ei ## Aufgaben -- [ ] Die Abläufe unter `tests/e2e/`, die Vorlagen anlegen, räumen sie in einem Nachlauf wieder weg — +- [x] Die Abläufe unter `tests/e2e/`, die Vorlagen anlegen, räumen sie in einem Nachlauf wieder weg — über ihr Präfix, nicht über gemerkte Kennungen (ein abgebrochener Lauf merkt sich nichts) -- [ ] Dasselbe für Geräte, Personen und Läufe prüfen, die die Abläufe anlegen -- [ ] Einmalig aufräumen: die vorhandenen Reste aus der Entwicklungsdatenbank entfernen -- [ ] Prüfen, ob ein Nachlauf das überhaupt leisten kann — ein Ablauf, der mitten im Lauf scheitert, +- [x] Dasselbe für Geräte, Personen und Läufe prüfen, die die Abläufe anlegen +- [x] Einmalig aufräumen: die vorhandenen Reste aus der Entwicklungsdatenbank entfernen +- [x] Prüfen, ob ein Nachlauf das überhaupt leisten kann — ein Ablauf, der mitten im Lauf scheitert, darf den Nachlauf nicht überspringen ## Akzeptanzkriterien -- [ ] Nach `pnpm test:e2e` steht in `Vorlage` keine Zeile mit `ZZ E2E` -- [ ] Ein abgebrochener Lauf lässt ebenfalls nichts stehen +- [x] Nach `pnpm test:e2e` steht in `Vorlage` keine Zeile mit `ZZ E2E` +- [x] Ein abgebrochener Lauf lässt ebenfalls nichts stehen + +## Beim Umsetzen dazugekommen + +- **Die Beschreibung oben war zur Hälfte überholt.** „Bei jedem Lauf kommen drei dazu“ stimmte beim + Schreiben, aber nicht mehr: alle 105 Reste stammen aus einem Fenster von sechzehn Stunden am + 25./26. September, und seither ist keine dazugekommen. `tests/e2e/vorlagen-editor.spec.ts` räumt + seit [#018](018-e2e-anleitung-pdf.md) über das Präfix auf, und das hält. Nachgewiesen an den + Zeitstempeln der Zeilen, nicht vermutet. +- **Dafür gab es eine neue Lücke, an anderer Stelle:** `anhaenge.spec.ts` (#032) und + `live.spec.ts` (#033) legen `ZZ E2E`-Läufe an und hatten **keinen** Nachlauf. In der + Entwicklungsdatenbank stand von ihnen nichts — nicht weil sie aufräumten, sondern weil die + E2E-Suite dort seit dem 26. September nicht mehr gelaufen war. +- **Geräumt wird jetzt zentral und an einer Stelle**: `tests/e2e/aufraeumen.ts`. Ein `afterAll` je + Datei ist die falsche Form für diese Zusage — beim nächsten Meilenstein fehlt wieder einer. +- **Vor *und* nach dem Lauf.** Ein `globalTeardown` läuft bei Testfehlern und bei Zeitabläufen, aber + nicht bei `Strg+C`. Der Vorlauf fängt genau das: ein Vorlauf lässt sich nicht überspringen. Er + **sagt es laut** („Rückstand aus einem früheren Lauf: …“), denn still wegzuräumen hieße, die + Ursache zu verdecken. +- **Das Präfix ist `ZZ` und nicht `ZZ E2E`.** Die Abläufe benutzen es in fünf Schreibweisen + (`ZZ E2E Editor …`, `ZZ-E2E--PC-01` — ein Hostname darf kein Leerzeichen haben —, + `ZZPruef…`, `ZZQuelle…`, `ZZ Abteilung …`). Alle fünf sind sinnvoll, alle fünf beginnen mit `ZZ`. + Darauf zu greifen deckt sie ab, ohne sechzehn Dateien umzubenennen. +- **Ein struktureller Test bewacht das** (`tests/betriebsstand.test.ts`): er liest die Abläufe und + schlägt an, wenn einer Zeilen anlegt, ohne das Präfix zu benutzen — und prüft, dass Vor- und + Nachlauf registriert sind. Er fand beim ersten Durchgang sofort zwei Dateien, die ich übersehen + hätte. +- **Protokoll-Waisen werden nicht erraten.** Eine Zeile, deren Gegenstand schon fort ist, bleibt + stehen; sie über den Namen im Json zu greifen wäre naheliegend und falsch, weil derselbe Name in + echter Historie einer noch bestehenden Zeile steht. Dass eine Protokollzeile ihren Gegenstand + überlebt, ist der Grund, warum `entitaetId` kein Fremdschlüssel ist. + +## Was der erste echte Lauf zutage brachte + +Der E2E-Durchgang war die ganze Zeit **rot**, und niemand hat es gemerkt — `pnpm test:e2e` steht +nicht im Tor vor dem Commit (Reviewbefund zu [#029](029-scim-endpunkt.md)). Sieben Fehlschläge, vier +Ursachen, keine davon ein Produktfehler, alle vier behoben: + +- `laeufe.spec.ts` zählte die Felder des Zustands auf und kannte `anzahlAnhaenge` nicht — das kam mit + #033 dazu. Die Aufzählung ist Absicht und bleibt: der Zustand ist die Antwort, die am häufigsten + hinausgeht, und ein Feld, das sich einschleicht, wandert ungesehen mit. +- `stammdaten.spec.ts` suchte „kommt aus dem Verzeichnis“ — seit #020 steht derselbe Satz auch am + Vorgesetzten-Feld, also zweimal auf der Seite. Playwright wies das als „strict mode violation“ ab. +- `vorlagen-editor.spec.ts` zog am Handy feste 300 px. Der Abstand zwischen den Anfassern ist + **532 px**, seit mit #030 und #031 Bausteinverweis und Abteilungsfilter in jeden Abschnitt kamen — + gemessen, nicht geschätzt. Dazu lag der erste Anfasser bei `y = -115`, also außerhalb des + Sichtfensters: der Touch-Start traf nichts. Jetzt wird erst gescrollt und die Zugweite aus dem + Layout genommen. Eine feste Pixelzahl in einem Sortiertest ist eine Zusage über das Layout, die + niemand pflegt. +- `onboarding.spec.ts` prüft den `New-ADUser`-Befehl zeichengenau und scheiterte an + `DC=firma,DC=de` statt `DC=firma,DC=local`: die **Entwicklungsdaten** wichen vom Seed ab, und der + Seed setzt bestehende Abteilungen absichtlich nicht zurück. Der Vorlauf prüft das jetzt und sagt, + was zu tun ist — statt einer Meldung, die nach einem Fehler im Befehlsbau aussieht. + +**Daraus folgt ein eigenes Issue:** `pnpm test:e2e` gehört ins Tor oder mindestens in eine +nachvollziehbare Regel, sonst wiederholt sich das. Siehe [#038](038-e2e-im-tor.md). diff --git a/docs/issues/036-reviewbefunde-phase-2.md b/docs/issues/036-reviewbefunde-phase-2.md new file mode 100644 index 0000000..40bd73d --- /dev/null +++ b/docs/issues/036-reviewbefunde-phase-2.md @@ -0,0 +1,136 @@ +# #036 Reviewbefunde aus der Durchsicht von Phase 2 + +**Status:** erledigt +**Im Tracker:** – +**Meilenstein:** v0.2.0 +**Labels:** phase-2, backend, test, docs +**Geschätzt:** 4 h **Tatsächlich:** 3 h 40 min + +## Ziel + +Acht Meilensteine von Phase 2 (M15–M22) lagen ohne fremden Blick auf `main`. Ein Review-Agent, der +den Code nicht geschrieben hat, hat sie gegen die Akzeptanzkriterien und gegen `CLAUDE.md` geprüft — +mit eigener Datenbank, Gegenproben und einem vollen Suitenlauf. + +**Ergebnis:** Die Kernregeln halten. Fünf von fünf Gegenproben gegen Versionierung, +Mandantengrenze, Abschlussregel und Selbstauskunft wurden rot. Alle Befunde lagen am Rand — in zwei +Route Handlern, einem Export-Pfad, in Betriebsdokumentation und in Kommentaren. + +**Die drei wertvollsten Befunde waren Gegenproben, die grün blieben.** Das ist das wiederkehrende +Muster dieses Projekts: nicht der falsche Code fällt auf, sondern der Code, der *aussieht*, als +prüfte er etwas. + +## Behoben + +### Schwer + +- [x] **Der Link in der Mail „Verzeichnis-Abgleich fehlgeschlagen“ war ein 404.** Gebaut wurde + `/verwaltung/verzeichnis` — `verwaltung` ist der Name des **Rechts**, die Seite liegt unter + `/admin/verzeichnis`. Das ist die eine Meldung, die an **alle** Administratoren geht, und + zwar im Moment eines Ausfalls: der Empfänger klickt und landet auf der + Nicht-gefunden-Seite. Die Pfade stehen jetzt als `MELDUNGS_PFADE` an einer Stelle, und ein + Test prüft **jeden gegen die Routen unter `src/app/`** — ein Pfad, den es nicht gibt, fällt + beim Testlauf auf und nicht im Postfach. +- [x] **Der Export eines Entwurfs mit Baustein lieferte einen leeren Abschnitt**, unter dem + Editor-Platzhalter „wird ersetzt“, und der Import nahm das stillschweigend an. Ein Redakteur + bringt eine noch unveröffentlichte Vorlage in eine zweite Installation — genau der Zweck des + Formats —, und der geteilte Abschnitt kommt dort ohne einen einzigen Schritt an. Baustein- + Abschnitte werden beim Export jetzt **aufgelöst**, wie beim Veröffentlichen; der Verweis + wandert weiterhin nicht mit. + +### Mittel + +- [x] **Der Route Handler, der Anhänge ausliefert, hatte keinen einzigen Test.** Die ganze + Rechteprüfung ließ sich durch einen hartcodierten ADMIN ersetzen, ohne dass ein Test rot + wurde. Das ist die **einzige** Stelle der Anwendung, die rohe Dateibytes ausliefert. Neu: + `tests/anhang-route.test.ts` fährt den Handler echt, acht Prüfungen — darunter das + Akzeptanzkriterium von #032 („nicht abrufbar **über seine Adresse**“), das bis hierher nur + auf Dienstebene geprüft war. Die Gegenprobe ist jetzt dreifach rot. +- [x] **`/api/scim` in der Proxy-Allowlist stand in keinem Test.** Die Zeile ließ sich entfernen, + ohne dass etwas rot wurde — und ohne sie leitet der Proxy jede Zustellung auf `/anmelden` um: + der Anbieter meldet „Provisioning failed“, in Laufzettel steht nichts, das Provisioning hört + still auf zu arbeiten. `OEFFENTLICH` ist jetzt exportiert, und ein Test prüft **in beide + Richtungen**: wer die Liste erweitert, muss es im Test aufschreiben. Eine Aufzählung, die nur + in einer Richtung geprüft wird, läuft dem Code hinterher. +- [x] **Ein offener Ereignisstrom überlebte Abmelden, Deaktivieren und Sitzungswiderruf.** Das + Recht wurde genau einmal geprüft, beim Aufbau, und Kennung samt Rolle für die Lebensdauer des + Stroms festgehalten. Ein Administrator deaktiviert ein Konto — und eine offen gelassene + Lasche bekommt weiter live Status, Schrittzustände und Zählwerte, bis jemand sie schließt. + `docs/architektur.md` verspricht „ein deaktivierter Benutzer wird auch mit gültigem Cookie + abgewiesen“; für einen Strom galt das nicht. Der Herzschlag prüft jetzt nach + (`sitzungNochGueltig`), eine Herabstufung engt den Strom ein statt ihn zu beenden. +- [x] **`README.md` kannte die Rolle `MITARBEITER` nicht**, und ein Satz daneben war seit #035 + falsch: „Unbekannte Benutzer werden beim ersten Login angelegt.“ Wer auf v0.2.0 hebt, liest + das README, nimmt an, für Leute ohne Rollengruppe bleibe alles wie bisher — und sperrt danach + genau diese Leute aus. #035 hatte den Satz in `docs/betrieb/authentik.md` richtiggestellt und + das README stehengelassen. Dazu: der Stand stand weiter auf „in Entwicklung auf v0.1.0“. +- [x] **`docs/betrieb/sicherung.md` nannte einen Anhänge-Pfad, den es nirgends gibt** + (`/opt/laufzettel/daten/anhaenge`) — als Rückfallwert in einem Skript, in dem ein `rm -rf` + steht. `deploy/install.sh` setzt `/var/lib/laufzettel/anhaenge`, und ohne die Variable nimmt + die Anwendung `./daten/anhaenge` relativ zu ihrem Arbeitsverzeichnis. Der Rückfall passte zu + **keinem** der drei. Jetzt `${ANHAENGE_PFAD:?…}`: fehlt die Variable, **bricht** der Befehl + ab, statt zu raten — vor dem `rm -rf`. Dasselbe Dokument hat denselben Fehler in #032 schon + einmal gehabt. +- [x] **Attributnamen wurden nur in der Server Action geprüft**, während sich + `src/server/graph/leser.ts` im Kommentar darauf berief („feste Feldnamen aus + `ATTRIBUT_MUSTER`, also ist hier nichts zu maskieren“). Nachgewiesen: ein Name mit Komma und + `$filter` kam durch den Dienst und schrieb einen **zweiten `$filter`** in die Graph-Adresse. + Hereinkommen konnte er nur mit `verwaltung` oder per Skript, Seed und `psql`, die + Sicherheitswirkung ist also klein — der Befund ist der **Ort** der Regel. Jetzt + `zuordnungPruefen` im Dienst (wirft) und Verwerfen in `zuordnungLesen` (damit eine kaputte + Zeile nicht die Quellenliste sprengt). + +### Leicht + +- [x] **Drei Kommentare versprachen mehr als der Code hält.** Der Kommentar am Konstanten-Block der + Wartezeiten sagte „nach dem vierten Fehlschlag wird aufgegeben“, während es der fünfte ist — + und das ist der Kommentar, den man zuerst liest. Die Migration der Benachrichtigungen + behauptete „der Dienst prüft dasselbe noch einmal, nur mit besserer Meldung“; tut er nicht, + und er soll es auch nicht (zwei Instanzen hinter einem Lastverteiler fragen beide und + bekommen beide „nein“ — nur der partielle Index hält). **Die Migration selbst bleibt + unverändert:** eine angewandte Migration wird nicht umgeschrieben, so wie eine + veröffentlichte `VorlagenVersion` nicht. Die Richtigstellung steht bei `einreihen`, wo sie + gelesen wird. +- [x] **Der Protokolleintrag „UPN mehrdeutig“ nannte immer „2“**, auch bei sieben Treffern: + `personZuUpn` hatte `take: 2`, und dieselbe Zahl ging in den Eintrag. Wer das Protokoll + liest, liest es, **um den Fall aufzulösen** — er muss wissen, wie viele Zeilen zu bereinigen + sind. `take` entfernt, Test mit vier Treffern. +- [x] **`anzahlSchritte` im Protokoll eines neuen Laufs zählte die Version, nicht den Lauf.** Bei + einer Vorlage mit abteilungsgebundenen Abschnitten (#031) behauptete der Eintrag mehr + Schritte, als der Lauf hat. Das Protokoll ist seit #023 ansehbar. +- [x] **Die Phase-2-Seiten fehlten in der 390-px-Prüfung.** Ergänzt: Bausteinliste, Schnellstart, + Protokoll, Zustellungen — gerade die Tabellen, also die Stellen, an denen horizontales + Scrollen entsteht. Jedes Issue berichtet eine Klickprobe von Hand; eine Probe von Hand ist + kein Regressionstest. +- [x] **Der Graph-Seitenabruf folgte `@odata.nextLink` ohne Host-Prüfung**, mit einem mandantenweit + gültigen Token im Kopf. Graph über TLS zu trauen ist vertretbar — aber `GRAPH_API_URL` ist + absichtlich einstellbar (nationale Clouds, Test-Doppelgänger), und ein Tippfehler darin + schickte das Token woandershin. `naechsteSeite` vergleicht jetzt den Ursprung. + +## Nicht behoben, bewusst + +- **`anhaengeDesLaufs` liegt in `services/` und wendet keine Mandantensicht an.** Beide Aufrufer + prüfen den Lauf vorher, und die Kommentare sagen das auch — aber es steht gegen die Regel aus + `CLAUDE.md` („`queries/*` wenden ihre Sicht selbst an, damit kein Aufrufer sie vergessen kann“). + Zwei Aufrufer in zwei Schichten gibt es schon; der dritte ist der, der es vergisst. Eigenes + Issue: [#037](037-anhaenge-lesemodell.md). +- **`/meine-geraete` fehlt weiter in der 390-px-Prüfung.** Die Seite verlangt `selbstauskunft`, und + das hat nur `MITARBEITER`; der E2E-Ablauf meldet sich als Admin an und würde die + Verweigerungsseite messen. Dafür braucht es ein eigenes Konto samt Personenbindung — siehe + [#037](037-anhaenge-lesemodell.md). +- **Das Graph-Token wird einmal je Abgleich geholt**, vor der Seitenschleife. Bei einem Mandanten, + dessen Abgleich länger als die Token-Lebensdauer läuft, hilft der Sicherheitsabstand nicht. Das + gehört auf die offene Prüfliste in `docs/betrieb/entra-id.md` und ist ohne echten Mandanten nicht + zu beurteilen. + +## Was der Review *nicht* gefunden hat + +Benannt, weil es den Befunden ihr Gewicht gibt: keine `kundeId` oder `quelleId` aus der Eingabe +statt aus Sitzung bzw. Token; kein Lesemodell, das seine Sicht ganz vergisst; keine +Client-Komponente mit einem Wert-Import aus `src/server/*`; kein Geheimnis in einer Sicht, Antwort +oder einem Protokolleintrag; kein gespeichertes `{{generiertes_passwort}}`; kein Codepfad, der +annimmt, dass nur ein Mensch einen Schritt erledigt. + +Ein Befund des Reviews war **falsch**: der Zustands-Handler habe keinen Vitest-Test. Er hat acht +direkte Aufrufe in `tests/lauf-ereignisse.test.ts`, darunter „weist ohne Anmeldung ab“ und „liefert +für einen fremden Kunden keinen Strom“. Nachgeprüft, bevor etwas geändert wurde. diff --git a/docs/issues/037-anhaenge-lesemodell.md b/docs/issues/037-anhaenge-lesemodell.md new file mode 100644 index 0000000..bf66a31 --- /dev/null +++ b/docs/issues/037-anhaenge-lesemodell.md @@ -0,0 +1,53 @@ +# #037 Zwei Nachzügler aus der Durchsicht von Phase 2 + +**Status:** offen +**Im Tracker:** – +**Meilenstein:** v0.3.0 +**Labels:** phase-2, backend, test +**Geschätzt:** 2 h **Tatsächlich:** – + +## Ziel + +Zwei Befunde aus [#036](036-reviewbefunde-phase-2.md), die mehr sind als eine Korrektur: beide +verlangen einen Umbau, und beide sind heute kein Schaden, sondern eine offene Flanke. + +## 1. `anhaengeDesLaufs` ist ein Lesemodell am falschen Ort + +`anhaengeDesLaufs(laufId)` liegt in `src/server/services/anhaenge.ts`, nimmt nur eine Lauf-Kennung +und wendet **weder Rollen- noch Mandantensicht** an. Beide Aufrufer +(`src/app/(app)/laeufe/[id]/page.tsx` und `src/server/queries/druck.ts`) prüfen den Lauf vorher, und +die Kommentare sagen das auch — es ist also heute kein Fehler. + +Es steht aber gegen die Regel aus `CLAUDE.md` und `docs/architektur.md` §2: Lesemodelle liegen in +`queries/*` und **wenden ihre Sicht selbst an, damit kein Aufrufer sie vergessen kann**. Zwei +Aufrufer in zwei Schichten gibt es schon; der dritte ist der, der es vergisst — und er wird nicht +auffallen, weil die zwei bestehenden korrekt sind. + +- [ ] Nach `src/server/queries/anhaenge.ts` ziehen, mit `HandelnderBenutzer` als Parameter +- [ ] Mandanten- und Rollensicht dort anwenden, nicht beim Aufrufer +- [ ] Test: ein Benutzer, der auf andere Kunden beschränkt ist, bekommt eine leere Liste — **ohne** + dass der Aufrufer vorher geprüft hat +- [ ] Gegenprobe: die Sicht entfernen, der Test muss rot werden + +## 2. `/meine-geraete` fehlt in der 390-px-Prüfung + +Die Seite ist das Ergebnis von #035 und ausdrücklich mit „am Handy kein horizontales Scrollen“ +abgenommen — von Hand. In `tests/e2e/mobil.spec.ts` steht sie nicht, und sie kann dort nicht +einfach ergänzt werden: sie verlangt das Recht `selbstauskunft`, und das hat nur `MITARBEITER`. Der +Ablauf meldet sich als Admin an und würde die Verweigerungsseite messen — die scrollt garantiert +nicht. + +- [ ] Ein `MITARBEITER`-Konto für die Abläufe: lokales Passwort, verknüpft mit einer + `Mitarbeiter`-Zeile (der CHECK `Benutzer_mitarbeiter_nur_bei_rolle` verlangt die Verknüpfung) +- [ ] Im Seed oder im E2E-Vorlauf? **Im Vorlauf**, denn ein Konto mit bekanntem Passwort gehört + nicht in einen Seed, der auch produktiv läuft +- [ ] `/meine-geraete` in die Seitenliste von `mobil.spec.ts` +- [ ] Dabei prüfen, was ein `MITARBEITER` auf den **anderen** Seiten sieht: die Rollenmatrix ist in + Vitest vollständig, aber kein Ablauf klickt sie durch + +## Akzeptanzkriterien + +- [ ] `anhaengeDesLaufs` liegt in `queries/`, und ein Test belegt die Sicht ohne Vorprüfung des + Aufrufers +- [ ] `/meine-geraete` wird bei 390 px gemessen, mit einem echten `MITARBEITER` +- [ ] Beide Gegenproben werden rot diff --git a/docs/issues/038-e2e-im-tor.md b/docs/issues/038-e2e-im-tor.md new file mode 100644 index 0000000..73e700d --- /dev/null +++ b/docs/issues/038-e2e-im-tor.md @@ -0,0 +1,54 @@ +# #038 Die E2E-Abläufe laufen in keinem Tor und waren monatelang rot + +**Status:** offen +**Im Tracker:** – +**Meilenstein:** v0.3.0 +**Labels:** phase-2, test, infra +**Geschätzt:** 3 h **Tatsächlich:** – + +## Ziel + +Beim Abschluss von [#027](027-e2e-reste-vorlagen.md) lief `pnpm test:e2e` erstmals seit dem +26. September wieder vollständig. **Sieben Abläufe waren rot**, und niemand hatte es gemerkt. + +Keiner der sieben war ein Produktfehler — und genau das ist das Problem. Vier Ursachen: + +| Ablauf | Ursache | Seit | +|---|---|---| +| `laeufe.spec.ts` | zählt die Felder des Zustands auf, `anzahlAnhaenge` fehlte | #033 | +| `stammdaten.spec.ts` | „kommt aus dem Verzeichnis“ steht seit #020 zweimal auf der Seite | #020 | +| `vorlagen-editor.spec.ts` | zieht am Handy feste 300 px, der Abstand ist 532 px | #030/#031 | +| `onboarding.spec.ts` | die **Entwicklungsdaten** wichen vom Seed ab | unbekannt | + +Alle vier sind behoben. Das Muster bleibt: **die E2E-Schicht trägt Zusagen, die das Commit-Tor nicht +fährt** — Rückfall auf Polling, Herzschlag, 390 px, die Auslieferung eines Anhangs über seine +Adresse. Steht eine Zusage nur in Playwright, und läuft Playwright nur gelegentlich, dann ist sie +eine Notiz und keine Prüfung. + +## Zu entscheiden + +- [ ] **`pnpm test:e2e` ins Tor vor dem Commit?** Dagegen: es verlangt einen **gebauten, + laufenden** Stand (`pnpm build && pnpm start`), dauert über vier Minuten und braucht eine + Datenbank mit Seed-Bestand. Vier Minuten vor jedem Commit ist ein Tor, das umgangen wird. +- [ ] **Oder eine kleinere, verbindliche Regel:** E2E läuft vor jedem Merge nach `main` — das wären + acht Läufe in Phase 2 gewesen und hätte jeden der vier Befunde am Tag seiner Entstehung + gezeigt. Das ist die Empfehlung. +- [ ] **Oder im CI** (`.gitea/workflows/ci.yml`). Dort ist kein Runner registriert; das zu ändern + ist eine eigene Entscheidung und gehört nicht in dieses Issue. + +## Aufgaben + +- [ ] Die gewählte Regel in `CLAUDE.md` aufschreiben, mit **gemessener** Dauer und der Begründung, + warum sie nicht im Commit-Tor steht +- [ ] Die Voraussetzungen, die `tests/e2e/global-setup.ts` prüft, in derselben Reihenfolge + dokumentieren — der Vorlauf ist inzwischen die ehrlichste Beschreibung dessen, was E2E braucht +- [ ] Prüfen, welche Zusagen **ausschließlich** in E2E stehen, und für jede entscheiden: gehört sie + zusätzlich in einen Vitest-Test, der den Handler direkt fährt? (Das Muster steht in + `tests/scim.test.ts`, `tests/anhang-route.test.ts` und `tests/lauf-ereignisse.test.ts`.) +- [ ] Mindestens die Rechteprüfung der beiden Phase-2-Handler ist damit schon abgedeckt — bleiben + Rückfall auf Polling, Herzschlag am echten Browser und 390 px + +## Akzeptanzkriterien + +- [ ] In `CLAUDE.md` steht, wann E2E läuft, und die Angabe ist gemessen, nicht geschätzt +- [ ] Eine Liste der Zusagen, die nur in E2E stehen — und zu jeder eine Entscheidung diff --git a/docs/issues/README.md b/docs/issues/README.md index fc95732..3304ec5 100644 --- a/docs/issues/README.md +++ b/docs/issues/README.md @@ -31,7 +31,7 @@ Diese Dateien sind die Quelle; `pnpm issues:sync` spiegelt sie in den Gitea-Trac | #024 | [Proxmox-Einzeiler auf einem echten PVE-Host gegenprüfen](024-proxmox-echtlauf.md) | offen | v0.2.0 | 2 h | – | | #025 | [Healthcheck gibt die rohe Datenbankfehlermeldung heraus](025-gesundheit-fehlermeldung.md) | erledigt | v0.2.0 | 1 h | 25 min | | #026 | [Zwei Konventionen präzisieren, die strenger klingen als sie gemeint sind](026-konventionen-praezisieren.md) | erledigt | v0.2.0 | 1 h | 20 min | -| #027 | [Die Editor-Abläufe lassen ihre Vorlagen stehen](027-e2e-reste-vorlagen.md) | offen | v0.2.0 | 1 h | – | +| #027 | [Die Editor-Abläufe lassen ihre Vorlagen stehen](027-e2e-reste-vorlagen.md) | erledigt | v0.2.0 | 1 h | 2 h 10 min | | #028 | [M15 – Quellenarten und Microsoft-Graph-Quelle](028-graph-quelle.md) | erledigt | v0.2.0 | 10 h | 4 h 10 min | | #029 | [M16 – SCIM-Endpunkt als Alternative zum Abgleich](029-scim-endpunkt.md) | erledigt | v0.2.0 | 12 h | 4 h 05 min | | #030 | [M17 – Wiederverwendbare Bausteine](030-bausteine.md) | erledigt | v0.2.0 | 12 h | 2 h 25 min | @@ -40,4 +40,7 @@ Diese Dateien sind die Quelle; `pnpm issues:sync` spiegelt sie in den Gitea-Trac | #033 | [M20 – Live-Aktualisierung per SSE statt Polling](033-sse-live.md) | erledigt | v0.2.0 | 6 h | 1 h 40 min | | #034 | [M21 – Benachrichtigungen per Mail und Webhook](034-benachrichtigungen.md) | erledigt | v0.2.0 | 10 h | 3 h 30 min | | #035 | [M22 – Mitarbeiter-Selbstauskunft](035-selbstauskunft.md) | erledigt | v0.2.0 | 8 h | 2 h 50 min | -| | **Summe** | | | **229 h** | **64 h 40 min** | +| #036 | [Reviewbefunde aus der Durchsicht von Phase 2](036-reviewbefunde-phase-2.md) | erledigt | v0.2.0 | 4 h | 3 h 40 min | +| #037 | [Zwei Nachzügler aus der Durchsicht von Phase 2](037-anhaenge-lesemodell.md) | offen | v0.3.0 | 2 h | – | +| #038 | [Die E2E-Abläufe laufen in keinem Tor und waren monatelang rot](038-e2e-im-tor.md) | offen | v0.3.0 | 3 h | – | +| | **Summe** | | | **238 h** | **70 h 30 min** | diff --git a/playwright.config.ts b/playwright.config.ts index ca594dd..85b855f 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -11,6 +11,12 @@ export default defineConfig({ * erst mitten im Lauf auf — als Zeitablauf oder als „0 Treffer“ (#018). */ globalSetup: "./tests/e2e/global-setup.ts", + /* + * Raeumt weg, was die Ablaeufe angelegt haben (#027). Laeuft auch nach + * fehlgeschlagenen Ablaeufen; ein harter Abbruch wird vom Vorlauf gefangen, + * der dasselbe tut. + */ + globalTeardown: "./tests/e2e/global-teardown.ts", // Die Ablaeufe aendern gemeinsame Daten, deshalb nacheinander. fullyParallel: false, workers: 1, diff --git a/src/app/api/laeufe/[id]/zustand/route.ts b/src/app/api/laeufe/[id]/zustand/route.ts index b24ca85..0da73ff 100644 --- a/src/app/api/laeufe/[id]/zustand/route.ts +++ b/src/app/api/laeufe/[id]/zustand/route.ts @@ -1,4 +1,5 @@ -import { RechteFehler, rechtErforderlich } from "@/server/auth/sitzungspflicht"; +import { RechteFehler, rechtErforderlich, sitzungNochGueltig } from "@/server/auth/sitzungspflicht"; +import type { Sitzung } from "@/lib/sitzung"; import { laufZustand } from "@/server/queries/laeufe"; import { HERZSCHLAG, herzschlagMs, maxStroeme, sseEreignis, willStrom } from "@/lib/sse"; import { aufLaufHoeren, stroemeOffen } from "@/server/lauf-ereignisse"; @@ -40,10 +41,14 @@ export async function GET( ): Promise { let benutzerId: string; let rolle: Parameters[1]["rolle"]; + // Die Sitzung wird festgehalten, weil der Strom sie spaeter noch braucht: + // `cookies()` gibt es in seinen Rueckrufen nicht mehr. + let sitzung: Sitzung; try { - const { benutzer } = await rechtErforderlich("lesen"); - benutzerId = benutzer.id; - rolle = benutzer.rolle; + const zugang = await rechtErforderlich("lesen"); + benutzerId = zugang.benutzer.id; + rolle = zugang.benutzer.rolle; + sitzung = zugang.sitzung; } catch (fehler) { if (fehler instanceof RechteFehler) { // 401 und nicht 403: der Client soll die Aktualisierung einstellen und @@ -54,7 +59,7 @@ export async function GET( } const { id } = await params; - const benutzer = { id: benutzerId, rolle }; + let benutzer = { id: benutzerId, rolle }; const zustand = await laufZustand(id, benutzer); if (!zustand) { // Nicht vorhanden und nicht sichtbar sind dieselbe Antwort – die @@ -151,7 +156,31 @@ export async function GET( return; } - herzschlag = setInterval(() => schicken(HERZSCHLAG), herzschlagMs()); + /* + * **Der Herzschlag prueft auch die Sitzung nach** (Reviewbefund zu #029). + * Vorher wurde das Recht genau einmal geprueft, beim Aufbau, und das Paar + * aus Kennung und Rolle fuer die Lebensdauer des Stroms festgehalten — ein + * deaktiviertes Konto bekam weiter live Daten, bis jemand die Lasche + * schloss. Die Pruefung kostet eine Abfrage je offenem Strom und + * Herzschlag (25 s); dieselbe Abfrage macht jede gewoehnliche Anfrage + * ohnehin. + * + * Eine **Herabstufung** beendet den Strom nicht, sondern engt ihn ein: die + * neue Rolle gilt ab dem naechsten Ereignis. Das ist dieselbe Regel wie + * „Rolle pro Anfrage aus der Datenbank“ (CLAUDE.md). + */ + herzschlag = setInterval(() => { + void (async () => { + const noch = await sitzungNochGueltig(sitzung, "lesen"); + if (!noch) { + schicken(sseEreignis("ende", JSON.stringify({ grund: "nicht-angemeldet" }))); + aufraeumen(); + return; + } + if (noch.rolle !== benutzer.rolle) benutzer = { id: benutzerId, rolle: noch.rolle }; + schicken(HERZSCHLAG); + })(); + }, herzschlagMs()); anfrage.signal.addEventListener("abort", aufraeumen); }, diff --git a/src/lib/verzeichnis.ts b/src/lib/verzeichnis.ts index e5958c6..7f9d0c8 100644 --- a/src/lib/verzeichnis.ts +++ b/src/lib/verzeichnis.ts @@ -151,11 +151,49 @@ export function zuordnungLesen(roh: unknown, art: "LDAP" | "GRAPH" = "LDAP"): At const gelesen = { ...ZUORDNUNGS_VORGABE[art] }; for (const schluessel of ZUORDNUNGS_FELDER) { const wert = felder[schluessel]; - if (typeof wert === "string" && wert.trim()) gelesen[schluessel] = wert.trim(); + if (typeof wert !== "string" || !wert.trim()) continue; + /* + * **Was kein Attributname ist, wird beim Lesen verworfen** und nicht + * weitergegeben. Hier zu werfen waere falsch: eine einzige kaputte Zeile in + * der Datenbank machte sonst die ganze Quellenliste unbenutzbar. Dass so + * etwas gar nicht erst hineinkommt, haelt `zuordnungPruefen` beim Schreiben. + * + * Der Grund fuer beides: aus diesen Namen entstehen eine LDAP-Attributliste + * und ein `$select` in der Graph-Adresse. `src/server/graph/leser.ts` berief + * sich im Kommentar auf diese Pruefung, waehrend sie nur in der Server Action + * stand — ein Name mit Komma und `$filter` kam durch den Dienst und landete + * in der Abfrage (Reviewbefund zu #029). + */ + if (!ATTRIBUT_MUSTER.test(wert.trim())) continue; + gelesen[schluessel] = wert.trim(); } return gelesen; } +/** + * Prueft eine Zuordnung, die **geschrieben** werden soll. Wirft nicht, sondern + * meldet — der Aufrufer entscheidet, ob daraus eine Fehlermeldung oder eine + * Ausnahme wird. + * + * Getrennt von `zuordnungAusEingabe`, weil die aus **Formularfeldern** liest und + * dabei leere Felder herausfallen laesst. Diese hier prueft eine fertige + * Zuordnung, egal woher sie kommt: Formular, Skript, Seed oder Import. + */ +export function zuordnungPruefen(zuordnung: Partial>): string | undefined { + for (const [schluessel, wert] of Object.entries(zuordnung)) { + if (wert === undefined || wert === null) continue; + if (typeof wert !== "string" || !ATTRIBUT_MUSTER.test(wert.trim())) { + const beschriftung = + (ZUORDNUNGS_BESCHRIFTUNG as Record)[schluessel] ?? schluessel; + return ( + `„${String(wert)}“ ist kein Attributname (${beschriftung}). ` + + "Erwartet werden Buchstaben, Ziffern und Bindestriche, beginnend mit einem Buchstaben." + ); + } + } + return undefined; +} + /** * Die Attributzuordnung aus den Formularfeldern, geprueft. * diff --git a/src/proxy.ts b/src/proxy.ts index 8cbc3b6..f3c8375 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -18,8 +18,17 @@ import { SITZUNG_COOKIE } from "@/lib/sitzung"; * den Proxy zu haengen. */ -/** Pfade, die ohne Anmeldung erreichbar sein muessen. */ -const OEFFENTLICH: readonly string[] = [ +/** + * Pfade, die ohne Anmeldung erreichbar sein muessen. + * + * **Exportiert, damit der Test die Liste nicht abschreiben muss.** Vorher zaehlte + * `tests/zugang.test.ts` die Pfade von Hand auf, und seit #029 fehlte dort + * `/api/scim` — eine Aufzaehlung laeuft dem Code hinterher (Reviewbefund zu + * #029). Jetzt prueft ein Test, dass **jeder** Eintrag hier auch im Test steht: + * ein neuer oeffentlicher Pfad ist eine Sicherheitsentscheidung und soll jemanden + * zwingen, sie aufzuschreiben. + */ +export const OEFFENTLICH: readonly string[] = [ "/anmelden", // Der Anmeldevorgang selbst – ohne ihn koennte sich niemand anmelden. "/api/auth", diff --git a/src/server/auth/benutzer.ts b/src/server/auth/benutzer.ts index 20c78e7..2480a8a 100644 --- a/src/server/auth/benutzer.ts +++ b/src/server/auth/benutzer.ts @@ -137,10 +137,19 @@ async function personZuUpn( const gesucht = upn.trim(); if (!gesucht) return { treffer: "keiner" }; + /* + * **Ohne `take`, damit die gemeldete Zahl die Zahl ist.** Vorher stand hier + * `take: 2` — fuer die Entscheidung genuegt das, aber `anzahl` ging damit in + * den Protokolleintrag „Anmeldung abgewiesen: UPN mehrdeutig“, und der sagte + * dann immer „2“, auch bei sieben Treffern (Reviewbefund zu #029). Wer das + * Protokoll liest, um den Fall aufzuloesen, bekam eine Zahl, die keine war. + * + * Die Abfrage laeuft gegen einen Index auf `upn` und liefert im Normalfall + * **einen** Treffer; mehrere sind ein Datenfehler, den jemand beheben muss. + */ const passende = await prisma.mitarbeiter.findMany({ where: { upn: { equals: gesucht, mode: "insensitive" }, aktiv: true }, select: { id: true, kundeId: true }, - take: 2, }); const erster = passende[0]; diff --git a/src/server/auth/sitzungspflicht.ts b/src/server/auth/sitzungspflicht.ts index 5d2a6e9..258d0d0 100644 --- a/src/server/auth/sitzungspflicht.ts +++ b/src/server/auth/sitzungspflicht.ts @@ -5,6 +5,7 @@ import type { Benutzer } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; import { SITZUNG_COOKIE, sitzungLesen, type Sitzung } from "@/lib/sitzung"; import { darf, type Recht } from "@/server/auth/rollen"; +import type { Rolle } from "@/generated/prisma/enums"; /** * Wer ist angemeldet und was darf er (#007). @@ -55,6 +56,39 @@ export type Angemeldet = { readonly sitzung: Sitzung; }; +/** + * Gilt diese Sitzung **jetzt noch**, und darf sie `recht`? + * + * Fuer langlebige Verbindungen. `angemeldet()` liest das Cookie und laeuft + * deshalb nur innerhalb einer Anfrage; ein Ereignisstrom lebt Minuten bis + * Stunden **nach** dem Ende der Anfrage weiter, und in seinen Rueckrufen gibt es + * keinen `cookies()`-Kontext mehr. Die Sitzung selbst ist zu dem Zeitpunkt + * laengst geprueft — nachzusehen ist, was sich seither in der Datenbank + * geaendert haben kann: `aktiv`, die Rolle und `sitzungenUngueltigAb`. + * + * **Warum das noetig ist:** Ohne diese Pruefung bekam eine offen gelassene + * Lasche weiter live Daten, nachdem ein Administrator das Konto deaktiviert und + * die Sitzungen beendet hatte — bis jemand die Lasche schloss. `docs/architektur.md` + * verspricht „ein deaktivierter Benutzer wird auch mit gueltigem Cookie + * abgewiesen“, und fuer einen offenen Strom galt das nicht (Reviewbefund zu #029). + * + * Die Rolle wird **mit**geliefert, nicht nur geprueft: wer waehrend eines Stroms + * herabgestuft wird, soll von da an die Sicht seiner neuen Rolle bekommen — das + * ist dieselbe Regel wie „Rolle pro Anfrage aus der Datenbank“ (CLAUDE.md). + */ +export async function sitzungNochGueltig( + sitzung: Sitzung, + recht: Recht, +): Promise<{ readonly rolle: Rolle } | undefined> { + const benutzer = await prisma.benutzer.findUnique({ + where: { id: sitzung.benutzerId }, + select: { id: true, aktiv: true, rolle: true, sitzungenUngueltigAb: true }, + }); + if (!sitzungBewerten(sitzung, benutzer).gueltig || !benutzer) return undefined; + if (!darf(benutzer.rolle, recht)) return undefined; + return { rolle: benutzer.rolle }; +} + /** * Der angemeldete Benutzer oder `undefined`. Leitet nicht weiter – dafuer gibt es * `sitzungErforderlich`. diff --git a/src/server/graph/leser.ts b/src/server/graph/leser.ts index 331bdaf..3f9bec6 100644 --- a/src/server/graph/leser.ts +++ b/src/server/graph/leser.ts @@ -102,6 +102,35 @@ export function angefragteFelder(zuordnung: AttributZuordnung): string[] { */ export const GRAPH_FILTER = "accountEnabled eq true"; +/** + * Die naechste Seitenadresse — **nur, wenn sie auf denselben Host zeigt**. + * + * Jede Seite wird mit `Authorization: Bearer ` geholt, und das Token gilt + * mandantenweit (`User.Read.All`). Der Antwort von Graph ueber TLS zu trauen ist + * vertretbar, aber `GRAPH_API_URL` ist absichtlich einstellbar — fuer nationale + * Clouds und fuer den Test-Doppelgaenger. Ein Tippfehler darin oder ein + * aufbrechender Firmenproxy dazwischen wuerde das Token sonst an eine fremde + * Adresse schicken (Reviewbefund zu #029). + * + * Die Pruefung kostet eine Zeile und schliesst den Fall. Ein `nextLink`, der + * woandershin zeigt, beendet den Abruf still: er ist kein Fehler des + * Verzeichnisses, und ein Abbruch mitten im Abgleich waere schlimmer als eine + * unvollstaendige Seite — der naechste Lauf holt sie. + */ +export function naechsteSeite(weiter: unknown, basis = graphBasis()): string | undefined { + if (typeof weiter !== "string" || !weiter.trim()) return undefined; + try { + if (new URL(weiter).origin !== new URL(basis).origin) { + console.error(`Graph: @odata.nextLink zeigt auf einen anderen Host als GRAPH_API_URL — Abruf beendet.`); + return undefined; + } + } catch { + // Keine brauchbare Adresse: dasselbe Ergebnis, nur ohne Erklaerung. + return undefined; + } + return weiter; +} + /** * Die erste Adresse der Abfrage. Danach folgt der Abruf `@odata.nextLink`. * @@ -272,7 +301,7 @@ export async function graphEintraegeStroemen( } const weiter = seite["@odata.nextLink"]; - adresse = typeof weiter === "string" && weiter ? weiter : undefined; + adresse = naechsteSeite(weiter); } return { gesamtGelesen, uebersprungen, seiten }; diff --git a/src/server/services/benachrichtigungen.ts b/src/server/services/benachrichtigungen.ts index aa3353f..e8c1891 100644 --- a/src/server/services/benachrichtigungen.ts +++ b/src/server/services/benachrichtigungen.ts @@ -39,10 +39,15 @@ export class MeldungsFehler extends Error { /** * Wie lange zwischen zwei Versuchen gewartet wird, in Minuten. * - * Die Laenge der Liste ist zugleich die Zahl der Versuche: nach dem vierten - * Fehlschlag wird aufgegeben. Wachsend, weil die haeufigste Ursache ein - * Mailserver ist, der gerade neu startet — eine Minute spaeter geht es meist - * wieder, und nach zwei Stunden ist es ein Fall fuer einen Menschen. + * **Vier Wartezeiten ergeben fuenf Versuche**, nicht vier: gewartet wird erst + * *nach* einem Fehlschlag, der fuenfte Versuch findet also keine Wartezeit mehr + * vor und wird aufgegeben. Der Kommentar hier behauptete „nach dem vierten“, + * waehrend `naechsterVersuch` weiter unten das Richtige sagt — und das ist der + * Kommentar, den man zuerst liest (Reviewbefund zu #029). + * + * Wachsend, weil die haeufigste Ursache ein Mailserver ist, der gerade neu + * startet — eine Minute spaeter geht es meist wieder, und nach zwei Stunden ist + * es ein Fall fuer einen Menschen. */ export const WARTEZEITEN_MINUTEN = [1, 5, 30, 120] as const; @@ -332,8 +337,26 @@ function appUrl(): string { return (process.env["APP_URL"]?.trim() || "").replace(/\/+$/, ""); } +/** + * Die Pfade, auf die eine Meldung zeigen kann — **an einer Stelle**. + * + * Sie stehen hier als Konstanten, weil ein Pfad in einer Mail nicht nachgesehen + * wird: er wird geklickt. `/verwaltung/verzeichnis` stand hier und war ein 404 — + * `verwaltung` ist der Name des **Rechts**, die Seite liegt unter `/admin/` + * (Reviewbefund zu #029). Aufgefallen ist das niemandem, weil der Test nur + * prueft, dass der **uebergebene** Link im Rumpf steht. + * + * `tests/benachrichtigungen.test.ts` prueft jeden dieser Pfade jetzt gegen die + * Routen unter `src/app/` — ein Pfad, den es nicht gibt, faellt beim Testlauf + * auf und nicht im Postfach eines Administrators. + */ +export const MELDUNGS_PFADE = { + lauf: "/laeufe", + verzeichnis: "/admin/verzeichnis", +} as const; + function laufLink(laufId: string): string { - return `${appUrl()}/laeufe/${laufId}`; + return `${appUrl()}${MELDUNGS_PFADE.lauf}/${laufId}`; } /** @@ -424,6 +447,16 @@ async function einreihen( * `skipDuplicates`, weil der partielle eindeutige Index fuer „ueberfaellig“ * genau hier zuschlaegt: der Zeitplan sieht jede Minute nach, und ohne das * waere jeder zweite Durchgang ein Fehler statt einer stillen Nichtaenderung. + * + * **Hier haelt nur die Datenbank, und das ist Absicht.** Eine zweite Pruefung + * im Dienst („gibt es die schon?“) waere ein Wettlauf: zwei Instanzen hinter + * einem Lastverteiler fragen beide und bekommen beide „nein“. Die Migration + * `20261005110000_benachrichtigungen` sagt im Kommentar das Gegenteil — sie + * verspricht eine Dienstpruefung „mit besserer Meldung“, die es nicht gibt und + * nicht geben soll (Reviewbefund zu #029). Eine angewandte Migration wird + * nicht nachtraeglich umgeschrieben, so wie eine veroeffentlichte + * `VorlagenVersion` nicht; die Richtigstellung steht deshalb hier, wo sie + * gelesen wird, und in `tests/datenbankregeln.test.ts`. */ const ergebnis = await prisma.zustellung.createMany({ data: zeilen, skipDuplicates: true }); return ergebnis.count; @@ -498,7 +531,7 @@ export async function abgleichMeldungEinreihen(quelleId: string, grund: string): const webhook = await webhookZugang(); if (admins.length === 0 && !webhook) return 0; - const link = `${appUrl()}/verwaltung/verzeichnis`; + const link = `${appUrl()}${MELDUNGS_PFADE.verzeichnis}`; const { betreff, rumpf } = abgleichMeldung(quelle.name, grund, link); return einreihen( Meldeanlass.ABGLEICH_FEHLGESCHLAGEN, diff --git a/src/server/services/laeufe.ts b/src/server/services/laeufe.ts index 4bffdd6..a9c54d5 100644 --- a/src/server/services/laeufe.ts +++ b/src/server/services/laeufe.ts @@ -566,7 +566,14 @@ export async function laufAnlegen(eingabe: NeuerLauf, benutzer: HandelnderBenutz vorlage: version.vorlagenName, vorlagenVersionId: version.id, status: LaufStatus.OFFEN, - anzahlSchritte: version.schrittIds.length, + /* + * Die Schritte **dieses Laufs**, nicht die der Version. Seit #031 + * fallen abteilungsfremde Abschnitte heraus; `version.schrittIds.length` + * behauptete deshalb mehr Schritte, als der Lauf hat (Reviewbefund zu + * #029). Das Protokoll ist seit #023 ansehbar — eine Zahl darin, die + * nicht zur Liste darunter passt, kostet jemanden eine Suche. + */ + anzahlSchritte: anzulegen.length, zugewiesenAnId, }, }, diff --git a/src/server/services/verzeichnis.ts b/src/server/services/verzeichnis.ts index 72b3490..0b958f2 100644 --- a/src/server/services/verzeichnis.ts +++ b/src/server/services/verzeichnis.ts @@ -14,6 +14,7 @@ import { INTERVALL_HOECHSTENS, INTERVALL_MINDESTENS, intervallMeldung, + zuordnungPruefen, } from "@/lib/verzeichnis"; import { eintraegeDerQuelleStroemen } from "@/server/verzeichnis/quellenleser"; import { urlPruefen } from "@/server/ldap/verbindung"; @@ -317,6 +318,18 @@ export async function quelleAnlegen( */ const felder = feldernJeArt(art, eingabe); + /* + * **Die Zuordnung wird hier geprueft, nicht nur in der Server Action.** + * Aus diesen Namen entsteht eine LDAP-Attributliste und ein `$select` in der + * Graph-Adresse; `src/server/graph/leser.ts` berief sich im Kommentar darauf, + * dass sie dem Muster entsprechen — waehrend die Pruefung eine Schicht + * darueber in `zuordnungAusFormular` stand. Ein Name mit Komma und `$filter` + * kam deshalb durch den Dienst und landete in der Abfrage (Reviewbefund zu + * #029). Ein Skript, der Seed und ein Import gehen nicht durch das Formular. + */ + const zuordnungsMangel = zuordnungPruefen(eingabe.attributZuordnung ?? {}); + if (zuordnungsMangel) throw new VerzeichnisFehler(zuordnungsMangel); + const zeile = await prisma.$transaction(async (tx) => { const angelegt = await tx.verzeichnisquelle.create({ data: { @@ -487,6 +500,8 @@ export async function quelleBearbeiten( } } if (aenderung.attributZuordnung !== undefined) { + const mangel = zuordnungPruefen(aenderung.attributZuordnung); + if (mangel) throw new VerzeichnisFehler(mangel); const zuordnung = zuordnungLesen({ ...zuordnungLesen(vorher.attributZuordnung), ...aenderung.attributZuordnung, diff --git a/src/server/services/vorlagen-austausch.ts b/src/server/services/vorlagen-austausch.ts index 60ea6d8..6cda312 100644 --- a/src/server/services/vorlagen-austausch.ts +++ b/src/server/services/vorlagen-austausch.ts @@ -42,7 +42,22 @@ export async function vorlageExportieren(versionId: string): Promise variablen: { orderBy: [{ position: "asc" }, { id: "asc" }] }, abschnitte: { orderBy: [{ position: "asc" }, { id: "asc" }], - include: { schritte: { orderBy: [{ position: "asc" }, { id: "asc" }] } }, + include: { + schritte: { orderBy: [{ position: "asc" }, { id: "asc" }] }, + /* + * Der Baustein mit. Ein Baustein-Abschnitt im **Entwurf** hat keine + * eigenen Schritte — die entstehen erst beim Veroeffentlichen. Ohne + * diese Zeilen exportierte ein Entwurf einen leeren Abschnitt unter + * dem Platzhaltertitel „wird ersetzt“ (Reviewbefund zu #029). + */ + bausteinVersion: { + select: { + titel: true, + baustein: { select: { name: true } }, + schritte: { orderBy: [{ position: "asc" }, { id: "asc" }] }, + }, + }, + }, }, }, }); @@ -68,13 +83,37 @@ export async function vorlageExportieren(versionId: string): Promise optionen: [...variable.optionen], pflicht: variable.pflicht, })), - abschnitte: version.abschnitte.map((abschnitt) => ({ - titel: abschnitt.titel, - // Wandert mit: ein Abteilungsname bedeutet in einer anderen Installation - // dasselbe. Der Baustein-Verweis wandert bewusst **nicht** mit (#030). - nurFuerAbteilungen: [...abschnitt.nurFuerAbteilungen], - schritte: abschnitt.schritte.map(schrittAbbilden), - })), + abschnitte: version.abschnitte.map((abschnitt) => { + /* + * **Ein Baustein-Abschnitt wird aufgeloest, nicht abgeschrieben.** + * + * Der Verweis selbst wandert bewusst nicht mit (#030): in der anderen + * Installation gibt es den Baustein vielleicht nicht, und eine Vorlage, + * die auf etwas Fehlendes zeigt, ist kein Austauschformat. Die **Inhalte** + * muessen aber mitwandern — und bei einem Entwurf stehen sie noch nicht + * am Abschnitt, sondern nur am Baustein. + * + * Vorher exportierte ein Entwurf deshalb `{"titel":"wird ersetzt", + * "schritte":[]}`: ein Redakteur brachte eine unveroeffentlichte Vorlage + * in eine zweite Installation, und der geteilte Abschnitt kam dort als + * leerer Abschnitt mit dem Editor-Platzhalter als Namen an. Die Checkliste + * war unvollstaendig, und nichts sagte es (Reviewbefund zu #029). + * + * Bei einer **veroeffentlichten** Version aendert das nichts: dort sind + * die Schritte beim Veroeffentlichen schon in den Abschnitt geschrieben + * worden, und der Titel kommt ohnehin aus dem Baustein. + */ + const quelle = abschnitt.bausteinVersion; + const ausBaustein = quelle !== null && abschnitt.schritte.length === 0; + + return { + titel: ausBaustein ? quelle.titel : abschnitt.titel, + // Wandert mit: ein Abteilungsname bedeutet in einer anderen Installation + // dasselbe. + nurFuerAbteilungen: [...abschnitt.nurFuerAbteilungen], + schritte: (ausBaustein ? quelle.schritte : abschnitt.schritte).map(schrittAbbilden), + }; + }), }, }; } @@ -90,7 +129,15 @@ export async function vorlageExportieren(versionId: string): Promise * nachsichtige Schema soll. Exportiert wird deshalb der rohe Inhalt; nur was gar * kein Objekt ist, gilt als leer. */ -function schrittAbbilden(schritt: VorlagenSchritt): AustauschSchritt { +function schrittAbbilden( + /* + * Nur die gelesenen Felder statt `VorlagenSchritt`: dieselbe Funktion bildet + * jetzt auch einen `BausteinSchritt` ab (Baustein-Abschnitte werden beim + * Export aufgeloest). Die beiden Modelle unterscheiden sich nur darin, woran + * ein Schritt haengt — und das steht ohnehin in keinem Export. + */ + schritt: Pick, +): AustauschSchritt { const roh = schritt.konfig; const konfig = typeof roh === "object" && roh !== null && !Array.isArray(roh) ? (roh as AustauschSchritt["konfig"]) : {}; diff --git a/tests/abteilungsabschnitte.test.ts b/tests/abteilungsabschnitte.test.ts index df7cfad..74900d0 100644 --- a/tests/abteilungsabschnitte.test.ts +++ b/tests/abteilungsabschnitte.test.ts @@ -170,6 +170,28 @@ describe.runIf(datenbankVerfuegbar)("Abteilungsabschnitte am Lauf", () => { expect(ergebnisse.map((e) => e.vorlagenSchritt.titel).sort()).toEqual(["Papiere", "Übergabe"]); }); + it("schreibt im Protokoll die Schritte dieses Laufs, nicht die der Version", async () => { + /* + * **Die Zahl war die der Version** (Reviewbefund zu #029). Bei einer Vorlage + * mit abteilungsgebundenen Abschnitten behauptete der Eintrag „Lauf angelegt“ + * mehr Schritte, als der Lauf hat. + * + * Das Protokoll ist seit #023 ansehbar: eine Zahl darin, die nicht zur Liste + * darunter passt, schickt jemanden auf die Suche nach Schritten, die es nie + * gab. + */ + const { admin, vertrieb, versionId } = await aufbau(VORLAGE); + const lauf = await laufAnlegen({ vorlagenVersionId: versionId, abteilungId: vertrieb.id }, admin); + + const wirklich = await testPrisma.schrittergebnis.count({ where: { laufId: lauf.id } }); + const eintrag = await testPrisma.protokoll.findFirstOrThrow({ + where: { laufId: lauf.id, aktion: "Lauf angelegt" }, + }); + + expect(wirklich).toBe(2); + expect(eintrag.nachher).toMatchObject({ anzahlSchritte: wirklich }); + }); + it("ändert die Abschnitte nicht, wenn später die Abteilung des Laufs wechselt", async () => { /* * Der Grund für das Festschreiben: ein archivierter Lauf muss genau die diff --git a/tests/anhang-route.test.ts b/tests/anhang-route.test.ts new file mode 100644 index 0000000..993c506 --- /dev/null +++ b/tests/anhang-route.test.ts @@ -0,0 +1,276 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { Betriebsmodus, VersionStatus } from "@/generated/prisma/enums"; +import type { Rolle } from "@/generated/prisma/enums"; +import { RechteFehler } from "@/server/auth/sitzungspflicht"; +import { anhangSpeichern } from "@/server/services/anhaenge"; +import { datenbankLeeren, datenbankVerfuegbar, testPrisma } from "./helpers/db"; + +/** + * Die Adresse, die einen Anhang ausliefert (#032). + * + * **Warum eine eigene Datei, obwohl `tests/anhaenge.test.ts` schon den Dienst + * prüft:** Das Akzeptanzkriterium von #032 lautet „ein Anhang eines fremden + * Kunden ist **über seine Adresse** nicht abrufbar“. Geprüft war bis hierher nur + * `anhangLesen` — der Dienst. Die ganze Rechteprüfung im Route Handler ließ sich + * durch einen hartcodierten ADMIN ersetzen, ohne dass ein einziger Test rot + * wurde (Reviewbefund zu #029). + * + * Das ist die teuerste Lücke, die eine Prüfung haben kann: dieser Handler ist + * die **einzige** Stelle der Anwendung, die rohe Dateibytes ausliefert. Fällt + * die Prüfung bei einem Umbau heraus, gibt jedes Foto jedem heraus, der eine + * Kennung hat — und das Tor vor dem Commit bleibt grün. + * + * Gefahren wird deshalb der **Handler selbst**, wie bei den SCIM-Handlern und + * beim Ereignisstrom. Nur die Sitzung wird gestellt; alles andere ist echt, bis + * hinunter zur Datei auf der Platte. + */ + +/* Der Handler wird echt gefahren; nur die Sitzung wird gestellt. */ +vi.mock("@/server/auth/sitzungspflicht", async (original) => { + const echt = await original(); + return { ...echt, rechtErforderlich: vi.fn() }; +}); + +const { rechtErforderlich } = await import("@/server/auth/sitzungspflicht"); +const { GET } = await import("@/app/api/anhaenge/[id]/route"); + +const VORHER = process.env["ANHAENGE_PFAD"]; +let ordner: string; + +beforeAll(async () => { + ordner = await mkdtemp(join(tmpdir(), "laufzettel-anhang-route-")); + process.env["ANHAENGE_PFAD"] = ordner; +}); + +afterAll(async () => { + if (VORHER === undefined) delete process.env["ANHAENGE_PFAD"]; + else process.env["ANHAENGE_PFAD"] = VORHER; + await rm(ordner, { recursive: true, force: true }); +}); + +/** Ein winziges, gültiges PNG. */ +const PNG = Buffer.from( + "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c489" + + "0000000a49444154789c6300010000050001" + + "0d0a2db40000000049454e44ae426082", + "hex", +); + +/** Ein winziges PDF – es wird anders ausgeliefert als ein Bild. */ +const PDF = Buffer.from("%PDF-1.4\n1 0 obj\n<<>>\nendobj\ntrailer\n<<>>\n%%EOF\n", "utf8"); + +async function aufbau() { + const kunde = await testPrisma.kunde.create({ + data: { name: "Eigene Organisation", kuerzel: "EIGEN", istEigeneOrganisation: true }, + }); + await testPrisma.einstellung.create({ data: { id: "global", modus: Betriebsmodus.INTERN } }); + const admin = await testPrisma.benutzer.create({ + data: { email: "admin@firma.example", name: "Admin", rolle: "ADMIN" }, + }); + + const vorlage = await testPrisma.vorlage.create({ data: { name: "PC Neuinstallation" } }); + const version = await testPrisma.vorlagenVersion.create({ + data: { vorlageId: vorlage.id, version: 1, status: VersionStatus.ENTWURF }, + }); + const abschnitt = await testPrisma.vorlagenAbschnitt.create({ + data: { versionId: version.id, titel: "Vorbereitung", position: 0 }, + }); + const schritt = await testPrisma.vorlagenSchritt.create({ + data: { abschnittId: abschnitt.id, titel: "Typenschild fotografieren", position: 0 }, + }); + await testPrisma.vorlagenVersion.update({ + where: { id: version.id }, + data: { status: VersionStatus.VEROEFFENTLICHT, veroeffentlichtAm: new Date() }, + }); + + const lauf = await testPrisma.lauf.create({ + data: { vorlagenVersionId: version.id, titel: "Ein Lauf", kundeId: kunde.id }, + }); + await testPrisma.schrittergebnis.create({ + data: { laufId: lauf.id, vorlagenSchrittId: schritt.id }, + }); + + return { + kunde, + admin: { id: admin.id, rolle: admin.rolle }, + laufId: lauf.id, + schrittId: schritt.id, + }; +} + +/** Legt einen Anhang an und gibt seine Kennung zurück. */ +async function anhangAnlegen( + laufId: string, + schrittId: string, + admin: { id: string; rolle: Rolle }, + abweichend: { dateiname?: string; medientyp?: string; inhalt?: Buffer } = {}, +): Promise { + const { id } = await anhangSpeichern( + { + laufId, + vorlagenSchrittId: schrittId, + dateiname: abweichend.dateiname ?? "typenschild.png", + medientyp: abweichend.medientyp ?? "image/png", + inhalt: new Uint8Array(abweichend.inhalt ?? PNG), + }, + admin, + ); + return id; +} + +const kennung = (id: string) => ({ params: Promise.resolve({ id }) }); +const anfrage = () => new Request("http://127.0.0.1:3300/api/anhaenge/egal"); + +describe.runIf(datenbankVerfuegbar)("Ein Anhang über seine Adresse", () => { + beforeEach(async () => { + await datenbankLeeren(); + vi.mocked(rechtErforderlich).mockReset(); + }); + + it("weist ohne Anmeldung ab, bevor eine Datei gelesen wird", async () => { + const { admin, laufId, schrittId } = await aufbau(); + const id = await anhangAnlegen(laufId, schrittId, admin); + + vi.mocked(rechtErforderlich).mockRejectedValue(new RechteFehler("Nicht angemeldet.")); + const antwort = await GET(anfrage(), kennung(id)); + + expect(antwort.status).toBe(401); + // Und kein Byte der Datei im Rumpf. + expect(await antwort.text()).not.toContain("PNG"); + }); + + it("weist auch ab, wenn die Rolle das Recht lesen nicht hat", async () => { + /* + * `MITARBEITER` hat seit #035 kein `lesen` — die Selbstauskunft ist dessen + * **Ersatz** und kein Zusatz. Dass ein Anhang damit nicht erreichbar ist, + * entscheidet `rechtErforderlich`; hier wird gezeigt, dass der Handler sich + * daran hält und nicht selbst weiterläuft. + */ + const { admin, laufId, schrittId } = await aufbau(); + const id = await anhangAnlegen(laufId, schrittId, admin); + + vi.mocked(rechtErforderlich).mockRejectedValue(new RechteFehler("Dafür fehlt dir die Berechtigung.")); + expect((await GET(anfrage(), kennung(id))).status).toBe(401); + }); + + it("liefert einem berechtigten Benutzer die Datei unverändert", async () => { + const { admin, laufId, schrittId } = await aufbau(); + const id = await anhangAnlegen(laufId, schrittId, admin); + + vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: admin } as never); + const antwort = await GET(anfrage(), kennung(id)); + + expect(antwort.status).toBe(200); + expect(antwort.headers.get("content-type")).toBe("image/png"); + expect(antwort.headers.get("content-length")).toBe(String(PNG.length)); + // Byte für Byte dasselbe – ein Bild, das unterwegs umkodiert wird, ist kein Beweismittel. + expect(Buffer.from(await antwort.arrayBuffer()).equals(PNG)).toBe(true); + }); + + it("zeigt ein Bild inline und lädt ein PDF herunter", async () => { + /* + * Ein PDF kann Skripte enthalten. Ein Anhang, den jemand aus einem fremden + * Lauf öffnet, soll nichts ausführen — deshalb `attachment` und + * `nosniff`. + */ + const { admin, laufId, schrittId } = await aufbau(); + vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: admin } as never); + + const bild = await anhangAnlegen(laufId, schrittId, admin); + const bildAntwort = await GET(anfrage(), kennung(bild)); + expect(bildAntwort.headers.get("content-disposition")).toBe('inline; filename="typenschild.png"'); + + const pdf = await anhangAnlegen(laufId, schrittId, admin, { + dateiname: "abnahme.pdf", + medientyp: "application/pdf", + inhalt: PDF, + }); + const pdfAntwort = await GET(anfrage(), kennung(pdf)); + expect(pdfAntwort.headers.get("content-disposition")).toBe('attachment; filename="abnahme.pdf"'); + expect(pdfAntwort.headers.get("x-content-type-options")).toBe("nosniff"); + }); + + it("lässt keinen Zwischenspeicher die Datei weitergeben", async () => { + // Ein Anhang gehört einem Kunden. Ein öffentlicher Cache gäbe ihn an den + // nächsten Betrachter weiter, und hinter einem Reverse Proxy ist das real. + const { admin, laufId, schrittId } = await aufbau(); + const id = await anhangAnlegen(laufId, schrittId, admin); + + vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: admin } as never); + expect((await GET(anfrage(), kennung(id))).headers.get("cache-control")).toContain("private"); + }); + + it("gibt den Anhang eines fremden Kunden nicht heraus – über seine Adresse", async () => { + /* + * **Das Akzeptanzkriterium von #032, zum ersten Mal an der Adresse + * geprüft.** Der Techniker ist auf einen anderen Kunden beschränkt; die + * Kennung des Anhangs kennt er. Ohne diese Prüfung reicht die Kennung. + */ + const vorher = process.env["OIDC_CUSTOMER_GROUP_PREFIX"]; + process.env["OIDC_CUSTOMER_GROUP_PREFIX"] = "kunde-"; + try { + const { admin, laufId, schrittId } = await aufbau(); + await testPrisma.einstellung.update({ + where: { id: "global" }, + data: { modus: Betriebsmodus.DIENSTLEISTUNG }, + }); + const id = await anhangAnlegen(laufId, schrittId, admin); + + const fremd = await testPrisma.kunde.create({ data: { name: "Fremd GmbH", kuerzel: "FRD" } }); + const techniker = await testPrisma.benutzer.create({ + data: { email: "tina@firma.example", name: "Tina", rolle: "TECHNIKER" }, + }); + await testPrisma.benutzerKunde.create({ + data: { benutzerId: techniker.id, kundeId: fremd.id }, + }); + + vi.mocked(rechtErforderlich).mockResolvedValue({ + benutzer: { id: techniker.id, rolle: techniker.rolle }, + } as never); + const antwort = await GET(anfrage(), kennung(id)); + + /* + * `404` und nicht `403`: nicht vorhanden und nicht sichtbar sind dieselbe + * Antwort. Die Unterscheidung wäre schon eine Auskunft über fremde Kunden. + */ + expect(antwort.status).toBe(404); + expect(antwort.headers.get("content-type")).toContain("application/json"); + const daten = (await antwort.json()) as Record; + // Und die Meldung nennt weder Dateiname noch Kunde. + expect(JSON.stringify(daten)).not.toContain("typenschild"); + expect(JSON.stringify(daten)).not.toContain("Eigene Organisation"); + } finally { + if (vorher === undefined) delete process.env["OIDC_CUSTOMER_GROUP_PREFIX"]; + else process.env["OIDC_CUSTOMER_GROUP_PREFIX"] = vorher; + } + }); + + it("antwortet auf eine Kennung, die es nicht gibt, mit 404", async () => { + const { admin } = await aufbau(); + vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: admin } as never); + + const antwort = await GET(anfrage(), kennung("gibt-es-nicht")); + expect(antwort.status).toBe(404); + }); + + it("sagt bei fehlender Datei, dass die Zeile steht und die Datei fehlt", async () => { + /* + * Der Fall nach einer Sicherung, die nur die Datenbank mitgenommen hat. Die + * Meldung muss das sagen und nicht „nicht gefunden“ behaupten — sonst sucht + * der Betreiber an der falschen Stelle (#032, und siehe + * `docs/betrieb/sicherung.md`). + */ + const { admin, laufId, schrittId } = await aufbau(); + const id = await anhangAnlegen(laufId, schrittId, admin); + await rm(ordner, { recursive: true, force: true }); + + vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: admin } as never); + const antwort = await GET(anfrage(), kennung(id)); + + expect(antwort.status).toBe(404); + expect(String((await antwort.json())["fehler"])).toMatch(/fehlt im Dateisystem/u); + }); +}); diff --git a/tests/bausteine.test.ts b/tests/bausteine.test.ts index 7672e3c..8101514 100644 --- a/tests/bausteine.test.ts +++ b/tests/bausteine.test.ts @@ -506,6 +506,60 @@ describe.runIf(datenbankVerfuegbar)("Bausteine", () => { ]); }); + it("exportiert auch einen Entwurf mit seinen Baustein-Schritten", async () => { + /* + * **Der Fall, der still einen leeren Abschnitt lieferte** (Reviewbefund zu + * #029). Ein Baustein-Abschnitt im Entwurf hat keine eigenen Schritte — + * die entstehen erst beim Veröffentlichen. Der Export schrieb deshalb + * `{"titel":"wird ersetzt","schritte":[]}`, und der Import nahm das + * stillschweigend an: das Schema lässt einen leeren Abschnitt zu. + * + * Ein Redakteur bringt eine noch unveröffentlichte Vorlage in eine zweite + * Installation — genau der Zweck des Formats. Dort kam der geteilte + * Abschnitt unter dem Editor-Platzhalter „wird ersetzt“ und ohne einen + * einzigen Schritt an. Die Checkliste war unvollständig, und nichts sagte + * es. + */ + const { adminId } = await grundbestand(); + const { versionId: bausteinVersionId } = await bausteinMitSchritten( + "Datensicherung prüfen", + "Datensicherung", + ["Sicherung läuft", "Rücksicherung geprüft"], + adminId, + ); + + // Wie `vorlageMitBaustein`, aber **ohne** zu veröffentlichen. + const { vorlage } = await vorlageAnlegen({ name: "Noch Entwurf" }, adminId); + const entwurf = await entwurfHolenOderErzeugen(vorlage.id, adminId); + await entwurfSpeichern(entwurf.id, { + variablen: [], + abschnitte: [ + { titel: "Eigener Abschnitt", schritte: [{ titel: "Eigener Schritt" }] }, + { titel: "wird ersetzt", bausteinVersionId, schritte: [] }, + ], + }); + + const austausch = await vorlageExportieren(entwurf.id); + + expect( + austausch.struktur.abschnitte.map((abschnitt) => ({ + titel: abschnitt.titel, + schritte: abschnitt.schritte.map((schritt) => schritt.titel), + })), + ).toEqual([ + { titel: "Eigener Abschnitt", schritte: ["Eigener Schritt"] }, + // Der Titel aus dem Baustein, nicht der Platzhalter – und die Schritte. + { titel: "Datensicherung", schritte: ["Sicherung läuft", "Rücksicherung geprüft"] }, + ]); + // Und weiterhin kein Verweis: in der anderen Installation gibt es den + // Baustein vielleicht nicht. + expect(JSON.stringify(austausch)).not.toContain(bausteinVersionId); + // Kein Abschnitt ohne Schritte – das ist die Zusage, die vorher brach. + for (const abschnitt of austausch.struktur.abschnitte) { + expect(abschnitt.schritte.length, abschnitt.titel).toBeGreaterThan(0); + } + }); + it("übernimmt beim Import keinen mitgeschickten Verweis", async () => { const { adminId } = await grundbestand(); const { versionId: bausteinVersionId } = await bausteinMitSchritten( diff --git a/tests/benachrichtigungen.test.ts b/tests/benachrichtigungen.test.ts index 5a64de3..f222579 100644 --- a/tests/benachrichtigungen.test.ts +++ b/tests/benachrichtigungen.test.ts @@ -1,4 +1,6 @@ import { createServer, type Server } from "node:http"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; import { Betriebsmodus, @@ -12,6 +14,7 @@ import { abgleichMeldung, laufMeldung, webhookRumpf } from "@/lib/meldungen"; import { SIGNATUR_KOPF, signaturBauen, signaturPruefen } from "@/lib/signatur"; import { verschluesseln } from "@/lib/geheimnisse"; import { + MELDUNGS_PFADE, WARTEZEITEN_MINUTEN, abgleichMeldungEinreihen, laufMeldungEinreihen, @@ -199,6 +202,31 @@ describe("Die Texte der Meldungen", () => { expect(abgleichMeldung("AD", "Grund", link).rumpf).toContain(link); }); + it("zeigt mit jedem Pfad auf eine Seite, die es wirklich gibt", () => { + /* + * **Der Test, der gefehlt hat.** Oben wird geprüft, dass der *übergebene* + * Link im Rumpf steht — das ist richtig und sagt nichts darüber, ob der + * Pfad existiert. `/verwaltung/verzeichnis` stand im Dienst und war ein + * 404: `verwaltung` ist der Name des Rechts, die Seite liegt unter + * `/admin/verzeichnis` (Reviewbefund zu #029). + * + * Ein Pfad in einer Mail wird nicht nachgesehen, sondern geklickt — und + * genau diese eine Meldung geht an **alle** Administratoren, im Moment + * eines Ausfalls. Geprüft wird deshalb gegen das Dateisystem: zu jedem Pfad + * muss unter `src/app/(app)/` eine `page.tsx` liegen. + */ + const wurzel = new URL("../src/app/(app)", import.meta.url).pathname; + for (const pfad of Object.values(MELDUNGS_PFADE)) { + const teile = pfad.replace(/^\//u, "").split("/"); + // Ein Pfad, der auf eine Kennung endet, liegt unter `[id]`. + const kandidaten = [join(wurzel, ...teile, "page.tsx"), join(wurzel, ...teile, "[id]", "page.tsx")]; + expect( + kandidaten.some((datei) => existsSync(datei)), + `${pfad} zeigt auf keine Seite – gesucht: ${kandidaten.join(" oder ")}`, + ).toBe(true); + } + }); + it("nimmt keinen Schrittinhalt auf — der Typ lässt es nicht zu", () => { /* * Die wichtigste Zusage dieses Meilensteins. Geprueft wird sie hier ueber diff --git a/tests/betriebsstand.test.ts b/tests/betriebsstand.test.ts index 38a7428..6235c46 100644 --- a/tests/betriebsstand.test.ts +++ b/tests/betriebsstand.test.ts @@ -1,5 +1,6 @@ -import { readFileSync } from "node:fs"; +import { readFileSync, readdirSync } from "node:fs"; import { describe, expect, it } from "vitest"; +import { E2E_PRAEFIX } from "./e2e/aufraeumen"; import { parse } from "yaml"; /** @@ -83,6 +84,62 @@ describe("Betriebsstand: docker-compose.yml", () => { }); }); +describe("Die Ende-zu-Ende-Abläufe räumen auf", () => { + /* + * **Der Test, der die Lücke beim nächsten Meilenstein fängt** (#027). + * + * In der Entwicklungsdatenbank standen 105 Vorlagen mit Namen wie + * `ZZ E2E Editor 1790324192596 C`. Der Editor-Ablauf räumt seit #018 selbst + * auf — aber zwei Abläufe aus Phase 2 (`anhaenge`, `live`) legten Zeilen an + * und hatten keinen Nachlauf. Das fiel nicht auf, weil die Suite davon nichts + * sieht: Reste sind kein Testfehler, sie sind ein Schaden an allem, was + * danach in dieselbe Datenbank sieht. + * + * Geräumt wird jetzt zentral in `tests/e2e/aufraeumen.ts`, vor **und** nach + * dem Lauf. Damit das trägt, muss zweierlei gelten, und beides steht hier: + * jeder Ablauf benutzt das Präfix, und der Vor- und Nachlauf sind registriert. + */ + + const ABLAEUFE = readdirSync("tests/e2e").filter((datei) => datei.endsWith(".spec.ts")); + + it("findet überhaupt Abläufe – sonst prüft der Test darunter nichts", () => { + // Ohne diese Zeile wäre eine leere Liste ein grüner Test. + expect(ABLAEUFE.length).toBeGreaterThan(10); + }); + + it("benennt jede angelegte Zeile mit dem Präfix, an dem aufgeräumt wird", () => { + /* + * Geprüft wird die **Umkehrung**: ein Ablauf, der Daten anlegt, ohne das + * Präfix zu benutzen, ist von keinem Aufräumen erfasst. Erkennbar ist das + * daran, dass er ein Formularfeld „Name“ oder „Titel“ füllt. + */ + const ohnePraefix: string[] = []; + for (const datei of ABLAEUFE) { + const inhalt = readFileSync(`tests/e2e/${datei}`, "utf8"); + const legtAn = /getByLabel\((?:"|')(?:Name|Titel|Bezeichnung)/u.test(inhalt); + if (legtAn && !inhalt.includes(E2E_PRAEFIX)) ohnePraefix.push(datei); + } + expect( + ohnePraefix, + `Diese Abläufe legen Zeilen an, ohne „${E2E_PRAEFIX}“ zu benutzen — das zentrale ` + + "Aufräumen findet sie nicht wieder.", + ).toEqual([]); + }); + + it("hat Vorlauf und Nachlauf registriert", () => { + /* + * Beides, und das ist der Punkt: der Nachlauf läuft nach einem Testfehler + * und nach einem Zeitablauf, aber nicht nach `Strg+C`. Dann findet der + * Vorlauf des nächsten Durchgangs die Reste — und ein Vorlauf lässt sich + * nicht überspringen. + */ + const konfiguration = readFileSync("playwright.config.ts", "utf8"); + expect(konfiguration).toContain("global-setup"); + expect(konfiguration).toContain("global-teardown"); + expect(readFileSync("tests/e2e/global-setup.ts", "utf8")).toContain("e2eAufraeumenMitBericht"); + }); +}); + describe("Betriebsstand: Entwicklungsdatenbank", () => { it("bindet nur auf localhost und auf einen eigenen, umstellbaren Port", () => { const stand = compose("docker-compose.dev.yml") as unknown as { diff --git a/tests/e2e/aufraeumen.ts b/tests/e2e/aufraeumen.ts new file mode 100644 index 0000000..34a74de --- /dev/null +++ b/tests/e2e/aufraeumen.ts @@ -0,0 +1,185 @@ +import { prisma } from "@/lib/db"; + +/** + * Was die Ende-zu-Ende-Abläufe hinterlassen, wieder wegräumen (#027). + * + * **Alles, was ein Ablauf anlegt, beginnt mit `ZZ`.** Das Präfix sortiert + * die Zeilen in jeder Liste ans Ende und ist zugleich der Griff, an dem sie + * wieder herausgehen — ohne gemerkte Kennungen, denn ein Lauf, der mitten im + * Ablauf abbricht, merkt sich nichts. + * + * **Warum vor *und* nach dem Lauf geräumt wird.** Ein `afterAll` je Datei und + * ein `globalTeardown` laufen bei einem Testfehler und bei einem Zeitablauf — + * aber nicht, wenn jemand `Strg+C` drückt oder der Prozess hart stirbt. Dann + * bleiben die Zeilen stehen, und der nächste Lauf findet sie vor. Deshalb räumt + * derselbe Code auch im Vorlauf: was vom letzten Mal übrig ist, geht weg, bevor + * der erste Ablauf startet. Ein Vorlauf kann nicht übersprungen werden. + * + * Aufgefallen ist das Ganze, weil in der Entwicklungsdatenbank 105 Vorlagen mit + * Namen wie `ZZ E2E Editor 1790324192596 C` standen — aus einem Fenster von + * sechzehn Stunden, in dem noch kein Ablauf aufräumte. Der Schaden liegt nicht + * bei den Tests, sondern bei allem, was **danach** in dieselbe Datenbank sieht: + * die Vorlagenliste, die Screenshots, jede Abfrage von Hand. Ein Fehler, der + * „zu viele Treffer“ heißt, kostet dann eine Stunde. + */ + +/** + * Das Präfix, das jeder Ablauf seinen Zeilen gibt. + * + * **`ZZ` und nicht `ZZ E2E`**, weil die Abläufe es in fünf Schreibweisen + * benutzen: `ZZ E2E Editor …`, `ZZ-E2E--PC-01` (Gerätenamen dürfen keine + * Leerzeichen haben), `ZZPruef…`, `ZZQuelle…` und `ZZ Abteilung …`. Alle fünf + * sind sinnvoll — ein Hostname sieht anders aus als ein Vorlagenname —, und + * alle fünf beginnen mit `ZZ`. Darauf zu greifen deckt sie ab, ohne sechzehn + * Dateien umzubenennen. + * + * Zwei Buchstaben sind kurz. Das ist hier in Ordnung: geräumt wird nur beim + * Ende-zu-Ende-Lauf und nur gegen die Entwicklungsdatenbank, und `ZZ` am + * **Anfang** eines Namens ist in echten Daten dieses Hauses nichts, was jemand + * tippt. Der Grund für die Wahl ist ohnehin derselbe: `ZZ` sortiert in jeder + * Liste ans Ende. + */ +export const E2E_PRAEFIX = "ZZ"; + +/** Was aufgeräumt wurde — damit der Aufrufer es melden kann. */ +export type Aufgeraeumt = Readonly>; + +/** + * Räumt alles mit `ZZ` weg und meldet, was es war. + * + * Die Reihenfolge ist nicht beliebig: erst was an einem Lauf hängt, dann die + * Läufe, dann woran ein Lauf hängen könnte. Ein Fremdschlüssel, der sich + * beschwert, ist dabei **kein** Fehler dieser Funktion, sondern die Auskunft, + * dass ein Ablauf etwas verknüpft hat, das er nicht aufräumen kann — dann soll + * es auffallen und nicht stillschweigend bleiben. + */ +export async function e2eRestlosAufraeumen(): Promise { + const praefix = { startsWith: E2E_PRAEFIX }; + const gezaehlt: Record = {}; + const merken = (was: string, anzahl: number): void => { + if (anzahl > 0) gezaehlt[was] = (gezaehlt[was] ?? 0) + anzahl; + }; + + /* + * `Protokoll.entitaetId` ist **kein** Fremdschlüssel: die Zeilen überleben + * ihren Gegenstand absichtlich (ein archivierter Lauf soll seine Geschichte + * behalten). Hier heißt das, dass sie als Waisen stehen bleiben, wenn sie + * nicht mitgenommen werden — darum werden die Kennungen vorher gesammelt. + */ + const laeufe = await prisma.lauf.findMany({ where: { titel: praefix }, select: { id: true } }); + const vorlagen = await prisma.vorlage.findMany({ + where: { name: praefix }, + select: { id: true, versionen: { select: { id: true } } }, + }); + const bausteine = await prisma.baustein.findMany({ + where: { name: praefix }, + select: { id: true, versionen: { select: { id: true } } }, + }); + const geraete = await prisma.geraet.findMany({ + where: { OR: [{ bezeichnung: praefix }, { hostname: praefix }] }, + select: { id: true }, + }); + const personen = await prisma.mitarbeiter.findMany({ + where: { anzeigename: praefix }, + select: { id: true }, + }); + const abteilungen = await prisma.abteilung.findMany({ + where: { name: praefix }, + select: { id: true }, + }); + const quellen = await prisma.verzeichnisquelle.findMany({ + where: { name: praefix }, + select: { id: true }, + }); + const benutzer = await prisma.benutzer.findMany({ + where: { name: praefix }, + select: { id: true }, + }); + + /* + * **Waisen werden nicht erraten.** Eine Protokollzeile, deren Gegenstand schon + * fort ist, bleibt stehen — ihre `entitaetId` ist hier nicht mehr zu finden. + * Sie über den Namen im Json zu greifen wäre naheliegend und falsch: derselbe + * Name steht auch in echter Historie einer Zeile, die es weiterhin gibt (etwa + * „Version veröffentlicht“ mit der Liste der eingesetzten Bausteine). Dass + * eine solche Zeile ihren Gegenstand überlebt, ist genau der Grund, warum + * `entitaetId` kein Fremdschlüssel ist. + */ + const laufIds = laeufe.map((zeile) => zeile.id); + const betroffen = [ + ...laufIds, + ...vorlagen.flatMap((v) => [v.id, ...v.versionen.map((version) => version.id)]), + ...bausteine.flatMap((b) => [b.id, ...b.versionen.map((version) => version.id)]), + ...geraete.map((zeile) => zeile.id), + ...personen.map((zeile) => zeile.id), + ...abteilungen.map((zeile) => zeile.id), + ...quellen.map((zeile) => zeile.id), + ...benutzer.map((zeile) => zeile.id), + ]; + + if (laufIds.length > 0) { + merken("Anhänge", (await prisma.anhang.deleteMany({ where: { laufId: { in: laufIds } } })).count); + merken("Kommentare", (await prisma.kommentar.deleteMany({ where: { laufId: { in: laufIds } } })).count); + merken( + "Schrittergebnisse", + (await prisma.schrittergebnis.deleteMany({ where: { laufId: { in: laufIds } } })).count, + ); + merken( + "Zustellungen", + (await prisma.zustellung.deleteMany({ where: { laufId: { in: laufIds } } })).count, + ); + } + + if (betroffen.length > 0) { + merken( + "Protokollzeilen", + ( + await prisma.protokoll.deleteMany({ + where: { OR: [{ entitaetId: { in: betroffen } }, { laufId: { in: laufIds } }] }, + }) + ).count, + ); + } + + merken("Läufe", (await prisma.lauf.deleteMany({ where: { titel: praefix } })).count); + merken("Vorlagen", (await prisma.vorlage.deleteMany({ where: { name: praefix } })).count); + merken("Bausteine", (await prisma.baustein.deleteMany({ where: { name: praefix } })).count); + merken( + "Geräte", + ( + await prisma.geraet.deleteMany({ + where: { OR: [{ bezeichnung: praefix }, { hostname: praefix }] }, + }) + ).count, + ); + merken("Personen", (await prisma.mitarbeiter.deleteMany({ where: { anzeigename: praefix } })).count); + merken("Abteilungen", (await prisma.abteilung.deleteMany({ where: { name: praefix } })).count); + merken( + "Verzeichnisquellen", + (await prisma.verzeichnisquelle.deleteMany({ where: { name: praefix } })).count, + ); + merken("Benutzer", (await prisma.benutzer.deleteMany({ where: { name: praefix } })).count); + + return gezaehlt; +} + +/** Dasselbe mit einer Zeile Ausgabe — für Vor- und Nachlauf. */ +export async function e2eAufraeumenMitBericht(wann: "vor" | "nach"): Promise { + const gezaehlt = await e2eRestlosAufraeumen(); + const summe = Object.values(gezaehlt).reduce((a, b) => a + b, 0); + if (summe === 0) return; + + const liste = Object.entries(gezaehlt) + .map(([was, anzahl]) => `${anzahl} ${was}`) + .join(", "); + /* + * Der Vorlauf **sagt es laut**: Zeilen, die er findet, sind der Rückstand + * eines abgebrochenen Durchgangs. Still wegzuräumen hieße, die Ursache zu + * verdecken — und ein Ablauf, der jedes Mal abbricht, fiele nie auf. + */ + console.info( + wann === "vor" + ? `E2E-Aufräumen (Rückstand aus einem früheren Lauf): ${liste}.` + : `E2E-Aufräumen: ${liste}.`, + ); +} diff --git a/tests/e2e/global-setup.ts b/tests/e2e/global-setup.ts index 1c7eb39..abf308d 100644 --- a/tests/e2e/global-setup.ts +++ b/tests/e2e/global-setup.ts @@ -1,7 +1,9 @@ import { chromium } from "@playwright/test"; import "dotenv/config"; import { prisma } from "@/lib/db"; +import { ABTEILUNGEN } from "@/../prisma/seed-daten/stammdaten"; import { anmelden, SEED_GERAET, SEED_TAG, SEED_VORLAGE } from "./helfer"; +import { e2eAufraeumenMitBericht } from "./aufraeumen"; /** * Was vor dem ersten Ablauf stimmen muss (#018). @@ -23,6 +25,14 @@ function fehlen(was: string, abhilfe: string): never { } export default async function vorbereiten(): Promise { + /* + * **Zuerst aufraeumen, dann pruefen** (#027). Was hier noch steht, ist der + * Rueckstand eines abgebrochenen Durchgangs: `globalTeardown` und die + * `afterAll`-Bloecke laufen bei einem Testfehler, aber nicht bei `Strg+C`. + * Ein Vorlauf laesst sich nicht ueberspringen — das ist der Unterschied. + */ + await e2eAufraeumenMitBericht("vor"); + if (!process.env["SEED_ADMIN_EMAIL"] || !process.env["SEED_ADMIN_PASSWORD"]) { fehlen( "SEED_ADMIN_EMAIL oder SEED_ADMIN_PASSWORD fehlt.", @@ -59,6 +69,37 @@ export default async function vorbereiten(): Promise { fehlen(`Das Beispielgerät ${SEED_GERAET} (${SEED_TAG}) fehlt.`, "pnpm db:seed laufen lassen."); } + /* + * 2c. Stehen die **Werte** des Seed-Bestands noch, nicht nur die Zeilen? + * + * Der Onboarding-Ablauf prueft den `New-ADUser`-Befehl **zeichengenau**, und + * darin steckt die Ziel-OU der Abteilung. In der Entwicklungsdatenbank stand + * an „Buchhaltung“ `DC=firma,DC=de` statt `DC=firma,DC=local` — von Hand + * geaendert oder aus einer aelteren Seed-Fassung. Der Seed setzt bestehende + * Abteilungen absichtlich **nicht** zurueck (gepflegte Daten sollen bleiben), + * also blieb die Abweichung stehen. + * + * Der Ablauf scheiterte daran mit „expected … received …“ — einer Meldung, die + * nach einem Fehler im Befehlsbau aussieht und keiner war. Hier faellt es + * vorher auf, und die Meldung sagt, was zu tun ist (Reviewbefund zu #029). + */ + for (const vorgabe of ABTEILUNGEN) { + const abteilung = await prisma.abteilung.findFirst({ + where: { name: vorgabe.name }, + select: { zielOu: true, standardGruppen: true }, + }); + if (!abteilung) continue; + if (abteilung.zielOu !== vorgabe.zielOu) { + fehlen( + `Die Abteilung „${vorgabe.name}“ hat eine abweichende Ziel-OU: ` + + `„${abteilung.zielOu}“ statt „${vorgabe.zielOu}“.`, + "Der Onboarding-Ablauf prüft den erzeugten Befehl zeichengenau. Entweder die Ziel-OU " + + "unter Admin → Abteilungen zurückstellen, oder gegen eine frische Datenbank fahren: " + + "pnpm db:seed:reset.", + ); + } + } + /* * 2b. In welchem Betriebsmodus laeuft die Installation? * diff --git a/tests/e2e/global-teardown.ts b/tests/e2e/global-teardown.ts new file mode 100644 index 0000000..48ee667 --- /dev/null +++ b/tests/e2e/global-teardown.ts @@ -0,0 +1,18 @@ +import { prisma } from "@/lib/db"; +import { e2eAufraeumenMitBericht } from "./aufraeumen"; + +/** + * Nach dem letzten Ablauf: wegräumen, was die Abläufe angelegt haben (#027). + * + * Läuft auch dann, wenn Abläufe fehlgeschlagen sind — Playwright ruft den + * Nachlauf nach dem Lauf, nicht nach dem Erfolg. Was er **nicht** abdeckt, ist + * ein harter Abbruch (`Strg+C`, `kill`); dafür räumt derselbe Code im Vorlauf, + * siehe `aufraeumen.ts`. + */ +export default async function aufraeumen(): Promise { + try { + await e2eAufraeumenMitBericht("nach"); + } finally { + await prisma.$disconnect(); + } +} diff --git a/tests/e2e/laeufe.spec.ts b/tests/e2e/laeufe.spec.ts index 6e4e524..5ecf245 100644 --- a/tests/e2e/laeufe.spec.ts +++ b/tests/e2e/laeufe.spec.ts @@ -273,7 +273,19 @@ test.describe("Der Zustands-Endpunkt", () => { expect(antwort.headers()["cache-control"]).toBe("no-store"); const zustand: unknown = await antwort.json(); + /* + * Die Felder **namentlich** und vollständig: der Zustand ist die einzige + * Antwort, die alle zehn Sekunden (seit #033 bei jeder Änderung) hinausgeht, + * und ein Feld, das sich hier einschleicht, wandert ungesehen mit. Deshalb + * `toEqual` und nicht `toContain`. + * + * `anzahlAnhaenge` kam mit #033 dazu und fehlte hier — der Ablauf war seitdem + * rot, und niemand hat es gemerkt, weil `pnpm test:e2e` nicht im Tor vor dem + * Commit steht (Reviewbefund zu #029). Wer hier ein Feld ergänzt, trägt es + * ein; genau das ist der Zweck dieser Liste. + */ expect(Object.keys(zustand as object).sort()).toEqual([ + "anzahlAnhaenge", "anzahlKommentare", "anzahlPflichtOffen", "geaendertAm", diff --git a/tests/e2e/mobil.spec.ts b/tests/e2e/mobil.spec.ts index 828c926..f4e4cfb 100644 --- a/tests/e2e/mobil.spec.ts +++ b/tests/e2e/mobil.spec.ts @@ -184,6 +184,22 @@ const SEITEN: readonly Hauptseite[] = [ */ { name: "Admin: Verzeichnis", pfad: () => "/admin/verzeichnis", nurModus: "INTERN" }, { name: "Kunden", pfad: () => "/kunden", nurModus: "DIENSTLEISTUNG" }, + /* + * **Die Seiten aus Phase 2.** Sie fehlten hier, obwohl jedes der Issues eine + * Klickprobe bei 390 px von Hand berichtet — eine Probe von Hand ist kein + * Regressionstest (Reviewbefund zu #029). Gerade die Bausteinliste und das + * Zustellungs-Protokoll sind Tabellen, und Tabellen sind die Stellen, an denen + * horizontales Scrollen entsteht. + * + * Nicht dabei: `/meine-geraete` (#035) — es verlangt das Recht + * `selbstauskunft`, und das hat nur `MITARBEITER`. Dieser Ablauf meldet sich + * als Admin an, der dort „kein Zugriff“ sähe; gemessen würde die + * Verweigerungsseite. Dafür braucht es ein eigenes Konto, siehe #037. + */ + { name: "Vorlagen: Bausteine", pfad: () => "/vorlagen/bausteine" }, + { name: "Admin: Schnellstart", pfad: () => "/admin/schnellstart" }, + { name: "Admin: Protokoll", pfad: () => "/admin/protokoll" }, + { name: "Admin: Zustellungen", pfad: () => "/admin/zustellungen" }, ]; type Messung = { diff --git a/tests/e2e/stammdaten.spec.ts b/tests/e2e/stammdaten.spec.ts index 0143cdf..65fd486 100644 --- a/tests/e2e/stammdaten.spec.ts +++ b/tests/e2e/stammdaten.spec.ts @@ -112,7 +112,17 @@ test.describe("Personen", () => { await treffer.click(); await page.waitForURL(/\/mitarbeiter\/[^/]+$/); - await expect(page.getByText(/kommt aus dem Verzeichnis/)).toBeVisible(); + /* + * **Der ganze Satz, nicht nur der Anfang.** „kommt aus dem Verzeichnis“ + * allein traf seit #020 zwei Stellen: dieselbe Auskunft steht auch am + * Vorgesetzten-Feld. Playwright wies das als „strict mode violation“ ab, und + * der Ablauf war seitdem rot — unbemerkt, weil `pnpm test:e2e` nicht im Tor + * vor dem Commit steht (Reviewbefund zu #029). + * + * Hier zählt die Auskunft an den **Stammdaten**: dass sie dort gepflegt + * werden und eine Änderung hier der nächste Abgleich zurücksetzen würde. + */ + await expect(page.getByText(/kommt aus dem Verzeichnis\. Die Stammdaten werden dort/u)).toBeVisible(); await expect(page.getByLabel("Vorname")).toHaveAttribute("readonly", ""); await expect(page.getByRole("button", { name: "Stammdaten speichern" })).toHaveCount(0); // Deaktivieren muss trotzdem gehen: das Offboarding wartet nicht auf den diff --git a/tests/e2e/vorlagen-editor.spec.ts b/tests/e2e/vorlagen-editor.spec.ts index 374dba4..04f61c3 100644 --- a/tests/e2e/vorlagen-editor.spec.ts +++ b/tests/e2e/vorlagen-editor.spec.ts @@ -132,8 +132,15 @@ test.describe("Vorlagen-Editor", () => { .evaluateAll((felder) => felder.map((feld) => (feld as HTMLInputElement).value)); expect(await werte()).toEqual(["Erster", "Zweiter"]); - // Der Anfasser ist mindestens 44 px hoch – sonst trifft ihn niemand. + /* + * **Erst in den Blick scrollen.** Am Handy stand der erste Anfasser nach dem + * Anlegen zweier Abschnitte bei `y = -115`, also oberhalb des Sichtfensters; + * der Touch-Start landete damit außerhalb und bewegte nichts. Das fiel + * nicht auf, weil `pnpm test:e2e` nicht im Tor vor dem Commit steht + * (Reviewbefund zu #029). + */ const anfasser = page.getByRole("button", { name: "Verschieben" }).first(); + await anfasser.scrollIntoViewIfNeeded(); const kasten = await anfasser.boundingBox(); expect(kasten?.height ?? 0).toBeGreaterThanOrEqual(44); expect(kasten?.width ?? 0).toBeGreaterThanOrEqual(44); @@ -142,13 +149,32 @@ test.describe("Vorlagen-Editor", () => { // eine Wischgeste ein Bildlauf bleibt und kein Verschieben. const x = kasten!.x + kasten!.width / 2; const y = kasten!.y + kasten!.height / 2; + + /* + * **Die Zugweite wird gemessen, nicht gesetzt.** Vorher waren es fest + * 12 × 25 px = 300; der Abstand zwischen den beiden Anfassern ist aber + * 532 px, seit mit #030 und #031 Bausteinverweis und Abteilungsfilter in + * jeden Abschnitt gekommen sind. Der Finger kam damit nicht mehr über den + * zweiten Abschnitt, und die Reihenfolge blieb stehen. + * + * Eine feste Weite in einem Sortiertest ist eine Zusage über das Layout, + * die niemand pflegt. Gemessen wird sie bei jedem Lauf richtig — und wenn + * der Abstand sich wieder ändert, bleibt der Test grün, statt eine + * Pixelzahl nachzuziehen. + */ + const zweiterAnfasser = await page.getByRole("button", { name: "Verschieben" }).nth(1).boundingBox(); + // Ein Stück über die Mitte des zweiten hinaus, sonst entscheidet dnd-kit + // sich fuer „bleibt“. + const weite = (zweiterAnfasser?.y ?? y + 300) - kasten!.y + kasten!.height; + const schritte = 20; + const sitzung = await page.context().newCDPSession(page); await sitzung.send("Input.dispatchTouchEvent", { type: "touchStart", touchPoints: [{ x, y }] }); await page.waitForTimeout(350); - for (let schritt = 1; schritt <= 12; schritt += 1) { + for (let schritt = 1; schritt <= schritte; schritt += 1) { await sitzung.send("Input.dispatchTouchEvent", { type: "touchMove", - touchPoints: [{ x, y: y + schritt * 25 }], + touchPoints: [{ x, y: y + (weite * schritt) / schritte }], }); await page.waitForTimeout(40); } diff --git a/tests/graph.test.ts b/tests/graph.test.ts index f6594c9..7c9634d 100644 --- a/tests/graph.test.ts +++ b/tests/graph.test.ts @@ -8,6 +8,7 @@ import { benutzerZuordnen, ersteAdresse, graphEintraegeStroemen, + naechsteSeite, } from "@/server/graph/leser"; import { GraphFehler, graphZugangAus, tokenHolen, tokenSpeicherLeeren } from "@/server/graph/zugang"; import { vorgesetzteAufloesen } from "@/server/ldap/zuordnung"; @@ -385,6 +386,33 @@ describe("Der Token-Austausch", () => { }); describe("Der Seitenabruf", () => { + it("folgt einem nextLink nur auf denselben Host", () => { + /* + * **Jede Seite wird mit dem Zugangstoken geholt**, und das gilt mandantenweit + * (`User.Read.All`). Der Antwort von Graph über TLS zu trauen ist + * vertretbar — aber `GRAPH_API_URL` ist absichtlich einstellbar, für + * nationale Clouds und für den Doppelgänger in diesem Test. Ein Tippfehler + * darin oder ein aufbrechender Firmenproxy dazwischen schickte das Token + * sonst an eine fremde Adresse (Reviewbefund zu #029). + */ + const basis = "https://graph.microsoft.com"; + expect(naechsteSeite(`${basis}/v1.0/users?$skiptoken=abc`, basis)).toBe( + `${basis}/v1.0/users?$skiptoken=abc`, + ); + + // Anderer Host: Abruf endet, statt das Token mitzuschicken. + expect(naechsteSeite("https://boese.example/v1.0/users", basis)).toBeUndefined(); + // Auch ein anderes Schema oder ein anderer Port ist ein anderer Ursprung. + expect(naechsteSeite("http://graph.microsoft.com/v1.0/users", basis)).toBeUndefined(); + expect(naechsteSeite("https://graph.microsoft.com:8443/v1.0/users", basis)).toBeUndefined(); + + // Und das Übliche: nichts, leer, kein gültiges URL. + expect(naechsteSeite(undefined, basis)).toBeUndefined(); + expect(naechsteSeite("", basis)).toBeUndefined(); + expect(naechsteSeite("kein-url", basis)).toBeUndefined(); + expect(naechsteSeite(42, basis)).toBeUndefined(); + }); + it("folgt @odata.nextLink über drei Seiten", async () => { aufzeichnung.seiten = [[nutzer(1), nutzer(2)], [nutzer(3), nutzer(4)], [nutzer(5)]]; diff --git a/tests/lauf-ereignisse.test.ts b/tests/lauf-ereignisse.test.ts index 19e7ee5..e9bb9a4 100644 --- a/tests/lauf-ereignisse.test.ts +++ b/tests/lauf-ereignisse.test.ts @@ -1,5 +1,6 @@ import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { Betriebsmodus, LaufStatus, VersionStatus } from "@/generated/prisma/enums"; +import type { Rolle } from "@/generated/prisma/enums"; import { HERZSCHLAG, KANAL, herzschlagMs, maxStroeme, sseEreignis, willStrom } from "@/lib/sse"; import { STROM_GESCHLOSSEN, @@ -7,6 +8,7 @@ import { sollZurueckfallen, } from "@/components/laeufe/lauf-aktualisierung"; import { RechteFehler } from "@/server/auth/sitzungspflicht"; +import type { Sitzung } from "@/lib/sitzung"; import { aufLaufHoeren, ereignisseSchliessen, stroemeOffen } from "@/server/lauf-ereignisse"; import { datenbankLeeren, datenbankVerfuegbar, testPrisma } from "./helpers/db"; @@ -305,10 +307,29 @@ describe.runIf(datenbankVerfuegbar)("Der Route Handler", () => { delete process.env["SSE_MAX_STROEME"]; }); - function angemeldetAls(benutzer: { id: string; rolle: "ADMIN" }): void { + /** + * Die Sitzung wird mit ihrer **echten Form** gestellt, nicht als `{}`. + * + * Vorher stand hier ein leeres Objekt, und genau daran ist ein Befund + * vorbeigekommen: der Strom prueft die Sitzung seit #029 beim Herzschlag nach, + * und mit einer Attrappe ohne `benutzerId` liesse sich das nicht zeigen. Eine + * Attrappe, die nicht die Form der Wirklichkeit hat, prueft etwas anderes als + * die Wirklichkeit — dieselbe Lehre wie beim Graph-Doppelgaenger in #028. + */ + function sitzungFuer(benutzerId: string, ausgestelltAm = Math.floor(Date.now() / 1000)): Sitzung { + return { + benutzerId, + email: "wer@firma.example", + name: "Wer", + herkunft: "passwort", + ausgestelltAm, + }; + } + + function angemeldetAls(benutzer: { id: string; rolle: Rolle }): void { vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: { id: benutzer.id, rolle: benutzer.rolle }, - sitzung: {}, + sitzung: sitzungFuer(benutzer.id), } as unknown as Awaited>); } @@ -365,7 +386,7 @@ describe.runIf(datenbankVerfuegbar)("Der Route Handler", () => { }); vi.mocked(rechtErforderlich).mockResolvedValue({ benutzer: { id: leser.id, rolle: "LESER" }, - sitzung: {}, + sitzung: sitzungFuer(leser.id), } as unknown as Awaited>); const antwort = await GET(anfrage("text/event-stream"), { @@ -458,6 +479,78 @@ describe.runIf(datenbankVerfuegbar)("Der Route Handler", () => { expect(gelesen.split(HERZSCHLAG).length - 1).toBeGreaterThanOrEqual(3); }); + it("beendet den Strom, wenn das Konto deaktiviert wird", async () => { + /* + * **Ein offener Strom hat das Deaktivieren überlebt** (Reviewbefund zu + * #029). Das Recht wurde genau einmal geprüft, beim Aufbau, und das Paar aus + * Kennung und Rolle für die Lebensdauer des Stroms festgehalten. Ein + * Administrator deaktivierte ein Konto — und eine offen gelassene Lasche auf + * dem Schreibtisch des Betroffenen bekam weiter live Status, Schrittzustände + * und die Zahl der Kommentare, bis jemand sie schloss. + * + * `docs/architektur.md` verspricht „ein deaktivierter Benutzer wird auch mit + * gültigem Cookie abgewiesen“. Für einen offenen Strom galt das nicht. + */ + process.env["SSE_HERZSCHLAG_MS"] = "40"; + const { laufId, admin } = await aufbau(); + angemeldetAls(admin as { id: string; rolle: "ADMIN" }); + + const antwort = await GET(anfrage("text/event-stream"), { + params: Promise.resolve({ id: laufId }), + }); + + await testPrisma.benutzer.update({ where: { id: admin.id }, data: { aktiv: false } }); + + const gelesen = await stromLesen(antwort, (text) => text.includes("nicht-angemeldet")); + expect(gelesen).toContain("nicht-angemeldet"); + // Und der Strom ist auch wirklich freigegeben, nicht nur stillgelegt. + await warteAuf(() => stroemeOffen() === 0, "Der Strom nach dem Deaktivieren"); + }); + + it("beendet den Strom, wenn die Sitzungen widerrufen werden", async () => { + /* + * Derselbe Weg, anderer Anlass: ein Administrator beendet die Sitzungen + * eines Benutzers (`sitzungenUngueltigAb`), ohne das Konto zu deaktivieren — + * etwa nach einem verlorenen Notebook. Dann muss der Strom genauso enden. + */ + process.env["SSE_HERZSCHLAG_MS"] = "40"; + const { laufId, admin } = await aufbau(); + angemeldetAls(admin as { id: string; rolle: "ADMIN" }); + + const antwort = await GET(anfrage("text/event-stream"), { + params: Promise.resolve({ id: laufId }), + }); + + await testPrisma.benutzer.update({ + where: { id: admin.id }, + // Eine Minute in der Zukunft: die Sitzung dieses Tests ist „von vorher“. + data: { sitzungenUngueltigAb: new Date(Date.now() + 60_000) }, + }); + + expect(await stromLesen(antwort, (text) => text.includes("nicht-angemeldet"))).toContain( + "nicht-angemeldet", + ); + }); + + it("hält den Strom, solange das Konto gilt", async () => { + /* + * Die Gegenrichtung, und sie ist nötig: eine Nachprüfung, die **immer** + * beendet, sähe im Test oben genauso aus. Hier bleibt der Strom über + * mehrere Herzschläge offen, ohne dass sich am Konto etwas ändert. + */ + process.env["SSE_HERZSCHLAG_MS"] = "40"; + const { laufId, admin } = await aufbau(); + angemeldetAls(admin as { id: string; rolle: "ADMIN" }); + + const antwort = await GET(anfrage("text/event-stream"), { + params: Promise.resolve({ id: laufId }), + }); + const gelesen = await stromLesen(antwort, (text) => text.split(HERZSCHLAG).length - 1 >= 3); + + expect(gelesen).not.toContain("nicht-angemeldet"); + expect(gelesen.split(HERZSCHLAG).length - 1).toBeGreaterThanOrEqual(3); + }); + it("fällt bei erreichter Obergrenze auf eine gewöhnliche Antwort zurück", async () => { process.env["SSE_MAX_STROEME"] = "1"; const { laufId, admin } = await aufbau(); diff --git a/tests/selbstauskunft.test.ts b/tests/selbstauskunft.test.ts index 6aa0ea3..47fd3b3 100644 --- a/tests/selbstauskunft.test.ts +++ b/tests/selbstauskunft.test.ts @@ -411,6 +411,42 @@ describe.runIf(datenbankVerfuegbar)("Zuordnung bei der Anmeldung", () => { // Ohne Urheber: das Konto gibt es an der Stelle vielleicht noch nicht. expect(protokoll[0]?.benutzerId).toBeNull(); expect(JSON.stringify(protokoll[0]?.nachher)).toContain("mhuber@firma.local"); + expect(protokoll[0]?.nachher).toMatchObject({ treffer: 2 }); + }); + + it("nennt im Protokoll die wirkliche Zahl der Treffer, nicht zwei", async () => { + /* + * **Die Zahl war geraten.** `personZuUpn` hatte `take: 2` — fuer die + * Entscheidung „mehrdeutig“ genuegt das, aber dieselbe Zahl ging in den + * Protokolleintrag und sagte dort immer „2“, auch bei vier Treffern + * (Reviewbefund zu #029). + * + * Wer das Protokoll liest, liest es, **um den Fall aufzuloesen** — er muss + * wissen, wie viele Zeilen er zu bereinigen hat. Eine Zahl, die nicht die + * Zahl ist, schickt ihn nach der zweiten Korrektur zurueck. + */ + await aufbau(); + for (const kuerzel of ["ZWEI", "DREI", "VIER"]) { + const weiterer = await testPrisma.kunde.create({ + data: { name: `${kuerzel} GmbH`, kuerzel }, + }); + await testPrisma.mitarbeiter.create({ + data: { + kundeId: weiterer.id, + vorname: "Maria", + nachname: "Huber", + anzeigename: `Maria Huber (${kuerzel})`, + upn: "mhuber@firma.local", + }, + }); + } + + await expect(benutzerAusOidcUebernehmen(anspruecke())).rejects.toThrow(); + + const eintrag = await testPrisma.protokoll.findFirstOrThrow({ + where: { aktion: { contains: "mehrdeutig" } }, + }); + expect(eintrag.nachher).toMatchObject({ treffer: 4 }); }); it("ordnet keine deaktivierte Person zu", async () => { diff --git a/tests/verzeichnis.test.ts b/tests/verzeichnis.test.ts index f3b634d..978d771 100644 --- a/tests/verzeichnis.test.ts +++ b/tests/verzeichnis.test.ts @@ -436,6 +436,57 @@ describe.runIf(datenbankVerfuegbar)("Verzeichnisquellen", () => { ); }); + it("weist einen Attributnamen ab, der keiner ist – auch am Formular vorbei", async () => { + /* + * **Die Regel sass eine Schicht ueber dem Code, der sich auf sie berief** + * (Reviewbefund zu #029). `src/server/graph/leser.ts` schrieb im Kommentar + * „die Werte selbst sind feste Feldnamen aus ATTRIBUT_MUSTER, also ist hier + * nichts zu maskieren“ — gepruefet wurde das aber nur in + * `zuordnungAusFormular`, also in der Server Action. + * + * Nachgewiesen wurde, dass genau dieser Name durch den Dienst kam und einen + * **zweiten `$filter`** in die Graph-Adresse schrieb. Hereinkommen konnte er + * nur mit `verwaltung` oder per Skript, Seed und `psql` — die + * Sicherheitswirkung ist also klein. Der Befund ist der Ort der Regel: + * CLAUDE.md verlangt sie in der Datenbank, und sie stand nicht einmal im + * Dienst. + */ + const { admin } = await grundbestand(); + const boese = { abteilung: "department,id&$filter=boese eq 1" }; + + await expect(quelleAnlegen(quellenEingabe({ attributZuordnung: boese }), admin)).rejects.toThrow( + /kein Attributname/u, + ); + + // Und beim Bearbeiten genauso. + const quelle = await quelleAnlegen(quellenEingabe(), admin); + await expect(quelleBearbeiten(quelle.id, { attributZuordnung: boese }, admin)).rejects.toThrow( + /kein Attributname/u, + ); + + // Die gespeicherte Zeile ist unberührt. + expect((await quelleHolen(quelle.id, admin))?.attributZuordnung.abteilung).toBe(AD_ZUORDNUNG.abteilung); + }); + + it("verwirft beim Lesen einen unbrauchbaren Namen, statt die Liste zu sprengen", async () => { + /* + * Die Gegenrichtung: eine Zeile, die vor dieser Prüfung angelegt wurde oder + * von Hand in die Datenbank kam. Hier zu werfen wäre falsch — eine einzige + * kaputte Zeile machte die ganze Quellenliste unbenutzbar. Sie bekommt + * deshalb die Vorgabe. + */ + const { admin } = await grundbestand(); + const quelle = await quelleAnlegen(quellenEingabe(), admin); + await testPrisma.verzeichnisquelle.update({ + where: { id: quelle.id }, + data: { attributZuordnung: { abteilung: "department,id&$filter=boese eq 1" } }, + }); + + const gelesen = await quelleHolen(quelle.id, admin); + expect(gelesen?.attributZuordnung.abteilung).toBe(AD_ZUORDNUNG.abteilung); + expect(JSON.stringify(gelesen)).not.toContain("$filter"); + }); + it("weist ein Intervall ausserhalb der Grenzen ab", async () => { const { admin } = await grundbestand(); await expect(quelleAnlegen(quellenEingabe({ abgleichIntervallMinuten: 1 }), admin)).rejects.toThrow( diff --git a/tests/zugang.test.ts b/tests/zugang.test.ts index 1590c6d..25d6ad7 100644 --- a/tests/zugang.test.ts +++ b/tests/zugang.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { SignJWT } from "jose"; import { sicheresZiel } from "@/server/auth/anmeldung"; -import { istOeffentlich } from "@/proxy"; +import { OEFFENTLICH, istOeffentlich } from "@/proxy"; import { kurzzeitTokenErzeugen, kurzzeitTokenLesen } from "@/lib/kurzzeit-token"; import { sitzungErzeugen, sitzungLesen } from "@/lib/sitzung"; @@ -67,20 +67,55 @@ describe("Ruecksprungziel gegen Steuerzeichen", () => { }); describe("Allowlist des Proxys", () => { + /** + * Jeder oeffentliche Pfad, einzeln aufgezaehlt — **und zwar absichtlich von + * Hand.** Diese Liste ist die Behauptung „genau das soll ohne Anmeldung + * gehen“; aus `OEFFENTLICH` abgeleitet wuerde sie nichts mehr behaupten. + * Dass sie vollstaendig ist, prueft der Test darunter. + */ + const ERWARTET_OEFFENTLICH = [ + "/anmelden", + "/api/auth", + "/api/gesundheit", + // Der SCIM-Endpunkt (#029): ein Identitaetsanbieter hat kein Cookie, er + // weist sich mit einem Bearer-Token aus, und jeder Handler darunter prueft + // es selbst. Ohne diesen Eintrag leitet der Proxy jede Zustellung auf + // /anmelden um, und das Provisioning hoert still auf zu arbeiten. + "/api/scim", + "/manifest.webmanifest", + "/icon.svg", + ] as const; + it("laesst genau die Pfade durch, die ohne Anmeldung gehen muessen", () => { for (const pfad of [ - "/anmelden", + ...ERWARTET_OEFFENTLICH, + // Und die Unterpfade, auf die es im Betrieb ankommt. "/api/auth/oidc/start", "/api/auth/oidc/rueckruf", "/api/auth/abmelden", - "/api/gesundheit", - "/manifest.webmanifest", - "/icon.svg", + "/api/scim/v2/Users", + "/api/scim/v2/ServiceProviderConfig", ]) { expect(istOeffentlich(pfad), pfad).toBe(true); } }); + it("hat keinen oeffentlichen Pfad, den dieser Test nicht kennt", () => { + /* + * **Der Test, der gefehlt hat.** `/api/scim` kam mit #029 in die Allowlist + * und stand in keiner Pruefung — die Zeile liess sich entfernen, ohne dass + * ein einziger Test rot wurde (Reviewbefund). Dabei ist der Eintrag beides: + * noetig, damit das Provisioning ueberhaupt ankommt, und eine + * Sicherheitsentscheidung, weil darunter jeder Handler sein Token selbst + * pruefen **muss**. + * + * Deshalb hier der Vergleich in die andere Richtung: wer `OEFFENTLICH` + * erweitert, muss es oben aufschreiben. Eine Liste, die nur in einer + * Richtung geprueft wird, laeuft dem Code hinterher. + */ + expect([...OEFFENTLICH].sort()).toEqual([...ERWARTET_OEFFENTLICH].sort()); + }); + it("laesst sich nicht durch einen angehaengten Namen austricksen", () => { // Ein reiner startsWith-Vergleich haette /anmeldenX durchgelassen. for (const pfad of [