From cd3ba1bc609c2f2946bfbc7ee2fccf3483eb71fb Mon Sep 17 00:00:00 2001 From: Mark Moffat Date: Sun, 23 Feb 2020 14:10:35 +1030 Subject: [PATCH] Fixes for CSRF --- package-lock.json | 55 +++++++++++++++++++++++++++++++++++ package.json | 1 + public/javascripts/admin.js | 6 ++++ routes/admin.js | 58 +++++++++++++++++++++++++------------ test/helper.js | 5 ++++ test/specs/discounts.js | 4 +++ views/layouts/layout.hbs | 1 + 7 files changed, 111 insertions(+), 19 deletions(-) diff --git a/package-lock.json b/package-lock.json index 45573db..3a39c1a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2508,6 +2508,16 @@ "integrity": "sha1-ojD2T1aDEOFJgAmUB5DsmVRbyn4=", "dev": true }, + "csrf": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz", + "integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==", + "requires": { + "rndm": "1.2.0", + "tsscmp": "1.0.6", + "uid-safe": "2.1.5" + } + }, "css-select": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/css-select/-/css-select-1.2.0.tgz", @@ -2524,6 +2534,41 @@ "resolved": "https://registry.npmjs.org/css-what/-/css-what-2.1.0.tgz", "integrity": "sha1-lGfQMsOM+u+58teVASUwYvh/ob0=" }, + "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": { + "cookie": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.0.tgz", + "integrity": "sha512-+Hp8fLp57wnUSt0tY0tHEXh4voZRDnoIrZPqlo3DPiI4y9lwg/jqx+1Om94/W6ZaPDOUbnjOt/99w66zk+l1Xg==" + }, + "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", + "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==" + } + } + }, "currently-unhandled": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/currently-unhandled/-/currently-unhandled-0.4.1.tgz", @@ -9667,6 +9712,11 @@ "glob": "7.1.5" } }, + "rndm": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz", + "integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w=" + }, "run-async": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/run-async/-/run-async-2.3.0.tgz", @@ -10758,6 +10808,11 @@ "integrity": "sha512-qOebF53frne81cf0S9B41ByenJ3/IuH8yJKngAX35CmiZySA0khhkovshKK+jGCaMnVomla7gVlIcc3EvKPbTQ==", "dev": true }, + "tsscmp": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", + "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==" + }, "tunnel-agent": { "version": "0.6.0", "resolved": "https://registry.npmjs.org/tunnel-agent/-/tunnel-agent-0.6.0.tgz", diff --git a/package.json b/package.json index 0d286f5..08e0542 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "connect-mongodb-session": "^2.2.0", "cookie-parser": "^1.4.4", "countries-list": "^2.5.0", + "csurf": "^1.11.0", "dotenv": "^8.2.0", "express": "^4.17.1", "express-handlebars": "^3.1.0", diff --git a/public/javascripts/admin.js b/public/javascripts/admin.js index 2ad5d8d..3499903 100644 --- a/public/javascripts/admin.js +++ b/public/javascripts/admin.js @@ -1,6 +1,12 @@ /* eslint-disable prefer-arrow-callback, no-var, no-tabs */ /* globals showNotification, slugify, numeral, moment, feather */ $(document).ready(function (){ + $.ajaxSetup({ + headers: { + 'csrf-token': $('meta[name="csrfToken"]').attr('content') + } + }); + $(document).on('click', '#btnGenerateAPIkey', function(e){ e.preventDefault(); $.ajax({ diff --git a/routes/admin.js b/routes/admin.js index 16fccc4..d8df3d7 100644 --- a/routes/admin.js +++ b/routes/admin.js @@ -9,9 +9,11 @@ const fs = require('fs'); const path = require('path'); const multer = require('multer'); const mime = require('mime-type/with-db'); +const csrf = require('csurf'); const { validateJson } = require('../lib/schema'); const ObjectId = require('mongodb').ObjectID; const router = express.Router(); +const csrfProtection = csrf({ cookie: true }); // Regex const emailRegex = /\S+@\S+\.\S+/; @@ -30,6 +32,15 @@ router.get('/admin/logout', (req, res) => { res.redirect('/'); }); +// Used for tests only +if(process.env.NODE_ENV === 'test'){ + router.get('/admin/csrf', csrfProtection, (req, res, next) => { + res.json({ + csrf: req.csrfToken() + }); + }); +} + // login form router.get('/admin/login', async (req, res) => { const db = req.app.db; @@ -134,7 +145,7 @@ router.post('/admin/setup_action', async (req, res) => { }); // dashboard -router.get('/admin/dashboard', restrict, async (req, res) => { +router.get('/admin/dashboard', csrfProtection, restrict, async (req, res) => { const db = req.app.db; // Collate data for dashboard @@ -183,12 +194,13 @@ router.get('/admin/dashboard', restrict, async (req, res) => { message: common.clearSessionValue(req.session, 'message'), messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, - config: req.app.config + config: req.app.config, + csrfToken: req.csrfToken() }); }); // settings -router.get('/admin/settings', restrict, (req, res) => { +router.get('/admin/settings', csrfProtection, restrict, (req, res) => { res.render('settings', { title: 'Cart settings', session: req.session, @@ -199,7 +211,8 @@ router.get('/admin/settings', restrict, (req, res) => { helpers: req.handlebars.helpers, config: req.app.config, footerHtml: typeof req.app.config.footerHtml !== 'undefined' ? escape.decode(req.app.config.footerHtml) : null, - googleAnalytics: typeof req.app.config.googleAnalytics !== 'undefined' ? escape.decode(req.app.config.googleAnalytics) : null + googleAnalytics: typeof req.app.config.googleAnalytics !== 'undefined' ? escape.decode(req.app.config.googleAnalytics) : null, + csrfToken: req.csrfToken() }); }); @@ -236,7 +249,7 @@ router.post('/admin/settings/update', restrict, checkAccess, (req, res) => { }); // settings menu -router.get('/admin/settings/menu', restrict, async (req, res) => { +router.get('/admin/settings/menu', csrfProtection, restrict, async (req, res) => { const db = req.app.db; res.render('settings-menu', { title: 'Cart menu', @@ -246,12 +259,13 @@ router.get('/admin/settings/menu', restrict, async (req, res) => { messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, config: req.app.config, - menu: common.sortMenu(await common.getMenu(db)) + menu: common.sortMenu(await common.getMenu(db)), + csrfToken: req.csrfToken() }); }); // page list -router.get('/admin/settings/pages', restrict, async (req, res) => { +router.get('/admin/settings/pages', csrfProtection, restrict, async (req, res) => { const db = req.app.db; const pages = await db.pages.find({}).toArray(); @@ -264,12 +278,13 @@ router.get('/admin/settings/pages', restrict, async (req, res) => { messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, config: req.app.config, - menu: common.sortMenu(await common.getMenu(db)) + menu: common.sortMenu(await common.getMenu(db)), + csrfToken: req.csrfToken() }); }); // pages new -router.get('/admin/settings/pages/new', restrict, checkAccess, async (req, res) => { +router.get('/admin/settings/pages/new', csrfProtection, restrict, checkAccess, async (req, res) => { const db = req.app.db; res.render('settings-page', { @@ -281,12 +296,13 @@ router.get('/admin/settings/pages/new', restrict, checkAccess, async (req, res) messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, config: req.app.config, - menu: common.sortMenu(await common.getMenu(db)) + menu: common.sortMenu(await common.getMenu(db)), + csrfToken: req.csrfToken() }); }); // pages editor -router.get('/admin/settings/pages/edit/:page', restrict, checkAccess, async (req, res) => { +router.get('/admin/settings/pages/edit/:page', csrfProtection, restrict, checkAccess, async (req, res) => { const db = req.app.db; const page = await db.pages.findOne({ _id: common.getId(req.params.page) }); const menu = common.sortMenu(await common.getMenu(db)); @@ -312,7 +328,8 @@ router.get('/admin/settings/pages/edit/:page', restrict, checkAccess, async (req messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, config: req.app.config, - menu + menu, + csrfToken: req.csrfToken() }); }); @@ -434,7 +451,7 @@ router.post('/admin/validatePermalink', async (req, res) => { }); // Discount codes -router.get('/admin/settings/discounts', restrict, checkAccess, async (req, res) => { +router.get('/admin/settings/discounts', csrfProtection, restrict, checkAccess, async (req, res) => { const db = req.app.db; const discounts = await db.discounts.find({}).toArray(); @@ -447,12 +464,13 @@ router.get('/admin/settings/discounts', restrict, checkAccess, async (req, res) admin: true, message: common.clearSessionValue(req.session, 'message'), messageType: common.clearSessionValue(req.session, 'messageType'), - helpers: req.handlebars.helpers + helpers: req.handlebars.helpers, + csrfToken: req.csrfToken() }); }); // Edit a discount code -router.get('/admin/settings/discount/edit/:id', restrict, checkAccess, async (req, res) => { +router.get('/admin/settings/discount/edit/:id', csrfProtection, restrict, checkAccess, async (req, res) => { const db = req.app.db; const discount = await db.discounts.findOne({ _id: common.getId(req.params.id) }); @@ -465,7 +483,8 @@ router.get('/admin/settings/discount/edit/:id', restrict, checkAccess, async (re message: common.clearSessionValue(req.session, 'message'), messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, - config: req.app.config + config: req.app.config, + csrfToken: req.csrfToken() }); }); @@ -524,7 +543,7 @@ router.post('/admin/settings/discount/update', restrict, checkAccess, async (req }); // Create a discount code -router.get('/admin/settings/discount/new', restrict, checkAccess, async (req, res) => { +router.get('/admin/settings/discount/new', csrfProtection, restrict, checkAccess, async (req, res) => { res.render('settings-discount-new', { title: 'Discount code create', session: req.session, @@ -532,12 +551,13 @@ router.get('/admin/settings/discount/new', restrict, checkAccess, async (req, re message: common.clearSessionValue(req.session, 'message'), messageType: common.clearSessionValue(req.session, 'messageType'), helpers: req.handlebars.helpers, - config: req.app.config + config: req.app.config, + csrfToken: req.csrfToken() }); }); // Create a discount code -router.post('/admin/settings/discount/create', restrict, checkAccess, async (req, res) => { +router.post('/admin/settings/discount/create', csrfProtection, restrict, checkAccess, async (req, res) => { const db = req.app.db; // Doc to insert diff --git a/test/helper.js b/test/helper.js index ade5146..717cd79 100644 --- a/test/helper.js +++ b/test/helper.js @@ -76,6 +76,11 @@ const runBefore = async () => { await g.db.orders.insertOne(order); }); + // Get csrf token + const csrf = await g.request + .get('/admin/csrf'); + g.csrf = csrf.body.csrf; + // Index everything await runIndexing(app); diff --git a/test/specs/discounts.js b/test/specs/discounts.js index e89bcc3..6f9d72b 100644 --- a/test/specs/discounts.js +++ b/test/specs/discounts.js @@ -139,6 +139,7 @@ test('[Fail] Add a bogus code', async t => { .send({ discountCode: 'some_bogus_code' }) + .set('csrf-token', g.csrf) .expect(400); t.deepEqual(res.body.message, 'Discount code is invalid or expired'); @@ -156,6 +157,7 @@ test('[Success] Create a new discount code', async t => { end: moment().add(7, 'days').format('DD/MM/YYYY HH:mm') }) .set('apiKey', g.users[0].apiKey) + .set('csrf-token', g.csrf) .expect(200); t.deepEqual(res.body.message, 'Discount code created successfully'); @@ -173,6 +175,7 @@ test('[Fail] Create a new discount code with invalid type', async t => { end: moment().add(7, 'days').format('DD/MM/YYYY HH:mm') }) .set('apiKey', g.users[0].apiKey) + .set('csrf-token', g.csrf) .expect(400); t.deepEqual(res.body[0].message, 'should be equal to one of the allowed values'); @@ -190,6 +193,7 @@ test('[Fail] Create a new discount code with existing code', async t => { end: moment().add(7, 'days').format('DD/MM/YYYY HH:mm') }) .set('apiKey', g.users[0].apiKey) + .set('csrf-token', g.csrf) .expect(400); t.deepEqual(res.body.message, 'Discount code already exists'); diff --git a/views/layouts/layout.hbs b/views/layouts/layout.hbs index 507a735..cd80b03 100644 --- a/views/layouts/layout.hbs +++ b/views/layouts/layout.hbs @@ -22,6 +22,7 @@ {{/if}} {{/if}} +