Skip to content

test: add tests for check-username-availability endpoint, improve testDb error handling - #18

Closed
Ahmadnaveedofficial wants to merge 25 commits into
10pshine-cohort-9:developfrom
Ahmadnaveedofficial:feature/backend/test-check-username-availability
Closed

test: add tests for check-username-availability endpoint, improve testDb error handling#18
Ahmadnaveedofficial wants to merge 25 commits into
10pshine-cohort-9:developfrom
Ahmadnaveedofficial:feature/backend/test-check-username-availability

Conversation

@Ahmadnaveedofficial

@Ahmadnaveedofficial Ahmadnaveedofficial commented Jul 24, 2026

Copy link
Copy Markdown

Overview

Added test coverage for the checkUsernameAvailability endpoint and improved error handling in test database helper functions.

What's Included

  • Added a new describe block in auth.test.ts covering:
    • Available username
    • Taken username
    • Validation errors (short username, invalid characters, missing query parameter)
  • Updated testDb.ts:
    • Wrapped connectTestDB and closeTestDB in try/catch blocks
    • Added Pino logging for clearer failure diagnostics

Summary by CodeRabbit

  • New Features

    • Added backend authentication and account-management capabilities, including registration, email OTP verification, login, logout, token refresh, password recovery, profile updates, and avatar uploads.
    • Added username availability checks, protected account operations, secure session handling, and health monitoring.
    • Added consistent validation, error responses, rate limiting, and support for email and image services.
  • Bug Fixes

    • Improved handling of invalid requests, expired sessions, upload errors, and unknown routes.
  • Tests

    • Added comprehensive integration coverage for authentication, account management, recovery, uploads, and authorization.
  • Chores

    • Removed the existing frontend application and its related configuration.

Ahmadnaveedofficial and others added 10 commits July 21, 2026 15:34
## Overview
This PR sets up the initial project structure for the Notes App (MERN stack) with TypeScript, including both backend and frontend scaffolding, dependency installation, and base configuration.

## Backend Setup (`/backend`)
- Initialized Node.js project with TypeScript configuration (`tsconfig.json`)
- Folder structure created: `src/config`, `src/controller`, `src/models`, `src/routes`, `src/middleware`, `src/utils`
- Core dependencies installed:
  - **Express** – REST API framework
  - **Mongoose** – MongoDB ODM
  - **bcrypt** – password hashing
  - **jsonwebtoken** – authentication (JWT)
  - **cookie-parser**, **cors**, **helmet**, **express-rate-limit** – security & middleware
  - **Pino / Pino-HTTP** – structured application logging
  - **Socket.IO** – real-time updates (optional feature)
  - **Multer** – file upload handling (for export/import feature)
  - **Zod** – schema validation
- Dev dependencies installed:
  - **TypeScript**, **ts-node-dev** – TS development workflow
  - **Mocha**, **Chai**, **Supertest**, **nyc** – backend unit testing & coverage
  - **ESLint**, **Prettier** – code quality and formatting
- Added `.env` and `.gitignore` (excludes `node_modules`, `.env`, `dist`, logs, coverage, etc.)

## Frontend Setup (`/frontend`)
- Scaffolded with **Vite + React + TypeScript**
- **Tailwind CSS v4** integrated via `@tailwindcss/vite`
- **shadcn/ui** initialized (Base UI, Nova preset) with path aliases (`@/*`) configured
- `.gitignore` updated to exclude environment files

## Notes
- No `node_modules` or `.env` files were committed (verified `.gitignore` working correctly)
- Backend `src/config`, `controller`, `models`, `routes`, `middleware`, `utils` folders are currently empty and will populate as features are implemented
- This is a scaffolding-only PR — no business logic or API endpoints implemented yet
…uth-and-user-test

## Overview
This PR implements complete user authentication for the Notes App, including OTP-based email verification, JWT access/refresh token flow, and full test coverage using Mocha/Chai/Supertest.

## What's Included

### Database
- **User model** (Mongoose + TypeScript): `name`, `username`, `email`, `password` (hashed via bcrypt), `avatar`, `isVerified`, OTP fields, refresh token, and password reset token fields
- Indexes on `email` and `username` for fast lookups
- Instance methods: `isPasswordCorrect`, `generateAccessToken`, `generateRefreshToken`

