-
Notifications
You must be signed in to change notification settings - Fork 397
✨ User Profile Enhancements & API Integration #2628
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
f4ce8a9
d3145fb
30dcc83
e7d03a1
1518ad1
4999031
2d0dae2
624b800
f84544c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| const User = require("../models/User"); | ||
|
|
||
| exports.getUsers = async (_, res) => { | ||
| try { | ||
| const user = await User.find(); | ||
|
|
||
| return res.status(200).json({ user }); | ||
| } catch (error) { | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return res.status(500).json({ error: error.message }); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| exports.getUserByUsername = async (req, res) => { | ||
| try { | ||
| const { user_name } = req.params; | ||
|
|
||
| if (!user_name) | ||
| return res.status(400).json({ message: "User name is required" }); | ||
|
|
||
| const user = await User.findOne({ username: user_name }); | ||
|
|
||
| if (!user) return res.status(404).json({ message: "Account not found" }); | ||
|
|
||
| return res.send(user); | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } catch (error) { | ||
| return res.status(500).json({ error: error.message }); | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| exports.updateUser = async (req, res) => { | ||
| try { | ||
| const { id } = req.params; | ||
| const { name, email, phone, gender, profile_picture } = req.body; | ||
|
|
||
| if (!name || !email || !phone || !gender || !profile_picture) | ||
| return res | ||
| .status(400) | ||
| .json({ message: "Enter the required fields to update your profile" }); | ||
|
|
||
| const user = await User.findById(id); | ||
|
|
||
| if (!user) return res.status(404).json({ message: "Account not found" }); | ||
|
|
||
| const updatedUser = await User.findByIdAndUpdate( | ||
| { _id: id }, | ||
| { name, email, phone, gender, profile_picture }, | ||
| { new: true } | ||
| ); | ||
|
|
||
| if (updatedUser) | ||
| return res.status(200).json({ message: "Profile updated", user: updatedUser }); | ||
|
|
||
| return res.status(500).json({ message: "Failed to your profile" }); | ||
| } catch (error) { | ||
| return res.status(500).send(error.message); | ||
| } | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConfirm authz: is this route protected and scoped to the acting user? If there’s no auth middleware on PUT /:id, any client could update any user by ID. Ensure a token-based guard and “self or admin” authorization. Run this to verify route protection and middleware usage: 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect user routes for auth middleware on PUT /:id
rg -n -C3 -P 'router\.(put|patch)\s*\(\s*[\'"]/:id[\'"]\s*,\s*[^)]*\)' backend/routes/userRoutes.js
# Look for auth middleware imports in routes/app
rg -n -C2 -P 'require\\(.*auth.*\\)|from\\s+[\'"].*auth.*[\'"]' backend/routes/userRoutes.js backend/app.js backend/middleware
# Search for common middleware names across backend
rg -n -C2 -P '\\b(auth|isAuth|authenticate|verify(Token|JWT)|requireAuth|protect)\\b' backendLength of output: 129 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect user routes for auth middleware on PUT /:id
rg -n -C3 -P "router\.(put|patch)\s*\(\s*['\"]/?:id['\"],\s*.*" backend/routes/userRoutes.js
# Look for auth middleware imports in routes/app and middleware directory
rg -n -C2 -P "require\(.*auth.*\)|from\s+['\"].*auth.*['\"]" backend/routes/userRoutes.js backend/app.js backend/middleware
# Search for common auth middleware names across the backend
rg -n -C2 -P "\b(auth|isAuth|authenticate|verify(Token|JWT)|requireAuth|protect)\b" backendLength of output: 845 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- Directory: backend/routes ----"
ls backend/routes || true
echo "---- Partial backend/app.js ----"
sed -n '1,200p' backend/app.js || true
echo "---- Mounts in backend/app.js ----"
grep -n "app.use" -C2 backend/app.js || true
if [ -f backend/routes/index.js ]; then
echo "---- Partial backend/routes/index.js ----"
sed -n '1,200p' backend/routes/index.js
echo "---- Mounts in backend/routes/index.js ----"
grep -n "router.use" -C2 backend/routes/index.js || true
fiLength of output: 2637 Protect updateUser endpoint with authentication and “self or admin” authorization The • File: - router.put("/:id", userController.updateUser);
+ const passport = require("../middlewares/Passport");
+ const { authorizeSelfOrAdmin } = require("../middlewares/authMiddleware");
+
+ router.put(
+ "/:id",
+ passport.authenticate("jwt", { session: false }),
+ authorizeSelfOrAdmin,
+ userController.updateUser
+ );• File: // Ensure the authenticated user is updating their own record or is an admin
exports.authorizeSelfOrAdmin = (req, res, next) => {
const actingUser = req.user; // set by passport
const targetId = req.params.id;
if (actingUser.id === targetId || actingUser.role === "admin") {
return next();
}
return res.status(403).json({ message: "Forbidden: insufficient privileges" });
};• Verify that your Passport JWT strategy populates These changes will close the security gap and ensure only authorized users can update profiles. 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,10 @@ const userSchema = new Schema({ | |
| unique: true, | ||
| trim: true, | ||
| }, | ||
| name: { | ||
| type: String, | ||
| default: null | ||
| }, | ||
| email: { | ||
| type: String, | ||
| required: false, | ||
|
|
@@ -29,6 +33,14 @@ const userSchema = new Schema({ | |
| trim: true, | ||
| match: [/^\d{10}$/, "Please enter a 10-digit phone number"], | ||
| }, | ||
| gender: { | ||
| type: String, | ||
| default: null | ||
| }, | ||
| profile_picture: { | ||
| type: String, | ||
| default: null | ||
| }, | ||
|
Comment on lines
+36
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Constrain gender values and validate/sanitize profile_picture URLs; also avoid null defaults.
- gender: {
- type: String,
- default: null
- },
- profile_picture: {
- type: String,
- default: null
- },
+ gender: {
+ type: String,
+ enum: ["male", "female", "other", "prefer_not_to_say", ""],
+ default: ""
+ },
+ profile_picture: {
+ type: String,
+ trim: true,
+ match: [/^https?:\/\/.+/i, "Please enter a valid URL (http/https)"],
+ default: ""
+ },Additionally, to prevent leaking sensitive fields (e.g., hash/salt from passport-local-mongoose, reset tokens) via GET endpoints, add a toJSON transform: // Place near schema definition
userSchema.set("toJSON", {
transform: (_doc, ret) => {
delete ret.hash;
delete ret.salt;
delete ret.resetPasswordToken;
delete ret.resetPasswordExpires;
delete ret.otp;
delete ret.otpExpires;
return ret;
}
});If these fields are already excluded in controllers, ignore this. Otherwise, this guardrail is highly recommended. |
||
| role: { | ||
| type: Number, | ||
| default: 0, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,27 @@ | ||
| const express = require("express"); | ||
| const router = express.Router(); | ||
| const User = require("../models/User"); | ||
| const userController = require("../controllers/userController") | ||
|
|
||
| router.post("/update-details", async (req, res) => { | ||
| try { | ||
| const { email, phone } = req.body; | ||
| const user = await User.findById(req.user.id); | ||
| if (user) { | ||
| user.email = email; | ||
| user.phone = phone; | ||
| await user.save(); | ||
| res.status(200).send("Details updated successfully"); | ||
| } else { | ||
| res.status(404).send("User not found"); | ||
| } | ||
| } catch (error) { | ||
| res.status(500).send(error.message); | ||
| } | ||
| }); | ||
| // router.post("/update-details", async (req, res) => { | ||
| // try { | ||
| // const { email, phone } = req.body; | ||
| // const user = await User.findById(req.user.id); | ||
| // if (user) { | ||
| // user.email = email; | ||
| // user.phone = phone; | ||
| // await user.save(); | ||
| // res.status(200).send("Details updated successfully"); | ||
| // } else { | ||
| // res.status(404).send("User not found"); | ||
| // } | ||
| // } catch (error) { | ||
| // res.status(500).send(error.message); | ||
| // } | ||
| // }); | ||
|
|
||
| router.get("/", userController.getUsers) | ||
| router.get("/:user_name", userController.getUserByUsername) | ||
| router.put("/:id", userController.updateUser); | ||
|
Comment on lines
+23
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAdd auth and input validation; current routes risk exposing/allowing unintended data access.
-router.get("/", userController.getUsers)
-router.get("/:user_name", userController.getUserByUsername)
-router.put("/:id", userController.updateUser);
+// Example (adjust to your existing middlewares)
+// const { requireAuth } = require("../middlewares/auth");
+// const { validateUserUpdate } = require("../middlewares/validators");
+// router.use(requireAuth);
+
+router.get("/", userController.getUsers);
+router.get("/username/:user_name", userController.getUserByUsername);
+router.put("/:id", /* validateUserUpdate, */ userController.updateUser);To verify exposure and consumers: 🏁 Script executed: #!/bin/bash
# 1) Confirm no public routes return sensitive fields
rg -nP 'getUsers|getUserByUsername|updateUser' backend/controllers -C4
# 2) Validate all frontend calls target /api/users and not legacy /api/auth/update-details
rg -nP '/api/users|/api/auth/update-details' -C2 src
# 3) Check for any unauthenticated fetches in the app (axios without withCredentials or auth headers)
rg -nP 'axios\.(get|put|post)\(' -n src | sed -n '1,200p'Length of output: 6771 🏁 Script executed: #!/bin/bash
echo "=== backend/routes/userRoutes.js ==="
sed -n '1,200p' backend/routes/userRoutes.js || echo "File not found"
echo -e "\n=== backend/middlewares directory ==="
if [ -d backend/middlewares ]; then
ls -R backend/middlewares
else
echo "No middlewares directory."
fi
echo -e "\n=== Files with 'auth' in name in backend ==="
find backend -type f | grep -i auth || echo "No auth files found"
echo -e "\n=== Files with 'validator' in name in backend ==="
find backend -type f | grep -i validator || echo "No validator files found"Length of output: 1586 🏁 Script executed: #!/bin/bash
# Display auth middleware definitions
sed -n '1,200p' backend/middlewares/Passport.js || echo "File not found"Length of output: 1580 Secure & Validate All User Routes To prevent unauthorized access and ensure only valid data reaches your controllers, the user routes must be guarded by authentication/authorization and input validation. Key changes required:
Areas to address:
Proposed diff in const express = require("express");
const router = express.Router();
+const passport = require("../middlewares/Passport"); // or your requireAuth wrapper
+const {
+ validateObjectIdParam,
+ validateUserUpdate
+} = require("../middlewares/validators/userValidator");
const userController = require("../controllers/userController");
-// Public routes – currently unprotected
-router.get("/", userController.getUsers)
-router.get("/:user_name", userController.getUserByUsername)
-router.put("/:id", userController.updateUser);
+// Protect all user endpoints
+router.use(passport.authenticate("session")); // or .authenticate("jwt")
+
+// List all users (auth + optional role check)
+router.get("/", userController.getUsers);
+
+// Lookup by username (validate path to avoid collision)
+router.get(
+ "/username/:user_name",
+ validateObjectIdParam.bind(null, "user_name", { type: "username" }), // optional: enforce format
+ userController.getUserByUsername
+);
+
+// Update user by ID (auth + ID & body validation)
+router.put(
+ "/:id",
+ validateObjectIdParam("id"),
+ validateUserUpdate,
+ userController.updateUser
+);
module.exports = router;And scaffold new files:
These refactors are mandatory to ensure your API does not expose or allow modifications of user data without proper checks. 🤖 Prompt for AI Agents |
||
|
|
||
| module.exports = router; | ||
Uh oh!
There was an error while loading. Please reload this page.