fix: standardise cookie settings

Author: Raphael Kabo

server/__tests__/middleware/identityMiddleware.test.ts

@@ -6,6 +6,7 @@ import type { Request, Response } from 'express';
 import { conf } from '@/server/config';
 import * as identityMiddleware from '@/server/middleware/identityMiddleware';
 import {
+	oauthCookieOptions,
 	performAuthorizationCodeFlow,
 	verifyAccessToken,
 	verifyIdToken,
@@ -148,6 +149,7 @@ describe('withOAuth middleware', () => {
 	});
 	it('returns next() if the route does not require sign in', async () => {
 		const req = {
+			cookies: {},
 			signedCookies: {},
 			originalUrl: '/help-centre',
 		};
@@ -184,18 +186,14 @@ describe('withOAuth middleware', () => {
 			next,
 		);
 
-		expect(res.clearCookie).toHaveBeenCalledWith('GU_ACCESS_TOKEN', {
-			signed: true,
-			secure: true,
-			httpOnly: true,
-			sameSite: 'strict',
-		});
-		expect(res.clearCookie).toHaveBeenCalledWith('GU_ID_TOKEN', {
-			signed: true,
-			secure: true,
-			httpOnly: true,
-			sameSite: 'strict',
-		});
+		expect(res.clearCookie).toHaveBeenCalledWith(
+			'GU_ACCESS_TOKEN',
+			oauthCookieOptions,
+		);
+		expect(res.clearCookie).toHaveBeenCalledWith(
+			'GU_ID_TOKEN',
+			oauthCookieOptions,
+		);
 		expect(performAuthorizationCodeFlow).toHaveBeenCalledWith(req, res, {
 			redirectUri: `https://manage.${conf.DOMAIN}/oauth/callback`,
 			scopes,

server/middleware/identityMiddleware.ts

@@ -3,6 +3,7 @@ import type { NextFunction, Request, Response } from 'express';
 import type { Scopes, VerifiedOAuthCookies } from '@/server/oauth';
 import {
 	OAuthAccessTokenCookieName,
+	oauthCookieOptions,
 	OAuthIdTokenCookieName,
 	performAuthorizationCodeFlow,
 	scopes,
@@ -98,6 +99,22 @@ export const withOAuth = async (
 	if (CYPRESS === 'SKIP_IDAPI') {
 		return next();
 	}
+
+	// Get the path of the current page and use it as our returnPath after the OAuth callback.
+	const returnPath = sanitizeReturnPath(req.originalUrl);
+
+	// If we have a GU_SO cookie, we've signed out recently, so we need to delete
+	// the access and ID tokens from the browser and redirect to the sign in page.
+	if (req.cookies['GU_SO']) {
+		res.clearCookie(OAuthAccessTokenCookieName, oauthCookieOptions);
+		res.clearCookie(OAuthIdTokenCookieName, oauthCookieOptions);
+		return performAuthorizationCodeFlow(req, res, {
+			redirectUri: `https://manage.${conf.DOMAIN}/oauth/callback`,
+			scopes,
+			returnPath,
+		});
+	}
+
 	// Is this a public route?
 	if (!requiresSignin(req.originalUrl)) {
 		// If we do have access and ID token cookies, we can attempt to verify them
@@ -115,31 +132,6 @@ export const withOAuth = async (
 		}
 	}
 
-	// Get the path of the current page and use it as our returnPath after the OAuth callback.
-	const returnPath = sanitizeReturnPath(req.originalUrl);
-
-	// If we have a GU_SO cookie, we've signed out recently, so we need to delete
-	// the access and ID tokens from the browser and redirect to the sign in page.
-	if (req.cookies['GU_SO']) {
-		res.clearCookie(OAuthAccessTokenCookieName, {
-			signed: true,
-			secure: true,
-			httpOnly: true,
-			sameSite: 'strict',
-		});
-		res.clearCookie(OAuthIdTokenCookieName, {
-			signed: true,
-			secure: true,
-			httpOnly: true,
-			sameSite: 'strict',
-		});
-		return performAuthorizationCodeFlow(req, res, {
-			redirectUri: `https://manage.${conf.DOMAIN}/oauth/callback`,
-			scopes,
-			returnPath,
-		});
-	}
-
 	try {
 		const { accessToken, idToken } = await verifyOAuthCookies(req);
 

server/oauth.ts

@@ -20,6 +20,13 @@ export interface VerifiedOAuthCookies {
 	idToken?: OktaJwtVerifier.Jwt;
 }
 
+export const oauthCookieOptions: CookieOptions = {
+	signed: true,
+	secure: true,
+	httpOnly: true,
+	sameSite: 'strict',
+};
+
 const sharedTokenVerifierOptions = (
 	oktaConfig: OktaConfig,
 ): OktaJwtVerifier.VerifierOptions => ({
@@ -190,10 +197,8 @@ export const performAuthorizationCodeFlow = async (
 	};
 	const encodedState = Buffer.from(JSON.stringify(state)).toString('base64');
 	res.cookie(OAuthStateCookieName, encodedState, {
-		httpOnly: true,
+		...oauthCookieOptions,
 		maxAge: ms('10m'),
-		secure: true,
-		signed: true,
 	});
 
 	// generate the /authorize endpoint url which we'll redirect the user to

server/routes/oauth.ts

@@ -4,6 +4,7 @@ import ms from 'ms';
 import {
 	exchangeAccessTokenForCookies,
 	ManageMyAccountOpenIdClient,
+	oauthCookieOptions,
 	OAuthStateCookieName,
 	setIDAPICookies,
 } from '@/server/oauth';
@@ -63,24 +64,16 @@ router.get('/callback', async (req: Request, res: Response) => {
 
 		// Set the access token and ID tokens as cookies
 		res.cookie('GU_ACCESS_TOKEN', tokenSet.access_token, {
-			signed: true,
-			secure: true,
-			httpOnly: true,
+			...oauthCookieOptions,
 			maxAge: ms('30m'), // Same expiry as set in Okta
 		});
 		res.cookie('GU_ID_TOKEN', tokenSet.id_token, {
-			signed: true,
-			secure: true,
-			httpOnly: true,
+			...oauthCookieOptions,
 			maxAge: ms('30m'), // Same expiry as set in Okta
 		});
 
 		// Delete state cookie, for it is no longer needed
-		res.clearCookie(OAuthStateCookieName, {
-			httpOnly: true,
-			secure: true,
-			signed: true,
-		});
+		res.clearCookie(OAuthStateCookieName, oauthCookieOptions);
 
 		// Exchange the access token for IDAPI cookies.  To maintain dual
 		// running of new, OAuth-powered API routes and 'classic' IDAPI