### Validation
- Zod schemas for every auth endpoint (signup, login, OTP verify/resend, update account, change/forgot/reset password) — all inputs validated at the boundary before hitting business logic

### Auth Flow
- **Register** → creates user (unverified) → generates 6-digit OTP → emails it via Nodemailer/SMTP
- **Verify OTP** → marks account as verified (login blocked until verified)
- **Resend OTP** → generates and sends a new code if the previous one expired
- **Login** → supports both email and username, issues short-lived access token + long-lived refresh token (JWT), sets httpOnly cookies
- **Refresh Token** → issues a new access token using a valid refresh token, with rotation
- **Logout** → clears refresh token from DB and cookies

### Account Management
- Get current user (`/me`)
- Update account details (name/username, with duplicate-username check)
- Update avatar (Cloudinary upload, old avatar auto-deleted)
- Change password (requires old password)
- Forgot/Reset password (crypto-generated reset token, hashed before storage, 15-min expiry, no user-enumeration leak)

### Infrastructure
- `asyncHandler` wrapper — no repeated try/catch in controllers
- Centralized `ApiError` (thrown on failure) / `ApiResponse` (returned on success) pattern, handled by a single global `errorHandler` middleware
- Pino structured logging for all key events (registration, login, errors, token refresh, etc.) and HTTP request/response logging via `pino-http`
- `verifyJWT` middleware protecting private routes
- Helmet, CORS (scoped to frontend origin), and rate limiting on all `/api` routes

## Endpoints Added

| Method | Route | Auth Required |
|---|---|---|
| POST | `/api/users/register` | No |
| POST | `/api/users/verify-otp` | No |
| POST | `/api/users/resend-otp` | No |
| POST | `/api/users/login` | No |
| POST | `/api/users/refresh-token` | No (uses refresh cookie) |
| POST | `/api/users/forgot-password` | No |
| POST | `/api/users/reset-password` | No |
| POST | `/api/users/logout` | ✅ |
| GET | `/api/users/me` | ✅ |
| PATCH | `/api/users/update-details` | ✅ |
| PATCH | `/api/users/update-avatar` | ✅ |
| POST | `/api/users/change-password` | ✅ |

## Security Notes
- Passwords hashed with bcrypt (10 salt rounds) via Mongoose `pre("save")` hook
- Access/refresh tokens signed with separate secrets, refresh tokens stored server-side and rotated on use
- Password reset tokens are hashed (SHA-256) before being stored — raw token never persisted
- `forgotPassword` returns the same response whether or not the email exists, to prevent user enumeration
- Sensitive fields (`password`, `refreshToken`, `otp`, `resetPasswordToken`) excluded from queries by default via `select: false`

## Testing
- Automated test suite (`tests/auth.test.ts`) using Mocha, Chai, Supertest, and Sinon (to stub outgoing emails)
- Covers: registration (success + validation failures + duplicates), OTP verification, login (email/username, unverified block, wrong password), and protected routes (auth check, logout, update details, change password)
- Manually verified end-to-end via Postman: register → verify OTP → login → access protected routes

## Notes for Reviewers
- Email sending requires SMTP credentials in `.env` — tests mock this via Sinon so they don't depend on a real inbox
- This PR does not include Notes CRUD yet — that will follow in a separate feature branch
…escript-version

## Bug
`npm run dev` was failing with `TypeError: Cannot read properties of undefined (reading 'fileExists')` due to `typescript` resolving to an unstable v7.x pre-release (via `^7.0.2` in package.json), which is incompatible with `ts-node`/`ts-node-dev`.

## Fix
Downgraded `typescript` to the stable `5.7.2` release.

## Testing
Verified `npm run dev` starts the server successfully without errors.
…o-logger-sensitive-data

## Bug
`pino-http` was logging the full request/response objects, including sensitive cookies (session tokens, JWT) and all headers, on every request. This is a security risk if logs are ever persisted or shipped to a log aggregator.

