From dce548938f6e3b88f43e5c2db0637031bbcbdb8d Mon Sep 17 00:00:00 2001 From: Marco Collura Date: Thu, 30 Nov 2023 16:35:44 +0000 Subject: [PATCH 1/2] Add hashed userId to whats-new-banner cookie key --- server/services/whatsNewCookieService.test.ts | 14 +++++++++----- server/services/whatsNewCookieService.ts | 9 ++++++--- server/utils/controllerUtils.test.ts | 12 ++++++------ server/utils/controllerUtils.ts | 5 +++-- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/server/services/whatsNewCookieService.test.ts b/server/services/whatsNewCookieService.test.ts index adb9fc432..7ba40dacd 100644 --- a/server/services/whatsNewCookieService.test.ts +++ b/server/services/whatsNewCookieService.test.ts @@ -1,24 +1,28 @@ import { Request, Response } from 'express' -import NotificationCookieService from './whatsNewCookieService' +import md5 from 'md5' +import WhatsNewCookieService from './whatsNewCookieService' describe('WhatsNew cookie', () => { let res: Response let req: Request + const id = '123' + const hashedId = md5(id) + beforeEach(() => { res = { cookie: jest.fn() } as unknown as Response - req = { cookies: { 'whats-new-banner': 1 } } as unknown as Request + req = { cookies: { [`whats-new-banner-${hashedId}`]: 1 } } as unknown as Request }) it('should store cookie correctly', () => { - NotificationCookieService.persistDismissedVersion(res, 1) - expect(res.cookie).toHaveBeenCalledWith('whats-new-banner', 1, { + WhatsNewCookieService.persistDismissedVersion(res, id, 1) + expect(res.cookie).toHaveBeenCalledWith(`whats-new-banner-${hashedId}`, 1, { httpOnly: true, maxAge: 4492800000, }) }) it('should find matching cookie', () => { - expect(NotificationCookieService.getDismissedVersion(req)).toEqual(1) + expect(WhatsNewCookieService.getDismissedVersion(req, id)).toEqual(1) }) }) diff --git a/server/services/whatsNewCookieService.ts b/server/services/whatsNewCookieService.ts index 47b079aa4..79a5735c4 100644 --- a/server/services/whatsNewCookieService.ts +++ b/server/services/whatsNewCookieService.ts @@ -1,12 +1,15 @@ import { Request, Response } from 'express' +import md5 from 'md5' export default class WhatsNewCookieService { static COOKIE_NAME = 'whats-new-banner' - static persistDismissedVersion = (res: Response, version: number) => { + static key = (id: string) => `${this.COOKIE_NAME}-${md5(id)}` + + static persistDismissedVersion = (res: Response, id: string, version: number) => { const oneYear = 52 * 24 * 3600000 - return res.cookie(this.COOKIE_NAME, version, { maxAge: oneYear, httpOnly: true }) + return res.cookie(this.key(id), version, { maxAge: oneYear, httpOnly: true }) } - static getDismissedVersion = (req: Request): number => Number(req.cookies[this.COOKIE_NAME]) + static getDismissedVersion = (req: Request, id: string): number => Number(req.cookies[this.key(id)]) } diff --git a/server/utils/controllerUtils.test.ts b/server/utils/controllerUtils.test.ts index 0ea72d820..b09c6e818 100644 --- a/server/utils/controllerUtils.test.ts +++ b/server/utils/controllerUtils.test.ts @@ -40,7 +40,7 @@ describe(ControllerUtils, () => { describe('with null serviceUser', () => { it('calls render on the response, passing the content view’s renderArgs augmented with a headerPresenter object in the locals', async () => { const req = { render: jest.fn(), locals: { user: null } } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } config.googleAnalyticsTrackingId = 'UA-TEST-ID' @@ -61,7 +61,7 @@ describe(ControllerUtils, () => { describe('with non-null serviceUser', () => { it('calls render on the response, passing the content view’s renderArgs, augmented with headerPresenter and serviceUserNotificationBannerArgs objects in the locals', async () => { const req = { render: jest.fn(), locals: { user: null } } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } const serviceUser = deliusServiceUserFactory.build() @@ -88,7 +88,7 @@ describe(ControllerUtils, () => { locals: { user: null }, query: { dismissWhatsNewBanner: false }, } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } const serviceUser = deliusServiceUserFactory.build() @@ -116,7 +116,7 @@ describe(ControllerUtils, () => { locals: { user: null }, query: { dismissWhatsNewBanner: true }, } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } const serviceUser = deliusServiceUserFactory.build() @@ -145,7 +145,7 @@ describe(ControllerUtils, () => { locals: { user: null }, query: { dismissWhatsNewBanner: false }, } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } const serviceUser = deliusServiceUserFactory.build() @@ -180,7 +180,7 @@ describe(ControllerUtils, () => { locals: { user: null }, query: { dismissWhatsNewBanner: false }, } as unknown as Request - const res = { render: jest.fn(), locals: { user: null } } as unknown as Response + const res = { render: jest.fn(), locals: { user: { userId: '123' } } } as unknown as Response const renderArgs: [string, Record] = ['myTemplate', { foo: '1', bar: '2' }] const contentView = { renderArgs } const serviceUser = deliusServiceUserFactory.build() diff --git a/server/utils/controllerUtils.ts b/server/utils/controllerUtils.ts index a6f27c89c..067f0a7cf 100644 --- a/server/utils/controllerUtils.ts +++ b/server/utils/controllerUtils.ts @@ -36,13 +36,14 @@ export default class ControllerUtils { ): Promise { let whatsNewBanner let showWhatsNewBanner = false + const { userId } = res.locals.user if (userType) { whatsNewBanner = await ReferenceDataService.getWhatsNewBanner(userType, req.originalUrl) - showWhatsNewBanner = whatsNewBanner?.version !== WhatsNewCookieService.getDismissedVersion(req) + showWhatsNewBanner = whatsNewBanner?.version !== WhatsNewCookieService.getDismissedVersion(req, userId) if (whatsNewBanner?.version && req.query.dismissWhatsNewBanner) { - WhatsNewCookieService.persistDismissedVersion(res, whatsNewBanner.version) + WhatsNewCookieService.persistDismissedVersion(res, userId, whatsNewBanner.version) showWhatsNewBanner = false } } From 4fd38e6cf18d811f9b43684cba3c87fb68af0aa2 Mon Sep 17 00:00:00 2001 From: Marco Collura Date: Thu, 30 Nov 2023 16:57:41 +0000 Subject: [PATCH 2/2] hash entire whats new banner key --- server/services/whatsNewCookieService.test.ts | 6 +++--- server/services/whatsNewCookieService.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/services/whatsNewCookieService.test.ts b/server/services/whatsNewCookieService.test.ts index 7ba40dacd..4c7cf7e29 100644 --- a/server/services/whatsNewCookieService.test.ts +++ b/server/services/whatsNewCookieService.test.ts @@ -7,16 +7,16 @@ describe('WhatsNew cookie', () => { let req: Request const id = '123' - const hashedId = md5(id) + const hashedKey = md5(`whats-new-banner-${id}`) beforeEach(() => { res = { cookie: jest.fn() } as unknown as Response - req = { cookies: { [`whats-new-banner-${hashedId}`]: 1 } } as unknown as Request + req = { cookies: { [hashedKey]: 1 } } as unknown as Request }) it('should store cookie correctly', () => { WhatsNewCookieService.persistDismissedVersion(res, id, 1) - expect(res.cookie).toHaveBeenCalledWith(`whats-new-banner-${hashedId}`, 1, { + expect(res.cookie).toHaveBeenCalledWith(hashedKey, 1, { httpOnly: true, maxAge: 4492800000, }) diff --git a/server/services/whatsNewCookieService.ts b/server/services/whatsNewCookieService.ts index 79a5735c4..946c7b599 100644 --- a/server/services/whatsNewCookieService.ts +++ b/server/services/whatsNewCookieService.ts @@ -4,7 +4,7 @@ import md5 from 'md5' export default class WhatsNewCookieService { static COOKIE_NAME = 'whats-new-banner' - static key = (id: string) => `${this.COOKIE_NAME}-${md5(id)}` + static key = (id: string) => md5(`${this.COOKIE_NAME}-${id}`) static persistDismissedVersion = (res: Response, id: string, version: number) => { const oneYear = 52 * 24 * 3600000