fix(260923-ku6): Verbindungstest prueft Formularwerte statt gespeicherten Stand

Nachbesserung aus dem Browser-Rundgang zu 260923-dhh (Befund 1): "Verbindung testen" pruefte bislang immer den in der Datenbank gespeicherten Server, nicht das ungespeicherte Formular. Eine im Formular abgeschaltete Zertifikatspruefung oder ein neu eingetipptes Token-Geheimnis wurden dadurch beim Test ignoriert und erst nach "Speichern" wirksam — eine Falle fuer genau den Ablauf, den Nutzer instinktiv waehlen (eintippen, testen, dann erst speichern). Neues `TestProxmoxServerDto` plus Merge-Baustein `resolveEffectiveTestServer` in `ProxmoxService`: normale Felder folgen dem Formular (auch wenn absichtlich geleert), Geheimnisfelder folgen der bestehenden "leer -> gespeicherten Wert behalten"-Regel, weil `ServerForm` sie beim Laden nie aus der Datenbank vorbefuellt. Neue Route `POST servers/test` (ohne `:id`) deckt die Neuanlage ab, wo es noch keinen gespeicherten Server gibt. Der Testen-Knopf steht jetzt immer zur Verfuegung, nicht mehr nur nach dem ersten Speichern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-23 15:11:56 +02:00
parent 3091b04673
commit 710034c80a
7 changed files with 372 additions and 32 deletions
@@ -147,3 +147,17 @@ export class UpdateProxmoxServerDto {
  @IsOptional()
  isActive?: boolean;
 }
+
+/**
+ * DTO fuer den Verbindungstest (Nachbesserung Befund 1, Rundgang zu Aufgabe 4):
+ * derselbe Feldsatz wie `UpdateProxmoxServerDto` — der Test soll auf JEDEM
+ * dieser Felder den ungespeicherten Formularwert pruefen koennen, nicht den
+ * gespeicherten Stand. Ein NICHT gesendetes oder leeres Geheimnisfeld heisst
+ * "gespeicherten Wert weiterverwenden" (Merge-Logik in
+ * `ProxmoxService.resolveEffectiveTestServer`), genau wie beim Bearbeiten.
+ * Fuer die Neuanlage (noch kein gespeicherter Server) bleiben alle Felder
+ * optional, weil es dort keinen gespeicherten Fallback gibt — ein fehlendes
+ * Pflichtfeld fuehrt dort einfach zum selben Fehlerschluessel wie ein leer
+ * gelassenes Feld beim Anlegen selbst (z. B. `zugang` ohne Geheimnis).
+ */
+export class TestProxmoxServerDto extends UpdateProxmoxServerDto {}
@@ -13,7 +13,11 @@ import { Role } from '@prisma/client';
 import { Roles } from '../auth/decorators/roles.decorator';
 import type { AuthenticatedRequest } from '../auth/types/auth-user';
 import { UseModule } from '../module-registry/module.guard';
-import { CreateProxmoxServerDto, UpdateProxmoxServerDto } from './dto/proxmox-server.dto';
+import {
+  CreateProxmoxServerDto,
+  TestProxmoxServerDto,
+  UpdateProxmoxServerDto,
+} from './dto/proxmox-server.dto';
 import { ProxmoxSchedulerService } from './proxmox-scheduler.service';
 import { ProxmoxService } from './proxmox.service';

@@ -22,8 +26,12 @@ import { ProxmoxService } from './proxmox.service';
 * `domaincheck.controller.ts`) — Aktivierung UND Freigabe. `tenantId` kommt
 * ausschliesslich aus `req.tenantId` (gesetzt vom `TenantGuard`), nie aus
 * Body oder Query. Lesen (`GET servers`) steht jedem Benutzer mit
- * Modulzugriff offen; Schreiben (`POST servers`, `POST servers/:id/poll`,
- * `POST servers/:id/test`) zusaetzlich `@Roles(ADMIN, SUPER_ADMIN)` (T-DHH-05).
+ * Modulzugriff offen; Schreiben (`POST servers`, `POST servers/test`,
+ * `POST servers/:id/poll`, `POST servers/:id/test`) zusaetzlich
+ * `@Roles(ADMIN, SUPER_ADMIN)` (T-DHH-05). `servers/test` (statisch, zwei
+ * Segmente) und `servers/:id/test` (drei Segmente) ueberschneiden sich
+ * nicht — beide POST, aber unterschiedliche Segmentzahl, deshalb keine
+ * Reihenfolge-Abhaengigkeit (anders als `GET :id` vs. statische Routen).
 */