## Fix
Added custom serializers to `pinoHttp` config so only `method`, `url`, `remoteAddress` (request) and `statusCode` (response) are logged — cookies and headers are no longer included.

## Testing
Verified via `npm run dev` + hitting `/health` — logs are now concise and contain no sensitive data.
…e-error

## Summary
This pull request fixes the `next is not a function` error that occurred during user registration.

## Changes Made
- Updated the Mongoose `pre("save")` middleware in `user.model.ts`.
- Removed the unnecessary `next()` callback from the async pre-save hook.
- Ensured passwords are hashed correctly before saving.
- Improved compatibility with the current Mongoose version.

## Issue Fixed
- Fixed `TypeError: next is not a function` when calling the `POST /api/users/register` endpoint.
- User registration now completes successfully without throwing a 500 Internal Server Error.

## Testing
- ✅ Backend server started successfully.
- ✅ MongoDB connection verified.
- ✅ Tested `POST /api/users/register` using Postman.
- ✅ Password hashing works as expected.
- ✅ No runtime errors observed after the fix.
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request replaces the frontend scaffold with a backend authentication service. The backend adds Express routing, MongoDB users, JWT authentication, OTP verification, password recovery, avatar uploads, email delivery, error handling, and integration tests.

Changes

Backend authentication platform

