From e42fdd35756a81ecaf788cef087429f1a82b2d97 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Mon, 4 Jul 2022 20:13:46 +0500 Subject: [PATCH] chore: conditionally call authorize middleware from authenticateToken --- api/public/swagger.yaml | 6 +- api/src/middlewares/authenticateToken.ts | 18 ++++- api/src/routes/api/code.ts | 3 +- api/src/routes/api/drive.ts | 14 ++-- api/src/routes/api/index.ts | 7 +- api/src/routes/api/permission.ts | 86 +++++++++------------- api/src/routes/api/spec/permission.spec.ts | 13 ++++ api/src/routes/api/stp.ts | 4 +- api/src/utils/appStreamConfig.ts | 3 +- 9 files changed, 79 insertions(+), 75 deletions(-) diff --git a/api/public/swagger.yaml b/api/public/swagger.yaml index a92bd29..afa176f 100644 --- a/api/public/swagger.yaml +++ b/api/public/swagger.yaml @@ -550,12 +550,12 @@ components: additionalProperties: false AuthorizedRoutesResponse: properties: - routes: + URIs: items: type: string type: array required: - - routes + - URIs type: object additionalProperties: false ExecuteReturnJsonPayload: @@ -1615,7 +1615,7 @@ paths: $ref: '#/components/schemas/AuthorizedRoutesResponse' examples: 'Example 1': - value: {routes: [/AppStream, /SASjsApi/stp/execute]} + value: {URIs: [/AppStream, /SASjsApi/stp/execute]} summary: 'Get authorized routes.' tags: - Info diff --git a/api/src/middlewares/authenticateToken.ts b/api/src/middlewares/authenticateToken.ts index 90c7027..be39165 100644 --- a/api/src/middlewares/authenticateToken.ts +++ b/api/src/middlewares/authenticateToken.ts @@ -1,8 +1,14 @@ import { RequestHandler, Request, Response, NextFunction } from 'express' import jwt from 'jsonwebtoken' import { csrfProtection } from '../app' -import { fetchLatestAutoExec, ModeType, verifyTokenInDB } from '../utils' +import { + fetchLatestAutoExec, + ModeType, + verifyTokenInDB, + getAuthorizedRoutes +} from '../utils' import { desktopUser } from './desktop' +import { authorize } from './authorize' export const authenticateAccessToken: RequestHandler = async ( req, @@ -15,6 +21,12 @@ export const authenticateAccessToken: RequestHandler = async ( return next() } + const authorizedRoutes = getAuthorizedRoutes() + const uri = req.baseUrl + req.path + const nextFunction = authorizedRoutes.includes(uri) + ? () => authorize(req, res, next) + : next + // if request is coming from web and has valid session // it can be validated. if (req.session?.loggedIn) { @@ -24,7 +36,7 @@ export const authenticateAccessToken: RequestHandler = async ( if (user) { if (user.isActive) { req.user = user - return csrfProtection(req, res, next) + return csrfProtection(req, res, nextFunction) } else return res.sendStatus(401) } } @@ -34,7 +46,7 @@ export const authenticateAccessToken: RequestHandler = async ( authenticateToken( req, res, - next, + nextFunction, process.env.ACCESS_TOKEN_SECRET as string, 'accessToken' ) diff --git a/api/src/routes/api/code.ts b/api/src/routes/api/code.ts index 4c7fb57..09171c0 100644 --- a/api/src/routes/api/code.ts +++ b/api/src/routes/api/code.ts @@ -1,13 +1,12 @@ import express from 'express' import { runCodeValidation } from '../../utils' import { CodeController } from '../../controllers/' -import { authorize } from '../../middlewares' const runRouter = express.Router() const controller = new CodeController() -runRouter.post('/execute', authorize, async (req, res) => { +runRouter.post('/execute', async (req, res) => { const { error, value: body } = runCodeValidation(req.body) if (error) return res.status(400).send(error.details[0].message) diff --git a/api/src/routes/api/drive.ts b/api/src/routes/api/drive.ts index 6b9115e..6126946 100644 --- a/api/src/routes/api/drive.ts +++ b/api/src/routes/api/drive.ts @@ -3,7 +3,6 @@ import { deleteFile, readFile } from '@sasjs/utils' import { publishAppStream } from '../appStream' -import { authorize } from '../../middlewares' import { multerSingle } from '../../middlewares/multer' import { DriveController } from '../../controllers/' import { @@ -20,7 +19,7 @@ const controller = new DriveController() const driveRouter = express.Router() -driveRouter.post('/deploy', authorize, async (req, res) => { +driveRouter.post('/deploy', async (req, res) => { const { error, value: body } = deployValidation(req.body) if (error) return res.status(400).send(error.details[0].message) @@ -49,7 +48,6 @@ driveRouter.post('/deploy', authorize, async (req, res) => { driveRouter.post( '/deploy/upload', - authorize, (...arg) => multerSingle('file', arg), async (req, res) => { if (!req.file) return res.status(400).send('"file" is not present.') @@ -113,7 +111,7 @@ driveRouter.post( } ) -driveRouter.get('/file', authorize, async (req, res) => { +driveRouter.get('/file', async (req, res) => { const { error: errQ, value: query } = fileParamValidation(req.query) if (errQ) return res.status(400).send(errQ.details[0].message) @@ -125,7 +123,7 @@ driveRouter.get('/file', authorize, async (req, res) => { } }) -driveRouter.get('/folder', authorize, async (req, res) => { +driveRouter.get('/folder', async (req, res) => { const { error: errQ, value: query } = folderParamValidation(req.query) if (errQ) return res.status(400).send(errQ.details[0].message) @@ -138,7 +136,7 @@ driveRouter.get('/folder', authorize, async (req, res) => { } }) -driveRouter.delete('/file', authorize, async (req, res) => { +driveRouter.delete('/file', async (req, res) => { const { error: errQ, value: query } = fileParamValidation(req.query) if (errQ) return res.status(400).send(errQ.details[0].message) @@ -153,7 +151,6 @@ driveRouter.delete('/file', authorize, async (req, res) => { driveRouter.post( '/file', - authorize, (...arg) => multerSingle('file', arg), async (req, res) => { const { error: errQ, value: query } = fileParamValidation(req.query) @@ -182,7 +179,6 @@ driveRouter.post( driveRouter.patch( '/file', - authorize, (...arg) => multerSingle('file', arg), async (req, res) => { const { error: errQ, value: query } = fileParamValidation(req.query) @@ -209,7 +205,7 @@ driveRouter.patch( } ) -driveRouter.get('/fileTree', authorize, async (req, res) => { +driveRouter.get('/fileTree', async (req, res) => { try { const response = await controller.getFileTree() res.send(response) diff --git a/api/src/routes/api/index.ts b/api/src/routes/api/index.ts index 45aa73b..04e4a19 100644 --- a/api/src/routes/api/index.ts +++ b/api/src/routes/api/index.ts @@ -36,7 +36,12 @@ router.use('/group', desktopRestrict, groupRouter) router.use('/stp', authenticateAccessToken, stpRouter) router.use('/code', authenticateAccessToken, codeRouter) router.use('/user', desktopRestrict, userRouter) -router.use('/permission', desktopRestrict, permissionRouter) +router.use( + '/permission', + desktopRestrict, + authenticateAccessToken, + permissionRouter +) router.use( '/', diff --git a/api/src/routes/api/permission.ts b/api/src/routes/api/permission.ts index d354744..1cab853 100644 --- a/api/src/routes/api/permission.ts +++ b/api/src/routes/api/permission.ts @@ -1,10 +1,6 @@ import express from 'express' import { PermissionController } from '../../controllers/' -import { - authenticateAccessToken, - verifyAdmin, - authorize -} from '../../middlewares' +import { verifyAdmin } from '../../middlewares' import { registerPermissionValidation, updatePermissionValidation @@ -13,65 +9,49 @@ import { const permissionRouter = express.Router() const controller = new PermissionController() -permissionRouter.get( - '/', - authenticateAccessToken, - authorize, - async (req, res) => { - try { - const response = await controller.getAllPermissions() - res.send(response) - } catch (err: any) { - const statusCode = err.code - delete err.code - res.status(statusCode).send(err.message) - } +permissionRouter.get('/', async (req, res) => { + try { + const response = await controller.getAllPermissions() + res.send(response) + } catch (err: any) { + const statusCode = err.code + delete err.code + res.status(statusCode).send(err.message) } -) +}) -permissionRouter.post( - '/', - authenticateAccessToken, - verifyAdmin, - async (req, res) => { - const { error, value: body } = registerPermissionValidation(req.body) - if (error) return res.status(400).send(error.details[0].message) +permissionRouter.post('/', verifyAdmin, async (req, res) => { + const { error, value: body } = registerPermissionValidation(req.body) + if (error) return res.status(400).send(error.details[0].message) - try { - const response = await controller.createPermission(body) - res.send(response) - } catch (err: any) { - const statusCode = err.code - delete err.code - res.status(statusCode).send(err.message) - } + try { + const response = await controller.createPermission(body) + res.send(response) + } catch (err: any) { + const statusCode = err.code + delete err.code + res.status(statusCode).send(err.message) } -) +}) -permissionRouter.patch( - '/:permissionId', - authenticateAccessToken, - verifyAdmin, - async (req: any, res) => { - const { permissionId } = req.params +permissionRouter.patch('/:permissionId', verifyAdmin, async (req: any, res) => { + const { permissionId } = req.params - const { error, value: body } = updatePermissionValidation(req.body) - if (error) return res.status(400).send(error.details[0].message) + const { error, value: body } = updatePermissionValidation(req.body) + if (error) return res.status(400).send(error.details[0].message) - try { - const response = await controller.updatePermission(permissionId, body) - res.send(response) - } catch (err: any) { - const statusCode = err.code - delete err.code - res.status(statusCode).send(err.message) - } + try { + const response = await controller.updatePermission(permissionId, body) + res.send(response) + } catch (err: any) { + const statusCode = err.code + delete err.code + res.status(statusCode).send(err.message) } -) +}) permissionRouter.delete( '/:permissionId', - authenticateAccessToken, verifyAdmin, async (req: any, res) => { const { permissionId } = req.params diff --git a/api/src/routes/api/spec/permission.spec.ts b/api/src/routes/api/spec/permission.spec.ts index ecd3587..2c66bab 100644 --- a/api/src/routes/api/spec/permission.spec.ts +++ b/api/src/routes/api/spec/permission.spec.ts @@ -150,6 +150,19 @@ describe('permission', () => { expect(res.body).toEqual({}) }) + it('should respond with Bad Request if uri is not valid', async () => { + const res = await request(app) + .post('/SASjsApi/permission') + .auth(adminAccessToken, { type: 'bearer' }) + .send({ + ...permission, + uri: '/some/random/api/endpoint' + }) + .expect(400) + + expect(res.body).toEqual({}) + }) + it('should respond with Bad Request if setting is missing', async () => { const res = await request(app) .post('/SASjsApi/permission') diff --git a/api/src/routes/api/stp.ts b/api/src/routes/api/stp.ts index 14a3dea..858feb5 100644 --- a/api/src/routes/api/stp.ts +++ b/api/src/routes/api/stp.ts @@ -2,14 +2,13 @@ import express from 'express' import { executeProgramRawValidation } from '../../utils' import { STPController } from '../../controllers/' import { FileUploadController } from '../../controllers/internal' -import { authorize } from '../../middlewares' const stpRouter = express.Router() const fileUploadController = new FileUploadController() const controller = new STPController() -stpRouter.get('/execute', authorize, async (req, res) => { +stpRouter.get('/execute', async (req, res) => { const { error, value: query } = executeProgramRawValidation(req.query) if (error) return res.status(400).send(error.details[0].message) @@ -33,7 +32,6 @@ stpRouter.get('/execute', authorize, async (req, res) => { stpRouter.post( '/execute', - authorize, fileUploadController.preUploadMiddleware, fileUploadController.getMulterUploadObject().any(), async (req, res: any) => { diff --git a/api/src/utils/appStreamConfig.ts b/api/src/utils/appStreamConfig.ts index c293b35..f4f137d 100644 --- a/api/src/utils/appStreamConfig.ts +++ b/api/src/utils/appStreamConfig.ts @@ -5,6 +5,8 @@ import { AppStreamConfig } from '../types' import { getAppStreamConfigPath } from './file' export const loadAppStreamConfig = async () => { + process.appStreamConfig = {} + if (process.env.NODE_ENV === 'test') return const appStreamConfigPath = getAppStreamConfigPath() @@ -21,7 +23,6 @@ export const loadAppStreamConfig = async () => { } catch (_) { appStreamConfig = {} } - process.appStreamConfig = {} for (const [streamServiceName, entry] of Object.entries(appStreamConfig)) { const { appLoc, streamWebFolder, streamLogo } = entry