feat(17-02): delete protection and 20-feed cap for personal RSS feeds

- remove(id, {userId, isAdmin}) replaces remove(id): single conditional deleteMany (id AND (owned-by-caller OR admin-on-platform-feed)) — no TOCTOU window, ownership check lives in the DB condition. Deletes nothing -> NotFoundException (never Forbidden, no existence leak) - createForUser rejects a caller's 21st personal feed with a clear German message (T-17-10); platform-wide feeds are not counted - DELETE /rss-feeds/:feedId moves from @Roles(ADMIN,SUPER_ADMIN) to @UseModule('tender-radar') — ownership check does the gating now - Tests use a Prisma double that actually evaluates the where condition (not a double that always "succeeds") for both deleteMany and count - Files modified: apps/api/src/tenders/tender-rss-feed.service.ts, apps/api/src/tenders/tenders.controller.ts, apps/api/src/tenders/tender-rss-feed.service.spec.ts, apps/api/src/tenders/tenders.controller.spec.ts
2026-08-12 11:41:55 +02:00
parent adb72f611f
commit 96161556db
4 changed files with 324 additions and 34 deletions
@@ -1,4 +1,4 @@
-import { BadRequestException } from '@nestjs/common';
+import { BadRequestException, NotFoundException } from '@nestjs/common';
 import { describe, expect, it } from 'vitest';
 import { TenderRssFeedSourceService } from './tender-rss-feed.service';

@@ -16,18 +16,23 @@ import { TenderRssFeedSourceService } from './tender-rss-feed.service';
 * Uses the same hand-rolled prisma-shaped fake convention as
 * tender-notification-pref.service.spec.ts / tender-saved-search.service.spec.ts
 * (in-memory Map, no live DB connection). Unlike the pre-Phase-17 fake, this
