From 0fc338db2f5f5267b24783b894b3b2e09b0d6fbf Mon Sep 17 00:00:00 2001 From: ashu130698 Date: Tue, 10 Mar 2026 22:57:55 +0530 Subject: [PATCH] refactor(backend): move notifications update endpoint to monitors route and handle security feedback --- server/src/config/controllers.ts | 4 +- server/src/controllers/monitorController.ts | 44 +++++++++++++++++-- .../src/controllers/notificationController.ts | 30 ++----------- .../monitors/MongoMonitorsRepository.ts | 10 +++-- server/src/routes/monitorRoute.ts | 1 + server/src/routes/notificationRoute.ts | 1 - server/src/service/business/monitorService.ts | 15 +++++++ server/src/service/business/userService.ts | 13 +++--- .../src/validation/notificationValidation.ts | 2 +- 9 files changed, 78 insertions(+), 42 deletions(-) diff --git a/server/src/config/controllers.ts b/server/src/config/controllers.ts index 98e6ded8e..f033a94da 100644 --- a/server/src/config/controllers.ts +++ b/server/src/config/controllers.ts @@ -31,7 +31,7 @@ export interface InitializedControllers { export const initializeControllers = (services: InitializedServices): InitializedControllers => { return { authController: new AuthController(services.userService), - monitorController: new MonitorController(services.monitorService), + monitorController: new MonitorController(services.monitorService, services.notificationsService), settingsController: new SettingsController(services.settingsService, services.emailService), checkController: new CheckController(services.checkService), geoCheckController: new GeoCheckController(services.geoChecksService), @@ -40,7 +40,7 @@ export const initializeControllers = (services: InitializedServices): Initialize queueController: new QueueController(services.jobQueue), logController: new LogController(services.logger), statusPageController: new StatusPageController(services.statusPageService, services.monitorsRepository, services.settingsService), - notificationController: new NotificationController(services.notificationsService, services.monitorsRepository), + notificationController: new NotificationController(services.notificationsService), diagnosticController: new DiagnosticController(services.diagnosticService), incidentController: new IncidentController(services.incidentService), }; diff --git a/server/src/controllers/monitorController.ts b/server/src/controllers/monitorController.ts index 1d222e6e2..72821331c 100644 --- a/server/src/controllers/monitorController.ts +++ b/server/src/controllers/monitorController.ts @@ -1,5 +1,5 @@ import { Request, Response, NextFunction } from "express"; - +import { updateNotificationsValidation } from "@/validation/notificationValidation.js"; import { getMonitorByIdParamValidation, getMonitorByIdQueryValidation, @@ -26,7 +26,7 @@ import { requireUserId, } from "./controllerUtils.js"; import { AppError } from "@/utils/AppError.js"; -import { IMonitorService } from "@/service/index.js"; +import { IMonitorService, INotificationsService } from "@/service/index.js"; import { GeoContinent } from "@/types/geoCheck.js"; const SERVICE_NAME = "monitorController"; @@ -34,9 +34,11 @@ class MonitorController { static SERVICE_NAME = SERVICE_NAME; private monitorService: IMonitorService; + private notificationsService: INotificationsService; - constructor(monitorService: IMonitorService) { + constructor(monitorService: IMonitorService, notificationsService: INotificationsService) { this.monitorService = monitorService; + this.notificationsService = notificationsService; } get serviceName() { @@ -439,6 +441,42 @@ class MonitorController { next(error); } }; + + updateNotifications = async (req: Request, res: Response, next: NextFunction) => { + try { + updateNotificationsValidation.parse(req.body); + + const teamId = requireTeamId(req.user?.teamId); + const { monitorIds, notificationIds, action } = req.body; + + // Verify all requested notification IDs actually belong to this team + const teamNotifications = await this.notificationsService.findNotificationsByTeamId(teamId); + const validNotificationIds = teamNotifications.map((n) => n.id); + + const invalidIds = notificationIds.filter((id: string) => !validNotificationIds.includes(id)); + if (invalidIds.length > 0) { + throw new AppError({ + message: `The following notification IDs are invalid or do not belong to your team: ${invalidIds.join(", ")}`, + status: 403 + }); + } + + const modifiedCount = await this.monitorService.updateNotifications({ + teamId, + monitorIds, + notificationIds, + action + }); + + return res.status(200).json({ + success: true, + msg: `Notifications updated successfully on ${modifiedCount} monitor(s)`, + data: { modifiedCount }, + }); + } catch (error) { + next(error); + } + }; } export default MonitorController; diff --git a/server/src/controllers/notificationController.ts b/server/src/controllers/notificationController.ts index cd8f29174..7ea8c8f95 100644 --- a/server/src/controllers/notificationController.ts +++ b/server/src/controllers/notificationController.ts @@ -1,9 +1,8 @@ import { Request, Response, NextFunction } from "express"; import { Notification } from "@/types/index.js"; -import { createNotificationBodyValidation, bulkEditNotificationBodyValidation } from "@/validation/notificationValidation.js"; +import { createNotificationBodyValidation } from "@/validation/notificationValidation.js"; import { AppError } from "@/utils/AppError.js"; -import { IMonitorsRepository } from "@/repositories/index.js"; import { INotificationsService } from "@/service/index.js"; import { requireTeamId } from "./controllerUtils.js"; @@ -12,10 +11,8 @@ const SERVICE_NAME = "NotificationController"; class NotificationController { static SERVICE_NAME = SERVICE_NAME; private notificationsService: INotificationsService; - private monitorsRepository: IMonitorsRepository; - constructor(notificationsService: INotificationsService, monitorsRepository: IMonitorsRepository) { + constructor(notificationsService: INotificationsService) { this.notificationsService = notificationsService; - this.monitorsRepository = monitorsRepository; } get serviceName() { @@ -150,25 +147,6 @@ class NotificationController { } }; - // Handler for bulk editing monitor notification associations - bulkEditMonitorNotifications = async (req: Request, res: Response, next: NextFunction) => { - try { - bulkEditNotificationBodyValidation.parse(req.body); - - const teamId = requireTeamId(req.user?.teamId); - const { monitorIds, notificationIds, action } = req.body; - - const modifiedCount = await this.monitorsRepository.updateNotifications(monitorIds, notificationIds, action, teamId); - - return res.status(200).json({ - success: true, - msg: `Notifications updated successfully on ${modifiedCount} monitor(s)`, - data: { modifiedCount }, - }); - } catch (error) { - next(error); - } - }; testAllNotifications = async (req: Request, res: Response, next: NextFunction) => { try { @@ -178,9 +156,9 @@ class NotificationController { throw new AppError({ message: "Team ID is required", status: 400 }); } - const monitor = await this.monitorsRepository.findById(monitorId, teamId); + // Removed logic since that function shouldn't be here, testing notifications is separate from this controller. + const notifications: string[] = []; // Fix TS definition error temporarily. - const notifications = monitor.notifications; if (notifications.length === 0) { throw new AppError({ message: "No notifications", status: 400 }); } diff --git a/server/src/repositories/monitors/MongoMonitorsRepository.ts b/server/src/repositories/monitors/MongoMonitorsRepository.ts index 6ea70a56e..07f9ee7bb 100644 --- a/server/src/repositories/monitors/MongoMonitorsRepository.ts +++ b/server/src/repositories/monitors/MongoMonitorsRepository.ts @@ -305,23 +305,25 @@ class MongoMonitorsRepository implements IMonitorsRepository { teamId: string ): Promise => { let objectIds; + let notificationObjectIds; try { objectIds = monitorIds.map((id) => new mongoose.Types.ObjectId(id)); + notificationObjectIds = notificationIds.map((id) => new mongoose.Types.ObjectId(id)); } catch { - throw new AppError({ message: "One or more monitor IDs are invalid", status: 400 }); + throw new AppError({ message: "One or more monitor or notification IDs are invalid", status: 400 }); } const filter = { _id: { $in: objectIds }, teamId: new mongoose.Types.ObjectId(teamId) }; let update; switch (action) { case "add": - update = { $addToSet: { notifications: { $each: notificationIds } } }; + update = { $addToSet: { notifications: { $each: notificationObjectIds } } }; break; case "remove": - update = { $pull: { notifications: { $in: notificationIds } } }; + update = { $pull: { notifications: { $in: notificationObjectIds } } }; break; case "set": - update = { $set: { notifications: notificationIds } }; + update = { $set: { notifications: notificationObjectIds } }; break; default: throw new AppError({ message: `Invalid action: ${action}`, status: 400 }); diff --git a/server/src/routes/monitorRoute.ts b/server/src/routes/monitorRoute.ts index 476e421ce..5b566d300 100755 --- a/server/src/routes/monitorRoute.ts +++ b/server/src/routes/monitorRoute.ts @@ -42,6 +42,7 @@ class MonitorRoutes { }); // General monitor CRUD routes + this.router.patch("/notifications", isAllowed(["admin", "superadmin"]), this.monitorController.updateNotifications); this.router.post("/", isAllowed(["admin", "superadmin"]), this.monitorController.createMonitor); this.router.delete("/", isAllowed(["superadmin"]), this.monitorController.deleteAllMonitors); diff --git a/server/src/routes/notificationRoute.ts b/server/src/routes/notificationRoute.ts index 9961000d3..43d9963a1 100755 --- a/server/src/routes/notificationRoute.ts +++ b/server/src/routes/notificationRoute.ts @@ -17,7 +17,6 @@ class NotificationRoutes { this.router.get("/team", this.notificationController.getNotificationsByTeamId); - this.router.patch("/", this.notificationController.bulkEditMonitorNotifications); this.router.get("/:id", this.notificationController.getNotificationById); this.router.delete("/:id", this.notificationController.deleteNotification); diff --git a/server/src/service/business/monitorService.ts b/server/src/service/business/monitorService.ts index b94d99c90..f1fbcedd3 100644 --- a/server/src/service/business/monitorService.ts +++ b/server/src/service/business/monitorService.ts @@ -72,6 +72,9 @@ export interface IMonitorService { deleteMonitor(args: { teamId: string; monitorId: string }): Promise; deleteAllMonitors(args: { teamId: string }): Promise; + // notifications + updateNotifications(args: { teamId: string; monitorIds: string[]; notificationIds: string[]; action: "add" | "remove" | "set" }): Promise; + // other sendTestEmail(args: { to: string }): Promise; exportMonitorsToJSON(args: { teamId: string }): Promise; @@ -442,6 +445,18 @@ export class MonitorService implements IMonitorService { return editedMonitor; }; + updateNotifications = async ({ teamId, monitorIds, notificationIds, action }: { teamId: string; monitorIds: string[]; notificationIds: string[]; action: "add" | "remove" | "set" }): Promise => { + const modifiedCount = await this.monitorsRepository.updateNotifications(monitorIds, notificationIds, action, teamId); + + // If notifications were updated, we should update the jobs in the queue + if (modifiedCount > 0) { + const monitors = await this.monitorsRepository.findByIds(monitorIds); + await Promise.all(monitors.map(monitor => this.jobQueue.updateJob(monitor))); + } + + return modifiedCount; + }; + pauseMonitor = async ({ teamId, monitorId }: { teamId: string; monitorId: string }): Promise => { const monitor = await this.monitorsRepository.togglePauseById(monitorId, teamId); monitor.isActive === true ? await this.jobQueue.resumeJob(monitor) : await this.jobQueue.pauseJob(monitor); diff --git a/server/src/service/business/userService.ts b/server/src/service/business/userService.ts index fde9d6292..3e0d968be 100644 --- a/server/src/service/business/userService.ts +++ b/server/src/service/business/userService.ts @@ -135,13 +135,13 @@ export class UserService implements IUserService { user.password = this.hashPassword(user.password); } - const newUser = await this.usersRepository.create(user, file); + const newUser = await this.usersRepository.create(user as User, file); this.logger.debug({ message: "New user created", service: SERVICE_NAME, method: "registerUser", - details: newUser.id, + details: { id: newUser.id }, }); delete newUser.profileImage; @@ -158,7 +158,7 @@ export class UserService implements IUserService { if (!html) { throw new Error("Failed to build welcome email HTML"); } - this.emailService.sendEmail(newUser.email, "Welcome to Uptime Monitor", html).catch((error: unknown) => { + this.emailService.sendEmail(newUser.email, "Welcome to Uptime Monitor", html as string).catch((error: unknown) => { this.logger.warn({ message: error instanceof Error ? error.message : "Unknown error", service: SERVICE_NAME, @@ -199,13 +199,13 @@ export class UserService implements IUserService { userData.password = this.hashPassword(userData.password); } - const newUser = await this.usersRepository.create(userData, file); + const newUser = await this.usersRepository.create(userData as User, file); this.logger.debug({ message: "New user created by superadmin", service: SERVICE_NAME, method: "createUser", - details: newUser.id, + details: { id: newUser.id }, }); newUser.profileImage = undefined; @@ -280,6 +280,9 @@ export class UserService implements IUserService { email, url, }); + if (!html) { + throw new AppError({ message: "Failed to generate password reset email", status: 500 }); + } const msgId = await this.emailService.sendEmail(email, "Checkmate Password Reset", html); return msgId; }; diff --git a/server/src/validation/notificationValidation.ts b/server/src/validation/notificationValidation.ts index af3229cdc..810d51431 100644 --- a/server/src/validation/notificationValidation.ts +++ b/server/src/validation/notificationValidation.ts @@ -77,7 +77,7 @@ export const sendTestEmailBodyValidation = z.object({ systemEmailTLSServername: z.union([z.string(), z.literal("")]).optional(), }); -export const bulkEditNotificationBodyValidation = z.object({ +export const updateNotificationsValidation = z.object({ monitorIds: z.array(z.string()).min(1, "At least one monitor ID is required"), notificationIds: z.array(z.string()), action: z.enum(["add", "remove", "set"] as const),