From 830d49d137f3b40d4e622bd7df2f889fc0f9e5ac Mon Sep 17 00:00:00 2001 From: dubemoyibe-star Date: Sat, 27 Jun 2026 19:15:36 +0100 Subject: [PATCH 1/4] fix(auth): prevent NoSQL injection in authentication controllers --- src/controllers/login.controller.ts | 16 +++-- src/controllers/signup.controller.ts | 16 +++-- src/validators/auth.validator.ts | 16 +++++ tests/login.controller.test.ts | 98 ++++++++++++++++++++++++++++ tests/signup.controller.test.ts | 74 +++++++++++++++++++-- 5 files changed, 206 insertions(+), 14 deletions(-) create mode 100644 src/validators/auth.validator.ts create mode 100644 tests/login.controller.test.ts diff --git a/src/controllers/login.controller.ts b/src/controllers/login.controller.ts index 4ba774e..774d17f 100644 --- a/src/controllers/login.controller.ts +++ b/src/controllers/login.controller.ts @@ -2,16 +2,22 @@ import { RequestHandler } from 'express'; import bcrypt from 'bcrypt'; import User from '../models/user'; import { generateAccessToken } from '../utils/token'; +import { LoginSchema } from '../validators/auth.validator'; +import z from 'zod'; export const loginController: RequestHandler = async (req, res, next) => { try { - const { email, password } = req.body; + const parsed = LoginSchema.safeParse(req.body); - if (!email || !password) { - res.status(400).json({ message: 'Email and password are required' }); - return; + if (!parsed.success) { + return res.status(400).json({ + error: 'Validation failed', + messages: z.treeifyError(parsed.error), + }); } + const { email, password } = parsed.data; + const user = await User.findOne({ email }); if (!user) { res.status(404).json({ message: 'User not found' }); @@ -45,4 +51,4 @@ export const loginController: RequestHandler = async (req, res, next) => { } catch (error: any) { res.status(500).json({ message: error.message }); } -}; +}; \ No newline at end of file diff --git a/src/controllers/signup.controller.ts b/src/controllers/signup.controller.ts index c2b083c..8a12168 100644 --- a/src/controllers/signup.controller.ts +++ b/src/controllers/signup.controller.ts @@ -3,6 +3,8 @@ import bcrypt from 'bcrypt'; import User from '../models/user'; import { generateOTP } from '../utils/otp'; import emailService from '../services/email.service'; +import { SignupSchema } from '../validators/auth.validator'; +import z from 'zod'; const OTP_EXPIRY_MINUTES = 10; @@ -11,13 +13,17 @@ export const signupController: RequestHandler = async ( res: Response, ) => { try { - const { name, email, password } = req.body; + const parsed = SignupSchema.safeParse(req.body); - if (!name || !email || !password) { - res.status(400).json({ message: 'All fields are required' }); - return; + if (!parsed.success) { + return res.status(400).json({ + error: 'Validation failed', + messages: z.treeifyError(parsed.error), + }); } + const { name, email, password } = parsed.data; + const existingUser = await User.findOne({ email }); if (existingUser) { res.status(400).json({ message: 'Email is already in use' }); @@ -55,4 +61,4 @@ export const signupController: RequestHandler = async ( } catch (error: any) { res.status(500).json({ message: error.message }); } -}; +}; \ No newline at end of file diff --git a/src/validators/auth.validator.ts b/src/validators/auth.validator.ts new file mode 100644 index 0000000..e58e5e6 --- /dev/null +++ b/src/validators/auth.validator.ts @@ -0,0 +1,16 @@ +import { z } from 'zod'; + +export const LoginSchema = z.object({ + email: z.string().email('Invalid email format'), + password: z.string().min(1, 'Password is required'), +}); + +export type LoginInput = z.infer; + +export const SignupSchema = z.object({ + name: z.string().min(1, 'Name is required'), + email: z.string().email('Invalid email format'), + password: z.string().min(1, 'Password is required'), +}); + +export type SignupInput = z.infer; \ No newline at end of file diff --git a/tests/login.controller.test.ts b/tests/login.controller.test.ts new file mode 100644 index 0000000..4319e39 --- /dev/null +++ b/tests/login.controller.test.ts @@ -0,0 +1,98 @@ +import { loginController } from '../src/controllers/login.controller'; +import User from '../src/models/user'; + +jest.mock('bcrypt', () => ({ + compare: jest.fn(), +})); + +jest.mock('../src/models/user', () => { + const MockUser = jest.fn(); + (MockUser as any).findOne = jest.fn(); + return { __esModule: true, default: MockUser }; +}); + +jest.mock('../src/utils/token', () => ({ + generateAccessToken: jest.fn().mockReturnValue('mock-token'), +})); + +describe('loginController', () => { + const createResponse = () => ({ + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns 400 when email is missing', async () => { + const req = { body: { password: 'secret123' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when password is missing', async () => { + const req = { body: { email: 'test@example.com' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when email is not a valid email format', async () => { + const req = { body: { email: 'not-an-email', password: 'secret123' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when email is a NoSQL injection payload ($ne)', async () => { + const req = { body: { email: { $ne: null }, password: 'secret123' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + expect(User.findOne).not.toHaveBeenCalled(); + }); + + it('returns 400 when email is a NoSQL injection payload ($gt)', async () => { + const req = { body: { email: { $gt: '' }, password: 'secret123' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when password is a NoSQL injection payload', async () => { + const req = { body: { email: 'test@example.com', password: { $ne: null } } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + expect(User.findOne).not.toHaveBeenCalled(); + }); + + it('returns 404 when user not found', async () => { + (User.findOne as jest.Mock).mockResolvedValue(null); + const req = { body: { email: 'nonexistent@example.com', password: 'secret123' } }; + const res = createResponse(); + await loginController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(404); + expect(res.json).toHaveBeenCalledWith({ message: 'User not found' }); + }); +}); \ No newline at end of file diff --git a/tests/signup.controller.test.ts b/tests/signup.controller.test.ts index dac1726..28ebdf1 100644 --- a/tests/signup.controller.test.ts +++ b/tests/signup.controller.test.ts @@ -48,9 +48,9 @@ describe('signup controller', () => { const res = createResponse(); await signupController(req as any, res as any, jest.fn()); expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ - message: 'All fields are required', - }); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); expect(User.findOne).not.toHaveBeenCalled(); }); @@ -110,4 +110,70 @@ describe('signup controller', () => { 'User registered successfully. Please verify your account with the OTP sent to your email.', }); }); -}); + + it('returns 400 when email is a NoSQL injection payload ($ne)', async () => { + const req = { + body: { + name: 'Test User', + email: { $ne: null }, + password: 'secret123', + }, + }; + const res = createResponse(); + await signupController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + expect(User.findOne).not.toHaveBeenCalled(); + }); + + it('returns 400 when email is a NoSQL injection payload ($gt)', async () => { + const req = { + body: { + name: 'Test User', + email: { $gt: '' }, + password: 'secret123', + }, + }; + const res = createResponse(); + await signupController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when name is a NoSQL injection payload', async () => { + const req = { + body: { + name: { $ne: null }, + email: 'test@example.com', + password: 'secret123', + }, + }; + const res = createResponse(); + await signupController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + }); + + it('returns 400 when password is a NoSQL injection payload', async () => { + const req = { + body: { + name: 'Test User', + email: 'test@example.com', + password: { $ne: null }, + }, + }; + const res = createResponse(); + await signupController(req as any, res as any, jest.fn()); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: 'Validation failed' }), + ); + expect(User.findOne).not.toHaveBeenCalled(); + }); +}); \ No newline at end of file From 36e727de5c073541654c7b7da432dd02a8b1861a Mon Sep 17 00:00:00 2001 From: dubemoyibe-star Date: Sat, 27 Jun 2026 20:05:12 +0100 Subject: [PATCH 2/4] fix(auth): secure OAuth JWT delivery with HTTP-only cookies --- src/routes/auth.route.ts | 13 ++++-- src/utils/helper.ts | 9 +++- tests/auth.middleware.test.ts | 87 +++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 5 deletions(-) diff --git a/src/routes/auth.route.ts b/src/routes/auth.route.ts index 1c0665c..b90081a 100644 --- a/src/routes/auth.route.ts +++ b/src/routes/auth.route.ts @@ -53,11 +53,16 @@ authRoute.get( const token = generateToken(req.user as any); - // Redirect to frontend with token - res.redirect(`${process.env.FRONTEND_URL}/oauth?token=${token}`); + const isProduction = process.env.NODE_ENV === 'production'; - // Send token in response body - // res.status(200).json({ token }); + res.cookie('token', token, { + httpOnly: true, + secure: isProduction, + sameSite: 'lax', + maxAge: 60 * 60 * 1000, + }); + + res.redirect(process.env.FRONTEND_URL!); }, ); diff --git a/src/utils/helper.ts b/src/utils/helper.ts index 59f4b1e..fd8eef9 100644 --- a/src/utils/helper.ts +++ b/src/utils/helper.ts @@ -2,8 +2,15 @@ import User from '../models/user'; var jwt = require('jsonwebtoken'); import { JwtVerify } from '../middlewares/jwt'; +const parseCookie = (cookieHeader: string | undefined, name: string): string | undefined => { + if (!cookieHeader) return undefined; + const cookies = cookieHeader.split(';').map((c) => c.trim().split('=')); + const cookie = cookies.find(([key]) => key === name); + return cookie?.[1]; +}; + const extractToken = (req: any): string | null => { - return req.headers.authorization?.split(' ')[1] || null; + return req.headers.authorization?.split(' ')[1] || parseCookie(req.headers.cookie, 'token') || null; }; const validateAndGetUser = async (token: string) => { diff --git a/tests/auth.middleware.test.ts b/tests/auth.middleware.test.ts index bb08ef8..0ec5fcb 100644 --- a/tests/auth.middleware.test.ts +++ b/tests/auth.middleware.test.ts @@ -5,6 +5,93 @@ import { JwtVerify } from '../src/middlewares/jwt'; jest.mock('../src/models/user'); jest.mock('../src/middlewares/jwt'); +const parseCookie = (header: string | undefined, name: string): string | undefined => { + if (!header) return undefined; + const cookies = header.split(';').map((c) => c.trim().split('=')); + const cookie = cookies.find(([key]) => key === name); + return cookie?.[1]; +}; + +describe('authGuard middleware — cookie-based auth (Google OAuth)', () => { + const createResponse = () => ({ + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }); + + const createRequest = () => ({ + headers: {}, + }); + + beforeEach(() => { + jest.clearAllMocks(); + (JwtVerify as jest.Mock).mockReturnValue({ + id: 'user-id-1', + email: 'user@example.com', + }); + }); + + it('accepts a valid token from the cookie header', async () => { + const verifiedUser = { + _id: 'user-id-1', + email: 'user@example.com', + provider: 'google', + emailVerifiedAt: undefined, + }; + (User.findById as jest.Mock).mockResolvedValue(verifiedUser); + + const token = 'valid.jwt.token'; + const req = { + headers: { + cookie: `token=${token}; other=value`, + }, + }; + const res = createResponse(); + const next = jest.fn(); + + await authGuard(req as any, res as any, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(res.status).not.toHaveBeenCalled(); + expect((req as any).user).toBe(verifiedUser); + }); + + it('rejects a request with an invalid cookie token', async () => { + (JwtVerify as jest.Mock).mockImplementation(() => { + throw new Error('Invalid token'); + }); + + const req = { + headers: { + cookie: 'token=invalid.jwt.token', + }, + }; + const res = createResponse(); + const next = jest.fn(); + + await authGuard(req as any, res as any, next); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ + error: 'Unauthorized: Invalid token', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('rejects a request with an empty cookie header', async () => { + const req = createRequest(); + const res = createResponse(); + const next = jest.fn(); + + await authGuard(req as any, res as any, next); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ + error: 'Unauthorized: No token provided', + }); + expect(next).not.toHaveBeenCalled(); + }); +}); + // Regression coverage for issue #122: a valid JWT alone must not grant access to // protected routes. Unverified local accounts have to be blocked at the guard, // not only at the login controller. From c58efec6e8ec261a4a20b211f12b21fafa660289 Mon Sep 17 00:00:00 2001 From: Oyibe Date: Sun, 28 Jun 2026 19:44:10 +0100 Subject: [PATCH 3/4] Update src/validators/auth.validator.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/validators/auth.validator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validators/auth.validator.ts b/src/validators/auth.validator.ts index e58e5e6..c7fdab5 100644 --- a/src/validators/auth.validator.ts +++ b/src/validators/auth.validator.ts @@ -8,7 +8,7 @@ export const LoginSchema = z.object({ export type LoginInput = z.infer; export const SignupSchema = z.object({ - name: z.string().min(1, 'Name is required'), + name: z.string().trim().min(1, 'Name is required'), email: z.string().email('Invalid email format'), password: z.string().min(1, 'Password is required'), }); From 6ebfae19bfbbf893c84917ba42c7bf0d02479ef1 Mon Sep 17 00:00:00 2001 From: dubemoyibe-star Date: Sun, 28 Jun 2026 19:59:16 +0100 Subject: [PATCH 4/4] implmented code rabbit review changes --- src/middlewares/jwt.ts | 6 +----- tests/auth.middleware.test.ts | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/middlewares/jwt.ts b/src/middlewares/jwt.ts index 690b191..2fcdd85 100644 --- a/src/middlewares/jwt.ts +++ b/src/middlewares/jwt.ts @@ -6,9 +6,5 @@ export interface JwtPayload { } export const JwtVerify = (token: string): JwtPayload => { - try { - return jwt.verify(token, process.env.JWT_SECRET as string) as JwtPayload; - } catch (error) { - throw new Error('Invalid or expired token!'); - } + return jwt.verify(token, process.env.JWT_SECRET as string) as JwtPayload; }; diff --git a/tests/auth.middleware.test.ts b/tests/auth.middleware.test.ts index 0ec5fcb..f46e223 100644 --- a/tests/auth.middleware.test.ts +++ b/tests/auth.middleware.test.ts @@ -1,6 +1,7 @@ import { authGuard } from '../src/middlewares/auth'; import User from '../src/models/user'; import { JwtVerify } from '../src/middlewares/jwt'; +import jwt from 'jsonwebtoken'; jest.mock('../src/models/user'); jest.mock('../src/middlewares/jwt'); @@ -57,7 +58,7 @@ describe('authGuard middleware — cookie-based auth (Google OAuth)', () => { it('rejects a request with an invalid cookie token', async () => { (JwtVerify as jest.Mock).mockImplementation(() => { - throw new Error('Invalid token'); + throw new jwt.JsonWebTokenError('Invalid token'); }); const req = {