Skip to content

Commit

Permalink
Merge pull request #2142 from ministryofjustice/IPB-811/whats-new-ban…
Browse files Browse the repository at this point in the history
…ner-cookies

Add hashed userId to whats-new-banner cookie key
  • Loading branch information
marcocolluramoj authored Dec 1, 2023
2 parents 2bec518 + 4fd38e6 commit 7d6377a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 16 deletions.
14 changes: 9 additions & 5 deletions server/services/whatsNewCookieService.test.ts
Original file line number Diff line number Diff line change
@@ -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 hashedKey = md5(`whats-new-banner-${id}`)

beforeEach(() => {
res = { cookie: jest.fn() } as unknown as Response
req = { cookies: { 'whats-new-banner': 1 } } as unknown as Request
req = { cookies: { [hashedKey]: 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(hashedKey, 1, {
httpOnly: true,
maxAge: 4492800000,
})
})

it('should find matching cookie', () => {
expect(NotificationCookieService.getDismissedVersion(req)).toEqual(1)
expect(WhatsNewCookieService.getDismissedVersion(req, id)).toEqual(1)
})
})
9 changes: 6 additions & 3 deletions server/services/whatsNewCookieService.ts
Original file line number Diff line number Diff line change
@@ -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) => md5(`${this.COOKIE_NAME}-${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)])
}
12 changes: 6 additions & 6 deletions server/utils/controllerUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
config.googleAnalyticsTrackingId = 'UA-TEST-ID'
Expand All @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
const serviceUser = deliusServiceUserFactory.build()
Expand All @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
const serviceUser = deliusServiceUserFactory.build()
Expand Down Expand Up @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
const serviceUser = deliusServiceUserFactory.build()
Expand Down Expand Up @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
const serviceUser = deliusServiceUserFactory.build()
Expand Down Expand Up @@ -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<string, unknown>] = ['myTemplate', { foo: '1', bar: '2' }]
const contentView = { renderArgs }
const serviceUser = deliusServiceUserFactory.build()
Expand Down
5 changes: 3 additions & 2 deletions server/utils/controllerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ export default class ControllerUtils {
): Promise<void> {
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
}
}
Expand Down

0 comments on commit 7d6377a

Please sign in to comment.