Skip to content

Commit 8e910dd

Browse files
authored
feat/df-820: Reads secrets from forms-manager (#1101)
* Reads secrets from forms-manager * Test coverage * Extra coverage * Coverage * Fixed env var for test * Handle error on get secret * Model bump * Resolved conflict * Extra coverage
1 parent 59c0170 commit 8e910dd

8 files changed

Lines changed: 176 additions & 14 deletions

File tree

jest.setup.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ process.env.SNS_SAVE_TOPIC_ARN =
1919
process.env.SNS_ADAPTER_TOPIC_ARN =
2020
'arn:aws:sns:eu-west-2:123456789012:test-adapter-topic'
2121
process.env.SNS_ENDPOINT = 'http://localhost:4566'
22+
process.env.PRIVATE_KEY_FOR_SECRETS = 'dummy-private-key'

package-lock.json

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@
4646
"license": "SEE LICENSE IN LICENSE",
4747
"dependencies": {
4848
"@aws-sdk/client-sns": "^3.997.0",
49-
"@defra/forms-engine-plugin": "^4.0.60",
50-
"@defra/forms-model": "^3.0.627",
49+
"@defra/forms-engine-plugin": "^4.0.62",
50+
"@defra/forms-model": "^3.0.629",
5151
"@defra/hapi-tracing": "^1.30.0",
5252
"@elastic/ecs-pino-format": "^1.5.0",
5353
"@hapi/boom": "^10.0.1",

src/config/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,14 @@ export const config = convict({
329329
nullable: false,
330330
default: 'defraforms@defra.gov.uk',
331331
env: 'FEEDBACK_VIA_EMAIL'
332+
} as SchemaObj<string | undefined>,
333+
334+
privateKeyForSecrets: {
335+
doc: 'The private key used to decrypt secret values',
336+
format: String,
337+
nullable: true,
338+
default: undefined,
339+
env: 'PRIVATE_KEY_FOR_SECRETS'
332340
} as SchemaObj<string | undefined>
333341
})
334342

src/server/services/formsService.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { FormStatus } from '@defra/forms-engine-plugin/types'
22
import { formMetadataSchema } from '@defra/forms-model'
33

44
import { config } from '~/src/config/index.js'
5+
import { decryptSecret } from '~/src/server/services/helpers/crypto.js'
56
import { getJson, postJson } from '~/src/server/services/httpService.js'
67

78
const managerUrl = config.get('managerUrl')
@@ -111,6 +112,21 @@ export async function validateSaveAndExitCredentials(
111112
return results
112113
}
113114

115+
/**
116+
* Retrieves a form secret and decrypts the value
117+
* @param {string} formId - the id of the form
118+
* @param {string} secretName - the name of the secret
119+
*/
120+
export async function getFormSecret(formId, secretName) {
121+
const response = await fetch(
122+
`${managerUrl}/forms/${formId}/secrets/${secretName}`
123+
)
124+
if (response.statusText !== 'OK') {
125+
return ''
126+
}
127+
return decryptSecret(await response.text())
128+
}
129+
114130
/**
115131
* @import { FormDefinition, FormMetadata } from '@defra/forms-model'
116132
* @import { SaveAndExitDetails, SaveAndExitResumeDetails } from '~/src/server/types.js'

src/server/services/formsService.test.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getFormDefinition,
66
getFormMetadata,
77
getFormMetadataById,
8+
getFormSecret,
89
getSaveAndExitDetails,
910
validateSaveAndExitCredentials
1011
} from '~/src/server/services/formsService.js'
@@ -16,6 +17,10 @@ const { MANAGER_URL, SUBMISSION_URL } = process.env
1617
const magicLinkId = '7ac201b2-bea3-490d-8ccb-2734b2794f7b'
1718

1819
jest.mock('~/src/server/services/httpService')
20+
jest.mock('node:crypto', () => ({
21+
...jest.requireActual('node:crypto'),
22+
privateDecrypt: () => 'decrypted-secret'
23+
}))
1924

2025
describe('Forms service', () => {
2126
const { definition, metadata } = fixtures.form
@@ -60,6 +65,19 @@ describe('Forms service', () => {
6065
updatedAt: expect.any(Date)
6166
})
6267
})
68+
69+
it('throws when validation error', async () => {
70+
jest.mocked(getJson).mockResolvedValue({
71+
res: /** @type {IncomingMessage} */ ({
72+
statusCode: StatusCodes.OK
73+
}),
74+
payload: { invalid: '123' }
75+
})
76+
77+
await expect(() => getFormMetadata(metadata.slug)).rejects.toThrow(
78+
'"title" is required'
79+
)
80+
})
6381
})
6482

