Skip to content

Commit

Permalink
Fix failing tests regarding form validation (#921)
Browse files Browse the repository at this point in the history 
* Add fix for failing tests regarding form validation

* Include build files for source changes

* Include a type fix

* Remove unnecessary log

* Update form validation to be more specific and accurate

* Add a fix for validation logic

* Fix the form validation errors to be accurate and update tests
  • Loading branch information
@deepjyoti30Alt
deepjyoti30Alt authored Sep 2, 2024
1 parent e9641cf commit cdf0961
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 73 deletions.
52 changes: 22 additions & 30 deletions lib/build/recipe/emailpassword/api/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Expand Down
57 changes: 25 additions & 32 deletions lib/ts/recipe/emailpassword/api/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Expand Down
97 changes: 86 additions & 11 deletions test/emailpassword/signupFeature.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -757,10 +757,12 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js]

app.use(errorHandler());

let response = await signUPRequest(app, "[email protected]", "validpass123");
assert(response.status === 400);

assert(JSON.parse(response.text).message === "Are you sending too many / too few formFields?");
let rawResponse = await signUPRequest(app, "[email protected]", "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
Expand Down Expand Up @@ -990,7 +992,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js]
},
],
})
.expect(400)
.end((err, res) => {
if (err) {
resolve(undefined);
Expand All @@ -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
Expand Down Expand Up @@ -1033,7 +1037,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js]
},
],
})
.expect(400)
.end((err, res) => {
if (err) {
resolve(undefined);
Expand All @@ -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)
Expand Down Expand Up @@ -1099,7 +1105,6 @@ describe(`signupFeature: ${printPath("[test/emailpassword/signupFeature.test.js]
},
],
})
.expect(400)
.end((err, res) => {
if (err) {
resolve(undefined);
Expand All @@ -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
Expand Down Expand Up @@ -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: "[email protected]",
},
{
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?");
});
});

0 comments on commit cdf0961

Please sign in to comment.