From 00dfdc0f0e6491b6660c6bf22976534cf2041398 Mon Sep 17 00:00:00 2001
From: Slavcho Ivanov <s.ivanov@intelligencenode.com>
Date: Wed, 6 Sep 2023 00:10:57 +0530
Subject: [PATCH] Fix a bug with the access to the expenses list of a campaign.
 (#545)

We used to honour only the coordinator, but the organizer is as important.
---
 .../campaign-file.controller.spec.ts          | 11 +++-----
 .../campaign-file/campaign-file.controller.ts | 11 +++-----
 apps/api/src/campaign/campaign.service.ts     | 26 ++++++++++++-------
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/apps/api/src/campaign-file/campaign-file.controller.spec.ts b/apps/api/src/campaign-file/campaign-file.controller.spec.ts
index 1ca210948..db17cabcd 100644
--- a/apps/api/src/campaign-file/campaign-file.controller.spec.ts
+++ b/apps/api/src/campaign-file/campaign-file.controller.spec.ts
@@ -48,7 +48,7 @@ describe('CampaignFileController', () => {
         ConfigService,
         {
           provide: CampaignService,
-          useValue: { getCampaignByIdAndCoordinatorId: jest.fn(() => null) },
+          useValue: { verifyCampaignOwner: jest.fn(() => null) },
         },
         VaultService,
         CampaignNewsService,
@@ -92,7 +92,7 @@ describe('CampaignFileController', () => {
     ).toEqual([fileId, fileId])
 
     expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
-    expect(campaignService.getCampaignByIdAndCoordinatorId).not.toHaveBeenCalled()
+    expect(campaignService.verifyCampaignOwner).not.toHaveBeenCalled()
     expect(campaignFileService.create).toHaveBeenCalledTimes(2)
   })
 
@@ -102,16 +102,13 @@ describe('CampaignFileController', () => {
     await expect(controller.create(campaignId, { roles: [] }, [], userMock)).rejects.toThrowError()
 
     expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
-    expect(campaignService.getCampaignByIdAndCoordinatorId).not.toHaveBeenCalled()
+    expect(campaignService.verifyCampaignOwner).not.toHaveBeenCalled()
   })
 
   it('should throw an error for user not owning updated campaign', async () => {
     await expect(controller.create(campaignId, { roles: [] }, [], userMock)).rejects.toThrowError()
 
     expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
-    expect(campaignService.getCampaignByIdAndCoordinatorId).toHaveBeenCalledWith(
-      campaignId,
-      personIdMock,
-    )
+    expect(campaignService.verifyCampaignOwner).toHaveBeenCalledWith(campaignId, personIdMock)
   })
 })
diff --git a/apps/api/src/campaign-file/campaign-file.controller.ts b/apps/api/src/campaign-file/campaign-file.controller.ts
index 458f183fe..dbd136931 100644
--- a/apps/api/src/campaign-file/campaign-file.controller.ts
+++ b/apps/api/src/campaign-file/campaign-file.controller.ts
@@ -23,7 +23,7 @@ import { FilesRoleDto } from './dto/files-role.dto'
 import { CampaignFileService } from './campaign-file.service'
 import { CampaignService } from '../campaign/campaign.service'
 import { KeycloakTokenParsed, isAdmin } from '../auth/keycloak'
-import { ApiTags } from '@nestjs/swagger';
+import { ApiTags } from '@nestjs/swagger'
 import { CampaignFileRole } from '@prisma/client'
 
 @ApiTags('campaign-file')
@@ -51,10 +51,7 @@ export class CampaignFileController {
     }
 
     if (!isAdmin(user)) {
-      const campaign = await this.campaignService.getCampaignByIdAndCoordinatorId(
-        campaignId,
-        person.id,
-      )
+      const campaign = await this.campaignService.verifyCampaignOwner(campaignId, person.id)
       if (!campaign) {
         throw new NotFoundException(
           'User ' + user.name + 'is not admin or coordinator of campaign with id: ' + campaignId,
@@ -88,8 +85,8 @@ export class CampaignFileController {
       'Content-Type': file.mimetype,
       'Content-Disposition': 'attachment; filename="' + file.filename + '"',
       'Cache-Control': file.mimetype.startsWith('image/')
-                        ? 'public, s-maxage=15552000, stale-while-revalidate=15552000, immutable' 
-                        : 'no-store'
+        ? 'public, s-maxage=15552000, stale-while-revalidate=15552000, immutable'
+        : 'no-store',
     })
 
     return new StreamableFile(file.stream)
diff --git a/apps/api/src/campaign/campaign.service.ts b/apps/api/src/campaign/campaign.service.ts
index cdc82ede5..e46c60d62 100644
--- a/apps/api/src/campaign/campaign.service.ts
+++ b/apps/api/src/campaign/campaign.service.ts
@@ -235,15 +235,23 @@ export class CampaignService {
     return campaigns
   }
 
-  async getCampaignByIdAndCoordinatorId(
-    campaignId: string,
-    coordinatorId: string,
-  ): Promise<Campaign | null> {
-    const campaign = await this.prisma.campaign.findFirst({
-      where: { id: campaignId, coordinator: { personId: coordinatorId } },
-      include: { coordinator: true },
+  // Check if the campaign exists by coordinator or organizer
+  async verifyCampaignOwner(campaignId: string, personId: string): Promise<Campaign | null> {
+    const campaignByCoordinator = await this.prisma.campaign.findFirst({
+      where: { id: campaignId, coordinator: { personId } },
+      include: { coordinator: true, organizer: true },
     })
-    return campaign
+
+    if (campaignByCoordinator !== null) {
+      return campaignByCoordinator
+    }
+
+    const campaignByOrganizer = await this.prisma.campaign.findFirst({
+      where: { id: campaignId, organizer: { personId } },
+      include: { coordinator: true, organizer: true },
+    })
+
+    return campaignByOrganizer
   }
 
   async getCampaignByIdWithPersonIds(id: string) {
@@ -1070,7 +1078,7 @@ export class CampaignService {
       throw new UnauthorizedException()
     }
 
-    const campaign = await this.getCampaignByIdAndCoordinatorId(campaignId, person.id)
+    const campaign = await this.verifyCampaignOwner(campaignId, person.id)
     if (!campaign) {
       throw new UnauthorizedException()
     }