diff --git a/docs/PLUGIN_OPTIONS.md b/docs/PLUGIN_OPTIONS.md index b74fa6c34..9cf4599e0 100644 --- a/docs/PLUGIN_OPTIONS.md +++ b/docs/PLUGIN_OPTIONS.md @@ -235,10 +235,9 @@ If provided, the `onRequest` plugin option will be invoked on each request to an ```ts export type OnRequestCallback = ( request: AnyFormRequest, - params: FormParams, - definition: FormDefinition, - metadata: FormMetadata -) => void + h: FormResponseToolkit, + context: FormContext +) => ResponseObject | FormResponseToolkit['continue'] | Promise ``` Here's an example of how it could be used to secure access to forms: @@ -247,13 +246,17 @@ Here's an example of how it could be used to secure access to forms: await server.register({ plugin, options: { - onRequest: (request , params, definition, metadata) => { - const { auth } = request + onRequest: async (request, h, context) => { + const { auth } = request - if (!auth.isAuthenticated) { - throw Boom.unauthorized() - } + // Check if user is authenticated + if (!auth.isAuthenticated) { + return h.redirect('/login').takeover() } + + // Return h.continue to resume with normal form processing + return h.continue + } } }) ``` diff --git a/src/server/plugins/engine/helpers.test.ts b/src/server/plugins/engine/helpers.test.ts index 323d645aa..6d8abc603 100644 --- a/src/server/plugins/engine/helpers.test.ts +++ b/src/server/plugins/engine/helpers.test.ts @@ -76,7 +76,8 @@ describe('Helpers', () => { h = { redirect: jest.fn().mockImplementation(() => response), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } }) diff --git a/src/server/plugins/engine/pageControllers/PageController.test.ts b/src/server/plugins/engine/pageControllers/PageController.test.ts index 8b30b4e98..10d5c52c8 100644 --- a/src/server/plugins/engine/pageControllers/PageController.test.ts +++ b/src/server/plugins/engine/pageControllers/PageController.test.ts @@ -160,7 +160,8 @@ describe('PageController', () => { const h: FormResponseToolkit = { redirect: jest.fn(), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } it('returns default route options', () => { diff --git a/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts b/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts index 32ef8a4e9..42c252ae8 100644 --- a/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/QuestionPageController.test.ts @@ -805,7 +805,8 @@ describe('QuestionPageController', () => { const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } it('returns default route options', () => { @@ -1373,7 +1374,8 @@ describe('QuestionPageController V2', () => { const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } it('returns default route options', () => { @@ -1532,7 +1534,8 @@ describe('Save and Exit functionality', () => { const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } beforeEach(() => { @@ -1663,7 +1666,8 @@ describe('Save and Exit functionality', () => { const mockH = { redirect: jest.fn().mockReturnValue(mockResponse), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } const postHandler = controller1.makePostRouteHandler() diff --git a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts index 3e3a367ef..2241cf805 100644 --- a/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts +++ b/src/server/plugins/engine/pageControllers/SummaryPageController.test.ts @@ -20,7 +20,8 @@ describe('SummaryPageController', () => { } const h: FormResponseToolkit = { redirect: jest.fn().mockReturnValue(response), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } beforeEach(() => { diff --git a/src/server/plugins/engine/plugin.ts b/src/server/plugins/engine/plugin.ts index 2c783d38d..a20451f16 100644 --- a/src/server/plugins/engine/plugin.ts +++ b/src/server/plugins/engine/plugin.ts @@ -34,7 +34,8 @@ export const plugin = { saveAndExit, nunjucks: nunjucksOptions, viewContext, - preparePageEventRequestOptions + preparePageEventRequestOptions, + onRequest } = options const cacheService = @@ -83,10 +84,15 @@ export const plugin = { ...getQuestionRoutes( getRouteOptions, postRouteOptions, - preparePageEventRequestOptions + preparePageEventRequestOptions, + onRequest + ), + ...getRepeaterSummaryRoutes(getRouteOptions, postRouteOptions, onRequest), + ...getRepeaterItemDeleteRoutes( + getRouteOptions, + postRouteOptions, + onRequest ), - ...getRepeaterSummaryRoutes(getRouteOptions, postRouteOptions), - ...getRepeaterItemDeleteRoutes(getRouteOptions, postRouteOptions), ...getFileUploadStatusRoutes() ] diff --git a/src/server/plugins/engine/routes/index.test.ts b/src/server/plugins/engine/routes/index.test.ts new file mode 100644 index 000000000..826ccef48 --- /dev/null +++ b/src/server/plugins/engine/routes/index.test.ts @@ -0,0 +1,316 @@ +import Boom from '@hapi/boom' +import { type ResponseObject, type ResponseToolkit } from '@hapi/hapi' + +import { + findPage, + getCacheService, + getPage, + proceed +} from '~/src/server/plugins/engine/helpers.js' +import { type FormModel } from '~/src/server/plugins/engine/models/FormModel.js' +import { type PageControllerClass } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js' +import { redirectOrMakeHandler } from '~/src/server/plugins/engine/routes/index.js' +import { + type AnyFormRequest, + type OnRequestCallback +} from '~/src/server/plugins/engine/types.js' +import { type FormResponseToolkit } from '~/src/server/routes/types.js' + +jest.mock('~/src/server/plugins/engine/helpers') + +describe('redirectOrMakeHandler', () => { + const mockServer = {} as unknown as Parameters< + typeof redirectOrMakeHandler + >[0]['server'] + const mockRequest: AnyFormRequest = { + server: mockServer, + app: {}, + params: { path: 'test-path' }, + query: {} + } as unknown as AnyFormRequest + + const mockH: FormResponseToolkit = { + redirect: jest.fn(), + view: jest.fn(), + continue: Symbol('continue') + } as unknown as FormResponseToolkit + + let mockPage: PageControllerClass + + const mockModel: FormModel = { + def: { + metadata: { + submission: { code: 'TEST-CODE' } + } as { submission: { code: string } } + }, + getFormContext: jest.fn().mockReturnValue({ + isForceAccess: false, + data: {} + }) + } as unknown as FormModel + + const mockMakeHandler = jest + .fn() + .mockResolvedValue({ statusCode: 200 } as ResponseObject) + + beforeEach(() => { + jest.clearAllMocks() + mockRequest.app = { model: mockModel } + + // Reset mock page + mockPage = { + getState: jest.fn().mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + mergeState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + getRelevantPath: jest.fn().mockReturnValue('/test-path'), + getSummaryPath: jest.fn().mockReturnValue('/summary'), + getHref: jest.fn().mockReturnValue('/test-href'), + path: '/test-path' + } as unknown as PageControllerClass + + // Reset mock model + mockModel.getFormContext = jest.fn().mockReturnValue({ + isForceAccess: false, + data: {} + }) + + // Setup mocks + ;(getCacheService as jest.Mock).mockReturnValue({ + getFlash: jest.fn().mockReturnValue({ errors: [] }) + }) + ;(getPage as jest.Mock).mockReturnValue(mockPage) + ;(findPage as jest.Mock).mockReturnValue({ next: [] }) + ;(proceed as jest.Mock).mockReturnValue({ statusCode: 302 }) + }) + + describe('onRequest callback functionality', () => { + it('should call onRequest callback when provided', async () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(undefined) + + await redirectOrMakeHandler( + mockRequest, + mockH, + onRequestCallback, + mockMakeHandler + ) + + expect(onRequestCallback).toHaveBeenCalledWith( + mockRequest, + mockH as ResponseToolkit, + expect.objectContaining({ + isForceAccess: false, + data: {} + }) + ) + }) + + it('should not call onRequest callback when not provided', async () => { + const onRequestCallback = jest.fn() + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + expect(onRequestCallback).not.toHaveBeenCalled() + }) + + it('should return takeover response when onRequest returns takeover response', async () => { + const takeoverResponse = { + statusCode: 302, + headers: { location: '/redirect-url' }, + _takeover: true + } as unknown as ResponseObject + + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(takeoverResponse) + + const result = await redirectOrMakeHandler( + mockRequest, + mockH, + onRequestCallback, + mockMakeHandler + ) + + expect(result).toBe(takeoverResponse) + expect(mockMakeHandler).not.toHaveBeenCalled() + }) + + it('should continue processing when onRequest returns h.continue', async () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(mockH.continue) + + await redirectOrMakeHandler( + mockRequest, + mockH, + onRequestCallback, + mockMakeHandler + ) + + expect(mockMakeHandler).toHaveBeenCalledWith(mockPage, expect.any(Object)) + }) + + it('should handle onRequest callback errors', async () => { + const error = new Error('onRequest callback error') + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockRejectedValue(error) + + await expect( + redirectOrMakeHandler( + mockRequest, + mockH, + onRequestCallback, + mockMakeHandler + ) + ).rejects.toThrow('onRequest callback error') + }) + }) + + describe('existing functionality', () => { + it('should throw error when model is missing', async () => { + mockRequest.app = {} + + await expect( + redirectOrMakeHandler(mockRequest, mockH, undefined, mockMakeHandler) + ).rejects.toThrow(Boom.notFound('No model found for /test-path')) + }) + + it('should call makeHandler when page is relevant', async () => { + const testPage = { + getState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + mergeState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + getSummaryPath: jest.fn().mockReturnValue('/summary'), + getHref: jest.fn().mockReturnValue('/test-href'), + getRelevantPath: jest.fn().mockReturnValue('/test-path'), + path: '/test-path' + } as unknown as PageControllerClass + ;(getPage as jest.Mock).mockReturnValue(testPage) + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + expect(mockMakeHandler).toHaveBeenCalledWith(testPage, expect.any(Object)) + }) + + it('should call makeHandler when context has force access', async () => { + mockModel.getFormContext = jest.fn().mockReturnValue({ + isForceAccess: true, + data: {} + }) + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + expect(mockMakeHandler).toHaveBeenCalledWith(mockPage, expect.any(Object)) + }) + + it('should redirect when page is not relevant', async () => { + const testPage = { + getState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + mergeState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + getSummaryPath: jest.fn().mockReturnValue('/summary'), + getHref: jest.fn().mockReturnValue('/test-href'), + getRelevantPath: jest.fn().mockReturnValue('/other-path'), + path: '/test-path' + } as unknown as PageControllerClass + ;(getPage as jest.Mock).mockReturnValue(testPage) + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + expect(proceed).toHaveBeenCalledWith(mockRequest, mockH, '/test-href') + expect(mockMakeHandler).not.toHaveBeenCalled() + }) + + it('should set returnUrl when redirecting and next pages exist', async () => { + const testPage = { + getState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + mergeState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + getSummaryPath: jest.fn().mockReturnValue('/summary'), + getRelevantPath: jest.fn().mockReturnValue('/other-path'), + path: '/test-path', + getHref: jest + .fn() + .mockReturnValueOnce('/summary-href') // First call: for summaryPath (returnUrl) + .mockReturnValueOnce('/relevant-path-href') // Second call: for relevantPath (redirect) + } as unknown as PageControllerClass + ;(getPage as jest.Mock).mockReturnValue(testPage) + ;(findPage as jest.Mock).mockReturnValue({ next: ['next-page'] }) + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + expect(mockRequest.query.returnUrl).toBe('/summary-href') + expect(proceed).toHaveBeenCalledWith( + mockRequest, + mockH, + '/relevant-path-href' + ) + }) + + it('should not set returnUrl when redirecting and no next pages exist', async () => { + const testPage = { + getState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + mergeState: jest + .fn() + .mockResolvedValue({ $$__referenceNumber: 'REF-123' }), + getSummaryPath: jest.fn().mockReturnValue('/summary'), + getHref: jest.fn().mockReturnValue('/test-href'), + getRelevantPath: jest.fn().mockReturnValue('/other-path'), + path: '/test-path' + } as unknown as PageControllerClass + ;(getPage as jest.Mock).mockReturnValue(testPage) + const returnUrlBefore = mockRequest.query.returnUrl + ;(findPage as jest.Mock).mockReturnValue({ next: [] }) + + await redirectOrMakeHandler( + mockRequest, + mockH, + undefined, + mockMakeHandler + ) + + // returnUrl should not be set if next pages don't exist + expect(mockRequest.query.returnUrl).toBe(returnUrlBefore) + expect(proceed).toHaveBeenCalledWith(mockRequest, mockH, '/test-href') + }) + }) +}) diff --git a/src/server/plugins/engine/routes/index.ts b/src/server/plugins/engine/routes/index.ts index 93ea2584c..37225060a 100644 --- a/src/server/plugins/engine/routes/index.ts +++ b/src/server/plugins/engine/routes/index.ts @@ -23,6 +23,7 @@ import * as defaultServices from '~/src/server/plugins/engine/services/index.js' import { type AnyFormRequest, type FormContext, + type OnRequestCallback, type PluginOptions } from '~/src/server/plugins/engine/types.js' import { @@ -33,6 +34,7 @@ import { export async function redirectOrMakeHandler( request: AnyFormRequest, h: FormResponseToolkit, + onRequest: OnRequestCallback | undefined, makeHandler: ( page: PageControllerClass, context: FormContext @@ -69,6 +71,14 @@ export async function redirectOrMakeHandler( const relevantPath = page.getRelevantPath(request, context) const summaryPath = page.getSummaryPath() + // Call the onRequest callback if it has been supplied + if (onRequest) { + const result = await onRequest(request, h, context) + if (result !== h.continue) { + return result + } + } + // Return handler for relevant pages or preview URL direct access if (relevantPath.startsWith(page.path) || context.isForceAccess) { return makeHandler(page, context) @@ -89,7 +99,7 @@ export function makeLoadFormPreHandler(server: Server, options: PluginOptions) { // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- hapi types are wrong const prefix = server.realm.modifiers.route.prefix ?? '' - const { services = defaultServices, controllers, onRequest } = options + const { services = defaultServices, controllers } = options const { formsService } = services @@ -166,11 +176,6 @@ export function makeLoadFormPreHandler(server: Server, options: PluginOptions) { server.app.models.set(key, item) } - // Call the onRequest callback if it has been supplied - if (onRequest) { - onRequest(request, params, item.model.def, metadata) - } - // Assign the model to the request data // for use in the downstream handler request.app.model = item.model diff --git a/src/server/plugins/engine/routes/questions.test.ts b/src/server/plugins/engine/routes/questions.test.ts index c9940ed5d..9b5e0ebe3 100644 --- a/src/server/plugins/engine/routes/questions.test.ts +++ b/src/server/plugins/engine/routes/questions.test.ts @@ -40,8 +40,14 @@ jest.mock('~/src/server/plugins/engine/routes/index') describe('makeGetHandler', () => { const hMock: FormResponseToolkit = { - redirect: jest.fn(), - view: jest.fn() + redirect: jest.fn().mockReturnValue({ + takeover: jest.fn().mockReturnValue({ + statusCode: 302, + headers: { location: '/redirect-url' } + }) + }), + view: jest.fn(), + continue: Symbol('continue') } beforeEach(() => { @@ -84,8 +90,9 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => - Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, _onRequest, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makeGetHandler()(requestMock, hMock) @@ -126,8 +133,9 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => - Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, _onRequest, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makeGetHandler()(requestMock, hMock) @@ -164,7 +172,12 @@ describe('makeGetHandler', () => { jest .mocked(redirectOrMakeHandler) .mockImplementation( - async (_req: AnyFormRequest, _h: FormResponseToolkit, fn) => { + async ( + _req: AnyFormRequest, + _h: FormResponseToolkit, + _onRequest, + fn + ) => { try { await fn(pageMock, contextMock) } catch (err) { @@ -179,12 +192,57 @@ describe('makeGetHandler', () => { expect(error).toEqual(Boom.notFound('No model found for /some-path')) }) + + it('should pass onRequest callback to redirectOrMakeHandler', async () => { + const onRequestCallback = jest.fn().mockResolvedValue(undefined) + const modelMock = { + basePath: 'some-base-path', + def: { name: 'Hello world' } + } as FormModel + + const pageMock = createMockPageController( + modelMock, + ( + _request: FormRequest, + _context: FormContext, + _h: FormResponseToolkit + ) => { + return Promise.resolve({} as unknown as ResponseObject) + } + ) + + const contextMock = { data: {}, model: {} } as unknown as FormContext + + const requestMock = { + params: { path: 'some-path' }, + app: { model: modelMock } + } as FormRequest + + jest + .mocked(redirectOrMakeHandler) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, onRequest, fn) => { + expect(onRequest).toBe(onRequestCallback) + return Promise.resolve(fn(pageMock, contextMock)) + } + ) + + await makeGetHandler(undefined, onRequestCallback)(requestMock, hMock) + + expect(redirectOrMakeHandler).toHaveBeenCalledWith( + requestMock, + hMock, + onRequestCallback, + expect.any(Function) + ) + }) }) describe('makePostHandler', () => { const hMock: FormResponseToolkit = { redirect: jest.fn(), - view: jest.fn() + view: jest.fn(), + continue: Symbol('continue') } beforeEach(() => { @@ -230,8 +288,9 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => - Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, _onRequest, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) const response = await makePostHandler()(requestMock, hMock) @@ -269,8 +328,9 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => - Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, _onRequest, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makePostHandler()(requestMock, hMock) @@ -310,8 +370,9 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) - .mockImplementation((_req: AnyFormRequest, _h: FormResponseToolkit, fn) => - Promise.resolve(fn(pageMock, contextMock)) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, _onRequest, fn) => + Promise.resolve(fn(pageMock, contextMock)) ) await makePostHandler()(requestMock, hMock) @@ -349,7 +410,12 @@ describe('makePostHandler', () => { jest .mocked(redirectOrMakeHandler) .mockImplementation( - async (_req: AnyFormRequest, _h: FormResponseToolkit, fn) => { + async ( + _req: AnyFormRequest, + _h: FormResponseToolkit, + _onRequest, + fn + ) => { try { await fn(pageMock, contextMock) } catch (err) { @@ -364,6 +430,51 @@ describe('makePostHandler', () => { expect(error).toEqual(Boom.notFound('No model found for /some-path')) }) + + it('should pass onRequest callback to redirectOrMakeHandler', async () => { + const onRequestCallback = jest.fn().mockResolvedValue(undefined) + const modelMock = { + basePath: 'some-base-path', + def: { name: 'Hello world' } + } as FormModel + + const pageMock = createMockPageController( + modelMock, + ( + _request: FormRequest, + _context: FormContext, + _h: FormResponseToolkit + ) => { + return Promise.resolve({} as unknown as ResponseObject) + } + ) + + const contextMock = { data: {}, model: {} } as unknown as FormContext + + const requestMock = { + params: { path: 'some-path' }, + app: { model: modelMock }, + payload: { some: 'payload' } + } as unknown as FormRequestPayload + + jest + .mocked(redirectOrMakeHandler) + .mockImplementation( + (_req: AnyFormRequest, _h: FormResponseToolkit, onRequest, fn) => { + expect(onRequest).toBe(onRequestCallback) + return Promise.resolve(fn(pageMock, contextMock)) + } + ) + + await makePostHandler(undefined, onRequestCallback)(requestMock, hMock) + + expect(redirectOrMakeHandler).toHaveBeenCalledWith( + requestMock, + hMock, + onRequestCallback, + expect.any(Function) + ) + }) }) function createMockPageController( diff --git a/src/server/plugins/engine/routes/questions.ts b/src/server/plugins/engine/routes/questions.ts index bdc9a9659..c6499b67e 100644 --- a/src/server/plugins/engine/routes/questions.ts +++ b/src/server/plugins/engine/routes/questions.ts @@ -26,6 +26,7 @@ import { import { type AnyFormRequest, type FormContext, + type OnRequestCallback, type PreparePageEventRequestOptions } from '~/src/server/plugins/engine/types.js' import { @@ -74,7 +75,8 @@ async function handleHttpEvent( } export function makeGetHandler( - preparePageEventRequestOptions?: PreparePageEventRequestOptions + preparePageEventRequestOptions?: PreparePageEventRequestOptions, + onRequest?: OnRequestCallback ) { return function getHandler(request: FormRequest, h: FormResponseToolkit) { const { params } = request @@ -83,34 +85,40 @@ export function makeGetHandler( return dispatchHandler(request, h) } - return redirectOrMakeHandler(request, h, async (page, context) => { - // Check for a page onLoad HTTP event and if one exists, - // call it and assign the response to the context data - const { events } = page - const { model } = request.app + return redirectOrMakeHandler( + request, + h, + onRequest, + async (page, context) => { + // Check for a page onLoad HTTP event and if one exists, + // call it and assign the response to the context data + const { events } = page + const { model } = request.app - if (!model) { - throw Boom.notFound(`No model found for /${params.path}`) - } + if (!model) { + throw Boom.notFound(`No model found for /${params.path}`) + } - if (events?.onLoad && events.onLoad.type === 'http') { - await handleHttpEvent( - request, - page, - context, - events.onLoad, - model, - preparePageEventRequestOptions - ) - } + if (events?.onLoad && events.onLoad.type === 'http') { + await handleHttpEvent( + request, + page, + context, + events.onLoad, + model, + preparePageEventRequestOptions + ) + } - return page.makeGetRouteHandler()(request, context, h) - }) + return page.makeGetRouteHandler()(request, context, h) + } + ) } } export function makePostHandler( - preparePageEventRequestOptions?: PreparePageEventRequestOptions + preparePageEventRequestOptions?: PreparePageEventRequestOptions, + onRequest?: OnRequestCallback ) { return function postHandler( request: FormRequestPayload, @@ -118,40 +126,45 @@ export function makePostHandler( ) { const { query } = request - return redirectOrMakeHandler(request, h, async (page, context) => { - const { pageDef } = page - const { isForceAccess } = context - const { model } = request.app - const { events } = page + return redirectOrMakeHandler( + request, + h, + onRequest, + async (page, context) => { + const { pageDef } = page + const { isForceAccess } = context + const { model } = request.app + const { events } = page - // Redirect to GET for preview URL direct access - if (isForceAccess && !hasFormComponents(pageDef)) { - return proceed(request, h, redirectPath(page.href, query)) - } + // Redirect to GET for preview URL direct access + if (isForceAccess && !hasFormComponents(pageDef)) { + return proceed(request, h, redirectPath(page.href, query)) + } - if (!model) { - throw Boom.notFound(`No model found for /${request.params.path}`) - } + if (!model) { + throw Boom.notFound(`No model found for /${request.params.path}`) + } - const response = await page.makePostRouteHandler()(request, context, h) + const response = await page.makePostRouteHandler()(request, context, h) - if ( - events?.onSave && - events.onSave.type === 'http' && - isSuccessful(response) - ) { - await handleHttpEvent( - request, - page, - context, - events.onSave, - model, - preparePageEventRequestOptions - ) - } + if ( + events?.onSave && + events.onSave.type === 'http' && + isSuccessful(response) + ) { + await handleHttpEvent( + request, + page, + context, + events.onSave, + model, + preparePageEventRequestOptions + ) + } - return response - }) + return response + } + ) } } @@ -164,13 +177,14 @@ function isSuccessful(response: ResponseObject): boolean { export function getRoutes( getRouteOptions: RouteOptions, postRouteOptions: RouteOptions, - preparePageEventRequestOptions?: PreparePageEventRequestOptions + preparePageEventRequestOptions?: PreparePageEventRequestOptions, + onRequest?: OnRequestCallback ): (ServerRoute | ServerRoute)[] { return [ { method: 'get', path: '/{slug}', - handler: makeGetHandler(preparePageEventRequestOptions), + handler: makeGetHandler(preparePageEventRequestOptions, onRequest), options: { ...getRouteOptions, validate: { @@ -197,7 +211,7 @@ export function getRoutes( { method: 'get', path: '/{slug}/{path}/{itemId?}', - handler: makeGetHandler(preparePageEventRequestOptions), + handler: makeGetHandler(preparePageEventRequestOptions, onRequest), options: { ...getRouteOptions, validate: { @@ -212,7 +226,7 @@ export function getRoutes( { method: 'get', path: '/preview/{state}/{slug}/{path}/{itemId?}', - handler: makeGetHandler(preparePageEventRequestOptions), + handler: makeGetHandler(preparePageEventRequestOptions, onRequest), options: { ...getRouteOptions, validate: { @@ -228,7 +242,7 @@ export function getRoutes( { method: 'post', path: '/{slug}/{path}/{itemId?}', - handler: makePostHandler(preparePageEventRequestOptions), + handler: makePostHandler(preparePageEventRequestOptions, onRequest), options: { ...postRouteOptions, validate: { @@ -250,7 +264,7 @@ export function getRoutes( { method: 'post', path: '/preview/{state}/{slug}/{path}/{itemId?}', - handler: makePostHandler(preparePageEventRequestOptions), + handler: makePostHandler(preparePageEventRequestOptions, onRequest), options: { ...postRouteOptions, validate: { diff --git a/src/server/plugins/engine/routes/repeaters/item-delete.test.ts b/src/server/plugins/engine/routes/repeaters/item-delete.test.ts new file mode 100644 index 000000000..7d272e691 --- /dev/null +++ b/src/server/plugins/engine/routes/repeaters/item-delete.test.ts @@ -0,0 +1,83 @@ +import { getRoutes } from '~/src/server/plugins/engine/routes/repeaters/item-delete.js' +import { type OnRequestCallback } from '~/src/server/plugins/engine/types.js' + +describe('repeater item-delete routes', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + describe('route configuration', () => { + it('should return all expected routes', () => { + const routes = getRoutes({}, {}, undefined) + + expect(routes).toHaveLength(4) + + const expectedPaths = [ + '/{slug}/{path}/{itemId}/confirm-delete', + '/preview/{state}/{slug}/{path}/{itemId}/confirm-delete' + ] + + expectedPaths.forEach((path) => { + const getRoute = routes.find( + (route) => route.method === 'get' && route.path === path + ) + const postRoute = routes.find( + (route) => route.method === 'post' && route.path === path + ) + + expect(getRoute).toBeDefined() + expect(postRoute).toBeDefined() + }) + }) + + it('should pass onRequest callback to handlers', () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(undefined) + const routes = getRoutes({}, {}, onRequestCallback) + + // Test that the handlers are created with the onRequest callback + const getRoute = routes.find( + (route) => + route.method === 'get' && + route.path === '/{slug}/{path}/{itemId}/confirm-delete' + ) + const postRoute = routes.find( + (route) => + route.method === 'post' && + route.path === '/{slug}/{path}/{itemId}/confirm-delete' + ) + + expect(getRoute?.handler).toBeDefined() + expect(postRoute?.handler).toBeDefined() + }) + }) + + describe('handler functionality', () => { + it('should create handlers that accept onRequest callback', () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(undefined) + const routes = getRoutes({}, {}, onRequestCallback) + + // Test that the handlers are created with the onRequest callback + const getRoute = routes.find( + (route) => + route.method === 'get' && + route.path === '/{slug}/{path}/{itemId}/confirm-delete' + ) + const postRoute = routes.find( + (route) => + route.method === 'post' && + route.path === '/{slug}/{path}/{itemId}/confirm-delete' + ) + + expect(getRoute?.handler).toBeDefined() + expect(postRoute?.handler).toBeDefined() + + // Test that handlers are functions + expect(typeof getRoute?.handler).toBe('function') + expect(typeof postRoute?.handler).toBe('function') + }) + }) +}) diff --git a/src/server/plugins/engine/routes/repeaters/item-delete.ts b/src/server/plugins/engine/routes/repeaters/item-delete.ts index f1d06cd40..fef152f85 100644 --- a/src/server/plugins/engine/routes/repeaters/item-delete.ts +++ b/src/server/plugins/engine/routes/repeaters/item-delete.ts @@ -6,6 +6,7 @@ import Joi from 'joi' import { FileUploadPageController } from '~/src/server/plugins/engine/pageControllers/FileUploadPageController.js' import { RepeatPageController } from '~/src/server/plugins/engine/pageControllers/RepeatPageController.js' import { redirectOrMakeHandler } from '~/src/server/plugins/engine/routes/index.js' +import { type OnRequestCallback } from '~/src/server/plugins/engine/types.js' import { type FormRequest, type FormRequestPayload, @@ -23,52 +24,57 @@ import { } from '~/src/server/schemas/index.js' // Item delete GET route -function getHandler(request: FormRequest, h: FormResponseToolkit) { - const { params } = request +function getHandler(onRequest?: OnRequestCallback) { + return async function (request: FormRequest, h: FormResponseToolkit) { + const { params } = request - return redirectOrMakeHandler(request, h, (page, context) => { - if ( - !( - page instanceof RepeatPageController || - page instanceof FileUploadPageController - ) - ) { - throw Boom.notFound(`No page found for /${params.path}`) - } + return redirectOrMakeHandler(request, h, onRequest, (page, context) => { + if ( + !( + page instanceof RepeatPageController || + page instanceof FileUploadPageController + ) + ) { + throw Boom.notFound(`No page found for /${params.path}`) + } - return page.makeGetItemDeleteRouteHandler()(request, context, h) - }) + return page.makeGetItemDeleteRouteHandler()(request, context, h) + }) + } } -function postHandler(request: FormRequestPayload, h: FormResponseToolkit) { - const { params } = request +function postHandler(onRequest?: OnRequestCallback) { + return async function (request: FormRequestPayload, h: FormResponseToolkit) { + const { params } = request - return redirectOrMakeHandler(request, h, (page, context) => { - const { isForceAccess } = context + return redirectOrMakeHandler(request, h, onRequest, (page, context) => { + const { isForceAccess } = context - if ( - isForceAccess || - !( - page instanceof RepeatPageController || - page instanceof FileUploadPageController - ) - ) { - throw Boom.notFound(`No page found for /${params.path}`) - } + if ( + isForceAccess || + !( + page instanceof RepeatPageController || + page instanceof FileUploadPageController + ) + ) { + throw Boom.notFound(`No page found for /${params.path}`) + } - return page.makePostItemDeleteRouteHandler()(request, context, h) - }) + return page.makePostItemDeleteRouteHandler()(request, context, h) + }) + } } export function getRoutes( getRouteOptions: RouteOptions, - postRouteOptions: RouteOptions + postRouteOptions: RouteOptions, + onRequest?: OnRequestCallback ): (ServerRoute | ServerRoute)[] { return [ { method: 'get', path: '/{slug}/{path}/{itemId}/confirm-delete', - handler: getHandler, + handler: getHandler(onRequest), options: { ...getRouteOptions, validate: { @@ -84,7 +90,7 @@ export function getRoutes( { method: 'get', path: '/preview/{state}/{slug}/{path}/{itemId}/confirm-delete', - handler: getHandler, + handler: getHandler(onRequest), options: { ...getRouteOptions, validate: { @@ -101,7 +107,7 @@ export function getRoutes( { method: 'post', path: '/{slug}/{path}/{itemId}/confirm-delete', - handler: postHandler, + handler: postHandler(onRequest), options: { ...postRouteOptions, validate: { @@ -124,7 +130,7 @@ export function getRoutes( { method: 'post', path: '/preview/{state}/{slug}/{path}/{itemId}/confirm-delete', - handler: postHandler, + handler: postHandler(onRequest), options: { ...postRouteOptions, validate: { diff --git a/src/server/plugins/engine/routes/repeaters/summary.test.ts b/src/server/plugins/engine/routes/repeaters/summary.test.ts new file mode 100644 index 000000000..379f97f4c --- /dev/null +++ b/src/server/plugins/engine/routes/repeaters/summary.test.ts @@ -0,0 +1,75 @@ +import { getRoutes } from '~/src/server/plugins/engine/routes/repeaters/summary.js' +import { type OnRequestCallback } from '~/src/server/plugins/engine/types.js' + +describe('repeater summary routes', () => { + describe('route configuration', () => { + it('should return all expected routes', () => { + const routes = getRoutes({}, {}, undefined) + + expect(routes).toHaveLength(4) + + const expectedPaths = [ + '/{slug}/{path}/summary', + '/preview/{state}/{slug}/{path}/summary' + ] + + expectedPaths.forEach((path) => { + const getRoute = routes.find( + (route) => route.method === 'get' && route.path === path + ) + const postRoute = routes.find( + (route) => route.method === 'post' && route.path === path + ) + + expect(getRoute).toBeDefined() + expect(postRoute).toBeDefined() + }) + }) + + it('should pass onRequest callback to handlers', () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(undefined) + const routes = getRoutes({}, {}, onRequestCallback) + + // Test that the handlers are created with the onRequest callback + const getRoute = routes.find( + (route) => + route.method === 'get' && route.path === '/{slug}/{path}/summary' + ) + const postRoute = routes.find( + (route) => + route.method === 'post' && route.path === '/{slug}/{path}/summary' + ) + + expect(getRoute?.handler).toBeDefined() + expect(postRoute?.handler).toBeDefined() + }) + }) + + describe('handler functionality', () => { + it('should create handlers that accept onRequest callback', () => { + const onRequestCallback: OnRequestCallback = jest + .fn() + .mockResolvedValue(undefined) + const routes = getRoutes({}, {}, onRequestCallback) + + // Test that the handlers are created with the onRequest callback + const getRoute = routes.find( + (route) => + route.method === 'get' && route.path === '/{slug}/{path}/summary' + ) + const postRoute = routes.find( + (route) => + route.method === 'post' && route.path === '/{slug}/{path}/summary' + ) + + expect(getRoute?.handler).toBeDefined() + expect(postRoute?.handler).toBeDefined() + + // Test that handlers are functions + expect(typeof getRoute?.handler).toBe('function') + expect(typeof postRoute?.handler).toBe('function') + }) + }) +}) diff --git a/src/server/plugins/engine/routes/repeaters/summary.ts b/src/server/plugins/engine/routes/repeaters/summary.ts index 3126ed9d2..9c52f7c5b 100644 --- a/src/server/plugins/engine/routes/repeaters/summary.ts +++ b/src/server/plugins/engine/routes/repeaters/summary.ts @@ -6,6 +6,7 @@ import Joi from 'joi' import { RepeatPageController } from '~/src/server/plugins/engine/pageControllers/RepeatPageController.js' import { redirectOrMakeHandler } from '~/src/server/plugins/engine/routes/index.js' +import { type OnRequestCallback } from '~/src/server/plugins/engine/types.js' import { type FormRequest, type FormRequestPayload, @@ -20,41 +21,46 @@ import { stateSchema } from '~/src/server/schemas/index.js' -function getHandler(request: FormRequest, h: FormResponseToolkit) { - const { params } = request +function getHandler(onRequest?: OnRequestCallback) { + return async function (request: FormRequest, h: FormResponseToolkit) { + const { params } = request - return redirectOrMakeHandler(request, h, (page, context) => { - if (!(page instanceof RepeatPageController)) { - throw Boom.notFound(`No repeater page found for /${params.path}`) - } + return redirectOrMakeHandler(request, h, onRequest, (page, context) => { + if (!(page instanceof RepeatPageController)) { + throw Boom.notFound(`No repeater page found for /${params.path}`) + } - return page.makeGetListSummaryRouteHandler()(request, context, h) - }) + return page.makeGetListSummaryRouteHandler()(request, context, h) + }) + } } -function postHandler(request: FormRequestPayload, h: FormResponseToolkit) { - const { params } = request +function postHandler(onRequest?: OnRequestCallback) { + return async function (request: FormRequestPayload, h: FormResponseToolkit) { + const { params } = request - return redirectOrMakeHandler(request, h, (page, context) => { - const { isForceAccess } = context + return redirectOrMakeHandler(request, h, onRequest, (page, context) => { + const { isForceAccess } = context - if (isForceAccess || !(page instanceof RepeatPageController)) { - throw Boom.notFound(`No repeater page found for /${params.path}`) - } + if (isForceAccess || !(page instanceof RepeatPageController)) { + throw Boom.notFound(`No repeater page found for /${params.path}`) + } - return page.makePostListSummaryRouteHandler()(request, context, h) - }) + return page.makePostListSummaryRouteHandler()(request, context, h) + }) + } } export function getRoutes( getRouteOptions: RouteOptions, - postRouteOptions: RouteOptions + postRouteOptions: RouteOptions, + onRequest?: OnRequestCallback ): (ServerRoute | ServerRoute)[] { return [ { method: 'get', path: '/{slug}/{path}/summary', - handler: getHandler, + handler: getHandler(onRequest), options: { ...getRouteOptions, validate: { @@ -69,7 +75,7 @@ export function getRoutes( { method: 'get', path: '/preview/{state}/{slug}/{path}/summary', - handler: getHandler, + handler: getHandler(onRequest), options: { ...getRouteOptions, validate: { @@ -85,7 +91,7 @@ export function getRoutes( { method: 'post', path: '/{slug}/{path}/summary', - handler: postHandler, + handler: postHandler(onRequest), options: { ...postRouteOptions, validate: { @@ -106,7 +112,7 @@ export function getRoutes( { method: 'post', path: '/preview/{state}/{slug}/{path}/summary', - handler: postHandler, + handler: postHandler(onRequest), options: { ...postRouteOptions, validate: { diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 1ff21d013..00f0e394f 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -1,8 +1,6 @@ import { type ComponentDef, type Event, - type FormDefinition, - type FormMetadata, type FormVersionMetadata, type Item, type List, @@ -38,7 +36,6 @@ import { import { type ViewContext } from '~/src/server/plugins/nunjucks/types.js' import { type FormAction, - type FormParams, type FormRequest, type FormRequestPayload, type FormResponseToolkit, @@ -368,10 +365,12 @@ export type PreparePageEventRequestOptions = ( export type OnRequestCallback = ( request: AnyFormRequest, - params: FormParams, - definition: FormDefinition, - metadata: FormMetadata -) => void + h: FormResponseToolkit, + context: FormContext +) => + | ResponseObject + | FormResponseToolkit['continue'] + | Promise export type SaveAndExitHandler = ( request: FormRequestPayload, diff --git a/src/server/routes/types.ts b/src/server/routes/types.ts index 5639840fb..704fccac6 100644 --- a/src/server/routes/types.ts +++ b/src/server/routes/types.ts @@ -37,7 +37,10 @@ export interface FormRequestPayloadRefs extends FormRequestRefs { export type FormRequest = Request export type FormRequestPayload = Request -export type FormResponseToolkit = Pick +export type FormResponseToolkit = Pick< + ResponseToolkit, + 'redirect' | 'view' | 'continue' +> export enum FormAction { Continue = 'continue', diff --git a/test/form/auth.test.js b/test/form/auth.test.js index 3d55666a0..372de169f 100644 --- a/test/form/auth.test.js +++ b/test/form/auth.test.js @@ -1,7 +1,6 @@ import { join } from 'node:path' import basic from '@hapi/basic' -import Boom from '@hapi/boom' import { StatusCodes } from 'http-status-codes' import { FORM_PREFIX } from '~/src/server/constants.js' @@ -82,12 +81,14 @@ describe('Auth', () => { server = await createServer({ services, - onRequest: (request /*, params, definition, metadata */) => { + onRequest: (request, h, _context) => { const { auth } = request if (!auth.isAuthenticated) { - throw Boom.unauthorized() + return h.redirect('/unauthorized').takeover() } + + return h.continue } }) @@ -112,7 +113,8 @@ describe('Auth', () => { url: `${basePath}/first-page` }) - expect(response.statusCode).toBe(StatusCodes.UNAUTHORIZED) + expect(response.statusCode).toBe(StatusCodes.MOVED_TEMPORARILY) + expect(response.headers.location).toBe('/unauthorized') }) test('authenticated get request to restricted form returns 200 OK', async () => {