- * one EVALUATES the `where` clause (OR/AND/equality) so `listForUser`'s
- * ownership scoping is actually proven, not just assumed.
+ * one EVALUATES the `where` clause (top-level keys AND'd together, `OR`/
+ * `AND` sub-clauses handled recursively) so `listForUser`'s ownership
+ * scoping AND `remove()`'s delete-protection condition (T-17-07) are
+ * actually proven, not just assumed — a double that ignored `where` and
+ * always "succeeded" would only fake the protection, not test it.
 */
 function matchesWhere(row: any, where: any): boolean {
  if (!where) return true;
-  if ('OR' in where) {
-    return (where.OR as any[]).some((clause) => matchesWhere(row, clause));
+  return Object.entries(where).every(([key, value]) => {
+    if (key === 'OR') {
+      return (value as any[]).some((clause) => matchesWhere(row, clause));
    }
-  if ('AND' in where) {
-    return (where.AND as any[]).every((clause) => matchesWhere(row, clause));
+    if (key === 'AND') {
+      return (value as any[]).every((clause) => matchesWhere(row, clause));
    }
-  return Object.entries(where).every(([key, value]) => row[key] === value);
+    return row[key] === value;
+  });
 }

 function makeFakePrisma() {
@@ -43,6 +48,8 @@ function makeFakePrisma() {
        }
        return all;
      },
+      count: async ({ where }: any = {}) =>
+        [...rows.values()].filter((row) => matchesWhere(row, where)).length,
      create: async ({ data }: any) => {
        seq += 1;
        const row = {
@@ -61,6 +68,17 @@ function makeFakePrisma() {
        rows.delete(where.id);
        return existing;
      },
+      /**
+       * REAL condition evaluation (T-17-07 precedent, tenders.controller.spec.ts
+       * ~line 1057): only rows matching `matchesWhere` are removed — a double
+       * that deleted unconditionally on `id` alone would defeat the entire
+       * point of this test file's ownership-protection tests.
+       */
+      deleteMany: async ({ where }: any) => {
+        const toDelete = [...rows.values()].filter((row) => matchesWhere(row, where));
+        for (const row of toDelete) rows.delete(row.id);
+        return { count: toDelete.length };
+      },
    },
    __rows: rows,
  };
@@ -271,19 +289,161 @@ describe('TenderRssFeedSourceService', () => {
      expect(prisma.__rows.size).toBe(2);
    });

-    it('remove() deletes the feed by id', async () => {
+  });
+
+  describe('remove() — delete protection (T-17-07, Phase 17 Plan 02 Task 2)', () => {
+    it('User A deletes their own feed: succeeds', async () => {
      const prisma = makeFakePrisma();
      const service = new TenderRssFeedSourceService(prisma as any);

-      const created = await service.createPlatform({
-        url: 'https://c.example-tenders.invalid/rss.xml',
-        label: 'c',
-      });
+      const created = await service.createForUser(
+        { userId: 'user-a', tenantId: 'tenant-a' },
+        { url: 'https://a.example-tenders.invalid/rss.xml', label: 'a' },
+      );

-      const result = await service.remove(created.id);
+      const result = await service.remove(created.id, { userId: 'user-a', isAdmin: false });

      expect(result).toEqual({ success: true });
      expect(prisma.__rows.size).toBe(0);
    });
+
+    it('User A tries to delete User B\'s feed: the feed stays, "not found"', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      const created = await service.createForUser(
+        { userId: 'user-b', tenantId: 'tenant-b' },
+        { url: 'https://b.example-tenders.invalid/rss.xml', label: 'b' },
+      );
+
+      await expect(
+        service.remove(created.id, { userId: 'user-a', isAdmin: false }),
+      ).rejects.toBeInstanceOf(NotFoundException);
+      expect(prisma.__rows.size).toBe(1);
+    });
+
+    it('User A (non-admin) tries to delete a platform-wide feed: the feed stays, "not found"', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      const created = await service.createPlatform({
+        url: 'https://platform.example-tenders.invalid/rss.xml',
+        label: 'platform',
+      });
+
+      await expect(
+        service.remove(created.id, { userId: 'user-a', isAdmin: false }),
+      ).rejects.toBeInstanceOf(NotFoundException);
+      expect(prisma.__rows.size).toBe(1);
+    });
+
+    it('An administrator deletes a platform-wide feed: succeeds', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      const created = await service.createPlatform({
+        url: 'https://platform.example-tenders.invalid/rss.xml',
+        label: 'platform',
+      });
+
+      const result = await service.remove(created.id, { userId: 'admin-1', isAdmin: true });
+
+      expect(result).toEqual({ success: true });
+      expect(prisma.__rows.size).toBe(0);
+    });
+
+    it('An administrator does NOT delete another user\'s personal feed unasked over this path: stays, "not found"', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      const created = await service.createForUser(
+        { userId: 'user-a', tenantId: 'tenant-a' },
+        { url: 'https://a.example-tenders.invalid/rss.xml', label: 'a' },
+      );
+
+      await expect(
+        service.remove(created.id, { userId: 'admin-1', isAdmin: true }),
+      ).rejects.toBeInstanceOf(NotFoundException);
+      expect(prisma.__rows.size).toBe(1);
+    });
+
+    it('removing a genuinely missing id: "not found", nothing to delete', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      await expect(
+        service.remove('does-not-exist', { userId: 'user-a', isAdmin: false }),
+      ).rejects.toBeInstanceOf(NotFoundException);
+    });
+  });
+
+  describe('createForUser() — personal feed cap (T-17-10)', () => {
+    it('rejects the 21st personal feed for the same user with a clear message', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+      const ctx = { userId: 'user-a', tenantId: 'tenant-a' };
+
+      for (let i = 0; i < 20; i += 1) {
+        await service.createForUser(ctx, {
+          url: `https://feed-${i}.example-tenders.invalid/rss.xml`,
+          label: `feed-${i}`,
+        });
+      }
+
+      await expect(
+        service.createForUser(ctx, {
+          url: 'https://feed-21.example-tenders.invalid/rss.xml',
+          label: 'feed-21',
+        }),
+      ).rejects.toThrow(BadRequestException);
+      expect(prisma.__rows.size).toBe(20);
+    });
+
+    it('platform-wide feeds do not count against a user\'s personal cap', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      for (let i = 0; i < 25; i += 1) {
+        await service.createPlatform({
+          url: `https://platform-${i}.example-tenders.invalid/rss.xml`,
+          label: `platform-${i}`,
+        });
+      }
+
+      const created = await service.createForUser(
+        { userId: 'user-a', tenantId: 'tenant-a' },
+        { url: 'https://mine.example-tenders.invalid/rss.xml', label: 'mine' },
+      );
+
+      expect(created.userId).toBe('user-a');
+    });
+  });
+
+  describe('createForUser() — the save-time hostname/SSRF guard also runs on the personal path (T-17-09)', () => {
+    it('rejects a denylisted URL via createForUser exactly like createPlatform', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      await expect(
+        service.createForUser(
+          { userId: 'user-a', tenantId: 'tenant-a' },
+          { url: 'https://www.vergabe24.de/rss.xml', label: 'vergabe24' },
+        ),
+      ).rejects.toThrow(BadRequestException);
+      expect(prisma.__rows.size).toBe(0);
+    });
+
+    it('rejects a private-host URL via createForUser (SSRF)', async () => {
+      const prisma = makeFakePrisma();
+      const service = new TenderRssFeedSourceService(prisma as any);
+
+      await expect(
+        service.createForUser(
+          { userId: 'user-a', tenantId: 'tenant-a' },
+          { url: 'http://192.168.1.1/feed', label: 'internal' },
+        ),
+      ).rejects.toThrow(BadRequestException);
+      expect(prisma.__rows.size).toBe(0);
+    });
  });
 });
