From 96161556dbe7b0be51e3d00091112dfc9105785a Mon Sep 17 00:00:00 2001 From: Schalli Date: Wed, 12 Aug 2026 11:41:55 +0200 Subject: [PATCH] feat(17-02): delete protection and 20-feed cap for personal RSS feeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../tenders/tender-rss-feed.service.spec.ts | 192 ++++++++++++++++-- .../src/tenders/tender-rss-feed.service.ts | 55 ++++- .../src/tenders/tenders.controller.spec.ts | 85 +++++++- apps/api/src/tenders/tenders.controller.ts | 26 ++- 4 files changed, 324 insertions(+), 34 deletions(-) diff --git a/apps/api/src/tenders/tender-rss-feed.service.spec.ts b/apps/api/src/tenders/tender-rss-feed.service.spec.ts index 511f1ee..b0ab4a5 100644 --- a/apps/api/src/tenders/tender-rss-feed.service.spec.ts +++ b/apps/api/src/tenders/tender-rss-feed.service.spec.ts @@ -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)); - } - if ('AND' in where) { - return (where.AND as any[]).every((clause) => matchesWhere(row, clause)); - } - return Object.entries(where).every(([key, value]) => row[key] === value); + return Object.entries(where).every(([key, value]) => { + if (key === 'OR') { + return (value as any[]).some((clause) => matchesWhere(row, clause)); + } + if (key === 'AND') { + return (value as any[]).every((clause) => matchesWhere(row, clause)); + } + 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); + }); }); }); diff --git a/apps/api/src/tenders/tender-rss-feed.service.ts b/apps/api/src/tenders/tender-rss-feed.service.ts index cf6e675..ba8e607 100644 --- a/apps/api/src/tenders/tender-rss-feed.service.ts +++ b/apps/api/src/tenders/tender-rss-feed.service.ts @@ -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 }; } diff --git a/apps/api/src/tenders/tenders.controller.spec.ts b/apps/api/src/tenders/tenders.controller.spec.ts index 33c1d39..1f75c9f 100644 --- a/apps/api/src/tenders/tenders.controller.spec.ts +++ b/apps/api/src/tenders/tenders.controller.spec.ts @@ -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)', () => { diff --git a/apps/api/src/tenders/tenders.controller.ts b/apps/api/src/tenders/tenders.controller.ts index bc11b58..dae3f5e 100644 --- a/apps/api/src/tenders/tenders.controller.ts +++ b/apps/api/src/tenders/tenders.controller.ts @@ -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) ─────