diff --git a/api/package-lock.json b/api/package-lock.json index 592a288..9a0a131 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -14,7 +14,6 @@ "connect-mongo": "^4.6.0", "cookie-parser": "^1.4.6", "cors": "^2.8.5", - "csurf": "^1.11.0", "express": "^4.17.1", "express-session": "^1.17.2", "helmet": "^5.0.2", @@ -37,7 +36,6 @@ "@types/bcryptjs": "^2.4.2", "@types/cookie-parser": "^1.4.2", "@types/cors": "^2.8.12", - "@types/csurf": "^1.11.2", "@types/express": "^4.17.12", "@types/express-session": "^1.17.4", "@types/jest": "^26.0.24", @@ -50,6 +48,7 @@ "@types/swagger-ui-express": "^4.1.3", "@types/unzipper": "^0.10.5", "adm-zip": "^0.5.9", + "csrf": "^3.1.0", "dotenv": "^10.0.0", "http-headers-validation": "^0.0.1", "jest": "^27.0.6", @@ -1833,15 +1832,6 @@ "integrity": "sha512-vt+kDhq/M2ayberEtJcIN/hxXy1Pk+59g2FV/ZQceeaTyCtCucjL2Q7FXlFjtWn4n15KCr1NE2lNNFhp0lEThw==", "dev": true }, - "node_modules/@types/csurf": { - "version": "1.11.2", - "resolved": "https://registry.npmjs.org/@types/csurf/-/csurf-1.11.2.tgz", - "integrity": "sha512-9bc98EnwmC1S0aSJiA8rWwXtgXtXHHOQOsGHptImxFgqm6CeH+mIOunHRg6+/eg2tlmDMX3tY7XrWxo2M/nUNQ==", - "dev": true, - "dependencies": { - "@types/express-serve-static-core": "*" - } - }, "node_modules/@types/express": { "version": "4.17.12", "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.12.tgz", @@ -3336,6 +3326,7 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz", "integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==", + "dev": true, "dependencies": { "rndm": "1.2.0", "tsscmp": "1.0.6", @@ -3369,40 +3360,6 @@ "integrity": "sha512-b0tGHbfegbhPJpxpiBPU2sCkigAqtM9O121le6bbOlgyV+NyGyCmVfJ6QW9eRjz8CpNfWEOYBIMIGRYkLwsIYg==", "dev": true }, - "node_modules/csurf": { - "version": "1.11.0", - "resolved": "https://registry.npmjs.org/csurf/-/csurf-1.11.0.tgz", - "integrity": "sha512-UCtehyEExKTxgiu8UHdGvHj4tnpE/Qctue03Giq5gPgMQ9cg/ciod5blZQ5a4uCEenNQjxyGuzygLdKUmee/bQ==", - "dependencies": { - "cookie": "0.4.0", - "cookie-signature": "1.0.6", - "csrf": "3.1.0", - "http-errors": "~1.7.3" - }, - "engines": { - "node": ">= 0.8.0" - } - }, - "node_modules/csurf/node_modules/http-errors": { - "version": "1.7.3", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.7.3.tgz", - "integrity": "sha512-ZTTX0MWrsQ2ZAhA1cejAwDLycFsd7I7nVtnkT3Ol0aqodaKW+0CTZDQ1uBv5whptCnc8e8HeRRJxRs0kmm/Qfw==", - "dependencies": { - "depd": "~1.1.2", - "inherits": "2.0.4", - "setprototypeof": "1.1.1", - "statuses": ">= 1.5.0 < 2", - "toidentifier": "1.0.0" - }, - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/csurf/node_modules/inherits": { - "version": "2.0.4", - "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", - "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" - }, "node_modules/csv-stringify": { "version": "5.6.5", "resolved": "https://registry.npmjs.org/csv-stringify/-/csv-stringify-5.6.5.tgz", @@ -8377,7 +8334,8 @@ "node_modules/rndm": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz", - "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w=" + "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w=", + "dev": true }, "node_modules/rotating-file-stream": { "version": "3.0.4", @@ -9389,6 +9347,7 @@ "version": "1.0.6", "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==", + "dev": true, "engines": { "node": ">=0.6.x" } @@ -11361,15 +11320,6 @@ "integrity": "sha512-vt+kDhq/M2ayberEtJcIN/hxXy1Pk+59g2FV/ZQceeaTyCtCucjL2Q7FXlFjtWn4n15KCr1NE2lNNFhp0lEThw==", "dev": true }, - "@types/csurf": { - "version": "1.11.2", - "resolved": "https://registry.npmjs.org/@types/csurf/-/csurf-1.11.2.tgz", - "integrity": "sha512-9bc98EnwmC1S0aSJiA8rWwXtgXtXHHOQOsGHptImxFgqm6CeH+mIOunHRg6+/eg2tlmDMX3tY7XrWxo2M/nUNQ==", - "dev": true, - "requires": { - "@types/express-serve-static-core": "*" - } - }, "@types/express": { "version": "4.17.12", "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.12.tgz", @@ -12584,6 +12534,7 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz", "integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==", + "dev": true, "requires": { "rndm": "1.2.0", "tsscmp": "1.0.6", @@ -12613,36 +12564,6 @@ } } }, - "csurf": { - "version": "1.11.0", - "resolved": "https://registry.npmjs.org/csurf/-/csurf-1.11.0.tgz", - "integrity": "sha512-UCtehyEExKTxgiu8UHdGvHj4tnpE/Qctue03Giq5gPgMQ9cg/ciod5blZQ5a4uCEenNQjxyGuzygLdKUmee/bQ==", - "requires": { - "cookie": "0.4.0", - "cookie-signature": "1.0.6", - "csrf": "3.1.0", - "http-errors": "~1.7.3" - }, - "dependencies": { - "http-errors": { - "version": "1.7.3", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.7.3.tgz", - "integrity": "sha512-ZTTX0MWrsQ2ZAhA1cejAwDLycFsd7I7nVtnkT3Ol0aqodaKW+0CTZDQ1uBv5whptCnc8e8HeRRJxRs0kmm/Qfw==", - "requires": { - "depd": "~1.1.2", - "inherits": "2.0.4", - "setprototypeof": "1.1.1", - "statuses": ">= 1.5.0 < 2", - "toidentifier": "1.0.0" - } - }, - "inherits": { - "version": "2.0.4", - "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", - "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" - } - } - }, "csv-stringify": { "version": "5.6.5", "resolved": "https://registry.npmjs.org/csv-stringify/-/csv-stringify-5.6.5.tgz", @@ -16379,7 +16300,8 @@ "rndm": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz", - "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w=" + "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w=", + "dev": true }, "rotating-file-stream": { "version": "3.0.4", @@ -17134,7 +17056,8 @@ "tsscmp": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", - "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==" + "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==", + "dev": true }, "tunnel-agent": { "version": "0.6.0", diff --git a/api/package.json b/api/package.json index ea5b9ff..56165ed 100644 --- a/api/package.json +++ b/api/package.json @@ -53,7 +53,6 @@ "connect-mongo": "^4.6.0", "cookie-parser": "^1.4.6", "cors": "^2.8.5", - "csurf": "^1.11.0", "express": "^4.17.1", "express-session": "^1.17.2", "helmet": "^5.0.2", @@ -73,7 +72,6 @@ "@types/bcryptjs": "^2.4.2", "@types/cookie-parser": "^1.4.2", "@types/cors": "^2.8.12", - "@types/csurf": "^1.11.2", "@types/express": "^4.17.12", "@types/express-session": "^1.17.4", "@types/jest": "^26.0.24", @@ -86,6 +84,7 @@ "@types/swagger-ui-express": "^4.1.3", "@types/unzipper": "^0.10.5", "adm-zip": "^0.5.9", + "csrf": "^3.1.0", "dotenv": "^10.0.0", "http-headers-validation": "^0.0.1", "jest": "^27.0.6", diff --git a/api/src/app.ts b/api/src/app.ts index 843a254..366c5ca 100644 --- a/api/src/app.ts +++ b/api/src/app.ts @@ -1,6 +1,5 @@ import path from 'path' -import express, { ErrorRequestHandler } from 'express' -import csrf, { CookieOptions } from 'csurf' +import express, { ErrorRequestHandler, CookieOptions } from 'express' import cookieParser from 'cookie-parser' import dotenv from 'dotenv' @@ -39,15 +38,7 @@ export const cookieOptions: CookieOptions = { maxAge: 24 * 60 * 60 * 1000 // 24 hours } -/*********************************** - * CSRF Protection * - ***********************************/ -export const csrfProtection = csrf({ cookie: cookieOptions }) - const onError: ErrorRequestHandler = (err, req, res, next) => { - if (err.code === 'EBADCSRFTOKEN') - return res.status(400).send('Invalid CSRF token!') - console.error(err.stack) res.status(500).send('Something broke!') } diff --git a/api/src/middlewares/authenticateToken.ts b/api/src/middlewares/authenticateToken.ts index 78e4bcb..901c6e7 100644 --- a/api/src/middlewares/authenticateToken.ts +++ b/api/src/middlewares/authenticateToken.ts @@ -1,6 +1,6 @@ import { RequestHandler, Request, Response, NextFunction } from 'express' import jwt from 'jsonwebtoken' -import { csrfProtection } from '../app' +import { csrfProtection } from './' import { fetchLatestAutoExec, ModeType, diff --git a/api/src/middlewares/authorize.ts b/api/src/middlewares/authorize.ts index 6b389fb..d27a578 100644 --- a/api/src/middlewares/authorize.ts +++ b/api/src/middlewares/authorize.ts @@ -10,9 +10,7 @@ import { getPath, isPublicRoute } from '../utils' export const authorize: RequestHandler = async (req, res, next) => { const { user } = req - if (!user) { - return res.sendStatus(401) - } + if (!user) return res.sendStatus(401) // no need to check for permissions when user is admin if (user.isAdmin) return next() diff --git a/api/src/middlewares/csrfProtection.ts b/api/src/middlewares/csrfProtection.ts new file mode 100644 index 0000000..55fd8ff --- /dev/null +++ b/api/src/middlewares/csrfProtection.ts @@ -0,0 +1,32 @@ +import { RequestHandler } from 'express' +import csrf from 'csrf' + +const csrfTokens = new csrf() +const secret = csrfTokens.secretSync() + +export const generateCSRFToken = () => csrfTokens.create(secret) + +export const csrfProtection: RequestHandler = (req, res, next) => { + if (req.method === 'GET') return next() + + // The default value is a function that reads the token from the following locations, in order: + // req.body._csrf - typically generated by the body-parser module. + // req.query._csrf - a built-in from Express.js to read from the URL query string. + // req.headers['csrf-token'] - the CSRF-Token HTTP request header. + // req.headers['xsrf-token'] - the XSRF-Token HTTP request header. + // req.headers['x-csrf-token'] - the X-CSRF-Token HTTP request header. + // req.headers['x-xsrf-token'] - the X-XSRF-Token HTTP request header. + + const token = + req.body?._csrf || + req.query?._csrf || + req.headers['csrf-token'] || + req.headers['xsrf-token'] || + req.headers['x-csrf-token'] || + req.headers['x-xsrf-token'] + + if (!csrfTokens.verify(secret, token)) { + return res.status(400).send('Invalid CSRF token!') + } + next() +} diff --git a/api/src/middlewares/index.ts b/api/src/middlewares/index.ts index 8e64643..209bda6 100644 --- a/api/src/middlewares/index.ts +++ b/api/src/middlewares/index.ts @@ -1,5 +1,6 @@ export * from './authenticateToken' +export * from './authorize' +export * from './csrfProtection' export * from './desktop' export * from './verifyAdmin' export * from './verifyAdminIfNeeded' -export * from './authorize' diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index 12bc4db..eebee67 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/api/src/routes/api/spec/web.spec.ts @@ -49,10 +49,9 @@ describe('web', () => { describe('SASLogon/login', () => { let csrfToken: string - let cookies: string beforeAll(async () => { - ;({ csrfToken, cookies } = await getCSRF(app)) + ;({ csrfToken } = await getCSRF(app)) }) afterEach(async () => { @@ -66,7 +65,6 @@ describe('web', () => { const res = await request(app) .post('/SASLogon/login') - .set('Cookie', cookies) .set('x-xsrf-token', csrfToken) .send({ username: user.username, @@ -82,15 +80,45 @@ describe('web', () => { isAdmin: user.isAdmin }) }) + + it('should respond with Bad Request if CSRF Token is not present', async () => { + await userController.createUser(user) + + const res = await request(app) + .post('/SASLogon/login') + .send({ + username: user.username, + password: user.password + }) + .expect(400) + + expect(res.text).toEqual('Invalid CSRF token!') + expect(res.body).toEqual({}) + }) + + it('should respond with Bad Request if CSRF Token is invalid', async () => { + await userController.createUser(user) + + const res = await request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', 'INVALID_CSRF_TOKEN') + .send({ + username: user.username, + password: user.password + }) + .expect(400) + + expect(res.text).toEqual('Invalid CSRF token!') + expect(res.body).toEqual({}) + }) }) describe('SASLogon/authorize', () => { let csrfToken: string - let cookies: string let authCookies: string beforeAll(async () => { - ;({ csrfToken, cookies } = await getCSRF(app)) + ;({ csrfToken } = await getCSRF(app)) await userController.createUser(user) @@ -99,12 +127,7 @@ describe('web', () => { password: user.password } - ;({ cookies: authCookies } = await performLogin( - app, - credentials, - cookies, - csrfToken - )) + ;({ authCookies } = await performLogin(app, credentials, csrfToken)) }) afterAll(async () => { @@ -116,17 +139,28 @@ describe('web', () => { it('should respond with authorization code', async () => { const res = await request(app) .post('/SASLogon/authorize') - .set('Cookie', [authCookies, cookies].join('; ')) + .set('Cookie', [authCookies].join('; ')) .set('x-xsrf-token', csrfToken) .send({ clientId }) expect(res.body).toHaveProperty('code') }) + it('should respond with Bad Request if CSRF Token is missing', async () => { + const res = await request(app) + .post('/SASLogon/authorize') + .set('Cookie', [authCookies].join('; ')) + .send({ clientId }) + .expect(400) + + expect(res.text).toEqual('Invalid CSRF token!') + expect(res.body).toEqual({}) + }) + it('should respond with Bad Request if clientId is missing', async () => { const res = await request(app) .post('/SASLogon/authorize') - .set('Cookie', [authCookies, cookies].join('; ')) + .set('Cookie', [authCookies].join('; ')) .set('x-xsrf-token', csrfToken) .send({}) .expect(400) @@ -138,7 +172,7 @@ describe('web', () => { it('should respond with Forbidden if clientId is incorrect', async () => { const res = await request(app) .post('/SASLogon/authorize') - .set('Cookie', [authCookies, cookies].join('; ')) + .set('Cookie', [authCookies].join('; ')) .set('x-xsrf-token', csrfToken) .send({ clientId: 'WrongClientID' @@ -153,27 +187,22 @@ describe('web', () => { const getCSRF = async (app: Express) => { // make request to get CSRF - const { header, text } = await request(app).get('/') - const cookies = header['set-cookie'].join() + const { text } = await request(app).get('/') - const csrfToken = extractCSRF(text) - return { csrfToken, cookies } + return { csrfToken: extractCSRF(text) } } const performLogin = async ( app: Express, credentials: { username: string; password: string }, - cookies: string, csrfToken: string ) => { const { header } = await request(app) .post('/SASLogon/login') - .set('Cookie', cookies) .set('x-xsrf-token', csrfToken) .send(credentials) - const newCookies: string = header['set-cookie'].join() - return { cookies: newCookies } + return { authCookies: header['set-cookie'].join() } } const extractCSRF = (text: string) => diff --git a/api/src/routes/appStream/index.ts b/api/src/routes/appStream/index.ts index ca2e279..1c397d5 100644 --- a/api/src/routes/appStream/index.ts +++ b/api/src/routes/appStream/index.ts @@ -1,6 +1,6 @@ import path from 'path' import express, { Request } from 'express' -import { authenticateAccessToken } from '../../middlewares' +import { authenticateAccessToken, generateCSRFToken } from '../../middlewares' import { folderExists } from '@sasjs/utils' import { addEntryToAppStreamConfig, getFilesFolder } from '../../utils' @@ -13,7 +13,7 @@ const router = express.Router() router.get('/', authenticateAccessToken, async (req, res) => { const content = appStreamHtml(process.appStreamConfig) - res.cookie('XSRF-TOKEN', req.csrfToken()) + res.cookie('XSRF-TOKEN', generateCSRFToken()) return res.send(content) }) diff --git a/api/src/routes/setupRoutes.ts b/api/src/routes/setupRoutes.ts index d7a7db9..a31aae6 100644 --- a/api/src/routes/setupRoutes.ts +++ b/api/src/routes/setupRoutes.ts @@ -4,7 +4,7 @@ import webRouter from './web' import apiRouter from './api' import appStreamRouter from './appStream' -import { csrfProtection } from '../app' +import { csrfProtection } from '../middlewares' export const setupRoutes = (app: Express) => { app.use('/SASjsApi', apiRouter) diff --git a/api/src/routes/web/sas9-web.ts b/api/src/routes/web/sas9-web.ts index 0efc6f3..f48577a 100644 --- a/api/src/routes/web/sas9-web.ts +++ b/api/src/routes/web/sas9-web.ts @@ -1,4 +1,5 @@ import express from 'express' +import { generateCSRFToken } from '../../middlewares' import { WebController } from '../../controllers' import { MockSas9Controller } from '../../controllers/mock-sas9' @@ -15,7 +16,7 @@ sas9WebRouter.get('/', async (req, res) => { } catch (_) { response = 'Web Build is not present' } finally { - const codeToInject = `` + const codeToInject = `` const injectedContent = response?.replace( '', `${codeToInject}` diff --git a/api/src/routes/web/sasviya-web.ts b/api/src/routes/web/sasviya-web.ts index 9809319..6cb0fc9 100644 --- a/api/src/routes/web/sasviya-web.ts +++ b/api/src/routes/web/sasviya-web.ts @@ -1,4 +1,5 @@ import express from 'express' +import { generateCSRFToken } from '../../middlewares' import { WebController } from '../../controllers/web' const sasViyaWebRouter = express.Router() @@ -11,7 +12,7 @@ sasViyaWebRouter.get('/', async (req, res) => { } catch (_) { response = 'Web Build is not present' } finally { - const codeToInject = `` + const codeToInject = `` const injectedContent = response?.replace( '', `${codeToInject}` diff --git a/api/src/routes/web/web.ts b/api/src/routes/web/web.ts index 03510b5..7b685a0 100644 --- a/api/src/routes/web/web.ts +++ b/api/src/routes/web/web.ts @@ -1,4 +1,5 @@ import express from 'express' +import { generateCSRFToken } from '../../middlewares' import { WebController } from '../../controllers/web' import { authenticateAccessToken, desktopRestrict } from '../../middlewares' import { authorizeValidation, loginWebValidation } from '../../utils' @@ -13,7 +14,7 @@ webRouter.get('/', async (req, res) => { } catch (_) { response = 'Web Build is not present' } finally { - const codeToInject = `` + const codeToInject = `` const injectedContent = response?.replace( '', `${codeToInject}`