Skip to content

Security Hardening and Code Cleanup #1

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

Open
Ajay-Mali wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Security Hardening and Code Cleanup #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15,691 changes: 9,594 additions & 6,097 deletions client/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/src/app/components/home/home.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class HomeComponent implements OnInit {

getAddresses() {
this._AddressService.getAddresses().subscribe(
(Address: { massage: string; count: string; data: any[] }) => {
(Address: { message: string; count: string; data: any[] }) => {
// console.log(Address);
this.Address = Address;
this.selectedAddress = {};
Expand Down
2 changes: 1 addition & 1 deletion client/src/app/services/userinfo.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class UserinfoService {

login(data: User) {
return this._http.post(this.LoginUrl, data).subscribe(
(data: { massage: string; Token: string }) => {
(data: { message: string; Token: string }) => {
// console.log(data.Token);

localStorage.setItem('token', 'Bearer ' + data.Token);
Expand Down
2 changes: 2 additions & 0 deletions server/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
MONGODB_URI=mongodb://localhost:27017/useraddress
JWT_SECRET=your_jwt_secret_here
2 changes: 1 addition & 1 deletion server/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ testem.log

# System Files
.DS_Store
Thumbs.db
Thumbs.db.env
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: .env is NOT gitignored — secrets will be committed to the repository.

Thumbs.db.env is clearly a merge/concatenation error. This should be two separate entries: Thumbs.db and .env. As-is, the .env file containing JWT_SECRET and MONGODB_URI is not excluded from version control, completely undermining the security objective of this PR.

🔐 Proposed fix 
-Thumbs.db.env
+Thumbs.db
+.env
📝 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
Thumbs.db.env
Thumbs.db
.env
🤖 Prompt for AI Agents 
In `@server/.gitignore` at line 46, The .gitignore currently contains a merged
entry "Thumbs.db.env" which fails to ignore the real .env file; update the
.gitignore by replacing the single "Thumbs.db.env" entry with two separate
entries "Thumbs.db" and ".env" so the OS thumbnail file and the environment
secrets file (containing JWT_SECRET, MONGODB_URI) are both properly excluded
from version control; ensure the literal ".env" line is present (and add any
other env variants like ".env.local" if your repo uses them).

109 changes: 49 additions & 60 deletions server/controllers/address-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,111 +2,100 @@ const { Add_Address } = require('../models/address-model');
const jwt = require('jsonwebtoken');

function getTokenData(data) {
// let data = req.headers.authorization;
let token = null;
let Payload = null;
// console.log(data);
if (Object.keys(data).length == 0 || data == '') {
return res.status(400).json({
error: 'Token is Required...'
});
} else {
token = data.split(' ')[1];
try {
Payload = jwt.verify(token, '1998');
return Payload.id;
} catch (err) {
console.log(err);
return res.status(400).json({
massage: 'invalid token'
});
}
if (!data || data === '') {
throw new Error('Token is Required...');
}
const token = data.split(' ')[1];
if (!token) {
throw new Error('Bearer Token is Required...');
}
try {
const Payload = jwt.verify(token, process.env.JWT_SECRET);
return Payload.id;
} catch (err) {
throw new Error('invalid token');
}
}
async function getAddress(req, res) {
// console.log(req.headers.authorization);

const id = getTokenData(req.headers.authorization);
try {
const id = getTokenData(req.headers.authorization);
const address = await Add_Address.find({ UserId: id });
// console.log(address);
if (address == '') {
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

address == '' relies on implicit array-to-string coercion.

[] == '' evaluates to true in JavaScript due to type coercion, so this accidentally works. Use an explicit length check for clarity and correctness.

Fix 
-		if (address == '') {
+		if (address.length === 0) {
📝 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 (address == '') {
if (address.length === 0) {
🤖 Prompt for AI Agents 
In `@server/controllers/address-controller.js` at line 24, The conditional if
(address == '') relies on coercion; change it to an explicit length check on the
address value (e.g., use address.length === 0) and guard for undefined/null if
needed (e.g., if (!address || address.length === 0)) so arrays aren't coerced to
strings; update the if that references the variable address in
address-controller.js accordingly.

return res.status(400).json({
massage: 'Data Not Found'
message: 'Data Not Found'
});
} else {
return res.status(200).json({
massage: 'Data is found...',
message: 'Data is found...',
count: address.length,
data: address
});
}
} catch (err) {
console.log(err);
if (err.message.includes('token') || err.message.includes('Token')) {
return res.status(401).json({ message: err.message });
}
return res.status(500).json({
massage: 'Server Error..'
message: 'Server Error..'
});
}
}

function addAddress(req, res) {
// console.log(req.headers.authorization);
const id = getTokenData(req.headers.authorization);
// console.log(id);
const body = req.body.Address;
// console.log(req.body.Address);
if (Object.keys(body).length == '') {
return res.status(400).json('Body Is Require ....');
}
async function addAddress(req, res) {
try {
const id = getTokenData(req.headers.authorization);
const body = req.body.Address;
if (!body || Object.keys(body).length === 0) {
return res.status(400).json('Body Is Require ....');
}

const data = {
UserId: id,
Address: body
};
const data = {
UserId: id,
Address: body
};

// console.log(data);
try {
Add_Address.create(data)
.then((data) => {
res.status(200).json({
massage: 'Address Add Succss..',
data: data
});
})
.catch((err) => {
console.log(err);
});
const savedAddress = await Add_Address.create(data);
res.status(200).json({
message: 'Address Add Succss..',
data: savedAddress
});
Comment on lines +46 to +63
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Two issues in addAddress: inconsistent error response format and wrong status code.

  1. Line 51: res.status(400).json('Body Is Require ....') sends a raw string. Every other endpoint returns { message: '...' }. This will break clients expecting a consistent shape.
  2. Line 60: A successful resource creation should return 201, not 200.
Suggested fix 
-		if (!body || Object.keys(body).length === 0) {
-			return res.status(400).json('Body Is Require ....');
-		}
+		if (!body || Object.keys(body).length === 0) {
+			return res.status(400).json({ message: 'Body Is Required' });
+		}
-		res.status(200).json({
+		res.status(201).json({
 			message: 'Address Add Succss..',
📝 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
async function addAddress(req, res) {
try {
const id = getTokenData(req.headers.authorization);
const body = req.body.Address;
if (!body || Object.keys(body).length === 0) {
return res.status(400).json('Body Is Require ....');
}
const data = {
UserId: id,
Address: body
};
const data = {
UserId: id,
Address: body
};
// console.log(data);
try {
Add_Address.create(data)
.then((data) => {
res.status(200).json({
massage: 'Address Add Succss..',
data: data
});
})
.catch((err) => {
console.log(err);
});
const savedAddress = await Add_Address.create(data);
res.status(200).json({
message: 'Address Add Succss..',
data: savedAddress
});
async function addAddress(req, res) {
try {
const id = getTokenData(req.headers.authorization);
const body = req.body.Address;
if (!body || Object.keys(body).length === 0) {
return res.status(400).json({ message: 'Body Is Required' });
}
const data = {
UserId: id,
Address: body
};
const savedAddress = await Add_Address.create(data);
res.status(201).json({
message: 'Address Add Succss..',
data: savedAddress
});
🤖 Prompt for AI Agents 
In `@server/controllers/address-controller.js` around lines 46 - 63, The
addAddress function should return consistent JSON error shapes and the correct
status for creations: replace the raw-string error response in addAddress (where
it currently does res.status(400).json('Body Is Require ....')) with a JSON
object like { message: 'Body is required' } and keep the 400 status, and change
the success response after Add_Address.create to use res.status(201).json(...)
instead of res.status(200).json(...), preserving the existing message and data
fields.

} catch (err) {
console.log(err);
if (err.message.includes('token') || err.message.includes('Token')) {
return res.status(401).json({ message: err.message });
}
return res.status(500).json({
massage: 'Server Error..'
message: 'Server Error..'
});
}
// res.send('addAddress is working...');
}

async function deleteAddress(req, res) {
// console.log(req.params.id);
const id = req.params.id;

try {
const DeleteData = await Add_Address.deleteOne({ _id: id });
const userId = getTokenData(req.headers.authorization);
const addressId = req.params.id;

const DeleteData = await Add_Address.deleteOne({ _id: addressId, UserId: userId });
if (DeleteData.deletedCount == 0) {
return res.status(400).json({
massage: 'Data Not Present'
message: 'Data Not Present or unauthorized'
});
} else {
return res.status(200).json({
massage: 'Delete Data Success....'
message: 'Delete Data Success....'
});
}
} catch (err) {
console.log(err);
if (err.message.includes('token') || err.message.includes('Token')) {
return res.status(401).json({ message: err.message });
}
return res.status(500).json({
massage: 'Server Error..'
message: 'Server Error..'
});
}
// res.send('addAddress is working...');
}

module.exports = { getAddress, addAddress, deleteAddress };
38 changes: 23 additions & 15 deletions server/controllers/user-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
const { User } = require('../models/user-model.js');
// const mongoose = require('mongoose');
const jwt = require('jsonwebtoken');
const bcrypt = require('bcryptjs');

function registration(req, res) {
// console.log(req.body);
async function registration(req, res) {
const data = req.body;

if (Object.keys(data).length == 0) {
Expand All @@ -13,22 +12,30 @@ function registration(req, res) {
}

try {
User.create(data).then((user) => {
res.status(201).json({
massage: 'User Created Success..'
});
const { name, email, password, address } = data;
const salt = await bcrypt.genSalt(10);
const hashedPassword = await bcrypt.hash(password, salt);

await User.create({
name,
email,
password: hashedPassword,
address
});
Comment on lines 14 to +24
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing validation for required fields before hashing.

If password is undefined (e.g., client omits it), bcrypt.hash(undefined, salt) will throw a non-descriptive error. Similarly, name and email could be missing. Validate required fields before proceeding.

Suggested guard 
 	try {
 		const { name, email, password, address } = data;
+		if (!name || !email || !password) {
+			return res.status(400).json({
+				message: 'name, email, and password are required'
+			});
+		}
 		const salt = await bcrypt.genSalt(10);
🤖 Prompt for AI Agents 
In `@server/controllers/user-controller.js` around lines 14 - 24, Before
generating a salt and calling bcrypt.hash, validate that required fields (name,
email, password) are present and non-empty in the incoming data; if any are
missing, return/throw a descriptive validation error instead of proceeding.
Update the controller code around the try block that uses bcrypt.genSalt,
bcrypt.hash and User.create to perform these checks on the variables name,
email, and password (from the destructured data) and short-circuit with an
appropriate error response before calling bcrypt.hash or User.create. Also
ensure you only call bcrypt.genSalt/ bcrypt.hash when password is a valid string
to avoid non-descriptive runtime errors.


res.status(201).json({
message: 'User Created Success..'
});
} catch (err) {
console.log(err);
return res.status(500).json({
massage: 'Server Error..'
message: 'Server Error..'
});
}
}

async function login(req, res) {
let data = req.body;
// console.log(data);
if (Object.keys(data).length == 0) {
return res.status(400).json({
error: 'Request Body is Required...'
Expand All @@ -40,29 +47,30 @@ async function login(req, res) {
const user = await User.findOne({ email });

if (user) {
if (user.password == password) {
const isMatch = await bcrypt.compare(password, user.password);
if (isMatch) {
let Payload = {
id: user._id
};
const token = jwt.sign(Payload, '1998');
const token = jwt.sign(Payload, process.env.JWT_SECRET);
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

JWT is signed without an expiration — tokens are valid forever.

If a token is leaked, there is no automatic revocation mechanism. Add an expiresIn option to limit the token's lifetime. Also, if process.env.JWT_SECRET is undefined, jwt.sign will use undefined as the secret, producing insecure tokens — consider validating at startup.

Suggested change 
-			const token = jwt.sign(Payload, process.env.JWT_SECRET);
+			const token = jwt.sign(Payload, process.env.JWT_SECRET, { expiresIn: '1d' });
🤖 Prompt for AI Agents 
In `@server/controllers/user-controller.js` at line 55, The jwt.sign call
currently creates tokens without expiration and uses process.env.JWT_SECRET
unchecked; update the token creation in the auth flow (the line using
jwt.sign(Payload, process.env.JWT_SECRET)) to pass an expiresIn option (e.g., a
short duration or config value) so tokens expire, and add startup-time
validation to fail fast if process.env.JWT_SECRET is missing or empty (validate
in app initialization or in the module that exports the secret) so jwt.sign
never runs with an undefined secret.

return res.json({
massage: 'login Success',
message: 'login Success',
Token: token
});
} else {
return res.status(400).json({
massage: 'Email Or Password Invlid'
message: 'Email Or Password Invlid'
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo: "Invlid" → "Invalid".

The same typo appears on Lines 62 and 67.

Fix 
-				message: 'Email Or Password Invlid'
+				message: 'Email Or Password Invalid'
📝 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
message: 'Email Or Password Invlid'
message: 'Email Or Password Invalid'
🤖 Prompt for AI Agents 
In `@server/controllers/user-controller.js` at line 62, Fix the typo in the error
message strings inside user-controller.js: find the occurrences of the message
property containing "Email Or Password Invlid" (used in the login/authentication
response handling, e.g., in the function handling sign-in or checkCredentials
within the controller) and correct "Invlid" to "Invalid" in both places (the two
occurrences noted around the authentication error responses). Ensure both
message fields now read "Email Or Password Invalid".

});
}
} else {
return res.status(400).json({
massage: 'Email Or Password Invlid'
message: 'Email Or Password Invlid'
});
}
} catch (err) {
console.log(err);
return res.status(500).json({
massage: 'Server Error..'
message: 'Server Error..'
});
}
}
Expand Down
20 changes: 10 additions & 10 deletions server/db/connection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
const mongoose = require('mongoose');

function connection() {
mongoose
.connect('mongodb://localhost:27017/useraddress', {
async function connection() {
const uri = process.env.MONGODB_URI || 'mongodb://localhost:27017/useraddress';
try {
await mongoose.connect(uri, {
useNewUrlParser: true,
useUnifiedTopology: true
})
.then(() => {
console.log('connected...');
})
.catch((err) => {
console.log(err);
useUnifiedTopology: true,
useCreateIndex: true // Added this for unique constraints which I will add in next step
});
console.log('connected...');
} catch (err) {
console.log(err);
}
Comment on lines +12 to +14
Copy link

@coderabbitai coderabbitai bot Feb 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Silent catch swallows connection failures — the server will run without a database.

If mongoose.connect fails, the error is only logged and the application continues to run. All subsequent database operations will fail with confusing errors. The connection function should propagate the failure so the application can crash fast or retry.

🐛 Proposed fix — re-throw so the caller can handle it 
 	} catch (err) {
-		console.log(err);
+		console.error('MongoDB connection failed:', err.message);
+		throw err;
 	}

Then in server/index.js, handle the rejection:

connection().catch(() => process.exit(1));
🤖 Prompt for AI Agents 
In `@server/db/connection.js` around lines 12 - 14, The catch block in the
connection function swallows mongoose.connect errors (see function connection
and call to mongoose.connect); change the catch to re-throw the caught error (or
return a rejected promise) instead of just console.log so failures propagate,
and update the caller (where connection() is invoked, e.g., server/index.js) to
handle the rejection (for example call connection().catch(() =>
process.exit(1))) so the app can crash fast or retry.

}

module.exports = { connection };
1 change: 1 addition & 0 deletions server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require('dotenv').config();
const express = require('express');
const app = express();
const port = 3000;
Expand Down
12 changes: 8 additions & 4 deletions server/models/user-model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,20 @@ const { Schema } = mongoose;
const UserSchema = new Schema(
{
name: {
type: 'String'
type: String,
required: true
},
email: {
type: 'String'
type: String,
required: true,
unique: true
},
password: {
type: 'String'
type: String,
required: true
},
address: {
type: 'String'
type: String
}
},
{
Expand Down
Loading