Feature/create batch #223
Feature/create batch #223
Conversation
…batches for clubs
…batches for clubs
|
@codewkaushik404 is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds JWT/bcrypt auth and jwtIsAuthenticated middleware, restructures Mongoose schemas with timestamps, introduces Certificate and CertificateBatch models plus createBatch controller and route with Zod validation, updates imports/usages of authentication middleware across many routes, and wires DB/cookie-parser in app entry. Changes
Sequence DiagramssequenceDiagram
participant Client
participant AuthRoute as "Auth Route"
participant Validator as "Auth Validator"
participant UserModel as "User Model"
participant Bcrypt as "bcrypt"
participant JWT as "JWT Signer"
Client->>AuthRoute: POST /login {username,password}
AuthRoute->>Validator: validate input
Validator-->>AuthRoute: valid
AuthRoute->>UserModel: find by username
UserModel-->>AuthRoute: user
AuthRoute->>Bcrypt: compare(password, hash)
Bcrypt-->>AuthRoute: match
AuthRoute->>JWT: sign token {user_id, role}
JWT-->>AuthRoute: token
AuthRoute-->>Client: return token and set cookie
sequenceDiagram
participant Client
participant CertRoute as "Certificate Route"
participant JWTAuth as "jwtIsAuthenticated"
participant Controller as "createBatch"
participant BatchValidator as "Batch Validator"
participant UserModel as "User Model"
participant OrgUnitModel as "OrgUnit Model"
participant ApproverLookup as "Approver Lookup"
participant CertBatchModel as "CertificateBatch Model"
Client->>CertRoute: POST /api/certificate-batches (Bearer)
CertRoute->>JWTAuth: validate token
JWTAuth-->>CertRoute: decoded user
CertRoute->>Controller: invoke createBatch
Controller->>UserModel: verify requester & position
Controller->>BatchValidator: validate payload
BatchValidator-->>Controller: valid
Controller->>OrgUnitModel: verify unit is Club & linked Council
Controller->>ApproverLookup: resolve GS & President
Controller->>UserModel: verify listed users exist
Controller->>CertBatchModel: create CertificateBatch
CertBatchModel-->>Controller: batch created
Controller-->>Client: return created batch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/config/db.js (1)
7-13:⚠️ Potential issue | 🟠 MajorFail fast when
MONGODB_URIis missing or connect fails.Right now the server can keep running without a DB connection, which will surface as confusing runtime errors later. Guard the env var and rethrow to halt startup on failure.
🔧 Proposed fix
const connectDB = async () => { try { const ConnectDB = process.env.MONGODB_URI; + if (!ConnectDB) { + throw new Error("MONGODB_URI is not set"); + } //Removing the options as they are no longer needed from mongoose6+ await mongoose.connect(ConnectDB); console.log("MongoDB Connected"); } catch (error) { console.error("MongoDB Connection Error:", error); + throw error; } };backend/routes/profile.js (3)
9-31:⚠️ Potential issue | 🟠 MajorEnforce ownership/role checks for
ID_No-based updates.
ID_Nocomes from the request body, so any authenticated user could update someone else’s photo unless you verify ownership or privileged roles. This is a horizontal privilege escalation risk.
Consider usingreq.useras the source of truth (or allow overrides only for explicit roles).🔒 Suggested guard (adjust role/field names to your auth model)
- const { ID_No } = req.body; + const { ID_No } = req.body; if (!ID_No) { return res.status(400).json({ error: "ID_No is required" }); } + if (req.user?.user_id !== ID_No) { + return res.status(403).json({ error: "Forbidden" }); + }
106-111:⚠️ Potential issue | 🟠 MajorRemove debug logs that include user data.
userIdandupdatedDetailscan contain PII (email, phone, DOB, etc.). Logging these in production is a compliance/privacy risk.🧹 Suggested removal
- console.log("Received userId:", userId); - console.log("Received updatedDetails:", updatedDetails);
219-220:⚠️ Potential issue | 🟡 MinorRemove manual
updated_atassignment; it's redundant with Mongoose timestamps.The schema uses
{ timestamps: true }, which automatically manages theupdatedAtfield (camelCase). Manually settingupdated_at(snake_case) is ignored under strict mode (the default) and doesn't serve any purpose.🧽 Suggested cleanup
- // Update the updated_at timestamp - user.updated_at = new Date();
🤖 Fix all issues with AI agents
In `@backend/controllers/certificateController.js`:
- Around line 14-16: There's a typo in the error JSON returned when a user isn't
found: the key is "messge" instead of "message"; update the response in the
controller (the block that checks `if (!user) { return
res.status(404).json({...}) }`) to use `message: "Invalid data (User not
found)"` so the client receives the correct key (look for the `user` variable
and the `res.status(404).json` call in certificateController.js).
- Around line 18-22: Update the role guard to require an explicit
CLUB_COORDINATOR role instead of allowing missing roles: replace the current
conditional that only checks user.role !== "CLUB_COORDINATOR" with a strict
check that returns 403 when user.role is falsy or not exactly "CLUB_COORDINATOR"
(i.e., if (!user.role || user.role !== "CLUB_COORDINATOR") return
res.status(403).json(...)); locate and change the check around the user.role
validation in certificateController.js where the current guard is implemented.
- Around line 39-116: The code dereferences and logs lookup results before null
checks (e.g., positionHolder, position, gensecObj, presidentHolder,
presidentObj), which can throw if a record is missing; move all console.log
calls until after the corresponding existence checks and add explicit guards for
presidentHolder and presidentObj (returning the proper error responses like the
existing patterns) after PositionHolder.findOne, Position.findById, User.findOne
(gensecObj), Position.findOne (presidentPosition), PositionHolder.findOne
(presidentHolder) and User.findById (presidentId) to ensure no property is
accessed on null/undefined.
- Around line 121-126: The check after calling zodObjectId.safeParse in the
users.map Promise (used to build userChecks) is incorrect because safeParse
always returns an object; change the conditional to check validation.success
(e.g., if (!validation.success)) and return the invalid response when success is
false; update any later uses that rely on validation.data to use validation.data
only when validation.success is true (inside the branch where the ID is valid).
In `@backend/middlewares/isAuthenticated.js`:
- Around line 13-28: In jwtIsAuthenticated, ensure process.env.JWT_SECRET_TOKEN
is present and return res.status(500).json({ message: "Server misconfiguration:
JWT secret missing" }) if not; when calling jwt.verify(token,
process.env.JWT_SECRET_TOKEN) pass an explicit algorithms allow-list (e.g.
algorithms: ['HS256']) to prevent algorithm downgrade attacks, assign the
verified payload to req.user as before, and keep the existing catch to return
401 for invalid/expired tokens.
In `@backend/models/certificateSchema.js`:
- Around line 62-66: The certificateId field is marked unique which causes
duplicate-key errors for multiple non-approved docs; remove the unique: true
from the certificateId field in the certificateSchema and instead add a partial
unique index on certificateId that only applies when status === "Approved"
(ensure the index also requires certificateId exists). Locate the schema
definition (certificateId in certificateSchema / CertificateSchema) and replace
the field-level unique constraint with a schema-level index using
partialFilterExpression for status:"Approved" (and certificateId existence) so
only approved documents enforce uniqueness.
- Around line 6-9: The ref name for the unit_id schema field is misspelled as
"Oraganizational_Unit"; update the unit_id field's ref to the correctly
registered model name "Organizational_Unit" so mongoose.populate() resolves
properly (locate the unit_id definition in the certificate schema and replace
the ref string).
In `@backend/routes/auth.js`:
- Around line 45-51: Replace incorrect res.json(401).json(...) calls with
res.status(401).json(...) in the auth route handler(s) where authentication
fails (the checks using bcrypt.compare and the earlier invalid-credentials
check), and make the same change for the other occurrence around the second
failure block (the one referenced at lines 89-92); update the two places that
call res.json(401).json to use res.status(401).json to set the HTTP status
correctly and avoid double-sending headers.
In `@backend/routes/feedbackRoutes.js`:
- Around line 189-212: The route currently trusts req.body.resolved_by which
allows spoofing; instead ignore the body field and derive the resolver from the
authenticated user (req.user). In the feedback resolution handler (where
feedbackId, actions_taken, resolved_by are read and Feedback.findById is
called), remove usage of req.body.resolved_by and set feedback.resolved_by =
req.user.id or req.user.email (whichever is the canonical identifier in your
auth model) after verifying req.user exists; keep validation for actions_taken
and the rest of the update logic, and ensure the API does not accept or persist
a client-supplied resolved_by.
In `@backend/routes/profile.js`:
- Around line 80-90: The code in the router.delete("/photo-delete",
isAuthenticated, async (req, res) => { ... }) handler calls user.findOne(...)
but `user` is undefined; update the handler to use the correct User model
reference (e.g., `User.findOne(...)`) and ensure the model is imported/required
at the top (or rename the local variable to avoid shadowing, e.g., `const
foundUser = await User.findOne({ user_id: ID_No })`), then update subsequent
references to use that variable (`foundUser`) instead of the undefined `user`.
In `@backend/utils/batchValidate.js`:
- Around line 3-10: The zodObjectId regex is too permissive (allows non-hex
chars); update the zodObjectId used by validateBatchSchema to enforce hex-only
ObjectIds by replacing the pattern with a hex-only regex (e.g.,
/^[0-9a-fA-F]{24}$/) so zodObjectId = zod.string().regex(...); ensure the same
updated zodObjectId is used in validateBatchSchema for unit_id and users array
elements.
🧹 Nitpick comments (1)
backend/package.json (1)
33-58: Verified: bcrypt, cookie-parser, and zod versions are secure and current.All three packages check out—bcrypt 6.0.0 has no known advisories (prior CVEs fixed in 5.0.0+), cookie-parser 1.4.7 has no direct vulnerabilities, and zod 4.3.6 is the current latest stable release.
However, I notice express-rate-limiter@1.3.1 is also in the dependencies. This package is effectively unmaintained (~10 years old) and, while no CVEs are recorded, poses a supply chain risk. If it's unused, remove it; if actively used, consider migrating to express-rate-limit (the maintained alternative, already in your dependencies at 7.5.1) which has recent security hardening.
| function jwtIsAuthenticated(req, res, next) { | ||
| let token; | ||
| const headerData = req.headers.authorization; | ||
| if (!headerData || !headerData.startsWith("Bearer ")) { | ||
| return res.status(401).json({ message: "User not authenticated " }); | ||
| } | ||
|
|
||
| token = headerData.split(" ")[1]; | ||
| try { | ||
| const userData = jwt.verify(token, process.env.JWT_SECRET_TOKEN); | ||
| req.user = userData; | ||
| //console.log(userData); | ||
| next(); | ||
| } catch (err) { | ||
| res.status(401).json({ message: "Invalid or expired token sent" }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat package.json | grep -A 5 -B 5 jsonwebtokenRepository: OpenLake/Student_Database_COSA
Length of output: 118
🌐 Web query:
jsonwebtoken npm library algorithms option security best practices
💡 Result:
jsonwebtoken (npm i jsonwebtoken) — algorithms option security best practices
- Pin/upgrade to v9+ and keep it updated. Versions
<= 8.5.1had a verification-bypass issue whenverify()was called without analgorithmsallow-list and with a falsy key, causing a dangerous default tonone(fixed in9.0.0). [1][2] - Always pass an explicit
algorithmsallow-list tojwt.verify()(don’t rely on defaults). Example: allow only what you actually issue. [3] - Never allow
nonein production. In v9, accepting unsigned tokens requires explicitly allowingnoneviaoptions.algorithms—don’t do this unless you have a very specific, safe use case. [4] - Don’t mix symmetric and asymmetric algorithms (e.g., don’t accept both
HS256andRS256for the same verifier). This helps avoid “algorithm confusion/downgrade” classes of mistakes. [5] - Prefer asymmetric signing (e.g.,
RS256/ES256) for most real systems, especially when multiple services verify tokens or key rotation matters. Auth0 (and many deployments) recommendRS256overHS256because verifiers only need a public key, and rotation is easier. [6] - Validate claims, not just the signature: set
issuerandaudience(and use expiration). This reduces token replay/misuse across services. [5] - Use
verify(), notdecode(), for untrusted tokens.decode()does not validate signatures/claims; treat decoded payload as untrusted input unless verified. [7]
Minimal safe verification pattern
import jwt from "jsonwebtoken";
const payload = jwt.verify(token, publicKeyPem, {
algorithms: ["RS256"], // strict allow-list
issuer: "https://issuer.example",
audience: "my-api",
// optionally: maxAge: "15m"
});Sources: GitHub Security Advisory + NVD for CVE-2022-23540 [1][2]; jsonwebtoken npm docs on algorithms option/defaults [3]; v8→v9 migration note on explicitly allowing none [4]; express-jwt guidance on required algorithms and not mixing HS/RS + issuer/audience [5]; Auth0 guidance recommending RS256 [6]; Auth0 JWT validation note to use verify() (not decode()) and disallow none [7].
🏁 Script executed:
find . -name "package.json" -type f | head -20Repository: OpenLake/Student_Database_COSA
Length of output: 120
🏁 Script executed:
rg "jwt\." --type js -A 2 -B 2 | head -100Repository: OpenLake/Student_Database_COSA
Length of output: 1563
🏁 Script executed:
rg "JWT_SECRET_TOKEN" --type js -A 3 -B 3Repository: OpenLake/Student_Database_COSA
Length of output: 2072
🏁 Script executed:
cat backend/package.json | jq '.dependencies.jsonwebtoken, .devDependencies.jsonwebtoken'Repository: OpenLake/Student_Database_COSA
Length of output: 87
🏁 Script executed:
cat -n backend/middlewares/isAuthenticated.jsRepository: OpenLake/Student_Database_COSA
Length of output: 2711
🏁 Script executed:
cat -n backend/routes/auth.js | grep -A 10 -B 5 "jwt.sign\|jwt.verify"Repository: OpenLake/Student_Database_COSA
Length of output: 2473
Explicitly restrict JWT algorithms and validate secret configuration.
jwt.verify() in jsonwebtoken v9+ requires passing an explicit algorithms allow-list; the current code relies on defaults, which bypasses algorithm-restriction security. Additionally, missing JWT_SECRET_TOKEN should return 500 (server misconfiguration) rather than 401 (auth failure).
Proposed fix
function jwtIsAuthenticated(req, res, next) {
let token;
const headerData = req.headers.authorization;
if (!headerData || !headerData.startsWith("Bearer ")) {
return res.status(401).json({ message: "User not authenticated " });
}
token = headerData.split(" ")[1];
try {
- const userData = jwt.verify(token, process.env.JWT_SECRET_TOKEN);
+ const secret = process.env.JWT_SECRET_TOKEN;
+ if (!secret) {
+ return res.status(500).json({ message: "JWT secret not configured" });
+ }
+ const userData = jwt.verify(token, secret, {
+ algorithms: ["HS256"],
+ });
req.user = userData;
//console.log(userData);
next();
} catch (err) {
res.status(401).json({ message: "Invalid or expired token sent" });
}
}📝 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.
| function jwtIsAuthenticated(req, res, next) { | |
| let token; | |
| const headerData = req.headers.authorization; | |
| if (!headerData || !headerData.startsWith("Bearer ")) { | |
| return res.status(401).json({ message: "User not authenticated " }); | |
| } | |
| token = headerData.split(" ")[1]; | |
| try { | |
| const userData = jwt.verify(token, process.env.JWT_SECRET_TOKEN); | |
| req.user = userData; | |
| //console.log(userData); | |
| next(); | |
| } catch (err) { | |
| res.status(401).json({ message: "Invalid or expired token sent" }); | |
| } | |
| function jwtIsAuthenticated(req, res, next) { | |
| let token; | |
| const headerData = req.headers.authorization; | |
| if (!headerData || !headerData.startsWith("Bearer ")) { | |
| return res.status(401).json({ message: "User not authenticated " }); | |
| } | |
| token = headerData.split(" ")[1]; | |
| try { | |
| const secret = process.env.JWT_SECRET_TOKEN; | |
| if (!secret) { | |
| return res.status(500).json({ message: "JWT secret not configured" }); | |
| } | |
| const userData = jwt.verify(token, secret, { | |
| algorithms: ["HS256"], | |
| }); | |
| req.user = userData; | |
| //console.log(userData); | |
| next(); | |
| } catch (err) { | |
| res.status(401).json({ message: "Invalid or expired token sent" }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@backend/middlewares/isAuthenticated.js` around lines 13 - 28, In
jwtIsAuthenticated, ensure process.env.JWT_SECRET_TOKEN is present and return
res.status(500).json({ message: "Server misconfiguration: JWT secret missing" })
if not; when calling jwt.verify(token, process.env.JWT_SECRET_TOKEN) pass an
explicit algorithms allow-list (e.g. algorithms: ['HS256']) to prevent algorithm
downgrade attacks, assign the verified payload to req.user as before, and keep
the existing catch to return 401 for invalid/expired tokens.
| certificateId: { | ||
| type: String, | ||
| unique: true, | ||
| required: function () { | ||
| return this.status === "Approved"; |
There was a problem hiding this comment.
certificateId unique constraint will reject multiple non-approved docs.
With unique: true, multiple pending records without certificateId can trigger duplicate-key errors. Use a partial unique index instead and remove unique: true from the field.
🧩 Proposed fix
certificateId: {
type: String,
- unique: true,
required: function () {
return this.status === "Approved";
},
},+certificateSchema.index(
+ { certificateId: 1 },
+ { unique: true, partialFilterExpression: { certificateId: { $exists: true } } },
+);MongoDB unique index behavior with missing field values (null/absent)
🤖 Prompt for AI Agents
In `@backend/models/certificateSchema.js` around lines 62 - 66, The certificateId
field is marked unique which causes duplicate-key errors for multiple
non-approved docs; remove the unique: true from the certificateId field in the
certificateSchema and instead add a partial unique index on certificateId that
only applies when status === "Approved" (ensure the index also requires
certificateId exists). Locate the schema definition (certificateId in
certificateSchema / CertificateSchema) and replace the field-level unique
constraint with a schema-level index using partialFilterExpression for
status:"Approved" (and certificateId existence) so only approved documents
enforce uniqueness.
| const feedbackId = req.params.id; | ||
| const { actions_taken, resolved_by } = req.body; | ||
| console.log(req.body); | ||
| console.log("User resolving feedback:", resolved_by); | ||
|
|
||
| if (!actions_taken || actions_taken.trim() === "") { | ||
| return res.status(400).json({ error: "Resolution comment is required." }); | ||
| } | ||
|
|
||
| if (feedback.is_resolved) { | ||
| return res.status(400).json({ error: "Feedback is already resolved." }); | ||
| } | ||
| try { | ||
| const feedback = await Feedback.findById(feedbackId); | ||
| if (!feedback) { | ||
| return res.status(404).json({ error: "Feedback not found" }); | ||
| } | ||
|
|
||
| feedback.is_resolved = true; | ||
| feedback.resolved_at = new Date(); | ||
| feedback.actions_taken = actions_taken; | ||
| feedback.resolved_by = resolved_by; | ||
| if (feedback.is_resolved) { | ||
| return res.status(400).json({ error: "Feedback is already resolved." }); | ||
| } | ||
|
|
||
| await feedback.save(); | ||
| feedback.is_resolved = true; | ||
| feedback.resolved_at = new Date(); | ||
| feedback.actions_taken = actions_taken; | ||
| feedback.resolved_by = resolved_by; | ||
|
|
There was a problem hiding this comment.
Derive resolved_by from the authenticated user, not the request body.
Clients can spoof resolved_by today. Use req.user and ignore the body field to preserve audit integrity.
🔐 Proposed fix
- const { actions_taken, resolved_by } = req.body;
+ const { actions_taken } = req.body;
+ const resolved_by = req.user?._id;
+ if (!resolved_by) {
+ return res.status(401).json({ error: "Unauthorized" });
+ }
console.log(req.body);
console.log("User resolving feedback:", resolved_by);📝 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.
| const feedbackId = req.params.id; | |
| const { actions_taken, resolved_by } = req.body; | |
| console.log(req.body); | |
| console.log("User resolving feedback:", resolved_by); | |
| if (!actions_taken || actions_taken.trim() === "") { | |
| return res.status(400).json({ error: "Resolution comment is required." }); | |
| } | |
| if (feedback.is_resolved) { | |
| return res.status(400).json({ error: "Feedback is already resolved." }); | |
| } | |
| try { | |
| const feedback = await Feedback.findById(feedbackId); | |
| if (!feedback) { | |
| return res.status(404).json({ error: "Feedback not found" }); | |
| } | |
| feedback.is_resolved = true; | |
| feedback.resolved_at = new Date(); | |
| feedback.actions_taken = actions_taken; | |
| feedback.resolved_by = resolved_by; | |
| if (feedback.is_resolved) { | |
| return res.status(400).json({ error: "Feedback is already resolved." }); | |
| } | |
| await feedback.save(); | |
| feedback.is_resolved = true; | |
| feedback.resolved_at = new Date(); | |
| feedback.actions_taken = actions_taken; | |
| feedback.resolved_by = resolved_by; | |
| const feedbackId = req.params.id; | |
| const { actions_taken } = req.body; | |
| const resolved_by = req.user?._id; | |
| if (!resolved_by) { | |
| return res.status(401).json({ error: "Unauthorized" }); | |
| } | |
| console.log(req.body); | |
| console.log("User resolving feedback:", resolved_by); | |
| if (!actions_taken || actions_taken.trim() === "") { | |
| return res.status(400).json({ error: "Resolution comment is required." }); | |
| } | |
| try { | |
| const feedback = await Feedback.findById(feedbackId); | |
| if (!feedback) { | |
| return res.status(404).json({ error: "Feedback not found" }); | |
| } | |
| if (feedback.is_resolved) { | |
| return res.status(400).json({ error: "Feedback is already resolved." }); | |
| } | |
| feedback.is_resolved = true; | |
| feedback.resolved_at = new Date(); | |
| feedback.actions_taken = actions_taken; | |
| feedback.resolved_by = resolved_by; |
🤖 Prompt for AI Agents
In `@backend/routes/feedbackRoutes.js` around lines 189 - 212, The route currently
trusts req.body.resolved_by which allows spoofing; instead ignore the body field
and derive the resolver from the authenticated user (req.user). In the feedback
resolution handler (where feedbackId, actions_taken, resolved_by are read and
Feedback.findById is called), remove usage of req.body.resolved_by and set
feedback.resolved_by = req.user.id or req.user.email (whichever is the canonical
identifier in your auth model) after verifying req.user exists; keep validation
for actions_taken and the rest of the update logic, and ensure the API does not
accept or persist a client-supplied resolved_by.
| router.delete("/photo-delete", isAuthenticated, async (req, res) => { | ||
| try { | ||
| const { ID_No } = req.query; // Get ID_No from frontend for DELETE | ||
| if (!ID_No) { return res.status(400).json({ error: "ID_No is required" }); } | ||
| if (!ID_No) { | ||
| return res.status(400).json({ error: "ID_No is required" }); | ||
| } | ||
|
|
||
| const user = await user.findOne({ user_id: ID_No }); | ||
| if (!user) { return res.status(404).json({ error: "User not found" }); } | ||
| if (!user) { | ||
| return res.status(404).json({ error: "User not found" }); | ||
| } |
There was a problem hiding this comment.
Fix User lookup (Line 87 uses an undefined identifier).
Line 87 uses user.findOne(...), but user isn’t defined. This will throw at runtime.
🐛 Fix the model reference
- const user = await user.findOne({ user_id: ID_No });
+ const user = await User.findOne({ user_id: ID_No });📝 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.
| router.delete("/photo-delete", isAuthenticated, async (req, res) => { | |
| try { | |
| const { ID_No } = req.query; // Get ID_No from frontend for DELETE | |
| if (!ID_No) { return res.status(400).json({ error: "ID_No is required" }); } | |
| if (!ID_No) { | |
| return res.status(400).json({ error: "ID_No is required" }); | |
| } | |
| const user = await user.findOne({ user_id: ID_No }); | |
| if (!user) { return res.status(404).json({ error: "User not found" }); } | |
| if (!user) { | |
| return res.status(404).json({ error: "User not found" }); | |
| } | |
| router.delete("/photo-delete", isAuthenticated, async (req, res) => { | |
| try { | |
| const { ID_No } = req.query; // Get ID_No from frontend for DELETE | |
| if (!ID_No) { | |
| return res.status(400).json({ error: "ID_No is required" }); | |
| } | |
| const user = await User.findOne({ user_id: ID_No }); | |
| if (!user) { | |
| return res.status(404).json({ error: "User not found" }); | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 87-87: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🤖 Prompt for AI Agents
In `@backend/routes/profile.js` around lines 80 - 90, The code in the
router.delete("/photo-delete", isAuthenticated, async (req, res) => { ... })
handler calls user.findOne(...) but `user` is undefined; update the handler to
use the correct User model reference (e.g., `User.findOne(...)`) and ensure the
model is imported/required at the top (or rename the local variable to avoid
shadowing, e.g., `const foundUser = await User.findOne({ user_id: ID_No })`),
then update subsequent references to use that variable (`foundUser`) instead of
the undefined `user`.
backend/utils/batchValidate.js
Outdated
| const zodObjectId = zod.string().regex(/^[0-9a-zA-Z]{24}$/, "Invalid ObjectId"); | ||
|
|
||
| const validateBatchSchema = zod.object({ | ||
| title: zod.string().min(5, "Title is required"), | ||
| unit_id: zodObjectId, | ||
| commonData: zod.record(zod.string(), zod.string()), | ||
| template_id: zod.string(), | ||
| users: zod.array(zodObjectId).min(1, "Atleast 1 user must be associated."), |
There was a problem hiding this comment.
ObjectId regex is too permissive.
[0-9a-zA-Z]{24} accepts non-hex characters; Mongo ObjectIds are hex-only.
✅ Proposed fix
-const zodObjectId = zod.string().regex(/^[0-9a-zA-Z]{24}$/, "Invalid ObjectId");
+const zodObjectId = zod
+ .string()
+ .regex(/^[a-f0-9]{24}$/i, "Invalid ObjectId");📝 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.
| const zodObjectId = zod.string().regex(/^[0-9a-zA-Z]{24}$/, "Invalid ObjectId"); | |
| const validateBatchSchema = zod.object({ | |
| title: zod.string().min(5, "Title is required"), | |
| unit_id: zodObjectId, | |
| commonData: zod.record(zod.string(), zod.string()), | |
| template_id: zod.string(), | |
| users: zod.array(zodObjectId).min(1, "Atleast 1 user must be associated."), | |
| const zodObjectId = zod | |
| .string() | |
| .regex(/^[a-f0-9]{24}$/i, "Invalid ObjectId"); | |
| const validateBatchSchema = zod.object({ | |
| title: zod.string().min(5, "Title is required"), | |
| unit_id: zodObjectId, | |
| commonData: zod.record(zod.string(), zod.string()), | |
| template_id: zod.string(), | |
| users: zod.array(zodObjectId).min(1, "Atleast 1 user must be associated."), |
🤖 Prompt for AI Agents
In `@backend/utils/batchValidate.js` around lines 3 - 10, The zodObjectId regex is
too permissive (allows non-hex chars); update the zodObjectId used by
validateBatchSchema to enforce hex-only ObjectIds by replacing the pattern with
a hex-only regex (e.g., /^[0-9a-fA-F]{24}$/) so zodObjectId =
zod.string().regex(...); ensure the same updated zodObjectId is used in
validateBatchSchema for unit_id and users array elements.
…ded and gracefully handle failure
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/routes/auth.js (1)
221-231:⚠️ Potential issue | 🔴 Critical
user.setPasswordwill crash — passport-local-mongoose plugin was removed.The
setPasswordmethod was provided by thepassport-local-mongooseplugin, which has been removed from the user schema in this PR. Calling it will throwTypeError: user.setPassword is not a function, breaking the password reset flow entirely. Replace with bcrypt hashing + save.Proposed fix
jwt.verify(token, secret); - user.setPassword(password, async (error) => { - if (error) { - return res.status(500).json({ message: "Error resetting password" }); - } - await user.save(); - return res - .status(200) - .json({ message: "Password has been reset successfully" }); - }); + const hashedPassword = await bcrypt.hash(password, Number(process.env.SALT)); + user.password = hashedPassword; + await user.save(); + return res + .status(200) + .json({ message: "Password has been reset successfully" });Note: You'll need to add
const bcrypt = require("bcrypt");at the top of the file if not already imported (it is imported at Line 14).
🤖 Fix all issues with AI agents
In `@backend/controllers/certificateController.js`:
- Around line 52-61: PositionHolder.findOne({ user_id: id }) can return an
arbitrary position when a user has multiple positions; change the check so you
verify the user holds a position for the specific unit being requested (either
use PositionHolder.findOne({ user_id: id, unit_id }) or fetch all with
PositionHolder.find({ user_id: id }) and assert one of the returned documents
has unit_id matching the requested unit_id) and then base the authorization
logic (the positionHolder variable and subsequent unit_id comparison) on that
filtered result.
- Around line 10-178: The createBatch async handler lacks error handling—wrap
the entire function body of createBatch in a try-catch (or use a top-level async
error wrapper) so any thrown errors from DB calls (User.findById,
PositionHolder.findOne, Position.findById, OrganizationalUnit.findById,
CertificateBatch.create, etc.) are caught; in the catch block log the error and
send a 500 response (e.g., res.status(500).json({ message: "Internal server
error" })) or call next(err) to pass to Express error middleware, ensuring all
existing early returns and validation responses remain unchanged.
In `@backend/routes/auth.js`:
- Around line 59-65: The cookie handling around jwt.sign/res.cookie is
inconsistent and insecure: align the cookie lifetime with the JWT expiry (make
res.cookie maxAge match the token expiresIn of "1h" or reduce the token expiry
to match the cookie), and add security flags when calling res.cookie — set
httpOnly: true, sameSite: 'lax' or 'strict', and secure: true in production (use
NODE_ENV === 'production' to conditionally enable secure). Ensure you still sign
the token with process.env.JWT_SECRET_TOKEN via jwt.sign and only adjust the
options passed to res.cookie to reflect the new maxAge and security flags.
- Around line 95-98: The bcrypt.hash call uses Number(process.env.SALT) which
becomes NaN if SALT is unset; change this to validate and/or provide a fallback
before calling bcrypt.hash: read process.env.SALT into a constant (e.g., const
saltEnv = process.env.SALT), parse it safely (e.g., const saltRounds =
Number.isInteger(Number(saltEnv)) ? Number(saltEnv) : <defaultIntLike10>), or
throw a clear startup error if invalid, then call bcrypt.hash(password,
saltRounds); alternatively add a startup validation that ensures
process.env.SALT is a valid integer and fails fast with a descriptive message so
hashedPassword = await bcrypt.hash(...) never receives NaN.
- Around line 100-112: The registration handler currently sends the full
Mongoose document returned by User.create (newUser) back to the client, leaking
the password hash; change the behavior so the response excludes sensitive fields
by either creating the user with a projection that omits "password" (or calling
.select('-password')/using create then refetch without password) or by
converting newUser to a plain object (e.g., newUser.toObject() or toJSON()) and
deleting the password and any other sensitive fields before calling res.json({
message: "Registered Successfully", user: safeUser }); update the code that
returns res.json(...) to use this sanitized object instead of newUser.
In `@backend/utils/batchValidate.js`:
- Around line 5-11: The validateBatchSchema's messages and constraints need
tightening: update the title validator in validateBatchSchema to use a clearer
message such as "Title must be at least 5 characters" instead of "Title is
required"; add .min(1) (or equivalent non-empty constraint) to template_id to
prevent empty strings being accepted; and fix the users array error message typo
from "Atleast 1 user must be associated." to "At least 1 user must be
associated." so the validators for title, template_id, and users are accurate
and user-friendly.
🧹 Nitpick comments (2)
backend/controllers/certificateController.js (1)
177-177: Return201 Createdfor successful resource creation.
res.json(...)sends a 200 by default. APOSTthat creates a new resource should conventionally return201.- res.json({ message: "New Batch created successfully", details: newBatch }); + res.status(201).json({ message: "New Batch created successfully", details: newBatch });backend/models/schema.js (1)
223-279:positionHolderSchemastill uses manual timestamps while sibling schemas use{ timestamps: true }.The updated schemas (
userSchema,organizationalUnitSchema,positionSchema) use Mongoose's automatictimestamps: true, butpositionHolderSchema(and several others) still define manualcreated_at/updated_atfields. Consider migrating these for consistency, or note it as a follow-up task.
| const positionHolder = await PositionHolder.findOne({ user_id: id }); | ||
| //console.log(positionHolder._id); | ||
| if (!positionHolder) { | ||
| return res | ||
| .status(403) | ||
| .json({ | ||
| message: | ||
| "Unauthorized to do the task as user doesn't hold any position in any unit", | ||
| }); | ||
| } |
There was a problem hiding this comment.
findOne returns an arbitrary position — a user holding multiple positions may bypass the unit check.
If a coordinator holds positions in multiple clubs, PositionHolder.findOne({ user_id: id }) returns whichever document Mongo finds first. The subsequent unit_id comparison at Line 74 could then fail or succeed unpredictably depending on which position is returned. Consider filtering by the requested unit_id directly, or fetching all positions and verifying the requested unit is among them.
🤖 Prompt for AI Agents
In `@backend/controllers/certificateController.js` around lines 52 - 61,
PositionHolder.findOne({ user_id: id }) can return an arbitrary position when a
user has multiple positions; change the check so you verify the user holds a
position for the specific unit being requested (either use
PositionHolder.findOne({ user_id: id, unit_id }) or fetch all with
PositionHolder.find({ user_id: id }) and assert one of the returned documents
has unit_id matching the requested unit_id) and then base the authorization
logic (the positionHolder variable and subsequent unit_id comparison) on that
filtered result.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/seed.js (2)
99-127:⚠️ Potential issue | 🟠 MajorCommenting out test entity creation breaks all downstream seed functions.
With this block commented out,
seedPositions(line 224–226),seedPositionHolders(line 273),seedEvents(line 399), andseedFeedbacks(line 520) all query for test units via/_TEST/or/Test/patterns and will find nothing. This means positions, position holders, events, achievements, and feedback will never be seeded — the script silently succeeds but produces an incomplete database.Either remove the downstream test-seeding functions or replace the block comment with a feature flag / CLI argument so the test data path can be toggled intentionally.
192-213:⚠️ Potential issue | 🔴 CriticalRemove the
passwordfield from the seeded user data object.The User schema (lines 22-28 in
backend/models/schema.js) explicitly defines apasswordfield that will persist to the database. When you setpassword: passwordin the userData object and pass it tonew User(userData), the plain-text password is stored in the document. AlthoughUser.register()hashes the password into separatehashandsaltfields, it does not clear the pre-existingpasswordfield—both the plain text and hash are persisted, creating a security vulnerability.Since
User.register(user, password)receives the password as its second argument, the field should not be included in the data object:Proposed fix
localAuthUsers.push({ username: userEmail, - password: password, role: "STUDENT", strategy: "local", onboardingComplete: true, personal_info: { name: `Demo Student ${i}`, email: userEmail }, academic_info: academic_info, });
🤖 Fix all issues with AI agents
In `@backend/controllers/certificateController.js`:
- Line 178: The response for creating the new batch currently uses res.json(...)
which returns HTTP 200; change the controller to return HTTP 201 for resource
creation by replacing the call with res.status(201).json({ message: "New Batch
created successfully", details: newBatch }); — update the response where
newBatch is returned (the controller action that calls res.json) to use
res.status(201).json(...) instead of res.json(...).
- Around line 139-143: The code calls councilObj.category.toUpperCase() which
throws if category is null/undefined; before calling toUpperCase (in the block
that defines category and looks up gensecObj via User.findOne), add an explicit
guard that checks councilObj.category (or the derived category variable) and
return a 400/validation error response with a clear message if it's missing,
otherwise call toUpperCase on the confirmed string (use a new variable like
categoryUpper) and use that for the User.findOne(`GENSEC_${categoryUpper}`)
lookup; ensure the check is performed before computing gensecObj and before the
existing approver-not-found 500 branch so clients get a useful 4xx error when
category is absent.
In `@backend/routes/auth.js`:
- Around line 59-69: The login handler sets the JWT only as an httpOnly cookie
via res.cookie but the jwtIsAuthenticated middleware only reads Authorization
Bearer tokens, so clients cannot send a Bearer header; fix by either returning
the token in the response body from the login handler (augment res.json in the
login route to include token) or update jwtIsAuthenticated to fall back to
reading req.cookies.token when Authorization header is missing (check
req.cookies.token before rejecting). Ensure references to the token variable
created with jwt.sign, the res.cookie call, and the jwtIsAuthenticated
middleware are updated consistently.
- Around line 32-51: The reset-password handler currently calls
user.setPassword(...) which no longer exists (removed passport-local-mongoose);
replace that logic by hashing the new password with bcrypt.hash using the same
SALT logic as registration (use Number(process.env.SALT) || 12), assign the
hashed value to user.password, and call await user.save(); ensure any callbacks
are converted to async/await and remove setPassword references so the handler
uses bcrypt.hash and user.save directly (look for symbols: user.setPassword,
reset-password handler, bcrypt.hash, SALT, user.save).
🧹 Nitpick comments (5)
backend/seed.js (1)
582-585: Remove deprecated Mongoose connection options.The project uses Mongoose 7.6.8, which no longer supports or recognizes
useNewUrlParseranduseUnifiedTopology. These options have had no effect since MongoDB Node.js driver v4.0.0 and may generate deprecation warnings. Passing them serves no purpose and should be removed.Proposed fix
- await mongoose.connect(process.env.MONGODB_URI, { - useNewUrlParser: true, - useUnifiedTopology: true, - }); + await mongoose.connect(process.env.MONGODB_URI);backend/controllers/certificateController.js (2)
147-166: N+1 database queries for user validation — prefer a bulk lookup.Each user ID triggers an individual
User.findByIdcall. For a batch of n users, this makes n separate DB round-trips. Replace with a single bulk query and then diff the result set:Proposed refactor
- const userChecks = await Promise.all( - users.map(async (uid) => { - const validation = zodObjectId.safeParse(uid); - if (!validation.success) { - return { uid, ok: false, reason: "Invalid ID" }; - } - - const userObj = await User.findById(uid); - if (!userObj) return { uid, ok: false, reason: "User not found" }; - - return { uid, ok: true }; - }), - ); - - const invalidData = userChecks.filter((c) => !c.ok); - if (invalidData.length > 0) { - return res - .status(400) - .json({ message: "Invalid user data sent", details: invalidData }); - } + // 1. Validate all IDs are well-formed ObjectIds + const invalidIds = users.filter((uid) => !zodObjectId.safeParse(uid).success); + if (invalidIds.length > 0) { + return res.status(400).json({ + message: "Invalid user data sent", + details: invalidIds.map((uid) => ({ uid, ok: false, reason: "Invalid ID" })), + }); + } + + // 2. Single bulk lookup + const foundUsers = await User.find({ _id: { $in: users } }).select("_id").lean(); + const foundIdSet = new Set(foundUsers.map((u) => u._id.toString())); + const missingIds = users.filter((uid) => !foundIdSet.has(uid)); + if (missingIds.length > 0) { + return res.status(400).json({ + message: "Invalid user data sent", + details: missingIds.map((uid) => ({ uid, ok: false, reason: "User not found" })), + }); + }
12-12: Remove commented-out debug logging.There are 9 commented-out
console.loglines scattered throughout the function (lines 12, 51, 54, 65, 82, 90, 98, 114, 138). These are development artifacts that add noise — remove them before merging.Also applies to: 51-51, 54-54, 65-65, 82-82, 90-90, 98-98, 114-114, 138-138
backend/routes/auth.js (2)
4-14: Remove commented-out import (dead code).Line 5 (
//const isIITBhilaiEmail = ...) is commented-out dead code. If the IIT Bhilai email check was intentionally removed from this route, delete the line entirely rather than leaving it commented out. Note that theloginValidateandregisterValidateZod schemas inbackend/utils/authValidate.jsstill enforce the@iitbhilai.ac.indomain via regex, so the domain gating is already handled at the validation layer.Proposed cleanup
const jwt = require("jsonwebtoken"); - -//const isIITBhilaiEmail = require("../utils/isIITBhilaiEmail"); - + const { loginValidate, registerValidate } = require("../utils/authValidate");
53-57: Remove commented-out debug line.Line 57 (
//console.log(payload)) is leftover debug code. Clean it up.
| const category = councilObj.category.toUpperCase(); | ||
| const gensecObj = await User.findOne({ role: `GENSEC_${category}` }); | ||
| if (!gensecObj || !presidentObj) { | ||
| return res.status(500).json({ message: "Approvers not found" }); | ||
| } |
There was a problem hiding this comment.
Guard against councilObj.category being null/undefined.
If category is missing on the council document, councilObj.category.toUpperCase() throws a TypeError. The outer try-catch catches it, but the client gets a generic 500 instead of a useful error. Add an explicit check.
Proposed fix
+ if (!councilObj.category) {
+ return res
+ .status(500)
+ .json({ message: "Council category is not defined" });
+ }
const category = councilObj.category.toUpperCase();
const gensecObj = await User.findOne({ role: `GENSEC_${category}` });🤖 Prompt for AI Agents
In `@backend/controllers/certificateController.js` around lines 139 - 143, The
code calls councilObj.category.toUpperCase() which throws if category is
null/undefined; before calling toUpperCase (in the block that defines category
and looks up gensecObj via User.findOne), add an explicit guard that checks
councilObj.category (or the derived category variable) and return a
400/validation error response with a clear message if it's missing, otherwise
call toUpperCase on the confirmed string (use a new variable like categoryUpper)
and use that for the User.findOne(`GENSEC_${categoryUpper}`) lookup; ensure the
check is performed before computing gensecObj and before the existing
approver-not-found 500 branch so clients get a useful 4xx error when category is
absent.
| users, | ||
| }); | ||
|
|
||
| res.json({ message: "New Batch created successfully", details: newBatch }); |
There was a problem hiding this comment.
Return 201 for resource creation.
res.json(...) sends a 200 by default. REST convention for successful resource creation is 201 Created.
Proposed fix
- res.json({ message: "New Batch created successfully", details: newBatch });
+ res.status(201).json({ message: "New Batch created successfully", details: newBatch });📝 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.
| res.json({ message: "New Batch created successfully", details: newBatch }); | |
| res.status(201).json({ message: "New Batch created successfully", details: newBatch }); |
🤖 Prompt for AI Agents
In `@backend/controllers/certificateController.js` at line 178, The response for
creating the new batch currently uses res.json(...) which returns HTTP 200;
change the controller to return HTTP 201 for resource creation by replacing the
call with res.status(201).json({ message: "New Batch created successfully",
details: newBatch }); — update the response where newBatch is returned (the
controller action that calls res.json) to use res.status(201).json(...) instead
of res.json(...).
| router.post("/login", async (req, res) => { | ||
| try { | ||
| const { username, password } = req.body; | ||
| const result = loginValidate.safeParse({ username, password }); | ||
|
|
||
| if (!result.success) { | ||
| return res | ||
| .status(400) | ||
| .json({ message: result.error.message || "Invalid data sent" }); | ||
| } | ||
|
|
||
| const user = await User.findOne({ username }); | ||
| if (!user) { | ||
| return res.status(401).json({ message: "Invalid user credentials" }); | ||
| } | ||
|
|
||
| const isValid = await bcrypt.compare(password, user.password); | ||
| if (!isValid) { | ||
| return res.status(401).json({ message: "Invalid user credentials" }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if passport-local-mongoose is still in dependencies
echo "=== Checking package.json for passport-local-mongoose ==="
find . -name "package.json" -type f | head -5 | xargs grep -l "passport-local-mongoose" 2>/dev/null || echo "Not found in package.json files"
echo ""
echo "=== Checking User schema file ==="
fd "user\|User" --type js -E node_modules | head -10
echo ""
echo "=== Checking for setPassword usage ==="
rg -n "setPassword" --type js -A 2 -B 2
echo ""
echo "=== Checking password-reset endpoint ==="
rg -n "password.*reset\|forgot.*password" --type js -i -A 10 | head -40Repository: OpenLake/Student_Database_COSA
Length of output: 4264
🏁 Script executed:
#!/bin/bash
# Read auth.js around line 224 to see full password-reset endpoint
echo "=== Password-reset endpoint in auth.js (lines 210-240) ==="
sed -n '210,240p' backend/routes/auth.js
echo ""
echo "=== Find User schema file ==="
fd "user\|schema" -i --type f -E node_modules -E dist | grep -i "model\|schema\|db"
echo ""
echo "=== Check models directory ==="
ls -la backend/models/ 2>/dev/null || echo "models/ not found"
echo ""
echo "=== Search for User model/schema definition ==="
rg -n "schema.*user\|User.*=.*schema\|plugin.*passport" -i --type-list 2>&1 | head -5Repository: OpenLake/Student_Database_COSA
Length of output: 1494
🏁 Script executed:
#!/bin/bash
# More flexible search for User schema
echo "=== Looking for User model with proper file type ==="
fd "\.js$" backend/models backend/db 2>/dev/null
echo ""
echo "=== Find where User is defined/imported ==="
rg -n "new Schema\|UserSchema\|export.*User" backend/ | head -20Repository: OpenLake/Student_Database_COSA
Length of output: 232
🏁 Script executed:
#!/bin/bash
# Read the User schema definition
echo "=== Reading backend/models/schema.js ==="
cat -n backend/models/schema.js | head -100
echo ""
echo "=== Check if passport-local-mongoose is imported/used ==="
grep -n "passport\|setPassword" backend/models/schema.jsRepository: OpenLake/Student_Database_COSA
Length of output: 2810
🏁 Script executed:
#!/bin/bash
# Check how registration endpoint handles password to see the correct pattern
echo "=== Finding register endpoint in auth.js ==="
rg -n "register" -i backend/routes/auth.js | head -5
echo ""
echo "=== Show register endpoint (lines 52-100 approx) ==="
sed -n '52,120p' backend/routes/auth.js | cat -n
echo ""
echo "=== Check bcrypt usage in auth.js ==="
rg -n "bcrypt" backend/routes/auth.jsRepository: OpenLake/Student_Database_COSA
Length of output: 2636
Update password-reset endpoint to use bcrypt instead of removed passport-local-mongoose.
At line 224, the reset-password handler calls user.setPassword(...), which is a method from the passport-local-mongoose plugin. Since this dependency was removed from package.json, that method will not exist at runtime, causing the password-reset feature to fail with a TypeError.
Update the reset-password endpoint to match the password-hashing pattern used in the register endpoint: hash with bcrypt.hash() and save directly with user.save(), rather than relying on the removed plugin.
Example fix pattern (from register endpoint)
const salt = Number(process.env.SALT) || 12;
const hashedPassword = await bcrypt.hash(password, salt);
user.password = hashedPassword;
await user.save();🤖 Prompt for AI Agents
In `@backend/routes/auth.js` around lines 32 - 51, The reset-password handler
currently calls user.setPassword(...) which no longer exists (removed
passport-local-mongoose); replace that logic by hashing the new password with
bcrypt.hash using the same SALT logic as registration (use
Number(process.env.SALT) || 12), assign the hashed value to user.password, and
call await user.save(); ensure any callbacks are converted to async/await and
remove setPassword references so the handler uses bcrypt.hash and user.save
directly (look for symbols: user.setPassword, reset-password handler,
bcrypt.hash, SALT, user.save).
| const token = jwt.sign(payload, process.env.JWT_SECRET_TOKEN, { | ||
| expiresIn: "1h", | ||
| }); | ||
|
|
||
| res.cookie("token", token, { | ||
| maxAge: 60 * 60 * 1000, | ||
| httpOnly: true, | ||
| sameSite: "lax" | ||
| }); | ||
|
|
||
| res.json({ message: "Login Successful"}); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f -e js | xargs grep -l "jwtIsAuthenticated" 2>/dev/null | head -20Repository: OpenLake/Student_Database_COSA
Length of output: 148
🏁 Script executed:
cat -n backend/middlewares/isAuthenticated.jsRepository: OpenLake/Student_Database_COSA
Length of output: 2711
Token retrieval mismatch: httpOnly cookie cannot be used with jwtIsAuthenticated middleware.
The login handler sets the JWT exclusively in an httpOnly cookie, but the jwtIsAuthenticated middleware reads the token only from the Authorization: Bearer <token> header (see backend/middlewares/isAuthenticated.js, lines 15-18). Since JavaScript cannot access httpOnly cookies and the response body does not include the token, clients have no way to obtain and send the token as a Bearer header.
To fix this, either:
- Also return the token in the response body (e.g.,
res.json({ message: "Login Successful", token })), or - Update
jwtIsAuthenticatedto also checkreq.cookies.tokenas a fallback.
🤖 Prompt for AI Agents
In `@backend/routes/auth.js` around lines 59 - 69, The login handler sets the JWT
only as an httpOnly cookie via res.cookie but the jwtIsAuthenticated middleware
only reads Authorization Bearer tokens, so clients cannot send a Bearer header;
fix by either returning the token in the response body from the login handler
(augment res.json in the login route to include token) or update
jwtIsAuthenticated to fall back to reading req.cookies.token when Authorization
header is missing (check req.cookies.token before rejecting). Ensure references
to the token variable created with jwt.sign, the res.cookie call, and the
jwtIsAuthenticated middleware are updated consistently.
1 participant
name: " Pull Request"
about: Propose and submit changes to the project for review
title: "PR: Implement createBatch endpoint ( /api/certificate-batch/ )"
labels: ""
assignees: harshitap1305, sakshi1755
Related Issue
Changes Introduced
isAuthenticatedmiddleware that verifies JWT from the Authorization header and attaches the verified user toreq.user.createBatchcontroller (implemented from scratch) to create certificate batches for a club with server-side approver resolution.validateBatchSchema,zodObjectId) and return proper 4xx/5xx responses on validation/auth errors.zodObjectId+ DB existence checks) for theusersarray before batch creation.Why This Change?
createBatchflow that:CertificateBatchdocument only when all checks passScreenshots
Testing
npm testin the relevant directory).Documentation Updates
README.mdwith new instructions.Checklist
git checkout -b feature/my-amazing-feature).Deployment Notes
Summary by CodeRabbit
New Features
Bug Fixes
Chores