PP-14951 upgrade passport with express-sessions

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",

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('/');
+    });
 }

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')
 

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()
       })

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 ? {
@@ -87,16 +87,25 @@ function configureServingPublicStaticFiles(instance) {
   instance.use('/assets/logos', express.static(path.join(process.cwd(), 'src/assets/logos'), cache))
 }
 
+function configureServerProxy(instance) {
+  if(serverBehindProxy) {
+    instance.set('trust proxy', 1)
+  }
+}
+
 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,
+      sameSite: 'lax'
     }
   }))
 }
@@ -204,6 +213,7 @@ const configure = [
   configureSentry,
   configureSentryRequestHandler,
   configureRequestParsing,
+  configureServerProxy,
   configureClientSessions,
   configureAuth,
   configureSecureHeaders,