@Controller('modules/proxmox')
@UseModule('proxmox')
@@ -85,13 +93,31 @@ export class ProxmoxController {
  }

  /**
-   * Verbindungstest (Aufgabe 4, `<behavior>`): liefert bei Erfolg eine
-   * Erfolgsmeldung und bei Misserfolg einen der sieben Fehlerschluessel
-   * samt kurzer Ergaenzung, OHNE den Zwischenlagerstand zu ueberschreiben.
+   * Verbindungstest fuer einen gespeicherten Server (Aufgabe 4, `<behavior>`;
+   * Nachbesserung Befund 1: `dto` traegt den aktuellen Formularstand,
+   * `ProxmoxService.testConnection` prueft diesen statt blind des
+   * gespeicherten Stands). Liefert bei Erfolg eine Erfolgsmeldung und bei
+   * Misserfolg einen der sieben Fehlerschluessel samt kurzer Ergaenzung,
+   * OHNE den Zwischenlagerstand zu ueberschreiben.
   */
  @Post('servers/:id/test')
  @Roles(Role.ADMIN, Role.SUPER_ADMIN)
-  async test(@Req() req: AuthenticatedRequest, @Param('id') id: string) {
-    return this.proxmoxService.testConnection(this.requireTenantId(req), id);
+  async test(
+    @Req() req: AuthenticatedRequest,
+    @Param('id') id: string,
+    @Body() dto: TestProxmoxServerDto,
+  ) {
+    return this.proxmoxService.testConnection(this.requireTenantId(req), id, dto);
+  }
+
+  /**
+   * Verbindungstest waehrend der Neuanlage (Nachbesserung Befund 1): es gibt
+   * noch keinen gespeicherten Server, `dto` ist deshalb die einzige Quelle.
+   */
+  @Post('servers/test')
+  @Roles(Role.ADMIN, Role.SUPER_ADMIN)
+  async testDraft(@Req() req: AuthenticatedRequest, @Body() dto: TestProxmoxServerDto) {
+    this.requireTenantId(req);
+    return this.proxmoxService.testDraftConnection(dto);
  }
 }
@@ -402,6 +402,115 @@ describe('ProxmoxService — Aufgabe 4 (Verbindungstest, Zehn-Sekunden-Sperre)',
    expect(status.reachable).toBe(false);
  });

