Skip to content

Commit 690877b

Browse files
fix/DF-469 forbidden error logs (#241)
* Remove duplicate crumb from context * Remove safeGenerateCrumb from context * Fix tests that rely on enforceCsrf being false * Default enforceCsrf: true for all environments * Fix component-state-errors.test.js
1 parent bbe7122 commit 690877b

File tree

15 files changed

+27
-154
lines changed

15 files changed

+27
-154
lines changed

src/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export const config = convict({
5555
},
5656
enforceCsrf: {
5757
format: Boolean,
58-
default: isProduction,
58+
default: true,
5959
env: 'ENFORCE_CSRF'
6060
},
6161

src/server/plugins/engine/helpers.test.ts

Lines changed: 0 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
getExponentialBackoffDelay,
1919
getPageHref,
2020
proceed,
21-
safeGenerateCrumb,
2221
setPageTitles,
2322
type GlobalScope
2423
} from '~/src/server/plugins/engine/helpers.js'
@@ -36,7 +35,6 @@ import {
3635
import {
3736
FormAction,
3837
FormStatus,
39-
type FormRequest,
4038
type FormResponseToolkit
4139
} from '~/src/server/routes/types.js'
4240
import definition from '~/test/form/definitions/basic.js'
@@ -493,78 +491,6 @@ describe('Helpers', () => {
493491
})
494492
})
495493

496-
describe('safeGenerateCrumb', () => {
497-
it('should return undefined when request.state is missing (malformed request)', () => {
498-
const malformedRequest = {
499-
server: {
500-
plugins: {
501-
crumb: {
502-
generate: jest.fn()
503-
}
504-
}
505-
},
506-
plugins: {},
507-
route: { settings: { plugins: {} } },
508-
path: '/test',
509-
url: { search: '' }
510-
// state intentionally omitted
511-
} as unknown as FormRequest
512-
513-
const crumbToken = safeGenerateCrumb(malformedRequest)
514-
expect(crumbToken).toBeUndefined()
515-
expect(
516-
malformedRequest.server.plugins.crumb.generate
517-
).not.toHaveBeenCalled()
518-
})
519-
520-
it('should return undefined if crumb is disabled in route settings', () => {
521-
const requestWithDisabledCrumb = {
522-
server: {
523-
plugins: {
524-
crumb: {
525-
generate: jest.fn().mockReturnValue('test-token')
526-
}
527-
}
528-
},
529-
plugins: {},
530-
route: { settings: { plugins: { crumb: false } } },
531-
path: '/test',
532-
url: { search: '' },
533-
state: {}
534-
} as unknown as FormRequest
535-
536-
const crumbToken = safeGenerateCrumb(requestWithDisabledCrumb)
537-
expect(crumbToken).toBeUndefined()
538-
expect(
539-
requestWithDisabledCrumb.server.plugins.crumb.generate
540-
).not.toHaveBeenCalled()
541-
})
542-
543-
it('should generate crumb when state exists and crumb plugin is available', () => {
544-
const mockCrumb = 'generated-crumb-value'
545-
const validRequest = {
546-
server: {
547-
plugins: {
548-
crumb: {
549-
generate: jest.fn().mockReturnValue(mockCrumb)
550-
}
551-
}
552-
},
553-
plugins: {},
554-
route: { settings: { plugins: {} } },
555-
path: '/test',
556-
url: { search: '' },
557-
state: {}
558-
} as unknown as FormRequest
559-
560-
const crumbToken = safeGenerateCrumb(validRequest)
561-
expect(crumbToken).toBe(mockCrumb)
562-
expect(validRequest.server.plugins.crumb.generate).toHaveBeenCalledWith(
563-
validRequest
564-
)
565-
})
566-
})
567-
568494
describe('getExponentialBackoffDelay', () => {
569495
it.each([
570496
{ depth: 1, expected: 2000 },

src/server/plugins/engine/helpers.ts

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { type FormModel } from '~/src/server/plugins/engine/models/FormModel.js'
2525
import { type PageControllerClass } from '~/src/server/plugins/engine/pageControllers/helpers/pages.js'
2626
import { stripParam } from '~/src/server/plugins/engine/pageControllers/helpers/state.js'
2727
import {
28-
type AnyFormRequest,
2928
type FormContext,
3029
type FormContextRequest,
3130
type FormSubmissionError
@@ -336,32 +335,6 @@ export function createError(componentName: string, message: string) {
336335
}
337336
}
338337

339-
/**
340-
* A small helper to safely generate a crumb token.
341-
* Checks that the crumb plugin is available, that crumb
342-
* is not disabled on the current route, and that cookies/state are present.
343-
*/
344-
export function safeGenerateCrumb(
345-
request: AnyFormRequest | null
346-
): string | undefined {
347-
// no request or no .state
348-
if (!request?.state) {
349-
return undefined
350-
}
351-
352-
// crumb plugin or its generate method doesn't exist
353-
if (!request.server.plugins.crumb.generate) {
354-
return undefined
355-
}
356-
357-
// crumb is explicitly disabled for this route
358-
if (request.route.settings.plugins?.crumb === false) {
359-
return undefined
360-
}
361-
362-
return request.server.plugins.crumb.generate(request)
363-
}
364-
365338
/**
366339
* Calculates an exponential backoff delay (in milliseconds) based on the current retry depth,
367340
* using a base delay of 2000ms (2 seconds) and doubling for each additional depth, while capping the delay at 25,000ms (25 seconds).

src/server/plugins/nunjucks/context.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ import { config } from '~/src/config/index.js'
88
import { createLogger } from '~/src/server/common/helpers/logging/logger.js'
99
import {
1010
checkFormStatus,
11-
encodeUrl,
12-
safeGenerateCrumb
11+
encodeUrl
1312
} from '~/src/server/plugins/engine/helpers.js'
1413

1514
const logger = createLogger()
@@ -51,7 +50,6 @@ export async function context(request) {
5150
// take consumers props first so we can override it
5251
...consumerViewContext,
5352
baseLayoutPath: pluginStorage.baseLayoutPath,
54-
crumb: safeGenerateCrumb(request),
5553
currentPath: `${request.path}${request.url.search}`,
5654
previewMode: isPreviewMode ? formState : undefined,
5755
slug: isResponseOK ? params?.slug : undefined

src/server/plugins/nunjucks/context.test.js

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -101,43 +101,6 @@ describe('Nunjucks context', () => {
101101
malformedRequest.server.plugins.crumb.generate
102102
).not.toHaveBeenCalled()
103103
})
104-
105-
it('should generate crumb when state exists', async () => {
106-
const mockCrumb = 'generated-crumb-value'
107-
const validRequest = /** @type {FormRequest} */ (
108-
/** @type {unknown} */ ({
109-
server: {
110-
plugins: {
111-
crumb: {
112-
generate: jest.fn().mockReturnValue(mockCrumb)
113-
},
114-
'forms-engine-plugin': {
115-
baseLayoutPath: 'randomValue'
116-
}
117-
}
118-
},
119-
plugins: {},
120-
route: {
121-
settings: {
122-
plugins: {}
123-
}
124-
},
125-
path: '/test',
126-
url: { search: '' },
127-
state: {},
128-
yar: {
129-
flash: jest.fn().mockReturnValue([]),
130-
commit: jest.fn()
131-
}
132-
})
133-
)
134-
135-
const { crumb } = await context(validRequest)
136-
expect(crumb).toBe(mockCrumb)
137-
expect(validRequest.server.plugins.crumb.generate).toHaveBeenCalledWith(
138-
validRequest
139-
)
140-
})
141104
})
142105
})
143106

