From 1faccc872fb1373008a82b567f1860318d49a8ac Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Fri, 22 May 2026 19:04:51 -0400 Subject: [PATCH 1/3] fix(event): close attendee-management IDOR on delete/update routes The DELETE/PATCH /events/:slug/attendees/:attendeeId routes had their @Permissions(ManageAttendees) decorator commented out (since #235, May 2025), so PermissionsGuard fell through (no metadata => allow) and any authenticated user could delete or mutate any attendee by id. - Re-enable @Permissions(ManageAttendees) on both routes - Route delete through EventManagementService for event-scoping - Add findEventAttendeeScopedToEvent() and verify the attendee belongs to :slug before update/delete (defense-in-depth vs cross-event IDOR) - Add e2e regression spec (RED proven against live API) WIP: GREEN e2e run still pending (running API serves main worktree). Controller unit spec passes (30/30). --- src/event-attendee/event-attendee.service.ts | 19 ++ src/event/event.controller.ts | 20 +- .../services/event-management.service.ts | 23 ++- .../attendee-authorization.e2e-spec.ts | 183 ++++++++++++++++++ 4 files changed, 233 insertions(+), 12 deletions(-) create mode 100644 test/event-attendees/attendee-authorization.e2e-spec.ts diff --git a/src/event-attendee/event-attendee.service.ts b/src/event-attendee/event-attendee.service.ts index 7cbe54a5..cd5d97d5 100644 --- a/src/event-attendee/event-attendee.service.ts +++ b/src/event-attendee/event-attendee.service.ts @@ -1044,6 +1044,25 @@ export class EventAttendeeService { return deleted; } + // Verifies the attendee record actually belongs to the given event before a + // management action (update/delete) acts on it. Without this, a user with + // ManageAttendees on event A could pass an attendeeId from event B (IDOR). + @Trace('event-attendee.findEventAttendeeScopedToEvent') + async findEventAttendeeScopedToEvent(attendeeId: number, eventId: number) { + await this.getTenantSpecificEventRepository(); + const attendee = await this.eventAttendeesRepository.findOne({ + where: { id: attendeeId, event: { id: eventId } }, + }); + + if (!attendee) { + throw new NotFoundException( + `Attendee with ID ${attendeeId} not found for this event`, + ); + } + + return attendee; + } + @Trace('event-attendee.getAttendeeById') async getAttendeeById(attendeeId: number) { await this.getTenantSpecificEventRepository(); diff --git a/src/event/event.controller.ts b/src/event/event.controller.ts index 87a344e3..b065cf88 100644 --- a/src/event/event.controller.ts +++ b/src/event/event.controller.ts @@ -30,7 +30,6 @@ import { AuthUser } from '../core/decorators/auth-user.decorator'; import { User } from '../user/domain/user'; import { PaginationDto } from '../utils/dto/pagination.dto'; import { UserEntity } from '../user/infrastructure/persistence/relational/entities/user.entity'; -import { EventAttendeeService } from '../event-attendee/event-attendee.service'; import { CreateEventAttendeeDto } from '../event-attendee/dto/create-eventAttendee.dto'; import { UpdateEventAttendeeDto } from '../event-attendee/dto/update-eventAttendee.dto'; import { QueryEventAttendeeDto } from '../event-attendee/dto/query-eventAttendee.dto'; @@ -68,7 +67,6 @@ export class EventController { private readonly eventManagementService: EventManagementService, private readonly eventQueryService: EventQueryService, private readonly eventRecommendationService: EventRecommendationService, - private readonly eventAttendeeService: EventAttendeeService, private readonly iCalendarService: ICalendarService, private readonly eventSeriesOccurrenceService: EventSeriesOccurrenceService, private readonly eventMailService: EventMailService, @@ -273,10 +271,10 @@ export class EventController { ); } - // @Permissions({ - // context: 'event', - // permissions: [EventAttendeePermission.ManageAttendees], - // }) + @Permissions({ + context: 'event', + permissions: [EventAttendeePermission.ManageAttendees], + }) @UseGuards(JWTAuthGuard, PermissionsGuard) @Patch(':slug/attendees/:attendeeId') @ApiOperation({ summary: 'Update event attendee' }) @@ -292,10 +290,10 @@ export class EventController { ); } - // @Permissions({ - // context: 'event', - // permissions: [EventAttendeePermission.ManageAttendees], - // }) + @Permissions({ + context: 'event', + permissions: [EventAttendeePermission.ManageAttendees], + }) @UseGuards(JWTAuthGuard, PermissionsGuard) @Delete(':slug/attendees/:attendeeId') @ApiOperation({ summary: 'Delete event attendee' }) @@ -303,7 +301,7 @@ export class EventController { @Param('slug') slug: string, @Param('attendeeId') attendeeId: number, ) { - return this.eventAttendeeService.deleteEventAttendee(attendeeId); + return this.eventManagementService.deleteEventAttendee(slug, attendeeId); } @Public() diff --git a/src/event/services/event-management.service.ts b/src/event/services/event-management.service.ts index 1de4658b..87a9eba7 100644 --- a/src/event/services/event-management.service.ts +++ b/src/event/services/event-management.service.ts @@ -2145,7 +2145,13 @@ export class EventManagementService { ) { await this.initializeRepository(); - await this.eventRepository.findOneOrFail({ where: { slug } }); + const event = await this.eventRepository.findOneOrFail({ where: { slug } }); + + // Ensure the attendee actually belongs to this event before mutating it. + await this.eventAttendeeService.findEventAttendeeScopedToEvent( + attendeeId, + event.id, + ); await this.eventAttendeeService.updateEventAttendee( attendeeId, @@ -2157,6 +2163,21 @@ export class EventManagementService { return await this.eventAttendeeService.showEventAttendee(attendeeId); } + @Trace('event-management.deleteEventAttendee') + async deleteEventAttendee(slug: string, attendeeId: number) { + await this.initializeRepository(); + + const event = await this.eventRepository.findOneOrFail({ where: { slug } }); + + // Ensure the attendee actually belongs to this event before deleting it. + await this.eventAttendeeService.findEventAttendeeScopedToEvent( + attendeeId, + event.id, + ); + + return await this.eventAttendeeService.deleteEventAttendee(attendeeId); + } + async delete(id: number): Promise { const event = await this.eventRepository.findOne({ where: { id }, diff --git a/test/event-attendees/attendee-authorization.e2e-spec.ts b/test/event-attendees/attendee-authorization.e2e-spec.ts new file mode 100644 index 00000000..181124d0 --- /dev/null +++ b/test/event-attendees/attendee-authorization.e2e-spec.ts @@ -0,0 +1,183 @@ +import request from 'supertest'; +import { + TESTING_APP_URL, + TESTING_ADMIN_EMAIL, + TESTING_ADMIN_PASSWORD, + TESTING_USER_EMAIL, + TESTING_USER_PASSWORD, + TESTING_TENANT_ID, +} from '../utils/constants'; +import { getAuthToken, createEvent } from '../utils/functions'; +import { EventType } from '../../src/core/constants/constant'; + +// Regression coverage for the attendee-management IDOR: +// the DELETE/PATCH `/events/:slug/attendees/:attendeeId` routes once had their +// `@Permissions(ManageAttendees)` decorator commented out, so any authenticated +// user could remove or mutate any other attendee by id. These tests pin the +// authorization contract so it can't silently regress again. +jest.setTimeout(60000); + +describe('Event attendee management authorization (e2e)', () => { + let organizerToken: string; // creates/owns the events -> ManageAttendees via owner + let attackerToken: string; // a different, non-organizer authenticated user + + const createdEventSlugs: string[] = []; + + function eventData(overrides = {}) { + return { + name: 'Authz Test Event', + slug: `authz-event-${Date.now()}-${Math.floor(Math.random() * 1e6)}`, + description: 'Event for attendee authorization tests', + type: EventType.Hybrid, + location: 'Test Location', + locationOnline: 'https://test-online-location.com', + maxAttendees: 100, + categories: [1], + lat: 40.7128, + lon: -74.006, + status: 'published', + timeZone: 'UTC', + ...overrides, + }; + } + + async function newEvent(token: string, overrides = {}) { + const ev = await createEvent(TESTING_APP_URL, token, eventData(overrides)); + createdEventSlugs.push(ev.slug); + return ev; + } + + async function attend(token: string, slug: string) { + const res = await request(TESTING_APP_URL) + .post(`/api/events/${slug}/attend`) + .set('Authorization', `Bearer ${token}`) + .set('x-tenant-id', TESTING_TENANT_ID) + .send({}); + expect(res.status).toBe(201); + return res.body; // includes attendee `id` + } + + function deleteAttendee(token: string, slug: string, attendeeId: number) { + return request(TESTING_APP_URL) + .delete(`/api/events/${slug}/attendees/${attendeeId}`) + .set('Authorization', `Bearer ${token}`) + .set('x-tenant-id', TESTING_TENANT_ID); + } + + function updateAttendee( + token: string, + slug: string, + attendeeId: number, + body: Record, + ) { + return request(TESTING_APP_URL) + .patch(`/api/events/${slug}/attendees/${attendeeId}`) + .set('Authorization', `Bearer ${token}`) + .set('x-tenant-id', TESTING_TENANT_ID) + .send(body); + } + + function listAttendees(token: string, slug: string) { + return request(TESTING_APP_URL) + .get(`/api/events/${slug}/attendees`) + .set('Authorization', `Bearer ${token}`) + .set('x-tenant-id', TESTING_TENANT_ID); + } + + beforeAll(async () => { + organizerToken = await getAuthToken( + TESTING_APP_URL, + TESTING_ADMIN_EMAIL, + TESTING_ADMIN_PASSWORD, + ); + attackerToken = await getAuthToken( + TESTING_APP_URL, + TESTING_USER_EMAIL, + TESTING_USER_PASSWORD, + ); + }); + + afterAll(async () => { + for (const slug of createdEventSlugs) { + await request(TESTING_APP_URL) + .delete(`/api/events/${slug}`) + .set('Authorization', `Bearer ${organizerToken}`) + .set('x-tenant-id', TESTING_TENANT_ID); + } + }); + + it('should forbid a non-organizer attendee from deleting another attendee (IDOR)', async () => { + const event = await newEvent(organizerToken); + const organizerAttendee = await attend(organizerToken, event.slug); + // attacker is a legitimate attendee, but has no management role + await attend(attackerToken, event.slug); + + const res = await deleteAttendee( + attackerToken, + event.slug, + organizerAttendee.id, + ); + + expect(res.status).toBe(403); + + // organizer's attendance must still be intact + const list = await listAttendees(organizerToken, event.slug); + const stillThere = list.body.data.find( + (a: { id: number }) => a.id === organizerAttendee.id, + ); + expect(stillThere).toBeDefined(); + }); + + it('should forbid a non-organizer attendee from updating another attendee (IDOR)', async () => { + const event = await newEvent(organizerToken); + const organizerAttendee = await attend(organizerToken, event.slug); + await attend(attackerToken, event.slug); + + const res = await updateAttendee( + attackerToken, + event.slug, + organizerAttendee.id, + { status: 'cancelled' }, + ); + + expect(res.status).toBe(403); + }); + + it('should allow the event owner to delete an attendee (positive control)', async () => { + const event = await newEvent(organizerToken); + const attackerAttendee = await attend(attackerToken, event.slug); + + const res = await deleteAttendee( + organizerToken, + event.slug, + attackerAttendee.id, + ); + + expect([200, 204]).toContain(res.status); + }); + + it('should not delete an attendee that belongs to a different event (cross-event IDOR)', async () => { + // Organizer owns event A (so the permission guard, scoped to A, will pass). + const eventA = await newEvent(organizerToken); + // Event B is owned by a different user; attacker attends B there. + const eventB = await newEvent(attackerToken); + const attendeeInB = await attend(attackerToken, eventB.slug); + + // Authorized for A, but targeting an attendee id that lives in B. + const res = await deleteAttendee( + organizerToken, + eventA.slug, + attendeeInB.id, + ); + + // Must NOT succeed: organizer has no authority over event B's roster. + expect([403, 404]).toContain(res.status); + + // Confirm the attendee in B was not removed. + const list = await listAttendees(attackerToken, eventB.slug); + const stillThere = list.body.data.find( + (a: { id: number }) => a.id === attendeeInB.id, + ); + expect(stillThere).toBeDefined(); + }); +}); From e5b625c6f39f9f14b11f2651b9e56241edb6404d Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Thu, 28 May 2026 14:20:44 -0400 Subject: [PATCH 2/3] test(event): add group-admin positive control to attendee authz e2e Adds a 5th scenario: a group admin who did NOT create the event can still manage attendees on group events (the CRMC multi-admin pattern). Ensures the IDOR fix doesn't regress group-level MANAGE_EVENTS grants. --- .../attendee-authorization.e2e-spec.ts | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/test/event-attendees/attendee-authorization.e2e-spec.ts b/test/event-attendees/attendee-authorization.e2e-spec.ts index 181124d0..0a28b70f 100644 --- a/test/event-attendees/attendee-authorization.e2e-spec.ts +++ b/test/event-attendees/attendee-authorization.e2e-spec.ts @@ -7,8 +7,16 @@ import { TESTING_USER_PASSWORD, TESTING_TENANT_ID, } from '../utils/constants'; -import { getAuthToken, createEvent } from '../utils/functions'; -import { EventType } from '../../src/core/constants/constant'; +import { + getAuthToken, + createEvent, + createGroup, + joinGroup, + approveMember, + getGroupMembers, + updateGroupMemberRole, +} from '../utils/functions'; +import { EventType, GroupStatus } from '../../src/core/constants/constant'; // Regression coverage for the attendee-management IDOR: // the DELETE/PATCH `/events/:slug/attendees/:attendeeId` routes once had their @@ -180,4 +188,60 @@ describe('Event attendee management authorization (e2e)', () => { ); expect(stillThere).toBeDefined(); }); + + it('should allow a group admin to delete an attendee on a group event they did not create (CRMC scenario)', async () => { + // Organizer creates a group and an event under it. + const group = await createGroup(TESTING_APP_URL, organizerToken, { + name: `IDOR Test Group ${Date.now()}`, + description: 'Group for attendee authz test', + status: GroupStatus.Published, + }); + const groupEvent = await newEvent(organizerToken, { group: group.id }); + + // Second user joins the group, gets approved and promoted to admin. + const membership = await joinGroup( + TESTING_APP_URL, + TESTING_TENANT_ID, + group.slug, + attackerToken, + ); + await approveMember( + TESTING_APP_URL, + TESTING_TENANT_ID, + group.slug, + membership.id, + organizerToken, + ); + const members = await getGroupMembers( + TESTING_APP_URL, + TESTING_TENANT_ID, + group.slug, + organizerToken, + ); + const memberRecord = members.find( + (m: { id: number }) => m.id === membership.id, + ); + await updateGroupMemberRole( + TESTING_APP_URL, + TESTING_TENANT_ID, + group.slug, + memberRecord.id, + 'admin', + organizerToken, + ); + + // Organizer attends the group event. + const organizerAttendee = await attend(organizerToken, groupEvent.slug); + + // Group admin (not event creator) deletes the organizer's attendance. + const res = await deleteAttendee( + attackerToken, + groupEvent.slug, + organizerAttendee.id, + ); + + // Should succeed: group admins have MANAGE_EVENTS which grants + // event-scoped operations including attendee management. + expect([200, 204]).toContain(res.status); + }); }); From d3d0f269dd3a300beabd8bb90a9429930705ec2d Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Thu, 28 May 2026 14:45:57 -0400 Subject: [PATCH 3/3] fix(test): simplify CRMC e2e - use membership.id directly Skip getGroupMembers call (403 in CI) and pass the joinGroup membership ID directly to updateGroupMemberRole. Verified locally: CRMC scenario passes, IDOR tests correctly fail against unfixed main. --- .../attendee-authorization.e2e-spec.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/test/event-attendees/attendee-authorization.e2e-spec.ts b/test/event-attendees/attendee-authorization.e2e-spec.ts index 0a28b70f..2950e37c 100644 --- a/test/event-attendees/attendee-authorization.e2e-spec.ts +++ b/test/event-attendees/attendee-authorization.e2e-spec.ts @@ -13,7 +13,6 @@ import { createGroup, joinGroup, approveMember, - getGroupMembers, updateGroupMemberRole, } from '../utils/functions'; import { EventType, GroupStatus } from '../../src/core/constants/constant'; @@ -212,20 +211,11 @@ describe('Event attendee management authorization (e2e)', () => { membership.id, organizerToken, ); - const members = await getGroupMembers( - TESTING_APP_URL, - TESTING_TENANT_ID, - group.slug, - organizerToken, - ); - const memberRecord = members.find( - (m: { id: number }) => m.id === membership.id, - ); await updateGroupMemberRole( TESTING_APP_URL, TESTING_TENANT_ID, group.slug, - memberRecord.id, + membership.id, 'admin', organizerToken, );