Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VB-3733, VB-3792 User login and booker homepage - tests and code improvements #51

Merged
merged 9 commits into from
May 13, 2024
23 changes: 23 additions & 0 deletions integration_tests/e2e/bookingJourney.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import TestData from '../../server/routes/testutils/testData'
import HomePage from '../pages/home'
import Page from '../pages/page'

context('Booking journey', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubHmppsAuthToken')
})

it('should complete the booking journey', () => {
cy.task('stubGetBookerReference')
cy.task('stubGetPrisoners', { prisoners: [TestData.prisonerInfoDto()] })
cy.signIn()

const homePage = Page.verifyOnPage(HomePage)
homePage.prisonerName().contains('John Smith')
homePage.startButton().contains('Start')

// TODO add to this test as booking journey implemented
})
})
18 changes: 12 additions & 6 deletions integration_tests/e2e/govukOneLogin.cy.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import IndexPage from '../pages/index'
import HomePage from '../pages/home'
import GovukOneLoginPage from '../pages/govukOneLogin'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming for consistency

import Page from '../pages/page'

// TODO fix skipped tests!
context.skip('Sign in with GOV.UK One Login', () => {
context('Sign in with GOV.UK One Login', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
Expand All @@ -30,14 +29,18 @@ context.skip('Sign in with GOV.UK One Login', () => {
})

it('User can sign in and view home page', () => {
cy.task('stubHmppsAuthToken')
cy.task('stubGetBookerReference')
cy.task('stubGetPrisoners', {})
cy.signIn()

const indexPage = Page.verifyOnPage(IndexPage)
indexPage.prisonerName().contains('Adam Greene')
Page.verifyOnPage(HomePage)
})

it('User can request a specific page and be redirected to this after sign in', () => {
const page = '/deep-link' // will be a 404, but OK as testing original URL preserved
cy.task('stubHmppsAuthToken')
cy.task('stubGetBookerReference')
cy.signIn({ failOnStatusCode: false }, undefined, page)
cy.location('pathname').should('equal', page)
cy.contains('404')
Expand All @@ -50,8 +53,11 @@ context.skip('Sign in with GOV.UK One Login', () => {
})

it('User can log out', () => {
cy.task('stubHmppsAuthToken')
cy.task('stubGetBookerReference')
cy.task('stubGetPrisoners', {})
cy.signIn()
const indexPage = Page.verifyOnPage(IndexPage)
const indexPage = Page.verifyOnPage(HomePage)
indexPage.signOut().click()
Page.verifyOnPage(GovukOneLoginPage)
cy.contains('You have been logged out.')
Expand Down
21 changes: 2 additions & 19 deletions integration_tests/mockApis/hmppsAuth.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
import jwt from 'jsonwebtoken'

import { stubFor } from './wiremock'

const createToken = () => {
const payload = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing unnecessary complexity in this stub

sub: 'system_client_id',
grant_type: 'client_credentials',
scope: ['read', 'write'],
auth_source: 'none',
iss: 'http://localhost:9091/auth/auth/issuer',
authorities: ['ROLE_VISIT_SCHEDULER'],
jti: 'NBmv9IH_xw89YFE_tFoBwI1zo9Y',
client_id: 'system_client_id',
}
return jwt.sign(payload, 'secret', { expiresIn: '1h' })
}

const stubHmppsAuthToken = () =>
stubFor({
request: {
Expand All @@ -26,13 +10,12 @@ const stubHmppsAuthToken = () =>
status: 200,
headers: {
'Content-Type': 'application/json;charset=UTF-8',
Location: 'http://localhost:3007/sign-in/callback?code=codexxxx&state=stateyyyy',
},
jsonBody: {
access_token: createToken(),
access_token: 'hmpps-auth-token',
token_type: 'bearer',
expires_in: 599,
scope: 'read',
scope: 'read write',
sub: 'system_client_id',
auth_source: 'none',
jti: 'NBmv9IH_xw89YFE_tFoBwI1zo9Y',
Expand Down
34 changes: 34 additions & 0 deletions integration_tests/mockApis/orchestration.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,41 @@
import { SuperAgentRequest } from 'superagent'
import { stubFor } from './wiremock'
import TestData from '../../server/routes/testutils/testData'
import { BookerReference, PrisonerInfoDto } from '../../server/data/orchestrationApiTypes'

export default {
stubGetBookerReference: (bookerReference = TestData.bookerReference()): SuperAgentRequest =>
stubFor({
request: {
method: 'PUT',
url: '/orchestration/public/booker/register/auth',
},
response: {
status: 200,
headers: { 'Content-Type': 'application/json;charset=UTF-8' },
jsonBody: bookerReference,
},
}),

stubGetPrisoners: ({
bookerReference = TestData.bookerReference(),
prisoners = [],
}: {
bookerReference: BookerReference
prisoners: PrisonerInfoDto[]
}): SuperAgentRequest =>
stubFor({
request: {
method: 'GET',
url: `/orchestration/public/booker/${bookerReference.value}/prisoners`,
},
response: {
status: 200,
headers: { 'Content-Type': 'application/json;charset=UTF-8' },
jsonBody: prisoners,
},
}),

stubOrchestrationPing: (status = 200): SuperAgentRequest =>
stubFor({
request: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import Page, { PageElement } from './page'

export default class IndexPage extends Page {
export default class HomePage extends Page {
constructor() {
super('Book a visit')
}

headerPhaseBanner = (): PageElement => cy.get('[data-qa=header-phase-banner]')

prisonerName = (): PageElement => cy.get('[data-test=prisoner-name]')

startButton = (): PageElement => cy.get('[data-test="start-booking"]')
}
2 changes: 1 addition & 1 deletion server/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default function createApp(services: Services): express.Application {
nunjucksSetup(app, services.applicationInfo)
app.use(calendarPreview(services)) // TODO - remove
app.use(setupGovukOneLogin())
app.use(populateCurrentBooker(services.bookerService)) // TODO add this to appSetup.ts for tests?
app.use(populateCurrentBooker(services.bookerService))
app.use(setUpCsrf())

app.use(routes(services))
Expand Down
56 changes: 0 additions & 56 deletions server/data/tokenStore.test.ts

This file was deleted.

29 changes: 0 additions & 29 deletions server/data/tokenStore.ts

This file was deleted.

99 changes: 99 additions & 0 deletions server/middleware/populateCurrentBooker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import type { Request, Response } from 'express'
import { BadRequest, NotFound } from 'http-errors'
import { createMockBookerService } from '../services/testutils/mocks'
import TestData from '../routes/testutils/testData'
import populateCurrentBooker from './populateCurrentBooker'
import logger from '../../logger'

jest.mock('../../logger')

describe('populateCurrentBooker', () => {
let req: Request
let res: Response
const next = jest.fn()

const bookerService = createMockBookerService()

beforeEach(() => {
jest.resetAllMocks()

req = { session: {} } as unknown as Request

res = {
locals: {
user: {
sub: 'user1',
email: 'user1@example.com',
phone_number: '+440123456789',
},
},
redirect: jest.fn(),
} as unknown as Response
})

it('should get booker reference and add to session if it is not already set', async () => {
const bookerReference = TestData.bookerReference().value
bookerService.getBookerReference.mockResolvedValue(bookerReference)

await populateCurrentBooker(bookerService)(req, res, next)

expect(bookerService.getBookerReference).toHaveBeenCalledWith({
oneLoginSub: res.locals.user.sub,
email: res.locals.user.email,
phoneNumber: res.locals.user.phone_number,
})
expect(req.session).toStrictEqual({ booker: { reference: bookerReference } })
expect(res.redirect).not.toHaveBeenCalled()
expect(next).toHaveBeenCalled()
})

it('should not get booker reference if it is already set in session', async () => {
req.session.booker = { reference: TestData.bookerReference().value }

await populateCurrentBooker(bookerService)(req, res, next)

expect(bookerService.getBookerReference).not.toHaveBeenCalled()
expect(res.redirect).not.toHaveBeenCalled()
expect(next).toHaveBeenCalled()
})

it('should redirect to /sign-out if user details are not present in res.locals', async () => {
delete res.locals.user
await populateCurrentBooker(bookerService)(req, res, next)

expect(bookerService.getBookerReference).not.toHaveBeenCalled()
expect(res.redirect).toHaveBeenCalledWith('/sign-out')
expect(next).not.toHaveBeenCalled()
})

it('should handle the booker not being found (404 error) by logging and redirecting to /autherror', async () => {
bookerService.getBookerReference.mockRejectedValue(new NotFound())

await populateCurrentBooker(bookerService)(req, res, next)

expect(bookerService.getBookerReference).toHaveBeenCalledWith({
oneLoginSub: res.locals.user.sub,
email: res.locals.user.email,
phoneNumber: res.locals.user.phone_number,
})
expect(res.redirect).toHaveBeenCalledWith('/autherror')
expect(next).not.toHaveBeenCalled()
expect(logger.info).toHaveBeenCalledWith('Failed to retrieve booker reference for: user1')
})

it('should propagate any other errors', async () => {
const error = new BadRequest('Oops!')
bookerService.getBookerReference.mockRejectedValue(error)

await populateCurrentBooker(bookerService)(req, res, next)

expect(bookerService.getBookerReference).toHaveBeenCalledWith({
oneLoginSub: res.locals.user.sub,
email: res.locals.user.email,
phoneNumber: res.locals.user.phone_number,
})
expect(res.redirect).not.toHaveBeenCalled()
expect(logger.info).not.toHaveBeenCalled()
expect(next).toHaveBeenCalledWith(error)
})
})
19 changes: 12 additions & 7 deletions server/middleware/populateCurrentBooker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,31 @@ import logger from '../../logger'
import BookerService from '../services/bookerService'
import { AuthDetailDto } from '../data/orchestrationApiTypes'

// TODO add tests
export default function populateCurrentBooker(bookerService: BookerService): RequestHandler {
return async (req, res, next) => {
// TODO if there is no SUB and EMAIL then log user out?
try {
if (!res.locals.user || !res.locals.user.sub || !res.locals.user.email) {
return res.redirect('/sign-out')
}

if (!req.session.booker) {
const authDetailDto: AuthDetailDto = {
oneLoginSub: res.locals.user.sub,
email: res.locals.user.email,
phoneNumber: res.locals.user.phone_number,
}
const reference = await bookerService.getBookerReference(authDetailDto)

const reference = await bookerService.getBookerReference(authDetailDto)
req.session.booker = { reference }
}
next()
return next()
} catch (error) {
logger.error(error, `Failed to retrieve booker reference for: ${res.locals.user?.sub}`)
// TODO Here it should log out user?
next(error)
if (error.status === 404) {
logger.info(`Failed to retrieve booker reference for: ${res.locals.user.sub}`)
return res.redirect('/autherror')
}

return next(error)
}
}
}
Loading
Loading