Skip to content

Commit

Permalink
Fixes for CSRF
Browse files Browse the repository at this point in the history
  • Loading branch information
Mark Moffat authored and Mark Moffat committed Feb 23, 2020
1 parent 9a8ccb8 commit cd3ba1b
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 19 deletions.
55 changes: 55 additions & 0 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions public/javascripts/admin.js
View file
Original file line number Diff line number Diff line change
@@ -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({
Expand Down
58 changes: 39 additions & 19 deletions routes/admin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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+/;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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()
});
});

Expand Down Expand Up @@ -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',
Expand All @@ -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();

Expand All @@ -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', {
Expand All @@ -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));
Expand All @@ -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()
});
});

Expand Down Expand Up @@ -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();
Expand All @@ -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) });
Expand All @@ -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()
});
});

Expand Down Expand Up @@ -524,20 +543,21 @@ 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,
admin: true,
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
Expand Down
5 changes: 5 additions & 0 deletions test/helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions test/specs/discounts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand Down
1 change: 1 addition & 0 deletions views/layouts/layout.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<meta name="description" content="{{snip config.cartDescription}}">
{{/if}}
{{/if}}
<meta name="csrfToken" content="{{csrfToken}}">
<meta name="keywords" content="{{config.cartTitle}}">
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.4.1/css/bootstrap.min.css" integrity="sha256-L/W5Wfqfa0sdBNIKN9cG6QA5F2qx4qICmU2VgLruv9Y=" crossorigin="anonymous" />
<link rel="stylesheet" href="/stylesheets/pushy{{config.env}}.css">
Expand Down

0 comments on commit cd3ba1b

Please sign in to comment.