diff --git a/src/lib/features/segment/segment-controller.ts b/src/lib/features/segment/segment-controller.ts index f9aaa6c160..f6b641aa07 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< {}, @@ -219,6 +220,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..853b408e84 --- /dev/null +++ b/src/lib/middleware/id-number-middleware.test.ts @@ -0,0 +1,40 @@ +import express from 'express'; +import supertest from 'supertest'; +import idNumberMiddleware from './id-number-middleware'; + +describe('idNumberMiddleware', () => { + const fakeLogger = { + debug: () => {}, + info: () => {}, + warn: jest.fn(), + error: jest.fn(), + fatal: console.error, + }; + + it('should pass when id is a valid integer', async () => { + const app = express(); + app.use('/:id', idNumberMiddleware({ getLogger: () => fakeLogger })); + 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({ getLogger: () => fakeLogger })); + 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);