Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix failing tests regarding form validation #921

Merged
merged 7 commits into from
Sep 2, 2024
Merged

Fix failing tests regarding form validation #921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7fd6ef2
Add fix for failing tests regarding form validation
deepjyoti30Alt Sep 2, 2024
c58ec6a
Include build files for source changes
deepjyoti30Alt Sep 2, 2024
65825c8
Include a type fix
deepjyoti30Alt Sep 2, 2024
d610ddd
Remove unnecessary log
deepjyoti30Alt Sep 2, 2024
aae5160
Update form validation to be more specific and accurate
deepjyoti30Alt Sep 2, 2024
95f66a9
Add a fix for validation logic
deepjyoti30Alt Sep 2, 2024
c84cdd5
Fix the form validation errors to be accurate and update tests
deepjyoti30Alt Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 22 additions & 30 deletions lib/build/recipe/emailpassword/api/utils.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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, "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
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: "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?");
});
});