Skip to content

Commit

Permalink
prevent double-spending of user-uploaded planning areas
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hotzevzl committed Nov 16, 2023
1 parent 557b277 commit 3f9216c
Showing 1 changed file with 44 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<SetProjectGridFromShapefile>
{
implements IInferredCommandHandler<SetProjectGridFromShapefile> {
constructor(
@InjectRepository(Project) private readonly projects: Repository<Project>,
private readonly events: ApiEventsService,
Expand All @@ -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 });
Expand Down Expand Up @@ -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<boolean> {
const planningArea = await transactionalEntityManager
.getRepository(PlanningArea)
.findOneBy({ id: planningAreaId, projectId: planningAreaId });
return !isNil(planningArea);
}
}

0 comments on commit 3f9216c

Please sign in to comment.