From 3b3ea94e9cfd41cd86128d2ad159dab0cf70b1cf Mon Sep 17 00:00:00 2001 From: baalmart Date: Thu, 17 Oct 2024 00:48:46 +0300 Subject: [PATCH 01/21] hotfixes for input validations for all readings endpoints --- src/device-registry/routes/v2/readings.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/device-registry/routes/v2/readings.js b/src/device-registry/routes/v2/readings.js index fd0a08312a..bf69e1d376 100644 --- a/src/device-registry/routes/v2/readings.js +++ b/src/device-registry/routes/v2/readings.js @@ -228,8 +228,15 @@ router.get( query("site_id") .optional() .notEmpty() - .withMessage("the provided site_id cannot be empty IF provided") - .trim(), + .withMessage("site_id cannot be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("site_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), query("primary") .optional() .notEmpty() From f5e7804e582da1448c9c500a3042a4d705b2b240 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:06:07 +0300 Subject: [PATCH 02/21] Update device registry staging image tag to stage-42690b37-1729375485 --- k8s/device-registry/values-stage.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/device-registry/values-stage.yaml b/k8s/device-registry/values-stage.yaml index 7620590876..fb5ba44781 100644 --- a/k8s/device-registry/values-stage.yaml +++ b/k8s/device-registry/values-stage.yaml @@ -6,7 +6,7 @@ app: replicaCount: 2 image: repository: eu.gcr.io/airqo-250220/airqo-stage-device-registry-api - tag: stage-f5ce3fc9-1729339366 + tag: stage-42690b37-1729375485 nameOverride: '' fullnameOverride: '' podAnnotations: {} From 2295d075332147dbe5400ef69a6006af44f4e998 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:06:25 +0300 Subject: [PATCH 03/21] Update AirQo exceedance production image tag to prod-c73a134b-1729375524 --- k8s/exceedance/values-prod-airqo.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/exceedance/values-prod-airqo.yaml b/k8s/exceedance/values-prod-airqo.yaml index 6ba80bd405..c1ceae4f44 100644 --- a/k8s/exceedance/values-prod-airqo.yaml +++ b/k8s/exceedance/values-prod-airqo.yaml @@ -4,6 +4,6 @@ app: configmap: env-exceedance-production image: repository: eu.gcr.io/airqo-250220/airqo-exceedance-job - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' From 932ec8045fb2076bf2bdac31e389cc4d532dbfab Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:06:34 +0300 Subject: [PATCH 04/21] Update KCCA exceedance production image tag to prod-c73a134b-1729375524 --- k8s/exceedance/values-prod-kcca.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/exceedance/values-prod-kcca.yaml b/k8s/exceedance/values-prod-kcca.yaml index 014b759366..8fb197a32c 100644 --- a/k8s/exceedance/values-prod-kcca.yaml +++ b/k8s/exceedance/values-prod-kcca.yaml @@ -4,6 +4,6 @@ app: configmap: env-exceedance-production image: repository: eu.gcr.io/airqo-250220/kcca-exceedance-job - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' From 0c74a21a9b45814dd611a8f807503b2f0dc9238a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:07:24 +0300 Subject: [PATCH 05/21] Update auth service production image tag to prod-c73a134b-1729375524 --- k8s/auth-service/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/auth-service/values-prod.yaml b/k8s/auth-service/values-prod.yaml index 99e323559a..f5ff13d618 100644 --- a/k8s/auth-service/values-prod.yaml +++ b/k8s/auth-service/values-prod.yaml @@ -6,7 +6,7 @@ app: replicaCount: 3 image: repository: eu.gcr.io/airqo-250220/airqo-auth-api - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' podAnnotations: {} From 669d2c98f3146028c4bb86bb3dc085aec838155d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:07:41 +0300 Subject: [PATCH 06/21] Update device registry production image tag to prod-c73a134b-1729375524 --- k8s/device-registry/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/device-registry/values-prod.yaml b/k8s/device-registry/values-prod.yaml index b69e0542f1..1de030eab1 100644 --- a/k8s/device-registry/values-prod.yaml +++ b/k8s/device-registry/values-prod.yaml @@ -6,7 +6,7 @@ app: replicaCount: 3 image: repository: eu.gcr.io/airqo-250220/airqo-device-registry-api - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' podAnnotations: {} From 540f5f13a0a506a88b257b218dc9e2cfbf458932 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:08:13 +0300 Subject: [PATCH 07/21] Update workflows prod image tag to prod-c73a134b-1729375524 --- k8s/workflows/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/workflows/values-prod.yaml b/k8s/workflows/values-prod.yaml index ef4b0a6bb4..91823c5349 100644 --- a/k8s/workflows/values-prod.yaml +++ b/k8s/workflows/values-prod.yaml @@ -10,7 +10,7 @@ images: initContainer: eu.gcr.io/airqo-250220/airqo-workflows-xcom redisContainer: eu.gcr.io/airqo-250220/airqo-redis containers: eu.gcr.io/airqo-250220/airqo-workflows - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' podAnnotations: {} From 4ab81d50273603c4b2a8ea1a9b97c9493733b56e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:08:19 +0300 Subject: [PATCH 08/21] Update spatial production image tag to prod-c73a134b-1729375524 --- k8s/spatial/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/spatial/values-prod.yaml b/k8s/spatial/values-prod.yaml index b9a4d60b87..dee4fa929c 100644 --- a/k8s/spatial/values-prod.yaml +++ b/k8s/spatial/values-prod.yaml @@ -6,7 +6,7 @@ app: replicaCount: 3 image: repository: eu.gcr.io/airqo-250220/airqo-spatial-api - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 nameOverride: '' fullnameOverride: '' podAnnotations: {} From 131d0414eff6a4b29fd08bebbe9059a4f904a285 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:08:31 +0300 Subject: [PATCH 09/21] Update analytics production image tag to prod-c73a134b-1729375524 --- k8s/analytics/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/analytics/values-prod.yaml b/k8s/analytics/values-prod.yaml index 6a4a3b578f..7e43522cf8 100644 --- a/k8s/analytics/values-prod.yaml +++ b/k8s/analytics/values-prod.yaml @@ -8,7 +8,7 @@ images: celeryWorker: eu.gcr.io/airqo-250220/airqo-analytics-celery-worker reportJob: eu.gcr.io/airqo-250220/airqo-analytics-report-job devicesSummaryJob: eu.gcr.io/airqo-250220/airqo-analytics-devices-summary-job - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 api: name: airqo-analytics-api label: analytics-api From c02f824f6e45649d9ee3de559ed373d601c9b331 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 01:09:03 +0300 Subject: [PATCH 10/21] Update predict production image tag to prod-c73a134b-1729375524 --- k8s/predict/values-prod.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/predict/values-prod.yaml b/k8s/predict/values-prod.yaml index d487bbba76..22a8e7f60c 100644 --- a/k8s/predict/values-prod.yaml +++ b/k8s/predict/values-prod.yaml @@ -7,7 +7,7 @@ images: predictJob: eu.gcr.io/airqo-250220/airqo-predict-job trainJob: eu.gcr.io/airqo-250220/airqo-train-job predictPlaces: eu.gcr.io/airqo-250220/airqo-predict-places-air-quality - tag: prod-d0d63dbd-1729340566 + tag: prod-c73a134b-1729375524 api: name: airqo-prediction-api label: prediction-api From 2ece323e16bc08905dd3ad1e824d2c4d0e0d19fe Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 09:58:34 +0300 Subject: [PATCH 11/21] enhanced validation for creating default preferences --- .../middleware/test/ut_setDefaultTenant.js | 57 ++++++ .../test/ut_validateSelectedSites.js | 186 +++++++++++++++++ .../middleware/validateSelectedSites.js | 143 +++++++++++++ src/auth-service/routes/v2/preferences.js | 188 +++++++----------- 4 files changed, 455 insertions(+), 119 deletions(-) create mode 100644 src/auth-service/middleware/test/ut_setDefaultTenant.js create mode 100644 src/auth-service/middleware/test/ut_validateSelectedSites.js create mode 100644 src/auth-service/middleware/validateSelectedSites.js diff --git a/src/auth-service/middleware/test/ut_setDefaultTenant.js b/src/auth-service/middleware/test/ut_setDefaultTenant.js new file mode 100644 index 0000000000..83f460b1da --- /dev/null +++ b/src/auth-service/middleware/test/ut_setDefaultTenant.js @@ -0,0 +1,57 @@ +require("module-alias/register"); +const { expect } = require("chai"); +const sinon = require("sinon"); +const setDefaultTenant = require("@middleware/setDefaultTenant"); +const constants = require("@config/constants"); + +describe("setDefaultTenant Middleware", () => { + let req, res, next; + + beforeEach(() => { + req = { + query: {}, + }; + res = {}; + next = sinon.stub(); + }); + + afterEach(() => { + sinon.restore(); // Restore the original functionality of stubbed methods + }); + + it("should set the default tenant if tenant is empty", () => { + // Set up the constant for testing + constants.DEFAULT_TENANT = "defaultTenant"; + + const middleware = setDefaultTenant; + middleware(req, res, next); + + expect(req.query.tenant).to.equal("defaultTenant"); + expect(next.calledOnce).to.be.true; // Ensure next() is called + }); + + it("should keep the existing tenant if provided", () => { + req.query.tenant = "customTenant"; + + const middleware = setDefaultTenant; + middleware(req, res, next); + + expect(req.query.tenant).to.equal("customTenant"); + expect(next.calledOnce).to.be.true; // Ensure next() is called + }); + + it("should use 'airqo' as the default tenant if no constant is defined", () => { + // Temporarily remove DEFAULT_TENANT for this test + const originalDefaultTenant = constants.DEFAULT_TENANT; + delete constants.DEFAULT_TENANT; + + const middleware = setDefaultTenant; + middleware(req, res, next); + + expect(req.query.tenant).to.equal("airqo"); + expect(next.calledOnce).to.be.true; // Ensure next() is called + + // Restore the original constant value after test + constants.DEFAULT_TENANT = originalDefaultTenant; + }); +}); diff --git a/src/auth-service/middleware/test/ut_validateSelectedSites.js b/src/auth-service/middleware/test/ut_validateSelectedSites.js new file mode 100644 index 0000000000..c73a26544d --- /dev/null +++ b/src/auth-service/middleware/test/ut_validateSelectedSites.js @@ -0,0 +1,186 @@ +require("module-alias/register"); +const { expect } = require("chai"); +const sinon = require("sinon"); +const validateSelectedSites = require("@middleware/validateSelectedSites"); +const { logText } = require("@utils/log"); + +describe("validateSelectedSites Middleware", () => { + let req, res, next; + + beforeEach(() => { + req = { + body: { + selected_sites: [], + }, + }; + res = { + status: sinon.stub().returnsThis(), + json: sinon.stub(), + }; + next = sinon.stub(); + sinon.stub(logText); // Mock logText function + }); + + afterEach(() => { + sinon.restore(); // Restore the original functionality of stubbed methods + }); + + describe("when selected_sites is not an array", () => { + it("should return a 400 error if selected_sites is not an array", () => { + req.body.selected_sites = "not an array"; + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + message: "selected_sites should be a non-empty array", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + }); + + describe("when selected_sites is an empty array", () => { + it("should return a 400 error if selected_sites is an empty array", () => { + req.body.selected_sites = []; + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + message: "selected_sites should be a non-empty array", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + }); + + describe("when a site is null or undefined", () => { + it("should return a 400 error for null site value", () => { + req.body.selected_sites = [null]; + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["Site value must not be null or undefined"], + }, + message: "bad request errors", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + }); + + describe("when required fields are missing", () => { + it("should return a 400 error for missing required fields", () => { + req.body.selected_sites = [{}]; // Site object with no fields + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": [ + "Missing required fields: site_id, name, search_name", + ], + "site[0]": ['Field "name" is missing'], + }, + message: "bad request errors", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + }); + + describe("when fields are of invalid types", () => { + it("should return a 400 error for invalid site_id format", () => { + req.body.selected_sites = [{ site_id: "invalid_id" }]; + + const middleware = validateSelectedSites(["site_id"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["site_id must be a valid MongoDB ObjectId"], + }, + message: "bad request errors", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + + it("should return a 400 error for non-string name field", () => { + req.body.selected_sites = [{ name: 123 }]; + + const middleware = validateSelectedSites(["name"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["name must be a non-empty string"], + }, + message: "bad request errors", + }) + ).to.be.true; + expect(next.notCalled).to.be.true; + }); + }); + + describe("when all validations pass", () => { + it("should call next() when all validations are successful", () => { + req.body.selected_sites = [ + { + site_id: "610a43c1909756001e235e93", + name: "Valid Site", + search_name: "Valid Search", + }, + ]; + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); + + middleware(req, res, next); + + expect(next.calledOnce).to.be.true; // Ensure next() is called + }); + }); +}); diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js new file mode 100644 index 0000000000..79d465ac29 --- /dev/null +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -0,0 +1,143 @@ +const { isMongoId } = require("validator"); +const { logText } = require("@utils/log"); +const isEmpty = require("is-empty"); + +const validateSelectedSites = (requiredFields, allowId = false) => { + return (req, res, next) => { + const selectedSites = req.body.selected_sites; + const errors = {}; // Object to hold error messages + + if (!Array.isArray(selectedSites) || selectedSites.length === 0) { + logText("selected_sites should be a non-empty array"); + return res.status(400).json({ + success: false, + message: "selected_sites should be a non-empty array", + }); + } + + selectedSites.forEach((site, index) => { + const siteErrors = []; // Array to hold errors for the current site + + if (!site) { + logText(`Site at index ${index} must not be null or undefined`); + siteErrors.push("Site value must not be null or undefined"); + } + + // Edge case: Check if _id field is present + if (!allowId && "_id" in site) { + logText(`_id field is not allowed at index ${index}`); + siteErrors.push("_id field is not allowed"); + } + + // Check for missing required fields + const missingFields = requiredFields.filter((field) => !(field in site)); + if (missingFields.length > 0) { + logText( + `Missing required fields at index ${index}: ${missingFields.join( + ", " + )}` + ); + siteErrors.push(`Missing required fields: ${missingFields.join(", ")}`); + } + + // Validate _id if allowed + if (allowId && site._id && !isMongoId(site._id)) { + logText(`_id must be a valid MongoDB ObjectId at index ${index}`); + siteErrors.push("_id must be a valid MongoDB ObjectId"); + } + + // Validate site_id if not allowing _id + if (!allowId && site.site_id && !isMongoId(site.site_id)) { + logText(`site_id must be a valid MongoDB ObjectId at index ${index}`); + siteErrors.push("site_id must be a valid MongoDB ObjectId"); + } + + // Validate numeric fields + const numericFields = [ + "latitude", + "longitude", + "approximate_latitude", + "approximate_longitude", + ]; + + numericFields.forEach((field) => { + if (field in site) { + const numValue = parseFloat(site[field]); + if (Number.isNaN(numValue)) { + logText(`${field} must be a valid number at index ${index}`); + siteErrors.push(`${field} must be a valid number`); + } + if ( + [ + "latitude", + "longitude", + "approximate_latitude", + "approximate_longitude", + ].includes(field) + ) { + if (Math.abs(numValue) > 90) { + logText(`${field} must be between -90 and 90 at index ${index}`); + siteErrors.push(`${field} must be between -90 and 90`); + } + } + if (field === "search_radius" && numValue <= 0) { + logText(`${field} must be greater than 0 at index ${index}`); + siteErrors.push(`${field} must be greater than 0`); + } + } + }); + + // Validate string fields + const stringFields = ["name", "search_name"]; + stringFields.forEach((field) => { + if (!(field in site)) { + logText(`Field "${field}" is missing at index ${index}`); + // Only add this error once, as it's already captured in missingFields + // No need to add it again here. + } else if ( + typeof site[field] !== "string" || + site[field].trim() === "" + ) { + logText(`${field} must be a non-empty string at index ${index}`); + siteErrors.push(`${field} must be a non-empty string`); + } + }); + + // Validate tags + const tags = site.site_tags; + if (!isEmpty(tags)) { + if (!Array.isArray(tags)) { + logText("site_tags must be an array"); + siteErrors.push("site_tags must be an array"); + } + + tags.forEach((tag, tagIndex) => { + if (typeof tag !== "string") { + logText( + `site_tags[${tagIndex}] must be a string at index ${index}` + ); + siteErrors.push(`site_tags[${tagIndex}] must be a string`); + } + }); + } + + // If there are any errors for this site, add them to the main errors object + if (siteErrors.length > 0) { + errors[`selected_sites[${index}]`] = siteErrors; + } + }); + + // If any errors were collected, respond with them + if (Object.keys(errors).length > 0) { + return res.status(400).json({ + success: false, + message: "bad request errors", + errors, + }); + } + + next(); // Proceed to the next middleware or route handler + }; +}; + +module.exports = validateSelectedSites; diff --git a/src/auth-service/routes/v2/preferences.js b/src/auth-service/routes/v2/preferences.js index 8590b84320..f793d5d882 100644 --- a/src/auth-service/routes/v2/preferences.js +++ b/src/auth-service/routes/v2/preferences.js @@ -10,6 +10,7 @@ const isEmpty = require("is-empty"); const { logText, logObject } = require("@utils/log"); const { isMongoId } = require("validator"); // const stringify = require("@utils/stringify"); +const validateSelectedSites = require("@middleware/validateSelectedSites"); const validatePagination = (req, res, next) => { const limit = parseInt(req.query.limit, 10); @@ -1172,144 +1173,93 @@ router.get( ); router.post( "/selected-sites", + query("tenant") + .optional() + .notEmpty() + .withMessage("tenant should not be empty if provided") + .trim() + .toLowerCase() + .bail() + .isIn(["kcca", "airqo"]) + .withMessage("the tenant value is not among the expected ones"), validateUniqueFieldsInSelectedSites, - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - body("selected_sites") - .exists() - .withMessage("selected_sites should be provided") - .bail() - .isArray() - .withMessage("selected_sites should be an array") - .bail() - .notEmpty() - .withMessage("selected_sites should not be empty"), - // body("selected_sites.*").custom( - // createValidateSelectedSitesField( - // ["site_id", "search_name", "name"], - // false - // ) - // ), - ], - ]), + validateSelectedSites(["site_id", "search_name", "name"], false), setJWTAuth, authJWT, createPreferenceController.addSelectedSites ); router.put( "/selected-sites/:site_id", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - param("site_id") - .exists() - .withMessage("the site_id parameter is required") - .bail() - .isMongoId() - .withMessage("site_id must be a valid MongoDB ObjectId") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - // body("selected_site") - // .custom(createValidateSelectedSitesField([], false)) - // .withMessage( - // "Invalid selected site data. Verify required fields and data types." - // ), - ], - ]), + query("tenant") + .optional() + .notEmpty() + .withMessage("tenant should not be empty if provided") + .trim() + .toLowerCase() + .bail() + .isIn(["kcca", "airqo"]) + .withMessage("the tenant value is not among the expected ones"), + param("site_id") + .exists() + .withMessage("the site_id parameter is required") + .bail() + .isMongoId() + .withMessage("site_id must be a valid MongoDB ObjectId") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), setJWTAuth, authJWT, createPreferenceController.updateSelectedSite ); router.delete( "/selected-sites/:site_id", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - param("site_id") - .exists() - .withMessage("the site_id parameter is required") - .bail() - .isMongoId() - .withMessage("site_id must be a valid MongoDB ObjectId") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ], - ]), + query("tenant") + .optional() + .notEmpty() + .withMessage("tenant should not be empty if provided") + .trim() + .toLowerCase() + .bail() + .isIn(["kcca", "airqo"]) + .withMessage("the tenant value is not among the expected ones"), + param("site_id") + .exists() + .withMessage("the site_id parameter is required") + .bail() + .isMongoId() + .withMessage("site_id must be a valid MongoDB ObjectId") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), setJWTAuth, authJWT, createPreferenceController.deleteSelectedSite ); router.get( "/:user_id", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - param("user_id") - .exists() - .withMessage("the user ID param is missing in the request") - .bail() - .trim() - .isMongoId() - .withMessage("the user ID must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ], - ]), + query("tenant") + .optional() + .notEmpty() + .withMessage("tenant should not be empty if provided") + .trim() + .toLowerCase() + .bail() + .isIn(["kcca", "airqo"]) + .withMessage("the tenant value is not among the expected ones"), + param("user_id") + .exists() + .withMessage("the user ID param is missing in the request") + .bail() + .trim() + .isMongoId() + .withMessage("the user ID must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), setJWTAuth, authJWT, createPreferenceController.list From 38c517fe7ec7ac8c23d2603b98bba75f53788587 Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 10:35:09 +0300 Subject: [PATCH 12/21] more enhancements for selected sites validation middleware --- .../test/ut_validateSelectedSites.js | 141 ++++++++++++- .../middleware/validateSelectedSites.js | 188 +++++++++++------- src/auth-service/routes/v2/preferences.js | 1 + 3 files changed, 255 insertions(+), 75 deletions(-) diff --git a/src/auth-service/middleware/test/ut_validateSelectedSites.js b/src/auth-service/middleware/test/ut_validateSelectedSites.js index c73a26544d..4342e9ad12 100644 --- a/src/auth-service/middleware/test/ut_validateSelectedSites.js +++ b/src/auth-service/middleware/test/ut_validateSelectedSites.js @@ -111,9 +111,10 @@ describe("validateSelectedSites Middleware", () => { success: false, errors: { "selected_sites[0]": [ - "Missing required fields: site_id, name, search_name", + 'Field "site_id" is missing', + 'Field "name" is missing', + 'Field "search_name" is missing', ], - "site[0]": ['Field "name" is missing'], }, message: "bad request errors", }) @@ -160,6 +161,136 @@ describe("validateSelectedSites Middleware", () => { ).to.be.true; expect(next.notCalled).to.be.true; }); + + it("should return a 400 error for invalid latitude value", () => { + req.body.selected_sites = [{ latitude: 100 }]; // Invalid latitude + + const middleware = validateSelectedSites(["latitude"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["latitude must be between -90 and 90"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for invalid longitude value", () => { + req.body.selected_sites = [{ longitude: -200 }]; // Invalid longitude + + const middleware = validateSelectedSites(["longitude"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["longitude must be between -180 and 180"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for invalid approximate_latitude value", () => { + req.body.selected_sites = [{ approximate_latitude: 100 }]; // Invalid approximate_latitude + + const middleware = validateSelectedSites(["approximate_latitude"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": [ + "approximate_latitude must be between -90 and 90", + ], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for invalid approximate_longitude value", () => { + req.body.selected_sites = [{ approximate_longitude: -200 }]; // Invalid approximate_longitude + + const middleware = validateSelectedSites(["approximate_longitude"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": [ + "approximate_longitude must be between -180 and 180", + ], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for non-array site_tags", () => { + req.body.selected_sites = [{ site_tags: "not_an_array" }]; // Invalid site_tags + + const middleware = validateSelectedSites(["site_tags"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["site_tags must be an array"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for non-string tags in site_tags", () => { + req.body.selected_sites = [{ site_tags: [123] }]; // Invalid tag + + const middleware = validateSelectedSites(["site_tags"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["site_tags[0] must be a string"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + + it("should return a 400 error for invalid isFeatured value", () => { + req.body.selected_sites = [{ isFeatured: "not_a_boolean" }]; // Invalid isFeatured + + const middleware = validateSelectedSites(["isFeatured"]); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["isFeatured must be a boolean"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); }); describe("when all validations pass", () => { @@ -169,6 +300,12 @@ describe("validateSelectedSites Middleware", () => { site_id: "610a43c1909756001e235e93", name: "Valid Site", search_name: "Valid Search", + latitude: -1.2345, + longitude: 34.5678, + approximate_latitude: -1.2345, + approximate_longitude: 34.5678, + site_tags: ["tag1", "tag2"], + country: "Country Name", }, ]; diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js index 79d465ac29..c80991b9c2 100644 --- a/src/auth-service/middleware/validateSelectedSites.js +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -4,18 +4,11 @@ const isEmpty = require("is-empty"); const validateSelectedSites = (requiredFields, allowId = false) => { return (req, res, next) => { - const selectedSites = req.body.selected_sites; + const selectedSites = req.body.selected_sites || req.body; // Check for wrapper field or direct body const errors = {}; // Object to hold error messages - if (!Array.isArray(selectedSites) || selectedSites.length === 0) { - logText("selected_sites should be a non-empty array"); - return res.status(400).json({ - success: false, - message: "selected_sites should be a non-empty array", - }); - } - - selectedSites.forEach((site, index) => { + // Helper function to validate a single site + const validateSite = (site, index) => { const siteErrors = []; // Array to hold errors for the current site if (!site) { @@ -29,16 +22,13 @@ const validateSelectedSites = (requiredFields, allowId = false) => { siteErrors.push("_id field is not allowed"); } - // Check for missing required fields - const missingFields = requiredFields.filter((field) => !(field in site)); - if (missingFields.length > 0) { - logText( - `Missing required fields at index ${index}: ${missingFields.join( - ", " - )}` - ); - siteErrors.push(`Missing required fields: ${missingFields.join(", ")}`); - } + // Validate required fields directly + requiredFields.forEach((field) => { + if (!(field in site)) { + logText(`Field "${field}" is missing at index ${index}`); + siteErrors.push(`Field "${field}" is missing`); + } + }); // Validate _id if allowed if (allowId && site._id && !isMongoId(site._id)) { @@ -46,64 +36,61 @@ const validateSelectedSites = (requiredFields, allowId = false) => { siteErrors.push("_id must be a valid MongoDB ObjectId"); } - // Validate site_id if not allowing _id - if (!allowId && site.site_id && !isMongoId(site.site_id)) { - logText(`site_id must be a valid MongoDB ObjectId at index ${index}`); - siteErrors.push("site_id must be a valid MongoDB ObjectId"); + // Validate site_id + if (site.site_id && typeof site.site_id !== "string") { + logText(`site_id must be a non-empty string at index ${index}`); + siteErrors.push("site_id must be a non-empty string"); } - // Validate numeric fields - const numericFields = [ - "latitude", - "longitude", - "approximate_latitude", - "approximate_longitude", - ]; + // Validate latitude + if (site.latitude !== undefined) { + const latValue = parseFloat(site.latitude); + if (Number.isNaN(latValue) || latValue < -90 || latValue > 90) { + logText(`latitude must be between -90 and 90 at index ${index}`); + siteErrors.push("latitude must be between -90 and 90"); + } + } - numericFields.forEach((field) => { - if (field in site) { - const numValue = parseFloat(site[field]); - if (Number.isNaN(numValue)) { - logText(`${field} must be a valid number at index ${index}`); - siteErrors.push(`${field} must be a valid number`); - } - if ( - [ - "latitude", - "longitude", - "approximate_latitude", - "approximate_longitude", - ].includes(field) - ) { - if (Math.abs(numValue) > 90) { - logText(`${field} must be between -90 and 90 at index ${index}`); - siteErrors.push(`${field} must be between -90 and 90`); - } - } - if (field === "search_radius" && numValue <= 0) { - logText(`${field} must be greater than 0 at index ${index}`); - siteErrors.push(`${field} must be greater than 0`); - } + // Validate longitude + if (site.longitude !== undefined) { + const longValue = parseFloat(site.longitude); + if (Number.isNaN(longValue) || longValue < -180 || longValue > 180) { + logText(`longitude must be between -180 and 180 at index ${index}`); + siteErrors.push("longitude must be between -180 and 180"); } - }); + } - // Validate string fields - const stringFields = ["name", "search_name"]; - stringFields.forEach((field) => { - if (!(field in site)) { - logText(`Field "${field}" is missing at index ${index}`); - // Only add this error once, as it's already captured in missingFields - // No need to add it again here. - } else if ( - typeof site[field] !== "string" || - site[field].trim() === "" + // Validate approximate_latitude + if (site.approximate_latitude !== undefined) { + const approxLatValue = parseFloat(site.approximate_latitude); + if ( + Number.isNaN(approxLatValue) || + approxLatValue < -90 || + approxLatValue > 90 ) { - logText(`${field} must be a non-empty string at index ${index}`); - siteErrors.push(`${field} must be a non-empty string`); + logText( + `approximate_latitude must be between -90 and 90 at index ${index}` + ); + siteErrors.push("approximate_latitude must be between -90 and 90"); } - }); + } + + // Validate approximate_longitude + if (site.approximate_longitude !== undefined) { + const approxLongValue = parseFloat(site.approximate_longitude); + if ( + Number.isNaN(approxLongValue) || + approxLongValue < -180 || + approxLongValue > 180 + ) { + logText( + `approximate_longitude must be between -180 and 180 at index ${index}` + ); + siteErrors.push("approximate_longitude must be between -180 and 180"); + } + } - // Validate tags + // Validate site_tags const tags = site.site_tags; if (!isEmpty(tags)) { if (!Array.isArray(tags)) { @@ -121,11 +108,66 @@ const validateSelectedSites = (requiredFields, allowId = false) => { }); } - // If there are any errors for this site, add them to the main errors object + // Validate optional string fields only when they are present + const optionalStringFields = [ + "country", + "district", + "sub_county", + "parish", + "county", + "city", + "generated_name", + "lat_long", + "formatted_name", + "region", + "search_name", + ]; + + optionalStringFields.forEach((field) => { + if (field in site) { + // Only check if the field is provided + if (typeof site[field] !== "string" || site[field].trim() === "") { + logText(`${field} must be a non-empty string at index ${index}`); + siteErrors.push(`${field} must be a non-empty string`); + } + } + }); + + // Validate isFeatured field + if ("isFeatured" in site) { + // Check only if provided + if (typeof site.isFeatured !== "boolean") { + logText(`isFeatured must be a boolean at index ${index}`); + siteErrors.push(`isFeatured must be a boolean`); + } + } + + return siteErrors; // Return collected errors for this site + }; + + // Check if selectedSites is an array or a single object + if (Array.isArray(selectedSites)) { + selectedSites.forEach((site, index) => { + const siteErrors = validateSite(site, index); + if (siteErrors.length > 0) { + errors[`selected_sites[${index}]`] = siteErrors; + } + }); + } else if (typeof selectedSites === "object" && selectedSites !== null) { + const siteErrors = validateSite(selectedSites, 0); // Treat as single object with index 0 if (siteErrors.length > 0) { - errors[`selected_sites[${index}]`] = siteErrors; + errors[`selected_sites[0]`] = siteErrors; // Use the same key format for consistency } - }); + } else { + logText( + "Request body should contain either an array of sites or a single site object" + ); + return res.status(400).json({ + success: false, + message: + "Request body should contain either an array of sites or a single site object", + }); + } // If any errors were collected, respond with them if (Object.keys(errors).length > 0) { diff --git a/src/auth-service/routes/v2/preferences.js b/src/auth-service/routes/v2/preferences.js index f793d5d882..3188b9a32b 100644 --- a/src/auth-service/routes/v2/preferences.js +++ b/src/auth-service/routes/v2/preferences.js @@ -1209,6 +1209,7 @@ router.put( .customSanitizer((value) => { return ObjectId(value); }), + validateSelectedSites([], false), setJWTAuth, authJWT, createPreferenceController.updateSelectedSite From cb49581969b8c47737682ccade51bfffd8e1fcca Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 13:57:13 +0300 Subject: [PATCH 13/21] increasing the maintanability of input validation for preferences --- .../middleware/validatePagination.js | 26 + .../middleware/validatePreferences.js | 227 ++++ .../middleware/validateSelectedSites.js | 4 +- src/auth-service/middleware/validateTenant.js | 15 + src/auth-service/models/Preference.js | 1 + src/auth-service/routes/v2/preferences.js | 1130 +---------------- 6 files changed, 320 insertions(+), 1083 deletions(-) create mode 100644 src/auth-service/middleware/validatePagination.js create mode 100644 src/auth-service/middleware/validatePreferences.js create mode 100644 src/auth-service/middleware/validateTenant.js diff --git a/src/auth-service/middleware/validatePagination.js b/src/auth-service/middleware/validatePagination.js new file mode 100644 index 0000000000..3ad0b7fcd9 --- /dev/null +++ b/src/auth-service/middleware/validatePagination.js @@ -0,0 +1,26 @@ +const validatePagination = (defaultLimit = 100, maxLimit = 1000) => { + return (req, res, next) => { + let limit = parseInt(req.query.limit || req.body.limit, 10); + const skip = parseInt(req.query.skip || req.body.skip, 10) || 0; + + // Set default limit if not provided or invalid + if (isNaN(limit) || limit < 1) { + limit = defaultLimit; + } + + // Cap the limit at maxLimit + if (limit > maxLimit) { + limit = maxLimit; + } + + // Set the validated limit and skip values in the request object + req.pagination = { + limit, + skip, + }; + + next(); + }; +}; + +module.exports = validatePagination; diff --git a/src/auth-service/middleware/validatePreferences.js b/src/auth-service/middleware/validatePreferences.js new file mode 100644 index 0000000000..be9be61927 --- /dev/null +++ b/src/auth-service/middleware/validatePreferences.js @@ -0,0 +1,227 @@ +const { body, oneOf, query } = require("express-validator"); +const { ObjectId } = require("mongoose").Types; +const { isMongoId } = require("validator"); +const { logText } = require("@utils/log"); +const isEmpty = require("is-empty"); + +const validateRequestBody = () => { + return oneOf([ + [ + body("user_id") + .exists() + .withMessage("the user_id should be provided in the request body") + .bail() + .notEmpty() + .withMessage("the provided user_id should not be empty") + .bail() + .trim() + .isMongoId() + .withMessage("the user_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), + body("pollutant") + .optional() + .notEmpty() + .withMessage("the provided pollutant should not be empty IF provided") + .bail() + .trim() + .isIn(["no2", "pm2_5", "pm10", "pm1"]) + .withMessage( + "the pollutant value is not among the expected ones which include: no2, pm2_5, pm10, pm1" + ), + body("frequency") + .optional() + .notEmpty() + .withMessage("the provided frequency should not be empty IF provided") + .bail() + .trim() + .toLowerCase() + .isIn(["daily", "hourly", "monthly"]) + .withMessage( + "the frequency value is not among the expected ones which include: daily, hourly and monthly" + ), + body("chartType") + .optional() + .notEmpty() + .withMessage("the provided chartType should not be empty IF provided") + .bail() + .trim() + .toLowerCase() + .isIn(["bar", "line", "pie"]) + .withMessage( + "the chartType value is not among the expected ones which include: bar, line and pie" + ), + body("startDate") + .optional() + .notEmpty() + .withMessage("the provided startDate should not be empty IF provided") + .bail() + .trim() + .isISO8601({ strict: true, strictSeparator: true }) + .withMessage("startDate must be a valid datetime."), + body("endDate") + .optional() + .notEmpty() + .withMessage("the provided endDate should not be empty IF provided") + .bail() + .trim() + .isISO8601({ strict: true, strictSeparator: true }) + .withMessage("endDate must be a valid datetime."), + body("airqloud_id") + .optional() + .notEmpty() + .withMessage("the provided airqloud_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the airqloud_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), + body("cohort_id") + .optional() + .notEmpty() + .withMessage("the provided cohort_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the cohort_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), + body("grid_id") + .optional() + .notEmpty() + .withMessage("the provided grid_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the grid_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), + body("chartTitle") + .optional() + .notEmpty() + .withMessage("the provided chartTitle should not be empty IF provided") + .bail() + .trim(), + body("period") + .optional() + .notEmpty() + .withMessage("the provided period should not be empty IF provided") + .bail() + .custom((value) => typeof value === "object") + .withMessage("the period should be an object"), + body("chartSubTitle") + .optional() + .notEmpty() + .withMessage( + "the provided chartSubTitle should not be empty IF provided" + ) + .bail() + .trim(), + body("site_ids") + .optional() + .notEmpty() + .withMessage("the provided site_ids should not be empty IF provided") + .bail() + .custom((value) => Array.isArray(value)) + .withMessage("the site_ids should be an array"), + body("site_ids.*") + .optional() + .notEmpty() + .withMessage("the provided site_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("site_id must be an object ID"), + body("device_ids") + .optional() + .notEmpty() + .withMessage("the provided device_ids should not be empty IF provided") + .bail() + .custom((value) => Array.isArray(value)) + .withMessage("the device_ids should be an array"), + body("device_ids.*") + .optional() + .notEmpty() + .withMessage("the provided device_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("device_id must be an object ID"), + query("id") + .optional() + .notEmpty() + .withMessage("the provided id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + query("user_id") + .optional() + .notEmpty() + .withMessage("the provided user_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the user_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + query("airqloud_id") + .optional() + .notEmpty() + .withMessage("the provided airqloud_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the airqloud_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + query("cohort_id") + .optional() + .notEmpty() + .withMessage("the provided cohort_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the cohort_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + query("grid_id") + .optional() + .notEmpty() + .withMessage("the provided grid_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the grid_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + query("site_id") + .optional() + .notEmpty() + .withMessage("the provided site_id should not be empty IF provided") + .bail() + .trim() + .isMongoId() + .withMessage("the site_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + ], + ]); +}; + +module.exports = validateRequestBody; diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js index c80991b9c2..b781ff25e2 100644 --- a/src/auth-service/middleware/validateSelectedSites.js +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -160,12 +160,12 @@ const validateSelectedSites = (requiredFields, allowId = false) => { } } else { logText( - "Request body should contain either an array of sites or a single site object" + "Request body(field) for Selected Sites should contain either an array of Site objects or a single site object" ); return res.status(400).json({ success: false, message: - "Request body should contain either an array of sites or a single site object", + "Request body(field) for Selected Sites should contain either an array of Site objects or a single site object", }); } diff --git a/src/auth-service/middleware/validateTenant.js b/src/auth-service/middleware/validateTenant.js new file mode 100644 index 0000000000..9637df84b0 --- /dev/null +++ b/src/auth-service/middleware/validateTenant.js @@ -0,0 +1,15 @@ +const { query } = require("express-validator"); + +const validateTenant = () => { + return query("tenant") + .optional() + .notEmpty() + .withMessage("tenant should not be empty if provided") + .trim() + .toLowerCase() + .bail() + .isIn(["kcca", "airqo"]) + .withMessage("the tenant value is not among the expected ones"); +}; + +module.exports = validateTenant; diff --git a/src/auth-service/models/Preference.js b/src/auth-service/models/Preference.js index 03ec254cd0..65eb5bca30 100644 --- a/src/auth-service/models/Preference.js +++ b/src/auth-service/models/Preference.js @@ -45,6 +45,7 @@ const siteSchema = new mongoose.Schema( sub_county: { type: String }, grid_id: { type: ObjectId }, createdAt: { type: Date }, + isFeatured: { type: Boolean, default: false }, }, { _id: false } ); diff --git a/src/auth-service/routes/v2/preferences.js b/src/auth-service/routes/v2/preferences.js index 3188b9a32b..94fd74c4b4 100644 --- a/src/auth-service/routes/v2/preferences.js +++ b/src/auth-service/routes/v2/preferences.js @@ -11,14 +11,9 @@ const { logText, logObject } = require("@utils/log"); const { isMongoId } = require("validator"); // const stringify = require("@utils/stringify"); const validateSelectedSites = require("@middleware/validateSelectedSites"); - -const validatePagination = (req, res, next) => { - const limit = parseInt(req.query.limit, 10); - const skip = parseInt(req.query.skip, 10); - req.query.limit = Number.isNaN(limit) || limit < 1 ? 100 : limit; - req.query.skip = Number.isNaN(skip) || skip < 0 ? 0 : skip; - next(); -}; +const validatePreferences = require("@middleware/validatePreferences"); +const validateTenant = require("@middleware/validateTenant"); +const validatePagination = require("@middleware/validatePagination"); const headers = (req, res, next) => { res.header("Access-Control-Allow-Origin", "*"); @@ -30,103 +25,6 @@ const headers = (req, res, next) => { next(); }; -function createValidateSelectedSitesField(requiredFields, allowId = false) { - return function (value) { - if (!value) { - throw new Error("Value must not be null or undefined"); - } - // Edge case: Check if _id field is present - if (!allowId && "_id" in value) { - throw new Error("_id field is not allowed"); - } - - if (!requiredFields.every((field) => field in value)) { - throw new Error( - `Missing required fields: ${requiredFields - .filter((field) => !(field in value)) - .join(", ")}` - ); - } - - 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) { - const numValue = parseFloat(value[field]); - if (Number.isNaN(numValue)) { - throw new Error(`${field} must be a valid number`); - } else if ( - field === "latitude" || - field === "longitude" || - field === "approximate_latitude" || - field === "approximate_longitude" - ) { - if (Math.abs(numValue) > 90) { - throw new Error(`${field} must be between -90 and 90`); - } - } else if (field === "search_radius") { - if (numValue <= 0) { - throw new Error(`${field} must be greater than 0`); - } - } - } - } - return true; - } - - function validateStringFields(fields) { - for (const field of fields) { - if ( - field in value && - (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; - } - - function validateTags(tags) { - if (isEmpty(tags)) { - return true; - } else if (!Array.isArray(tags)) { - throw new Error("site_tags must be an array"); - } else { - tags.forEach((tag, index) => { - if (typeof tag !== "string") { - throw new Error(`site_tags[${index}] must be a string`); - } - }); - } - } - - const numericValid = validateNumericFields([ - "latitude", - "longitude", - "approximate_latitude", - "approximate_longitude", - ]); - - const stringValid = validateStringFields(["name", "search_name"]); - const tags = value && value.site_tags; - const tagValid = validateTags(tags); - - return numericValid && stringValid && tagValid; - }; -} - const validateUniqueFieldsInSelectedSites = (req, res, next) => { const selectedSites = req.body.selected_sites; @@ -199,989 +97,83 @@ const validateUniqueFieldsInSelectedSites = (req, res, next) => { next(); }; - router.use(headers); -router.use(validatePagination); +router.use(validatePagination(100, 1000)); router.post( "/upsert", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - body("user_id") - .exists() - .withMessage("the user_id should be provided in the request body") - .bail() - .notEmpty() - .withMessage("the provided user_id should not be empty") - .bail() - .trim() - .isMongoId() - .withMessage("the user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("pollutant") - .optional() - .notEmpty() - .withMessage("the provided pollutant should not be empty IF provided") - .bail() - .trim() - .isIn(["no2", "pm2_5", "pm10", "pm1"]) - .withMessage( - "the pollutant value is not among the expected ones which include: no2, pm2_5, pm10, pm1" - ), - body("frequency") - .optional() - .notEmpty() - .withMessage("the provided frequency should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["daily", "hourly", "monthly"]) - .withMessage( - "the frequency value is not among the expected ones which include: daily, hourly and monthly" - ), - body("chartType") - .optional() - .notEmpty() - .withMessage("the provided chartType should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["bar", "line", "pie"]) - .withMessage( - "the chartType value is not among the expected ones which include: bar, line and pie" - ), - body("startDate") - .optional() - .notEmpty() - .withMessage("the provided startDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("startDate must be a valid datetime."), - body("endDate") - .optional() - .notEmpty() - .withMessage("the provided endDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("endDate must be a valid datetime."), - - body("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("cohort_id") - .optional() - .notEmpty() - .withMessage("the provided cohort_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the cohort_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("grid_id") - .optional() - .notEmpty() - .withMessage("the provided grid_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the grid_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("period") - .optional() - .notEmpty() - .withMessage("the provided period should not be empty IF provided") - .bail() - .custom((value) => { - return typeof value === "object"; - }) - .withMessage("the period should be an object"), - body("chartSubTitle") - .optional() - .notEmpty() - .withMessage( - "the provided chartSubTitle should not be empty IF provided" - ) - .bail() - .trim(), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("site_ids") - .optional() - .notEmpty() - .withMessage("the provided site_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the site_ids should be an array"), - body("site_ids.*") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("site_id must be an object ID"), - body("device_ids") - .optional() - .notEmpty() - .withMessage("the provided device_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the device_ids should be an array"), - body("device_ids.*") - .optional() - .notEmpty() - .withMessage("the provided device_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("device_id must be an object ID"), - body("selected_sites") - .optional() - .notEmpty() - .withMessage("the selected_sites should not be empty IF provided") - .bail() - .custom((value) => { - 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." - // ), - ], - ]), + validateTenant(), + validatePreferences(), + validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.upsert ); router.patch( "/replace", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - body("user_id") - .exists() - .withMessage("the user_id should be provided in the request body") - .bail() - .notEmpty() - .withMessage("the provided user_id should not be empty") - .bail() - .trim() - .isMongoId() - .withMessage("the user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("pollutant") - .optional() - .notEmpty() - .withMessage("the provided pollutant should not be empty IF provided") - .bail() - .trim() - .isIn(["no2", "pm2_5", "pm10", "pm1"]) - .withMessage( - "the pollutant value is not among the expected ones which include: no2, pm2_5, pm10, pm1" - ), - body("frequency") - .optional() - .notEmpty() - .withMessage("the provided frequency should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["daily", "hourly", "monthly"]) - .withMessage( - "the frequency value is not among the expected ones which include: daily, hourly and monthly" - ), - body("chartType") - .optional() - .notEmpty() - .withMessage("the provided chartType should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["bar", "line", "pie"]) - .withMessage( - "the chartType value is not among the expected ones which include: bar, line and pie" - ), - body("startDate") - .optional() - .notEmpty() - .withMessage("the provided startDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("startDate must be a valid datetime."), - body("endDate") - .optional() - .notEmpty() - .withMessage("the provided endDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("endDate must be a valid datetime."), - - body("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("cohort_id") - .optional() - .notEmpty() - .withMessage("the provided cohort_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the cohort_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("grid_id") - .optional() - .notEmpty() - .withMessage("the provided grid_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the grid_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("period") - .optional() - .notEmpty() - .withMessage("the provided period should not be empty IF provided") - .bail() - .custom((value) => { - return typeof value === "object"; - }) - .withMessage("the period should be an object"), - body("chartSubTitle") - .optional() - .notEmpty() - .withMessage( - "the provided chartSubTitle should not be empty IF provided" - ) - .bail() - .trim(), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("site_ids") - .optional() - .notEmpty() - .withMessage("the provided site_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the site_ids should be an array"), - body("site_ids.*") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("site_id must be an object ID"), - body("device_ids") - .optional() - .notEmpty() - .withMessage("the provided device_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the device_ids should be an array"), - body("device_ids.*") - .optional() - .notEmpty() - .withMessage("the provided device_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("device_id must be an object ID"), - body("selected_sites") - .optional() - .notEmpty() - .withMessage("the selected_sites should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the selected_sites should be an array"), - // body("selected_sites.*") - // .optional() - // .custom( - // createValidateSelectedSitesField(["_id", "search_name", "name"], true) - // ), - ], - ]), + validateTenant(), + validatePreferences(), + validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.replace ); router.put( "/:user_id", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - param("user_id") - .exists() - .withMessage( - "the record's identifier is missing in request, consider using the user_id" - ) - .bail() - .trim() - .isMongoId() - .withMessage("user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ]), - oneOf([ - [ - body("pollutant") - .optional() - .notEmpty() - .withMessage("the provided pollutant should not be empty IF provided") - .bail() - .trim() - .isIn(["no2", "pm2_5", "pm10", "pm1"]) - .withMessage( - "the pollutant value is not among the expected ones which include: no2, pm2_5, pm10, pm1" - ), - body("frequency") - .optional() - .notEmpty() - .withMessage("the provided frequency should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["daily", "hourly", "monthly"]) - .withMessage( - "the frequency value is not among the expected ones which include: daily, hourly and monthly" - ), - body("chartType") - .optional() - .notEmpty() - .withMessage("the provided chartType should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["bar", "line", "pie"]) - .withMessage( - "the chartType value is not among the expected ones which include: bar, line and pie" - ), - body("startDate") - .optional() - .notEmpty() - .withMessage("the provided startDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("startDate must be a valid datetime."), - body("endDate") - .optional() - .notEmpty() - .withMessage("the provided endDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("endDate must be a valid datetime."), - - body("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("cohort_id") - .optional() - .notEmpty() - .withMessage("the provided cohort_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the cohort_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("grid_id") - .optional() - .notEmpty() - .withMessage("the provided grid_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the grid_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("period") - .optional() - .notEmpty() - .withMessage("the provided period should not be empty IF provided") - .bail() - .custom((value) => { - return typeof value === "object"; - }) - .withMessage("the period should be an object"), - body("chartSubTitle") - .optional() - .notEmpty() - .withMessage( - "the provided chartSubTitle should not be empty IF provided" - ) - .bail() - .trim(), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .bail() - .trim(), - body("site_ids") - .optional() - .notEmpty() - .withMessage("the provided site_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the site_ids should be an array"), - body("site_ids.*") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("site_id must be an object ID"), - body("device_ids") - .optional() - .notEmpty() - .withMessage("the provided device_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the device_ids should be an array"), - body("device_ids.*") - .optional() - .notEmpty() - .withMessage("the provided device_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("device_id must be an object ID"), - body("selected_sites") - .optional() - .notEmpty() - .withMessage("the selected_sites should not be empty IF provided") - .bail() - .custom((value) => { - 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." - // ), - ], - ]), + validateTenant(), + param("user_id") + .exists() + .withMessage( + "the record's identifier is missing in request, consider using the user_id" + ) + .bail() + .trim() + .isMongoId() + .withMessage("user_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), + validatePreferences(), + validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.update ); router.post( "/", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - body("pollutant") - .optional() - .notEmpty() - .withMessage("the provided pollutant should not be empty IF provided") - .bail() - .trim() - .isIn(["no2", "pm2_5", "pm10", "pm1"]) - .withMessage( - "the pollutant value is not among the expected ones which include: no2, pm2_5, pm10, pm1" - ), - body("frequency") - .optional() - .notEmpty() - .withMessage("the provided frequently should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["daily", "hourly", "monthly"]) - .withMessage( - "the frequency value is not among the expected ones which include: daily, hourly and monthly" - ), - body("chartType") - .optional() - .notEmpty() - .withMessage("the provided chartType should not be empty IF provided") - .bail() - .trim() - .toLowerCase() - .isIn(["bar", "line", "pie"]) - .withMessage( - "the chartType value is not among the expected ones which include: bar, line and pie" - ), - body("startDate") - .optional() - .notEmpty() - .withMessage("the provided startDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("startDate must be a valid datetime."), - body("endDate") - .optional() - .notEmpty() - .withMessage("the provided endDate should not be empty IF provided") - .bail() - .trim() - .toDate() - .isISO8601({ strict: true, strictSeparator: true }) - .withMessage("endDate must be a valid datetime."), - body("user_id") - .exists() - .withMessage("the user_id should be provided in the request body") - .bail() - .notEmpty() - .withMessage("the provided user_id should not be empty") - .bail() - .trim() - .isMongoId() - .withMessage("the user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("chartTitle") - .optional() - .notEmpty() - .withMessage("the provided chartTitle should not be empty IF provided") - .trim(), - body("period") - .optional() - .notEmpty() - .withMessage("the provided period should not be empty IF provided") - .bail() - .custom((value) => { - return typeof value === "object"; - }) - .bail() - .withMessage("the period should be an object"), - body("chartSubTitle") - .optional() - .notEmpty() - .withMessage( - "the provided chartSubTitle should not be empty IF provided" - ) - .trim(), - body("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - body("site_ids") - .optional() - .notEmpty() - .withMessage("the provided site_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the site_ids should be an array"), - body("site_ids.*") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("site_id must be an object ID"), - body("device_ids") - .optional() - .notEmpty() - .withMessage("the provided device_ids should not be empty IF provided") - .bail() - .custom((value) => { - return Array.isArray(value); - }) - .withMessage("the device_ids should be an array"), - body("device_ids.*") - .optional() - .notEmpty() - .withMessage("the provided device_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("device_id must be an object ID"), - body("selected_sites") - .optional() - .notEmpty() - .withMessage("the selected_sites should not be empty IF provided") - .bail() - .custom((value) => { - 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." - // ), - ], - ]), + validateTenant(), + validatePreferences(), + validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.create ); router.get( "/", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - query("id") - .optional() - .notEmpty() - .withMessage("the provided id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("user_id") - .optional() - .notEmpty() - .withMessage("the provided user_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("cohort_id") - .optional() - .notEmpty() - .withMessage("the provided cohort_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the cohort_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("grid_id") - .optional() - .notEmpty() - .withMessage("the provided grid_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the grid_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("site_id") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the site_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ], - ]), + validateTenant(), + validatePreferences(), createPreferenceController.list ); router.delete( "/:user_id", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - param("user_id") - .exists() - .withMessage("the the user_id is missing in request") - .bail() - .trim() - .isMongoId() - .withMessage("user_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ]), + validateTenant(), + param("user_id") + .exists() + .withMessage("the the user_id is missing in request") + .bail() + .trim() + .isMongoId() + .withMessage("user_id must be an object ID") + .bail() + .customSanitizer((value) => { + return ObjectId(value); + }), setJWTAuth, authJWT, createPreferenceController.delete ); router.get( "/selected-sites", - oneOf([ - [ - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), - ], - ]), - oneOf([ - [ - query("airqloud_id") - .optional() - .notEmpty() - .withMessage("the provided airqloud_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the airqloud_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("cohort_id") - .optional() - .notEmpty() - .withMessage("the provided cohort_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the cohort_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("grid_id") - .optional() - .notEmpty() - .withMessage("the provided grid_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the grid_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - query("site_id") - .optional() - .notEmpty() - .withMessage("the provided site_id should not be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("the site_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), - ], - ]), + validateTenant(), + validatePreferences(), createPreferenceController.listSelectedSites ); router.post( "/selected-sites", - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), + validateTenant(), validateUniqueFieldsInSelectedSites, validateSelectedSites(["site_id", "search_name", "name"], false), setJWTAuth, @@ -1190,15 +182,7 @@ router.post( ); router.put( "/selected-sites/:site_id", - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), + validateTenant(), param("site_id") .exists() .withMessage("the site_id parameter is required") @@ -1216,15 +200,7 @@ router.put( ); router.delete( "/selected-sites/:site_id", - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), + validateTenant(), param("site_id") .exists() .withMessage("the site_id parameter is required") @@ -1241,15 +217,7 @@ router.delete( ); router.get( "/:user_id", - query("tenant") - .optional() - .notEmpty() - .withMessage("tenant should not be empty if provided") - .trim() - .toLowerCase() - .bail() - .isIn(["kcca", "airqo"]) - .withMessage("the tenant value is not among the expected ones"), + validateTenant(), param("user_id") .exists() .withMessage("the user ID param is missing in the request") From f07953dd0e77ab56393db2d9bf26aebb0a7fe00c Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:18:14 +0300 Subject: [PATCH 14/21] more enhancements to improve maintainability of preferences --- .../test/ut_validateSelectedSites.js | 203 +++++++++++------- .../middleware/validatePreferences.js | 12 +- .../middleware/validateSelectedSites.js | 100 ++++++--- src/auth-service/routes/v2/preferences.js | 110 ++++------ 4 files changed, 242 insertions(+), 183 deletions(-) diff --git a/src/auth-service/middleware/test/ut_validateSelectedSites.js b/src/auth-service/middleware/test/ut_validateSelectedSites.js index 4342e9ad12..fabc3945d5 100644 --- a/src/auth-service/middleware/test/ut_validateSelectedSites.js +++ b/src/auth-service/middleware/test/ut_validateSelectedSites.js @@ -2,33 +2,30 @@ require("module-alias/register"); const { expect } = require("chai"); const sinon = require("sinon"); const validateSelectedSites = require("@middleware/validateSelectedSites"); -const { logText } = require("@utils/log"); +const { logText, logObject } = require("@utils/log"); describe("validateSelectedSites Middleware", () => { let req, res, next; beforeEach(() => { req = { - body: { - selected_sites: [], - }, + body: {}, }; res = { status: sinon.stub().returnsThis(), json: sinon.stub(), }; next = sinon.stub(); - sinon.stub(logText); // Mock logText function + sinon.stub(logText); + sinon.stub(logObject); }); afterEach(() => { - sinon.restore(); // Restore the original functionality of stubbed methods + sinon.restore(); }); - describe("when selected_sites is not an array", () => { - it("should return a 400 error if selected_sites is not an array", () => { - req.body.selected_sites = "not an array"; - + describe("when selected_sites is not defined", () => { + it("should call next() if selected_sites is undefined", () => { const middleware = validateSelectedSites([ "site_id", "name", @@ -36,20 +33,13 @@ describe("validateSelectedSites Middleware", () => { ]); middleware(req, res, next); - expect(res.status.calledWith(400)).to.be.true; - expect( - res.json.calledWith({ - success: false, - message: "selected_sites should be a non-empty array", - }) - ).to.be.true; - expect(next.notCalled).to.be.true; + expect(next.calledOnce).to.be.true; }); }); - describe("when selected_sites is an empty array", () => { - it("should return a 400 error if selected_sites is an empty array", () => { - req.body.selected_sites = []; + describe("when selected_sites is not an array or object", () => { + it("should return a 400 error if selected_sites is neither an array nor an object", () => { + req.body.selected_sites = "not an array or object"; const middleware = validateSelectedSites([ "site_id", @@ -62,7 +52,8 @@ describe("validateSelectedSites Middleware", () => { expect( res.json.calledWith({ success: false, - message: "selected_sites should be a non-empty array", + message: + "Request body(field) for Selected Sites should contain either an array of Site objects or be omitted.", }) ).to.be.true; expect(next.notCalled).to.be.true; @@ -96,7 +87,7 @@ describe("validateSelectedSites Middleware", () => { describe("when required fields are missing", () => { it("should return a 400 error for missing required fields", () => { - req.body.selected_sites = [{}]; // Site object with no fields + req.body.selected_sites = [{}]; const middleware = validateSelectedSites([ "site_id", @@ -125,7 +116,7 @@ describe("validateSelectedSites Middleware", () => { describe("when fields are of invalid types", () => { it("should return a 400 error for invalid site_id format", () => { - req.body.selected_sites = [{ site_id: "invalid_id" }]; + req.body.selected_sites = [{ site_id: 123 }]; const middleware = validateSelectedSites(["site_id"]); middleware(req, res, next); @@ -135,26 +126,7 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": ["site_id must be a valid MongoDB ObjectId"], - }, - message: "bad request errors", - }) - ).to.be.true; - expect(next.notCalled).to.be.true; - }); - - it("should return a 400 error for non-string name field", () => { - req.body.selected_sites = [{ name: 123 }]; - - const middleware = validateSelectedSites(["name"]); - middleware(req, res, next); - - expect(res.status.calledWith(400)).to.be.true; - expect( - res.json.calledWith({ - success: false, - errors: { - "selected_sites[0]": ["name must be a non-empty string"], + "selected_sites[0]": ["site_id must be a non-empty string"], }, message: "bad request errors", }) @@ -163,7 +135,7 @@ describe("validateSelectedSites Middleware", () => { }); it("should return a 400 error for invalid latitude value", () => { - req.body.selected_sites = [{ latitude: 100 }]; // Invalid latitude + req.body.selected_sites = [{ latitude: "invalid" }]; const middleware = validateSelectedSites(["latitude"]); middleware(req, res, next); @@ -181,7 +153,7 @@ describe("validateSelectedSites Middleware", () => { }); it("should return a 400 error for invalid longitude value", () => { - req.body.selected_sites = [{ longitude: -200 }]; // Invalid longitude + req.body.selected_sites = [{ longitude: "invalid" }]; const middleware = validateSelectedSites(["longitude"]); middleware(req, res, next); @@ -198,10 +170,10 @@ describe("validateSelectedSites Middleware", () => { ).to.be.true; }); - it("should return a 400 error for invalid approximate_latitude value", () => { - req.body.selected_sites = [{ approximate_latitude: 100 }]; // Invalid approximate_latitude + it("should return a 400 error for non-array site_tags", () => { + req.body.selected_sites = [{ site_tags: "not_an_array" }]; - const middleware = validateSelectedSites(["approximate_latitude"]); + const middleware = validateSelectedSites(["site_tags"]); middleware(req, res, next); expect(res.status.calledWith(400)).to.be.true; @@ -209,19 +181,17 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": [ - "approximate_latitude must be between -90 and 90", - ], + "selected_sites[0]": ["site_tags must be an array"], }, message: "bad request errors", }) ).to.be.true; }); - it("should return a 400 error for invalid approximate_longitude value", () => { - req.body.selected_sites = [{ approximate_longitude: -200 }]; // Invalid approximate_longitude + it("should return a 400 error for invalid isFeatured value", () => { + req.body.selected_sites = [{ isFeatured: "not_a_boolean" }]; - const middleware = validateSelectedSites(["approximate_longitude"]); + const middleware = validateSelectedSites(["isFeatured"]); middleware(req, res, next); expect(res.status.calledWith(400)).to.be.true; @@ -229,19 +199,26 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": [ - "approximate_longitude must be between -180 and 180", - ], + "selected_sites[0]": ["isFeatured must be a boolean"], }, message: "bad request errors", }) ).to.be.true; }); + }); - it("should return a 400 error for non-array site_tags", () => { - req.body.selected_sites = [{ site_tags: "not_an_array" }]; // Invalid site_tags + describe("when duplicate values are found", () => { + it("should return a 400 error for duplicate site_id", () => { + req.body.selected_sites = [ + { site_id: "site1", name: "Site 1", search_name: "Search 1" }, + { site_id: "site1", name: "Site 2", search_name: "Search 2" }, + ]; - const middleware = validateSelectedSites(["site_tags"]); + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); middleware(req, res, next); expect(res.status.calledWith(400)).to.be.true; @@ -249,17 +226,24 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": ["site_tags must be an array"], + "selected_sites[1]": ["Duplicate site_id: site1"], }, message: "bad request errors", }) ).to.be.true; }); - it("should return a 400 error for non-string tags in site_tags", () => { - req.body.selected_sites = [{ site_tags: [123] }]; // Invalid tag + it("should return a 400 error for duplicate search_name", () => { + req.body.selected_sites = [ + { site_id: "site1", name: "Site 1", search_name: "Search" }, + { site_id: "site2", name: "Site 2", search_name: "Search" }, + ]; - const middleware = validateSelectedSites(["site_tags"]); + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); middleware(req, res, next); expect(res.status.calledWith(400)).to.be.true; @@ -267,17 +251,24 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": ["site_tags[0] must be a string"], + "selected_sites[1]": ["Duplicate search_name: Search"], }, message: "bad request errors", }) ).to.be.true; }); - it("should return a 400 error for invalid isFeatured value", () => { - req.body.selected_sites = [{ isFeatured: "not_a_boolean" }]; // Invalid isFeatured + it("should return a 400 error for duplicate name", () => { + req.body.selected_sites = [ + { site_id: "site1", name: "Site", search_name: "Search 1" }, + { site_id: "site2", name: "Site", search_name: "Search 2" }, + ]; - const middleware = validateSelectedSites(["isFeatured"]); + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); middleware(req, res, next); expect(res.status.calledWith(400)).to.be.true; @@ -285,7 +276,7 @@ describe("validateSelectedSites Middleware", () => { res.json.calledWith({ success: false, errors: { - "selected_sites[0]": ["isFeatured must be a boolean"], + "selected_sites[1]": ["Duplicate name: Site"], }, message: "bad request errors", }) @@ -297,7 +288,7 @@ describe("validateSelectedSites Middleware", () => { it("should call next() when all validations are successful", () => { req.body.selected_sites = [ { - site_id: "610a43c1909756001e235e93", + site_id: "site1", name: "Valid Site", search_name: "Valid Search", latitude: -1.2345, @@ -306,6 +297,7 @@ describe("validateSelectedSites Middleware", () => { approximate_longitude: 34.5678, site_tags: ["tag1", "tag2"], country: "Country Name", + isFeatured: true, }, ]; @@ -314,10 +306,77 @@ describe("validateSelectedSites Middleware", () => { "name", "search_name", ]); + middleware(req, res, next); + + expect(next.calledOnce).to.be.true; + }); + }); + + describe("when allowId is true", () => { + it("should allow _id field and validate it as a MongoDB ObjectId", () => { + req.body.selected_sites = [ + { + _id: "507f1f77bcf86cd799439011", + site_id: "site1", + name: "Valid Site", + search_name: "Valid Search", + }, + ]; + + const middleware = validateSelectedSites( + ["site_id", "name", "search_name"], + true + ); + middleware(req, res, next); + + expect(next.calledOnce).to.be.true; + }); + + it("should return a 400 error for invalid _id format", () => { + req.body.selected_sites = [ + { + _id: "invalid_id", + site_id: "site1", + name: "Valid Site", + search_name: "Valid Search", + }, + ]; + + const middleware = validateSelectedSites( + ["site_id", "name", "search_name"], + true + ); + middleware(req, res, next); + + expect(res.status.calledWith(400)).to.be.true; + expect( + res.json.calledWith({ + success: false, + errors: { + "selected_sites[0]": ["_id must be a valid MongoDB ObjectId"], + }, + message: "bad request errors", + }) + ).to.be.true; + }); + }); + describe("when selected_sites is a single object", () => { + it("should validate a single object when selected_sites is not an array", () => { + req.body.selected_sites = { + site_id: "site1", + name: "Valid Site", + search_name: "Valid Search", + }; + + const middleware = validateSelectedSites([ + "site_id", + "name", + "search_name", + ]); middleware(req, res, next); - expect(next.calledOnce).to.be.true; // Ensure next() is called + expect(next.calledOnce).to.be.true; }); }); }); diff --git a/src/auth-service/middleware/validatePreferences.js b/src/auth-service/middleware/validatePreferences.js index be9be61927..ee219e2a2a 100644 --- a/src/auth-service/middleware/validatePreferences.js +++ b/src/auth-service/middleware/validatePreferences.js @@ -8,11 +8,9 @@ const validateRequestBody = () => { return oneOf([ [ body("user_id") - .exists() - .withMessage("the user_id should be provided in the request body") - .bail() + .optional() .notEmpty() - .withMessage("the provided user_id should not be empty") + .withMessage("the provided user_id should not be empty IF provided") .bail() .trim() .isMongoId() @@ -215,11 +213,7 @@ const validateRequestBody = () => { .bail() .trim() .isMongoId() - .withMessage("the site_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), + .withMessage("the site_id must be an object ID"), ], ]); }; diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js index b781ff25e2..943f2b9b66 100644 --- a/src/auth-service/middleware/validateSelectedSites.js +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -1,44 +1,56 @@ const { isMongoId } = require("validator"); -const { logText } = require("@utils/log"); +const { logText, logObject } = require("@utils/log"); const isEmpty = require("is-empty"); const validateSelectedSites = (requiredFields, allowId = false) => { return (req, res, next) => { - const selectedSites = req.body.selected_sites || req.body; // Check for wrapper field or direct body + const selectedSites = req.body.selected_sites || req.body; // Get selected_sites directly const errors = {}; // Object to hold error messages + if (allowId && !req.body.selected_sites) { + return next(); + } + // If selectedSites is not defined, we can skip validation + if (selectedSites === undefined) { + return next(); // Proceed to the next middleware or route handler + } + + // Early check for selectedSites type + if (!Array.isArray(selectedSites) && typeof selectedSites !== "object") { + return res.status(400).json({ + success: false, + message: + "Request body(field) for Selected Sites should contain either an array of Site objects or be omitted.", + }); + } + // Helper function to validate a single site const validateSite = (site, index) => { const siteErrors = []; // Array to hold errors for the current site if (!site) { - logText(`Site at index ${index} must not be null or undefined`); siteErrors.push("Site value must not be null or undefined"); } // Edge case: Check if _id field is present if (!allowId && "_id" in site) { - logText(`_id field is not allowed at index ${index}`); siteErrors.push("_id field is not allowed"); } // Validate required fields directly requiredFields.forEach((field) => { if (!(field in site)) { - logText(`Field "${field}" is missing at index ${index}`); siteErrors.push(`Field "${field}" is missing`); } }); // Validate _id if allowed if (allowId && site._id && !isMongoId(site._id)) { - logText(`_id must be a valid MongoDB ObjectId at index ${index}`); siteErrors.push("_id must be a valid MongoDB ObjectId"); } // Validate site_id if (site.site_id && typeof site.site_id !== "string") { - logText(`site_id must be a non-empty string at index ${index}`); siteErrors.push("site_id must be a non-empty string"); } @@ -46,7 +58,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { if (site.latitude !== undefined) { const latValue = parseFloat(site.latitude); if (Number.isNaN(latValue) || latValue < -90 || latValue > 90) { - logText(`latitude must be between -90 and 90 at index ${index}`); siteErrors.push("latitude must be between -90 and 90"); } } @@ -55,7 +66,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { if (site.longitude !== undefined) { const longValue = parseFloat(site.longitude); if (Number.isNaN(longValue) || longValue < -180 || longValue > 180) { - logText(`longitude must be between -180 and 180 at index ${index}`); siteErrors.push("longitude must be between -180 and 180"); } } @@ -68,9 +78,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { approxLatValue < -90 || approxLatValue > 90 ) { - logText( - `approximate_latitude must be between -90 and 90 at index ${index}` - ); siteErrors.push("approximate_latitude must be between -90 and 90"); } } @@ -83,9 +90,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { approxLongValue < -180 || approxLongValue > 180 ) { - logText( - `approximate_longitude must be between -180 and 180 at index ${index}` - ); siteErrors.push("approximate_longitude must be between -180 and 180"); } } @@ -94,15 +98,11 @@ const validateSelectedSites = (requiredFields, allowId = false) => { const tags = site.site_tags; if (!isEmpty(tags)) { if (!Array.isArray(tags)) { - logText("site_tags must be an array"); siteErrors.push("site_tags must be an array"); } tags.forEach((tag, tagIndex) => { if (typeof tag !== "string") { - logText( - `site_tags[${tagIndex}] must be a string at index ${index}` - ); siteErrors.push(`site_tags[${tagIndex}] must be a string`); } }); @@ -127,7 +127,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { if (field in site) { // Only check if the field is provided if (typeof site[field] !== "string" || site[field].trim() === "") { - logText(`${field} must be a non-empty string at index ${index}`); siteErrors.push(`${field} must be a non-empty string`); } } @@ -137,7 +136,6 @@ const validateSelectedSites = (requiredFields, allowId = false) => { if ("isFeatured" in site) { // Check only if provided if (typeof site.isFeatured !== "boolean") { - logText(`isFeatured must be a boolean at index ${index}`); siteErrors.push(`isFeatured must be a boolean`); } } @@ -145,27 +143,73 @@ const validateSelectedSites = (requiredFields, allowId = false) => { return siteErrors; // Return collected errors for this site }; - // Check if selectedSites is an array or a single object + // If selectedSites is defined as an array, validate each item. if (Array.isArray(selectedSites)) { selectedSites.forEach((site, index) => { const siteErrors = validateSite(site, index); if (siteErrors.length > 0) { - errors[`selected_sites[${index}]`] = siteErrors; + errors[`selected_sites[${index}]`] = + errors[`selected_sites[${index}]`] || []; + errors[`selected_sites[${index}]`].push(...siteErrors); + } + }); + + // Unique checks after validating each item + const uniqueSiteIds = new Set(); + const uniqueSearchNames = new Set(); + const uniqueNames = new Set(); + + selectedSites.forEach((item, idx) => { + // Check for duplicate site_id + if (item.site_id !== undefined) { + if (uniqueSiteIds.has(item.site_id)) { + errors[`selected_sites[${idx}]`] = + errors[`selected_sites[${idx}]`] || []; + errors[`selected_sites[${idx}]`].push( + `Duplicate site_id: ${item.site_id}` + ); + } else { + uniqueSiteIds.add(item.site_id); + } + } + + // Check for duplicate search_name + if (item.search_name !== undefined) { + if (uniqueSearchNames.has(item.search_name)) { + errors[`selected_sites[${idx}]`] = + errors[`selected_sites[${idx}]`] || []; + errors[`selected_sites[${idx}]`].push( + `Duplicate search_name: ${item.search_name}` + ); + } else { + uniqueSearchNames.add(item.search_name); + } + } + + // Check for duplicate name + if (item.name !== undefined) { + if (uniqueNames.has(item.name)) { + errors[`selected_sites[${idx}]`] = + errors[`selected_sites[${idx}]`] || []; + errors[`selected_sites[${idx}]`].push( + `Duplicate name: ${item.name}` + ); + } else { + uniqueNames.add(item.name); + } } }); } else if (typeof selectedSites === "object" && selectedSites !== null) { const siteErrors = validateSite(selectedSites, 0); // Treat as single object with index 0 if (siteErrors.length > 0) { - errors[`selected_sites[0]`] = siteErrors; // Use the same key format for consistency + errors[`selected_sites[0]`] = errors[`selected_sites[0]`] || []; + errors[`selected_sites[0]`].push(...siteErrors); } } else { - logText( - "Request body(field) for Selected Sites should contain either an array of Site objects or a single site object" - ); return res.status(400).json({ success: false, message: - "Request body(field) for Selected Sites should contain either an array of Site objects or a single site object", + "Request body(field) for Selected Sites should contain either an array of Site objects or a single Site object", }); } diff --git a/src/auth-service/routes/v2/preferences.js b/src/auth-service/routes/v2/preferences.js index 94fd74c4b4..febcbb2888 100644 --- a/src/auth-service/routes/v2/preferences.js +++ b/src/auth-service/routes/v2/preferences.js @@ -24,85 +24,24 @@ const headers = (req, res, next) => { res.header("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, PATCH"); next(); }; - -const validateUniqueFieldsInSelectedSites = (req, res, next) => { - const selectedSites = req.body.selected_sites; - - // Create Sets to track unique values for each field - const uniqueSiteIds = new Set(); - const uniqueSearchNames = new Set(); - const uniqueNames = new Set(); - - const duplicateSiteIds = []; - const duplicateSearchNames = []; - const duplicateNames = []; - - selectedSites.forEach((item) => { - // Check for duplicate site_id if it exists - if (item.site_id !== undefined) { - if (uniqueSiteIds.has(item.site_id)) { - duplicateSiteIds.push(item.site_id); - } else { - uniqueSiteIds.add(item.site_id); - } - } - - // Check for duplicate search_name if it exists - if (item.search_name !== undefined) { - if (uniqueSearchNames.has(item.search_name)) { - duplicateSearchNames.push(item.search_name); - } else { - uniqueSearchNames.add(item.search_name); - } - } - - // Check for duplicate name if it exists - if (item.name !== undefined) { - if (uniqueNames.has(item.name)) { - duplicateNames.push(item.name); - } else { - uniqueNames.add(item.name); - } - } - }); - - // Prepare error messages based on duplicates found - let errorMessage = ""; - if (duplicateSiteIds.length > 0) { - errorMessage += - "Duplicate site_ids found: " + - [...new Set(duplicateSiteIds)].join(", ") + - ". "; - } - if (duplicateSearchNames.length > 0) { - errorMessage += - "Duplicate search_names found: " + - [...new Set(duplicateSearchNames)].join(", ") + - ". "; - } - if (duplicateNames.length > 0) { - errorMessage += - "Duplicate names found: " + - [...new Set(duplicateNames)].join(", ") + - ". "; - } - - // If any duplicates were found, respond with an error - if (errorMessage) { - return res.status(400).json({ - success: false, - message: errorMessage.trim(), - }); - } - - next(); -}; router.use(headers); router.use(validatePagination(100, 1000)); router.post( "/upsert", validateTenant(), + body("user_id") + .exists() + .withMessage("the user_id should be provided in the request body") + .bail() + .notEmpty() + .withMessage("the provided user_id should not be empty") + .bail() + .trim() + .isMongoId() + .withMessage("the user_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), validatePreferences(), validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.upsert @@ -110,6 +49,18 @@ router.post( router.patch( "/replace", validateTenant(), + body("user_id") + .exists() + .withMessage("the user_id should be provided in the request body") + .bail() + .notEmpty() + .withMessage("the provided user_id should not be empty") + .bail() + .trim() + .isMongoId() + .withMessage("the user_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), validatePreferences(), validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.replace @@ -137,6 +88,18 @@ router.put( router.post( "/", validateTenant(), + body("user_id") + .exists() + .withMessage("the user_id should be provided in the request body") + .bail() + .notEmpty() + .withMessage("the provided user_id should not be empty") + .bail() + .trim() + .isMongoId() + .withMessage("the user_id must be an object ID") + .bail() + .customSanitizer((value) => ObjectId(value)), validatePreferences(), validateSelectedSites(["_id", "search_name", "name"], true), createPreferenceController.create @@ -174,7 +137,6 @@ router.get( router.post( "/selected-sites", validateTenant(), - validateUniqueFieldsInSelectedSites, validateSelectedSites(["site_id", "search_name", "name"], false), setJWTAuth, authJWT, From 9d163e243b0f328c1729cd2b64d6c3dba384e15f Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:45:45 +0300 Subject: [PATCH 15/21] This adjustment ensures that null values do not pass the validation check unintentionally. --- src/auth-service/middleware/validateSelectedSites.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js index 943f2b9b66..4e73d2f31e 100644 --- a/src/auth-service/middleware/validateSelectedSites.js +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -16,7 +16,10 @@ const validateSelectedSites = (requiredFields, allowId = false) => { } // Early check for selectedSites type - if (!Array.isArray(selectedSites) && typeof selectedSites !== "object") { + if ( + !Array.isArray(selectedSites) && + (typeof selectedSites !== "object" || selectedSites === null) + ) { return res.status(400).json({ success: false, message: From 9f8b0e58f11b6e4b7bffdc94efd3b3841e7e25a3 Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:47:39 +0300 Subject: [PATCH 16/21] This ensures that site_id is a string containing meaningful data. --- src/auth-service/middleware/validateSelectedSites.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth-service/middleware/validateSelectedSites.js b/src/auth-service/middleware/validateSelectedSites.js index 4e73d2f31e..ffdb67711e 100644 --- a/src/auth-service/middleware/validateSelectedSites.js +++ b/src/auth-service/middleware/validateSelectedSites.js @@ -53,7 +53,7 @@ const validateSelectedSites = (requiredFields, allowId = false) => { } // Validate site_id - if (site.site_id && typeof site.site_id !== "string") { + if (typeof site.site_id !== "string" || site.site_id.trim() === "") { siteErrors.push("site_id must be a non-empty string"); } From d086474879ebedd062d965369415ddc238298ec9 Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:48:29 +0300 Subject: [PATCH 17/21] Removing Unused Imports for Cleaner Code --- src/auth-service/middleware/validatePreferences.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/auth-service/middleware/validatePreferences.js b/src/auth-service/middleware/validatePreferences.js index ee219e2a2a..3f39142cf3 100644 --- a/src/auth-service/middleware/validatePreferences.js +++ b/src/auth-service/middleware/validatePreferences.js @@ -1,8 +1,5 @@ const { body, oneOf, query } = require("express-validator"); const { ObjectId } = require("mongoose").Types; -const { isMongoId } = require("validator"); -const { logText } = require("@utils/log"); -const isEmpty = require("is-empty"); const validateRequestBody = () => { return oneOf([ From 7362e4d44f9bc075d392aa55756dbdde9b524a19 Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:52:24 +0300 Subject: [PATCH 18/21] prevents potential issues arising from type coercion, making the code more robust and predictable --- src/auth-service/middleware/validatePagination.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth-service/middleware/validatePagination.js b/src/auth-service/middleware/validatePagination.js index 3ad0b7fcd9..d85c00a7a5 100644 --- a/src/auth-service/middleware/validatePagination.js +++ b/src/auth-service/middleware/validatePagination.js @@ -4,7 +4,7 @@ const validatePagination = (defaultLimit = 100, maxLimit = 1000) => { const skip = parseInt(req.query.skip || req.body.skip, 10) || 0; // Set default limit if not provided or invalid - if (isNaN(limit) || limit < 1) { + if (Number.isNaN(limit) || limit < 1) { limit = defaultLimit; } From 3681482d1d9c8b40648e9f29c58d76aa535a2f09 Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 16:53:44 +0300 Subject: [PATCH 19/21] make it easier to add or modify allowed tenants without changing the core validation logic --- src/auth-service/middleware/validateTenant.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/auth-service/middleware/validateTenant.js b/src/auth-service/middleware/validateTenant.js index 9637df84b0..0430807974 100644 --- a/src/auth-service/middleware/validateTenant.js +++ b/src/auth-service/middleware/validateTenant.js @@ -1,4 +1,5 @@ const { query } = require("express-validator"); +const ALLOWED_TENANTS = ["kcca", "airqo"]; const validateTenant = () => { return query("tenant") @@ -8,7 +9,7 @@ const validateTenant = () => { .trim() .toLowerCase() .bail() - .isIn(["kcca", "airqo"]) + .isIn(ALLOWED_TENANTS) .withMessage("the tenant value is not among the expected ones"); }; From d207eaeca584c0bbeedb724a135daf448d0cc532 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 20 Oct 2024 17:43:42 +0300 Subject: [PATCH 20/21] Update auth service staging image tag to stage-03191d9c-1729435343 --- k8s/auth-service/values-stage.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/k8s/auth-service/values-stage.yaml b/k8s/auth-service/values-stage.yaml index 062c71b6ab..0e9193e3f7 100644 --- a/k8s/auth-service/values-stage.yaml +++ b/k8s/auth-service/values-stage.yaml @@ -6,7 +6,7 @@ app: replicaCount: 2 image: repository: eu.gcr.io/airqo-250220/airqo-stage-auth-api - tag: stage-6fb34054-1729313663 + tag: stage-03191d9c-1729435343 nameOverride: '' fullnameOverride: '' podAnnotations: {} From c8fa4e6ad8f12531e22856688c912617acf8038a Mon Sep 17 00:00:00 2001 From: baalmart Date: Sun, 20 Oct 2024 18:50:23 +0300 Subject: [PATCH 21/21] Refactor and enhance validateOptionalObjectId middleware with comprehensive unit tests --- src/device-registry/bin/server.js | 12 ++- .../test/ut_validateOptionalObjectId.js | 100 ++++++++++++++++++ .../middleware/validateOptionalObjectId.js | 32 ++++++ src/device-registry/routes/v2/readings.js | 83 +-------------- src/device-registry/utils/errors.js | 10 ++ 5 files changed, 155 insertions(+), 82 deletions(-) create mode 100644 src/device-registry/middleware/test/ut_validateOptionalObjectId.js create mode 100644 src/device-registry/middleware/validateOptionalObjectId.js diff --git a/src/device-registry/bin/server.js b/src/device-registry/bin/server.js index 1c31b52a66..cae0c11909 100644 --- a/src/device-registry/bin/server.js +++ b/src/device-registry/bin/server.js @@ -13,7 +13,7 @@ connectToMongoDB(); const morgan = require("morgan"); const compression = require("compression"); const helmet = require("helmet"); -const { HttpError } = require("@utils/errors"); +const { HttpError, BadRequestError } = require("@utils/errors"); const isDev = process.env.NODE_ENV === "development"; const isProd = process.env.NODE_ENV === "production"; const options = { mongooseConnection: mongoose.connection }; @@ -83,6 +83,12 @@ app.use(function(err, req, res, next) { message: err.message, errors: err.errors, }); + } else if (err instanceof BadRequestError) { + return res.status(err.statusCode).json({ + success: false, + message: err.message, + errors: err.errors, + }); } else if (err instanceof SyntaxError) { res.status(400).json({ success: false, @@ -136,9 +142,9 @@ app.use(function(err, req, res, next) { logger.error(`Internal Server Error --- ${stringify(err)}`); logObject("Internal Server Error", err); logger.error(`Stack Trace: ${err.stack}`); - res.status(err.status || 500).json({ + res.status(err.statusCode || err.status || 500).json({ success: false, - message: "Internal Server Error - app entry", + message: err.message || "Internal Server Error", errors: { message: err.message }, }); } diff --git a/src/device-registry/middleware/test/ut_validateOptionalObjectId.js b/src/device-registry/middleware/test/ut_validateOptionalObjectId.js new file mode 100644 index 0000000000..5b5d62eb44 --- /dev/null +++ b/src/device-registry/middleware/test/ut_validateOptionalObjectId.js @@ -0,0 +1,100 @@ +require("module-alias/register"); +const { expect } = require("chai"); +const sinon = require("sinon"); +const mongoose = require("mongoose"); +const { BadRequestError } = require("@utils/errors"); +const validateOptionalObjectId = require("@middleware/validateOptionalObjectId"); + +describe("validateOptionalObjectId", () => { + let req, res, next; + + beforeEach(() => { + req = { + query: {}, + }; + res = {}; + next = sinon.spy(); + }); + + afterEach(() => { + sinon.restore(); + }); + + it("should call next() if the field is not present in the query", () => { + const middleware = validateOptionalObjectId("testField"); + middleware(req, res, next); + expect(next.calledOnce).to.be.true; + expect(next.args[0]).to.be.empty; + }); + + it("should validate a single valid ObjectId", () => { + const validObjectId = new mongoose.Types.ObjectId().toString(); + req.query.testField = validObjectId; + const middleware = validateOptionalObjectId("testField"); + middleware(req, res, next); + expect(next.calledOnce).to.be.true; + expect(next.args[0]).to.be.empty; + }); + + it("should validate multiple valid ObjectIds", () => { + const validObjectIds = [ + new mongoose.Types.ObjectId().toString(), + new mongoose.Types.ObjectId().toString(), + ]; + req.query.testField = validObjectIds.join(","); + const middleware = validateOptionalObjectId("testField"); + middleware(req, res, next); + expect(next.calledOnce).to.be.true; + expect(next.args[0]).to.be.empty; + }); + + it("should throw BadRequestError for a single invalid ObjectId", () => { + req.query.testField = "invalidObjectId"; + const middleware = validateOptionalObjectId("testField"); + expect(() => middleware(req, res, next)).to.throw(BadRequestError); + expect(next.called).to.be.false; + try { + middleware(req, res, next); + } catch (error) { + expect(error).to.be.instanceOf(BadRequestError); + expect(error.message).to.equal("Validation failed for testField"); + expect(error.errors).to.deep.equal([ + "Invalid testField format: invalidObjectId", + ]); + } + }); + + it("should throw BadRequestError for multiple ObjectIds with some invalid", () => { + const validObjectId = new mongoose.Types.ObjectId().toString(); + req.query.testField = `${validObjectId},invalidObjectId`; + const middleware = validateOptionalObjectId("testField"); + expect(() => middleware(req, res, next)).to.throw(BadRequestError); + expect(next.called).to.be.false; + try { + middleware(req, res, next); + } catch (error) { + expect(error).to.be.instanceOf(BadRequestError); + expect(error.message).to.equal("Validation failed for testField"); + expect(error.errors).to.deep.equal([ + "Invalid testField format: invalidObjectId", + ]); + } + }); + + it("should handle an array of ObjectIds in the query", () => { + const validObjectId = new mongoose.Types.ObjectId().toString(); + req.query.testField = [validObjectId, "invalidObjectId"]; + const middleware = validateOptionalObjectId("testField"); + expect(() => middleware(req, res, next)).to.throw(BadRequestError); + expect(next.called).to.be.false; + try { + middleware(req, res, next); + } catch (error) { + expect(error).to.be.instanceOf(BadRequestError); + expect(error.message).to.equal("Validation failed for testField"); + expect(error.errors).to.deep.equal([ + "Invalid testField format: invalidObjectId", + ]); + } + }); +}); diff --git a/src/device-registry/middleware/validateOptionalObjectId.js b/src/device-registry/middleware/validateOptionalObjectId.js new file mode 100644 index 0000000000..d6c49023c8 --- /dev/null +++ b/src/device-registry/middleware/validateOptionalObjectId.js @@ -0,0 +1,32 @@ +const { isValidObjectId } = require("mongoose"); +const { BadRequestError } = require("@utils/errors"); + +const validateOptionalObjectId = (field) => { + return (req, res, next) => { + if (req.query[field]) { + let values; + if (Array.isArray(req.query[field])) { + values = req.query[field]; + } else { + values = req.query[field].toString().split(","); + } + + const errors = []; + for (const value of values) { + if (!isValidObjectId(value)) { + errors.push(`Invalid ${field} format: ${value}`); + } + } + + if (errors.length > 0) { + throw new BadRequestError({ + message: `Validation failed for ${field}`, + errors: errors, + }); + } + } + next(); + }; +}; + +module.exports = validateOptionalObjectId; diff --git a/src/device-registry/routes/v2/readings.js b/src/device-registry/routes/v2/readings.js index e086cf80b0..19c2d4cea4 100644 --- a/src/device-registry/routes/v2/readings.js +++ b/src/device-registry/routes/v2/readings.js @@ -4,41 +4,8 @@ const eventController = require("@controllers/create-event"); const constants = require("@config/constants"); const mongoose = require("mongoose"); const ObjectId = mongoose.Types.ObjectId; -const { logElement, logText, logObject } = require("@utils/log"); -const NetworkModel = require("@models/Network"); -const { - check, - oneOf, - query, - body, - param, - validationResult, -} = require("express-validator"); - -const decimalPlaces = require("decimal-places"); -const numeral = require("numeral"); - -// Define a custom function to check if a value is a valid ObjectId -const isValidObjectId = (value) => { - return mongoose.Types.ObjectId.isValid(value); -}; - -const addCategoryQueryParam = (req, res, next) => { - req.query.path = "public"; - next(); -}; - -const validNetworks = async () => { - const networks = await NetworkModel("airqo").distinct("name"); - return networks.map((network) => network.toLowerCase()); -}; - -const validateNetwork = async (value) => { - const networks = await validNetworks(); - if (!networks.includes(value.toLowerCase())) { - throw new Error("Invalid network"); - } -}; +const { oneOf, query, validationResult } = require("express-validator"); +const validateOptionalObjectId = require("@middleware/validateOptionalObjectId"); const validatePagination = (req, res, next) => { let limit = parseInt(req.query.limit, 10); @@ -57,40 +24,6 @@ const validatePagination = (req, res, next) => { next(); }; -// Custom validation function to check if values are valid MongoDB ObjectIds -const isValidObjectIds = (value) => { - const ids = value.split(","); - return ids.every((id) => /^[0-9a-fA-F]{24}$/.test(id)); // Check if each ID is a valid ObjectId -}; - -// Middleware for validation -const validateObjectId = (paramName) => { - return [ - query(paramName) - .custom(isValidObjectIds) - .withMessage(`Invalid ${paramName}`), - ]; -}; - -const validateOptionalObjectId = (field) => { - return (req, res, next) => { - if (req.query[field]) { - let values; - if (Array.isArray(req.query[field])) { - values = req.query[field]; - } else { - values = req.query[field].toString().split(","); - } - for (const value of values) { - if (!isValidObjectId(value)) { - throw new Error(`Invalid ${field} format: ${value}`); - } - } - } - next(); - }; -}; - const headers = (req, res, next) => { res.setHeader("Access-Control-Allow-Origin", "*"); res.header( @@ -244,7 +177,7 @@ router.get( query("device_id") .optional() .notEmpty() - .withMessage("the provided device_id cannot be empty IF provided") + .withMessage("device_id cannot be empty IF provided") .trim(), query("lat_long") .optional() @@ -277,15 +210,7 @@ router.get( query("site_id") .optional() .notEmpty() - .withMessage("site_id cannot be empty IF provided") - .bail() - .trim() - .isMongoId() - .withMessage("site_id must be an object ID") - .bail() - .customSanitizer((value) => { - return ObjectId(value); - }), + .withMessage("site_id cannot be empty IF provided"), query("primary") .optional() .notEmpty() diff --git a/src/device-registry/utils/errors.js b/src/device-registry/utils/errors.js index d664fd1d60..19a4582b02 100644 --- a/src/device-registry/utils/errors.js +++ b/src/device-registry/utils/errors.js @@ -12,6 +12,15 @@ class HttpError extends Error { } } +class BadRequestError extends Error { + constructor({ message, errors }) { + super(message); + this.name = "BadRequestError"; + this.statusCode = 400; + this.errors = errors; + } +} + const convertErrorArrayToObject = (arrays) => { const initialValue = {}; return arrays.reduce((obj, item) => { @@ -42,5 +51,6 @@ const extractErrorsFromRequest = (req) => { module.exports = { HttpError, + BadRequestError, extractErrorsFromRequest, };