From 0734f450fc098e3302e57efa14e1e940e657086e Mon Sep 17 00:00:00 2001 From: Hugo Jeffreys Date: Tue, 14 Apr 2026 13:07:02 +0100 Subject: [PATCH] PP-14951 upgrade passport with express-sessions --- package-lock.json | 114 +++++++++++++++++++++----------- package.json | 7 +- src/lib/auth.ts | 8 ++- src/lib/auth/github/strategy.js | 2 +- src/tests/session.spec.js | 2 +- src/web/server.js | 21 +++--- 6 files changed, 101 insertions(+), 53 deletions(-) diff --git a/package-lock.json b/package-lock.json index fdd82c23c..950a2fab1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,7 @@ "deep-diff": "1.0.2", "dotenv": "16.0.3", "express": "^5.2.1", + "express-session": "^1.19.0", "fast-csv": "^4.3.6", "google-libphonenumber": "~3.2.21", "govuk-frontend": "^5.10.2", @@ -36,8 +37,8 @@ "morgan": "^1.10.1", "multer": "2.1.1", "nunjucks": "3.2.4", - "passport": "~0.5.3", - "passport-github": "1.1.0", + "passport": "~0.7.0", + "passport-github2": "^0.1.12", "qs": "6.15.0", "rfc822-validate": "^1.0.0", "sass": "^1.62.1", @@ -60,7 +61,7 @@ "@types/morgan": "^1.9.2", "@types/multer": "^1.4.5", "@types/node": "20.5.9", - "@types/passport": "1.0.6", + "@types/passport": "^1.0.17", "@types/qs": "6.9.6", "@types/stripe": "6.25.8", "chai": "^4.3.4", @@ -1792,14 +1793,6 @@ "node": ">=6" } }, - "node_modules/@sentry/node/node_modules/cookie": { - "version": "0.7.2", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.2.tgz", - "integrity": "sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==", - "engines": { - "node": ">= 0.6" - } - }, "node_modules/@sentry/node/node_modules/tslib": { "version": "1.14.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", @@ -2878,10 +2871,11 @@ "dev": true }, "node_modules/@types/passport": { - "version": "1.0.6", - "resolved": "https://registry.npmjs.org/@types/passport/-/passport-1.0.6.tgz", - "integrity": "sha512-9oKfrJXuAxvyxdrtMCxKkHgmd6DMO8NDOLvMJ1LvIWd6/xP+i81PAkpTaEca7VhJX9S009RciwZL/j6dsLsHrA==", + "version": "1.0.17", + "resolved": "https://registry.npmjs.org/@types/passport/-/passport-1.0.17.tgz", + "integrity": "sha512-aciLyx+wDwT2t2/kJGJR2AEeBz0nJU4WuRX04Wu9Dqc5lSUtwu0WERPHYsLhF9PtseiAMPBGNUOtFjxZ56prsg==", "dev": true, + "license": "MIT", "dependencies": { "@types/express": "*" } @@ -4478,6 +4472,15 @@ "node": ">= 0.6" } }, + "node_modules/cookie": { + "version": "0.7.2", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.2.tgz", + "integrity": "sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==", + "license": "MIT", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/cookie-signature": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz", @@ -4595,14 +4598,6 @@ "node": ">= 0.8.0" } }, - "node_modules/csurf/node_modules/cookie": { - "version": "0.7.2", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.2.tgz", - "integrity": "sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==", - "engines": { - "node": ">= 0.6" - } - }, "node_modules/csurf/node_modules/http-errors": { "version": "1.7.3", "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.7.3.tgz", @@ -5571,14 +5566,59 @@ "express": ">= 4.11" } }, - "node_modules/express/node_modules/cookie": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.1.tgz", - "integrity": "sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==", + "node_modules/express-session": { + "version": "1.19.0", + "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.19.0.tgz", + "integrity": "sha512-0csaMkGq+vaiZTmSMMGkfdCOabYv192VbytFypcvI0MANrp+4i/7yEkJ0sbAEhycQjntaKGzYfjfXQyVb7BHMA==", + "license": "MIT", + "dependencies": { + "cookie": "~0.7.2", + "cookie-signature": "~1.0.7", + "debug": "~2.6.9", + "depd": "~2.0.0", + "on-headers": "~1.1.0", + "parseurl": "~1.3.3", + "safe-buffer": "~5.2.1", + "uid-safe": "~2.1.5" + }, "engines": { - "node": ">= 0.6" + "node": ">= 0.8.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/express-session/node_modules/cookie-signature": { + "version": "1.0.7", + "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.7.tgz", + "integrity": "sha512-NXdYc3dLr47pBkpUCHtKSwIOQXLVn8dZEuywboCOJY/osA0wFSLlSawr3KN8qXJEyX66FcONTH8EIlVuK0yyFA==", + "license": "MIT" + }, + "node_modules/express-session/node_modules/debug": { + "version": "2.6.9", + "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", + "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", + "license": "MIT", + "dependencies": { + "ms": "2.0.0" + } + }, + "node_modules/express-session/node_modules/depd": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/depd/-/depd-2.0.0.tgz", + "integrity": "sha512-g7nH6P6dyDioJogAAGprGpCtVImJhpPk/roCzdb3fIh61/s/nPsfR6onyMwkCAR/OlC3yBC0lESvUoQEAssIrw==", + "license": "MIT", + "engines": { + "node": ">= 0.8" } }, + "node_modules/express-session/node_modules/ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", + "license": "MIT" + }, "node_modules/express/node_modules/cookie-signature": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.2.2.tgz", @@ -8226,13 +8266,14 @@ } }, "node_modules/passport": { - "version": "0.5.3", - "resolved": "https://registry.npmjs.org/passport/-/passport-0.5.3.tgz", - "integrity": "sha512-gGc+70h4gGdBWNsR3FuV3byLDY6KBTJAIExGFXTpQaYfbbcHCBlRRKx7RBQSpqEqc5Hh2qVzRs7ssvSfOpkUEA==", + "version": "0.7.0", + "resolved": "https://registry.npmjs.org/passport/-/passport-0.7.0.tgz", + "integrity": "sha512-cPLl+qZpSc+ireUvt+IzqbED1cHHkDoVYMo30jbJIdOOjQ1MQYZBPiNvmi8UM6lJuOpTPXJGZQk0DtC4y61MYQ==", "license": "MIT", "dependencies": { "passport-strategy": "1.x.x", - "pause": "0.0.1" + "pause": "0.0.1", + "utils-merge": "^1.0.1" }, "engines": { "node": ">= 0.4.0" @@ -8242,15 +8283,15 @@ "url": "https://github.com/sponsors/jaredhanson" } }, - "node_modules/passport-github": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/passport-github/-/passport-github-1.1.0.tgz", - "integrity": "sha512-XARXJycE6fFh/dxF+Uut8OjlwbFEXgbPVj/+V+K7cvriRK7VcAOm+NgBmbiLM9Qv3SSxEAV+V6fIk89nYHXa8A==", + "node_modules/passport-github2": { + "version": "0.1.12", + "resolved": "https://registry.npmjs.org/passport-github2/-/passport-github2-0.1.12.tgz", + "integrity": "sha512-3nPUCc7ttF/3HSP/k9sAXjz3SkGv5Nki84I05kSQPo01Jqq1NzJACgMblCK0fGcv9pKCG/KXU3AJRDGLqHLoIw==", "dependencies": { "passport-oauth2": "1.x.x" }, "engines": { - "node": ">= 0.4.0" + "node": ">= 0.8.0" } }, "node_modules/passport-oauth2": { @@ -9064,7 +9105,6 @@ "version": "5.2.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==", - "dev": true, "funding": [ { "type": "github", diff --git a/package.json b/package.json index 0a91a5a6a..8558e1264 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "deep-diff": "1.0.2", "dotenv": "16.0.3", "express": "^5.2.1", + "express-session": "^1.19.0", "fast-csv": "^4.3.6", "google-libphonenumber": "~3.2.21", "govuk-frontend": "^5.10.2", @@ -56,8 +57,8 @@ "morgan": "^1.10.1", "multer": "2.1.1", "nunjucks": "3.2.4", - "passport": "~0.5.3", - "passport-github": "1.1.0", + "passport": "~0.7.0", + "passport-github2": "^0.1.12", "qs": "6.15.0", "rfc822-validate": "^1.0.0", "sass": "^1.62.1", @@ -80,7 +81,7 @@ "@types/morgan": "^1.9.2", "@types/multer": "^1.4.5", "@types/node": "20.5.9", - "@types/passport": "1.0.6", + "@types/passport": "^1.0.17", "@types/qs": "6.9.6", "@types/stripe": "6.25.8", "chai": "^4.3.4", diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 8d166a9d8..92eb6d626 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -36,8 +36,10 @@ export function unauthorised(req: Request, res: Response) { res.status(403).send('User does not have permissions to access the resource') } -export function revokeSession(req: Request, res: Response) { +export function revokeSession(req: Request, res: Response, next: NextFunction) { logger.info(`Revoking session for user ${req.user && req.user.username}`) - req.logout() - res.redirect('/') + req.logout((err?: unknown) => { + if (err) return next(err); + res.redirect('/'); + }); } diff --git a/src/lib/auth/github/strategy.js b/src/lib/auth/github/strategy.js index 97e9710c6..16d8a0587 100644 --- a/src/lib/auth/github/strategy.js +++ b/src/lib/auth/github/strategy.js @@ -1,5 +1,5 @@ // github OAuth strategy -const { Strategy } = require('passport-github') +const { Strategy } = require('passport-github2') const config = require('../../../config') const logger = require('../../logger') diff --git a/src/tests/session.spec.js b/src/tests/session.spec.js index cf765a4d4..3723f6d76 100644 --- a/src/tests/session.spec.js +++ b/src/tests/session.spec.js @@ -9,7 +9,7 @@ describe('session cookies', () => { .get('/healthcheck') .expect(200) .end((err, res) => { - expect(res.headers['set-cookie'][0]).to.contain('expires=') + expect(res.headers['set-cookie'][0]).to.contain('Expires=') expect(res.headers['set-cookie'][0]).to.not.contain('Invalid Date') done() }) diff --git a/src/web/server.js b/src/web/server.js index 4cf50ac5b..234b7cce9 100644 --- a/src/web/server.js +++ b/src/web/server.js @@ -4,7 +4,7 @@ const express = require('express') const metrics = require('@govuk-pay/pay-js-metrics') const helmet = require('helmet') const flash = require('connect-flash') -const sessions = require('client-sessions') +const sessions = require('express-session') const nunjucks = require('nunjucks') const csurf = require('csurf') const Sentry = require('@sentry/node') @@ -36,9 +36,9 @@ const { const app = express() -function configureSecureHeaders(instance) { - const serverBehindProxy = server.HTTP_PROXY +const serverBehindProxy = server.HTTP_PROXY +function configureSecureHeaders(instance) { // only set certain proxy configured headers if not behind a proxy, the proxy // will be responsible for setting these headers const helmetOptions = serverBehindProxy ? { @@ -88,17 +88,22 @@ function configureServingPublicStaticFiles(instance) { } function configureClientSessions(instance) { - const serverBehindProxy = server.HTTP_PROXY const thirtyMinutesInMillis = 30 * 60 * 1000 instance.use(sessions({ - cookieName: 'session', + name: 'session', secret: server.COOKIE_SESSION_ENCRYPTION_SECRET, - duration: server.SESSION_COOKIE_DURATION_IN_MILLIS || thirtyMinutesInMillis, - activeDuration: 5 * 60 * 1000, + resave: false, + rolling: true, + saveUninitialized: false, cookie: { - secureProxy: serverBehindProxy + maxAge: server.SESSION_COOKIE_DURATION_IN_MILLIS || thirtyMinutesInMillis, + secure: serverBehindProxy, + httpOnly: true, } })) + if (serverBehindProxy) { + instance.set('trust proxy', 1) + } } function configureAuth(instance) {