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 689727b1db..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,8 +210,7 @@ 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"), 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, };