diff --git a/src/lib/error/api-error.test.ts b/src/lib/error/api-error.test.ts index 1675c27388..7e1cae4039 100644 --- a/src/lib/error/api-error.test.ts +++ b/src/lib/error/api-error.test.ts @@ -21,9 +21,9 @@ describe('OpenAPI error conversion', () => { const { description } = fromOpenApiValidationError({})(error); // it tells the user that the property is required - expect(description.includes('required')); + expect(description.includes('required')).toBeTruthy(); // it tells the user the name of the missing property - expect(description.includes(error.params.missingProperty)); + expect(description.includes(error.params.missingProperty)).toBeTruthy(); }); it('Gives useful error messages for type errors', () => { @@ -45,9 +45,11 @@ describe('OpenAPI error conversion', () => { })(error); // it provides the message - expect(description.includes(error.message)); + expect(description.includes(error.message)).toBeTruthy(); // it tells the user what they provided - expect(description.includes(JSON.stringify(parameterValue))); + expect( + description.includes(JSON.stringify(parameterValue)), + ).toBeTruthy(); }); it('Gives useful pattern error messages', () => { @@ -69,11 +71,11 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the pattern it should match is - expect(description.includes(error.params.pattern)); + expect(description.includes(error.params.pattern)).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('description')); + expect(description.includes('description')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(requestDescription)); + expect(description.includes(requestDescription)).toBeTruthy(); }); it('Gives useful min/maxlength error messages', () => { @@ -95,11 +97,13 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the pattern it should match is - expect(description.includes(error.params.limit.toString())); + expect( + description.includes(error.params.limit.toString()), + ).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('description')); + expect(description.includes('description')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(requestDescription)); + expect(description.includes(requestDescription)).toBeTruthy(); }); it('Handles numerical min/max errors', () => { @@ -123,13 +127,15 @@ describe('OpenAPI error conversion', () => { })(error); // it tells the user what the limit is - expect(description.includes(error.params.limit.toString())); + expect( + description.includes(error.params.limit.toString()), + ).toBeTruthy(); // it tells the user what kind of comparison it performed - expect(description.includes(error.params.comparison)); + expect(description.includes(error.params.comparison)).toBeTruthy(); // it tells the user which property it pertains to - expect(description.includes('newprop')); + expect(description.includes('newprop')).toBeTruthy(); // it tells the user what they provided - expect(description.includes(propertyValue.toString())); + expect(description.includes(propertyValue.toString())).toBeTruthy(); }); it('Handles multiple errors', () => { @@ -175,6 +181,65 @@ describe('OpenAPI error conversion', () => { ); }); + describe('Disallowed additional properties', () => { + it('gives useful messages for base-level properties', () => { + const openApiError = { + keyword: 'additionalProperties', + instancePath: '', + dataPath: '.body', + schemaPath: + '#/components/schemas/addonCreateUpdateSchema/additionalProperties', + params: { additionalProperty: 'bogus' }, + message: 'should NOT have additional properties', + }; + + const error = fromOpenApiValidationError({ bogus: 5 })( + openApiError, + ); + + expect( + error.description.includes( + openApiError.params.additionalProperty, + ), + ).toBeTruthy(); + expect(error.description).toMatch(/\broot\b/i); + expect(error.description).toMatch(/\badditional properties\b/i); + }); + + it('gives useful messages for nested properties', () => { + const request2 = { + nestedObject: { + nested2: { extraPropertyName: 'illegal property' }, + }, + }; + const openApiError = { + keyword: 'additionalProperties', + instancePath: '', + dataPath: '.body.nestedObject.nested2', + schemaPath: + '#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/nested2/additionalProperties', + params: { additionalProperty: 'extraPropertyName' }, + message: 'should NOT have additional properties', + }; + + const error = fromOpenApiValidationError(request2)(openApiError); + + expect( + error.description.includes('nestedObject.nested2'), + ).toBeTruthy(); + expect( + error.description.includes( + openApiError.params.additionalProperty, + ), + ).toBeTruthy(); + expect( + error.description + .toLowerCase() + .includes('additional properties'), + ).toBeTruthy(); + }); + }); + it('Handles deeply nested properties gracefully', () => { const error = { keyword: 'type', @@ -191,9 +256,9 @@ describe('OpenAPI error conversion', () => { })(error); // it should hold the full path to the error - expect(description.includes('nestedObject.a.b')); + expect(description).toMatch(/\bnestedObject.a.b\b/); // it should include the value that the user sent - expect(description.includes('[]')); + expect(description.includes('[]')).toBeTruthy(); }); it('Handles deeply nested properties on referenced schemas', () => { @@ -212,8 +277,8 @@ describe('OpenAPI error conversion', () => { })(error); // it should hold the full path to the error - expect(description.includes('nestedObject.a.b')); + expect(description).toMatch(/\bnestedObject.a.b\b/); // it should include the value that the user sent - expect(description.includes(illegalValue)); + expect(description.includes(illegalValue)).toBeTruthy(); }); }); diff --git a/src/lib/error/api-error.ts b/src/lib/error/api-error.ts index 1a80d005f1..cdaff9c42c 100644 --- a/src/lib/error/api-error.ts +++ b/src/lib/error/api-error.ts @@ -279,8 +279,24 @@ export const fromOpenApiValidationError = description, message: description, }; + } else if (validationError.keyword === 'additionalProperties') { + const path = + (propertyName ? propertyName + '.' : '') + + validationError.params.additionalProperty; + const description = `The ${ + propertyName ? `\`${propertyName}\`` : 'root' + } object of the request body does not allow additional properties. Your request included the \`${path}\` property.`; + return { + path, + description, + message: description, + }; } else { - const youSent = JSON.stringify(requestBody[propertyName]); + const input = propertyName + .split('.') + .reduce((x, prop) => x[prop], requestBody); + + const youSent = JSON.stringify(input); const description = `The .${propertyName} property ${validationError.message}. You sent ${youSent}.`; return { description, diff --git a/src/lib/features/export-import-toggles/export-import-service.ts b/src/lib/features/export-import-toggles/export-import-service.ts index a0397bd677..d478cabfcb 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -372,9 +372,13 @@ export default class ExportImportService { 'Some of the context fields you are trying to import are not supported.', // @ts-ignore-error We know that the array contains at least one // element here. - errors: unsupportedContextFields.map((field) => ({ - description: `${field.name} is not supported.`, - })), + details: unsupportedContextFields.map((field) => { + const description = `${field.name} is not supported.`; + return { + description, + message: description, + }; + }), }); } } @@ -453,9 +457,14 @@ export default class ExportImportService { 'Some of the strategies you are trying to import are not supported.', // @ts-ignore-error We know that the array contains at least one // element here. - errors: unsupportedStrategies.map((strategy) => ({ - description: `${strategy.name} is not supported.`, - })), + details: unsupportedStrategies.map((strategy) => { + const description = `${strategy.name} is not supported.`; + + return { + description, + message: description, + }; + }), }); } } diff --git a/src/lib/features/export-import-toggles/export-import.e2e.test.ts b/src/lib/features/export-import-toggles/export-import.e2e.test.ts index b61f31d348..7d75ed7fb0 100644 --- a/src/lib/features/export-import-toggles/export-import.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import.e2e.test.ts @@ -673,11 +673,7 @@ test('reject import with unknown context fields', async () => { 400, ); - expect( - body.errors.includes((error) => - error.description.includes('ContextField1'), - ), - ); + expect(body.details[0].description).toMatch(/\bContextField1\b/); }); test('reject import with unsupported strategies', async () => { @@ -697,11 +693,7 @@ test('reject import with unsupported strategies', async () => { 400, ); - expect( - body.errors.includes((error) => - error.description.includes('customStrategy'), - ), - ); + expect(body.details[0].description).toMatch(/\bcustomStrategy\b/); }); test('validate import data', async () => { diff --git a/src/lib/routes/admin-api/strategy.test.ts b/src/lib/routes/admin-api/strategy.test.ts index 1a6ce3bcaf..f1d13402f3 100644 --- a/src/lib/routes/admin-api/strategy.test.ts +++ b/src/lib/routes/admin-api/strategy.test.ts @@ -57,7 +57,7 @@ test('require a name when creating a new strategy', async () => { ['name', 'property', 'required'].every((word) => res.body.details[0].description.includes(word), ), - ); + ).toBeTruthy(); }); }); diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 451299d1e2..c249bb27b0 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -184,8 +184,10 @@ test('Trying to add a strategy configuration to environment not connected to tog }) .expect(400) .expect((r) => { - expect(r.body.message.includes('environment')); - expect(r.body.message.includes('project')); + expect( + r.body.details[0].message.includes('environment'), + ).toBeTruthy(); + expect(r.body.details[0].message.includes('project')).toBeTruthy(); }); }); @@ -776,12 +778,12 @@ test('Trying to patch variants on a feature toggle should trigger an OperationDe ]) .expect(403) .expect((res) => { - expect(res.body.message.includes('PATCH')); + expect(res.body.message.includes('PATCH')).toBeTruthy(); expect( res.body.message.includes( '/api/admin/projects/:project/features/:feature/variants', ), - ); + ).toBeTruthy(); }); }); diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 7805365a4d..fbf8c84899 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -777,8 +777,10 @@ test('Should be denied move feature toggle to project where the user does not ha ); } catch (e) { expect(e.name).toContain('NoAccess'); - expect(e.message.includes('permission')); - expect(e.message.includes(permissions.MOVE_FEATURE_TOGGLE)); + expect(e.message.includes('permission')).toBeTruthy(); + expect( + e.message.includes(permissions.MOVE_FEATURE_TOGGLE), + ).toBeTruthy(); } }); diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 44afcb4a2a..2fe56648f1 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -538,8 +538,8 @@ test('should not change project if feature toggle project does not match current 'wrong-project-id', ); } catch (err) { - expect(err.message.toLowerCase().includes('permission')); - expect(err.message.includes(MOVE_FEATURE_TOGGLE)); + expect(err.message.toLowerCase().includes('permission')).toBeTruthy(); + expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy(); } }); @@ -604,8 +604,8 @@ test('should fail if user is not authorized', async () => { project.id, ); } catch (err) { - expect(err.message.toLowerCase().includes('permission')); - expect(err.message.includes(MOVE_FEATURE_TOGGLE)); + expect(err.message.toLowerCase().includes('permission')).toBeTruthy(); + expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy(); } });