diff --git a/src/lib/features/segment/admin-segment.e2e.test.ts b/src/lib/features/segment/admin-segment.e2e.test.ts index b60fcd30ef..46010d7b7f 100644 --- a/src/lib/features/segment/admin-segment.e2e.test.ts +++ b/src/lib/features/segment/admin-segment.e2e.test.ts @@ -194,6 +194,16 @@ test('should create segments', async () => { expect(segments.map((s) => s.name)).toEqual(['a', 'b', 'c']); }); +test('should return 400 for non numeeric segment ID', async () => { + const { body } = await app.request + .get(`${SEGMENTS_BASE_PATH}/stringName`) + .expect(400); + expect(body).toMatchObject({ + message: + 'Request validation failed: your request body or params contain invalid data: ID should be an integer', + }); +}); + test('should update segments', async () => { await app.createSegment({ name: 'a', diff --git a/src/lib/features/segment/segment-controller.ts b/src/lib/features/segment/segment-controller.ts index f9aaa6c160..ec72e8f5f2 100644 --- a/src/lib/features/segment/segment-controller.ts +++ b/src/lib/features/segment/segment-controller.ts @@ -41,6 +41,7 @@ import { import { anonymiseKeys, extractUserIdFromUser } from '../../util'; import { BadDataError } from '../../error'; +import idNumberMiddleware from '../../middleware/id-number-middleware'; type IUpdateFeatureStrategySegmentsRequest = IAuthRequest< {}, @@ -159,6 +160,7 @@ export class SegmentsController extends Controller { 200: createResponseSchema('segmentStrategiesSchema'), }, }), + idNumberMiddleware(), ], }); @@ -180,6 +182,7 @@ export class SegmentsController extends Controller { ...getStandardResponses(401, 403, 409), }, }), + idNumberMiddleware(), ], }); @@ -200,6 +203,7 @@ export class SegmentsController extends Controller { ...getStandardResponses(400, 401, 403, 409, 415), }, }), + idNumberMiddleware(), ], }); @@ -219,6 +223,7 @@ export class SegmentsController extends Controller { ...getStandardResponses(404), }, }), + idNumberMiddleware(), ], }); diff --git a/src/lib/middleware/id-number-middleware.test.ts b/src/lib/middleware/id-number-middleware.test.ts new file mode 100644 index 0000000000..f3310799c7 --- /dev/null +++ b/src/lib/middleware/id-number-middleware.test.ts @@ -0,0 +1,32 @@ +import express from 'express'; +import supertest from 'supertest'; +import idNumberMiddleware from './id-number-middleware'; + +describe('idNumberMiddleware', () => { + it('should pass when id is a valid integer', async () => { + const app = express(); + app.use('/:id', idNumberMiddleware()); + app.get('/:id', (req, res) => { + res.status(200).send('Valid ID'); + }); + + await supertest(app) + .get('/123') + .expect(200) + .expect((res) => { + expect(res.text).toBe('Valid ID'); + }); + }); + it('should throw BadDataError when id is not a valid integer', async () => { + const app = express(); + app.use('/:id', idNumberMiddleware()); + app.get('/:id', (req, res) => { + res.status(200).send('This should not be executed'); + }); + + const { body } = await supertest(app).get('/abc').expect(400); + expect(body).toMatchObject({ + details: [{ message: 'ID should be an integer' }], + }); + }); +}); diff --git a/src/lib/middleware/id-number-middleware.ts b/src/lib/middleware/id-number-middleware.ts new file mode 100644 index 0000000000..c5200e0792 --- /dev/null +++ b/src/lib/middleware/id-number-middleware.ts @@ -0,0 +1,16 @@ +import { BadDataError } from '../error'; + +const idNumberMiddleware = (): any => { + return async (req, res, next) => { + const { id } = req.params; + if (!Number.isInteger(Number(id))) { + res.status(400).send( + new BadDataError('ID should be an integer').toJSON(), + ); + return; + } + next(); + }; +}; + +export default idNumberMiddleware; diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index 68406fa150..0764ad45cd 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -51,12 +51,13 @@ import { type AdminCountSchema, adminCountSchema, } from '../../openapi/spec/admin-count-schema'; -import { BadDataError, ForbiddenError } from '../../error'; +import { ForbiddenError } from '../../error'; import { createUserResponseSchema, type CreateUserResponseSchema, } from '../../openapi/spec/create-user-response-schema'; import type { IRoleWithPermissions } from '../../types/stores/access-store'; +import idNumberMiddleware from '../../middleware/id-number-middleware'; export default class UserAdminController extends Controller { private flagResolver: IFlagResolver; @@ -354,6 +355,7 @@ export default class UserAdminController extends Controller { ...getStandardResponses(400, 401, 404), }, }), + idNumberMiddleware(), ], }); @@ -375,6 +377,7 @@ export default class UserAdminController extends Controller { ...getStandardResponses(400, 401, 403, 404), }, }), + idNumberMiddleware(), ], }); @@ -395,6 +398,7 @@ export default class UserAdminController extends Controller { ...getStandardResponses(401, 403, 404), }, }), + idNumberMiddleware(), ], }); } @@ -505,9 +509,6 @@ export default class UserAdminController extends Controller { async getUser(req: Request, res: Response): Promise { const { id } = req.params; - if (!Number.isInteger(Number(id))) { - throw new BadDataError('User id should be an integer'); - } const user = await this.userService.getUser(Number(id)); this.openApiService.respondWithValidation( @@ -607,9 +608,7 @@ export default class UserAdminController extends Controller { const { user, params, body } = req; const { id } = params; const { name, email, rootRole } = body; - if (!Number.isInteger(Number(id))) { - throw new BadDataError('User id should be an integer'); - } + await this.throwIfScimUser({ id: Number(id) }); const normalizedRootRole = Number.isInteger(Number(rootRole)) ? Number(rootRole) @@ -639,9 +638,7 @@ export default class UserAdminController extends Controller { async deleteUser(req: IAuthRequest, res: Response): Promise { const { user, params } = req; const { id } = params; - if (!Number.isInteger(Number(id))) { - throw new BadDataError('User id should be an integer'); - } + await this.throwIfScimUser({ id: Number(id) }); await this.userService.deleteUser(+id, user); diff --git a/src/lib/routes/admin-api/user/pat.ts b/src/lib/routes/admin-api/user/pat.ts index b56abae116..246b7b075f 100644 --- a/src/lib/routes/admin-api/user/pat.ts +++ b/src/lib/routes/admin-api/user/pat.ts @@ -26,6 +26,7 @@ import { createPatSchema, } from '../../../openapi/spec/create-pat-schema'; import { ForbiddenError, NotFoundError } from '../../../error'; +import idNumberMiddleware from '../../../middleware/id-number-middleware'; export default class PatController extends Controller { private patService: PatService; @@ -109,6 +110,7 @@ export default class PatController extends Controller { ...getStandardResponses(401, 403, 404), }, }), + idNumberMiddleware(), ], }); }