From deac12c540a1f1bb8ed85171262618487d28c92b Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Tue, 18 Nov 2014 14:54:54 -0500 Subject: [PATCH] closed #2424 --- public/src/app.js | 16 ++++++++++++++-- public/src/client/login.js | 20 +++++++++++++++++--- public/src/client/register.js | 21 +++++++++++++++++++-- public/src/modules/composer/uploads.js | 21 +++++++-------------- public/src/modules/uploader.js | 7 +++++-- src/controllers/accounts.js | 1 - src/controllers/admin.js | 7 ++----- src/controllers/api.js | 1 + src/controllers/categories.js | 1 - src/controllers/index.js | 2 -- src/controllers/topics.js | 1 - src/middleware/admin.js | 1 - src/middleware/middleware.js | 1 - src/routes/admin.js | 9 +++++---- src/routes/api.js | 2 +- src/routes/authentication.js | 24 +++++++++++------------- src/routes/index.js | 18 +++++++++++------- src/views/admin/footer.tpl | 1 - src/views/admin/manage/categories.tpl | 2 -- src/views/admin/settings/footer.tpl | 1 - 20 files changed, 93 insertions(+), 64 deletions(-) diff --git a/public/src/app.js b/public/src/app.js index db3815073b..de2ac9557c 100644 --- a/public/src/app.js +++ b/public/src/app.js @@ -149,6 +149,10 @@ var socket, app.cacheBuster = config['cache-buster']; + require(['csrf'], function(csrf) { + csrf.set(data.csrf_token); + }); + bootbox.setDefaults({ locale: config.userLang }); @@ -164,8 +168,16 @@ var socket, }; app.logout = function() { - $.post(RELATIVE_PATH + '/logout', function() { - window.location.href = RELATIVE_PATH + '/'; + require(['csrf'], function(csrf) { + $.ajax(RELATIVE_PATH + '/logout', { + type: 'POST', + headers: { + 'x-csrf-token': csrf.get() + }, + success: function() { + window.location.href = RELATIVE_PATH + '/'; + } + }); }); }; diff --git a/public/src/client/login.js b/public/src/client/login.js index 23d7e3838f..2f0d9634cf 100644 --- a/public/src/client/login.js +++ b/public/src/client/login.js @@ -1,7 +1,7 @@ "use strict"; /* global define, app, RELATIVE_PATH */ -define('forum/login', function() { +define('forum/login', ['csrf'], function(csrf) { var Login = {}; Login.init = function() { @@ -14,7 +14,7 @@ define('forum/login', function() { if (!$('#username').val() || !$('#password').val()) { translator.translate('[[error:invalid-username-or-password]]', function(translated) { - errorEl.find('p').text(translated) + errorEl.find('p').text(translated); errorEl.show(); }); } else { @@ -22,7 +22,21 @@ define('forum/login', function() { if (!submitEl.hasClass('disabled')) { submitEl.addClass('disabled'); - formEl.submit(); + formEl.ajaxSubmit({ + headers: { + 'x-csrf-token': csrf.get() + }, + success: function(data, status) { + window.location.href = data; + }, + error: function(data, status) { + translator.translate(data.responseText, config.defaultLang, function(translated) { + errorEl.find('p').text(translated) + errorEl.show(); + submitEl.removeClass('disabled'); + }); + } + }); } } }); diff --git a/public/src/client/register.js b/public/src/client/register.js index 6780faae59..01b8846371 100644 --- a/public/src/client/register.js +++ b/public/src/client/register.js @@ -3,7 +3,7 @@ /* globals define, app, utils, socket, config */ -define('forum/register', function() { +define('forum/register', ['csrf'], function(csrf) { var Register = {}, validationError = false, successIcon = ''; @@ -176,7 +176,24 @@ define('forum/register', function() { e.preventDefault(); validateForm(function() { if (!validationError) { - registerBtn.parents('form').trigger('submit'); + registerBtn.addClass('disabled'); + + registerBtn.parents('form').ajaxSubmit({ + headers: { + 'x-csrf-token': csrf.get() + }, + success: function(data, status) { + window.location.href = data; + }, + error: function(data, status) { + var errorEl = $('#register-error-notify'); + translator.translate(data.responseText, config.defaultLang, function(translated) { + errorEl.find('p').text(translated) + errorEl.show(); + registerBtn.removeClass('disabled'); + }); + } + }); } }); }); diff --git a/public/src/modules/composer/uploads.js b/public/src/modules/composer/uploads.js index 46b0277bcd..e7bc473cb5 100644 --- a/public/src/modules/composer/uploads.js +++ b/public/src/modules/composer/uploads.js @@ -2,7 +2,7 @@ /* globals define, utils, config, app */ -define('composer/uploads', ['composer/preview'], function(preview) { +define('composer/uploads', ['composer/preview', 'csrf'], function(preview, csrf) { var uploads = { inProgress: {} }; @@ -235,16 +235,13 @@ define('composer/uploads', ['composer/preview'], function(preview) { textarea.val(current.replace(re, filename + '](' + text + ')')); } - $(this).find('#postUploadCsrf').val($('#csrf').attr('data-csrf')); - - if (formData) { - formData.append('_csrf', $('#csrf').attr('data-csrf')); - } - uploads.inProgress[post_uuid] = uploads.inProgress[post_uuid] || []; uploads.inProgress[post_uuid].push(1); $(this).ajaxSubmit({ + headers: { + 'x-csrf-token': csrf.get() + }, resetForm: true, clearForm: true, formData: formData, @@ -291,19 +288,15 @@ define('composer/uploads', ['composer/preview'], function(preview) { thumbForm.attr('action', config.relative_path + params.route); thumbForm.off('submit').submit(function() { - var csrf = $('#csrf').attr('data-csrf'); - $(this).find('#thumbUploadCsrf').val(csrf); - - if(formData) { - formData.append('_csrf', csrf); - } - spinner.removeClass('hide'); uploads.inProgress[post_uuid] = uploads.inProgress[post_uuid] || []; uploads.inProgress[post_uuid].push(1); $(this).ajaxSubmit({ + headers: { + 'x-csrf-token': csrf.get() + }, formData: formData, error: onUploadError, success: function(uploads) { diff --git a/public/src/modules/uploader.js b/public/src/modules/uploader.js index 47ba1c1e37..4156507683 100644 --- a/public/src/modules/uploader.js +++ b/public/src/modules/uploader.js @@ -1,4 +1,4 @@ -define('uploader', function() { +define('uploader', ['csrf'], function(csrf) { var module = {}, maybeParse = function(response) { @@ -20,7 +20,7 @@ define('uploader', function() { uploadForm[0].reset(); uploadForm.attr('action', route); uploadForm.find('#params').val(JSON.stringify(params)); - uploadForm.find('#csrfToken').val($('#csrf').attr('data-csrf')); + // uploadForm.find('#csrfToken').val(csrf.get()); if(fileSize) { uploadForm.find('#upload-file-size').html(fileSize); @@ -61,6 +61,9 @@ define('uploader', function() { } $(this).ajaxSubmit({ + headers: { + 'x-csrf-token': csrf.get() + }, error: function(xhr) { xhr = maybeParse(xhr); error('Error: ' + xhr.status); diff --git a/src/controllers/accounts.js b/src/controllers/accounts.js index 96d83a5a07..4c49330c79 100644 --- a/src/controllers/accounts.js +++ b/src/controllers/accounts.js @@ -345,7 +345,6 @@ accountsController.accountEdit = function(req, res, next) { } userData.hasPassword = !!password; - userData.csrf = req.csrfToken(); res.render('account/edit', userData); }); diff --git a/src/controllers/admin.js b/src/controllers/admin.js index 475dd60e77..52bb3b7f18 100644 --- a/src/controllers/admin.js +++ b/src/controllers/admin.js @@ -141,8 +141,7 @@ function filterAndRenderCategories(req, res, next, active) { }); res.render('admin/manage/categories', { - categories: categoryData, - csrf: req.csrfToken() + categories: categoryData }); }); } @@ -223,9 +222,7 @@ adminController.languages.get = function(req, res, next) { adminController.settings.get = function(req, res, next) { var term = req.params.term ? req.params.term : 'general'; - res.render('admin/settings/' + term, { - 'csrf': req.csrfToken() - }); + res.render('admin/settings/' + term); }; adminController.logger.get = function(req, res, next) { diff --git a/src/controllers/api.js b/src/controllers/api.js index 4a797b8645..e6c5d478c1 100644 --- a/src/controllers/api.js +++ b/src/controllers/api.js @@ -52,6 +52,7 @@ apiController.getConfig = function(req, res, next) { config['css-buster'] = meta.css.hash; config.requireEmailConfirmation = parseInt(meta.config.requireEmailConfirmation, 10) === 1; config.topicPostSort = meta.config.topicPostSort || 'oldest_to_newest'; + config.csrf_token = req.csrfToken(); if (!req.user) { if (res.locals.isAPI) { diff --git a/src/controllers/categories.js b/src/controllers/categories.js index d2bc7d57f7..a9928015d8 100644 --- a/src/controllers/categories.js +++ b/src/controllers/categories.js @@ -239,7 +239,6 @@ categoriesController.get = function(req, res, next) { data.currentPage = page; data['feeds:disableRSS'] = parseInt(meta.config['feeds:disableRSS'], 10) === 1; - data.csrf = req.csrfToken(); if (!res.locals.isAPI) { // Paginator for noscript diff --git a/src/controllers/index.js b/src/controllers/index.js index 0c2f3db763..5631170474 100644 --- a/src/controllers/index.js +++ b/src/controllers/index.js @@ -143,7 +143,6 @@ Controllers.login = function(req, res, next) { data.alternate_logins = loginStrategies.length > 0; data.authentication = loginStrategies; - data.token = req.csrfToken(); data.showResetLink = emailersPresent; data.allowLocalLogin = parseInt(meta.config.allowLocalLogin, 10) === 1; data.allowRegistration = parseInt(meta.config.allowRegistration, 10) === 1; @@ -174,7 +173,6 @@ Controllers.register = function(req, res, next) { data.authentication = loginStrategies; - data.token = req.csrfToken(); data.minimumUsernameLength = meta.config.minimumUsernameLength; data.maximumUsernameLength = meta.config.maximumUsernameLength; data.minimumPasswordLength = meta.config.minimumPasswordLength; diff --git a/src/controllers/topics.js b/src/controllers/topics.js index 6ecebc2364..6d21aacb68 100644 --- a/src/controllers/topics.js +++ b/src/controllers/topics.js @@ -239,7 +239,6 @@ topicsController.get = function(req, res, next) { data['reputation:disabled'] = parseInt(meta.config['reputation:disabled'], 10) === 1; data['downvote:disabled'] = parseInt(meta.config['downvote:disabled'], 10) === 1; data['feeds:disableRSS'] = parseInt(meta.config['feeds:disableRSS'], 10) === 1; - data.csrf = req.csrfToken(); topics.increaseViewCount(tid); diff --git a/src/middleware/admin.js b/src/middleware/admin.js index 673e87f895..8ac5125213 100644 --- a/src/middleware/admin.js +++ b/src/middleware/admin.js @@ -71,7 +71,6 @@ middleware.buildHeader = function(req, res, next) { return next(err); } var data = { - csrf: req.csrfToken ? req.csrfToken() : undefined, relative_path: nconf.get('relative_path'), plugins: pluginData.custom_header.plugins, authentication: pluginData.custom_header.authentication, diff --git a/src/middleware/middleware.js b/src/middleware/middleware.js index c17008893b..d5d19710e8 100644 --- a/src/middleware/middleware.js +++ b/src/middleware/middleware.js @@ -260,7 +260,6 @@ middleware.renderHeader = function(req, res, callback) { 'cache-buster': meta.config['cache-buster'] ? 'v=' + meta.config['cache-buster'] : '', 'brand:logo': meta.config['brand:logo'] || '', 'brand:logo:display': meta.config['brand:logo']?'':'hide', - csrf: req.csrfToken ? req.csrfToken() : undefined, navigation: custom_header.navigation, allowRegistration: meta.config.allowRegistration === undefined || parseInt(meta.config.allowRegistration, 10) === 1, searchEnabled: plugins.hasListeners('filter:search.query') diff --git a/src/routes/admin.js b/src/routes/admin.js index 6650f32f2a..ccda470788 100644 --- a/src/routes/admin.js +++ b/src/routes/admin.js @@ -21,6 +21,7 @@ function apiRoutes(app, middleware, controllers) { function adminRouter(middleware, controllers) { var router = express.Router(); + router.use(middleware.applyCSRF); router.use(middleware.admin.buildHeader); router.get('/', controllers.admin.home); @@ -45,9 +46,9 @@ function addRoutes(router, middleware, controllers) { router.get('/general/languages', controllers.admin.languages.get); router.get('/general/sounds', controllers.admin.sounds.get); - router.get('/manage/categories', middleware.applyCSRF, controllers.admin.categories.active); - router.get('/manage/categories/active', middleware.applyCSRF, controllers.admin.categories.active); - router.get('/manage/categories/disabled', middleware.applyCSRF, controllers.admin.categories.disabled); + router.get('/manage/categories', controllers.admin.categories.active); + router.get('/manage/categories/active', controllers.admin.categories.active); + router.get('/manage/categories/disabled', controllers.admin.categories.disabled); router.get('/manage/tags', controllers.admin.tags.get); @@ -62,7 +63,7 @@ function addRoutes(router, middleware, controllers) { router.get('/manage/groups', controllers.admin.groups.get); - router.get('/settings/:term?', middleware.applyCSRF, controllers.admin.settings.get); + router.get('/settings/:term?', controllers.admin.settings.get); router.get('/appearance/:term?', controllers.admin.appearance.get); diff --git a/src/routes/api.js b/src/routes/api.js index 05057e8b37..f41728e2b3 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -198,7 +198,7 @@ module.exports = function(app, middleware, controllers) { var router = express.Router(); app.use('/api', router); - router.get('/config', controllers.api.getConfig); + router.get('/config', middleware.applyCSRF, controllers.api.getConfig); router.get('/widgets/render', controllers.api.renderWidgets); router.get('/user/uid/:uid', middleware.checkGlobalPrivacySettings, controllers.accounts.getUserByUID); diff --git a/src/routes/authentication.js b/src/routes/authentication.js index ab6ef5abc4..f1ac98e4c4 100644 --- a/src/routes/authentication.js +++ b/src/routes/authentication.js @@ -56,7 +56,7 @@ })); }); - router.post('/logout', logout); + router.post('/logout', Auth.middleware.applyCSRF, logout); router.post('/register', Auth.middleware.applyCSRF, register); router.post('/login', Auth.middleware.applyCSRF, login); @@ -147,8 +147,7 @@ function continueLogin(req, res, next) { passport.authenticate('local', function(err, userData, info) { if (err) { - req.flash('error', err.message); - return res.redirect(nconf.get('relative_path') + '/login'); + return res.status(403).send(err.message); } if (!userData) { @@ -156,8 +155,7 @@ info = '[[error:invalid-username-or-password]]'; } - req.flash('error', info); - return res.redirect(nconf.get('relative_path') + '/login'); + return res.status(403).send(info); } // Alter user cookie depending on passed-in option @@ -174,8 +172,7 @@ uid: userData.uid }, function(err) { if (err) { - req.flash('error', err.message); - return res.redirect(nconf.get('relative_path') + '/login'); + return res.status(403).send(err.message); } if (userData.uid) { user.logIP(userData.uid, req.ip); @@ -184,11 +181,12 @@ } if (!req.session.returnTo) { - res.redirect(nconf.get('relative_path') + '/'); + res.status(200).send(nconf.get('relative_path') + '/'); } else { var next = req.session.returnTo; delete req.session.returnTo; - res.redirect(nconf.get('relative_path') + next); + + res.status(200).send(nconf.get('relative_path') + next); } }); })(req, res, next); @@ -196,7 +194,7 @@ function register(req, res) { if (parseInt(meta.config.allowRegistration, 10) === 0) { - return res.status(403).send(''); + return res.sendStatus(403); } var userData = {}; @@ -242,10 +240,10 @@ } ], function(err, data) { if (err) { - req.flash('error', err.message); - return res.redirect(nconf.get('relative_path') + '/register'); + return res.status(400).send(err.message); } - res.redirect(nconf.get('relative_path') + (data.referrer ? data.referrer : '/')); + + res.status(200).send(nconf.get('relative_path') + (data.referrer ? data.referrer : '/')); }); } diff --git a/src/routes/index.js b/src/routes/index.js index c538d2e4bf..6ca2a12e44 100644 --- a/src/routes/index.js +++ b/src/routes/index.js @@ -20,7 +20,7 @@ var nconf = require('nconf'), function mainRoutes(app, middleware, controllers) { setupPageRoute(app, '/', middleware, [], controllers.home); - var loginRegisterMiddleware = [middleware.applyCSRF, middleware.redirectToAccountIfLoggedIn]; + var loginRegisterMiddleware = [middleware.redirectToAccountIfLoggedIn]; setupPageRoute(app, '/login', middleware, loginRegisterMiddleware, controllers.login); setupPageRoute(app, '/register', middleware, loginRegisterMiddleware, controllers.register); @@ -40,8 +40,8 @@ function staticRoutes(app, middleware, controllers) { function topicRoutes(app, middleware, controllers) { app.get('/api/topic/teaser/:topic_id', controllers.topics.teaser); - setupPageRoute(app, '/topic/:topic_id/:slug/:post_index?', middleware, [middleware.applyCSRF], controllers.topics.get); - setupPageRoute(app, '/topic/:topic_id/:slug?', middleware, [middleware.applyCSRF, middleware.addSlug], controllers.topics.get); + setupPageRoute(app, '/topic/:topic_id/:slug/:post_index?', middleware, [], controllers.topics.get); + setupPageRoute(app, '/topic/:topic_id/:slug?', middleware, [middleware.addSlug], controllers.topics.get); } function tagRoutes(app, middleware, controllers) { @@ -55,8 +55,8 @@ function categoryRoutes(app, middleware, controllers) { setupPageRoute(app, '/unread', middleware, [middleware.authenticate], controllers.categories.unread); app.get('/api/unread/total', middleware.authenticate, controllers.categories.unreadTotal); - setupPageRoute(app, '/category/:category_id/:slug/:topic_index', middleware, [middleware.applyCSRF], controllers.categories.get); - setupPageRoute(app, '/category/:category_id/:slug?', middleware, [middleware.applyCSRF, middleware.addSlug], controllers.categories.get); + setupPageRoute(app, '/category/:category_id/:slug/:topic_index', middleware, [], controllers.categories.get); + setupPageRoute(app, '/category/:category_id/:slug?', middleware, [middleware.addSlug], controllers.categories.get); } function accountRoutes(app, middleware, controllers) { @@ -70,7 +70,7 @@ function accountRoutes(app, middleware, controllers) { setupPageRoute(app, '/user/:userslug/topics', middleware, middlewares, controllers.accounts.getTopics); setupPageRoute(app, '/user/:userslug/favourites', middleware, accountMiddlewares, controllers.accounts.getFavourites); - setupPageRoute(app, '/user/:userslug/edit', middleware, [middleware.applyCSRF].concat(accountMiddlewares), controllers.accounts.accountEdit); + setupPageRoute(app, '/user/:userslug/edit', middleware, accountMiddlewares, controllers.accounts.accountEdit); setupPageRoute(app, '/user/:userslug/settings', middleware, accountMiddlewares, controllers.accounts.accountSettings); setupPageRoute(app, '/notifications', middleware, [middleware.authenticate], controllers.accounts.getNotifications); @@ -98,7 +98,7 @@ function groupRoutes(app, middleware, controllers) { function setupPageRoute(router, name, middleware, middlewares, controller) { middlewares = middlewares.concat([middleware.incrementPageViews, middleware.updateLastOnlineTime]); - router.get(name, middleware.buildHeader, middlewares, controller); + router.get(name, middleware.applyCSRF, middleware.buildHeader, middlewares, controller); router.get('/api' + name, middlewares, controller); } @@ -179,6 +179,10 @@ function handleErrors(err, req, res, next) { //console.error(err.stack, req.path); winston.error(req.path + '\n', err.stack); + if (err.code === 'EBADCSRFTOKEN') { + return res.sendStatus(403); + } + var status = err.status || 500; res.status(status); diff --git a/src/views/admin/footer.tpl b/src/views/admin/footer.tpl index 394d4499b5..66c1ddc335 100644 --- a/src/views/admin/footer.tpl +++ b/src/views/admin/footer.tpl @@ -17,7 +17,6 @@

-
diff --git a/src/views/admin/manage/categories.tpl b/src/views/admin/manage/categories.tpl index 23dcd66949..b3b9427abb 100644 --- a/src/views/admin/manage/categories.tpl +++ b/src/views/admin/manage/categories.tpl @@ -142,8 +142,6 @@
- - diff --git a/src/views/admin/settings/footer.tpl b/src/views/admin/settings/footer.tpl index 8e9185ef8d..77b495e33c 100644 --- a/src/views/admin/settings/footer.tpl +++ b/src/views/admin/settings/footer.tpl @@ -1,5 +1,4 @@ -