Layer / File(s) Summary
Contracts and user data model
backend/src/constant.ts, backend/src/schema/*, backend/src/models/user.model.ts, backend/src/utils/Api*.ts, backend/src/types/express.d.ts
Adds validation schemas, API response types, authentication constants, Express request typing, and a Mongoose user model with password and JWT methods.
Authentication supporting services
backend/src/utils/*
Adds JWT hashing and rotation, OTP generation, SMTP email delivery, cookies, logging, and Cloudinary file operations.
Application bootstrap and middleware
backend/src/app.ts, backend/src/server.ts, backend/src/config/db.ts, backend/src/middleware/*, backend/package.json, backend/tsconfig.json, backend/.mocharc.json
Configures the Express service, database startup, security middleware, request logging, rate limiting, uploads, error handling, TypeScript, and test scripts.
User routes and controller flows
backend/src/routes/user.routes.ts, backend/src/controller/user.controller.ts
Adds registration, OTP verification, login, logout, refresh-token rotation, protected account operations, avatar updates, password changes, and password-reset endpoints.
Integration test environment and coverage
backend/tests/auth.test.ts, backend/tests/helpers/testDb.ts
Adds isolated MongoDB test helpers and integration coverage for authentication, account management, uploads, tokens, recovery, and validation.
Environment ignore rules
backend/.gitignore
Expands environment-file ignoring from .env to .env*.

Frontend scaffold removal

Layer / File(s) Summary
Frontend project removal
frontend/package.json, frontend/index.html, frontend/src/*, frontend/*.json, frontend/*.config.*, frontend/README.md, frontend/.gitignore
Removes the frontend package, Vite and TypeScript configuration, React entrypoints, styling, UI components, utilities, documentation, and ignore rules.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Express
  participant UserController
  participant MongoDB
  participant EmailService
  Client->>Express: Send authentication request
  Express->>UserController: Validate and dispatch request
  UserController->>MongoDB: Create or query user state
  UserController->>EmailService: Send OTP or reset email
  UserController-->>Client: Return response and authentication cookies
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the added endpoint tests and improved test database error handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Warning

⚠️ This pull request shows signs of AI-generated slop (redundant_comments). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

…eRabbit security review (login enumeration, PII logging, test DB safety guard, CORS production fallback)
…eRabbit security review (login enumeration, PII logging, test DB safety guard, CORS production fallback)
- Type user variable explicitly in registerUser

- Handle duplicate-key race condition in updateAccountDetails

- Guard invalid ObjectId and wrap findById in auth middleware

- Exclude sensitive fields from req.user projection

- Hide internal error messages on unexpected 500s

- Map Multer errors to proper 4xx responses

- Wrap bcrypt hashing in try/catch in pre-save hook

- Add try/catch + logging to clearCollections test helper

- Hash refresh tokens at rest instead of storing plaintext

- Validate SMTP config and derive secure flag from port

- Fix extractPublicId to preserve folder-prefixed public_ids

- Use crypto.randomInt for OTP generation

- Migrate to Zod 4 top-level z.email()

- Reject disallowed file types with ApiError 400 in multer

- Keep .env.example trackable in .gitignore

- Invalidate refresh token on password change/reset

- Fix avatar upload race: cleanup temp file, reorder save/delete

- Add tests for resend-otp, forgot/reset-password, refresh-token, avatar upload

- Use real OTP instead of hardcoded value in lockout test
… validation, and error-handling fixes per CodeRabbit review
@Ahmadnaveedofficial
Ahmadnaveedofficial marked this pull request as draft July 31, 2026 10:38
@Ahmadnaveedofficial
Ahmadnaveedofficial force-pushed the feature/backend/test-check-username-availability branch from 78b0070 to b7426f5 Compare July 31, 2026 18:01
@Ahmadnaveedofficial
Ahmadnaveedofficial marked this pull request as ready for review August 1, 2026 12:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (10)
backend/src/controller/user.controller.ts (2)

40-57: 🚀 Performance & Scalability | 🔵 Trivial

Add rate limiting to the username lookup endpoint.

checkUsernameAvailability runs an unauthenticated database query per request. An attacker can enumerate registered usernames and can also use the endpoint to generate load. Apply a per-IP rate limit on this route, and consider a short cache for negative results.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/controller/user.controller.ts` around lines 40 - 57, Add per-IP
rate limiting to the route invoking checkUsernameAvailability, limiting
unauthenticated username availability queries before the database lookup; use
the project’s existing rate-limiter middleware or conventions, and optionally
cache short-lived unavailable/negative lookup results without changing the
response contract.

184-192: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider hashing the OTP and comparing it with a timing-safe function.

Line 184 compares a stored plaintext OTP with a plain !==. The OTP is a bearer credential for account verification. A database read exposes valid OTPs directly. Store a hash of the OTP, and compare with crypto.timingSafeEqual on equal-length buffers. The existing lockout limits brute force, so this is a posture improvement rather than an active exploit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/controller/user.controller.ts` around lines 184 - 192, Update the
OTP generation, persistence, and verification flow in the user controller to
store only a cryptographic hash of the OTP, then compare the submitted OTP hash
with the stored hash using crypto.timingSafeEqual after ensuring equal-length
buffers. Preserve the existing OTP expiry, attempt counting, lockout, and
cleanup behavior, and handle missing or malformed stored values without
throwing.
backend/tests/auth.test.ts (1)

218-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Import MAX_OTP_ATTEMPTS instead of hardcoding 5.

The loop count and the test title both encode the value 5. If the constant in backend/src/constant.ts changes, then this test fails for an unrelated reason.

♻️ Proposed change
-    it("should lock out user after 5 failed OTP attempts", async () => {
+    it("should lock out user after MAX_OTP_ATTEMPTS failed OTP attempts", async () => {
@@
-      // Fail 5 times with wrong OTP
-      for (let i = 0; i < 5; i++) {
+      // Fail MAX_OTP_ATTEMPTS times with wrong OTP
+      for (let i = 0; i < MAX_OTP_ATTEMPTS; i++) {

Add the import at the top of the file:

import { MAX_OTP_ATTEMPTS } from "../src/constant";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/auth.test.ts` around lines 218 - 246, Import MAX_OTP_ATTEMPTS
from the constants module in the auth tests, then use it for the failed-OTP loop
count and update the test title to avoid hardcoding 5. Keep the existing lockout
assertions and request flow unchanged.
backend/.gitignore (1)

5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Confirm .env* does not hide an example env file.

.env* on line 5 ignores every file that starts with .env, including .env.example. If the project intends to commit a documented example file, add a negation pattern.

📝 Proposed fix to keep an example file trackable
 .env*
 .env.local
 .env.*.local
+!.env.example
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/.gitignore` at line 5, Update the backend .gitignore patterns so the
broad .env* ignore remains in place while explicitly unignoring .env.example,
allowing the documented example environment file to be tracked.
backend/src/app.ts (1)

56-62: 🧹 Nitpick | 🔵 Trivial

Verify trust proxy setting if deployed behind a reverse proxy.

The rate limiter at Line 57 uses the default IP-based key generator. If the app runs behind a reverse proxy or load balancer, configure app.set("trust proxy", ...) with the correct value. Otherwise express-rate-limit can rate-limit incorrectly or reject requests due to unexpected X-Forwarded-For headers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/app.ts` around lines 56 - 62, Configure Express’s trust proxy
setting before the rate limiter middleware in app, using the deployment’s actual
proxy hop configuration so the default IP-based key generator handles
X-Forwarded-For correctly. Keep the existing limiter options and app.use("/api",
limiter) behavior unchanged.
backend/src/middleware/multer.middleware.ts (1)

23-36: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider validating file content, not just the client-supplied MIME type.

fileFilter trusts file.mimetype, which the client sets and can spoof. Combined with the extension fix above, add an extension allowlist or a magic-byte check (for example with the file-type package) for stronger validation against disguised uploads.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/middleware/multer.middleware.ts` around lines 23 - 36, Strengthen
validation in fileFilter instead of relying only on the client-supplied
file.mimetype. Add validation using an extension allowlist or, preferably,
magic-byte detection via the file-type package, and reject files whose content
is not an allowed JPEG, PNG, or WebP type while preserving the existing ApiError
response.
backend/src/middleware/auth.middleware.ts (1)

23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider an explicit check for ACCESS_TOKEN_SECRET.

The type assertion process.env.ACCESS_TOKEN_SECRET as string hides a missing secret. If the variable is undefined, jwt.verify throws, and the generic catch turns this into "Invalid or expired access token", masking a server misconfiguration as a client error. db.ts validates MONGODB_URL explicitly before use; apply the same pattern here for clearer diagnostics.

🔧 Proposed fix
+    const accessTokenSecret = process.env.ACCESS_TOKEN_SECRET;
+    if (!accessTokenSecret) {
+      logger.error("ACCESS_TOKEN_SECRET is not defined in env");
+      throw new ApiError(500, "Server configuration error");
+    }
+
     let decodedToken: DecodedToken;
     try {
       decodedToken = jwt.verify(
         token,
-        process.env.ACCESS_TOKEN_SECRET as string,
+        accessTokenSecret,
       ) as DecodedToken;
     } catch {
       throw new ApiError(401, "Invalid or expired access token");
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/middleware/auth.middleware.ts` around lines 23 - 31, Validate
ACCESS_TOKEN_SECRET explicitly before the jwt.verify call in the
token-authentication flow, following the existing MONGODB_URL validation pattern
in db.ts. If the secret is missing, surface a clear server configuration error
outside the generic invalid-token catch; otherwise pass the validated secret to
jwt.verify and preserve the existing 401 handling for invalid or expired tokens.
backend/src/schema/checkUsernameSchema.ts (1)

3-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the username length and character rules into one shared definition.

The username min length, max length, and /^[a-zA-Z0-9_]+$/ regex are defined independently in four places. Extract a shared constant or Zod schema fragment so a future rule change only requires one edit.

  • backend/src/schema/checkUsernameSchema.ts#L3-L12: import a shared usernameSchema fragment instead of redefining min/max/regex here.
  • backend/src/schema/signupSchema.ts#L8-L15: reuse the same shared usernameSchema fragment for the username field.
  • backend/src/schema/updateAccountSchema.ts#L10-L18: reuse the shared usernameSchema fragment, wrapped in .optional().
  • backend/src/models/user.model.ts#L42-L47: reference a shared exported regex/length constants (for example from constant.ts) for match, minlength, and maxlength, instead of a fourth independent copy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/schema/checkUsernameSchema.ts` around lines 3 - 12, Extract the
shared username constraints into one reusable definition. In
backend/src/schema/checkUsernameSchema.ts:3-12,
backend/src/schema/signupSchema.ts:8-15, and
backend/src/schema/updateAccountSchema.ts:10-18, reuse the shared usernameSchema
fragment, applying .optional() only in updateAccountSchema. In
backend/src/models/user.model.ts:42-47, replace the duplicated regex and length
values with shared exported constants from the chosen common module.
backend/src/utils/cloudinary.ts (2)

52-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Fix resource_type mismatch between upload and delete.

uploadOnCloudinary (Line 59) uploads with resource_type: "auto", letting Cloudinary detect image, video, or raw resources. deleteFromCloudinary (Line 90) always calls destroy with resource_type: "image".

For any non-image asset uploaded through this utility, deletion will fail. The failure is only logged as a warning (Line 98-101), not thrown, so the caller sees a successful-looking flow while the asset is orphaned on Cloudinary. Current callers only pass avatar images, so this isn't exercised yet, but the function is exported as a general-purpose utility and this gap will resurface if it's reused for other resource types.

Track and pass the actual resource_type from the upload response instead of hardcoding it.

♻️ Proposed fix
 export const uploadOnCloudinary = async (
   localFilePath: string,
-): Promise<string | null> => {
+): Promise<{ url: string; resourceType: string } | null> => {
   if (!localFilePath) return null;

   try {
     const response = await cloudinary.uploader.upload(localFilePath, {
       resource_type: "auto",
     });

     removeLocalFile(localFilePath);
     logger.info({ url: response.secure_url }, "File uploaded to Cloudinary");

-    return response.secure_url;
+    return { url: response.secure_url, resourceType: response.resource_type };
   } catch (error) {
     removeLocalFile(localFilePath);
     logger.error(error, "Cloudinary upload failed");
     return null;
   }
 };

 export const deleteFromCloudinary = async (
   fileUrl: string,
+  resourceType: string = "image",
 ): Promise<boolean> => {
   ...
     const result = await cloudinary.uploader.destroy(publicId, {
-      resource_type: "image",
+      resource_type: resourceType,
     });

Note: this changes uploadOnCloudinary's return shape, so update callers (for example, updateAvatar in user.controller.ts) accordingly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/utils/cloudinary.ts` around lines 52 - 107, Update
uploadOnCloudinary to return the uploaded asset’s secure URL together with the
resource_type reported by Cloudinary, then update deleteFromCloudinary to accept
and pass that recorded resource type to cloudinary.uploader.destroy instead of
hardcoding "image"; adjust callers such as updateAvatar to use the new return
shape while preserving existing success and failure behavior.

5-9: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Validate Cloudinary env vars at startup, consistent with sendEmail.ts.

cloudinary.config() casts process.env.CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET with as string without checking they are set. If any variable is missing, the config silently holds undefined values. Failures only surface later as generic upload/delete errors, without indicating the missing credential is the root cause.

sendEmail.ts validates its required SMTP env vars and throws immediately if any are missing. Apply the same fail-fast pattern here for consistent, diagnosable startup behavior.

♻️ Proposed fix
 import { v2 as cloudinary } from "cloudinary";
 import fs from "fs";
 import logger from "./logger";

+const { CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, CLOUDINARY_API_SECRET } = process.env;
+
+if (!CLOUDINARY_CLOUD_NAME || !CLOUDINARY_API_KEY || !CLOUDINARY_API_SECRET) {
+  throw new Error(
+    "Missing required Cloudinary configuration: CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET must all be set",
+  );
+}
+
 cloudinary.config({
-  cloud_name: process.env.CLOUDINARY_CLOUD_NAME as string,
-  api_key: process.env.CLOUDINARY_API_KEY as string,
-  api_secret: process.env.CLOUDINARY_API_SECRET as string,
+  cloud_name: CLOUDINARY_CLOUD_NAME,
+  api_key: CLOUDINARY_API_KEY,
+  api_secret: CLOUDINARY_API_SECRET,
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/utils/cloudinary.ts` around lines 5 - 9, Update the Cloudinary
initialization around cloudinary.config to validate CLOUDINARY_CLOUD_NAME,
CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET before configuration, following
the fail-fast validation pattern used by sendEmail.ts. Throw a clear startup
error identifying missing credentials, and only pass validated values to
cloudinary.config instead of relying on unchecked string casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/src/middleware/multer.middleware.ts`:
- Around line 17-20: Update the filename callback in multer.diskStorage to stop
incorporating file.originalname into the stored path. Generate the name solely
from server-controlled data, and if retaining an extension, extract it with
path.extname and accept it only when it matches an explicit allowed-extension
list; otherwise use no extension.

In `@backend/src/models/user.model.ts`:
- Around line 106-114: Update the password-hashing error handling in the
userSchema pre-save hook to rethrow the caught error after any existing logging,
ensuring Mongoose aborts the save instead of persisting the unhashed password.

In `@backend/tests/helpers/testDb.ts`:
- Around line 4-13: Update connectTestDB to validate MONGODB_URL before calling
mongoose.connect, rejecting a missing value with a clear error; after the
connection opens, verify the connected database name is exactly the permitted
test database name and fail immediately otherwise. Keep the existing connection
error logging and rethrow behavior, and reuse the database-name guard enforced
by closeTestDB if available.
- Around line 15-31: Update closeTestDB so mongoose.connection.close() always
executes in a finally block, including when the database-name guard or
dropDatabase() fails. Keep the guard validation and error logging behavior
intact, and ensure failures are still propagated after cleanup.

---

Nitpick comments:
In `@backend/.gitignore`:
- Line 5: Update the backend .gitignore patterns so the broad .env* ignore
remains in place while explicitly unignoring .env.example, allowing the
documented example environment file to be tracked.

In `@backend/src/app.ts`:
- Around line 56-62: Configure Express’s trust proxy setting before the rate
limiter middleware in app, using the deployment’s actual proxy hop configuration
so the default IP-based key generator handles X-Forwarded-For correctly. Keep
the existing limiter options and app.use("/api", limiter) behavior unchanged.

In `@backend/src/controller/user.controller.ts`:
- Around line 40-57: Add per-IP rate limiting to the route invoking
checkUsernameAvailability, limiting unauthenticated username availability
queries before the database lookup; use the project’s existing rate-limiter
middleware or conventions, and optionally cache short-lived unavailable/negative
lookup results without changing the response contract.
- Around line 184-192: Update the OTP generation, persistence, and verification
flow in the user controller to store only a cryptographic hash of the OTP, then
compare the submitted OTP hash with the stored hash using crypto.timingSafeEqual
after ensuring equal-length buffers. Preserve the existing OTP expiry, attempt
counting, lockout, and cleanup behavior, and handle missing or malformed stored
values without throwing.

In `@backend/src/middleware/auth.middleware.ts`:
- Around line 23-31: Validate ACCESS_TOKEN_SECRET explicitly before the
jwt.verify call in the token-authentication flow, following the existing
MONGODB_URL validation pattern in db.ts. If the secret is missing, surface a
clear server configuration error outside the generic invalid-token catch;
otherwise pass the validated secret to jwt.verify and preserve the existing 401
handling for invalid or expired tokens.

In `@backend/src/middleware/multer.middleware.ts`:
- Around line 23-36: Strengthen validation in fileFilter instead of relying only
on the client-supplied file.mimetype. Add validation using an extension
allowlist or, preferably, magic-byte detection via the file-type package, and
reject files whose content is not an allowed JPEG, PNG, or WebP type while
preserving the existing ApiError response.

In `@backend/src/schema/checkUsernameSchema.ts`:
- Around line 3-12: Extract the shared username constraints into one reusable
definition. In backend/src/schema/checkUsernameSchema.ts:3-12,
backend/src/schema/signupSchema.ts:8-15, and
backend/src/schema/updateAccountSchema.ts:10-18, reuse the shared usernameSchema
fragment, applying .optional() only in updateAccountSchema. In
backend/src/models/user.model.ts:42-47, replace the duplicated regex and length
values with shared exported constants from the chosen common module.

In `@backend/src/utils/cloudinary.ts`:
- Around line 52-107: Update uploadOnCloudinary to return the uploaded asset’s
secure URL together with the resource_type reported by Cloudinary, then update
deleteFromCloudinary to accept and pass that recorded resource type to
cloudinary.uploader.destroy instead of hardcoding "image"; adjust callers such
as updateAvatar to use the new return shape while preserving existing success
and failure behavior.
- Around line 5-9: Update the Cloudinary initialization around cloudinary.config
to validate CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET
before configuration, following the fail-fast validation pattern used by
sendEmail.ts. Throw a clear startup error identifying missing credentials, and
only pass validated values to cloudinary.config instead of relying on unchecked
string casts.

In `@backend/tests/auth.test.ts`:
- Around line 218-246: Import MAX_OTP_ATTEMPTS from the constants module in the
auth tests, then use it for the failed-OTP loop count and update the test title
to avoid hardcoding 5. Keep the existing lockout assertions and request flow
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fb15c5e7-550d-4711-95cd-a07ccbddca7e

📥 Commits

Reviewing files that changed from the base of the PR and between 258fb67 and b7426f5.

⛔ Files ignored due to path filters (7)
  • backend/package-lock.json is excluded by !**/package-lock.json
  • frontend/package-lock.json is excluded by !**/package-lock.json
  • frontend/public/favicon.svg is excluded by !**/*.svg
  • frontend/public/icons.svg is excluded by !**/*.svg
  • frontend/src/assets/hero.png is excluded by !**/*.png
  • frontend/src/assets/react.svg is excluded by !**/*.svg
  • frontend/src/assets/vite.svg is excluded by !**/*.svg
📒 Files selected for processing (50)
  • backend/.gitignore
  • backend/.mocharc.json
  • backend/package.json
  • backend/src/app.ts
  • backend/src/config/db.ts
  • backend/src/constant.ts
  • backend/src/controller/user.controller.ts
  • backend/src/middleware/auth.middleware.ts
  • backend/src/middleware/errorHandler.ts
  • backend/src/middleware/multer.middleware.ts
  • backend/src/models/user.model.ts
  • backend/src/routes/user.routes.ts
  • backend/src/schema/changePasswordSchema.ts
  • backend/src/schema/checkUsernameSchema.ts
  • backend/src/schema/forgotPasswordSchema.ts
  • backend/src/schema/loginSchema.ts
  • backend/src/schema/otpSchema.ts
  • backend/src/schema/refreshTokenSchema.ts
  • backend/src/schema/resetPasswordSchema.ts
  • backend/src/schema/signupSchema.ts
  • backend/src/schema/updateAccountSchema.ts
  • backend/src/server.ts
  • backend/src/types/express.d.ts
  • backend/src/utils/ApiError.ts
  • backend/src/utils/ApiResponse.ts
  • backend/src/utils/asyncHandler.ts
  • backend/src/utils/cloudinary.ts
  • backend/src/utils/cookieOptions.ts
  • backend/src/utils/generateOtp.ts
  • backend/src/utils/generateTokens.ts
  • backend/src/utils/logger.ts
  • backend/src/utils/sendEmail.ts
  • backend/tests/auth.test.ts
  • backend/tests/helpers/testDb.ts
  • backend/tsconfig.json
  • frontend/.gitignore
  • frontend/README.md
  • frontend/components.json
  • frontend/eslint.config.js
  • frontend/index.html
  • frontend/package.json
  • frontend/src/App.tsx
  • frontend/src/components/ui/button.tsx
  • frontend/src/index.css
  • frontend/src/lib/utils.ts
  • frontend/src/main.tsx
  • frontend/tsconfig.app.json
  • frontend/tsconfig.json
  • frontend/tsconfig.node.json
  • frontend/vite.config.ts
💤 Files with no reviewable changes (15)
  • frontend/src/main.tsx
  • frontend/src/lib/utils.ts
  • frontend/components.json
  • frontend/tsconfig.json
  • frontend/.gitignore
  • frontend/eslint.config.js
  • frontend/index.html
  • frontend/src/components/ui/button.tsx
  • frontend/src/App.tsx
  • frontend/vite.config.ts
  • frontend/package.json
  • frontend/src/index.css
  • frontend/tsconfig.node.json
  • frontend/README.md
  • frontend/tsconfig.app.json

Comment thread backend/src/middleware/multer.middleware.ts
Comment thread backend/src/models/user.model.ts
Comment thread backend/tests/helpers/testDb.ts
Comment thread backend/tests/helpers/testDb.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant