From fb56fd36c5cc7e7f62a84f162ffa2f71c6b7a1b1 Mon Sep 17 00:00:00 2001 From: Florian Wilde Date: Mon, 12 Oct 2026 17:44:46 +0200 Subject: [PATCH] fix(widerruf): Massenwiderruf nimmt erzeugte Schluessel an (#032) Der neue E2E-Test (zwei im Browser erzeugte Schluessel mit Public Key auf Ziel B, Auswahl in der Liste, frische Bestaetigung ueber den Mock-OIDC mit auth_time, gemeinsamer Dry-Run, Anwenden) fand zwei Fehler im Produkt: - Die Massenwiderruf-Seite filterte die IDs aus ?id= mit /^[a-z0-9]{1,64}$/. Vom Worker erzeugte Schluessel tragen aber eine UUID mit Bindestrichen, nur Prisma-Defaults sind cuids. Damit fiel jeder erzeugte Schluessel still aus der Auswahl, die Seite meldete "Keine Schluessel gewaehlt". Der Filter steckt jetzt als idsAusSuchparameter in src/lib/schluessel.ts, erlaubt Bindestriche und hat einen eigenen Test. - Der Hinweis nach verweigerter Bestaetigung sagte "erneut Widerruf vorbereiten waehlen", der Knopf heisst beim Massenwiderruf aber "2 Schluessel widerrufen". Er nennt jetzt den Knopf, den es gibt. Der Test prueft danach, dass beide Keys aus authorized_keys auf B verschwunden und WIDERRUFEN sind. datenbank() und oidcBestaetigen() wandern dafuer aus schluessel.spec.ts nach tests/e2e/hilfen.ts. Co-Authored-By: Claude Opus 5.5 --- src/app/(app)/schluessel/widerruf/page.tsx | 13 +-- src/components/widerruf/WiderrufFormular.tsx | 10 +-- src/lib/schluessel.ts | 11 +++ tests/e2e/hilfen.ts | 25 ++++++ tests/e2e/schluessel.spec.ts | 25 +----- tests/e2e/widerruf.spec.ts | 91 +++++++++++++++++++- tests/schluessel.test.ts | 10 +++ 7 files changed, 144 insertions(+), 41 deletions(-) diff --git a/src/app/(app)/schluessel/widerruf/page.tsx b/src/app/(app)/schluessel/widerruf/page.tsx index f034ab5..28565ac 100644 --- a/src/app/(app)/schluessel/widerruf/page.tsx +++ b/src/app/(app)/schluessel/widerruf/page.tsx @@ -9,6 +9,7 @@ import { Leerzustand } from "@/components/ui/Leerzustand"; import { Seitenkopf } from "@/components/ui/Seitenkopf"; import { WiderrufAblauf } from "@/components/widerruf/WiderrufAblauf"; import { WiderrufFormular } from "@/components/widerruf/WiderrufFormular"; +import { idsAusSuchparameter } from "@/lib/schluessel"; import { darf } from "@/server/auth/rechte"; import { bestaetigungFrisch, sitzungErforderlich } from "@/server/auth/sitzungspflicht"; import { widerrufUebersicht } from "@/server/queries/widerruf"; @@ -17,14 +18,6 @@ export const metadata: Metadata = { title: "Massenwiderruf" }; type Suchparameter = Promise<{ id?: string | string[]; zielkonten?: string | string[] }>; -function liste(wert: string | string[] | undefined): string[] { - const roh = Array.isArray(wert) ? wert : wert ? [wert] : []; - return [...new Set(roh.flatMap((w) => w.split(",")).filter((w) => /^[a-z0-9]{1,64}$/i.test(w)))].slice( - 0, - 200, - ); -} - /** * Massenwiderruf (#024): mehrere Schluessel – etwa alle einer Person nach Projektende – in einem * Durchgang mit gemeinsamem Dry-Run. Nur Admins; das Vormerken verlangt eine frische @@ -32,7 +25,7 @@ function liste(wert: string | string[] | undefined): string[] { */ export default async function MassenwiderrufSeite({ searchParams }: { searchParams: Suchparameter }) { const parameter = await searchParams; - const ids = liste(parameter.id); + const ids = idsAusSuchparameter(parameter.id); const selbst = `/schluessel/widerruf?${ids.map((id) => `id=${id}`).join("&")}`; const { benutzer } = await sitzungErforderlich("massenwiderruf", selbst); @@ -61,7 +54,7 @@ export default async function MassenwiderrufSeite({ searchParams }: { searchPara const offeneStati = (s: { status: string }) => s.status === "AKTIV" || s.status === "ABGELAUFEN"; const nochNichtEingeleitet = uebersicht.schluessel.filter((s) => offeneStati(s) && !s.eingeleitet); const laufend = uebersicht.schluessel.filter((s) => offeneStati(s) && s.eingeleitet); - const vorauswahl = liste(parameter.zielkonten); + const vorauswahl = idsAusSuchparameter(parameter.zielkonten); return (
diff --git a/src/components/widerruf/WiderrufFormular.tsx b/src/components/widerruf/WiderrufFormular.tsx index 96254db..83b032e 100644 --- a/src/components/widerruf/WiderrufFormular.tsx +++ b/src/components/widerruf/WiderrufFormular.tsx @@ -32,6 +32,7 @@ export function WiderrufFormular({ const [laeuft, starten] = useTransition(); const gesperrt = uebersicht.schluessel.filter((s) => s.gesperrt); const mehrere = uebersicht.schluessel.length > 1; + const knopfText = mehrere ? `${uebersicht.schluessel.length} Schlüssel widerrufen` : "Widerruf vorbereiten"; const ziel = (() => { const url = new URL(ruecksprung, "http://x"); @@ -100,7 +101,8 @@ export function WiderrufFormular({ href={`/bestaetigen?ziel=${encodeURIComponent(ruecksprung)}`} className="text-accent inline-flex min-h-11 items-center text-[14px] font-medium" > - Anmeldung bestätigen und danach erneut „Widerruf vorbereiten“ wählen + {/* Nennt den Knopf, den es hier wirklich gibt – beim Massenwiderruf heisst er anders (#032). */} + Anmeldung bestätigen und danach erneut „{knopfText}“ wählen )}
@@ -124,11 +126,7 @@ export function WiderrufFormular({ }) } > - {laeuft - ? "Wird vorgemerkt …" - : mehrere - ? `${uebersicht.schluessel.length} Schlüssel widerrufen` - : "Widerruf vorbereiten"} + {laeuft ? "Wird vorgemerkt …" : knopfText}
diff --git a/src/lib/schluessel.ts b/src/lib/schluessel.ts index dfcb0ae..2c6197d 100644 --- a/src/lib/schluessel.ts +++ b/src/lib/schluessel.ts @@ -293,3 +293,14 @@ export function publicKeyZeileMitKommentar(zeile: string, kommentar: string): st const [typ, base64] = zeile.trim().split(/\s+/); return kommentar ? `${typ} ${base64} ${kommentar}` : `${typ} ${base64}`; } + +/** + * IDs aus einem Suchparameter (`?id=a&id=b` oder `?id=a,b`), etwa fuer den Massenwiderruf (#024). + * Erlaubt Bindestriche: Vom Worker erzeugte Schluessel tragen eine UUID, nur Prisma-Defaults sind + * cuids – ohne Bindestrich fielen alle erzeugten Schluessel still aus der Auswahl (#032). + */ +export function idsAusSuchparameter(wert: string | string[] | undefined, hoechstens = 200): string[] { + const roh = Array.isArray(wert) ? wert : wert ? [wert] : []; + const gueltig = roh.flatMap((w) => w.split(",")).filter((w) => /^[a-z0-9-]{1,64}$/i.test(w)); + return [...new Set(gueltig)].slice(0, hoechstens); +} diff --git a/tests/e2e/hilfen.ts b/tests/e2e/hilfen.ts index edcb594..5ff39e6 100644 --- a/tests/e2e/hilfen.ts +++ b/tests/e2e/hilfen.ts @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; import { expect, type Page } from "@playwright/test"; +import pg from "pg"; import "dotenv/config"; /** @@ -35,6 +36,30 @@ export async function anmelden(page: Page): Promise { await expect(page.getByRole("heading", { name: "Übersicht" })).toBeVisible(); } +/** Direkter Zugriff auf die Datenbank des gebauten Stands – fuer Vorbedingungen und Pruefungen. */ +export async function datenbank(arbeit: (client: pg.Client) => Promise): Promise { + const client = new pg.Client({ connectionString: process.env["DATABASE_URL"] }); + await client.connect(); + try { + return await arbeit(client); + } finally { + await client.end(); + } +} + +/** + * Erneute Anmeldung beim Mock-OIDC (#014). Der Mock uebernimmt nur die Claims aus dem + * Formular; `auth_time` muss deshalb mitgeschickt werden, sonst lehnt sshkv die Bestaetigung ab. + */ +export async function oidcBestaetigen(page: Page): Promise { + await expect(page).toHaveURL(/\/bestaetigen\?ziel=/); + await page.getByRole("link", { name: /erneut anmelden/ }).click(); + await page.locator('button[data-rolle="admin"]').click(); + const claims = JSON.parse(await page.locator("#claims").inputValue()) as Record; + await page.locator("#claims").fill(JSON.stringify({ ...claims, auth_time: Math.floor(Date.now() / 1000) })); + await page.getByRole("button", { name: "Anmelden", exact: true }).click(); +} + export async function hostEinrichten(page: Page, name: string, port: string): Promise { await page.goto("/hosts/neu"); await page.getByLabel("Name").fill(name); diff --git a/tests/e2e/schluessel.spec.ts b/tests/e2e/schluessel.spec.ts index 330747d..cb00e85 100644 --- a/tests/e2e/schluessel.spec.ts +++ b/tests/e2e/schluessel.spec.ts @@ -1,7 +1,7 @@ import { expect, test, type Page } from "@playwright/test"; -import pg from "pg"; import "dotenv/config"; import { schluesselpaarErzeugen } from "../../src/lib/ssh/openssh"; +import { datenbank, oidcBestaetigen } from "./hilfen"; /** * Schluessel im gebauten Stand mit laufendem Worker (#012–#014). Testdaten heissen zz-e2e-… @@ -17,16 +17,6 @@ async function oidcAnmelden(page: Page, rolle: "admin" | "operator" | "reader") await expect(page.getByRole("heading", { name: "Übersicht" })).toBeVisible(); } -async function datenbank(arbeit: (client: pg.Client) => Promise): Promise { - const client = new pg.Client({ connectionString: process.env["DATABASE_URL"] }); - await client.connect(); - try { - return await arbeit(client); - } finally { - await client.end(); - } -} - function eindeutig(): string { return `zz-e2e-${Date.now().toString(36)}`; } @@ -104,19 +94,6 @@ test("Leser hinterlegt einen eigenen Public Key und entfernt ihn wieder", async await expect(page.locator("li", { hasText: name })).toHaveCount(0); }); -/** - * Erneute Anmeldung beim Mock-OIDC (#014). Der Mock uebernimmt nur die Claims aus dem - * Formular; `auth_time` muss deshalb mitgeschickt werden, sonst lehnt sshkv die Bestaetigung ab. - */ -async function oidcBestaetigen(page: Page) { - await expect(page).toHaveURL(/\/bestaetigen\?ziel=/); - await page.getByRole("link", { name: /erneut anmelden/ }).click(); - await page.locator('button[data-rolle="admin"]').click(); - const claims = JSON.parse(await page.locator("#claims").inputValue()) as Record; - await page.locator("#claims").fill(JSON.stringify({ ...claims, auth_time: Math.floor(Date.now() / 1000) })); - await page.getByRole("button", { name: "Anmelden", exact: true }).click(); -} - test("Admin zeigt den Private Key nach Bestätigung an, lädt ihn herunter und löscht ihn", async ({ page, }) => { diff --git a/tests/e2e/widerruf.spec.ts b/tests/e2e/widerruf.spec.ts index dda76c4..b402e70 100644 --- a/tests/e2e/widerruf.spec.ts +++ b/tests/e2e/widerruf.spec.ts @@ -1,10 +1,12 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type Page } from "@playwright/test"; import { anmelden, archivieren, aufZiel, + datenbank, hostEinrichten, kernAufbauen, + oidcBestaetigen, PORT, zieleZuruecksetzen, } from "./hilfen"; @@ -59,3 +61,90 @@ test("Widerruf mit einem Klick", async ({ page }) => { await archivieren(page, hostA); await archivieren(page, hostB); }); + +/** Schluessel erzeugen, Public Key auf `host` zuweisen und ausrollen. Liefert den Key-Teil. */ +async function publicKeyAusrollen(page: Page, zweck: string, host: string): Promise { + await page.goto("/schluessel/neu"); + await page.getByLabel("Zweck").fill(zweck); + await page.getByRole("button", { name: "Schlüssel erzeugen" }).click(); + await expect(page.getByRole("heading", { name: zweck })).toBeVisible({ timeout: 30_000 }); + const zeile = (await page.getByLabel("Public Key", { exact: true }).textContent()) ?? ""; + const keyTeil = zeile.trim().split(/\s+/)[1] ?? ""; + expect(keyTeil).toMatch(/^AAAA/); + + await page.getByRole("button", { name: "Ausrollen" }).nth(1).click(); + const oeffentlich = page.getByRole("dialog", { name: "Public Key zuweisen" }); + await oeffentlich.getByLabel("Ziel", { exact: true }).selectOption({ label: host }); + await oeffentlich.getByRole("button", { name: "Zuweisen" }).click(); + await expect(oeffentlich).toBeHidden(); + await page.getByRole("button", { name: "Dry-Run starten" }).click(); + await page.getByRole("button", { name: /Änderung anwenden/ }).click({ timeout: 30_000 }); + await expect(page.getByText("Ausgerollt.")).toBeVisible({ timeout: 60_000 }); + return keyTeil; +} + +test("Massenwiderruf mit frischer Bestätigung (#032)", async ({ page }) => { + const kennung = Date.now().toString(36); + const hostB = `zz-e2e-mb-${kennung}`; + const praefix = `zz-e2e Massen ${kennung}`; + const zwecke = [`${praefix} eins`, `${praefix} zwei`]; + + await anmelden(page); + // Eine Bestaetigung aus der Anmeldung eben oder einem frueheren Lauf soll nicht mitzaehlen. + await datenbank((client) => + client.query(`UPDATE "Benutzer" SET "bestaetigtAm" = NULL WHERE "email" = 'admin@sshkv.test'`), + ); + await hostEinrichten(page, hostB, PORT.b); + const keys = []; + for (const zweck of zwecke) keys.push(await publicKeyAusrollen(page, zweck, hostB)); + const vorher = aufZiel("ziel-b", "cat /root/.ssh/authorized_keys"); + for (const key of keys) expect(vorher).toContain(key); + + // Auswahl in der Liste – ein GET-Formular auf die Massenwiderruf-Seite. + await page.goto(`/schluessel?q=${encodeURIComponent(praefix)}`); + for (const zweck of zwecke) + await page.getByRole("checkbox", { name: `${zweck} für den Massenwiderruf auswählen` }).check(); + await page.getByRole("button", { name: "Ausgewählte widerrufen" }).click(); + await expect(page).toHaveURL(/\/schluessel\/widerruf\?id=/); + await expect(page.getByRole("heading", { name: "2 Schlüssel widerrufen?" })).toBeVisible(); + for (const zweck of zwecke) + await expect( + page.getByRole("region", { name: `Widerruf von ${zweck}` }).getByText(`root@${hostB}`), + ).toBeVisible(); + + // Ohne frische Bestaetigung merkt sshkv nichts vor und schickt zur erneuten Anmeldung. + await page.getByRole("button", { name: "2 Schlüssel widerrufen" }).click(); + await page.getByRole("link", { name: /Anmeldung bestätigen/ }).click(); + await oidcBestaetigen(page); + await expect(page).toHaveURL(/\/schluessel\/widerruf\?id=/); + const eingeleitet = await datenbank((client) => + client.query( + `SELECT count(*)::int AS n FROM "Schluessel" WHERE "name" = ANY($1) AND "widerrufEingeleitetAm" IS NOT NULL`, + [zwecke], + ), + ); + expect(eingeleitet.rows[0]).toEqual({ n: 0 }); + + await page.getByRole("button", { name: "2 Schlüssel widerrufen" }).click(); + await expect(page.getByRole("heading", { name: "Widerruf läuft" })).toBeVisible(); + + // Ein gemeinsamer Dry-Run fuer beide Schluessel, dann anwenden; der Abschluss laeuft danach. + await page.getByRole("button", { name: "Dry-Run starten" }).click(); + await expect(page.getByText(`root@${hostB}:/root/.ssh/authorized_keys`)).toBeVisible({ timeout: 30_000 }); + await page.getByRole("button", { name: /Änderung anwenden/ }).click(); + await expect(page.getByRole("heading", { name: "Widerruf läuft" })).toBeHidden({ timeout: 60_000 }); + const gewaehlt = page.locator("section", { + has: page.getByRole("heading", { name: "Gewählte Schlüssel" }), + }); + await expect(gewaehlt.getByText("widerrufen", { exact: true })).toHaveCount(2); + await expect(gewaehlt.getByText("Widerruf unvollständig")).toHaveCount(0); + + const nachher = aufZiel("ziel-b", "cat /root/.ssh/authorized_keys"); + for (const key of keys) expect(nachher).not.toContain(key); + const status = await datenbank((client) => + client.query(`SELECT "status" FROM "Schluessel" WHERE "name" = ANY($1)`, [zwecke]), + ); + expect(status.rows).toEqual([{ status: "WIDERRUFEN" }, { status: "WIDERRUFEN" }]); + + await archivieren(page, hostB); +}); diff --git a/tests/schluessel.test.ts b/tests/schluessel.test.ts index 364b750..994d118 100644 --- a/tests/schluessel.test.ts +++ b/tests/schluessel.test.ts @@ -9,6 +9,7 @@ import { ablaufZustand, fingerprintKurz, freierSlug, + idsAusSuchparameter, slugAus, typAnzeige, erzeugenSchema, @@ -66,6 +67,15 @@ function kern(privateKey: string): string { } describe("Regeln (rein)", () => { + it("liest IDs aus dem Suchparameter, auch UUIDs vom Worker (#032)", () => { + const uuid = "5f703ff1-3877-4af4-af08-84338237fcd8"; + expect(idsAusSuchparameter([uuid, "cm1abc"])).toEqual([uuid, "cm1abc"]); + expect(idsAusSuchparameter(`a,b,a`)).toEqual(["a", "b"]); + expect(idsAusSuchparameter(["x y", "../etc", ""])).toEqual([]); + expect(idsAusSuchparameter(undefined)).toEqual([]); + expect(idsAusSuchparameter(["a", "b", "c"], 2)).toEqual(["a", "b"]); + }); + it.each([ ["Backup srv-backup01 → srv-pve01", "backup-srv-backup01-srv-pve01"], ["Größter Schlüssel für Ärzte", "groesster-schluessel-fuer-aerzte"],