From c8135ec300f24d28919fdda7c261b0dd5ce34d38 Mon Sep 17 00:00:00 2001 From: Hugo Gresse Date: Sat, 12 Oct 2024 15:25:01 +0200 Subject: [PATCH] Fix: Prevent wrong field name/types to be added in Firestore through the API (#144) * Prevent wrong field name/types to be added in Firestore * minor CI build changes --- .github/workflows/build.yml | 7 +- .../convertBodySessionToSession.ts | 82 +++++ .../overwriteSpeakerSessions.test.ts | 309 +++++++++++++++--- .../overwriteSpeakerSessions.ts | 9 +- .../verifyOverwriteData.ts | 4 +- functions/src/types.ts | 15 +- src/services/hooks/useSessions.ts | 11 +- 7 files changed, 368 insertions(+), 69 deletions(-) create mode 100644 functions/src/api/routes/overwriteSpeakerSessions/convertBodySessionToSession.ts diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e4fac22..695d243 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,10 +1,10 @@ -name: Build project +name: Build & test front & functions project on: [push, pull_request] jobs: build: - name: Build front project + name: Build & test front & functions project runs-on: ubuntu-latest steps: @@ -14,10 +14,11 @@ jobs: - run: bun install --frozen-lockfile - run: bun run build - run: bun run tscheck + - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version-file: .nvmrc + node-version-file: ./functions/.nvmrc - name: Build serverless functions working-directory: ./functions diff --git a/functions/src/api/routes/overwriteSpeakerSessions/convertBodySessionToSession.ts b/functions/src/api/routes/overwriteSpeakerSessions/convertBodySessionToSession.ts new file mode 100644 index 0000000..c05e3d3 --- /dev/null +++ b/functions/src/api/routes/overwriteSpeakerSessions/convertBodySessionToSession.ts @@ -0,0 +1,82 @@ +import { Session, Event } from '../../../types' +import { OverwriteSpeakerSessionsType } from './overwriteSpeakerSessions' + +type ElementType = T extends (infer U)[] ? U : never + +export const convertBodySessionToSession = ( + session: ElementType, + event: Event +): Session => { + const realSession: Partial = { + id: session.id, + } + + if (session.dateStart && session.dateEnd) { + realSession.dates = { + start: session.dateStart, + end: session.dateEnd, + } + } + + // TypeScript doesn't support type at compile time, and transformers is not working with vite. So we need to do this shit + if (session.title) { + realSession.title = session.title + } + if (session.abstract) { + realSession.abstract = session.abstract + } + if (session.durationMinutes) { + realSession.durationMinutes = session.durationMinutes + } + if (session.hideTrackTitle) { + realSession.hideTrackTitle = session.hideTrackTitle + } + if (session.showInFeedback) { + realSession.showInFeedback = session.showInFeedback + } + if (session.imageUrl) { + realSession.imageUrl = session.imageUrl + } + if (session.videoLink) { + realSession.videoLink = session.videoLink + } + if (session.presentationLink) { + realSession.presentationLink = session.presentationLink + } + if (session.language) { + realSession.language = session.language + } + if (session.level) { + realSession.level = session.level + } + if (session.note) { + realSession.note = session.note + } + if (session.teasingHidden) { + realSession.teasingHidden = session.teasingHidden + } + + if (session.trackId || session.trackName) { + realSession.trackId = (event.tracks || []).find( + (track) => track.name === session.trackId || track.name === session.trackName + )?.id + } + + if (session.categoryId || session.categoryName) { + realSession.category = (event.categories || []).find( + (cat) => cat.name === session.categoryId || cat.name === session.categoryName + )?.id + } + + if (session.formatId || session.formatName) { + realSession.format = (event.formats || []).find( + (format) => format.name === session.formatId || format.name === session.formatName + )?.id + } + + if (session.speakerIds) { + realSession.speakers = session.speakerIds + } + + return realSession as Session +} diff --git a/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.test.ts b/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.test.ts index e2bd507..ac4aaab 100644 --- a/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.test.ts +++ b/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.test.ts @@ -145,21 +145,49 @@ describe('overwriteSpeakerSessions', () => { return Promise.resolve({}) }) + const baseEvent = Promise.resolve({ + data: () => + ({ + id: eventId, + name: 'eventTestfull', + apiKey: 'xxx', + } as Partial), + exists: true, + }) + // First call, get the somewhat empty event + // Second call, get the updated event + const getEventSpy = vi.fn(() => baseEvent) + + getEventSpy + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return Promise.resolve({ + data: () => + ({ + id: eventId, + apiKey: 'xxx', + tracks: [ + { + id: 'newTrackId', + name: 'newTrackName', + }, + ], + } as Partial), + exists: true, + }) + }) + vi.spyOn(fastify.firebase, 'firestore').mockImplementation(() => { return getMockedFirestore( {}, { doc: vi.fn(() => ({ - get: vi.fn(() => - Promise.resolve({ - data: () => - ({ - id: eventId, - apiKey: 'xxx', - } as Partial), - exists: true, - }) - ), + get: getEventSpy, update: updateSpy, })), } @@ -199,7 +227,6 @@ describe('overwriteSpeakerSessions', () => { id: 'sessionId', title: 'sessionTitle', trackId: 'newTrackId', - trackName: 'newTrackName', updatedAt: expect.any(Object), }) }) @@ -258,7 +285,6 @@ describe('overwriteSpeakerSessions', () => { id: 'sessionId', title: 'sessionTitle', trackId: 'oldTrackId', - trackName: 'oldTrackName', updatedAt: expect.any(Object), }) }) @@ -269,21 +295,50 @@ describe('overwriteSpeakerSessions', () => { return Promise.resolve({}) }) + const baseEvent = Promise.resolve({ + data: () => + ({ + id: eventId, + name: 'eventTestfull', + apiKey: 'xxx', + } as Partial), + exists: true, + }) + // First call, get the somewhat empty event + // Second call, get the updated event + const getEventSpy = vi.fn(() => baseEvent) + + getEventSpy + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return Promise.resolve({ + data: () => + ({ + id: eventId, + apiKey: 'xxx', + formats: [ + { + id: 'newFormatId', + name: 'newFormatName', + durationMinutes: 20, + }, + ], + } as Partial), + exists: true, + }) + }) + vi.spyOn(fastify.firebase, 'firestore').mockImplementation(() => { return getMockedFirestore( {}, { doc: vi.fn(() => ({ - get: vi.fn(() => - Promise.resolve({ - data: () => - ({ - id: eventId, - apiKey: 'xxx', - } as Partial), - exists: true, - }) - ), + get: getEventSpy, update: updateSpy, })), } @@ -323,8 +378,7 @@ describe('overwriteSpeakerSessions', () => { expect(updateSpy).toHaveBeenNthCalledWith(2, { id: 'sessionId', title: 'sessionTitle', - formatId: 'newFormatId', - formatName: 'newFormatName', + format: 'newFormatId', updatedAt: expect.any(Object), }) }) @@ -382,13 +436,12 @@ describe('overwriteSpeakerSessions', () => { expect(updateSpy).toHaveBeenNthCalledWith(1, { id: 'sessionId', title: 'sessionTitle', - formatId: 'oldFormatId', - formatName: 'oldFormatName', + format: 'oldFormatId', updatedAt: expect.any(Object), }) }) // Category - test('should create the session with a new category', async () => { + test('should create the session with category null as the getEvent returned no category', async () => { const updateSpy = vi.fn(() => { return Promise.resolve({}) }) @@ -448,9 +501,100 @@ describe('overwriteSpeakerSessions', () => { expect(updateSpy).toHaveBeenNthCalledWith(2, { id: 'sessionId', title: 'sessionTitle', - categoryId: 'newCategoryId', - categoryName: 'newCategoryName', - categoryColor: '#124590', + category: undefined, + updatedAt: expect.any(Object), + }) + }) + test('should create the session with a new category', async () => { + const updateSpy = vi.fn(() => { + return Promise.resolve({}) + }) + + const baseEvent = Promise.resolve({ + data: () => + ({ + id: eventId, + name: 'eventTestfull', + apiKey: 'xxx', + } as Partial), + exists: true, + }) + // First call, get the somewhat empty event + // Second call, get the updated event + const getEventSpy = vi.fn(() => baseEvent) + + getEventSpy + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return Promise.resolve({ + data: () => + ({ + id: eventId, + apiKey: 'xxx', + categories: [ + { + id: 'newCategoryId', + name: 'newCategoryName', + color: '#124590', + }, + ], + } as Partial), + exists: true, + }) + }) + + vi.spyOn(fastify.firebase, 'firestore').mockImplementation(() => { + return getMockedFirestore( + {}, + { + doc: vi.fn(() => ({ + get: getEventSpy, + update: updateSpy, + })), + } + ) + }) + const res = await fastify.inject({ + method: 'post', + url: `/v1/${eventId}/overwriteSpeakerSponsors?apiKey=xxx`, + payload: { + sessions: [ + { + id: 'sessionId', + title: 'sessionTitle', + categoryId: 'newCategoryId', + categoryName: 'newCategoryName', + categoryColor: '#124590', + }, + ], + speakers: [], + }, + }) + expect(res.statusCode).to.equal(201) + expect(JSON.parse(res.body)).toMatchObject({ + success: true, + }) + expect(updateSpy).toHaveBeenCalledTimes(2) + expect(updateSpy).toHaveBeenNthCalledWith(1, { + categories: { + elements: [ + { + id: 'newCategoryId', + name: 'newCategoryName', + color: '#124590', + }, + ], + }, + }) + expect(updateSpy).toHaveBeenNthCalledWith(2, { + id: 'sessionId', + title: 'sessionTitle', + category: 'newCategoryId', updatedAt: expect.any(Object), }) }) @@ -508,8 +652,7 @@ describe('overwriteSpeakerSessions', () => { expect(updateSpy).toHaveBeenNthCalledWith(1, { id: 'sessionId', title: 'sessionTitle', - categoryId: 'oldCategoryId', - categoryName: 'oldCategoryName', + category: 'oldCategoryId', updatedAt: expect.any(Object), }) }) @@ -830,22 +973,87 @@ describe('overwriteSpeakerSessions', () => { const setSpy = vi.fn(() => { return Promise.resolve({}) }) + // First call, get the somewhat empty event + // Second call, get the updated event + const getEventSpy = vi.fn(() => { + return Promise.resolve({ + data: () => + ({ + id: eventId, + apiKey: 'xxx', + shouldntBe: 'here', + } as Partial), + exists: true, + }) + }) + + const baseEvent = Promise.resolve({ + data: () => + ({ + id: eventId, + name: 'eventTestfull', + apiKey: 'xxx', + } as Partial), + exists: true, + }) + getEventSpy + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return baseEvent + }) + .mockImplementationOnce(() => { + return Promise.resolve({ + data: () => + ({ + id: eventId, + apiKey: 'xxx', + formats: [ + { + id: 'newFormatId', + name: 'newFormatName', + durationMinutes: 20, + }, + { + id: 'newFormatId2', + name: 'newFormatName2', + durationMinutes: 20, + }, + ], + categories: [ + { + id: 'newCategoryId', + name: 'newCategoryName', + color: '#053aa7', + }, + { + id: 'newCategoryId2', + name: 'newCategoryName2', + color: '#123123', + }, + ], + tracks: [ + { + id: 'newTrackId', + name: 'newTrackName', + }, + { + id: 'newTrackId2', + name: 'newTrackName2', + }, + ], + } as Partial), + exists: true, + }) + }) vi.spyOn(fastify.firebase, 'firestore').mockImplementation(() => { return getMockedFirestore( {}, { doc: vi.fn(() => ({ - get: vi.fn(() => - Promise.resolve({ - data: () => - ({ - id: eventId, - apiKey: 'xxx', - } as Partial), - exists: true, - }) - ), + get: getEventSpy, set: setSpy, update: updateSpy, })), @@ -910,6 +1118,7 @@ describe('overwriteSpeakerSessions', () => { success: true, }) expect(res.statusCode).to.equal(201) + expect(getEventSpy).toHaveBeenCalledTimes(13) expect(updateSpy).toHaveBeenCalledTimes(8) expect(updateSpy).toHaveBeenNthCalledWith(1, { tracks: { @@ -976,15 +1185,19 @@ describe('overwriteSpeakerSessions', () => { }, }) expect(updateSpy).toHaveBeenNthCalledWith(7, { - categoryName: 'newCategoryName', - categoryId: 'newCategoryId', - categoryColor: '#053aa7', - formatId: 'newFormatId', - formatName: 'newFormatName', + category: 'newCategoryId', + format: 'newFormatId', id: 'sessionId', title: 'sessionTitle', trackId: 'newTrackId', - trackName: 'newTrackName', + updatedAt: expect.any(Object), + }) + expect(updateSpy).toHaveBeenNthCalledWith(8, { + category: 'newCategoryId2', + format: 'newFormatId2', + id: 'sessionId2', + title: 'sessionTitle2', + trackId: 'newTrackId2', updatedAt: expect.any(Object), }) expect(setSpy).toHaveBeenCalledTimes(2) diff --git a/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.ts b/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.ts index daec79a..9d2a4ee 100644 --- a/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.ts +++ b/functions/src/api/routes/overwriteSpeakerSessions/overwriteSpeakerSessions.ts @@ -5,6 +5,7 @@ import { verifyOverwriteData } from './verifyOverwriteData' import { EventDao } from '../../dao/eventDao' import { SessionDao } from '../../dao/sessionDao' import { SpeakerDao } from '../../dao/speakerDao' +import { convertBodySessionToSession } from './convertBodySessionToSession' const MAX_STRING_LENGTH = 10000 @@ -258,10 +259,16 @@ export const overwriteSpeakerSessions = (fastify: FastifyInstance, options: any, } } + const updatedEvent = await EventDao.getEvent(fastify.firebase, eventId) + // Sessions for (const session of request.body.sessions) { try { - await SessionDao.updateOrCreateSession(fastify.firebase, eventId, session) + await SessionDao.updateOrCreateSession( + fastify.firebase, + eventId, + convertBodySessionToSession(session, updatedEvent) + ) } catch (error) { console.error('error creating session', error) // @ts-ignore diff --git a/functions/src/api/routes/overwriteSpeakerSessions/verifyOverwriteData.ts b/functions/src/api/routes/overwriteSpeakerSessions/verifyOverwriteData.ts index b0cadce..e2a66b3 100644 --- a/functions/src/api/routes/overwriteSpeakerSessions/verifyOverwriteData.ts +++ b/functions/src/api/routes/overwriteSpeakerSessions/verifyOverwriteData.ts @@ -115,8 +115,8 @@ const ensureIdOrNameFit = ( } } - const existWithName = event[type].find((t) => t.name === name) - const existWithId = event[type].find((t) => t.id === id) + const existWithName = (event[type] || []).find((t) => t.name === name) + const existWithId = (event[type] || []).find((t) => t.id === id) if (!existWithName && !existWithId) { if (!id) { diff --git a/functions/src/types.ts b/functions/src/types.ts index 4629d58..40fb834 100644 --- a/functions/src/types.ts +++ b/functions/src/types.ts @@ -1,7 +1,3 @@ -import firebase from 'firebase-admin' - -type Timestamp = firebase.firestore.Timestamp - export interface Track { id: string name: string @@ -25,8 +21,8 @@ export interface Format { } export interface DateType { - start: Date | null - end: Date | null + start: Date | string | null + end: Date | string | null } export interface Social { @@ -57,7 +53,7 @@ export interface Session { conferenceHallId: string | null title: string abstract: string | null - dates: Timestamp | null + dates: DateType | null durationMinutes: number speakers: string[] trackId: string | null @@ -66,7 +62,6 @@ export interface Session { presentationLink: string | null videoLink: string | null imageUrl: string | null - tags: string[] format: string | null category: string | null image: string | null @@ -113,8 +108,8 @@ export interface Event { members: string[] conferenceHallId: string | null dates: DateType - formats: Format[] - categories: Category[] + formats: Format[] | null + categories: Category[] | null tracks: Track[] webhooks: Webhooks[] createdAt: Date diff --git a/src/services/hooks/useSessions.ts b/src/services/hooks/useSessions.ts index 7ef118f..6ad1192 100644 --- a/src/services/hooks/useSessions.ts +++ b/src/services/hooks/useSessions.ts @@ -6,11 +6,12 @@ import { useFirestoreCollection, UseQueryResult } from './firestoreQueryHook' const hydrateSession = (event: Event, sp: UseQueryResult, data: Session[]) => { return data.map((session: Session) => ({ ...session, - speakersData: sp.data - ? (session.speakers - .map((speakerId) => (sp.data ? sp.data[speakerId] : undefined)) - .filter((s) => !!s) as Speaker[]) - : undefined, + speakersData: + sp.data && session.speakers + ? (session.speakers + .map((speakerId) => (sp.data ? sp.data[speakerId] : undefined)) + .filter((s) => !!s) as Speaker[]) + : undefined, formatText: getSessionFormatText(event, session), categoryObject: event.categories ? event.categories.find((c) => session.category === c.id) : null, }))