From c3e3befc17102ee1754e1403193040b4f79fb2a7 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 2 Aug 2022 18:04:00 +0500 Subject: [PATCH 1/7] feat: add public group to DB on seed --- api/src/model/Group.ts | 2 ++ api/src/utils/seedDB.ts | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/api/src/model/Group.ts b/api/src/model/Group.ts index 6341825..3185f44 100644 --- a/api/src/model/Group.ts +++ b/api/src/model/Group.ts @@ -3,6 +3,8 @@ import { GroupDetailsResponse } from '../controllers' import User, { IUser } from './User' const AutoIncrement = require('mongoose-sequence')(mongoose) +export const PUBLIC_GROUP_NAME = 'Public' + export interface GroupPayload { /** * Name of the group diff --git a/api/src/utils/seedDB.ts b/api/src/utils/seedDB.ts index 7bff3a6..a43b374 100644 --- a/api/src/utils/seedDB.ts +++ b/api/src/utils/seedDB.ts @@ -1,5 +1,5 @@ import Client from '../model/Client' -import Group from '../model/Group' +import Group, { PUBLIC_GROUP_NAME } from '../model/Group' import User from '../model/User' import Configuration, { ConfigurationType } from '../model/Configuration' @@ -31,6 +31,15 @@ export const seedDB = async (): Promise => { console.log(`DB Seed - Group created: ${GROUP.name}`) } + // Checking if 'Public' Group is already in the database + const publicGroupExist = await Group.findOne({ name: PUBLIC_GROUP.name }) + if (!publicGroupExist) { + const group = new Group(PUBLIC_GROUP) + await group.save() + + console.log(`DB Seed - Group created: ${PUBLIC_GROUP.name}`) + } + // Checking if user is already in the database let usernameExist = await User.findOne({ username: ADMIN_USER.username }) if (!usernameExist) { @@ -68,6 +77,13 @@ const GROUP = { name: 'AllUsers', description: 'Group contains all users' } + +const PUBLIC_GROUP = { + name: PUBLIC_GROUP_NAME, + description: + 'It is a special group that bypasses authentication for particular routes.' +} + const CLIENT = { clientId: 'clientID1', clientSecret: 'clientSecret' From d3a516c36e45aa1cc76c30c744e6a0e5bd553165 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 2 Aug 2022 18:05:28 +0500 Subject: [PATCH 2/7] fix: add restriction on add/remove user to public group --- api/src/controllers/group.ts | 9 ++++++++- api/src/routes/api/spec/group.spec.ts | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/api/src/controllers/group.ts b/api/src/controllers/group.ts index 9f6e41b..c5b8681 100644 --- a/api/src/controllers/group.ts +++ b/api/src/controllers/group.ts @@ -10,7 +10,7 @@ import { Body } from 'tsoa' -import Group, { GroupPayload } from '../model/Group' +import Group, { GroupPayload, PUBLIC_GROUP_NAME } from '../model/Group' import User from '../model/User' import { UserResponse } from './user' @@ -241,6 +241,13 @@ const updateUsersListInGroup = async ( message: 'Group not found.' } + if (group.name === PUBLIC_GROUP_NAME) + throw { + code: 400, + status: 'Bad Request', + message: `Can't add/remove user to '${PUBLIC_GROUP_NAME}' group.` + } + const user = await User.findOne({ id: userId }) if (!user) throw { diff --git a/api/src/routes/api/spec/group.spec.ts b/api/src/routes/api/spec/group.spec.ts index 86196ce..4b34e9e 100644 --- a/api/src/routes/api/spec/group.spec.ts +++ b/api/src/routes/api/spec/group.spec.ts @@ -5,6 +5,7 @@ import request from 'supertest' import appPromise from '../../../app' import { UserController, GroupController } from '../../../controllers/' import { generateAccessToken, saveTokensInDB } from '../../../utils' +import { PUBLIC_GROUP_NAME } from '../../../model/Group' const clientId = 'someclientID' const adminUser = { @@ -27,6 +28,12 @@ const group = { description: 'DC group for testing purposes.' } +const PUBLIC_GROUP = { + name: PUBLIC_GROUP_NAME, + description: + 'It is a special group that bypasses authentication for particular routes.' +} + const userController = new UserController() const groupController = new GroupController() @@ -535,6 +542,24 @@ describe('group', () => { expect(res.text).toEqual('User not found.') expect(res.body).toEqual({}) }) + + it('should respond with Bad Request when adding user to Public group', async () => { + const dbGroup = await groupController.createGroup(PUBLIC_GROUP) + const dbUser = await userController.createUser({ + ...user, + username: 'publicUser' + }) + + const res = await request(app) + .post(`/SASjsApi/group/${dbGroup.groupId}/${dbUser.id}`) + .auth(adminAccessToken, { type: 'bearer' }) + .send() + .expect(400) + + expect(res.text).toEqual( + `Can't add/remove user to '${PUBLIC_GROUP_NAME}' group.` + ) + }) }) describe('RemoveUser', () => { From 68515f95a65d422e29c0ed6028f3ea0ae8d9b1bf Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 2 Aug 2022 18:06:33 +0500 Subject: [PATCH 3/7] feat: bypass authentication when route is enabled for public group --- api/src/middlewares/authenticateToken.ts | 9 ++++++- api/src/utils/index.ts | 1 + api/src/utils/isPublicRoute.ts | 31 ++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 api/src/utils/isPublicRoute.ts diff --git a/api/src/middlewares/authenticateToken.ts b/api/src/middlewares/authenticateToken.ts index 24ed1e8..0c02127 100644 --- a/api/src/middlewares/authenticateToken.ts +++ b/api/src/middlewares/authenticateToken.ts @@ -5,7 +5,9 @@ import { fetchLatestAutoExec, ModeType, verifyTokenInDB, - isAuthorizingRoute + isAuthorizingRoute, + isPublicRoute, + publicUser } from '../utils' import { desktopUser } from './desktop' import { authorize } from './authorize' @@ -21,6 +23,11 @@ export const authenticateAccessToken: RequestHandler = async ( return next() } + if (await isPublicRoute(req)) { + req.user = publicUser + return next() + } + const nextFunction = isAuthorizingRoute(req) ? () => authorize(req, res, next) : next diff --git a/api/src/utils/index.ts b/api/src/utils/index.ts index 13bc904..32b5b79 100644 --- a/api/src/utils/index.ts +++ b/api/src/utils/index.ts @@ -16,6 +16,7 @@ export * from './getRunTimeAndFilePath' export * from './getServerUrl' export * from './instantiateLogger' export * from './isDebugOn' +export * from './isPublicRoute' export * from './zipped' export * from './parseLogToArray' export * from './removeTokensInDB' diff --git a/api/src/utils/isPublicRoute.ts b/api/src/utils/isPublicRoute.ts new file mode 100644 index 0000000..d971d93 --- /dev/null +++ b/api/src/utils/isPublicRoute.ts @@ -0,0 +1,31 @@ +import { Request } from 'express' +import { getPath } from './getAuthorizedRoutes' +import Group, { PUBLIC_GROUP_NAME } from '../model/Group' +import Permission from '../model/Permission' +import { PermissionSettingForRoute } from '../controllers' +import { RequestUser } from '../types' + +export const isPublicRoute = async (req: Request): Promise => { + const group = await Group.findOne({ name: PUBLIC_GROUP_NAME }) + if (group) { + const path = getPath(req) + + const groupPermission = await Permission.findOne({ + path, + group: group?._id + }) + if (groupPermission?.setting === PermissionSettingForRoute.grant) + return true + } + + return false +} + +export const publicUser: RequestUser = { + userId: 12345, + clientId: 'public_app', + username: 'publicUser', + displayName: 'Public User', + isAdmin: false, + isActive: true +} From f978814ca7e2244b48d422da946e89f910089a8b Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 2 Aug 2022 22:16:41 +0500 Subject: [PATCH 4/7] chore: code refactor --- api/src/middlewares/authenticateToken.ts | 64 ++++++++++++++---------- api/src/middlewares/authorize.ts | 5 +- api/src/utils/isPublicRoute.ts | 2 +- 3 files changed, 42 insertions(+), 29 deletions(-) diff --git a/api/src/middlewares/authenticateToken.ts b/api/src/middlewares/authenticateToken.ts index 0c02127..dc1f42e 100644 --- a/api/src/middlewares/authenticateToken.ts +++ b/api/src/middlewares/authenticateToken.ts @@ -23,11 +23,6 @@ export const authenticateAccessToken: RequestHandler = async ( return next() } - if (await isPublicRoute(req)) { - req.user = publicUser - return next() - } - const nextFunction = isAuthorizingRoute(req) ? () => authorize(req, res, next) : next @@ -48,7 +43,7 @@ export const authenticateAccessToken: RequestHandler = async ( return res.sendStatus(401) } - authenticateToken( + await authenticateToken( req, res, nextFunction, @@ -57,8 +52,12 @@ export const authenticateAccessToken: RequestHandler = async ( ) } -export const authenticateRefreshToken: RequestHandler = (req, res, next) => { - authenticateToken( +export const authenticateRefreshToken: RequestHandler = async ( + req, + res, + next +) => { + await authenticateToken( req, res, next, @@ -67,7 +66,7 @@ export const authenticateRefreshToken: RequestHandler = (req, res, next) => { ) } -const authenticateToken = ( +const authenticateToken = async ( req: Request, res: Response, next: NextFunction, @@ -90,26 +89,37 @@ const authenticateToken = ( const authHeader = req.headers['authorization'] const token = authHeader?.split(' ')[1] - if (!token) return res.sendStatus(401) - jwt.verify(token, key, async (err: any, data: any) => { - if (err) return res.sendStatus(401) + try { + if (!token) throw 'Unauthorized' - // verify this valid token's entry in DB - const user = await verifyTokenInDB( - data?.userId, - data?.clientId, - token, - tokenType - ) + jwt.verify(token, key, async (err: any, data: any) => { + if (err) throw 'Unauthorized' - if (user) { - if (user.isActive) { - req.user = user - if (tokenType === 'accessToken') req.accessToken = token - return next() - } else return res.sendStatus(401) + // verify this valid token's entry in DB + const user = await verifyTokenInDB( + data?.userId, + data?.clientId, + token, + tokenType + ) + + if (user) { + if (user.isActive) { + req.user = user + if (tokenType === 'accessToken') req.accessToken = token + return next() + } else throw 'Unauthorized' + } + + throw 'Unauthorized' + }) + } catch (error) { + if (await isPublicRoute(req)) { + req.user = publicUser + return next() } - return res.sendStatus(401) - }) + + res.sendStatus(401) + } } diff --git a/api/src/middlewares/authorize.ts b/api/src/middlewares/authorize.ts index 3901b3e..6b389fb 100644 --- a/api/src/middlewares/authorize.ts +++ b/api/src/middlewares/authorize.ts @@ -5,7 +5,7 @@ import { PermissionSettingForRoute, PermissionType } from '../controllers/permission' -import { getPath } from '../utils' +import { getPath, isPublicRoute } from '../utils' export const authorize: RequestHandler = async (req, res, next) => { const { user } = req @@ -17,6 +17,9 @@ export const authorize: RequestHandler = async (req, res, next) => { // no need to check for permissions when user is admin if (user.isAdmin) return next() + // no need to check for permissions when route is Public + if (await isPublicRoute(req)) return next() + const dbUser = await User.findOne({ id: user.userId }) if (!dbUser) return res.sendStatus(401) diff --git a/api/src/utils/isPublicRoute.ts b/api/src/utils/isPublicRoute.ts index d971d93..0b121ee 100644 --- a/api/src/utils/isPublicRoute.ts +++ b/api/src/utils/isPublicRoute.ts @@ -22,7 +22,7 @@ export const isPublicRoute = async (req: Request): Promise => { } export const publicUser: RequestUser = { - userId: 12345, + userId: 0, clientId: 'public_app', username: 'publicUser', displayName: 'Public User', From 254bc07da744a9708109bfb792be70aa3f6284f4 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 2 Aug 2022 23:05:42 +0500 Subject: [PATCH 5/7] fix: call jwt.verify in synchronous way --- api/src/middlewares/authenticateToken.ts | 33 +++++++++++------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/api/src/middlewares/authenticateToken.ts b/api/src/middlewares/authenticateToken.ts index dc1f42e..78e4bcb 100644 --- a/api/src/middlewares/authenticateToken.ts +++ b/api/src/middlewares/authenticateToken.ts @@ -93,27 +93,24 @@ const authenticateToken = async ( try { if (!token) throw 'Unauthorized' - jwt.verify(token, key, async (err: any, data: any) => { - if (err) throw 'Unauthorized' + const data: any = jwt.verify(token, key) - // verify this valid token's entry in DB - const user = await verifyTokenInDB( - data?.userId, - data?.clientId, - token, - tokenType - ) + const user = await verifyTokenInDB( + data?.userId, + data?.clientId, + token, + tokenType + ) - if (user) { - if (user.isActive) { - req.user = user - if (tokenType === 'accessToken') req.accessToken = token - return next() - } else throw 'Unauthorized' - } + if (user) { + if (user.isActive) { + req.user = user + if (tokenType === 'accessToken') req.accessToken = token + return next() + } else throw 'Unauthorized' + } - throw 'Unauthorized' - }) + throw 'Unauthorized' } catch (error) { if (await isPublicRoute(req)) { req.user = publicUser From bbfd53e79e16126b0c45b210343c28ce1cef1bc8 Mon Sep 17 00:00:00 2001 From: Allan Bowe <4420615+allanbowe@users.noreply.github.com> Date: Tue, 2 Aug 2022 19:32:44 +0100 Subject: [PATCH 6/7] Update group.spec.ts --- api/src/routes/api/spec/group.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/routes/api/spec/group.spec.ts b/api/src/routes/api/spec/group.spec.ts index 4b34e9e..fe1072e 100644 --- a/api/src/routes/api/spec/group.spec.ts +++ b/api/src/routes/api/spec/group.spec.ts @@ -31,7 +31,7 @@ const group = { const PUBLIC_GROUP = { name: PUBLIC_GROUP_NAME, description: - 'It is a special group that bypasses authentication for particular routes.' + 'A special group that can be used to bypass authentication for particular routes.' } const userController = new UserController() From 98e501334f51de434224992dd44c6175a598d76c Mon Sep 17 00:00:00 2001 From: Allan Bowe <4420615+allanbowe@users.noreply.github.com> Date: Tue, 2 Aug 2022 19:33:16 +0100 Subject: [PATCH 7/7] Update seedDB.ts --- api/src/utils/seedDB.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/utils/seedDB.ts b/api/src/utils/seedDB.ts index a43b374..5187900 100644 --- a/api/src/utils/seedDB.ts +++ b/api/src/utils/seedDB.ts @@ -81,7 +81,7 @@ const GROUP = { const PUBLIC_GROUP = { name: PUBLIC_GROUP_NAME, description: - 'It is a special group that bypasses authentication for particular routes.' + 'A special group that can be used to bypass authentication for particular routes.' } const CLIENT = {