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

temporarily disabled input validation #3684

Merged
merged 1 commit into from
Oct 18, 2024
Merged

temporarily disabled input validation #3684

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
95 changes: 49 additions & 46 deletions src/auth-service/routes/v2/preferences.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ function createValidateSelectedSitesField(requiredFields, allowId = false) {
throw new Error("_id field is not allowed");
}

// Check for required fields
if (!requiredFields.every((field) => field in value)) {
throw new Error(
`Missing required fields: ${requiredFields
Expand All @@ -48,6 +47,16 @@ function createValidateSelectedSitesField(requiredFields, allowId = false) {
);
}

const isValidId = isMongoId(value._id);
if (!isValidId && allowId) {
throw new Error("_id must be a valid MongoDB ObjectId");
}

const isValidSiteId = isMongoId(value.site_id);
if (!isValidSiteId && !allowId) {
throw new Error("site_id must be a valid MongoDB ObjectId");
}

function validateNumericFields(fields) {
for (const field of fields) {
if (field in value) {
Expand Down Expand Up @@ -80,6 +89,9 @@ function createValidateSelectedSitesField(requiredFields, allowId = false) {
(typeof value[field] !== "string" || value[field].trim() === "")
) {
throw new Error(`${field} must be a non-empty string`);
} else if (!(field in value)) {
// Log missing field
throw new Error(`Field "${field}" is missing`);
}
}
return true;
Expand All @@ -99,11 +111,6 @@ function createValidateSelectedSitesField(requiredFields, allowId = false) {
}
}

const isValidSiteId = "site_id" in value && isMongoId(value.site_id);
if (!isValidSiteId) {
throw new Error("site_id must be a valid MongoDB ObjectId");
}

const numericValid = validateNumericFields([
"latitude",
"longitude",
Expand All @@ -112,11 +119,10 @@ function createValidateSelectedSitesField(requiredFields, allowId = false) {
]);

const stringValid = validateStringFields(["name", "search_name"]);

const tags = value && value.site_tags;
const tagValid = validateTags(tags);

return numericValid && stringValid && tagValid && isValidSiteId;
return numericValid && stringValid && tagValid;
};
}

Expand Down Expand Up @@ -386,14 +392,14 @@ router.post(
return Array.isArray(value);
})
.withMessage("the selected_sites should be an array"),
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
),
// body("selected_sites.*")
// .optional()
// .custom(
// createValidateSelectedSitesField(["_id", "search_name", "name"], true)
// )
// .withMessage(
// "Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
// ),
Comment on lines +395 to +402
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider the Risks of Disabling Validation for 'selected_sites'

Commenting out the validation for selected_sites.* removes important checks on user input. While temporarily disabling validation might be necessary, please be mindful of potential issues such as processing invalid data or introducing security vulnerabilities. It's advisable to ensure that other safeguards are in place to handle unvalidated input appropriately.

],
]),
createPreferenceController.upsert
Expand Down Expand Up @@ -592,9 +598,6 @@ router.patch(
// .optional()
// .custom(
// createValidateSelectedSitesField(["_id", "search_name", "name"], true)
// )
// .withMessage(
// "Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
// ),
],
]),
Expand Down Expand Up @@ -791,14 +794,14 @@ router.put(
return Array.isArray(value);
})
.withMessage("the selected_sites should be an array"),
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
),
// body("selected_sites.*")
// .optional()
// .custom(
// createValidateSelectedSitesField(["_id", "search_name", "name"], true)
// )
// .withMessage(
// "Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
// ),
Comment on lines +797 to +804
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Assess Impact of Disabled Validation in Update Route

The validation for selected_sites.* in the update route has been commented out. This could allow invalid or malformed data to be accepted during updates. Please review this change to ensure it doesn't adversely affect the application's stability or security.

],
]),
createPreferenceController.update
Expand Down Expand Up @@ -961,14 +964,14 @@ router.post(
return Array.isArray(value);
})
.withMessage("the selected_sites should be an array"),
body("selected_sites.*")
.optional()
.custom(
createValidateSelectedSitesField(["_id", "search_name", "name"], true)
)
.withMessage(
"Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
),
// body("selected_sites.*")
// .optional()
// .custom(
// createValidateSelectedSitesField(["_id", "search_name", "name"], true)
// )
// .withMessage(
// "Invalid selected_sites format. Verify required fields (latitude, longitude, search_name, name, approximate_latitude, approximate_longitude), numeric fields (latitude, longitude, approximate_latitude, approximate_longitude, search_radius if present), string fields (name, search_name), and ensure site_tags is an array of strings."
// ),
Comment on lines +967 to +974
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review Disabled Validation in Create Route

Disabling the validation for selected_sites.* during creation may lead to inconsistent or incorrect data being stored. Consider implementing alternative validation measures or documenting this change thoroughly for future reference.

],
]),
createPreferenceController.create
Expand Down Expand Up @@ -1194,12 +1197,12 @@ router.post(
.bail()
.notEmpty()
.withMessage("selected_sites should not be empty"),
body("selected_sites.*").custom(
createValidateSelectedSitesField(
["site_id", "search_name", "name"],
false
)
),
// body("selected_sites.*").custom(
// createValidateSelectedSitesField(
// ["site_id", "search_name", "name"],
// false
// )
// ),
Comment on lines +1200 to +1205
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential Risks with Disabled Validation in 'addSelectedSites'

By commenting out the validation for selected_sites.*, the application might accept invalid data when adding selected sites. It's important to ensure that this does not compromise data quality or application security.

],
]),
setJWTAuth,
Expand Down Expand Up @@ -1233,11 +1236,11 @@ router.put(
.customSanitizer((value) => {
return ObjectId(value);
}),
body("selected_site")
.custom(createValidateSelectedSitesField([], false))
.withMessage(
"Invalid selected site data. Verify required fields and data types."
),
// body("selected_site")
// .custom(createValidateSelectedSitesField([], false))
// .withMessage(
// "Invalid selected site data. Verify required fields and data types."
// ),
Comment on lines +1239 to +1243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Evaluate Disabled Validation in 'updateSelectedSite'

The absence of validation for updating a selected site could result in unintended data modifications. Please consider the necessity of this change and whether additional checks are needed elsewhere.

],
]),
setJWTAuth,
Expand Down
Loading