From 25fabd6a3b152cde1509446e9e38d2900cdcdb2e Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 23 Oct 2024 14:42:30 +0100 Subject: [PATCH 1/7] check: 'back' link takes user to referer, when arriving deep link --- src/controllers/deepLinkController.js | 7 +++-- src/controllers/pageController.js | 30 +++++++++++++++--- test/unit/PageController.test.js | 45 +++++++++++++++++++++++++++ test/unit/deepLinkController.test.js | 2 +- 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/controllers/deepLinkController.js b/src/controllers/deepLinkController.js index cb2bf139..0fd21146 100644 --- a/src/controllers/deepLinkController.js +++ b/src/controllers/deepLinkController.js @@ -33,8 +33,11 @@ class DeepLinkController extends PageController { req.sessionModel.set('dataset', dataset) const datasetInfo = datasets.get(dataset) ?? { dataSubject: '', requiresGeometryTypeSelection: false } req.sessionModel.set('data-subject', datasetInfo.dataSubject) - req.sessionModel.set(this.checkToolDeepLinkSessionKey, - { 'data-subject': datasetInfo.dataSubject, orgName, dataset, datasetName: datasetInfo.text }) + const sessionData = { 'data-subject': datasetInfo.dataSubject, orgName, dataset, datasetName: datasetInfo.text } + if (req.headers.referer) { + sessionData.referer = req.headers.referer + } + req.sessionModel.set(this.checkToolDeepLinkSessionKey, sessionData) this.#addHistoryStep(req, '/check/dataset') diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index 6f314d4d..18e3c33d 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -2,6 +2,25 @@ import hmpoFormWizard from 'hmpo-form-wizard' import { logPageView } from '../utils/logging.js' const { Controller } = hmpoFormWizard +/** + * If we arrived at the page via deep from another page, we'll use that page as the back link. + * + * @param {string} url current page URL + * @param {{ referer?: string }} deepLinkInfo deep link info from the session + * @returns {string|undefined} back link URL + */ +function wizardBackLink (currentUrl, deepLinkInfo) { + if (deepLinkInfo && 'referer' in deepLinkInfo) { + const { referer } = deepLinkInfo + const entryPoint = '/check/upload-method' + if (referer && currentUrl === entryPoint) { + return referer + } + } + + return undefined +} + class PageController extends Controller { checkToolDeepLinkSessionKey = 'check-tool-deep-link' @@ -11,11 +30,14 @@ class PageController extends Controller { } locals (req, res, next) { - if (this.options.backLink) { - req.form.options.lastPage = this.options.backLink - } + let backLink if (req.sessionModel) { - req.form.options.deepLink = req.sessionModel.get(this.checkToolDeepLinkSessionKey) + const deepLinkInfo = req.sessionModel.get(this.checkToolDeepLinkSessionKey) + backLink = wizardBackLink(req.originalUrl, deepLinkInfo) + } + backLink = backLink ?? this.options.backLink + if (backLink) { + req.form.options.lastPage = backLink } super.locals(req, res, next) } diff --git a/test/unit/PageController.test.js b/test/unit/PageController.test.js index dd94bac1..5c138a52 100644 --- a/test/unit/PageController.test.js +++ b/test/unit/PageController.test.js @@ -39,3 +39,48 @@ describe('PageController', () => { expect(callArgs.service).toEqual('lpa-data-validation-frontend') }) }) + +describe('Correctly detects the wizard back link', () => { + const referer = '/this-is-where-we-came-from' + const makeReq = () => { + return ({ + originalUrl: '/check/upload-method', + sessionID: '123', + sessionModel: { + get: vi.fn().mockReturnValue({ referer }) + }, + form: { + options: {} + } + }) + } + + it('arriving at the deep link entry point', () => { + const pageController = new PageController({ route: '/upload-method' }) + const req = makeReq() + pageController.locals(req, {}, vi.fn()) + expect(req.form.options.lastPage).toEqual(referer) + }) + + it('arriving at some other step', () => { + const pageController = new PageController({ route: '/upload-method' }) + const req = { ...makeReq(), originalUrl: '/check/another-step' } + pageController.locals(req, {}, vi.fn()) + expect(req.form.options.lastPage).toBe(undefined) + }) + + it('don\'t touch the back link if there\'s no deep link session info (lastPage not set)', () => { + const pageController = new PageController({ route: '/upload-method' }) + const req = { ...makeReq(), sessionModel: new Map() } + pageController.locals(req, {}, vi.fn()) + expect(req.form.options.lastPage).toEqual(undefined) + }) + + it('don\'t touch the back link if there\'s no deep link session info (lastPage set)', () => { + const pageController = new PageController({ route: '/upload-method' }) + pageController.options.backLink = '/go-back' + const req = { ...makeReq(), sessionModel: new Map() } + pageController.locals(req, {}, vi.fn()) + expect(req.form.options.lastPage).toEqual('/go-back') + }) +}) diff --git a/test/unit/deepLinkController.test.js b/test/unit/deepLinkController.test.js index 5fafe574..48e74389 100644 --- a/test/unit/deepLinkController.test.js +++ b/test/unit/deepLinkController.test.js @@ -4,7 +4,7 @@ import DeepLinkController from '../../src/controllers/deepLinkController.js' function mockRequestObject () { const sessionModel = new Map() const journeyModel = new Map() - return { sessionModel, journeyModel, query: {} } + return { sessionModel, journeyModel, query: {}, headers: {} } } function mockMiddlewareArgs (reqOpts) { From f9f70856c57fd28949ed4c4acd266f6e23b4d786 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 23 Oct 2024 16:51:11 +0100 Subject: [PATCH 2/7] fix: bring back sesion info --- src/controllers/pageController.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index 18e3c33d..95ec8cc3 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -31,10 +31,12 @@ class PageController extends Controller { locals (req, res, next) { let backLink - if (req.sessionModel) { - const deepLinkInfo = req.sessionModel.get(this.checkToolDeepLinkSessionKey) + const deepLinkInfo = req.sessionModel.get(this.checkToolDeepLinkSessionKey) + if (deepLinkInfo) { + req.form.options.deepLink = deepLinkInfo backLink = wizardBackLink(req.originalUrl, deepLinkInfo) } + backLink = backLink ?? this.options.backLink if (backLink) { req.form.options.lastPage = backLink From db141978074033a8be0c33f365f051482a2e7839 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 23 Oct 2024 16:51:49 +0100 Subject: [PATCH 3/7] check: back link for 'tree' dataset entry point via deep link --- src/controllers/pageController.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index 95ec8cc3..f273d4c4 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -6,14 +6,16 @@ const { Controller } = hmpoFormWizard * If we arrived at the page via deep from another page, we'll use that page as the back link. * * @param {string} url current page URL - * @param {{ referer?: string }} deepLinkInfo deep link info from the session + * @param {{ referer?: string, dataset: string }} deepLinkInfo deep link info from the session * @returns {string|undefined} back link URL */ function wizardBackLink (currentUrl, deepLinkInfo) { if (deepLinkInfo && 'referer' in deepLinkInfo) { - const { referer } = deepLinkInfo - const entryPoint = '/check/upload-method' - if (referer && currentUrl === entryPoint) { + const { referer, dataset } = deepLinkInfo + if (dataset === 'tree' && currentUrl === '/check/geometry-type') { + return referer + } + if (dataset !== 'tree' && currentUrl === '/check/upload-method') { return referer } } From de206c21b97857e4b9a2796128bbc0c2df7b47c1 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 23 Oct 2024 16:59:35 +0100 Subject: [PATCH 4/7] fix: safely access the session model --- src/controllers/pageController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index f273d4c4..c42a3964 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -33,7 +33,7 @@ class PageController extends Controller { locals (req, res, next) { let backLink - const deepLinkInfo = req.sessionModel.get(this.checkToolDeepLinkSessionKey) + const deepLinkInfo = req?.sessionModel?.get(this.checkToolDeepLinkSessionKey) if (deepLinkInfo) { req.form.options.deepLink = deepLinkInfo backLink = wizardBackLink(req.originalUrl, deepLinkInfo) From b0b921331fb28fbb285c09cae2506f826efa5cbd Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 24 Oct 2024 10:47:40 +0100 Subject: [PATCH 5/7] let's not spread the misspelled header name ('referer') --- src/controllers/deepLinkController.js | 2 +- src/controllers/pageController.js | 10 +++++----- test/unit/PageController.test.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/controllers/deepLinkController.js b/src/controllers/deepLinkController.js index 0fd21146..5592959d 100644 --- a/src/controllers/deepLinkController.js +++ b/src/controllers/deepLinkController.js @@ -35,7 +35,7 @@ class DeepLinkController extends PageController { req.sessionModel.set('data-subject', datasetInfo.dataSubject) const sessionData = { 'data-subject': datasetInfo.dataSubject, orgName, dataset, datasetName: datasetInfo.text } if (req.headers.referer) { - sessionData.referer = req.headers.referer + sessionData.referrer = req.headers.referer } req.sessionModel.set(this.checkToolDeepLinkSessionKey, sessionData) diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index c42a3964..09dd4d3b 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -6,17 +6,17 @@ const { Controller } = hmpoFormWizard * If we arrived at the page via deep from another page, we'll use that page as the back link. * * @param {string} url current page URL - * @param {{ referer?: string, dataset: string }} deepLinkInfo deep link info from the session + * @param {{ referrer?: string, dataset: string }} deepLinkInfo deep link info from the session * @returns {string|undefined} back link URL */ function wizardBackLink (currentUrl, deepLinkInfo) { - if (deepLinkInfo && 'referer' in deepLinkInfo) { - const { referer, dataset } = deepLinkInfo + if (deepLinkInfo && 'referrer' in deepLinkInfo) { + const { referrer, dataset } = deepLinkInfo if (dataset === 'tree' && currentUrl === '/check/geometry-type') { - return referer + return referrer } if (dataset !== 'tree' && currentUrl === '/check/upload-method') { - return referer + return referrer } } diff --git a/test/unit/PageController.test.js b/test/unit/PageController.test.js index 5c138a52..c49390c2 100644 --- a/test/unit/PageController.test.js +++ b/test/unit/PageController.test.js @@ -41,13 +41,13 @@ describe('PageController', () => { }) describe('Correctly detects the wizard back link', () => { - const referer = '/this-is-where-we-came-from' + const referrer = '/this-is-where-we-came-from' const makeReq = () => { return ({ originalUrl: '/check/upload-method', sessionID: '123', sessionModel: { - get: vi.fn().mockReturnValue({ referer }) + get: vi.fn().mockReturnValue({ referrer }) }, form: { options: {} @@ -59,7 +59,7 @@ describe('Correctly detects the wizard back link', () => { const pageController = new PageController({ route: '/upload-method' }) const req = makeReq() pageController.locals(req, {}, vi.fn()) - expect(req.form.options.lastPage).toEqual(referer) + expect(req.form.options.lastPage).toEqual(referrer) }) it('arriving at some other step', () => { From a362b247da6dfbb59c1c30f7bd1e6c293738fea2 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 24 Oct 2024 12:50:30 +0100 Subject: [PATCH 6/7] validate the 'referer' URL --- src/controllers/deepLinkController.js | 26 +++++++++++++++++++++++--- test/unit/PageController.test.js | 2 +- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/controllers/deepLinkController.js b/src/controllers/deepLinkController.js index 5592959d..1d9e4db9 100644 --- a/src/controllers/deepLinkController.js +++ b/src/controllers/deepLinkController.js @@ -11,6 +11,28 @@ const QueryParams = v.object({ orgName: NonEmptyString }) +/** + * Potentially updates sessionData with 'referrer' + * + * @param req + * @param sessionData + */ +function maybeSetReferrer (req, sessionData) { + if (req.headers.referer) { + try { + /* eslint-disable-next-line no-new */ + new URL(req.headers.referer) + sessionData.referrer = req.headers.referer + } catch (err) { + logger.info('DeepLinkController.get(): invalid referrer URL, skipping', { + type: types.App, + referrer: req.headers.referer, + errorMessage: err.message + }) + } + } +} + /** * Handles deep links in the Check Tool. * @@ -34,9 +56,7 @@ class DeepLinkController extends PageController { const datasetInfo = datasets.get(dataset) ?? { dataSubject: '', requiresGeometryTypeSelection: false } req.sessionModel.set('data-subject', datasetInfo.dataSubject) const sessionData = { 'data-subject': datasetInfo.dataSubject, orgName, dataset, datasetName: datasetInfo.text } - if (req.headers.referer) { - sessionData.referrer = req.headers.referer - } + maybeSetReferrer(req, sessionData) req.sessionModel.set(this.checkToolDeepLinkSessionKey, sessionData) this.#addHistoryStep(req, '/check/dataset') diff --git a/test/unit/PageController.test.js b/test/unit/PageController.test.js index c49390c2..c9a42fd2 100644 --- a/test/unit/PageController.test.js +++ b/test/unit/PageController.test.js @@ -41,7 +41,7 @@ describe('PageController', () => { }) describe('Correctly detects the wizard back link', () => { - const referrer = '/this-is-where-we-came-from' + const referrer = 'https://example.com/this-is-where-we-came-from' const makeReq = () => { return ({ originalUrl: '/check/upload-method', From cd707375e20c08c6432ae2673d6f64a96bf1affc Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 24 Oct 2024 12:59:02 +0100 Subject: [PATCH 7/7] wrap back link extraction in a try/catch --- src/controllers/pageController.js | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index 09dd4d3b..2b0aaf0f 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -1,5 +1,6 @@ import hmpoFormWizard from 'hmpo-form-wizard' -import { logPageView } from '../utils/logging.js' +import { logPageView, types } from '../utils/logging.js' +import logger from '../utils/logger.js' const { Controller } = hmpoFormWizard /** @@ -32,17 +33,25 @@ class PageController extends Controller { } locals (req, res, next) { - let backLink - const deepLinkInfo = req?.sessionModel?.get(this.checkToolDeepLinkSessionKey) - if (deepLinkInfo) { - req.form.options.deepLink = deepLinkInfo - backLink = wizardBackLink(req.originalUrl, deepLinkInfo) - } + try { + let backLink + const deepLinkInfo = req?.sessionModel?.get(this.checkToolDeepLinkSessionKey) + if (deepLinkInfo) { + req.form.options.deepLink = deepLinkInfo + backLink = wizardBackLink(req.originalUrl, deepLinkInfo) + } - backLink = backLink ?? this.options.backLink - if (backLink) { - req.form.options.lastPage = backLink + backLink = backLink ?? this.options.backLink + if (backLink) { + req.form.options.lastPage = backLink + } + } catch (e) { + logger.warn('PageController.locals(): error setting back link', { + type: types.App, + errorMessage: e.message + }) } + super.locals(req, res, next) } }