From e5a4273ab57ca34c803a7043bc16313690c69ae6 Mon Sep 17 00:00:00 2001 From: teetyff Date: Mon, 29 Jun 2026 04:37:47 +0100 Subject: [PATCH 1/2] feat: add media ownership verification and authorization --- src/controllers/media.controller.ts | 51 +++++++ src/models/media.ts | 14 ++ src/services/event-ticket.service.ts | 13 ++ tests/media.controller.test.ts | 208 +++++++++++++++++++++++++++ 4 files changed, 286 insertions(+) create mode 100644 src/models/media.ts create mode 100644 tests/media.controller.test.ts diff --git a/src/controllers/media.controller.ts b/src/controllers/media.controller.ts index 4d9dcf8..237a1d4 100644 --- a/src/controllers/media.controller.ts +++ b/src/controllers/media.controller.ts @@ -1,6 +1,28 @@ import { RequestHandler } from 'express'; import { MediaService } from '../services/media.service'; import { cloudinaryService } from '../lib/cloudinary'; +import Media from '../models/media'; +import EventTicket from '../models/event-ticket'; + +async function verifyMediaOwnership(req: any, publicId: string): Promise { + const userId = req.user?._id || req.user?.id; + if (!userId) return false; + + const isAdmin = (req.user as any)?.role === 'admin'; + if (isAdmin) return true; + + const mediaRecord = await Media.findOne({ publicId }); + if (mediaRecord && mediaRecord.userId.toString() === userId.toString()) { + return true; + } + + const eventTicket = await EventTicket.findOne({ cloudinary_public_id: publicId }); + if (eventTicket && eventTicket.organizedBy.toString() === userId.toString()) { + return true; + } + + return false; +} export const uploadMedia: RequestHandler = async (req, res) => { try { @@ -15,6 +37,19 @@ export const uploadMedia: RequestHandler = async (req, res) => { const result = await MediaService.upload(req.file.buffer, { folder }); + const userId = req.user?._id || req.user?.id; + if (userId) { + try { + await Media.findOneAndUpdate( + { publicId: result.publicId }, + { userId: userId, publicId: result.publicId }, + { upsert: true, new: true }, + ); + } catch (e) { + if ((e as any)?.code !== 11000) throw e; + } + } + res.status(201).json({ message: 'File uploaded successfully', data: result, @@ -39,6 +74,14 @@ export const invalidateMedia: RequestHandler = async (req, res) => { }); } + const authorized = await verifyMediaOwnership(req, publicId); + if (!authorized) { + return res.status(403).json({ + error: 'Forbidden', + message: 'You are not authorized to invalidate this media', + }); + } + const result = await MediaService.invalidate(publicId); res.status(200).json({ @@ -68,6 +111,14 @@ export const destroyMedia: RequestHandler = async (req, res) => { }); } + const authorized = await verifyMediaOwnership(req, publicId); + if (!authorized) { + return res.status(403).json({ + error: 'Forbidden', + message: 'You are not authorized to delete this media', + }); + } + const result = await MediaService.destroy(publicId); res.status(200).json({ diff --git a/src/models/media.ts b/src/models/media.ts new file mode 100644 index 0000000..1c7f443 --- /dev/null +++ b/src/models/media.ts @@ -0,0 +1,14 @@ +import mongoose, { Schema, Document } from 'mongoose'; + +export interface IMedia extends Document { + publicId: string; + userId: mongoose.Types.ObjectId; +} + +const mediaSchema = new Schema({ + publicId: { type: String, required: true, unique: true }, + userId: { type: Schema.Types.ObjectId, ref: 'User' }, +}, { timestamps: true }); + +const Media = mongoose.model('Media', mediaSchema); +export default Media; diff --git a/src/services/event-ticket.service.ts b/src/services/event-ticket.service.ts index 622fbaf..d0d108c 100644 --- a/src/services/event-ticket.service.ts +++ b/src/services/event-ticket.service.ts @@ -2,6 +2,7 @@ import mongoose from 'mongoose'; import EventTicket, { IEventTicket } from '../models/event-ticket'; import TicketOrder, { ITicketOrder } from '../models/ticket-order'; import User from '../models/user'; +import Media from '../models/media'; import zkEmailNotificationService from './zk-email-notification.service'; import { CreateEventStepTwoInput } from '../validators/event.validator'; @@ -319,6 +320,18 @@ export class EventTicketService { requiresVerification, }); + if (event.cloudinary_public_id && baseEventData.organizedBy) { + try { + await Media.findOneAndUpdate( + { publicId: event.cloudinary_public_id }, + { userId: baseEventData.organizedBy, publicId: event.cloudinary_public_id }, + { upsert: true, new: true }, + ); + } catch (e) { + if ((e as any)?.code !== 11000) throw e; + } + } + return event; } catch (error) { throw new Error( diff --git a/tests/media.controller.test.ts b/tests/media.controller.test.ts new file mode 100644 index 0000000..d32d523 --- /dev/null +++ b/tests/media.controller.test.ts @@ -0,0 +1,208 @@ +import { destroyMedia, invalidateMedia, uploadMedia } from '../src/controllers/media.controller'; +import Media from '../src/models/media'; +import EventTicket from '../src/models/event-ticket'; +import { MediaService } from '../src/services/media.service'; + +jest.mock('../src/models/media'); +jest.mock('../src/models/event-ticket'); +jest.mock('../src/services/media.service'); + +describe('media controller — IDOR protection (issue #132)', () => { + const createResponse = () => { + const res = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + return res; + }; + + const createRequest = (user: any, body: any = {}, file: any = null) => ({ + user, + body, + file, + query: {}, + }); + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('destroyMedia', () => { + it('returns 400 when publicId is missing', async () => { + const req = createRequest({ _id: 'user-1', role: 'user' }, {}); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + error: 'Invalid request', + message: 'A valid "publicId" string is required in the request body', + }); + }); + + it('returns 403 when user does not own the media', async () => { + (Media.findOne as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + userId: 'other-user-id', + }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(Media.findOne).toHaveBeenCalledWith({ publicId: 'media-123' }); + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'You are not authorized to delete this media', + }); + }); + + it('returns 403 when admin is missing role (not admin)', async () => { + (Media.findOne as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + userId: 'other-user-id', + }); + + const req = createRequest({ _id: 'user-1' }, { publicId: 'media-123' }); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(403); + }); + + it('allows admin to destroy any media', async () => { + (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); + + const req = createRequest({ _id: 'admin-1', role: 'admin' }, { publicId: 'media-123' }); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(Media.findOne).not.toHaveBeenCalled(); + expect(EventTicket.findOne).not.toHaveBeenCalled(); + expect(MediaService.destroy).toHaveBeenCalledWith('media-123'); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('allows user to destroy their own media', async () => { + (Media.findOne as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + userId: 'user-1', + }); + (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(MediaService.destroy).toHaveBeenCalledWith('media-123'); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('allows user to destroy media owned by their event ticket', async () => { + (Media.findOne as jest.Mock).mockResolvedValue(null); + (EventTicket.findOne as jest.Mock).mockResolvedValue({ + cloudinary_public_id: 'media-456', + organizedBy: 'user-1', + }); + (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-456' }); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(EventTicket.findOne).toHaveBeenCalledWith({ cloudinary_public_id: 'media-456' }); + expect(MediaService.destroy).toHaveBeenCalledWith('media-456'); + expect(res.status).toHaveBeenCalledWith(200); + }); + }); + + describe('invalidateMedia', () => { + it('returns 400 when publicId is missing', async () => { + const req = createRequest({ _id: 'user-1', role: 'user' }, {}); + const res = createResponse(); + + await invalidateMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(400); + }); + + it('returns 403 when user does not own the media', async () => { + (Media.findOne as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + userId: 'other-user-id', + }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const res = createResponse(); + + await invalidateMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'You are not authorized to invalidate this media', + }); + }); + + it('allows admin to invalidate any media', async () => { + (MediaService.invalidate as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + url: 'https://example.com/image.jpg', + }); + + const req = createRequest({ _id: 'admin-1', role: 'admin' }, { publicId: 'media-123' }); + const res = createResponse(); + + await invalidateMedia(req as any, res as any, jest.fn()); + + expect(MediaService.invalidate).toHaveBeenCalledWith('media-123'); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it('allows user to invalidate their own media', async () => { + (Media.findOne as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + userId: 'user-1', + }); + (MediaService.invalidate as jest.Mock).mockResolvedValue({ + publicId: 'media-123', + url: 'https://example.com/image.jpg', + }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const res = createResponse(); + + await invalidateMedia(req as any, res as any, jest.fn()); + + expect(MediaService.invalidate).toHaveBeenCalledWith('media-123'); + expect(res.status).toHaveBeenCalledWith(200); + }); + }); + + describe('uploadMedia', () => { + it('returns 400 when no file provided', async () => { + const req = createRequest({ _id: 'user-1', role: 'user' }, {}, null); + const res = createResponse(); + + await uploadMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + error: 'No file provided', + message: 'An image file is required in the "image" field', + }); + }); + }); +}); \ No newline at end of file From 46d514a95ab2da2d5243a30055f52f0fadb5a840 Mon Sep 17 00:00:00 2001 From: teetyff Date: Mon, 29 Jun 2026 05:38:01 +0100 Subject: [PATCH 2/2] fix: make Media.userId required and add transaction to event creation - Make userId required in Media schema to match IMedia interface - Wrap EventTicket.create and Media ownership check in MongoDB session transaction - Verify existing Media belongs to organizer instead of upserting (prevents ownership transfer) - Reject event creation if media doesn't exist or isn't owned by organizer - Add test for non-existent publicId authorization path in destroyMedia/invalidateMedia - Add test for successful uploadMedia path with Media.findOneAndUpdate assertion - Use resetAllMocks in beforeEach to prevent mock bleed between tests --- src/models/media.ts | 11 ++- src/services/event-ticket.service.ts | 65 ++++++++------ tests/media.controller.test.ts | 128 ++++++++++++++++++++++++--- 3 files changed, 163 insertions(+), 41 deletions(-) diff --git a/src/models/media.ts b/src/models/media.ts index 1c7f443..6ced6f4 100644 --- a/src/models/media.ts +++ b/src/models/media.ts @@ -5,10 +5,13 @@ export interface IMedia extends Document { userId: mongoose.Types.ObjectId; } -const mediaSchema = new Schema({ - publicId: { type: String, required: true, unique: true }, - userId: { type: Schema.Types.ObjectId, ref: 'User' }, -}, { timestamps: true }); +const mediaSchema = new Schema( + { + publicId: { type: String, required: true, unique: true }, + userId: { type: Schema.Types.ObjectId, ref: 'User', required: true }, + }, + { timestamps: true }, +); const Media = mongoose.model('Media', mediaSchema); export default Media; diff --git a/src/services/event-ticket.service.ts b/src/services/event-ticket.service.ts index d0d108c..8c9f5a3 100644 --- a/src/services/event-ticket.service.ts +++ b/src/services/event-ticket.service.ts @@ -271,6 +271,9 @@ export class EventTicketService { tags: string[]; }, ): Promise { + const session = await mongoose.startSession(); + session.startTransaction(); + try { const { privacyLevel, @@ -300,40 +303,50 @@ export class EventTicketService { attendanceMode || this.mapPrivacyLevelToAttendanceMode(privacyLevel); // Create the event with all privacy settings - const event = await EventTicket.create({ - ...baseEventData, - privacyLevel, - attendanceMode: mappedAttendanceMode, - eventType, - locationType, - location, - paymentPrivacy, - offerReceipts, - hasZkEmailUpdates, - hasEventReminders, - ticketType: ticketTypes, - totalTickets, - availableTickets: totalTickets, - soldTickets: 0, - isPublished, - allowAnonymous, - requiresVerification, - }); + const event = await EventTicket.create( + [ + { + ...baseEventData, + privacyLevel, + attendanceMode: mappedAttendanceMode, + eventType, + locationType, + location, + paymentPrivacy, + offerReceipts, + hasZkEmailUpdates, + hasEventReminders, + ticketType: ticketTypes, + totalTickets, + availableTickets: totalTickets, + soldTickets: 0, + isPublished, + allowAnonymous, + requiresVerification, + }, + ], + { session }, + ).then((docs) => docs[0]); if (event.cloudinary_public_id && baseEventData.organizedBy) { - try { - await Media.findOneAndUpdate( - { publicId: event.cloudinary_public_id }, - { userId: baseEventData.organizedBy, publicId: event.cloudinary_public_id }, - { upsert: true, new: true }, + const existingMedia = await Media.findOne({ + publicId: event.cloudinary_public_id, + userId: baseEventData.organizedBy, + }).session(session); + if (!existingMedia) { + throw new Error( + `Media with publicId ${event.cloudinary_public_id} does not exist or is not owned by organizer`, ); - } catch (e) { - if ((e as any)?.code !== 11000) throw e; } } + await session.commitTransaction(); + session.endSession(); + return event; } catch (error) { + await session.abortTransaction(); + session.endSession(); throw new Error( `Failed to create event with privacy settings: ${error instanceof Error ? error.message : 'Unknown error'}`, ); diff --git a/tests/media.controller.test.ts b/tests/media.controller.test.ts index d32d523..a7f90c0 100644 --- a/tests/media.controller.test.ts +++ b/tests/media.controller.test.ts @@ -1,4 +1,8 @@ -import { destroyMedia, invalidateMedia, uploadMedia } from '../src/controllers/media.controller'; +import { + destroyMedia, + invalidateMedia, + uploadMedia, +} from '../src/controllers/media.controller'; import Media from '../src/models/media'; import EventTicket from '../src/models/event-ticket'; import { MediaService } from '../src/services/media.service'; @@ -24,7 +28,7 @@ describe('media controller — IDOR protection (issue #132)', () => { }); beforeEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); jest.spyOn(console, 'error').mockImplementation(() => {}); }); @@ -52,7 +56,10 @@ describe('media controller — IDOR protection (issue #132)', () => { userId: 'other-user-id', }); - const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'media-123' }, + ); const res = createResponse(); await destroyMedia(req as any, res as any, jest.fn()); @@ -79,10 +86,38 @@ describe('media controller — IDOR protection (issue #132)', () => { expect(res.status).toHaveBeenCalledWith(403); }); + it('returns 403 when publicId does not exist anywhere', async () => { + (Media.findOne as jest.Mock).mockResolvedValue(null); + (EventTicket.findOne as jest.Mock).mockResolvedValue(null); + + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'nonexistent-public-id' }, + ); + const res = createResponse(); + + await destroyMedia(req as any, res as any, jest.fn()); + + expect(Media.findOne).toHaveBeenCalledWith({ + publicId: 'nonexistent-public-id', + }); + expect(EventTicket.findOne).toHaveBeenCalledWith({ + cloudinary_public_id: 'nonexistent-public-id', + }); + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'You are not authorized to delete this media', + }); + }); + it('allows admin to destroy any media', async () => { (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); - const req = createRequest({ _id: 'admin-1', role: 'admin' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'admin-1', role: 'admin' }, + { publicId: 'media-123' }, + ); const res = createResponse(); await destroyMedia(req as any, res as any, jest.fn()); @@ -100,7 +135,10 @@ describe('media controller — IDOR protection (issue #132)', () => { }); (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); - const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'media-123' }, + ); const res = createResponse(); await destroyMedia(req as any, res as any, jest.fn()); @@ -117,12 +155,17 @@ describe('media controller — IDOR protection (issue #132)', () => { }); (MediaService.destroy as jest.Mock).mockResolvedValue({ result: 'ok' }); - const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-456' }); + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'media-456' }, + ); const res = createResponse(); await destroyMedia(req as any, res as any, jest.fn()); - expect(EventTicket.findOne).toHaveBeenCalledWith({ cloudinary_public_id: 'media-456' }); + expect(EventTicket.findOne).toHaveBeenCalledWith({ + cloudinary_public_id: 'media-456', + }); expect(MediaService.destroy).toHaveBeenCalledWith('media-456'); expect(res.status).toHaveBeenCalledWith(200); }); @@ -144,11 +187,39 @@ describe('media controller — IDOR protection (issue #132)', () => { userId: 'other-user-id', }); - const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'media-123' }, + ); + const res = createResponse(); + + await invalidateMedia(req as any, res as any, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: 'You are not authorized to invalidate this media', + }); + }); + + it('returns 403 when publicId does not exist anywhere', async () => { + (Media.findOne as jest.Mock).mockResolvedValue(null); + (EventTicket.findOne as jest.Mock).mockResolvedValue(null); + + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'nonexistent-public-id' }, + ); const res = createResponse(); await invalidateMedia(req as any, res as any, jest.fn()); + expect(Media.findOne).toHaveBeenCalledWith({ + publicId: 'nonexistent-public-id', + }); + expect(EventTicket.findOne).toHaveBeenCalledWith({ + cloudinary_public_id: 'nonexistent-public-id', + }); expect(res.status).toHaveBeenCalledWith(403); expect(res.json).toHaveBeenCalledWith({ error: 'Forbidden', @@ -162,7 +233,10 @@ describe('media controller — IDOR protection (issue #132)', () => { url: 'https://example.com/image.jpg', }); - const req = createRequest({ _id: 'admin-1', role: 'admin' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'admin-1', role: 'admin' }, + { publicId: 'media-123' }, + ); const res = createResponse(); await invalidateMedia(req as any, res as any, jest.fn()); @@ -181,7 +255,10 @@ describe('media controller — IDOR protection (issue #132)', () => { url: 'https://example.com/image.jpg', }); - const req = createRequest({ _id: 'user-1', role: 'user' }, { publicId: 'media-123' }); + const req = createRequest( + { _id: 'user-1', role: 'user' }, + { publicId: 'media-123' }, + ); const res = createResponse(); await invalidateMedia(req as any, res as any, jest.fn()); @@ -204,5 +281,34 @@ describe('media controller — IDOR protection (issue #132)', () => { message: 'An image file is required in the "image" field', }); }); + + it('calls Media.findOneAndUpdate with publicId and userId after successful upload', async () => { + const mockFile = { buffer: Buffer.from('test-image') }; + const mockUploadResult = { + publicId: 'uploaded-public-id', + url: 'https://example.com/image.jpg', + }; + + (MediaService.upload as jest.Mock).mockResolvedValue(mockUploadResult); + (Media.findOneAndUpdate as jest.Mock).mockResolvedValue({ + publicId: 'uploaded-public-id', + userId: 'user-1', + }); + + const req = createRequest({ _id: 'user-1', role: 'user' }, {}, mockFile); + const res = createResponse(); + + await uploadMedia(req as any, res as any, jest.fn()); + + expect(MediaService.upload).toHaveBeenCalledWith(mockFile.buffer, { + folder: undefined, + }); + expect(Media.findOneAndUpdate).toHaveBeenCalledWith( + { publicId: mockUploadResult.publicId }, + { userId: 'user-1', publicId: mockUploadResult.publicId }, + { upsert: true, new: true }, + ); + expect(res.status).toHaveBeenCalledWith(201); + }); }); -}); \ No newline at end of file +});