From 3f9216c0504b18ac56c0e39f43c28af3ae481adb Mon Sep 17 00:00:00 2001 From: andrea rota Date: Tue, 17 Oct 2023 13:41:03 +0100 Subject: [PATCH] prevent double-spending of user-uploaded planning areas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When setting project grid from shapefile, the operation should fail if the projectId of the planning area being updated does not match the planning area's own id. Explanation: When creating a project with a custom grid or planning area, the flow is: - first, user uploads a shapefile, app gets the id of the planning area that was created from it (if they upload a grid file, the planning area is backported from it, as a union of all the geometries) - when actually requesting to create a project, the app sends the `planningAreaId` obtained in the previous step The issue here is - since planning areas should not be linked to a random project, but we don’t yet have a `projectId` by the time the planning area is created, we set its `projectId` to the same value as its `id`. But then, when we actually create the project, in `SetProjectGridFromShapefileHandler` while we link the previously-updated planning area to the project, we don’t check if `projectId === id`, which ends up allowing to reuse the same planning area in case several requests to create a project with the same planning area id (for example, as a side effect of MRXNM-484 and sibling stories), which in practice ends up “stealing” planning areas created by earlier projects that use the same `planningAreaId` in the `POST` request payload. --- ...set-project-grid-from-shapefile.handler.ts | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/api/apps/api/src/modules/projects/planning-unit-grid/set-project-grid-from-shapefile.handler.ts b/api/apps/api/src/modules/projects/planning-unit-grid/set-project-grid-from-shapefile.handler.ts index d4eb2dc474..c8458c2579 100644 --- a/api/apps/api/src/modules/projects/planning-unit-grid/set-project-grid-from-shapefile.handler.ts +++ b/api/apps/api/src/modules/projects/planning-unit-grid/set-project-grid-from-shapefile.handler.ts @@ -17,11 +17,11 @@ import { v4 } from 'uuid'; import { SetProjectGridFromShapefile } from './set-project-grid-from-shapefile.command'; import { CostSurfacePuDataEntity } from '@marxan/cost-surfaces'; import { CostRangeService } from '@marxan-api/modules/scenarios/cost-range-service'; +import { isNil } from 'lodash'; @CommandHandler(SetProjectGridFromShapefile) export class SetProjectGridFromShapefileHandler - implements IInferredCommandHandler -{ + implements IInferredCommandHandler { constructor( @InjectRepository(Project) private readonly projects: Repository, private readonly events: ApiEventsService, @@ -48,6 +48,16 @@ export class SetProjectGridFromShapefileHandler }); await this.entityManager.transaction(async (manager) => { + if ( + !(await this.isPlanningAreaNotLinkedToAnyProjectYet( + planningAreaId, + manager, + )) + ) { + throw new Error( + `Planning area ${planningAreaId} is already linked to a project: no new project can be created using it as its own planning area.`, + ); + } await manager .getRepository(ProjectsPuEntity) .update({ planningAreaId }, { projectId }); @@ -90,4 +100,36 @@ export class SetProjectGridFromShapefileHandler this.eventBus.publish(new PlanningUnitSet(projectId)); } + + /** + * When a custom planning area has just been uploaded (either as a planning + * area shapefile or as a planning grid shapefile, from which we create the + * planning area itself), it is not linked to any projects to start with. + * + * Only once a project is created (in apidb), we then set up its grid and + * planning area from the previously-created planning area. + * + * However, planning areas where `project_id is null` are considered as + * dangling from the garbage collector, so we create custom planning areas + * initially with its `projectId` column set to the `id` of the planning area + * record itself: this can be used as a proxy of the planning area not being + * linked to any project. + * + * Once a project is created, we then update the planning area's `projectId` + * to match the actual `id` of the new project. To avoid "double spending" of + * a planning area (for example, if an API consumer issues more than one + * request to create a project, supplying the same `planningAreaId`, for + * whatever reason), we need to check that the planning area is not linked to + * any project yet (therefore, that `id = projectId`), before linking it to a + * project. + */ + private async isPlanningAreaNotLinkedToAnyProjectYet( + planningAreaId: string, + transactionalEntityManager: EntityManager, + ): Promise { + const planningArea = await transactionalEntityManager + .getRepository(PlanningArea) + .findOneBy({ id: planningAreaId, projectId: planningAreaId }); + return !isNil(planningArea); + } }