Merge pull request #483 from DEFRA/fix/df-870-reuse

fix/df-870: Can use old save-and-exit link to extract most recent

Author: alexluckett

src/models/form.js

@@ -22,6 +22,14 @@ export const getSavedLinkResponseSchema = Joi.object({
   invalidPasswordAttempts: Joi.number().min(0).required()
 }).label('getSavedLinkResponse')
 
+export const getSavedLinkGoneSchema = Joi.object({
+  output: {
+    payload: {
+      latestId: Joi.string().required()
+    }
+  }
+}).label('getSavedLinkGoneResponse')
+
 export const validateSavedLinkResponseSchema = Joi.object({
   form: {
     id: Joi.string().required(),

src/repositories/save-and-exit-repository.js

@@ -35,8 +35,7 @@ export async function getSaveAndExitRecord(id) {
   try {
     const timer = createTimer()
     const result = await coll.findOne({
-      magicLinkId: id,
-      consumed: { $ne: true }
+      magicLinkId: id
     })
 
     logger.info(
@@ -54,6 +53,50 @@ export async function getSaveAndExitRecord(id) {
   }
 }
 
+/**
+ * Find the latest (active) link in a group of save-and-exit records
+ * @param {string} groupId - group id of save-and-exit record
+ * @returns { Promise<WithId<SaveAndExitDocument> | null> }
+ */
+export async function getLatestSaveAndExitByGroup(groupId) {
+  const event = {
+    category: saveAndExitLabel,
+    action: 'read-latest-record-by-group',
+    reference: groupId
+  }
+  logger.info({ event }, 'Reading latest save and exit record')
+
+  const coll = /** @type {Collection<SaveAndExitDocument>} */ (
+    db.collection(SAVE_AND_EXIT_COLLECTION_NAME)
+  )
+
+  try {
+    const timer = createTimer()
+    const result = await coll
+      .find({
+        magicLinkGroupId: groupId,
+        consumed: { $ne: true }
+      })
+      .sort({
+        createdAt: -1
+      })
+      .toArray()
+
+    logger.info(
+      { event: { ...event, duration: timer.elapsed } },
+      `Read latest save and exit record (${timer.elapsed}ms)`
+    )
+
+    return result.length ? result[0] : null
+  } catch (err) {
+    logger.error(
+      { err, event },
+      `Failed to read latest save and exit record - ${getErrorMessage(err)}`
+    )
+    throw err
+  }
+}
+
 /**
  * Creates a save and exit record from SubmissionRecordInput
  * @param {Omit<SaveAndExitDocument, 'expireAt'>} recordInput

src/repositories/save-and-exit-repository.test.js

@@ -8,6 +8,7 @@ import {
   createSaveAndExitRecord,
   deleteSaveAndExitGroup,
   findExpiringRecords,
+  getLatestSaveAndExitByGroup,
   getSaveAndExitRecord,
   incrementInvalidPasswordAttempts,
   lockRecordForExpiryEmail,
@@ -70,7 +71,7 @@ describe('save-and-exit-repository', () => {
   })
 
   describe('getSaveAndExitRecord', () => {
-    it('should get save and exit record', async () => {
+    it('should get save and exit record if not comsumed', async () => {
       mockCollection.findOne.mockReturnValueOnce(submissionDocument)
       const submissionRecord = await getSaveAndExitRecord(
         STUB_SAVE_AND_EXIT_RECORD_ID
@@ -88,6 +89,38 @@ describe('save-and-exit-repository', () => {
     })
   })
 
+  describe('getLatestSaveAndExitByGroup', () => {
+    it('should get latest save and exit record by group', async () => {
+      const document1WithGroup = {
+        ...submissionDocument,
+        magicLinkId: 'id1',
+        magicLinkGroupId: 'magic-group-id'
+      }
+      const document2WithGroup = {
+        ...submissionDocument,
+        magicLinkId: 'id2',
+        magicLinkGroupId: 'magic-group-id'
+      }
+      mockCollection.find.mockReturnValueOnce({
+        sort: jest.fn(() => {
+          return { toArray: () => [document2WithGroup, document1WithGroup] }
+        })
+      })
+      const submissionRecord =
+        await getLatestSaveAndExitByGroup('magic-group-id')
+      expect(submissionRecord).toEqual(document2WithGroup)
+    })
+
+    it('should handle get latest save and exit record failures', async () => {
+      mockCollection.find.mockImplementation(() => {
+        throw new Error('an error')
+      })
+      await expect(
+        getLatestSaveAndExitByGroup(STUB_SAVE_AND_EXIT_RECORD_ID)
+      ).rejects.toThrow(new Error('an error'))
+    })
+  })
+
   describe('createSaveAndExitRecord', () => {
     it('should create a save and exit record when no previous relevant ones', async () => {
       jest.mocked(

src/routes/form.js

@@ -3,6 +3,7 @@ import Joi from 'joi'
 
 import {
   formSubmitResponseSchema,
+  getSavedLinkGoneSchema,
   getSavedLinkResponseSchema,
   magicLinkSchema,
   validateSavedLinkResponseSchema
@@ -67,7 +68,8 @@ export default [
       },
       response: {
         status: {
-          200: getSavedLinkResponseSchema
+          200: getSavedLinkResponseSchema,
+          410: getSavedLinkGoneSchema
         }
       }
     }

src/routes/form.test.js

@@ -1,4 +1,5 @@
 import { FormStatus, SecurityQuestionsEnum } from '@defra/forms-model'
+import Boom from '@hapi/boom'
 import { StatusCodes } from 'http-status-codes'
 
 import { createServer } from '~/src/api/server.js'
@@ -177,6 +178,26 @@ describe('Forms route', () => {
       })
     })
 
+    test('Testing GET /save-and-exit route returns record with latest id when current one is consumed', async () => {
+      jest.mocked(getSavedLinkDetails).mockImplementationOnce(() => {
+        const boomError = Boom.resourceGone('consumed magic link')
+        boomError.output.payload = {
+          ...boomError.output.payload,
+          latestId: 'latest-link-id'
+        }
+        throw boomError
+      })
+      const response = await server.inject({
+        method: 'GET',
+        url: `/save-and-exit/${GUID_EMPTY}`
+      })
+
+      expect(response.statusCode).toEqual(StatusCodes.GONE)
+      expect(response.result).toMatchObject({
+        latestId: 'latest-link-id'
+      })
+    })
+
     test('Testing POST /save-and-exit route fails if with invalid payload', async () => {
       // @ts-expect-error - invalid type due to invalid payload
       jest.mocked(validateSavedLinkCredentials).mockResolvedValue({})

src/services/save-and-exit-service.js

@@ -5,6 +5,7 @@ import argon2 from 'argon2'
 import { createLogger } from '~/src/helpers/logging/logger.js'
 import {
   deleteSaveAndExitGroup,
+  getLatestSaveAndExitByGroup,
   getSaveAndExitRecord,
   incrementInvalidPasswordAttempts,
   resetSaveAndExitRecord
@@ -13,6 +14,7 @@ import {
 const logger = createLogger()
 
 const INVALID_MAGIC_LINK = 'Invalid magic link'
+const CONSUMED_MAGIC_LINK = 'Magic link has already been consumed'
 
 /**
  * Get the save and exit link by magic link id
@@ -25,6 +27,20 @@ export async function getSavedLinkDetails(magicLinkId) {
     throw Boom.notFound(INVALID_MAGIC_LINK)
   }
 
+  if (record.consumed && record.magicLinkGroupId) {
+    // Look for a non-comsumed link of the same group
+    // This allows a user to use an old email link but still always point them at the latest save-and-exit record
+    const latestRecord = await getLatestSaveAndExitByGroup(
+      record.magicLinkGroupId
+    )
+    const boomError = Boom.resourceGone(CONSUMED_MAGIC_LINK)
+    boomError.output.payload = {
+      ...boomError.output.payload,
+      latestId: latestRecord?.magicLinkId
+    }
+    throw boomError
+  }
+
   return {
     form: record.form,
     question: record.security.question,

src/services/save-and-exit-service.test.js

@@ -3,6 +3,7 @@ import { SecurityQuestionsEnum } from '@defra/forms-model'
 import { buildDbDocument } from '~/src/repositories/__stubs__/save-and-exit.js'
 import {
   deleteSaveAndExitGroup,
+  getLatestSaveAndExitByGroup,
   getSaveAndExitRecord,
   incrementInvalidPasswordAttempts,
   markSaveAndExitRecordAsConsumed,
@@ -96,6 +97,36 @@ describe('save-and-exit service', () => {
       )
     })
 
+    test('should throw if link consumed)', async () => {
+      jest.mocked(getSaveAndExitRecord).mockResolvedValue({
+        // @ts-expect-error - partial record value
+        form: {
+          id: '1234'
+        },
+        security: {
+          question: SecurityQuestionsEnum.MemorablePlace,
+          answer: ''
+        },
+        consumed: true,
+        magicLinkGroupId: 'group-id'
+      })
+      jest.mocked(getLatestSaveAndExitByGroup).mockResolvedValue({
+        // @ts-expect-error - partial record value
+        form: {
+          id: '1234'
+        },
+        security: {
+          question: SecurityQuestionsEnum.MemorablePlace,
+          answer: ''
+        },
+        consumed: false,
+        magicLinkGroupId: 'group-id'
+      })
+      await expect(getSavedLinkDetails('12345')).rejects.toThrow(
+        'Magic link has already been consumed'
+      )
+    })
+
     test('should return valid result)', async () => {
       jest.mocked(getSaveAndExitRecord).mockResolvedValue({
         // @ts-expect-error - partial record value