Skip to content

feat: harden developer auth (JWT expiry, OTP hashing, brute-force pro… #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yash-pouranik merged 4 commits into from
Mar 4, 2026
Merged

feat: harden developer auth (JWT expiry, OTP hashing, brute-force pro… #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
042d25e
feat: harden developer auth (JWT expiry, OTP hashing, brute-force pro…
yash-pouranik Mar 4, 2026
e14b3ec
fix: address PR review comments (authLimiter order, crypto.randomInt,…
yash-pouranik Mar 4, 2026
d717f23
fixed parameter issue in sendotp-method
yash-pouranik Mar 4, 2026
f117d4e
Update backend/utils/emailService.js
yash-pouranik Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions backend/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
# ── Stage: Production ─────────────────────────────────────────────────────────
# Stage: Production
FROM node:20-alpine

# Set working directory
WORKDIR /app

# Install dependencies first (layer caching)
COPY package*.json ./
COPY --chown=node:node package*.json ./
RUN npm ci --omit=dev

# Copy source
COPY . .
COPY --chown=node:node . .

# Switch to non-root user (security best practice)
USER node

# Expose backend port
EXPOSE 1234
Expand Down
7 changes: 6 additions & 1 deletion backend/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const dashboardLimiter = rateLimit({
skip: (req) => process.env.NODE_ENV === 'development',
});

// FIX 3: Strict limiter for auth endpoints (login / register)
const { authLimiter } = require('./middleware/auth_limiter');


const adminWhitelist = ['https://urbackend.bitbros.in'];

Expand Down Expand Up @@ -74,7 +77,9 @@ const storageRoute = require('./routes/storage');
const schemaRoute = require('./routes/schemas');

// ROUTES SETUP
app.use('/api/auth', dashboardLimiter, authRoute); // Developer Auth
app.use('/api/auth/login', authLimiter); // Strict limiter on login
app.use('/api/auth/register', authLimiter); // Strict limiter on register
app.use('/api/auth', dashboardLimiter, authRoute); // Developer Auth (general)
app.use('/api/projects', dashboardLimiter, projectRoute); // Project Mgmt
app.use('/api/userAuth', limiter, logger, userAuthRoute);
app.use('/api/data', limiter, cors(adminCorsOptions), logger, dataRoute);
Expand Down
142 changes: 111 additions & 31 deletions backend/controllers/auth.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,53 @@ const bcrypt = require("bcryptjs");
const z = require("zod");
const jwt = require("jsonwebtoken");
const sendOtp = require("../utils/emailService");
const crypto = require("crypto");
const {
loginSchema,
changePasswordSchema,
deleteAccountSchema,
onlyEmailSchema
onlyEmailSchema,
resetPasswordSchema,
verifyOtpSchema
} = require("../utils/input.validation");

const JWT_EXPIRES_IN = '7d';
const OTP_MAX_ATTEMPTS = 5;

async function createAndStoreOtp(userId) {
const otp = crypto.randomInt(100000, 1000000).toString();

// Delete any existing OTP for this user
await otpSchema.deleteOne({ userId });

// Hash OTP before storing
const salt = await bcrypt.genSalt(10);
const hashedOtp = await bcrypt.hash(otp, salt);

await new otpSchema({ userId, otp: hashedOtp }).save();
return otp;
}

// HELPER: Validate OTP with attempt tracking
async function validateOtp(userId, passedOtp) {
const otpDoc = await otpSchema.findOne({ userId });
if (!otpDoc) throw { status: 400, message: "No OTP found. Please request a new one." };

if (otpDoc.attempts >= OTP_MAX_ATTEMPTS) {
await otpDoc.deleteOne();
throw { status: 429, message: "Too many incorrect attempts. Please request a new OTP." };
}

const isMatch = await bcrypt.compare(passedOtp.toString(), otpDoc.otp);
if (!isMatch) {
otpDoc.attempts += 1;
await otpDoc.save();
const remaining = OTP_MAX_ATTEMPTS - otpDoc.attempts;
throw { status: 400, message: `Incorrect OTP. ${remaining} attempt(s) remaining.` };
Comment thread
yash-pouranik marked this conversation as resolved.
}

return otpDoc;
}

module.exports.register = async (req, res) => {
try {
Expand Down Expand Up @@ -47,19 +86,20 @@ module.exports.login = async (req, res) => {
const validPass = await bcrypt.compare(password, dev.password);
if (!validPass) return res.status(400).json({ error: "Invalid password" });

const token = jwt.sign({ _id: dev._id, isVerified: dev.isVerified }, process.env.JWT_SECRET);
// FIX 1: JWT now expires in 7 days
const token = jwt.sign(
{ _id: dev._id, isVerified: dev.isVerified },
process.env.JWT_SECRET,
{ expiresIn: JWT_EXPIRES_IN }
);
res.json({ token });
} catch (err) {
console.log("LOGIN ERROR CAUGHT:", err.message);
console.log("Is ZodError?", err instanceof z.ZodError);

if (err instanceof z.ZodError) {
return res.status(400).json({
error: "Validation Failed",
details: err.issues
});
}

console.error("Server Error:", err);
res.status(500).json({ error: "Internal Server Error" });
}
Expand Down Expand Up @@ -114,55 +154,95 @@ module.exports.deleteAccount = async (req, res) => {
module.exports.sendOtp = async (req, res) => {
try {
const { email } = onlyEmailSchema.parse(req.body);
const otp = Math.floor(100000 + Math.random() * 900000);
// Secure: OTP logging removed

const existingUser = await Developer.findOne({ email });
if (!existingUser) return res.status(400).json({ error: "User not found" });

if (existingUser.isVerified) return res.status(400).json({ error: "User already verified" });

const existingOtp = await otpSchema.findOne({ userId: existingUser._id });
if (existingOtp) {
await existingOtp.deleteOne();
}
const newOtp = new otpSchema(
{
userId: existingUser._id,
otp
});
newOtp.save();
await sendOtp(email, otp);
const otp = await createAndStoreOtp(existingUser._id);

await sendOtp(email, otp); // Send raw OTP to user's email
res.json({ message: "OTP sent successfully" });
} catch (err) {
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
res.status(500).json({ error: "Internal Server Error" });
}
}



module.exports.verifyOtp = async (req, res) => {
try {
const { email } = onlyEmailSchema.parse(req.body);
const { otp } = req.body;
const existingUser = await Developer.findOne({ email }).sort({ createdAt: -1 });
if (!existingUser) return res.status(400).json({ error: "User not found" });
const { email, otp } = verifyOtpSchema.parse(req.body);

const existingOtp = await otpSchema.findOne({ userId: existingUser._id });
if (!existingOtp) return res.status(400).json({ error: "You havn't requested an OTP" });
const existingUser = await Developer.findOne({ email });
if (!existingUser) return res.status(400).json({ error: "User not found" });

// Secure: OTP verification logging removed
if (existingOtp.otp !== otp) return res.status(400).json({ error: "Incorrect OTP" });
const otpDoc = await validateOtp(existingUser._id, otp);

await existingOtp.deleteOne();
await otpDoc.deleteOne();
existingUser.isVerified = true;
await existingUser.save();

// Generate new token with isVerified: true
const token = jwt.sign({ _id: existingUser._id, isVerified: existingUser.isVerified }, process.env.JWT_SECRET);
// FIX 1: JWT with expiry
const token = jwt.sign(
{ _id: existingUser._id, isVerified: true },
process.env.JWT_SECRET,
{ expiresIn: JWT_EXPIRES_IN }
);

res.status(200).json({ message: "OTP verified successfully", token });
} catch (err) {
if (err.status) return res.status(err.status).json({ error: err.message });
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
res.status(500).json({ error: "Internal Server Error" });
}
}


// FIX 5: Forgot Password — generate + send reset OTP
module.exports.forgotPassword = async (req, res) => {
try {
const { email } = onlyEmailSchema.parse(req.body);

const dev = await Developer.findOne({ email });
// Return same message regardless of whether email exists (prevents user enumeration)
if (!dev) return res.status(200).json({ message: "If this email is registered, an OTP has been sent." });

const otp = await createAndStoreOtp(dev._id);

await sendOtp(email, otp, { subject: "Password Reset OTP — urBackend" });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
res.status(200).json({ message: "If this email is registered, an OTP has been sent." });
} catch (err) {
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
res.status(500).json({ error: "Internal Server Error" });
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}


// FIX 5: Reset Password — verify OTP then set new password
module.exports.resetPassword = async (req, res) => {
try {
const { email, otp, newPassword } = resetPasswordSchema.parse(req.body);

const dev = await Developer.findOne({ email });
if (!dev) return res.status(400).json({ error: "User not found" });

const otpDoc = await validateOtp(dev._id, otp);

// OTP matched — update password
await otpDoc.deleteOne();
const salt = await bcrypt.genSalt(10);
dev.password = await bcrypt.hash(newPassword, salt);
await dev.save();

res.status(200).json({ message: "Password reset successfully. Please log in with your new password." });
} catch (err) {
if (err.status) return res.status(err.status).json({ error: err.message });
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
Comment on lines +245 to +246

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Zod v4 ZodError issues vs errors property

💡 Result:

In Zod v4, the canonical array of validation problems on a ZodError is:

  • error.issues: ZodIssue[]

The old v3 property:

  • error.errors

is no longer reliable in v4 (many users hit it as “removed” during v3→v4 migration), and the expected replacement is to read error.issues instead. [1], [2]

Fix: replace err.errors with err.issues.

If you need temporary compatibility (supporting both v3 + v4 in shared code), use:

const issues = (err as any).issues ?? (err as any).errors ?? [];

Sources: Zod v4 migration/changelog (notes error-handling changes and uses error.issues) [3], plus migration discussions noting .errors → .issues [1], [2].

References: [1] [2] [3]

🏁 Script executed:

# Find and examine the auth.controller.js file
find . -name "auth.controller.js" -type f

Repository: yash-pouranik/urBackend

Length of output: 107

🏁 Script executed:

# Examine the auth.controller.js file, focusing on lines around 64 and 254-255
cat -n ./backend/controllers/auth.controller.js | sed -n '60,70p;250,260p'

Repository: yash-pouranik/urBackend

Length of output: 887

Change line 254 to use err.issues for Zod v4 compatibility.

Line 254 uses err.errors, which is the deprecated Zod v3 property and unreliable in Zod v4. The canonical validation details array in Zod v4 is err.issues, already used correctly on line 64. Update this handler for consistency:

Suggested fix 
-        if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
+        if (err instanceof z.ZodError) return res.status(400).json({ error: err.issues });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
if (err instanceof z.ZodError) return res.status(400).json({ error: err.issues });
console.error(err);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/controllers/auth.controller.js` around lines 254 - 255, Update the
Zod error handling in the auth controller to use the Zod v4 property: replace
the deprecated err.errors with err.issues in the catch branch that checks "if
(err instanceof z.ZodError)" (the same pattern used earlier on line 64); return
res.status(400).json({ error: err.issues }) so validation responses are
consistent and compatible with Zod v4.

res.status(500).json({ error: "Internal Server Error" });
}
Expand Down
13 changes: 13 additions & 0 deletions backend/middleware/auth_limiter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const rateLimit = require('express-rate-limit');

// Strict 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." },
skip: (req) => process.env.NODE_ENV === 'development',
standardHeaders: true,
legacyHeaders: false,
});
Comment on lines +4 to +11

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for rate-limit store configurations and dependencies
echo "=== Searching for rate-limit store references ==="
rg -n "express-rate-limit|rate-limit-redis|RedisStore|store:" backend --max-count=20

echo -e "\n=== Checking package.json for rate-limit related dependencies ==="
cat package.json | grep -A 10 '"dependencies"' | grep -i "rate-limit\|redis"

echo -e "\n=== Looking for Redis configuration ==="
rg -n "redis|REDIS" --max-count=20

echo -e "\n=== Checking for multi-instance/deployment configs ==="
ls -la | grep -E "docker|compose|kubernetes|helm|deploy"
find . -maxdepth 2 -type f \( -name "docker-compose*" -o -name "Dockerfile" -o -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | head -10

Repository: yash-pouranik/urBackend

Length of output: 5703

🏁 Script executed:

#!/bin/bash
echo "=== auth_limiter.js (lines 1-20) ==="
head -20 backend/middleware/auth_limiter.js

echo -e "\n=== api_usage.js (lines 1-20) ==="
head -20 backend/middleware/api_usage.js

echo -e "\n=== app.js rateLimit usage (line 13 context) ==="
sed -n '10,30p' backend/app.js

echo -e "\n=== redis.js config ==="
cat backend/config/redis.js

Repository: yash-pouranik/urBackend

Length of output: 2902

Integrate Redis store with rate-limiters for multi-instance deployments.

The middleware currently uses the default in-memory store. While Redis is already configured in the project (backend/config/redis.js, docker-compose), the rate-limiters (authLimiter, api_usage, and dashboardLimiter) bypass it. In production with multiple instances, each process maintains separate counters, allowing distributed clients to bypass limits.

Pass the Redis client to express-rate-limit via the store option. This requires installing rate-limit-redis and integrating it with the existing Redis configuration:

Example fix for auth_limiter.js 
const rateLimit = require('express-rate-limit');
const RedisStore = require('rate-limit-redis');
const redis = require('../config/redis');

const authLimiter = rateLimit({
    store: new RedisStore({
        client: redis,
        prefix: 'rl:auth:'
    }),
    windowMs: 15 * 60 * 1000,
    max: 10,
    message: { error: "Too many attempts. Please try again in 15 minutes." },
    skip: (req) => process.env.NODE_ENV === 'development',
    standardHeaders: true,
    legacyHeaders: false,
});

module.exports = { authLimiter };

Apply the same pattern to api_usage.js and the dashboard limiter in app.js.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/middleware/auth_limiter.js` around lines 4 - 11, The current
rate-limiters (authLimiter, api_usage, and dashboardLimiter) use the default
in-memory store and must be switched to a shared Redis store for multi-instance
deployments; install and require rate-limit-redis, import the existing Redis
client (from backend/config/redis.js), and pass a new RedisStore instance as the
store option when creating each limiter (e.g., set store: new RedisStore({
client: redis, prefix: 'rl:auth:' }) for authLimiter), keeping other options
(windowMs, max, message, skip, standardHeaders, legacyHeaders) unchanged; apply
the same pattern to api_usage and the dashboardLimiter in app.js so all limiters
use the centralized Redis client.


module.exports = { authLimiter };
6 changes: 4 additions & 2 deletions backend/models/otp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
const mongoose = require('mongoose');

const otpSchema = new mongoose.Schema({
userId: { type: mongoose.Schema.Types.ObjectId, required: true, ref: 'User' },
otp: { type: String, required: true },
userId: { type: mongoose.Schema.Types.ObjectId, required: true, ref: 'Developer' },
otp: { type: String, required: true }, // bcrypt hash (FIX 2)
attempts: { type: Number, default: 0 }, // brute-force guard (FIX 4)
Comment on lines +4 to +6

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Enforce one active OTP document per user.

Line [4] is not unique. Concurrent OTP requests can create multiple records, weakening Line [6] attempt tracking and allowing stale OTP documents to persist.

🧱 Suggested schema hardening 
-    userId: { type: mongoose.Schema.Types.ObjectId, required: true, ref: 'Developer' },
+    userId: { type: mongoose.Schema.Types.ObjectId, required: true, ref: 'Developer', unique: true, index: true },
     otp: { type: String, required: true },           // bcrypt hash (FIX 2)
-    attempts: { type: Number, default: 0 },           // brute-force guard (FIX 4)
+    attempts: { type: Number, default: 0, min: 0 },   // brute-force guard (FIX 4)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/models/otp.js` around lines 4 - 6, Add a uniqueness constraint and
safe upsert flow so only one active OTP per user exists: update the OTP mongoose
schema to create a unique index on userId (e.g., schema.index({ userId: 1 }, {
unique: true })) and remove relying on non-unique field definitions; also change
OTP creation code to use findOneAndUpdate({ userId }, { $set: { otp, attempts:
0, ... } }, { upsert: true, new: true }) (or handle duplicate key errors) so new
requests replace existing documents instead of inserting duplicates; reference
the userId, otp, and attempts symbols in backend/models/otp.js for these
changes.

createdAt: { type: Date, default: Date.now, expires: '10m' } // MongoDB TTL index
});

module.exports = mongoose.model('Otp', otpSchema);

13 changes: 9 additions & 4 deletions backend/routes/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const {
changePassword,
deleteAccount,
sendOtp,
verifyOtp
verifyOtp,
forgotPassword,
resetPassword
} = require('../controllers/auth.controller');


Expand All @@ -23,9 +25,12 @@ router.put('/change-password', authorization, changePassword);
// DELETE ACCOUNT (Protected - Danger Zone)
router.delete('/delete-account', authorization, deleteAccount);

// OTP (email verification)
router.post('/send-otp', sendOtp);


router.post('/verify-otp', verifyOtp);

module.exports = router;
// PASSWORD RESET (FIX 5)
router.post('/forgot-password', forgotPassword);
router.post('/reset-password', resetPassword);

module.exports = router;
4 changes: 2 additions & 2 deletions backend/utils/emailService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ const dotenv = require('dotenv');
dotenv.config();
const resend = new Resend(process.env.RESEND_API_KEY || 're_dummy_key_for_testing');

async function sendOtp(email, otp) {
async function sendOtp(email, otp, { subject = "Verify your urBackend account" } = {}) {
try {
const { data, error } = await resend.emails.send({
from: 'urBackend <urbackend@bitbros.in>',
to: email,
subject: 'Verify your urBackend account',
subject: subject,
html: `
<div style="font-family: Arial, sans-serif; background:#f4f6f8; padding:40px 0;">
<div style="max-width:500px; margin:auto; background:white; border-radius:10px; padding:30px; text-align:center; box-shadow:0 4px 12px rgba(0,0,0,0.08);">
Expand Down
12 changes: 12 additions & 0 deletions backend/utils/input.validation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ module.exports.onlyEmailSchema = z.object({
email: z.string().email("Invalid email format")
});

module.exports.verifyOtpSchema = z.object({
email: z.string().email("Invalid email format"),
otp: z.string().length(6, "OTP must be 6 digits")
});

module.exports.resetPasswordSchema = z.object({
email: z.string().email("Invalid email format"),
otp: z.string().length(6, "OTP must be 6 digits"),
newPassword: z.string().min(6, "Password must be at least 6 characters").max(100, "Password is too long.")
});
Comment on lines +46 to +50

@coderabbitai coderabbitai Bot Mar 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Tighten OTP validation to match the generated format.

Line [43] currently accepts any string with length ≥ 6. Since OTPs are generated as 6-digit codes, enforce exactly six digits to avoid wasting attempts on invalid formats.

🔧 Suggested change 
 module.exports.resetPasswordSchema = z.object({
     email: z.string().email("Invalid email format"),
-    otp: z.string().min(6, "OTP is required"),
+    otp: z.string().regex(/^\d{6}$/, "OTP must be a 6-digit code"),
     newPassword: z.string().min(6, "Password must be at least 6 characters").max(100, "Password is too long.")
 });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/utils/input.validation.js` around lines 41 - 45, The
resetPasswordSchema's otp currently uses z.string().min(6, ...) allowing any
string of length >=6; tighten validation to require exactly six numeric digits
by replacing the otp rule with a Zod pattern such as z.string().regex(/^\d{6}$/,
"OTP must be a 6-digit code") so the schema (resetPasswordSchema) only accepts
the generated 6-digit OTP format.



module.exports.createProjectSchema = z.object({
name: z.string().min(1, "Project name is required"),
description: z.string().optional()
Expand Down
4 changes: 0 additions & 4 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ services:
image: mongo:7
container_name: urbackend_mongo
restart: unless-stopped
ports:
- '27017:27017'
volumes:
- mongo_data:/data/db

Expand All @@ -15,8 +13,6 @@ services:
image: redis:latest
container_name: urbackend_redis
restart: unless-stopped
ports:
- '6379:6379'
volumes:
- redis_data:/data

Expand Down