From 75f0d4fd514169137a45b53391043fd28d2674ee Mon Sep 17 00:00:00 2001 From: Nick Szaunig Date: Sat, 22 Aug 2026 22:50:00 +0200 Subject: [PATCH] fix(backend): IDOR in POST /notifications/:id/read (Modul 26) markRead() pruefte bisher nicht, ob die Benachrichtigung dem anfragenden Nutzer gehoert - jeder authentifizierte Nutzer konnte fremde Benachrichtigungen als gelesen markieren. Jetzt: 403 bei fremder Benachrichtigung, weiterhin erlaubt fuer eigene und Broadcast-Benachrichtigungen (userId = null). E2E-Regressionstest ergaenzt. --- .../notifications/notifications.controller.ts | 4 +-- .../notifications.service.spec.ts | 33 ++++++++++++++++--- .../notifications/notifications.service.ts | 11 +++++-- .../test/notifications.e2e-spec.ts | 27 +++++++++++++-- 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/ApiCore-Backend/src/notifications/notifications.controller.ts b/ApiCore-Backend/src/notifications/notifications.controller.ts index bbc73eb..b8b6b29 100644 --- a/ApiCore-Backend/src/notifications/notifications.controller.ts +++ b/ApiCore-Backend/src/notifications/notifications.controller.ts @@ -22,7 +22,7 @@ export class NotificationsController { } @Post(':id/read') - markRead(@Param('id') id: string) { - return this.notificationsService.markRead(id); + markRead(@Param('id') id: string, @CurrentUser() actor: AuthenticatedUser) { + return this.notificationsService.markRead(id, actor.id); } } diff --git a/ApiCore-Backend/src/notifications/notifications.service.spec.ts b/ApiCore-Backend/src/notifications/notifications.service.spec.ts index b1892c6..60906ed 100644 --- a/ApiCore-Backend/src/notifications/notifications.service.spec.ts +++ b/ApiCore-Backend/src/notifications/notifications.service.spec.ts @@ -1,4 +1,4 @@ -import { NotFoundException } from '@nestjs/common'; +import { ForbiddenException, NotFoundException } from '@nestjs/common'; import { NotificationsService } from './notifications.service'; import { PrismaService } from '../prisma/prisma.service'; @@ -55,19 +55,44 @@ describe('NotificationsService', () => { it('throws NotFoundException when marking an unknown notification as read', async () => { prisma.notification.findUnique.mockResolvedValueOnce(null); - await expect(service.markRead('missing')).rejects.toBeInstanceOf( + await expect(service.markRead('missing', 'user-1')).rejects.toBeInstanceOf( NotFoundException, ); }); - it('marks a notification as read', async () => { + it("throws ForbiddenException when marking another user's notification as read", async () => { prisma.notification.findUnique.mockResolvedValueOnce({ id: 'n1', + userId: 'owner', + read: false, + }); + await expect(service.markRead('n1', 'someone-else')).rejects.toBeInstanceOf( + ForbiddenException, + ); + }); + + it('marks a broadcast notification (no owner) as read for any user', async () => { + prisma.notification.findUnique.mockResolvedValueOnce({ + id: 'n1', + userId: null, read: false, }); prisma.notification.update.mockResolvedValueOnce({ id: 'n1', read: true }); - const result = await service.markRead('n1'); + const result = await service.markRead('n1', 'any-user'); + + expect(result.read).toBe(true); + }); + + it("marks the owner's own notification as read", async () => { + prisma.notification.findUnique.mockResolvedValueOnce({ + id: 'n1', + userId: 'user-1', + read: false, + }); + prisma.notification.update.mockResolvedValueOnce({ id: 'n1', read: true }); + + const result = await service.markRead('n1', 'user-1'); expect(result.read).toBe(true); }); diff --git a/ApiCore-Backend/src/notifications/notifications.service.ts b/ApiCore-Backend/src/notifications/notifications.service.ts index 3ef3608..edb06b7 100644 --- a/ApiCore-Backend/src/notifications/notifications.service.ts +++ b/ApiCore-Backend/src/notifications/notifications.service.ts @@ -1,4 +1,8 @@ -import { Injectable, NotFoundException } from '@nestjs/common'; +import { + ForbiddenException, + Injectable, + NotFoundException, +} from '@nestjs/common'; import { PrismaService } from '../prisma/prisma.service'; import { Notification, NotificationPriority } from '../../generated/prisma'; @@ -36,13 +40,16 @@ export class NotificationsService { }); } - async markRead(id: string): Promise { + async markRead(id: string, actingUserId: string): Promise { const notification = await this.prisma.notification.findUnique({ where: { id }, }); if (!notification) { throw new NotFoundException('Notification not found'); } + if (notification.userId !== null && notification.userId !== actingUserId) { + throw new ForbiddenException("Cannot modify another user's notification"); + } return this.prisma.notification.update({ where: { id }, data: { read: true }, diff --git a/ApiCore-Backend/test/notifications.e2e-spec.ts b/ApiCore-Backend/test/notifications.e2e-spec.ts index d1b0765..2692d72 100644 --- a/ApiCore-Backend/test/notifications.e2e-spec.ts +++ b/ApiCore-Backend/test/notifications.e2e-spec.ts @@ -17,8 +17,11 @@ describe('Notifications (e2e)', () => { let prisma: PrismaService; const beekeeperEmail = `notifications-e2e-beekeeper-${Date.now()}@example.com`; const password = 'SuperSecret123!'; + const staffEmail = `notifications-e2e-staff-${Date.now()}@example.com`; let beekeeperId: string; + let staffId: string; let beekeeperToken: string; + let staffToken: string; let notificationId: string; beforeAll(async () => { @@ -35,7 +38,12 @@ describe('Notifications (e2e)', () => { }, }); beekeeperId = beekeeper.id; + const staff = await prisma.user.create({ + data: { email: staffEmail, passwordHash, name: 'Staff', role: 'STAFF' }, + }); + staffId = staff.id; beekeeperToken = (await login(app, beekeeperEmail, password)).accessToken; + staffToken = (await login(app, staffEmail, password)).accessToken; const notification = await prisma.notification.create({ data: { @@ -49,9 +57,15 @@ describe('Notifications (e2e)', () => { afterAll(async () => { await prisma.notification.deleteMany({ where: { id: notificationId } }); - await prisma.refreshToken.deleteMany({ where: { userId: beekeeperId } }); - await prisma.auditLog.deleteMany({ where: { userId: beekeeperId } }); - await prisma.user.deleteMany({ where: { id: beekeeperId } }); + await prisma.refreshToken.deleteMany({ + where: { userId: { in: [beekeeperId, staffId] } }, + }); + await prisma.auditLog.deleteMany({ + where: { userId: { in: [beekeeperId, staffId] } }, + }); + await prisma.user.deleteMany({ + where: { id: { in: [beekeeperId, staffId] } }, + }); await app.close(); }); @@ -70,6 +84,13 @@ describe('Notifications (e2e)', () => { ).toBe(true); }); + it("rejects marking another user's notification as read", () => { + return request(app.getHttpServer()) + .post(`/api/v1/notifications/${notificationId}/read`) + .set('Authorization', `Bearer ${staffToken}`) + .expect(403); + }); + it('marks a notification as read', async () => { const response = await request(app.getHttpServer()) .post(`/api/v1/notifications/${notificationId}/read`)