-
Notifications
You must be signed in to change notification settings - Fork 22
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.
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
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request focus on the validation logic for the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## staging #3684 +/- ##
========================================
Coverage 27.04% 27.04%
========================================
Files 146 146
Lines 21340 21340
Branches 274 274
========================================
Hits 5772 5772
Misses 15568 15568 |
Auth-service changes in this PR available for preview here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/auth-service/routes/v2/preferences.js (8 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/auth-service/routes/v2/preferences.js (2)
50-59
: Validation Logic for '_id' and 'site_id' Fields Looks GoodThe added validation correctly checks if '_id' and 'site_id' are valid MongoDB ObjectIds based on the 'allowId' parameter. This enhances the robustness of the input validation.
92-94
: Improved Handling of Missing String FieldsAdding an error throw for missing required string fields ensures that all necessary data is present and valid. This improvement strengthens data integrity.
// 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." | ||
// ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
// 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." | ||
// ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
// 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." | ||
// ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
// body("selected_sites.*").custom( | ||
// createValidateSelectedSitesField( | ||
// ["site_id", "search_name", "name"], | ||
// false | ||
// ) | ||
// ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
// body("selected_site") | ||
// .custom(createValidateSelectedSitesField([], false)) | ||
// .withMessage( | ||
// "Invalid selected site data. Verify required fields and data types." | ||
// ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Description
temporarily disabled input validation
Changes Made
Testing
Affected Services
Endpoints Ready for Testing
API Documentation Updated?
Additional Notes
temporarily disabled input validation
Summary by CodeRabbit
New Features
selected_sites
field to ensure proper data integrity.Bug Fixes
Refactor