diff --git a/lib/build/recipe/emailpassword/api/utils.js b/lib/build/recipe/emailpassword/api/utils.js index 8ba082c73..b947c990e 100644 --- a/lib/build/recipe/emailpassword/api/utils.js +++ b/lib/build/recipe/emailpassword/api/utils.js @@ -54,40 +54,32 @@ function newBadRequestError(message) { // and also validate optional form fields only when present async function validateFormOrThrowError(inputs, configFormFields, tenantId, userContext) { let validationErrors = []; - let requiredFormFields = new Set(); - configFormFields.forEach((formField) => { - if (!formField.optional) { - requiredFormFields.add(formField.id); - } - }); - if (inputs.length < requiredFormFields.size || inputs.length > configFormFields.length) { - throw newBadRequestError("Are you sending too many / too few formFields?"); + // Throw an error if the user has provided more than the allowed + // formFields. + if (inputs.length > configFormFields.length) { + throw newBadRequestError("Are you sending too many formFields?"); } + // NOTE: We don't need to check for required fields at all as that will + // be picked up in the below codeblock. for (const formField of configFormFields) { const input = inputs.find((input) => input.id === formField.id); - if (formField.optional) { - // Validate optional inputs only when they are present - if (input && input.value.length > 0) { - const error = await formField.validate(input.value, tenantId, userContext); - if (error) { - validationErrors.push({ - error, - id: formField.id, - }); - } - } - } else { - if (input && input.value.length > 0) { - const error = await formField.validate(input.value, tenantId, userContext); - if (error) { - validationErrors.push({ - error, - id: formField.id, - }); - } - } else { + // Add the not optional error if input is not passed + // and the field is not optional. + const isValidInput = + !!input && + ((typeof input.value === "string" && input.value.length > 0) || + (typeof input.value === "object" && Object.values(input.value).length > 0)); + if (!formField.optional && !isValidInput) { + validationErrors.push({ + error: "Field is not optional", + id: formField.id, + }); + } + if (isValidInput) { + const error = await formField.validate(input.value, tenantId, userContext); + if (error) { validationErrors.push({ - error: "Field is not optional", + error, id: formField.id, }); } diff --git a/lib/ts/recipe/emailpassword/api/utils.ts b/lib/ts/recipe/emailpassword/api/utils.ts index 1f45333ca..b30721035 100644 --- a/lib/ts/recipe/emailpassword/api/utils.ts +++ b/lib/ts/recipe/emailpassword/api/utils.ts @@ -87,51 +87,44 @@ function newBadRequestError(message: string) { async function validateFormOrThrowError( inputs: { id: string; - value: string; + value: string | object | undefined; }[], configFormFields: NormalisedFormField[], tenantId: string, userContext: UserContext ) { let validationErrors: { id: string; error: string }[] = []; - let requiredFormFields = new Set(); - - configFormFields.forEach((formField) => { - if (!formField.optional) { - requiredFormFields.add(formField.id); - } - }); - if (inputs.length < requiredFormFields.size || inputs.length > configFormFields.length) { - throw newBadRequestError("Are you sending too many / too few formFields?"); + // Throw an error if the user has provided more than the allowed + // formFields. + if (inputs.length > configFormFields.length) { + throw newBadRequestError("Are you sending too many formFields?"); } + // NOTE: We don't need to check for required fields at all as that will + // be picked up in the below codeblock. + for (const formField of configFormFields) { const input = inputs.find((input) => input.id === formField.id); - if (formField.optional) { - // Validate optional inputs only when they are present - if (input && input.value.length > 0) { - const error = await formField.validate(input.value, tenantId, userContext); - if (error) { - validationErrors.push({ - error, - id: formField.id, - }); - } - } - } else { - if (input && input.value.length > 0) { - const error = await formField.validate(input.value, tenantId, userContext); - if (error) { - validationErrors.push({ - error, - id: formField.id, - }); - } - } else { + // Add the not optional error if input is not passed + // and the field is not optional. + const isValidInput = + !!input && + ((typeof input.value === "string" && input.value.length > 0) || + (typeof input.value === "object" && Object.values(input.value).length > 0)); + if (!formField.optional && !isValidInput) { + validationErrors.push({ + error: "Field is not optional", + id: formField.id, + }); + } + + if (isValidInput) { + const error = await formField.validate(input!.value, tenantId, userContext); + if (error) { validationErrors.push({ - error: "Field is not optional", + error, id: formField.id, }); } diff --git a/test/emailpassword/signupFeature.test.js b/test/emailpassword/signupFeature.test.js index 9d9bab9bf..371c98d30 100644 --- a/test/emailpassword/signupFeature.test.js +++ b/test/emailpassword/signupFeature.test.js @@ -727,7 +727,7 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] * - Make sure that the input email is trimmed * - Pass a non string value in the formFields array and make sure it passes through the signUp API and is sent in the handlePostSignup as that type */ - it("test formFields added in config but not in inout to signup, check error about it being missing", async function () { + it("test formFields added in config but not in input to signup, check error about it being missing", async function () { const connectionURI = await startST(); STExpress.init({ supertokens: { @@ -757,10 +757,12 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] app.use(errorHandler()); - let response = await signUPRequest(app, "random@gmail.com", "validpass123"); - assert(response.status === 400); - - assert(JSON.parse(response.text).message === "Are you sending too many / too few formFields?"); + let rawResponse = await signUPRequest(app, "random@gmail.com", "validpass123"); + const response = JSON.parse(rawResponse.text); + assert(response.status === "FIELD_ERROR"); + assert(response.formFields.length === 1); + assert(response.formFields[0].error === "Field is not optional"); + assert(response.formFields[0].id === "testField"); }); //- Good test case without optional @@ -990,7 +992,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] }, ], }) - .expect(400) .end((err, res) => { if (err) { resolve(undefined); @@ -999,7 +1000,10 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] } }) ); - assert(response.message === "Are you sending too many / too few formFields?"); + assert(response.status === "FIELD_ERROR"); + assert(response.formFields.length === 1); + assert(response.formFields[0].error === "Field is not optional"); + assert(response.formFields[0].id === "email"); }); // Input formFields has no password field (and not in config @@ -1033,7 +1037,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] }, ], }) - .expect(400) .end((err, res) => { if (err) { resolve(undefined); @@ -1042,7 +1045,10 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] } }) ); - assert(response.message === "Are you sending too many / too few formFields?"); + assert(response.status === "FIELD_ERROR"); + assert(response.formFields.length === 1); + assert(response.formFields[0].error === "Field is not optional"); + assert(response.formFields[0].id === "password"); }); // Input form field has different number of custom fields than in config form fields) @@ -1099,7 +1105,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] }, ], }) - .expect(400) .end((err, res) => { if (err) { resolve(undefined); @@ -1108,7 +1113,10 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] } }) ); - assert(response.message === "Are you sending too many / too few formFields?"); + assert(response.status === "FIELD_ERROR"); + assert(response.formFields.length === 1); + assert(response.formFields[0].error === "Field is not optional"); + assert(response.formFields[0].id === "testField2"); }); // Input form field has same number of custom fields as in config form field, but some ids mismatch @@ -1832,4 +1840,71 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js] assert(r3.failureReason === "Password should be greater than 5 characters"); } }); + + // test case where more than the configured form fields are passed. + it("test bad case when too many formFields are passed", async function () { + const connectionURI = await startST(); + STExpress.init({ + supertokens: { + connectionURI, + }, + appInfo: { + apiDomain: "api.supertokens.io", + appName: "SuperTokens", + websiteDomain: "supertokens.io", + }, + recipeList: [ + EmailPassword.init({ + signUpFeature: { + formFields: [ + { + id: "testField", + }, + ], + }, + }), + Session.init({ getTokenTransferMethod: () => "cookie" }), + ], + }); + const app = express(); + + app.use(middleware()); + + app.use(errorHandler()); + + let response = await new Promise((resolve) => + request(app) + .post("/auth/signup") + .send({ + formFields: [ + { + id: "password", + value: "validpass123", + }, + { + id: "email", + value: "random@gmail.com", + }, + { + id: "testField", + value: "some value", + }, + { + id: "extraField", + value: "some value", + }, + ], + }) + .expect(400) + .end((err, res) => { + if (err) { + resolve(undefined); + } else { + resolve(JSON.parse(res.text)); + } + }) + ); + + assert(response.message == "Are you sending too many formFields?"); + }); });