@@ -1,8 +1,17 @@
-import { BadRequestException, Injectable } from '@nestjs/common';
+import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common';
 import { PrismaService } from '../prisma/prisma.service';
 import type { TenderRssFeedDto } from './dto/tender-rss-feed.dto';
 import { DENYLISTED_PORTALS } from './source-registry';

+/**
+ * T-17-10 (DoS): caps how many PERSONAL feeds a single user may register.
+ * The create path is open to every module user since Phase 17 (D-02) and
+ * every active feed drives an outbound fetch on each poll tick — without a
+ * ceiling this would be a convenient lever to pad the platform-wide poll
+ * schedule. Platform-wide feeds are unaffected (admin-only to create).
+ */
+const MAX_PERSONAL_FEEDS_PER_USER = 20;
+
 /**
 * TenderRssFeedSourceService — CRUD for the RSS feed list
 * (`TenderRssFeedSource`, D-14/D-08). No `forTenant()`/RLS — ownership is
@@ -45,13 +54,27 @@ export class TenderRssFeedSourceService {
    });
  }

-  /** Creates a personal feed owned by `ctx.userId` (D-02). */
+  /**
+   * Creates a personal feed owned by `ctx.userId` (D-02). Rejects with a
+   * clear German message once the caller already owns
+   * `MAX_PERSONAL_FEEDS_PER_USER` feeds (T-17-10) — platform-wide feeds are
+   * never counted against this cap.
+   */
  async createForUser(
    ctx: { userId: string; tenantId: string },
    dto: TenderRssFeedDto,
  ) {
    this.assertUrlAllowed(dto.url);

+    const existingCount = await this.prisma.tenderRssFeedSource.count({
+      where: { userId: ctx.userId },
+    });
+    if (existingCount >= MAX_PERSONAL_FEEDS_PER_USER) {
+      throw new BadRequestException(
+        `Sie haben bereits die Obergrenze von ${MAX_PERSONAL_FEEDS_PER_USER} eigenen RSS-Feeds erreicht. Entfernen Sie zuerst einen bestehenden Feed, bevor Sie einen neuen anlegen.`,
+      );
+    }
+
    return this.prisma.tenderRssFeedSource.create({
      data: {
        url: dto.url,
@@ -80,8 +103,32 @@ export class TenderRssFeedSourceService {
    });
  }

-  async remove(id: string) {
-    await this.prisma.tenderRssFeedSource.delete({ where: { id } });
+  /**
+   * Removes a feed — but only if the caller is allowed to (T-17-07): either
+   * the feed is owned by the caller, or the caller is ADMIN/SUPER_ADMIN
+   * AND the feed is platform-wide (`userId = null`). Deliberately a SINGLE
+   * conditional `deleteMany` rather than "read, check ownership in
+   * application code, then delete" — no time-of-check/time-of-use window,
+   * and the ownership comparison lives in the database condition, not in
+   * TypeScript. When nothing matches (foreign feed, or a non-admin
+   * targeting a platform-wide feed), throws `NotFoundException` — never
+   * `ForbiddenException` — so the response never confirms whether a
+   * feed with that id exists at all.
+   */
+  async remove(id: string, ctx: { userId: string; isAdmin: boolean }) {
+    const { userId, isAdmin } = ctx;
+
+    const result = await this.prisma.tenderRssFeedSource.deleteMany({
+      where: {
+        id,
+        OR: [{ userId }, ...(isAdmin ? [{ userId: null }] : [])],
+      },
+    });
+
+    if (result.count === 0) {
+      throw new NotFoundException('RSS-Feed nicht gefunden.');
+    }
+
    return { success: true };
  }

@@ -1,4 +1,4 @@
-import { BadRequestException, NotFoundException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
 import { Role } from '@prisma/client';
 import { describe, expect, it, vi } from 'vitest';
 import { ROLES_KEY } from '../auth/decorators/roles.decorator';
@@ -82,9 +82,14 @@ function makeFakeTriageService() {
  };
 }

-/** Minimal authenticated Express Request fake (userId/tenantId only). */
-function makeFakeRequest(userId = 'u1', tenantId = 'tenant1') {
-  return { user: { id: userId, tenantId }, tenantId } as any;
+/**
+ * Minimal authenticated Express Request fake (userId/tenantId/role).
+ * `role` defaults to `Role.USER` — Phase 17, Plan 02 (T-17-08) added it so
+ * `extractTriageContext` can resolve the caller's role from the SAME place
+ * `RolesGuard` reads it, for the `POST /rss-feeds` scope:'platform' check.
+ */
+function makeFakeRequest(userId = 'u1', tenantId = 'tenant1', role: Role = Role.USER) {
+  return { user: { id: userId, tenantId, role }, tenantId } as any;
 }

 /**
@@ -962,7 +967,7 @@ describe('TendersController — RSS-feeds personal + platform-wide (Plan 14-02 D
    ).rejects.toBeInstanceOf(BadRequestException);
  });

-  it('DELETE /rss-feeds/:feedId delegates to tenderRssFeedSource.remove(feedId)', async () => {
+  it('DELETE /rss-feeds/:feedId delegates to tenderRssFeedSource.remove(feedId, {userId, isAdmin}) — isAdmin false for a USER', async () => {
    const prisma = makeFakePrisma();
    const scheduler = { setInterval: vi.fn(), stopJob: vi.fn() } as any;
    const rssFeedService = makeFakeRssFeedService();
@@ -976,11 +981,77 @@ describe('TendersController — RSS-feeds personal + platform-wide (Plan 14-02 D
      makeFakeEmailConfigService() as any,
    );

-    const result = await controller.removeRssFeed('feed-1');
+    const result = await controller.removeRssFeed(
+      'feed-1',
+      makeFakeRequest('u1', 'tenant1', Role.USER),
+    );

-    expect(rssFeedService.remove).toHaveBeenCalledWith('feed-1');
+    expect(rssFeedService.remove).toHaveBeenCalledWith('feed-1', { userId: 'u1', isAdmin: false });
    expect(result).toEqual({ success: true });
  });
+
+  it('DELETE /rss-feeds/:feedId passes isAdmin true for an ADMIN account', async () => {
+    const prisma = makeFakePrisma();
+    const scheduler = { setInterval: vi.fn(), stopJob: vi.fn() } as any;
+    const rssFeedService = makeFakeRssFeedService();
+    const controller = new TendersController(
+      prisma as any,
+      scheduler,
+      makeFakeTriageService() as any,
+      makeFakeSavedSearchService() as any,
+      makeFakeNotificationPrefService() as any,
+      rssFeedService as any,
+      makeFakeEmailConfigService() as any,
+    );
+
+    await controller.removeRssFeed('feed-1', makeFakeRequest('admin-1', 'tenant1', Role.ADMIN));
+
+    expect(rssFeedService.remove).toHaveBeenCalledWith('feed-1', { userId: 'admin-1', isAdmin: true });
+  });
+
+  it('POST /rss-feeds with scope:"platform" and role USER is rejected with ForbiddenException, service never called (T-17-08)', async () => {
+    const prisma = makeFakePrisma();
+    const scheduler = { setInterval: vi.fn(), stopJob: vi.fn() } as any;
+    const rssFeedService = makeFakeRssFeedService();
+    const controller = new TendersController(
+      prisma as any,
+      scheduler,
+      makeFakeTriageService() as any,
+      makeFakeSavedSearchService() as any,
+      makeFakeNotificationPrefService() as any,
+      rssFeedService as any,
+      makeFakeEmailConfigService() as any,
+    );
+
+    const dto = { url: 'https://service.bund.de/rss.xml', label: 'service-bund', scope: 'platform' } as any;
+
+    await expect(
+      controller.createRssFeed(dto, makeFakeRequest('u1', 'tenant1', Role.USER)),
+    ).rejects.toBeInstanceOf(ForbiddenException);
+    expect(rssFeedService.createPlatform).not.toHaveBeenCalled();
+    expect(rssFeedService.createForUser).not.toHaveBeenCalled();
+  });
+
+  it('POST /rss-feeds with scope:"platform" and role ADMIN delegates to tenderRssFeedSource.createPlatform(dto) (T-17-08)', async () => {
+    const prisma = makeFakePrisma();
+    const scheduler = { setInterval: vi.fn(), stopJob: vi.fn() } as any;
+    const rssFeedService = makeFakeRssFeedService();
+    const controller = new TendersController(
+      prisma as any,
+      scheduler,
+      makeFakeTriageService() as any,
+      makeFakeSavedSearchService() as any,
+      makeFakeNotificationPrefService() as any,
+      rssFeedService as any,
+      makeFakeEmailConfigService() as any,
+    );
+
+    const dto = { url: 'https://service.bund.de/rss.xml', label: 'service-bund', scope: 'platform' } as any;
+    await controller.createRssFeed(dto, makeFakeRequest('admin-1', 'tenant1', Role.ADMIN));
+
+    expect(rssFeedService.createPlatform).toHaveBeenCalledWith(dto);
+    expect(rssFeedService.createForUser).not.toHaveBeenCalled();
+  });
 });

 describe('TendersController — email-config (Plan 14-03, per-user since Phase 17 Plan 01 D-01, T-14-03-05/T-17-01)', () => {
@@ -302,15 +302,27 @@ export class TendersController {
  }

  /**
-   * DELETE /modules/tender-radar/rss-feeds/:feedId — remove a global RSS
-   * feed. Uses `:feedId` (not `:id`) so this route can never be confused
-   * with the Tender `:id` route below (Pitfall 5, same convention as
-   * `saved-searches/:searchId`).
+   * DELETE /modules/tender-radar/rss-feeds/:feedId — remove a feed.
+   * Ownership is enforced entirely inside the service's single conditional
+   * `deleteMany` (T-17-07): the caller may delete their own feed, or — if
+   * ADMIN/SUPER_ADMIN — a platform-wide feed. Anything else (someone
+   * else's personal feed, or a non-admin targeting a platform-wide feed)
+   * surfaces as `NotFoundException`, never `ForbiddenException` — the
+   * response never confirms whether a foreign id exists. Phase 17: replaced
+   * `@Roles(ADMIN, SUPER_ADMIN)` with `@UseModule('tender-radar')` — every
+   * module user may reach this route now, the ownership check does the
+   * rest.
+   *
+   * Uses `:feedId` (not `:id`) so this route can never be confused with
+   * the Tender `:id` route below (Pitfall 5, same convention as
+   * `saved-searches/:searchId`). Route position UNCHANGED.
   */
  @Delete('rss-feeds/:feedId')
-  @Roles(Role.ADMIN, Role.SUPER_ADMIN)
-  async removeRssFeed(@Param('feedId') feedId: string) {
-    return this.tenderRssFeedSource.remove(feedId);
+  @UseModule('tender-radar')
+  async removeRssFeed(@Param('feedId') feedId: string, @Req() req: Request) {
+    const { userId, role } = this.extractTriageContext(req);
+    const isAdmin = role === Role.ADMIN || role === Role.SUPER_ADMIN;
+    return this.tenderRssFeedSource.remove(feedId, { userId, isAdmin });
  }

  // ─── E-Mail-Alerts config (ModuleGuard-gated, PER-USER, Phase 17 D-01) ─────