From 716e9ba6d1a6d046ed9444c8a3fbb106505a534f Mon Sep 17 00:00:00 2001 From: Morgan Ludtke Date: Mon, 4 Mar 2024 09:23:04 -0600 Subject: [PATCH 1/2] fix: add security around application list --- api/src/controllers/application.controller.ts | 7 +++++-- api/src/services/application.service.ts | 11 ++++++++++- .../permission-as-no-user.e2e-spec.ts | 4 ++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/api/src/controllers/application.controller.ts b/api/src/controllers/application.controller.ts index 6c1bf2a9b1..040a211487 100644 --- a/api/src/controllers/application.controller.ts +++ b/api/src/controllers/application.controller.ts @@ -73,8 +73,11 @@ export class ApplicationController { operationId: 'list', }) @ApiOkResponse({ type: PaginatedApplicationDto }) - async list(@Query() queryParams: ApplicationQueryParams) { - return await this.applicationService.list(queryParams); + async list( + @Request() req: ExpressRequest, + @Query() queryParams: ApplicationQueryParams, + ) { + return await this.applicationService.list(queryParams, req); } @Get(`mostRecentlyCreated`) diff --git a/api/src/services/application.service.ts b/api/src/services/application.service.ts index 6e6cfbe481..59342979ff 100644 --- a/api/src/services/application.service.ts +++ b/api/src/services/application.service.ts @@ -2,8 +2,10 @@ import { BadRequestException, Injectable, NotFoundException, + ForbiddenException, } from '@nestjs/common'; import crypto from 'crypto'; +import { Request as ExpressRequest } from 'express'; import { Prisma, YesNoEnum } from '@prisma/client'; import { PrismaService } from './prisma.service'; import { Application } from '../dtos/applications/application.dto'; @@ -84,7 +86,14 @@ export class ApplicationService { this set can either be paginated or not depending on the params it will return both the set of applications, and some meta information to help with pagination */ - async list(params: ApplicationQueryParams): Promise { + async list( + params: ApplicationQueryParams, + req: ExpressRequest, + ): Promise { + const user = mapTo(User, req['user']); + if (!user) { + throw new ForbiddenException(); + } const whereClause = this.buildWhereClause(params); const count = await this.prisma.applications.count({ diff --git a/api/test/integration/permission-tests/permission-as-no-user.e2e-spec.ts b/api/test/integration/permission-tests/permission-as-no-user.e2e-spec.ts index 0706b09855..8b0380cd20 100644 --- a/api/test/integration/permission-tests/permission-as-no-user.e2e-spec.ts +++ b/api/test/integration/permission-tests/permission-as-no-user.e2e-spec.ts @@ -185,11 +185,11 @@ describe('Testing Permissioning of endpoints as logged out user', () => { }); }); - it('should succeed for list endpoint', async () => { + it('should be forbidden for list endpoint', async () => { await request(app.getHttpServer()) .get(`/applications?`) .set('Cookie', cookies) - .expect(200); + .expect(403); }); it('should succeed for retrieve endpoint', async () => { From ba400f78f1706a80bbec688c568368afcd563455 Mon Sep 17 00:00:00 2001 From: Morgan Ludtke Date: Mon, 4 Mar 2024 09:40:18 -0600 Subject: [PATCH 2/2] fix: test fixes --- api/test/integration/application.e2e-spec.ts | 4 ++++ api/test/unit/services/application.service.spec.ts | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/api/test/integration/application.e2e-spec.ts b/api/test/integration/application.e2e-spec.ts index 28f2af96fb..facec388dd 100644 --- a/api/test/integration/application.e2e-spec.ts +++ b/api/test/integration/application.e2e-spec.ts @@ -129,6 +129,7 @@ describe('Application Controller Tests', () => { const res = await request(app.getHttpServer()) .get(`/applications?${query}`) + .set('Cookie', cookies) .expect(200); expect(res.body.items.length).toBe(0); }); @@ -174,6 +175,7 @@ describe('Application Controller Tests', () => { const res = await request(app.getHttpServer()) .get(`/applications?${query}`) + .set('Cookie', cookies) .expect(200); expect(res.body.items.length).toBeGreaterThanOrEqual(2); @@ -211,6 +213,7 @@ describe('Application Controller Tests', () => { const res = await request(app.getHttpServer()) .get(`/applications`) + .set('Cookie', cookies) .expect(200); expect(res.body.items.length).toBeGreaterThanOrEqual(2); @@ -652,6 +655,7 @@ describe('Application Controller Tests', () => { let geocodingOptions = savedPreferences[0].options[0]; // This catches the edge case where the geocoding hasn't completed yet if (geocodingOptions.extraData.length === 1) { + // I'm unsure why removing this console log makes this test fail. This should be looked into console.log(''); savedApplication = await prisma.applications.findMany({ where: { diff --git a/api/test/unit/services/application.service.spec.ts b/api/test/unit/services/application.service.spec.ts index 2ad01fb12f..f0b92c2564 100644 --- a/api/test/unit/services/application.service.spec.ts +++ b/api/test/unit/services/application.service.spec.ts @@ -9,6 +9,7 @@ import { } from '@prisma/client'; import { randomUUID } from 'crypto'; import dayjs from 'dayjs'; +import { Request as ExpressRequest } from 'express'; import { PrismaService } from '../../../src/services/prisma.service'; import { ApplicationService } from '../../../src/services/application.service'; import { ApplicationQueryParams } from '../../../src/dtos/applications/application-query-params.dto'; @@ -268,6 +269,12 @@ describe('Testing application service', () => { }); it('should get applications from list() when applications are available', async () => { + const requestingUser = { + firstName: 'requesting fName', + lastName: 'requesting lName', + email: 'requestingUser@email.com', + jurisdictions: [{ id: 'juris id' }], + } as unknown as User; const date = new Date(); const mockedValue = mockApplicationSet(3, date); prisma.applications.findMany = jest.fn().mockResolvedValue(mockedValue); @@ -284,7 +291,11 @@ describe('Testing application service', () => { page: 1, }; - expect(await service.list(params)).toEqual({ + expect( + await service.list(params, { + user: requestingUser, + } as unknown as ExpressRequest), + ).toEqual({ items: mockedValue.map((mock) => ({ ...mock, flagged: true })), meta: { currentPage: 1,