src/server/routes/dummy-api.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ describe('Dummy API', () => {
2121

2222
beforeAll(async () => {
2323
MockDate.set('2025-01-01T00:00:00Z')
24-
server = await createServer()
24+
server = await createServer({
25+
enforceCsrf: false
26+
})
2527
await server.initialize()
2628
})
2729

test/condition/checkboxes.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ describe('Checkboxes based conditions', () => {
2121
beforeAll(async () => {
2222
server = await createServer({
2323
formFileName: 'checkboxes.json',
24-
formFilePath: resolve(import.meta.dirname, '../form/definitions')
24+
formFilePath: resolve(import.meta.dirname, '../form/definitions'),
25+
enforceCsrf: false
2526
})
2627

2728
await server.initialize()

test/condition/radios.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ describe('Radio based conditions', () => {
2121
beforeAll(async () => {
2222
server = await createServer({
2323
formFileName: 'radios.json',
24-
formFilePath: resolve(import.meta.dirname, '../form/definitions')
24+
formFilePath: resolve(import.meta.dirname, '../form/definitions'),
25+
enforceCsrf: false
2526
})
2627

2728
await server.initialize()

test/condition/text.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ describe('TextField based conditions', () => {
2121
beforeAll(async () => {
2222
server = await createServer({
2323
formFileName: 'text.json',
24-
formFilePath: resolve(import.meta.dirname, '../form/definitions')
24+
formFilePath: resolve(import.meta.dirname, '../form/definitions'),
25+
enforceCsrf: false
2526
})
2627

2728
await server.initialize()

test/form/component-state-errors.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ describe('Component State Error Tests - File Upload', () => {
5353
beforeAll(async () => {
5454
server = await createServer({
5555
formFileName: 'file-upload-basic.js',
56-
formFilePath: join(import.meta.dirname, 'definitions')
56+
formFilePath: join(import.meta.dirname, 'definitions'),
57+
enforceCsrf: false
5758
})
5859

5960
await server.initialize()

0 commit comments

Comments
 (0)