From bf82a8adfa50d5c4f3e79ad276b1346a5fb98236 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 19 May 2024 14:06:26 -0700 Subject: [PATCH] Patches XSS vulnerability found in team mngmt API routes --- README.md | 5 +++-- eslint.config.js | 8 ++++---- src/routers/team.js | 11 +++++++---- tests/team.test.js | 37 ++++++++++++++++++++++++++++++++----- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 5b38f9c..5f0ad65 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,9 @@ Main features: - Exclusive Slack App to control and test changes. -- **JavaScript lib**: (https://github.com/switcherapi/switcher-client-master) -- **Java lib**: (https://github.com/switcherapi/switcher-client) +- **JS Client SDK**: (https://github.com/switcherapi/switcher-client-js) +- **Deno Client SDK**: (https://github.com/switcherapi/switcher-client-deno) +- **Java Client SDK**: (https://github.com/switcherapi/switcher-client-java) - **Switcher Management**: (https://github.com/switcherapi/switcher-management) - **Switcher Slack App**: (https://github.com/switcherapi/switcher-slack-app) diff --git a/eslint.config.js b/eslint.config.js index dfc87bd..29ac66d 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -1,13 +1,13 @@ -import js from "@eslint/js"; -import globals from "globals"; +import js from '@eslint/js'; +import globals from 'globals'; export default [ { ...js.configs.recommended, - files: ["src/**/*.js"] + files: ['src/**/*.js'] }, { - files: ["src/**/*.js"], + files: ['src/**/*.js'], rules: { quotes: ['error', 'single'], semi: ['error', 'always'], diff --git a/src/routers/team.js b/src/routers/team.js index c6ae7cb..7aad7fe 100644 --- a/src/routers/team.js +++ b/src/routers/team.js @@ -1,6 +1,6 @@ import express from 'express'; import { auth } from '../middleware/auth.js'; -import { check, query } from 'express-validator'; +import { body, check, query } from 'express-validator'; import { ActionTypes, RouterTypes } from '../models/permission.js'; import { validate, verifyInputUpdateParameters } from '../middleware/validators.js'; import { verifyOwnership } from '../helpers/index.js'; @@ -148,7 +148,8 @@ router.delete('/team/member/invite/remove/:id/:request_id', auth, [ }); router.patch('/team/member/add/:id', auth, [ - check('id').isMongoId() + check('id').isMongoId(), + body('member').isMongoId() ], validate, verifyInputUpdateParameters(['member']), async (req, res) => { try { const adminMember = await Services.addTeamMember(req.body.member, req.params.id, req.admin); @@ -159,7 +160,8 @@ router.patch('/team/member/add/:id', auth, [ }); router.patch('/team/member/remove/:id', auth, [ - check('id').isMongoId() + check('id').isMongoId(), + body('member').isMongoId() ], validate, verifyInputUpdateParameters(['member']), async (req, res) => { try { const adminMember = await Services.removeTeamMember(req.body.member, req.params.id, req.admin); @@ -170,7 +172,8 @@ router.patch('/team/member/remove/:id', auth, [ }); router.patch('/team/permission/remove/:id', auth, [ - check('id').isMongoId() + check('id').isMongoId(), + body('permission').isMongoId() ], validate, verifyInputUpdateParameters(['permission']), async (req, res) => { try { const team = await Services.removeTeamPermission(req.body.permission, req.params.id, req.admin); diff --git a/tests/team.test.js b/tests/team.test.js index 5350a44..4ef6ba0 100644 --- a/tests/team.test.js +++ b/tests/team.test.js @@ -448,7 +448,7 @@ describe('Updating team members tests', () => { await request(app) .patch(`/team/member/add/${team1Id}`) .set('Authorization', `Bearer ${adminMasterAccountToken}`) - .send().expect(400); + .send().expect(422); }); test('TEAM_SUITE - Should NOT add a team member - Member already joined', async () => { @@ -461,12 +461,21 @@ describe('Updating team members tests', () => { }); test('TEAM_SUITE - Should NOT add a team member - Invalid parameter', async () => { + // Test - invalid parameter key await request(app) .patch(`/team/member/add/${team1Id}`) .set('Authorization', `Bearer ${adminMasterAccountToken}`) .send({ admin: adminAccountId - }).expect(400); + }).expect(422); + + // Test - invalid parameter value + await request(app) + .patch(`/team/member/add/${team1Id}`) + .set('Authorization', `Bearer ${adminMasterAccountToken}`) + .send({ + member: 'INVALID_ID' + }).expect(422); }); test('TEAM_SUITE - Should NOT remove a team member - Team not found', async () => { @@ -509,16 +518,25 @@ describe('Updating team members tests', () => { await request(app) .patch(`/team/member/remove/${team1Id}`) .set('Authorization', `Bearer ${adminMasterAccountToken}`) - .send().expect(400); + .send().expect(422); }); test('TEAM_SUITE - Should NOT remove a team member - Invalid parameter', async () => { + // Test - invalid parameter key await request(app) .patch(`/team/member/remove/${team1Id}`) .set('Authorization', `Bearer ${adminMasterAccountToken}`) .send({ admin: adminAccountId - }).expect(400); + }).expect(422); + + // Test - invalid parameter value + await request(app) + .patch(`/team/member/remove/${team1Id}`) + .set('Authorization', `Bearer ${adminMasterAccountToken}`) + .send({ + member: 'INVALID_ID' + }).expect(422); }); test('TEAM_SUITE - Should remove a team member', async () => { @@ -650,12 +668,21 @@ describe('Updating team permissions tests', () => { }); test('TEAM_SUITE - Should NOT remove a permission - Invalid parameter', async () => { + // Test - invalid parameter key await request(app) .patch(`/team/permission/remove/${team1Id}`) .set('Authorization', `Bearer ${adminMasterAccountToken}`) .send({ member: permission1Id - }).expect(400); + }).expect(422); + + // Test - invalid parameter value + await request(app) + .patch(`/team/permission/remove/${team1Id}`) + .set('Authorization', `Bearer ${adminMasterAccountToken}`) + .send({ + permission: 'INVALID_ID' + }).expect(422); }); test('TEAM_SUITE - Should NOT remove a permission - Permission not found', async () => {