diff --git a/package-lock.json b/package-lock.json index 0f02698412..2237fd2178 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "habitica", - "version": "4.140.4", + "version": "4.140.5", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 20a597921f..f4141aa111 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "habitica", "description": "A habit tracker app which treats your goals like a Role Playing Game.", - "version": "4.140.4", + "version": "4.140.5", "main": "./website/server/index.js", "dependencies": { "@babel/core": "^7.9.0", @@ -52,6 +52,7 @@ "morgan": "^1.10.0", "nconf": "^0.10.0", "node-gcm": "^1.0.2", + "on-headers": "^1.0.2", "passport": "^0.4.1", "passport-facebook": "^3.0.0", "passport-google-oauth2": "^0.2.0", diff --git a/test/api/unit/middlewares/cache.test.js b/test/api/unit/middlewares/cache.test.js new file mode 100644 index 0000000000..6501054273 --- /dev/null +++ b/test/api/unit/middlewares/cache.test.js @@ -0,0 +1,31 @@ +import { + generateRes, + generateReq, + generateNext, +} from '../../../helpers/api-unit.helper'; +import { + disableCache, +} from '../../../../website/server/middlewares/cache'; + +describe('cache middlewares', () => { + let res; let req; let + next; + + beforeEach(() => { + req = generateReq(); + res = generateRes(); + next = generateNext(); + }); + + describe('disableCache', () => { + it('sets the correct headers', () => { + disableCache(req, res, next); + expect(res.set).to.have.been.calledWith('Cache-Control', 'no-store'); + expect(next).to.have.been.calledOnce; + }); + + xit('removes the etag header', () => { + // @TODO how to stub onHeaders + }); + }); +}); diff --git a/website/server/controllers/api-v3/status.js b/website/server/controllers/api-v3/status.js index c957388d59..fe42c417eb 100644 --- a/website/server/controllers/api-v3/status.js +++ b/website/server/controllers/api-v3/status.js @@ -1,3 +1,7 @@ +import { + disableCache, +} from '../../middlewares/cache'; + const api = {}; /** @@ -15,6 +19,8 @@ const api = {}; api.getStatus = { method: 'GET', url: '/status', + // explicitly disable caching so that the server is always checked + middlewares: [disableCache], async handler (req, res) { res.respond(200, { status: 'up', diff --git a/website/server/controllers/api-v4/faq.js b/website/server/controllers/api-v4/faq.js index 171e72edad..4dfce05f24 100644 --- a/website/server/controllers/api-v4/faq.js +++ b/website/server/controllers/api-v4/faq.js @@ -1,5 +1,4 @@ import _ from 'lodash'; -import { query } from 'express-validator/check'; import { langCodes } from '../../libs/i18n'; import apiError from '../../libs/apiError'; import common from '../../../common'; @@ -55,12 +54,9 @@ function _deleteOtherPlatformsAnswers (faqObject, platform) { api.faq = { method: 'GET', url: '/faq', - middlewares: [ - query('platform') - .optional() - .isIn(['web', 'android', 'ios']).withMessage(apiError('invalidPlatform')), - ], async handler (req, res) { + req.checkQuery('platform').optional().isIn(['web', 'android', 'ios'], apiError('guildsPaginateBooleanString')); + const validationErrors = req.validationErrors(); if (validationErrors) throw validationErrors; diff --git a/website/server/controllers/top-level/payments/paypal.js b/website/server/controllers/top-level/payments/paypal.js index 5885076a0f..84ff60d7ab 100644 --- a/website/server/controllers/top-level/payments/paypal.js +++ b/website/server/controllers/top-level/payments/paypal.js @@ -1,5 +1,6 @@ /* eslint-disable camelcase */ import paypalPayments from '../../../libs/payments/paypal'; +import logger from '../../../libs/logger'; import shared from '../../../../common'; import { authWithSession, @@ -171,7 +172,9 @@ api.ipn = { async handler (req, res) { res.sendStatus(200); - await paypalPayments.ipn(req.body); + paypalPayments + .ipn(req.body) + .catch(err => logger.error(err)); }, }; diff --git a/website/server/libs/routes.js b/website/server/libs/routes.js index a596200ad5..f46ba997ed 100644 --- a/website/server/libs/routes.js +++ b/website/server/libs/routes.js @@ -3,6 +3,9 @@ import _ from 'lodash'; import { getUserLanguage, } from '../middlewares/language'; +import { + disableCache, +} from '../middlewares/cache'; // Wrapper function to handler `async` route handlers that return promises // It takes the async function, execute it and pass any error to next (args[2]) @@ -28,22 +31,26 @@ export function readController (router, controller, overrides = []) { return false; }); - const middlewaresToAdd = [getUserLanguage]; + method = method.toLowerCase(); - if (action.noLanguage !== true) { + // disable caching for all routes with mandatory or optional authentication + if (authMiddlewareIndex !== -1) { + middlewares.unshift(disableCache); + } + + if (action.noLanguage !== true) { // unless getting the language is explictly disabled // the user will be authenticated, getUserLanguage after authentication if (authMiddlewareIndex !== -1) { if (authMiddlewareIndex === middlewares.length - 1) { - middlewares.push(...middlewaresToAdd); + middlewares.push(getUserLanguage); } else { - middlewares.splice(authMiddlewareIndex + 1, 0, ...middlewaresToAdd); + middlewares.splice(authMiddlewareIndex + 1, 0, getUserLanguage); } } else { // no auth, getUserLanguage as the first middleware - middlewares.unshift(...middlewaresToAdd); + middlewares.unshift(getUserLanguage); } } - method = method.toLowerCase(); const fn = handler ? _wrapAsyncFn(handler) : noop; router[method](url, ...middlewares, fn); diff --git a/website/server/middlewares/cache.js b/website/server/middlewares/cache.js new file mode 100644 index 0000000000..228e7d14eb --- /dev/null +++ b/website/server/middlewares/cache.js @@ -0,0 +1,14 @@ +import onHeaders from 'on-headers'; + +export function disableCache (req, res, next) { + res.set('Cache-Control', 'no-store'); + + // Remove the etag header when caching is disabled + // @TODO Unfortunately it's not possible to prevent the creation right now + // See this issue https://github.com/expressjs/express/issues/2472 + onHeaders(res, function removeEtag () { + this.removeHeader('ETag'); + }); + + return next(); +}