From 1250ffe70084d8f3b2f053e5bc1dbfc20ce1b4e5 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 10 Mar 2024 12:51:48 -0700 Subject: [PATCH 1/2] Fixed XSS vulnerability - sanitized permission action req --- src/routers/admin.js | 6 ++++-- tests/admin.test.js | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/routers/admin.js b/src/routers/admin.js index c401a36..cd62187 100644 --- a/src/routers/admin.js +++ b/src/routers/admin.js @@ -7,6 +7,7 @@ import { permissionCache } from '../helpers/cache'; import { responseException } from '../exceptions'; import * as Services from '../services/admin'; import { SwitcherKeys } from '../external/switcher-api-facade'; +import { checkActionType } from '../models/permission'; const router = new express.Router(); @@ -101,6 +102,7 @@ router.get('/admin/me', auth, async (req, res) => { router.post('/admin/collaboration/permission', auth, [ check('domain', 'Domain Id is required').isMongoId(), check('action', 'Array of actions is required').isArray({ min: 1 }), + check('action.*').custom(async value => checkActionType([value])), check('router', 'Router name is required').isLength({ min: 1 }), check('environment').isString().optional() ], validate, async (req, res) => { @@ -124,9 +126,9 @@ router.post('/admin/collaboration/permission', auth, [ try { await verifyOwnership(req.admin, element, req.body.domain, action_perm, req.body.router, false, req.body.environment); - result.push({ action: action_perm.toString(), result: 'ok' }); + result.push({ action: action_perm, result: 'ok' }); } catch (e) { - result.push({ action : action_perm.toString(), result: 'nok' }); + result.push({ action : action_perm, result: 'nok' }); } } diff --git a/tests/admin.test.js b/tests/admin.test.js index 18a0b27..e76f446 100644 --- a/tests/admin.test.js +++ b/tests/admin.test.js @@ -949,6 +949,22 @@ describe('Testing Admin collaboration endpoint - Reading permissions', () => { token = responseLogin.body.jwt.token; }); + test('ADMIN_SUITE - Should NOT read permissions given invalid action request', async () => { + const response = await request(app) + .post('/admin/collaboration/permission') + .set('Authorization', `Bearer ${token}`) + .send({ + domain: domainId, + action: ['INVALID_ACTION'], + router: RouterTypes.GROUP, + environment: EnvType.DEFAULT + }) + .expect(422); + + expect(response.body.errors[0].msg).toEqual( + 'Permission validation failed: action: \'INVALID_ACTION\' is not a valid enum value.'); + }); + test('ADMIN_SUITE - Should read permissions given request - Group', async () => { const response = await request(app) .post('/admin/collaboration/permission') From ab88cc628fc2ddb14810c1acb39e1f794dfd5b86 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 10 Mar 2024 12:57:40 -0700 Subject: [PATCH 2/2] Removed deprecated mongoose ObjectId constructor --- src/services/config.js | 6 ++---- tests/config.test.js | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/services/config.js b/src/services/config.js index a936d89..f283624 100644 --- a/src/services/config.js +++ b/src/services/config.js @@ -1,4 +1,3 @@ -import mongoose from 'mongoose'; import { response } from './common'; import { Config } from '../models/config'; import { formatInput, verifyOwnership, checkEnvironmentStatusRemoval } from '../helpers'; @@ -258,15 +257,14 @@ export async function removeComponent(id, args, admin) { export async function updateComponent(id, args, admin) { const config = await verifyAddComponentInput(id, admin); - const componentIds = args.components.map(component => new mongoose.Types.ObjectId(component)); - const components = await getComponents({ _id: { $in: componentIds } }); + const components = await getComponents({ _id: { $in: args.components } }); if (components.length != args.components.length) { throw new NotFoundError('One or more component was not found'); } config.updatedBy = admin.email; - config.components = componentIds; + config.components = args.components; await config.save(); updateDomainVersion(config.domain); diff --git a/tests/config.test.js b/tests/config.test.js index 98d5c36..45d28f0 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -590,7 +590,7 @@ describe('Testing component association', () => { }).expect(200); // DB validation - document updated - const config = await Config.findById(configId1).exec() + const config = await Config.findById(configId1).exec(); expect(config.components.includes(responseComponent.body.component._id)).toBe(true); });