feat/df-890: Handles deletion and rename of secrets

README.md

@@ -7,6 +7,7 @@ Core delivery platform Node.js Backend Template.
     - [Node.js](#nodejs)
   - [Local development](#local-development)
     - [Setup](#setup)
+    - [Generating a public/private key pair](#generating-a-publicprivate-key-pair)
     - [Development](#development)
     - [Production](#production)
     - [Npm scripts](#npm-scripts)
@@ -72,6 +73,26 @@ PUBLIC_KEY_FOR_SECRETS="<base64-encoded-public-key>"
 
 For proxy options, see https://www.npmjs.com/package/proxy-from-env which is used by https://github.com/TooTallNate/proxy-agents/tree/main/packages/proxy-agent. It's currently supports Hapi Wreck only, e.g. in the JWKS lookup.
 
+### Generating a public/private key pair
+
+The public key is used by forms-manager to encrypt secrets. The private key is used by forms-runner to decrypt secrets.
+
+```
+openssl genrsa -out private.pem 4096
+openssl rsa -pubout -in private.pem -out public.pem
+cat public.pem     #[copy the output and paste as <public-key> in the next command]
+echo -n "<public-key>" | base64
+```
+
+Copy/paste the encoded result as the env var PUBLIC_KEY_FOR_SECRETS (in forms-manager)
+
+```
+cat private.pem     #[copy the output and paste as <private-key> in the next command]
+echo -n "<private-key>" | base64
+```
+
+Copy/paste the encoded result as the env var PRIVATE_KEY_FOR_SECRETS (in forms-runner)
+
 ### Development
 
 To run the application in `development` mode run:

docker-compose.integration-test.yml

@@ -105,6 +105,8 @@ services:
       SNS_TOPIC_ARN: arn:aws:sns:eu-west-2:000000000000:forms_manager_events
       AWS_ACCESS_KEY_ID: test
       AWS_SECRET_ACCESS_KEY: test
+      # PUBLIC_KEY is a test value - do not use for any deployments
+      PUBLIC_KEY_FOR_SECRETS: "LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQ0lqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FnOEFNSUlDQ2dLQ0FnRUE0WU53YUJjenlBdWpJZ2k2aTBmawpINUVYeGxtTmNBd1AzUkpRaDB2ckxjeDIxU2RCSkFOUU5JWmdTcWhST3ViUlozWTJWQ2Zsbkd3cFVSTTNrY0tFCkNHUlk2Nk5leVUwL3QzS2hVOUY1cG1heVBSVUFiWkRjalFCL0xYbVNGUUdGelZDQlp0ZjBIeHJiSnBUNDFiSGIKR05udFpDdm94WjZQOUJxdU4wa2c0Y1JXcTdyWUozVE1xWXArc1RzQU01UFZwMVh6WklIRFJ2UVJ2bktnMDVUUQovRFA1cXcvYkdwelVsQ01WVU9MTHhxL2xTY040RnhDRTRUUi9kSWNNM25wTU1PUUVjaEhLcEE2SWs3UmhmQml0CjZBSEpINWN2c0dWUTFHUmlvUXBIWXBWclZLRFV5ZmIrS3VzbjlWUTVTQ3JGZDJ3WWRaRzBkdVNOZFp3VklJQm8KVElWZjlrSUMrZjl5YTVLMmcvalJhMXUyVGdNc3UvZnM4UWVRZHk2disxNkFtWFA1cXNXYUtFK00zRkJoRitrZAozbS91YjNJVFRRVmplYWY1M2ZsTVlMZlZmN2NVR2FQN0VIajlUY0dGYVlGTzhuWGNWbS9lT2lkMXBGL3p5NCtzCnpFZG5oOFNRYm5ncElwdFJiMk51Wmw5RWZNR01CTFlyQ3FiWnMyYU1KV0N5M1FGWFZDeTYzUzZhRDkrbzQ1N0cKUDNGVnNZY2d2eHAzTGVTQ0phUHMzL0FlSjh1MUFSeHZwMG02ZjdPLy9yMFFQYzJnd2RyZjRVMUkwdlduclh2eApCc2RMWU5NMXVYVjhMKzExYloxeGpVaFU4QldTd3dnTTdQMGxqVTJ4UE9WbG55MlFGeXh0ZTgzK0E5SXNBcmFxCmthNlhEdDhYZkNZYm84dEpnem9mbEs4Q0F3RUFBUT09Ci0tLS0tRU5EIFBVQkxJQyBLRVktLS0tLQ=="
     depends_on:
       mongo_test:
         condition: service_healthy

package.json

@@ -42,7 +42,7 @@
   "dependencies": {
     "@aws-sdk/client-s3": "^3.995.0",
     "@aws-sdk/client-sns": "^3.995.0",
-    "@defra/forms-model": "^3.0.629",
+    "@defra/forms-model": "^3.0.633",
     "@defra/hapi-tracing": "^1.29.0",
     "@elastic/ecs-pino-format": "^1.5.0",
     "@hapi/hapi": "^21.4.4",

src/api/forms/repositories/secrets-repository.js

@@ -64,6 +64,7 @@ export async function exists(formId, secretName, session) {
     return {
       exists: !!document,
       createdAt: document?.createdAt,
+      renamedAt: document?.renamedAt,
       updatedAt: document?.updatedAt
     }
   } catch (err) {
@@ -130,6 +131,98 @@ export async function save(formId, secretName, secretValue, session) {
   }
 }
 
+/**
+ * Deletes a form secret from the database
+ * @param {string} formId - id of the form
+ * @param {string} secretName - name of the secret
+ * @param {ClientSession} session - mongo transaction session
+ */
+export async function deleteSecret(formId, secretName, session) {
+  logger.info(`Deleting secret '${secretName}' for form ID ${formId}`)
+
+  const coll = /** @satisfies {Collection<FormSecret>} */ (
+    db.collection(SECRETS_COLLECTION_NAME)
+  )
+
+  try {
+    const result = await coll.deleteOne(
+      {
+        formId,
+        secretName
+      },
+      { session }
+    )
+
+    logger.info(
+      `Secret deleted with name '${secretName}' for form ID ${formId}`
+    )
+
+    return result
+  } catch (err) {
+    const message = `Secret with name '${secretName}' for form ID ${formId} failed to delete`
+
+    if (err instanceof MongoServerError) {
+      logger.error(
+        err,
+        `[mongoError] ${message} - MongoDB error code: ${err.code} - ${err.message}`
+      )
+    } else {
+      logger.error(err, `[deleteError] ${message} - ${getErrorMessage(err)}`)
+    }
+    throw err
+  }
+}
+
+/**
+ * Changes the name of a form secret in the database
+ * @param {string} formId - id of the form
+ * @param {string} secretNameFrom - name of the secret to be renamed
+ * @param {string} secretNameTo - the new name of the secret
+ * @param {ClientSession} session - mongo transaction session
+ */
+export async function rename(formId, secretNameFrom, secretNameTo, session) {
+  logger.info(
+    `Renaming secret '${secretNameFrom}' to '${secretNameTo}' for form ID ${formId}`
+  )
+
+  const coll = /** @satisfies {Collection<FormSecret>} */ (
+    db.collection(SECRETS_COLLECTION_NAME)
+  )
+
+  const now = new Date()
+
+  try {
+    const result = await coll.findOneAndUpdate(
+      {
+        formId,
+        secretName: secretNameFrom
+      },
+      {
+        $set: { secretName: secretNameTo, renamedAt: now }
+      },
+      { session }
+    )
+
+    logger.info(
+      `Secret renamed from '${secretNameFrom}' to '${secretNameTo}' for form ID ${formId}`
+    )
+
+    return result
+  } catch (err) {
+    const message = `Secret with name '${secretNameFrom}' for form ID ${formId} failed to rename`
+
+    if (err instanceof MongoServerError) {
+      logger.error(
+        err,
+        `[mongoError] ${message} - MongoDB error code: ${err.code} - ${err.message}`
+      )
+    } else {
+      logger.error(err, `[renameError] ${message} - ${getErrorMessage(err)}`)
+    }
+    throw err
+  }
+}
+
 /**
  * @import { FormSecret } from '@defra/forms-model'
  * @import { ClientSession, Collection } from 'mongodb'

src/api/forms/repositories/secrets-repository.test.js

@@ -3,8 +3,10 @@ import { MongoServerError } from 'mongodb'
 
 import { buildMockCollection } from '~/src/api/forms/__stubs__/mongo.js'
 import {
+  deleteSecret,
   exists,
   get,
+  rename,
   save
 } from '~/src/api/forms/repositories/secrets-repository.js'
 import author from '~/src/api/forms/service/__stubs__/author.js'
@@ -186,6 +188,79 @@ describe('secrets-repository', () => {
     })
   })
 
+  describe('delete', () => {
+    it('should delete if the secret exists', async () => {
+      mockCollection.deleteOne.mockResolvedValue({ deletedCount: 1 })
+      const result = await deleteSecret(formId, 'my-secret', mockSession)
+      expect(result).toEqual({ deletedCount: 1 })
+    })
+
+    it('should throw if db error', async () => {
+      mockCollection.deleteOne.mockImplementationOnce(() => {
+        throw new MongoServerError({ message: 'DB error deleting' })
+      })
+      await expect(() =>
+        deleteSecret(formId, 'my-secret', mockSession)
+      ).rejects.toThrow('DB error deleting')
+      expect(mockLoggerError).toHaveBeenCalledWith(
+        expect.anything(),
+        "[mongoError] Secret with name 'my-secret' for form ID fe339c6a-1f6e-4ab8-88c6-73fa1528dc90 failed to delete - MongoDB error code: undefined - DB error deleting"
+      )
+    })
+
+    it('should throw if other error', async () => {
+      mockCollection.deleteOne.mockImplementationOnce(() => {
+        throw new Error('DB error deleting')
+      })
+      await expect(() =>
+        deleteSecret(formId, 'my-secret', mockSession)
+      ).rejects.toThrow('DB error deleting')
+      expect(mockLoggerError).toHaveBeenCalledWith(
+        expect.anything(),
+        "[deleteError] Secret with name 'my-secret' for form ID fe339c6a-1f6e-4ab8-88c6-73fa1528dc90 failed to delete - DB error deleting"
+      )
+    })
+  })
+
+  describe('rename', () => {
+    it('should rename if the secret exists', async () => {
+      mockCollection.findOneAndUpdate.mockResolvedValue(secret)
+      const result = await rename(
+        formId,
+        'my-secret before',
+        'my secret after',
+        mockSession
+      )
+      expect(result).toEqual(secret)
+    })
+
+    it('should throw if db error', async () => {
+      mockCollection.findOneAndUpdate.mockImplementationOnce(() => {
+        throw new MongoServerError({ message: 'DB error rename' })
+      })
+      await expect(() =>
+        rename(formId, 'my-secret before', 'my secret after', mockSession)
+      ).rejects.toThrow('DB error rename')
+      expect(mockLoggerError).toHaveBeenCalledWith(
+        expect.anything(),
+        "[mongoError] Secret with name 'my-secret before' for form ID fe339c6a-1f6e-4ab8-88c6-73fa1528dc90 failed to rename - MongoDB error code: undefined - DB error rename"
+      )
+    })
+
+    it('should throw if other error', async () => {
+      mockCollection.findOneAndUpdate.mockImplementationOnce(() => {
+        throw new Error('DB error rename')
+      })
+      await expect(() =>
+        rename(formId, 'my-secret before', 'my secret after', mockSession)
+      ).rejects.toThrow('DB error rename')
+      expect(mockLoggerError).toHaveBeenCalledWith(
+        expect.anything(),
+        "[renameError] Secret with name 'my-secret before' for form ID fe339c6a-1f6e-4ab8-88c6-73fa1528dc90 failed to rename - DB error rename"
+      )
+    })
+  })
+
   describe('save', () => {
     it('should return true if the secret exists', async () => {
       mockCollection.findOneAndUpdate.mockResolvedValue(secret)

src/api/forms/service/definition.js

@@ -11,6 +11,11 @@ import { makeFormLiveErrorMessages } from '~/src/api/forms/constants.js'
 import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js'
 import { deleteDraft } from '~/src/api/forms/repositories/form-definition-repository.js'
 import * as formMetadata from '~/src/api/forms/repositories/form-metadata-repository.js'
+import {
+  deleteSecret,
+  exists,
+  rename
+} from '~/src/api/forms/repositories/secrets-repository.js'
 import { getValidationSchema } from '~/src/api/forms/service/helpers/definition.js'
 import { getForm } from '~/src/api/forms/service/index.js'
 import { existsFormSecret } from '~/src/api/forms/service/secrets.js'
@@ -30,6 +35,7 @@ import {
 import { client } from '~/src/mongo.js'
 
 export const PAYMENT_LIVE_API_KEY = 'payment-live-api-key'
+export const PAYMENT_LIVE_API_KEY_PENDING = 'payment-live-api-key-pending'
 
 /**
  * Retrieves a paginated list of forms with filter options
@@ -253,6 +259,34 @@ function validateFormForPublishing(
   }
 }
 
+/**
+ * Overwrite a live payment API key with the value of the pending live payment API key.
+ * Essentially make the pending key the live one for forms-runner to access.
+ * @param {boolean} formHasPayment - true if the form includes a payment question
+ * @param {string} formId - the id of the form
+ * @param {ClientSession} session
+ */
+export async function makePaymentKeyLive(formHasPayment, formId, session) {
+  if (!formHasPayment) {
+    return
+  }
+
+  const pendingKeyExists = await exists(
+    formId,
+    PAYMENT_LIVE_API_KEY_PENDING,
+    session
+  )
+  if (pendingKeyExists.exists) {
+    await deleteSecret(formId, PAYMENT_LIVE_API_KEY, session)
+    await rename(
+      formId,
+      PAYMENT_LIVE_API_KEY_PENDING,
+      PAYMENT_LIVE_API_KEY,
+      session
+    )
+  }
+}
+
 /**
  * Creates the live form from the current draft state
  * @param {string} formId - ID of the form
@@ -274,8 +308,11 @@ export async function createLiveFromDraft(formId, author) {
     /** @type {boolean} */
     let paymentKeyExists
     if (formHasPayment) {
-      const { exists } = await existsFormSecret(formId, PAYMENT_LIVE_API_KEY)
-      paymentKeyExists = exists
+      const [liveExists, pendingExists] = await Promise.all([
+        existsFormSecret(formId, PAYMENT_LIVE_API_KEY),
+        existsFormSecret(formId, PAYMENT_LIVE_API_KEY_PENDING)
+      ])
+      paymentKeyExists = liveExists.exists || pendingExists.exists
     } else {
       paymentKeyExists = true
     }
@@ -325,6 +362,9 @@ export async function createLiveFromDraft(formId, author) {
 
         await createFormVersion(formId, session)
 
+        // Make payment key live if a pending one is stored
+        await makePaymentKeyLive(formHasPayment, formId, session)
+
         // Publish audit message
         await publishLiveCreatedFromDraftEvent(formId, now, author)
       })