From 38e5864de618fd2e71f04c612cc9f32b63095e55 Mon Sep 17 00:00:00 2001 From: Ayush Date: Thu, 4 Jun 2026 00:44:04 +0530 Subject: [PATCH 1/4] feat(dashboard-api): update middlewares for AppError and ApiResponse --- .../src/__tests__/authMiddleware.test.js | 50 +++++++++++-------- .../src/__tests__/auth_limiter.test.js | 50 ++++++++++++++----- .../src/middlewares/authMiddleware.js | 5 +- .../src/middlewares/auth_limiter.js | 5 +- .../src/middlewares/loadProjectForAdmin.js | 7 +-- 5 files changed, 78 insertions(+), 39 deletions(-) diff --git a/apps/dashboard-api/src/__tests__/authMiddleware.test.js b/apps/dashboard-api/src/__tests__/authMiddleware.test.js index 67bab1f0f..eb944862e 100644 --- a/apps/dashboard-api/src/__tests__/authMiddleware.test.js +++ b/apps/dashboard-api/src/__tests__/authMiddleware.test.js @@ -1,5 +1,17 @@ 'use strict'; +class AppError extends Error { + constructor(statusCode, message) { + super(message); + this.statusCode = statusCode; + this.isOperational = true; + } +} + +jest.mock('@urbackend/common', () => ({ + AppError, +})); + jest.mock('jsonwebtoken'); const jwt = require('jsonwebtoken'); @@ -29,7 +41,7 @@ describe('authMiddleware', () => { expect(jwt.verify).toHaveBeenCalledWith('cookietoken', 'test-secret'); expect(req.user).toEqual({ _id: 'user1' }); expect(next).toHaveBeenCalledTimes(1); - expect(res.status).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(); }); }); @@ -93,11 +105,9 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(res.json).toHaveBeenCalledWith({ - error: 'Access Denied: No Token Provided', - }); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(401); + expect(next.mock.calls[0][0].message).toBe('Access Denied: No Token Provided'); }); test('rejects header with wrong scheme (Token)', () => { @@ -105,8 +115,8 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(401); }); test('rejects header with no scheme (bare token only)', () => { @@ -114,8 +124,8 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(401); }); test('rejects empty Authorization header', () => { @@ -123,8 +133,8 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(401); }); test('rejects header with extra whitespace but no token value', () => { @@ -134,8 +144,8 @@ describe('authMiddleware', () => { // After trimming and splitting on whitespace, only the scheme 'Bearer' // remains and there is no token part, so it's treated as missing token. - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(401); }); }); @@ -150,9 +160,9 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ error: 'Invalid Token' }); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(400); + expect(next.mock.calls[0][0].message).toBe('Invalid Token'); }); test('rejects a malformed / invalid JWT', () => { @@ -165,9 +175,9 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ error: 'Invalid Token' }); - expect(next).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + expect(next.mock.calls[0][0].statusCode).toBe(400); + expect(next.mock.calls[0][0].message).toBe('Invalid Token'); }); test('attaches decoded payload to req.user on success', () => { diff --git a/apps/dashboard-api/src/__tests__/auth_limiter.test.js b/apps/dashboard-api/src/__tests__/auth_limiter.test.js index a41020822..542292588 100644 --- a/apps/dashboard-api/src/__tests__/auth_limiter.test.js +++ b/apps/dashboard-api/src/__tests__/auth_limiter.test.js @@ -1,9 +1,43 @@ 'use strict'; +class AppError extends Error { + constructor(statusCode, message) { + super(message); + this.statusCode = statusCode; + this.isOperational = true; + } +} + +jest.mock('@urbackend/common', () => ({ + AppError, +})); + const express = require('express'); const request = require('supertest'); const { authLimiter } = require('../middlewares/auth_limiter'); +// Helper: creates a test app with the limiter and a global error handler +// that mirrors the real dashboard-api error handler behavior. +const createTestApp = () => { + const app = express(); + app.use(authLimiter); + app.get('/test', (_req, res) => res.json({ ok: true })); + + // Global error handler (mirrors dashboard-api app.js) + app.use((err, _req, res, _next) => { + if (err.isOperational && err.statusCode) { + return res.status(err.statusCode).json({ + success: false, + error: 'Error', + message: err.message, + }); + } + res.status(500).json({ error: 'Internal Server Error' }); + }); + + return app; +}; + describe('authLimiter', () => { test('exports authLimiter as a middleware function', () => { expect(typeof authLimiter).toBe('function'); @@ -14,9 +48,7 @@ describe('authLimiter', () => { try { process.env.NODE_ENV = 'development'; - const app = express(); - app.use(authLimiter); - app.get('/test', (_req, res) => res.json({ ok: true })); + const app = createTestApp(); // Exceed the configured max (10) — all should still succeed because // the limiter is skipped in development. @@ -34,9 +66,7 @@ describe('authLimiter', () => { try { process.env.NODE_ENV = 'production'; - const app = express(); - app.use(authLimiter); - app.get('/test', (_req, res) => res.json({ ok: true })); + const app = createTestApp(); // Exhaust the 10-request window. for (let i = 0; i < 10; i++) { @@ -47,9 +77,7 @@ describe('authLimiter', () => { // The 11th request should be rate-limited. const blocked = await request(app).get('/test'); expect(blocked.status).toBe(429); - expect(blocked.body).toEqual({ - error: 'Too many attempts. Please try again in 15 minutes.', - }); + expect(blocked.body.message).toBe('Too many attempts. Please try again in 15 minutes.'); } finally { process.env.NODE_ENV = originalEnv; } @@ -60,9 +88,7 @@ describe('authLimiter', () => { try { process.env.NODE_ENV = 'production'; - const app = express(); - app.use(authLimiter); - app.get('/test', (_req, res) => res.json({ ok: true })); + const app = createTestApp(); const res = await request(app).get('/test'); // standardHeaders:true → RateLimit-* headers should be present. diff --git a/apps/dashboard-api/src/middlewares/authMiddleware.js b/apps/dashboard-api/src/middlewares/authMiddleware.js index 01a9343cb..8880c15d8 100644 --- a/apps/dashboard-api/src/middlewares/authMiddleware.js +++ b/apps/dashboard-api/src/middlewares/authMiddleware.js @@ -1,4 +1,5 @@ const jwt = require('jsonwebtoken'); +const { AppError } = require('@urbackend/common'); module.exports = function (req, res, next) { // Check for token in cookies (Primary for Web) @@ -17,7 +18,7 @@ module.exports = function (req, res, next) { // Check if any token was provided if (!token) { - return res.status(401).json({ error: 'Access Denied: No Token Provided' }); + return next(new AppError(401, 'Access Denied: No Token Provided')); } try { @@ -35,6 +36,6 @@ module.exports = function (req, res, next) { console.error(err); } - res.status(400).json({ error: 'Invalid Token' }); + return next(new AppError(400, 'Invalid Token')); } }; diff --git a/apps/dashboard-api/src/middlewares/auth_limiter.js b/apps/dashboard-api/src/middlewares/auth_limiter.js index 887345486..df249fef0 100644 --- a/apps/dashboard-api/src/middlewares/auth_limiter.js +++ b/apps/dashboard-api/src/middlewares/auth_limiter.js @@ -1,10 +1,11 @@ const rateLimit = require('express-rate-limit'); +const { AppError } = require('@urbackend/common'); // limiter for sensitive auth endpoints (login, register) const authLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 10, - message: { error: "Too many attempts. Please try again in 15 minutes." }, + handler: (req, res, next) => next(new AppError(429, "Too many attempts. Please try again in 15 minutes.")), skip: (req) => process.env.NODE_ENV === 'development', standardHeaders: true, legacyHeaders: false, @@ -13,7 +14,7 @@ const authLimiter = rateLimit({ const publicLimiter = rateLimit({ windowMs: 15 * 60 * 1000, // 15 minutes max: 500, - message: { error: "Too many requests. Please try again later." }, + handler: (req, res, next) => next(new AppError(429, "Too many requests. Please try again later.")), skip: (req) => process.env.NODE_ENV === 'development', standardHeaders: true, legacyHeaders: false, diff --git a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js index 514c8afc7..7ac5a053c 100644 --- a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js +++ b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js @@ -1,19 +1,20 @@ // FUNCTION - LOAD PROJECT FOR ADMIN (MIDDLEWARE) const Project = require('../models/Project'); +const { AppError } = require('@urbackend/common'); module.exports = async (req, res, next) => { try { const { projectId } = req.params; - if (!projectId) return res.status(400).json({ error: "Project ID is required" }); + if (!projectId) return next(new AppError(400, "Project ID is required")); const project = await Project.findOne({ _id: projectId, owner: req.user._id }); if (!project) { - return res.status(404).json({ error: "Project not found or access denied" }); + return next(new AppError(404, "Project not found or access denied")); } req.project = project; next(); } catch (err) { - res.status(500).json({ error: err.message }); + next(err); } }; From 651a4ea62f663f2b47d43c66d11650238caf427e Mon Sep 17 00:00:00 2001 From: Ayush Date: Tue, 9 Jun 2026 21:37:50 +0530 Subject: [PATCH 2/4] fix(dashboard-api): address code rabbit review for loadProjectForAdmin and authMiddleware --- .../src/__tests__/authMiddleware.test.js | 4 +- .../src/__tests__/loadProjectForAdmin.test.js | 71 +++++++++++++++++++ .../src/middlewares/authMiddleware.js | 2 +- .../src/middlewares/loadProjectForAdmin.js | 3 +- 4 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js diff --git a/apps/dashboard-api/src/__tests__/authMiddleware.test.js b/apps/dashboard-api/src/__tests__/authMiddleware.test.js index eb944862e..9505aea57 100644 --- a/apps/dashboard-api/src/__tests__/authMiddleware.test.js +++ b/apps/dashboard-api/src/__tests__/authMiddleware.test.js @@ -161,7 +161,7 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); expect(next).toHaveBeenCalledWith(expect.any(AppError)); - expect(next.mock.calls[0][0].statusCode).toBe(400); + expect(next.mock.calls[0][0].statusCode).toBe(401); expect(next.mock.calls[0][0].message).toBe('Invalid Token'); }); @@ -176,7 +176,7 @@ describe('authMiddleware', () => { authMiddleware(req, res, next); expect(next).toHaveBeenCalledWith(expect.any(AppError)); - expect(next.mock.calls[0][0].statusCode).toBe(400); + expect(next.mock.calls[0][0].statusCode).toBe(401); expect(next.mock.calls[0][0].message).toBe('Invalid Token'); }); diff --git a/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js b/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js new file mode 100644 index 000000000..0fcbdd12e --- /dev/null +++ b/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js @@ -0,0 +1,71 @@ +'use strict'; + +const { Project } = require('@urbackend/common'); +const loadProjectForAdmin = require('../middlewares/loadProjectForAdmin'); + +class AppError extends Error { + constructor(statusCode, message) { + super(message); + this.statusCode = statusCode; + } +} + +jest.mock('@urbackend/common', () => ({ + AppError, + Project: { + findOne: jest.fn() + } +})); + + +describe('loadProjectForAdmin Middleware', () => { + let req, res, next; + + beforeEach(() => { + req = { + params: {}, + user: { _id: 'user123' } + }; + res = {}; + next = jest.fn(); + jest.clearAllMocks(); + }); + + it('should call next with AppError(400) if projectId is missing', async () => { + await loadProjectForAdmin(req, res, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + const error = next.mock.calls[0][0]; + expect(error.statusCode).toBe(400); + expect(error.message).toBe("Project ID is required"); + expect(Project.findOne).not.toHaveBeenCalled(); + }); + + it('should call next with AppError(404) if project is not found', async () => { + req.params.projectId = 'proj123'; + Project.findOne.mockResolvedValueOnce(null); + + await loadProjectForAdmin(req, res, next); + + expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', owner: 'user123' }); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(expect.any(AppError)); + const error = next.mock.calls[0][0]; + expect(error.statusCode).toBe(404); + expect(error.message).toBe("Project not found or access denied"); + }); + + it('should set req.project and call next without error if project is found', async () => { + req.params.projectId = 'proj123'; + const mockProject = { _id: 'proj123', name: 'Test Project' }; + Project.findOne.mockResolvedValueOnce(mockProject); + + await loadProjectForAdmin(req, res, next); + + expect(Project.findOne).toHaveBeenCalledWith({ _id: 'proj123', owner: 'user123' }); + expect(req.project).toEqual(mockProject); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); + }); +}); diff --git a/apps/dashboard-api/src/middlewares/authMiddleware.js b/apps/dashboard-api/src/middlewares/authMiddleware.js index 8880c15d8..bee883a65 100644 --- a/apps/dashboard-api/src/middlewares/authMiddleware.js +++ b/apps/dashboard-api/src/middlewares/authMiddleware.js @@ -36,6 +36,6 @@ module.exports = function (req, res, next) { console.error(err); } - return next(new AppError(400, 'Invalid Token')); + return next(new AppError(401, 'Invalid Token')); } }; diff --git a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js index 7ac5a053c..8dcd9c5c5 100644 --- a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js +++ b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js @@ -1,6 +1,5 @@ // FUNCTION - LOAD PROJECT FOR ADMIN (MIDDLEWARE) -const Project = require('../models/Project'); -const { AppError } = require('@urbackend/common'); +const { Project, AppError } = require('@urbackend/common'); module.exports = async (req, res, next) => { try { From d6f77a17c160160f84eebef454490f89c1181c93 Mon Sep 17 00:00:00 2001 From: Ayush Date: Tue, 9 Jun 2026 21:42:13 +0530 Subject: [PATCH 3/4] fix(test): mock @urbackend/common before require to prevent CI redis initialization crash --- .../dashboard-api/src/__tests__/loadProjectForAdmin.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js b/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js index 0fcbdd12e..a2f734968 100644 --- a/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js +++ b/apps/dashboard-api/src/__tests__/loadProjectForAdmin.test.js @@ -1,8 +1,5 @@ 'use strict'; -const { Project } = require('@urbackend/common'); -const loadProjectForAdmin = require('../middlewares/loadProjectForAdmin'); - class AppError extends Error { constructor(statusCode, message) { super(message); @@ -17,6 +14,9 @@ jest.mock('@urbackend/common', () => ({ } })); +const { Project } = require('@urbackend/common'); +const loadProjectForAdmin = require('../middlewares/loadProjectForAdmin'); + describe('loadProjectForAdmin Middleware', () => { let req, res, next; From 348a2986a5ee8ee148909ef1fc6d024c7166b5a4 Mon Sep 17 00:00:00 2001 From: Ayush Date: Tue, 9 Jun 2026 22:47:52 +0530 Subject: [PATCH 4/4] added Utility to loadProjectForAdmin --- apps/dashboard-api/src/middlewares/loadProjectForAdmin.js | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js index 8dcd9c5c5..9bc03c32e 100644 --- a/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js +++ b/apps/dashboard-api/src/middlewares/loadProjectForAdmin.js @@ -1,4 +1,5 @@ // FUNCTION - LOAD PROJECT FOR ADMIN (MIDDLEWARE) + const { Project, AppError } = require('@urbackend/common'); module.exports = async (req, res, next) => {