diff --git a/services/web/app/src/infrastructure/Server.js b/services/web/app/src/infrastructure/Server.js index 6a9d36f8b9..9c10015721 100644 --- a/services/web/app/src/infrastructure/Server.js +++ b/services/web/app/src/infrastructure/Server.js @@ -12,6 +12,7 @@ const Csrf = require('./Csrf') const sessionsRedisClient = UserSessionsRedis.client() +const SessionAutostartMiddleware = require('./SessionAutostartMiddleware') const SessionStoreManager = require('./SessionStoreManager') const session = require('express-session') const RedisStore = require('connect-redis')(session) @@ -95,6 +96,7 @@ RedirectManager.apply(webRouter) ProxyManager.apply(publicApiRouter) webRouter.use(cookieParser(Settings.security.sessionSecret)) +SessionAutostartMiddleware.applyInitialMiddleware(webRouter) webRouter.use( session({ resave: false, @@ -149,16 +151,18 @@ webRouter.use(translations.setLangBasedOnDomainMiddlewear) // Measure expiry from last request, not last login webRouter.use(function(req, res, next) { - req.session.touch() - if (AuthenticationController.isUserLoggedIn(req)) { - UserSessionsManager.touch( - AuthenticationController.getSessionUser(req), - err => { - if (err) { - logger.err({ err }, 'error extending user session') + if (!req.session.noSessionCallback) { + req.session.touch() + if (AuthenticationController.isUserLoggedIn(req)) { + UserSessionsManager.touch( + AuthenticationController.getSessionUser(req), + err => { + if (err) { + logger.err({ err }, 'error extending user session') + } } - } - ) + ) + } } next() }) @@ -166,6 +170,8 @@ webRouter.use(function(req, res, next) { webRouter.use(ReferalConnect.use) expressLocals(webRouter, privateApiRouter, publicApiRouter) +webRouter.use(SessionAutostartMiddleware.invokeCallbackMiddleware) + if (app.get('env') === 'production') { logger.info('Production Enviroment') app.enable('view cache') @@ -240,6 +246,7 @@ if (enableWebRouter || notDefined(enableWebRouter)) { } metrics.injectMetricsRoute(webRouter) + Router.initialize(webRouter, privateApiRouter, publicApiRouter) module.exports = { diff --git a/services/web/app/src/infrastructure/SessionAutostartMiddleware.js b/services/web/app/src/infrastructure/SessionAutostartMiddleware.js new file mode 100644 index 0000000000..a670cb19f5 --- /dev/null +++ b/services/web/app/src/infrastructure/SessionAutostartMiddleware.js @@ -0,0 +1,67 @@ +const Settings = require('settings-sharelatex') + +// SessionAutostartMiddleware provides a mechanism to force certain routes not +// to get an automatic session where they don't have one already. This allows us +// to work around issues where we might overwrite a user's login cookie with one +// that is hidden by a `SameSite` setting. +// +// When registering a route with disableSessionAutostartForRoute, a callback +// should be provided that handles the case that a session is not available. +// This will be called as a standard middleware with (req, res, next) - calling +// next will continue and sett up a session as normal, otherwise the app can +// perform a different operation as usual + +class SessionAutostartMiddleware { + constructor() { + this.middleware = this.middleware.bind(this) + this._cookieName = Settings.cookieName + this._noAutostartCallbacks = new Map() + } + + static applyInitialMiddleware(router) { + const middleware = new SessionAutostartMiddleware() + router.sessionAutostartMiddleware = middleware + router.use(middleware.middleware) + } + + disableSessionAutostartForRoute(route, method, callback) { + if (typeof callback !== 'function') { + throw new Error('callback not provided when disabling session autostart') + } + + if (!this._noAutostartCallbacks[route]) { + this._noAutostartCallbacks[route] = new Map() + } + + this._noAutostartCallbacks[route][method] = callback + } + + autostartCallbackForRequest(req) { + return ( + this._noAutostartCallbacks[req.path] && + this._noAutostartCallbacks[req.path][req.method] + ) + } + + middleware(req, res, next) { + if (!req.signedCookies[this._cookieName]) { + const callback = this.autostartCallbackForRequest(req) + if (callback) { + req.session = { + noSessionCallback: callback + } + } + } + + next() + } + + static invokeCallbackMiddleware(req, res, next) { + if (req.session.noSessionCallback) { + return req.session.noSessionCallback(req, res, next) + } + next() + } +} + +module.exports = SessionAutostartMiddleware diff --git a/services/web/app/src/infrastructure/SessionStoreManager.js b/services/web/app/src/infrastructure/SessionStoreManager.js index 1d459e22c4..e707fb8b74 100644 --- a/services/web/app/src/infrastructure/SessionStoreManager.js +++ b/services/web/app/src/infrastructure/SessionStoreManager.js @@ -53,14 +53,15 @@ module.exports = { }, validationMiddleware(req, res, next) { - if (!checkValidationToken(req)) { - // the session must exist for it to fail validation - req.session.destroy(() => { - return next(new Error('invalid session')) - }) - } else { - return next() + if (!req.session.noSessionCallback) { + if (!checkValidationToken(req)) { + // the session must exist for it to fail validation + return req.session.destroy(() => { + return next(new Error('invalid session')) + }) + } } + next() }, hasValidationToken(req) { diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index fe10cdd234..4a38ad5828 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -390,6 +390,11 @@ module.exports = settings = # cookie with a secure flag (recommended). secureCookie: false + # 'SameSite' cookie setting. Can be set to 'lax', 'none' or 'strict' + # 'lax' is recommended, as 'strict' will prevent people linking to projects + # https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.2.7 + sameSiteCookie: 'lax' + # If you are running ShareLaTeX behind a proxy (like Apache, Nginx, etc) # then set this to true to allow it to correctly detect the forwarded IP # address and http/https protocol information. diff --git a/services/web/test/unit/src/Security/SessionAutostartMiddlewareTests.js b/services/web/test/unit/src/Security/SessionAutostartMiddlewareTests.js new file mode 100644 index 0000000000..0898876076 --- /dev/null +++ b/services/web/test/unit/src/Security/SessionAutostartMiddlewareTests.js @@ -0,0 +1,68 @@ +const sinon = require('sinon') +const chai = require('chai') +const { expect } = chai +const modulePath = + '../../../../app/src/infrastructure/SessionAutostartMiddleware.js' +const SandboxedModule = require('sandboxed-module') + +describe('SessionAutostartMiddleware', function() { + let SessionAutostartMiddleware, middleware, Settings + const cookieName = 'coookieee' + const excludedRoute = '/wombat/potato' + const excludedMethod = 'POST' + const excludedCallback = () => 'call me' + + beforeEach(function() { + Settings = { + cookieName: cookieName + } + + SessionAutostartMiddleware = SandboxedModule.require(modulePath, { + globals: { + console: console + }, + requires: { + 'settings-sharelatex': Settings + } + }) + + middleware = new SessionAutostartMiddleware() + middleware.disableSessionAutostartForRoute( + excludedRoute, + excludedMethod, + excludedCallback + ) + }) + + describe('middleware', function() { + let req, next + + beforeEach(function() { + req = { path: excludedRoute, method: excludedMethod, signedCookies: {} } + next = sinon.stub() + }) + + it('executes the callback for the excluded route', function() { + middleware.middleware(req, {}, next) + expect(req.session.noSessionCallback).to.equal(excludedCallback) + }) + + it('does not execute the callback if the method is not excluded', function() { + req.method = 'GET' + middleware.middleware(req, {}, next) + expect(req.session).not.to.exist + }) + + it('does not execute the callback if the path is not excluded', function() { + req.path = '/giraffe' + middleware.middleware(req, {}, next) + expect(req.session).not.to.exist + }) + + it('does not execute the callback if there is a cookie', function() { + req.signedCookies[cookieName] = 'a very useful session cookie' + middleware.middleware(req, {}, next) + expect(req.session).not.to.exist + }) + }) +})