diff --git a/api/src/middlewares/verifyAdminIfNeeded.ts b/api/src/middlewares/verifyAdminIfNeeded.ts index 999edfa..de39eac 100644 --- a/api/src/middlewares/verifyAdminIfNeeded.ts +++ b/api/src/middlewares/verifyAdminIfNeeded.ts @@ -8,8 +8,8 @@ export const verifyAdminIfNeeded: RequestHandler = (req, res, next) => { if (!user?.isAdmin) { let adminAccountRequired: boolean = true - if (req.params.userId) { - adminAccountRequired = user?.userId !== req.params.userId + if (req.params.uid) { + adminAccountRequired = user?.userId !== req.params.uid } else if (req.params.username) { adminAccountRequired = user?.username !== req.params.username } diff --git a/api/src/routes/api/group.ts b/api/src/routes/api/group.ts index 7db3f77..7b2be9c 100644 --- a/api/src/routes/api/group.ts +++ b/api/src/routes/api/group.ts @@ -1,7 +1,11 @@ import express from 'express' import { GroupController } from '../../controllers/' import { authenticateAccessToken, verifyAdmin } from '../../middlewares' -import { getGroupValidation, registerGroupValidation } from '../../utils' +import { + getGroupValidation, + registerGroupValidation, + uidValidation +} from '../../utils' const groupRouter = express.Router() @@ -34,7 +38,10 @@ groupRouter.get('/', authenticateAccessToken, async (req, res) => { }) groupRouter.get('/:uid', authenticateAccessToken, async (req, res) => { - const { uid } = req.params + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + + const { uid } = params const controller = new GroupController() try { @@ -76,7 +83,6 @@ groupRouter.post( const response = await controller.addUserToGroup(groupUid, userUid) res.send(response) } catch (err: any) { - console.log('err :>> ', err) res.status(err.code).send(err.message) } } @@ -104,7 +110,10 @@ groupRouter.delete( authenticateAccessToken, verifyAdmin, async (req, res) => { - const { uid } = req.params + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + + const { uid } = params const controller = new GroupController() try { diff --git a/api/src/routes/api/permission.ts b/api/src/routes/api/permission.ts index b45af93..c5d5390 100644 --- a/api/src/routes/api/permission.ts +++ b/api/src/routes/api/permission.ts @@ -3,6 +3,7 @@ import { PermissionController } from '../../controllers/' import { verifyAdmin } from '../../middlewares' import { registerPermissionValidation, + uidValidation, updatePermissionValidation } from '../../utils' @@ -35,7 +36,10 @@ permissionRouter.post('/', verifyAdmin, async (req, res) => { }) permissionRouter.patch('/:uid', verifyAdmin, async (req: any, res) => { - const { uid } = req.params + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + + const { uid } = params const { error, value: body } = updatePermissionValidation(req.body) if (error) return res.status(400).send(error.details[0].message) @@ -51,8 +55,10 @@ permissionRouter.patch('/:uid', verifyAdmin, async (req: any, res) => { }) permissionRouter.delete('/:uid', verifyAdmin, async (req: any, res) => { - const { uid } = req.params + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + const { uid } = params try { await controller.deletePermission(uid) res.status(200).send('Permission Deleted!') diff --git a/api/src/routes/api/spec/group.spec.ts b/api/src/routes/api/spec/group.spec.ts index 72215a2..711c31c 100644 --- a/api/src/routes/api/spec/group.spec.ts +++ b/api/src/routes/api/spec/group.spec.ts @@ -11,6 +11,7 @@ import { } from '../../../utils' import Group, { PUBLIC_GROUP_NAME } from '../../../model/Group' import User from '../../../model/User' +import { randomBytes } from 'crypto' const clientId = 'someclientID' const adminUser = { @@ -75,7 +76,7 @@ describe('group', () => { .send(group) .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -201,8 +202,10 @@ describe('group', () => { }) it('should respond with Not Found if groupId is incorrect', async () => { + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .delete(`/SASjsApi/group/1234`) + .delete(`/SASjsApi/group/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -253,7 +256,7 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -274,7 +277,7 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -292,8 +295,10 @@ describe('group', () => { }) it('should respond with Not Found if groupId is incorrect', async () => { + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .get('/SASjsApi/group/1234') + .get(`/SASjsApi/group/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -312,7 +317,7 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -333,7 +338,7 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -379,7 +384,7 @@ describe('group', () => { expect(res.body).toEqual([ { - groupId: expect.anything(), + uid: expect.anything(), name: group.name, description: group.description } @@ -401,7 +406,7 @@ describe('group', () => { expect(res.body).toEqual([ { - groupId: expect.anything(), + uid: expect.anything(), name: group.name, description: group.description } @@ -431,13 +436,13 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) expect(res.body.users).toEqual([ { - id: expect.anything(), + uid: expect.anything(), username: user.username, displayName: user.displayName } @@ -465,7 +470,7 @@ describe('group', () => { expect(res.body.groups).toEqual([ { - groupId: expect.anything(), + uid: expect.anything(), name: group.name, description: group.description } @@ -486,13 +491,13 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) expect(res.body.users).toEqual([ { - id: expect.anything(), + uid: expect.anything(), username: 'addUserRandomUser', displayName: user.displayName } @@ -526,8 +531,10 @@ describe('group', () => { }) it('should respond with Not Found if groupId is incorrect', async () => { + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .post('/SASjsApi/group/123/123') + .post(`/SASjsApi/group/${hexValue}/123`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -538,8 +545,10 @@ describe('group', () => { it('should respond with Not Found if userId is incorrect', async () => { const dbGroup = await groupController.createGroup(group) + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .post(`/SASjsApi/group/${dbGroup.uid}/123`) + .post(`/SASjsApi/group/${dbGroup.uid}/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -626,7 +635,7 @@ describe('group', () => { .send() .expect(200) - expect(res.body.groupId).toBeTruthy() + expect(res.body.uid).toBeTruthy() expect(res.body.name).toEqual(group.name) expect(res.body.description).toEqual(group.description) expect(res.body.isActive).toEqual(true) @@ -723,8 +732,10 @@ describe('group', () => { }) it('should respond with Not Found if groupId is incorrect', async () => { + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .delete('/SASjsApi/group/123/123') + .delete(`/SASjsApi/group/${hexValue}/123`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -735,8 +746,10 @@ describe('group', () => { it('should respond with Not Found if userId is incorrect', async () => { const dbGroup = await groupController.createGroup(group) + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .delete(`/SASjsApi/group/${dbGroup.uid}/123`) + .delete(`/SASjsApi/group/${dbGroup.uid}/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) diff --git a/api/src/routes/api/spec/permission.spec.ts b/api/src/routes/api/spec/permission.spec.ts index b8580ff..ccdd120 100644 --- a/api/src/routes/api/spec/permission.spec.ts +++ b/api/src/routes/api/spec/permission.spec.ts @@ -17,6 +17,7 @@ import { PermissionDetailsResponse } from '../../../controllers' import { generateAccessToken, saveTokensInDB } from '../../../utils' +import { randomBytes } from 'crypto' const deployPayload = { appLoc: 'string', @@ -281,17 +282,19 @@ describe('permission', () => { expect(res.body).toEqual({}) }) - it('should respond with Bad Request if principalId is not a number', async () => { + it('should respond with Bad Request if principalId is not a string of 24 hex characters', async () => { const res = await request(app) .post('/SASjsApi/permission') .auth(adminAccessToken, { type: 'bearer' }) .send({ ...permission, - principalId: 'someCharacters' + principalId: randomBytes(10).toString('hex') }) .expect(400) - expect(res.text).toEqual('"principalId" must be a number') + expect(res.text).toEqual( + '"principalId" length must be 24 characters long' + ) expect(res.body).toEqual({}) }) @@ -321,7 +324,7 @@ describe('permission', () => { .auth(adminAccessToken, { type: 'bearer' }) .send({ ...permission, - principalId: 123 + principalId: randomBytes(12).toString('hex') }) .expect(404) @@ -336,7 +339,7 @@ describe('permission', () => { .send({ ...permission, principalType: 'group', - principalId: 123 + principalId: randomBytes(12).toString('hex') }) .expect(404) @@ -437,8 +440,9 @@ describe('permission', () => { }) it('should respond with not found (404) if permission with provided id does not exist', async () => { + const hexValue = randomBytes(12).toString('hex') const res = await request(app) - .patch('/SASjsApi/permission/123') + .patch(`/SASjsApi/permission/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send({ setting: PermissionSettingForRoute.deny @@ -466,8 +470,10 @@ describe('permission', () => { }) it('should respond with not found (404) if permission with provided id does not exists', async () => { + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .delete('/SASjsApi/permission/123') + .delete(`/SASjsApi/permission/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) diff --git a/api/src/routes/api/spec/user.spec.ts b/api/src/routes/api/spec/user.spec.ts index 734dc5b..71ceb88 100644 --- a/api/src/routes/api/spec/user.spec.ts +++ b/api/src/routes/api/spec/user.spec.ts @@ -1,3 +1,4 @@ +import { randomBytes } from 'crypto' import { Express } from 'express' import mongoose, { Mongoose } from 'mongoose' import { MongoMemoryServer } from 'mongodb-memory-server' @@ -690,8 +691,10 @@ describe('user', () => { it('should respond with Not Found if userId is incorrect', async () => { await controller.createUser(user) + const hexValue = randomBytes(12).toString('hex') + const res = await request(app) - .get('/SASjsApi/user/1234') + .get(`/SASjsApi/user/${hexValue}`) .auth(adminAccessToken, { type: 'bearer' }) .send() .expect(404) @@ -803,13 +806,13 @@ describe('user', () => { expect(res.body).toEqual([ { - id: expect.anything(), + uid: expect.anything(), username: adminUser.username, displayName: adminUser.displayName, isAdmin: adminUser.isAdmin }, { - id: expect.anything(), + uid: expect.anything(), username: user.username, displayName: user.displayName, isAdmin: user.isAdmin @@ -831,13 +834,13 @@ describe('user', () => { expect(res.body).toEqual([ { - id: expect.anything(), + uid: expect.anything(), username: adminUser.username, displayName: adminUser.displayName, isAdmin: adminUser.isAdmin }, { - id: expect.anything(), + uid: expect.anything(), username: 'randomUser', displayName: user.displayName, isAdmin: user.isAdmin diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index f5529aa..06aa7f1 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/api/src/routes/api/spec/web.spec.ts @@ -145,7 +145,7 @@ describe('web', () => { expect(res.body.loggedIn).toBeTruthy() expect(res.body.user).toEqual({ - id: expect.any(Number), + id: expect.anything(), username: user.username, displayName: user.displayName, isAdmin: user.isAdmin, diff --git a/api/src/routes/api/user.ts b/api/src/routes/api/user.ts index 2dcaf14..987e027 100644 --- a/api/src/routes/api/user.ts +++ b/api/src/routes/api/user.ts @@ -9,6 +9,7 @@ import { deleteUserValidation, getUserValidation, registerUserValidation, + uidValidation, updateUserValidation } from '../../utils' @@ -57,7 +58,10 @@ userRouter.get( ) userRouter.get('/:uid', authenticateAccessToken, async (req, res) => { - const { uid } = req.params + const { error, value: params } = uidValidation(req.params) + if (error) return res.status(400).send(error.details[0].message) + + const { uid } = params const controller = new UserController() try { @@ -102,7 +106,11 @@ userRouter.patch( verifyAdminIfNeeded, async (req, res) => { const { user } = req - const { uid } = req.params + + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + + const { uid } = params // only an admin can update `isActive` and `isAdmin` fields const { error, value: body } = updateUserValidation(req.body, user!.isAdmin) @@ -152,7 +160,11 @@ userRouter.delete( verifyAdminIfNeeded, async (req, res) => { const { user } = req - const { uid } = req.params + + const { error: uidError, value: params } = uidValidation(req.params) + if (uidError) return res.status(400).send(uidError.details[0].message) + + const { uid } = params // only an admin can delete user without providing password const { error, value: data } = deleteUserValidation(req.body, user!.isAdmin) diff --git a/api/src/utils/validation.ts b/api/src/utils/validation.ts index 10d456e..11f2634 100644 --- a/api/src/utils/validation.ts +++ b/api/src/utils/validation.ts @@ -12,6 +12,11 @@ const groupnameSchema = Joi.string().lowercase().alphanum().min(3).max(16) export const blockFileRegex = /\.(exe|sh|htaccess)$/i +export const uidValidation = (data: any) => + Joi.object({ + uid: Joi.string().length(24).hex().required() + }).validate(data) + export const getUserValidation = (data: any): Joi.ValidationResult => Joi.object({ username: usernameSchema.required() @@ -113,7 +118,7 @@ export const registerPermissionValidation = (data: any): Joi.ValidationResult => principalType: Joi.string() .required() .valid(...Object.values(PrincipalType)), - principalId: Joi.string().required() + principalId: Joi.string().length(24).hex().required() }).validate(data) export const updatePermissionValidation = (data: any): Joi.ValidationResult =>