+  it('Nachbesserung Befund 1: testConnection prueft die im Formular abgeschaltete Zertifikatspruefung, nicht den gespeicherten Stand (Server wurde MIT tlsRejectUnauthorized:true angelegt)', async () => {
+    const prisma = makeFakePrisma();
+    const service = new ProxmoxService(prisma as any, crypto as any);
+    // Gespeichert: Zertifikatspruefung AN (Vorgabe).
+    const created = await service.createServer('tenant-a', TOKEN_DTO);
+
+    const dispatchers: unknown[] = [];
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(async (_url: string, options: RequestInit & { dispatcher?: unknown }) => {
+        dispatchers.push(options.dispatcher);
+        return new Response(JSON.stringify(pveResourcesBody()), { status: 200 });
+      }),
+    );
+
+    // Formular: Zertifikatspruefung wurde vom Nutzer AUSGESCHALTET, aber noch nicht gespeichert.
+    await service.testConnection('tenant-a', (created as any).id, {
+      tlsRejectUnauthorized: false,
+    });
+
+    // Vor der Korrektur wurde ausschliesslich der gespeicherte Server (Zertifikatspruefung AN)
+    // getestet — dieser Test waere ohne die Korrektur rot, weil dispatchers[0] dann `undefined` waere.
+    expect(dispatchers[0]).toBeInstanceOf(Agent);
+    // biome-ignore lint/suspicious/noExplicitAny: Test-Attrappe traegt `options` nicht im echten undici-Typ
+    expect((dispatchers[0] as any).options).toEqual({ connect: { rejectUnauthorized: false } });
+  });
+
+  it('Nachbesserung Befund 1: ein im Formular NEU eingetipptes Token-Geheimnis wird getestet, nicht das gespeicherte', async () => {
+    const prisma = makeFakePrisma();
+    const service = new ProxmoxService(prisma as any, crypto as any);
+    const created = await service.createServer('tenant-a', TOKEN_DTO);
+
+    let capturedAuth: string | null = null;
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(async (_url: string, options: RequestInit) => {
+        capturedAuth = (options.headers as Record<string, string>).Authorization;
+        return new Response(JSON.stringify(pveResourcesBody()), { status: 200 });
+      }),
+    );
+
+    await service.testConnection('tenant-a', (created as any).id, {
+      tokenSecret: 'ein-anderes-geheimnis',
+    });
+
+    // Ohne die Korrektur wuerde hier weiterhin TOKEN_DTO.tokenSecret gesendet — roter Test.
+    expect(capturedAuth).toBe(`PVEAPIToken=${TOKEN_DTO.tokenId}=ein-anderes-geheimnis`);
+  });
+
+  it('Nachbesserung Befund 1: leer gelassenes Geheimnisfeld im Formular nutzt weiterhin das gespeicherte Token-Geheimnis', async () => {
+    const prisma = makeFakePrisma();
+    const service = new ProxmoxService(prisma as any, crypto as any);
+    const created = await service.createServer('tenant-a', TOKEN_DTO);
+
+    let capturedAuth: string | null = null;
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(async (_url: string, options: RequestInit) => {
+        capturedAuth = (options.headers as Record<string, string>).Authorization;
+        return new Response(JSON.stringify(pveResourcesBody()), { status: 200 });
+      }),
+    );
+
+    // Formular sendet kein tokenSecret (Feld leer gelassen) — wie `ServerForm.buildPayload()`.
+    await service.testConnection('tenant-a', (created as any).id, {});
+
+    expect(capturedAuth).toBe(`PVEAPIToken=${TOKEN_DTO.tokenId}=${TOKEN_DTO.tokenSecret}`);
+  });
+
+  it('Nachbesserung Befund 1: testDraftConnection testet einen noch nicht gespeicherten Server ausschliesslich mit den Formularwerten', async () => {
+    const prisma = makeFakePrisma();
+    const service = new ProxmoxService(prisma as any, crypto as any);
+
+    let capturedUrl: string | null = null;
+    vi.stubGlobal(
+      'fetch',
+      vi.fn(async (url: string) => {
+        capturedUrl = url;
+        return new Response(JSON.stringify(pveResourcesBody()), { status: 200 });
+      }),
+    );
+
+    const result = await service.testDraftConnection({
+      productType: 'pve',
+      baseUrl: 'https://neu.intern:8006',
+      authMethod: 'token',
+      tokenId: 'root@pam!neu',
+      tokenSecret: 'frisches-geheimnis',
+    });
+
+    expect(result.reachable).toBe(true);
+    expect(capturedUrl).toBe('https://neu.intern:8006/api2/json/cluster/resources');
+  });
+
+  it('Nachbesserung Befund 1: testDraftConnection ohne Geheimnis liefert den Fehlerschluessel "zugang", statt zu werfen', async () => {
+    const prisma = makeFakePrisma();
+    const service = new ProxmoxService(prisma as any, crypto as any);
+
+    const result = await service.testDraftConnection({
+      productType: 'pve',
+      baseUrl: 'https://neu.intern:8006',
+      authMethod: 'token',
+      tokenId: 'root@pam!neu',
+    });
+
+    expect(result.reachable).toBe(false);
+    expect(result.errorKind).toBe('zugang');
+  });
+
  it('POST servers/:id/poll verweigert einen zweiten Durchlauf innerhalb von zehn Sekunden und liefert den vorhandenen Stand', async () => {
    const prisma = makeFakePrisma();
    const service = new ProxmoxService(prisma as any, crypto as any);
@@ -6,9 +6,33 @@ import { forSystem, forTenant } from '../prisma/prisma-tenant.extension';
 import { buildTicketCookieHeader, buildTokenAuthHeader, loginTicket } from './proxmox-auth';
 import { proxmoxGet, type ProxmoxGetResult } from './proxmox-client.service';
 import { listPbsDatastoreNames, normalizePbs, normalizePmg, normalizePve } from './proxmox-normalize';
-import type { CreateProxmoxServerDto, UpdateProxmoxServerDto } from './dto/proxmox-server.dto';
+import type {
+  CreateProxmoxServerDto,
+  TestProxmoxServerDto,
+  UpdateProxmoxServerDto,
+} from './dto/proxmox-server.dto';
 import type { ProxmoxErrorKind, ProxmoxPollResult, ProxmoxProductType } from './proxmox.types';

+/**
+ * Nur die Felder, die eine Abfrage tatsaechlich braucht (Nachbesserung
+ * Befund 1) — `pollOne`/`buildAuthHeaders`/`getWithRetry` nehmen diesen
+ * schmalen Ausschnitt statt der vollen `ProxmoxServer`-Zeile entgegen, damit
+ * `resolveEffectiveTestServer` unten eine rein im Speicher gebaute Mischung
+ * aus Formular- und gespeicherten Werten uebergeben kann, ohne eine
+ * vollstaendige Datenbankzeile vorzutaeuschen.
+ */
+type ProxmoxCredentialSource = Pick<
+  ProxmoxServer,
+  | 'productType'
+  | 'baseUrl'
+  | 'authMethod'
+  | 'tokenId'
+  | 'encryptedTokenSecret'
+  | 'username'
+  | 'encryptedPassword'
+  | 'tlsRejectUnauthorized'
+>;
+
 /**
 * Erkennungsform fuer "schon verschluesselt" — woertlich aus
 * `ldap-config.service.ts:17` uebernommen (Format `iv:authTag:ciphertext`,
@@ -217,7 +241,7 @@ export class ProxmoxService {
   * Ticket-Anmeldung aus (die einzige nicht-lesende Anfrage des Moduls,
   * D-01) — deshalb `async`.
   */
-  private async buildAuthHeaders(server: ProxmoxServer): Promise<AuthHeaderResult> {
+  private async buildAuthHeaders(server: ProxmoxCredentialSource): Promise<AuthHeaderResult> {
    if (server.authMethod === 'token') {
      const tokenSecret = this.decryptSecret(server.encryptedTokenSecret);
      if (!server.tokenId || !tokenSecret) {
@@ -273,7 +297,7 @@ export class ProxmoxService {
   * zwei Stunden einen Fehlalarm. Ein zweites 401 bleibt `'zugang'`.
   */
  private async getWithRetry(
-    server: ProxmoxServer,
+    server: ProxmoxCredentialSource,
    session: { headers: Record<string, string>; retried: boolean },
    path: string,
  ): Promise<ProxmoxGetResult> {
@@ -306,7 +330,7 @@ export class ProxmoxService {
   * `pbs` und `pmg` (Aufgabe 3), jeweils mit Token ODER Benutzer/Passwort
   * (Aufgabe 2).
   */
-  private async pollOne(server: ProxmoxServer): Promise<ProxmoxPollResult> {
+  private async pollOne(server: ProxmoxCredentialSource): Promise<ProxmoxPollResult> {
    const authHeaders = await this.buildAuthHeaders(server);
    if (!authHeaders.ok) {
      return {
@@ -491,21 +515,79 @@ export class ProxmoxService {
  }

  /**
-   * Verbindungstest (Aufgabe 4, `<behavior>`, `POST servers/:id/test`):
-   * benutzt denselben Klienten und dieselbe Fehleruebersetzung wie der
+   * Mischt Formularwerte (`dto`, ungespeichert) mit dem gespeicherten Server
+   * (`existing`, `null` bei der Neuanlage) zu genau den Feldern, die eine
+   * Abfrage braucht (Nachbesserung Befund 1). Zwei Regeln, je nachdem, ob
+   * das Formular das Feld beim Laden vorbefuellt (Vorbild `serverToForm`):
+   *
+   * - Normale Felder (`productType`, `baseUrl`, `authMethod`, `tokenId`,
+   *   `username`, `tlsRejectUnauthorized`): das Formular zeigt immer den
+   *   zuletzt gespeicherten Wert an, bis der Nutzer ihn aendert — ein vom
+   *   Aufrufer GESENDETES Feld gilt also als Formularwert, auch wenn es
+   *   absichtlich geleert wurde (`tokenId: ''` -> `null`). Nur ein NICHT
+   *   gesendetes Feld (Aufrufer ohne diesen Schluessel im Body) faellt auf
+   *   den gespeicherten Wert zurueck.
+   * - Geheimnisfelder (`tokenSecret`/`password`): `ServerForm` befuellt
+   *   diese beim Laden bewusst NIE aus der Datenbank (Geheimnis nie im
+   *   Klartext anzeigen). Ein leeres Feld bedeutet hier deshalb NICHT
+   *   "Nutzer will loeschen", sondern "Nutzer hat nichts eingetippt" ->
+   *   gespeicherten (verschluesselten) Wert weiterverwenden. Ein gefuelltes
+   *   Feld ist der eingetippte Klartext und wird unveraendert durchgereicht;
+   *   `decryptSecret()` erkennt anhand der Form `iv:authTag:ciphertext`
+   *   automatisch, ob entschluesselt werden muss, und laesst Klartext sonst
+   *   unangetastet.
+   */
+  private resolveEffectiveTestServer(
+    existing: ProxmoxServer | null,
+    dto: TestProxmoxServerDto,
+  ): ProxmoxCredentialSource {
+    return {
+      productType: dto.productType ?? existing?.productType ?? 'pve',
+      baseUrl: dto.baseUrl ?? existing?.baseUrl ?? '',
+      authMethod: dto.authMethod ?? existing?.authMethod ?? 'token',
+      tokenId: dto.tokenId !== undefined ? dto.tokenId || null : (existing?.tokenId ?? null),
+      encryptedTokenSecret: dto.tokenSecret
+        ? dto.tokenSecret
+        : (existing?.encryptedTokenSecret ?? null),
+      username: dto.username !== undefined ? dto.username || null : (existing?.username ?? null),
+      encryptedPassword: dto.password ? dto.password : (existing?.encryptedPassword ?? null),
+      tlsRejectUnauthorized: dto.tlsRejectUnauthorized ?? existing?.tlsRejectUnauthorized ?? true,
+    };
+  }
+
+  /**
+   * Verbindungstest fuer einen GESPEICHERTEN Server (Aufgabe 4, `<behavior>`,
+   * `POST servers/:id/test`; Nachbesserung Befund 1: prueft jetzt die
+   * Formularwerte aus `dto`, nicht mehr blind den gespeicherten Stand).
+   * Benutzt denselben Klienten und dieselbe Fehleruebersetzung wie der
   * Planer, schreibt aber NICHT ins Zwischenlager — ein Testklick darf den
   * zuletzt gemessenen Stand nicht ueberschreiben (Vorbild
   * `TenderEmailConfigService.testConnection`/LDAP-Test). Keine
   * Zehn-Sekunden-Sperre: ein Test ist ein bewusster Einzelklick, kein
   * automatisierter Auffrischungsweg.
   */
-  async testConnection(tenantId: string, serverId: string): Promise<ProxmoxPollResult | null> {
+  async testConnection(
+    tenantId: string,
+    serverId: string,
+    dto: TestProxmoxServerDto = {},
+  ): Promise<ProxmoxPollResult | null> {
    const tenantPrisma = forTenant(this.prisma, tenantId);
    const server = await tenantPrisma.proxmoxServer.findUnique({ where: { id: serverId } });
    if (!server || server.tenantId !== tenantId) {
      return null;
    }
-    return this.pollOne(server);
+    return this.pollOne(this.resolveEffectiveTestServer(server, dto));
+  }
+
+  /**
+   * Verbindungstest waehrend der Neuanlage (Nachbesserung Befund 1,
+   * `POST servers/test`, ohne `:id`) — es gibt noch keinen gespeicherten
+   * Server, also ausschliesslich die Formularwerte aus `dto`. Fehlende
+   * Pflichtangaben (z. B. kein Geheimnis) fuehren zum selben Fehlerschluessel
+   * wie beim Abfragen eines gespeicherten Servers ohne Zugang (`zugang`).
+   */
+  async testDraftConnection(dto: TestProxmoxServerDto): Promise<ProxmoxPollResult> {
+    return this.pollOne(this.resolveEffectiveTestServer(null, dto));
  }

  /**