6583
describe('getFormMetadataById', () => {
@@ -102,6 +120,19 @@ describe('Forms service', () => {
102120
updatedAt: expect.any(Date)
103121
})
104122
})
123+
124+
it('throws when validation error', async () => {
125+
jest.mocked(getJson).mockResolvedValue({
126+
res: /** @type {IncomingMessage} */ ({
127+
statusCode: StatusCodes.OK
128+
}),
129+
payload: { invalid: '123' }
130+
})
131+
132+
await expect(() => getFormMetadataById(metadata.id)).rejects.toThrow(
133+
'"title" is required'
134+
)
135+
})
105136
})
106137

107138
describe('getFormDefinition', () => {
@@ -168,6 +199,59 @@ describe('Forms service', () => {
168199
{ payload: { securityAnswer: 'answer' } }
169200
)
170201
})
202+
203+
it('throws if no results', async () => {
204+
// @ts-expect-error - partial mock of payload
205+
jest.mocked(postJson).mockResolvedValue({
206+
res: /** @type {IncomingMessage} */ ({
207+
statusCode: StatusCodes.OK
208+
}),
209+
payload: undefined
210+
})
211+
212+
await expect(() =>
213+
validateSaveAndExitCredentials(magicLinkId, 'answer')
214+
).rejects.toThrow(
215+
'Unexpected empty response in validateSaveAndExitCredentials'
216+
)
217+
})
218+
})
219+
220+
describe('getFormSecret', () => {
221+
beforeEach(() => {
222+
// @ts-expect-error - mock fetch
223+
global.fetch = jest.fn(() =>
224+
Promise.resolve({
225+
text: () => Promise.resolve('secret-value'),
226+
statusText: 'OK'
227+
})
228+
)
229+
})
230+
231+
it('calls correct url', async () => {
232+
const res = await getFormSecret(metadata.id, 'secret-name')
233+
234+
expect(fetch).toHaveBeenCalledWith(
235+
`${MANAGER_URL}/forms/${metadata.id}/secrets/secret-name`
236+
)
237+
expect(res).toBe('decrypted-secret')
238+
})
239+
240+
it('handles missing secret', async () => {
241+
// @ts-expect-error - mock fetch
242+
global.fetch = jest.fn(() =>
243+
Promise.resolve({
244+
text: () => Promise.resolve('secret-value'),
245+
statusText: 'Error'
246+
})
247+
)
248+
const res = await getFormSecret(metadata.id, 'secret-name')
249+
250+
expect(fetch).toHaveBeenCalledWith(
251+
`${MANAGER_URL}/forms/${metadata.id}/secrets/secret-name`
252+
)
253+
expect(res).toBe('')
254+
})
171255
})
172256
})
173257

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import crypto from 'node:crypto'
2+
3+
import { config } from '~/src/config/index.js'
4+
5+
/**
6+
* @param {string} secretValue - cleartext secret value
7+
* @returns {string} base64-encoded result
8+
*/
9+
export function decryptSecret(secretValue) {
10+
const privateKey = config.get('privateKeyForSecrets')
11+
if (!privateKey) {
12+
throw new Error('Private key is missing')
13+
}
14+
const buffer = Buffer.from(secretValue, 'base64')
15+
const decrypted = crypto.privateDecrypt(privateKey, buffer)
16+
return decrypted.toString()
17+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { config } from '~/src/config/index.js'
2+
import { decryptSecret } from '~/src/server/services/helpers/crypto.js'
3+
4+
jest.mock('~/src/config/index.ts', () => ({
5+
config: {
6+
get: jest.fn((key) => {
7+
if (key === 'privateKeyForSecrets') return 'abcdef'
8+
return 'mock-value'
9+
})
10+
}
11+
}))
12+
jest.mock('node:crypto', () => ({
13+
privateDecrypt: () => 'decrypted-secret'
14+
}))
15+
16+
describe('crypto helpers', () => {
17+
describe('decryptSecret', () => {
18+
it('should throw is private key is missing', () => {
19+
jest.mocked(config.get).mockImplementationOnce((key) => {
20+
if (key === 'privateKeyForSecrets') return undefined
21+
return 'mock-value'
22+
})
23+
expect(() => decryptSecret('some-string')).toThrow(
24+
'Private key is missing'
25+
)
26+
})
27+
28+
it('should return decrypted value', () => {
29+
jest.mocked(config.get).mockImplementationOnce((key) => {
30+
if (key === 'privateKeyForSecrets') return 'private-key'
31+
return 'mock-value'
32+
})
33+
expect(decryptSecret('some-string')).toBe('decrypted-secret')
34+
})
35+
})
36+
})

0 commit comments

Comments
 (0)