Fixed Issue #6#44
Conversation
…o only iiitl.ac.in emails using resend
|
Someone is attempting to deploy a commit to the mrimmortal09's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a MongoDB-backed authentication system (NextAuth + custom handlers), client auth pages and wrappers, rate-limiting and logging, password reset/set flows, middleware for Google redirect handling, and supporting DB models/utilities. Providers are wired into the app layout and Navbar is session-aware. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant Client as App Client
participant RateLimit as RateLimit API
participant NextAuth as NextAuth Handler
participant Email as Email Service
participant DB as MongoDB
participant SetPwd as SetPassword API
User->>Browser: Open /login
Browser->>Client: Render LoginClient
User->>Client: Submit email (and optional password)
Client->>RateLimit: POST /api/rate-limit (email)
RateLimit-->>Client: { allowed: true/false }
alt rate limited
Client-->>Browser: Show rate-limit error
else allowed
Client->>NextAuth: signIn("email" or "credentials" or "google")
NextAuth->>DB: lookup/create user, staging for Google, create session record
NextAuth->>Email: send magic link (email provider)
Email-->>User: Magic link delivered
User->>NextAuth: Click magic link -> verify
NextAuth->>DB: create session, set needsPassword if user.password missing
NextAuth-->>Browser: Session cookie / response
Browser->>Client: HomeWrapper observes session
alt needsPassword true
Client->>Browser: Redirect /set-password?email=...
Browser->>SetPwd: POST /api/set-password (email, password)
SetPwd->>DB: hash & insert user, delete staging
SetPwd-->>Browser: { success: true }
Browser->>Client: Reload -> authenticated without needsPassword
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
app/layout.tsx (1)
6-6: Remove inline “✅ ADD THIS” marker from production code.This looks like a temporary review note and should be cleaned up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/layout.tsx` at line 6, Remove the inline review marker from the import statement in app/layout.tsx: change the import line that currently reads "import Providers from \"./providers\"; // ✅ ADD THIS" to a clean import without the comment; ensure only the module import for Providers remains and no review-note comment is left in the file so the symbol Providers is imported normally and production code contains no temporary markers.lib/logger.ts (1)
17-24: Use a single MongoDB access layer in this helper.
clientPromiseis awaited but never used, andmongoose.connect()opens a separate connection anyway. Keeping both the native client and Mongoose on the auth/logging path adds extra connection lifecycle complexity without a payoff.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/logger.ts` around lines 17 - 24, The code awaits clientPromise but never uses it while also calling mongoose.connect, causing duplicate MongoDB access layers; pick one and remove the other. Either remove the clientPromise await and rely solely on mongoose by ensuring mongoose.connect(process.env.MONGODB_URI!) is used before calling Log.create, or stop using mongoose entirely and use the native client from clientPromise to insert into the logs collection (replace Log.create with an insertOne on the client.db(...).collection(...)). Update references to clientPromise, mongoose.connect, and Log.create accordingly so only one connection layer is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/rate-limit/route.ts`:
- Around line 5-9: The handler currently does unguarded await req.json() and
passes whatever value came back into checkRateLimit, causing 500s or polluted
limiter state; wrap the JSON parse in a try/catch to return a 400/invalid-body
response on parse errors, then validate that the parsed object has an email
property of type string (non-empty) before calling checkRateLimit(email); if
validation fails return a 400 with a clear error message instead of invoking
checkRateLimit, and otherwise call checkRateLimit(email) and return
NextResponse.json(result).
In `@app/set-password/page.tsx`:
- Around line 7-25: The client currently reads email via searchParams and
includes it in the POST body (see searchParams, email, and handleSubmit which
calls fetch("/api/set-password")). Remove the email from the request payload and
stop deriving target account from the URL: send only the new password (or a
password + server-validated token) in the body, and let the API derive the user
from the authenticated session; update handleSubmit to omit email and rely on
the server-side /api/set-password to validate the session user instead of
trusting client-supplied email.
In `@lib/auth.ts`:
- Around line 67-71: The session handler currently sets
session.user.needsPassword based on !user.password which also catches magic-link
accounts; instead persist an explicit flag on the user record when the Google
OAuth sign-up flow creates the account (e.g., createdViaGoogle or
needsGoogleOnboarding) and use that flag in async session({ session, user }) to
decide whether to mark session.user.needsPassword; update the Google
OAuth/account-creation path to write that flag (and clear it after onboarding)
and replace the !user.password check in the session function with a check for
the explicit flag (referencing the session function and
session.user.needsPassword and the user creation callback that handles Google
signups).
- Around line 19-33: In sendVerificationRequest, validate the identifier email
domain before attempting to send: use the existing identifier parameter and
check it endsWith("@iiitl.ac.in"); if it does not, log the event via
logEvent(identifier, "INVALID_DOMAIN") and throw an Error (so you reject
non-@iiitl.ac.in addresses early). Keep the existing rate-limit check
(checkRateLimit) before this domain check and ensure you do not call
resend.emails.send unless both rate limit and domain validation pass; reference
sendVerificationRequest, checkRateLimit, logEvent, and resend.emails.send when
making the change.
- Around line 12-46: Google OAuth users are being persisted immediately and
there is no credentials login or password-save endpoint; fix by (1) adding a
Credentials provider to the NextAuth providers array so password-based sign-in
is supported (add a Credentials(...) provider with authorize that verifies
hashed passwords against the Mongo user document), (2) implement the missing
POST /api/set-password handler to accept email+password, hash the password
(e.g., bcrypt) and save it to the existing user document (use the same user
model the MongoDBAdapter uses), and (3) decide how to prevent automatic
persistence of OAuth users—either remove the requirement for a password or
intercept the OAuth flow: implement NextAuth callbacks.signIn or override
adapter.createUser to stop creation for provider === 'google' and redirect to
your /set-password flow, or alternatively persist a minimal unverified record
and require set-password to finish account setup; update NextAuth handler (the
NextAuth(...) config in auth.ts), the Google provider config, and the Mongo
adapter/adapter.createUser or callbacks.signIn to implement the chosen behavior.
In `@lib/mongodb.ts`:
- Line 1: Remove the debugging console.log that prints the raw MongoDB URI to
avoid leaking credentials; locate the call to console.log("MONGODB_URI:",
process.env.MONGODB_URI) in lib/mongodb.ts and delete it (or replace with a
non-sensitive check that logs only presence/absence or a masked/hashed value) so
no raw connection string is emitted to logs.
In `@lib/rateLimit.ts`:
- Around line 1-23: The in-memory Map in checkRateLimit with raw email keys
won't work in multi-instance/serverless environments, allows case-based
bypasses, and never evicts keys; replace the process-local Map (requests) by
using a centralized store (e.g., Redis) and normalize the email inside
checkRateLimit (trim + toLowerCase) to prevent case bypasses, implement an
atomic Redis-based sliding window (either use INCR with EXPIRE per normalized
email or a ZSET pattern: ZREMRANGEBYSCORE old entries, ZADD current timestamp,
ZCOUNT window) to enforce the 5-per-hour limit across instances, and ensure keys
have an expiry/TTL so memory is reclaimed when no activity occurs.
In `@models/Log.ts`:
- Around line 3-7: The LogSchema currently stores raw emails indefinitely and
allows malformed documents; update the schema to enforce validation and
retention by: (1) make type an enum (e.g., allowed values like "auth_success"
and "auth_failure") and validate requiredness; (2) add an email field validator
(match a simple email regex and max length) and mark email required only for
auth event types; (3) add a TTL index on timestamp (e.g., expires after 90 days)
to auto-remove old auth logs; and (4) implement a pre('save') hook on LogSchema
that, for auth_* types, replaces the raw email with a deterministic hash (e.g.,
sha256) before persisting to avoid storing plaintext emails. Ensure you
reference LogSchema, timestamp, type, email, and the pre('save') hook when
making these changes.
In `@package.json`:
- Around line 13-21: The package.json depends on the Auth.js v5 adapter
(`@auth/mongodb-adapter`) which conflicts with next-auth v4; replace the
dependency "@auth/mongodb-adapter" with the NextAuth v4-compatible adapter
"@next-auth/mongodb-adapter" (or alternatively migrate the whole auth stack to
Auth.js v5) and adjust the "mongodb" version to match whatever the chosen
adapter requires, then reinstall (npm/yarn) and regenerate the lockfile; update
references/usages of the adapter import in your auth setup (where you reference
the adapter in next-auth configuration) to import from
"@next-auth/mongodb-adapter" so runtime and dependency versions align.
---
Nitpick comments:
In `@app/layout.tsx`:
- Line 6: Remove the inline review marker from the import statement in
app/layout.tsx: change the import line that currently reads "import Providers
from \"./providers\"; // ✅ ADD THIS" to a clean import without the comment;
ensure only the module import for Providers remains and no review-note comment
is left in the file so the symbol Providers is imported normally and production
code contains no temporary markers.
In `@lib/logger.ts`:
- Around line 17-24: The code awaits clientPromise but never uses it while also
calling mongoose.connect, causing duplicate MongoDB access layers; pick one and
remove the other. Either remove the clientPromise await and rely solely on
mongoose by ensuring mongoose.connect(process.env.MONGODB_URI!) is used before
calling Log.create, or stop using mongoose entirely and use the native client
from clientPromise to insert into the logs collection (replace Log.create with
an insertOne on the client.db(...).collection(...)). Update references to
clientPromise, mongoose.connect, and Log.create accordingly so only one
connection layer is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8cff0aa4-267a-4e25-85cf-0a9fd8938b91
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
app/HomeWrapper.tsxapp/api/auth/[...nextauth]/route.tsapp/api/rate-limit/route.tsapp/layout.tsxapp/login/LoginClient.tsxapp/login/page.tsxapp/page.tsxapp/providers.tsxapp/set-password/page.tsxlib/auth.tslib/logger.tslib/mongodb.tslib/rateLimit.tsmodels/Log.tspackage.jsonpnpm-workspace.yaml
| const LogSchema = new mongoose.Schema({ | ||
| email: String, | ||
| type: String, | ||
| timestamp: { type: Date, default: Date.now }, | ||
| }) |
There was a problem hiding this comment.
Add retention + validation constraints to auth logs.
This stores raw emails indefinitely and allows malformed log documents. For auth-failure logs, that is a privacy/compliance risk and weakens monitoring quality.
Proposed hardening
const LogSchema = new mongoose.Schema({
- email: String,
- type: String,
- timestamp: { type: Date, default: Date.now },
+ email: { type: String, required: true, lowercase: true, trim: true, index: true },
+ type: { type: String, required: true, index: true },
+ timestamp: { type: Date, default: Date.now, index: true },
})
+
+// Retain auth logs for 90 days (adjust to policy)
+LogSchema.index({ timestamp: 1 }, { expireAfterSeconds: 60 * 60 * 24 * 90 })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@models/Log.ts` around lines 3 - 7, The LogSchema currently stores raw emails
indefinitely and allows malformed documents; update the schema to enforce
validation and retention by: (1) make type an enum (e.g., allowed values like
"auth_success" and "auth_failure") and validate requiredness; (2) add an email
field validator (match a simple email regex and max length) and mark email
required only for auth event types; (3) add a TTL index on timestamp (e.g.,
expires after 90 days) to auto-remove old auth logs; and (4) implement a
pre('save') hook on LogSchema that, for auth_* types, replaces the raw email
with a deterministic hash (e.g., sha256) before persisting to avoid storing
plaintext emails. Ensure you reference LogSchema, timestamp, type, email, and
the pre('save') hook when making these changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/set-password/route.ts`:
- Around line 21-30: The current updateOne unconditionally overwrites password
and always returns success; modify the filter in
db.collection("users").updateOne to only match users who do not already have a
password (e.g., include password: { $exists: false } or password: null alongside
the email) so the update is atomic/one-time for the setup flow, then examine the
returned result (result.matchedCount / result.modifiedCount) and if no document
was modified return an error response (using NextResponse.json) instead of
success; reference db.collection("users").updateOne, the email and
hashedPassword variables, and the NextResponse.json call when implementing the
check.
- Around line 5-30: The POST handler currently allows any caller to set a
password for any email; restrict this by authenticating the request and using
the server-side session user identity instead of the client-supplied email. In
the POST function, validate the session (e.g., via getServerSession / JWT
verification) to obtain the authenticated user's email or id, reject
unauthenticated requests with 401, and call db.collection("users").updateOne
using that server-derived identifier (not the request body email); remove or
ignore the client-supplied email field to prevent account takeover while keeping
the password hashing (bcrypt.hash) and update logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8dd2396-d938-461e-98ed-5bcaad43c3ff
📒 Files selected for processing (5)
.gitignoreapp/HomeWrapper.tsxapp/api/set-password/route.tsapp/set-password/page.tsxlib/auth.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (3)
- app/HomeWrapper.tsx
- app/set-password/page.tsx
- lib/auth.ts
…m, along with debugging the accidental exposure of mongodb url.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
lib/auth.ts (1)
16-18:⚠️ Potential issue | 🔴 CriticalGoogle users still appear to be persisted before password setup finishes.
Using the stock
MongoDBAdapter(clientPromise)with Google OAuth means the adapter flow still owns user creation. I don't see anything here that defers persistence until/set-passwordcompletes, so the “save the user only after password creation” acceptance criterion still looks unmet.In Auth.js / NextAuth with MongoDBAdapter and Google OAuth, when is the user document created relative to callbacks.signIn and custom onboarding pages? What extension point can prevent persisting the user until a custom password-setup flow completes?Also applies to: 49-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/auth.ts` around lines 16 - 18, The current setup uses MongoDBAdapter(clientPromise) directly so NextAuth persists Google users immediately; to defer persistence until your /set-password flow you must wrap or replace the adapter's createUser behavior. Modify authOptions to use a custom adapter wrapper that delegates to MongoDBAdapter(clientPromise) but overrides createUser (and optionally linkAccount) to no-op or store the OAuth profile in the session when provider === 'google', and then call the original adapter.createUser later from your /set-password handler to insert the finalized user; also update callbacks.signIn to redirect Google logins to /set-password (returning false/redirect) so the flow uses the deferred createUser logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/login/route.ts`:
- Around line 15-26: The login API currently authenticates any user with a
stored password because it queries and compares password before enforcing the
institutional email restriction; reapply the same domain check used in
lib/auth.ts before querying/authenticating by validating the incoming email
(e.g., check email.endsWith("@iiitl.ac.in") or call the shared validation
helper) inside the route handler that uses clientPromise/db and the users
collection, and return a 403/400 JSON response if the domain is invalid so only
allowed-domain accounts proceed to db.collection("users").findOne(...) and
bcrypt.compare(...).
- Around line 52-55: The catch block in app/api/login/route.ts should stop
returning err.message and err.stack to clients; instead keep logging the
detailed error on the server (e.g., console.error("Login Error:", err) or use
your logger) and return a generic 500 response body such as { error: "Internal
server error" } using NextResponse.json with status 500; update the catch in the
login request handler (the catch surrounding the login logic / NextResponse
return) to remove any exposure of (err as Error).message or .stack.
In `@app/api/register/route.ts`:
- Around line 19-30: Normalize the incoming email once (e.g., const
canonicalEmail = email.trim().toLowerCase()) and use that canonicalEmail for the
domain check, the findOne lookup and the insertOne operation instead of the raw
client-supplied email; update references to existingUser and the
db.collection("users").insertOne call to use canonicalEmail so casing/whitespace
cannot create duplicate identities.
- Around line 18-24: The registration code currently returns 400 responses for
invalid email domains and duplicate accounts (seen around the email domain check
using the email variable and the NextResponse.json returns) but does not write
audit logs; update the /api/register flow to emit a structured audit/log entry
for every rejected sign-up (invalid-domain and duplicate-account paths)
immediately before each NextResponse.json error return, including the attempted
email, reason ("invalid_domain" or "duplicate_account"), client IP or request
identifier, and timestamp; reuse the existing application logger/audit function
used elsewhere (e.g., processLogger or requestLogger) so the log format matches
other audits and ensure the logging call appears in the same blocks as the
checks that currently return the NextResponse.json.
In `@components/Navbar.tsx`:
- Line 56: Update the signOut call in the Navbar onClick handler: replace the
deprecated option name callbackUrl with redirectTo and pass the same target
("/") to maintain the same redirect behavior; locate the onClick that calls
signOut (the anonymous handler in Navbar using signOut({ callbackUrl: "/" }))
and change the option key to redirectTo so signOut({ redirectTo: "/" }) is used
instead.
In `@lib/auth.ts`:
- Around line 25-37: The console.log calls that print the raw email identifier
(in the DOMAIN CHECK and RATE LIMIT CHECK blocks and the similar 67-69 area)
must be removed or replaced with non-PII messages; instead rely on
logEvent(identifier, ...) for audit/audit trails and use redacted or generic
stdout messages (e.g., "Domain rejected" or "Rate limit exceeded" without the
identifier) so raw emails are not written to application logs; update
occurrences around the domain check (identifier, logEvent) and the rate-limit
branch (checkRateLimit, rateLimitResult) as well as the repeated lines at 67-69
accordingly.
In `@lib/logger.ts`:
- Around line 6-10: The email schema in lib/logger.ts currently enforces
required: true and a domain match, which causes Log.create calls (from auth code
paths that log rejected or empty emails) to fail and drop monitoring data;
update the email field in the Log schema (the email property inside the model
definition for Log) to not enforce domain-restriction or requiredness for
logging purposes — e.g., make it optional (remove required: true or set
required: false) and remove or relax the match regex so Log.create can persist
empty or non-@iiitl.ac.in values generated by lib/auth.ts (the code paths that
call Log.create).
In `@lib/rateLimit.ts`:
- Around line 29-39: The current two-step check (RateLimit.countDocuments)
followed by insertion (RateLimit.create) is not atomic; replace this with a
single atomic update (e.g., RateLimit.findOneAndUpdate or RateLimit.updateOne
with upsert) that both checks the existing count within the past hour and
inserts/increments only when under the limit so concurrent requests cannot
bypass the 5-per-hour rule; specifically, change the logic around
RateLimit.countDocuments and RateLimit.create to use a query that matches the
email and enforces the timestamp window (or a per-email counter document with a
query like { email: normalizedEmail, count: { $lt: 5 } }) and an update that
atomically increments/creates and returns whether the operation succeeded,
falling back to allowed: false when the conditional update did not modify a
document.
---
Duplicate comments:
In `@lib/auth.ts`:
- Around line 16-18: The current setup uses MongoDBAdapter(clientPromise)
directly so NextAuth persists Google users immediately; to defer persistence
until your /set-password flow you must wrap or replace the adapter's createUser
behavior. Modify authOptions to use a custom adapter wrapper that delegates to
MongoDBAdapter(clientPromise) but overrides createUser (and optionally
linkAccount) to no-op or store the OAuth profile in the session when provider
=== 'google', and then call the original adapter.createUser later from your
/set-password handler to insert the finalized user; also update callbacks.signIn
to redirect Google logins to /set-password (returning false/redirect) so the
flow uses the deferred createUser logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d08cf266-09ee-442f-b4b4-b7ac59efd2d6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
app/api/login/route.tsapp/api/rate-limit/route.tsapp/api/register/route.tsapp/api/set-password/route.tsapp/login/LoginClient.tsxapp/register/RegisterClient.tsxapp/register/page.tsxapp/set-password/page.tsxcomponents/Navbar.tsxlib/auth.tslib/logger.tslib/mongodb.tslib/rateLimit.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/mongodb.ts
- app/api/rate-limit/route.ts
- app/set-password/page.tsx
- package.json
- app/api/set-password/route.ts
|
@coderabbitai Explain the flow for login, in how many ways can a user sign up? |
|
@coderabbitai Explain the Flow for forget passoword. |
|
Tip For best results, initiate chat on the files or code changes.
🔐 Sign-Up / Sign-In FlowsThere are 3 ways a user can sign up or sign in, all gated to 1. 📧 Email + Password (Traditional)Sign-Up: Sign-In: 2. 🔗 Magic Link (Passwordless Email)Sign-In (no password entered on /login):
3. 🟦 Google OAuthSign-Up / Sign-In: 🚦 Domain Enforcement Summary
🗺️ Flow Diagramflowchart TD
A[User visits /login or /register] --> B{Choose method}
B --> C[Email + Password]
B --> D[Magic Link]
B --> E[Google OAuth]
C -->|Register| F[POST /api/register\nValidate domain\nHash pw\nInsert user]
F --> G[POST /api/login\nCreate session\nSet cookie]
G --> Home["/ Home"]
C -->|Login| H[POST /api/login\nbcrypt.compare\nCreate session]
H --> Home
D --> I["POST /api/rate-limit\n<=5/hour check"]
I -->|Allowed| J[signIn email\nResend magic link]
J --> K[User clicks link\nNextAuth verifies]
K --> Home
I -->|Blocked| Err1[Show rate limit error]
E --> L[Google OAuth\nforce account selection]
L --> M{"profile.hd<br/>= iiitl.ac.in?"}
M -->|No| N[Log GOOGLE_REJECT<br/>Access Denied page]
M -->|Yes| O{"Has password<br/>in DB?"}
O -->|No| P["needsPassword=true<br/>Redirect /set-password"]
P --> Q["POST /api/set-password<br/>Hash & store pw"]
Q --> Home
O -->|Yes| Home
In summary: there are 3 sign-up paths — Email+Password, Google OAuth (IIITL domain only), and Magic Link — with failed non-IIITL attempts logged to MongoDB ( |
|
Tip For best results, initiate chat on the files or code changes.
🔑 Implicit "Forgot Password" via Magic LinksequenceDiagram
actor User
participant LoginPage as /login (LoginClient.tsx)
participant RateLimit as /api/rate-limit
participant NextAuth as NextAuth Email Provider (lib/auth.ts)
participant Resend as Resend (Email Service)
participant DB as MongoDB
User->>LoginPage: Enter `@iiitl.ac.in` email, leave password blank → Click "Sign in"
LoginPage->>RateLimit: POST { email }
RateLimit->>DB: Count requests in last 1 hour
alt >= 5 requests
RateLimit-->>LoginPage: { allowed: false } → Show "Too many requests" error
else < 5 requests
RateLimit->>DB: Insert new RateLimit document
RateLimit-->>LoginPage: { allowed: true }
LoginPage->>NextAuth: signIn("email", { email, callbackUrl: "/" })
NextAuth->>DB: Log event if domain invalid
NextAuth->>Resend: Send magic link to user's `@iiitl.ac.in` email
Resend-->>User: Email with sign-in link
LoginPage-->>User: "Check your email" screen
User->>NextAuth: Click magic link
NextAuth->>DB: Validate token from verification_tokens collection
NextAuth-->>User: Authenticated → Redirect to /
end
📋 Step-by-Step
|
MrImmortal09
left a comment
There was a problem hiding this comment.
@Freny07 Last few changes(check coderrabit reviews), and we are good to go
|
Got it, working on it. |
c4f9a62 to
04da815
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
app/api/set-password/route.ts (1)
9-33:⚠️ Potential issue | 🔴 CriticalThe set-password flow still is not bound to the Google-authenticated user.
After
middleware.tsdeletes thegoogle-redirectcookie, this handler trusts only the client-suppliedStagingUserrow. That means anyone who can guess or learn a pending signup email can call this endpoint first and claim the account. The route needs a server-issued one-time proof tied to the Google callback, not just the email address.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/set-password/route.ts` around lines 9 - 33, The handler currently trusts a client-supplied email and the existence of a StagingUser row; require and validate a server-issued one-time proof tied to the Google callback instead: expect a oneTimeToken (or oneTimeProof) in the request body or header, fetch the StagingUser by email and compare the provided token to the stored token on the StagingUser document (and check expiresAt), then atomically consume/invalidate that token (delete the StagingUser or clear the token/mark used) before creating the account; update validations around canonical/email, staged lookup, and replace the current staged-only check with token verification so only the Google-authenticated flow that received the server token can set the password.
🧹 Nitpick comments (1)
app/reset-password/ResetPasswordClient.tsx (1)
67-67: Optional: clean up redirect timer on unmount.The delayed redirect works, but the timer is never cleared if the component unmounts before it fires.
♻️ Optional cleanup pattern
-import { useState } from "react" +import { useEffect, useRef, useState } from "react" @@ const [done, setDone] = useState(false) + const redirectTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) + + useEffect(() => { + return () => { + if (redirectTimerRef.current) clearTimeout(redirectTimerRef.current) + } + }, []) @@ - setTimeout(() => router.replace("/login"), 2000) + redirectTimerRef.current = setTimeout(() => router.replace("/login"), 2000)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/reset-password/ResetPasswordClient.tsx` at line 67, The component schedules a redirect with setTimeout in ResetPasswordClient (setTimeout(() => router.replace("/login"), 2000)) but never clears the timer on unmount; capture the timeout ID (e.g., const timeoutId = setTimeout(...)) and clear it in a cleanup (useEffect return or useRef + cleanup) using clearTimeout(timeoutId) so the redirect won’t run after the component unmounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/forgot-password/route.ts`:
- Around line 39-40: The console.log that prints the full reset URL (variable
resetUrl built from token in route.ts) exposes a bearer token; remove the debug
log that outputs resetUrl/token and instead log a non-sensitive message (e.g.,
"password reset email sent" or include only a masked token or request id) from
the forgot-password handler so no full reset links or tokens are written to
logs.
In `@app/api/login/route.ts`:
- Around line 45-55: The code infers cookie security from req.url
(useSecureCookies) which is unreliable behind proxies; instead determine secure
cookies from a trusted source (e.g., X-Forwarded-Proto header validated by your
proxy/trust configuration or a deployment config/env var like NEXTAUTH_URL or
FORCE_SECURE_COOKIES) and use that boolean when computing cookieName and calling
cookieStore.set; update the logic around useSecureCookies, cookieName, and
cookieStore.set to consult the trusted header/config and fallback to a safe
default (secure=true) if uncertain.
In `@app/api/register/route.ts`:
- Around line 48-64: The insertOne + catch block currently lets duplicate-key
errors surface as a generic 500; update the catch handling around
db.collection("users").insertOne so that when the error is a Mongo duplicate-key
(E11000 / MongoServerError code 11000) you log a clear duplicate message and
return a deterministic client response (e.g., NextResponse.json({ error: "Email
already registered" }, { status: 409 })), otherwise keep the existing
console.error and return the 500 via NextResponse.json; reference the insertOne
call and the existing catch/NextResponse.json usage to locate where to branch on
err.code/name.
- Around line 11-16: The 400 response for missing required fields in the
registration handler currently returns NextResponse.json without auditing;
update the missing-fields branch to call logEvent (the same audit function used
elsewhere in this route) before returning, passing context like event name
("signup_rejected_missing_fields"), the provided email/branch if present (or
note missing), and any request metadata, then return the existing
NextResponse.json; ensure you reference the existing logEvent function used
elsewhere in this file so the audit entry is recorded for this rejection path.
In `@app/api/reset-password/route.ts`:
- Around line 17-39: Replace the separate findOne/deleteOne and updateOne flow
with a single MongoDB transaction: start a session (client.startSession()),
start a transaction, call PasswordResetToken.findOneAndDelete({ token }, {
session }) to atomically consume the token and get the email, then call
db.collection("users").updateOne({ email: deletedDoc.email }, { $set: {
password: hashed } }, { session }) and check the update result.matchedCount; if
matchedCount is 0 abort the transaction and return an error, otherwise commit
the transaction; ensure you close the session and surface errors appropriately.
Use the existing symbols PasswordResetToken, findOneAndDelete, updateOne, token,
record/email, and client.startSession()/session.withTransaction to locate and
implement the change.
In `@lib/rateLimit.ts`:
- Around line 4-7: The rate limiter currently relies on TTL removal of the
document keyed by email (RateLimitSchema with unique email and windowStart)
which can race with upserts and produce E11000 duplicates and false blocks;
change the logic that creates/increments buckets to always do an atomic
find-and-update on the existing document by email: if the found document's
windowStart is expired, reset its fields in-place (set count to 1 and set
windowStart = now), otherwise increment count; implement this with a single
findOneAndUpdate (or equivalent) using a conditional update (query by email and
check windowStart) or a two-step atomic query using Mongo operators so no new
doc insertion depends on TTL cleanup and the unique index is never violated.
In `@models/PasswordResetToken.ts`:
- Around line 5-6: The schema currently stores plaintext reset tokens in the
token field which is unsafe; change the model to persist a token hash (e.g.,
tokenHash) instead of the raw token, update the schema field (token ->
tokenHash) to be String + unique, and ensure any code that creates tokens (token
generation function) returns the raw token to the caller but saves only its
SHA-256 hash; when verifying in your reset flow, hash the incoming token the
same way and compare against tokenHash using a constant-time comparison (e.g.,
crypto.timingSafeEqual) and keep expiresAt with the TTL index unchanged; also
remove any places that log or persist the raw token and adjust queries that
previously matched token to match tokenHash instead.
In `@package.json`:
- Around line 16-20: There are two duplicate "dotenv" entries in package.json
dependencies; remove the redundant one so only a single "dotenv" dependency
remains (keep the desired version string "^17.4.1"), updating the "dependencies"
object to contain a single "dotenv" key and ensuring package.json remains valid
JSON; look for the duplicate "dotenv" lines in the dependencies block to delete
the extra entry.
---
Duplicate comments:
In `@app/api/set-password/route.ts`:
- Around line 9-33: The handler currently trusts a client-supplied email and the
existence of a StagingUser row; require and validate a server-issued one-time
proof tied to the Google callback instead: expect a oneTimeToken (or
oneTimeProof) in the request body or header, fetch the StagingUser by email and
compare the provided token to the stored token on the StagingUser document (and
check expiresAt), then atomically consume/invalidate that token (delete the
StagingUser or clear the token/mark used) before creating the account; update
validations around canonical/email, staged lookup, and replace the current
staged-only check with token verification so only the Google-authenticated flow
that received the server token can set the password.
---
Nitpick comments:
In `@app/reset-password/ResetPasswordClient.tsx`:
- Line 67: The component schedules a redirect with setTimeout in
ResetPasswordClient (setTimeout(() => router.replace("/login"), 2000)) but never
clears the timer on unmount; capture the timeout ID (e.g., const timeoutId =
setTimeout(...)) and clear it in a cleanup (useEffect return or useRef +
cleanup) using clearTimeout(timeoutId) so the redirect won’t run after the
component unmounts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c041722-2609-4b06-8e86-b7fac9a07a83
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.gitignoreapp/access-denied/page.tsxapp/api/forgot-password/route.tsapp/api/login/route.tsapp/api/register/route.tsapp/api/reset-password/route.tsapp/api/set-password/route.tsapp/forgot-password/page.tsxapp/layout.tsxapp/login/LoginClient.tsxapp/reset-password/ResetPasswordClient.tsxapp/reset-password/page.tsxapp/set-password/SetPasswordClient.tsxapp/set-password/page.tsxcomponents/Navbar.tsxlib/auth.tslib/logger.tslib/mongodb.tslib/rateLimit.tsmiddleware.tsmodels/PasswordResetToken.tsmodels/StagingUser.tspackage.json
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- lib/mongodb.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/set-password/page.tsx
- app/layout.tsx
- lib/logger.ts
- components/Navbar.tsx
| const useSecureCookies = req.url.startsWith("https://") | ||
| const cookieName = useSecureCookies ? "__Secure-next-auth.session-token" : "next-auth.session-token" | ||
|
|
||
| const cookieStore = await cookies() | ||
| cookieStore.set(cookieName, sessionToken, { | ||
| httpOnly: true, | ||
| sameSite: "lax", | ||
| secure: useSecureCookies, | ||
| path: "/", | ||
| expires, | ||
| }) |
There was a problem hiding this comment.
Do not infer cookie security from req.url.
Behind a reverse proxy or load balancer, the public request can be HTTPS while the app still sees an internal http:// URL. In that case this path will issue a non-secure session cookie for a password login. Please derive this from trusted proxy headers or deployment config instead.
🔐 Proposed fix
- const useSecureCookies = req.url.startsWith("https://")
+ const forwardedProto = req.headers.get("x-forwarded-proto")?.split(",")[0]?.trim()
+ const useSecureCookies =
+ forwardedProto === "https" || process.env.NODE_ENV === "production"📝 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 useSecureCookies = req.url.startsWith("https://") | |
| const cookieName = useSecureCookies ? "__Secure-next-auth.session-token" : "next-auth.session-token" | |
| const cookieStore = await cookies() | |
| cookieStore.set(cookieName, sessionToken, { | |
| httpOnly: true, | |
| sameSite: "lax", | |
| secure: useSecureCookies, | |
| path: "/", | |
| expires, | |
| }) | |
| const forwardedProto = req.headers.get("x-forwarded-proto")?.split(",")[0]?.trim() | |
| const useSecureCookies = | |
| forwardedProto === "https" || process.env.NODE_ENV === "production" | |
| const cookieName = useSecureCookies ? "__Secure-next-auth.session-token" : "next-auth.session-token" | |
| const cookieStore = await cookies() | |
| cookieStore.set(cookieName, sessionToken, { | |
| httpOnly: true, | |
| sameSite: "lax", | |
| secure: useSecureCookies, | |
| path: "/", | |
| expires, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/login/route.ts` around lines 45 - 55, The code infers cookie security
from req.url (useSecureCookies) which is unreliable behind proxies; instead
determine secure cookies from a trusted source (e.g., X-Forwarded-Proto header
validated by your proxy/trust configuration or a deployment config/env var like
NEXTAUTH_URL or FORCE_SECURE_COOKIES) and use that boolean when computing
cookieName and calling cookieStore.set; update the logic around
useSecureCookies, cookieName, and cookieStore.set to consult the trusted
header/config and fallback to a safe default (secure=true) if uncertain.
|
Working as per the above review. |
|
@coderabbitai full-review |
|
The PR description images show:
✅ Actions performedFull review triggered. |
|
@Freny07 Run pnpm run lint is failing. |
|
Working on it |
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (9)
app/api/forgot-password/route.ts (1)
39-40:⚠️ Potential issue | 🔴 CriticalDo not log live password-reset links.
The URL contains the reset bearer token, so log access becomes account-reset access. Remove this before merge.
🔒 Suggested fix
const resetUrl = `${process.env.NEXTAUTH_URL}/reset-password?token=${token}` - console.log("Reset URL:", resetUrl)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/forgot-password/route.ts` around lines 39 - 40, Remove the sensitive debug log that prints the password-reset link: do not log resetUrl or token (the console.log("Reset URL:", resetUrl) call). Either delete that console.log or replace it with a non-sensitive message (e.g., log that a reset email was sent without including resetUrl or token) inside the same route handler where resetUrl and token are created.lib/rateLimit.ts (1)
4-7:⚠️ Potential issue | 🟠 MajorThis can still falsely block the first request of a new hour.
The upsert only matches buckets whose
windowStartis still inside the active hour, but the old row can remain until MongoDB's TTL worker deletes it. WithIn MongoDB, are TTL index deletions immediate, and can an expired document with a unique key remain briefly and block a new upsert on the same key?Also applies to: 25-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/rateLimit.ts` around lines 4 - 7, The current upsert matches on both email and windowStart, which lets a stale document (pending TTL removal) block a fresh bucket; change the logic to match only on email and perform an atomic conditional update that either resets the bucket when windowStart is stale or increments count if still inside the window. Concretely, replace the upsert that uses windowStart in the query with a findOneAndUpdate on { email } using an update pipeline (or explicit two-branch update) that checks windowStart vs. now and: if stale -> set count=1 and windowStart=now; else -> increment count; keep upsert:true and return the updated document. Update code paths that reference RateLimitSchema / email / windowStart / count to use this new atomic conditional upsert so TTL race conditions no longer block first requests of a new window.models/PasswordResetToken.ts (1)
3-6:⚠️ Potential issue | 🔴 CriticalStore password-reset tokens as hashes, not plaintext.
These tokens are bearer credentials. If this collection leaks, every unexpired row is immediately usable to reset an account. Persist a hash (for example
tokenHash) and hash the incoming token before lookup in the reset route.🔐 Suggested direction
const PasswordResetTokenSchema = new Schema({ email: { type: String, required: true, lowercase: true, trim: true }, - token: { type: String, required: true, unique: true }, + tokenHash: { type: String, required: true, unique: true }, expiresAt: { type: Date, required: true, index: { expireAfterSeconds: 0 } }, })You would then update
app/api/forgot-password/route.tsto save only the hash, andapp/api/reset-password/route.tsto hash the presented token before querying.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@models/PasswordResetToken.ts` around lines 3 - 6, PasswordResetTokenSchema currently stores plaintext token; change the schema field from token to tokenHash (keep expiresAt index) and remove any plaintext token storage so the DB only contains the token hash, then update the forgot-password handler to generate the reset token, hash it (e.g., crypto.createHash('sha256').update(token).digest('hex') or bcrypt) and persist tokenHash via PasswordResetTokenSchema, and update the reset-password handler to hash the incoming presented token the same way and query by tokenHash (and validate expiresAt) instead of matching plaintext token; also preserve uniqueness/indexing on tokenHash and update any usages referencing the old token property.app/api/set-password/route.ts (2)
9-18:⚠️ Potential issue | 🔴 CriticalEmail-only staging lookup still allows account claiming.
StagingUser.findOne({ email: canonical, ... })treats knowledge of the email as sufficient proof. If a pending staging row exists for another user, any caller can POST that email here and create the real account before the legitimate Google-authenticated user completes setup. Bind this flow to a server-side session/user identity or a one-time secret token, not just the client-supplied email.Also applies to: 23-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/set-password/route.ts` around lines 9 - 18, The current POST handler reads email/password from req.json and uses StagingUser.findOne({ email: canonical, ... }) allowing anyone who knows an email to claim a staging row; change the flow to require and validate a server-issued identifier (session-bound user id or a one-time secret token) instead of trusting the client email alone: update the handler that calls StagingUser.findOne to look up by a server-validated session user id or by a one-time token field (e.g., stagingToken) passed by the client and matched against the stored token on the StagingUser row, reject requests that lack/ fail token or session validation, and only then allow creating the account and clearing the staging entry—do not permit lookup/claiming by email alone.
39-59:⚠️ Potential issue | 🟠 MajorThe staged signup is still not consumed atomically.
This is still
findOne/insertOne/deleteOneacross two collections, so concurrent requests can race into a duplicate insert or leave the staging row behind after a partial failure. The acceptance criteria explicitly calls out atomic password setup, so this path needs a transaction or an equivalent one-time consume mechanism.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/set-password/route.ts` around lines 39 - 59, The current flow uses separate findOne/insertOne/deleteOne calls (existing, db.collection("users").insertOne, StagingUser.deleteOne) which is racy; wrap the staging-consume and real-user insert in a single MongoDB transaction (start a session and call session.withTransaction) so you read/verify the staging record, insert into db.collection("users"), and delete the StagingUser row atomically; alternatively, atomically consume the staging row with StagingUser.findOneAndDelete (ensuring it returns the staged document) and then insert the user while checking for duplicate-key errors (unique index on email) and handling retries—update the code paths that call existing, db.collection("users").insertOne, and StagingUser.deleteOne in route.ts to use one of these atomic patterns.app/api/register/route.ts (2)
48-64:⚠️ Potential issue | 🟠 MajorHandle duplicate-key races around the insert path.
The preflight lookup on Line 31 does not make
insertOne()safe. Two concurrent requests can both pass the existence check, and one will fall through to the generic 500 handler instead of the deterministic duplicate response/logging used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/register/route.ts` around lines 48 - 64, The insert path in the registration route currently returns a generic 500 on any error; make the catch block around db.collection("users").insertOne(...) detect duplicate-key errors (e.g. MongoServerError with code 11000 or an E11000 message) and handle them deterministically: log a clear duplicate-key message including the conflicting email/unique field and return a specific response (409 Conflict or a consistent duplicate-account JSON) instead of the generic 500; leave other errors to be logged and returned as 500 as before.
11-15:⚠️ Potential issue | 🟠 MajorRejected signups with missing fields are still invisible to audit logs.
This 400 path returns before calling
logEvent, so one rejection class still bypasses the abuse-monitoring requirement from the PR. Log the attempt before returning, even if the email is absent or malformed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/register/route.ts` around lines 11 - 15, In the register route handler in app/api/register/route.ts, ensure we record rejected signup attempts with missing fields by calling the existing logEvent (or audit logging) function before returning the 400 response; gather whatever metadata is available (e.g., provided email or "unknown", client IP, and which fields were missing), await logEvent if it returns a promise, pass a clear event name like "signup_rejected" and a reason "missing_fields", then return the NextResponse.json({ error: "Missing required fields" }, { status: 400 }) as before so the rejection is logged for abuse monitoring.app/api/reset-password/route.ts (1)
17-40:⚠️ Potential issue | 🔴 CriticalConsume reset token atomically and verify user update.
Line 17 + Line 39 perform check-then-delete in separate steps, so concurrent requests can reuse a token. Also Line 36 ignores
matchedCount, which can return success with no account updated.Proposed patch
- const record = await PasswordResetToken.findOne({ - token, - expiresAt: { $gt: new Date() }, - }) - - if (!record) { - return NextResponse.json( - { error: "Reset link is invalid or has expired." }, - { status: 400 } - ) - } - - const hashed = await bcrypt.hash(password, 10) - - const client = await clientPromise - const db = client.db() - - await db - .collection("users") - .updateOne({ email: record.email }, { $set: { password: hashed } }) - - // Delete the token so it can't be reused - await PasswordResetToken.deleteOne({ token }) + const hashed = await bcrypt.hash(password, 10) + const client = await clientPromise + const db = client.db() + const session = client.startSession() + + let updated = false + try { + await session.withTransaction(async () => { + const record = await PasswordResetToken.findOneAndDelete({ + token, + expiresAt: { $gt: new Date() }, + }) + + if (!record) { + throw new Error("INVALID_OR_EXPIRED_TOKEN") + } + + const result = await db + .collection("users") + .updateOne({ email: record.email }, { $set: { password: hashed } }, { session }) + + if (result.matchedCount !== 1) { + throw new Error("USER_NOT_FOUND") + } + updated = true + }) + } finally { + await session.endSession() + } + + if (!updated) { + return NextResponse.json( + { error: "Reset link is invalid or has expired." }, + { status: 400 } + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/reset-password/route.ts` around lines 17 - 40, Replace the separate token lookup and later deletion with an atomic consume: use PasswordResetToken.findOneAndDelete(...) (instead of PasswordResetToken.findOne) to both validate and remove the token in one step, store the returned document (e.g., deletedRecord.value) and treat a missing return as an invalid/expired token; then perform the user update using db.collection("users").updateOne({ email: deletedRecord.value.email }, { $set: { password: hashed } }) and check the update result’s matchedCount/modifiedCount to ensure a user was actually updated, returning the same error response if no user was updated.app/api/login/route.ts (1)
45-55:⚠️ Potential issue | 🟠 MajorDon't derive cookie security from
req.url.Behind a proxy, the public request can be HTTPS while the app still sees an internal
http://URL. In that case this path issues a non-secure session cookie for a successful login.🔧 Suggested fix
- const useSecureCookies = req.url.startsWith("https://") + const forwardedProto = req.headers.get("x-forwarded-proto")?.split(",")[0]?.trim() + const useSecureCookies = + forwardedProto === "https" || process.env.NODE_ENV === "production"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/login/route.ts` around lines 45 - 55, Do not rely on req.url to decide cookie security; instead derive the protocol from trusted proxy headers (e.g. check req.headers.get("x-forwarded-proto") first and fall back to req.url's protocol) and use that to set useSecureCookies and cookieName before calling cookies() and cookieStore.set; update the logic around useSecureCookies, cookieName, and cookieStore.set to treat the cookie as secure when the resolved protocol equals "https" so the "__Secure-..." prefix and secure flag are always correct behind proxies.
🧹 Nitpick comments (2)
app/register/RegisterClient.tsx (1)
132-139: Align the password minimum with the reset flow.This form accepts 6 characters, but
app/reset-password/ResetPasswordClient.tsxrejects anything under 8. That inconsistency will confuse users and drifts the auth policy.🔧 Suggested fix
- minLength={6} + minLength={8}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/register/RegisterClient.tsx` around lines 132 - 139, The password input in RegisterClient uses minLength={6}, which conflicts with ResetPasswordClient's 8-character minimum; update the registration password constraint to enforce the same minimum as the reset flow by changing the password input's minLength and any related client-side validation in RegisterClient (e.g., the password state handling and setPassword usage) to require at least 8 characters so both RegisterClient and ResetPasswordClient enforce the same policy.app/api/login/route.ts (1)
39-43: Add a TTL index for the new session collection.These inserts add an
expiresfield, but nothing in the provided Mongo setup creates a TTL index on it. Expired sessions will keep accumulating until something cleans them up manually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/login/route.ts` around lines 39 - 43, Create a TTL index on the "sessions" collection so documents with the expires field are auto-removed: ensure during DB initialization (or before calling db.collection("sessions").insertOne) you call createIndex on the "sessions" collection targeting the expires field with expireAfterSeconds: 0 (e.g., db.collection("sessions").createIndex({ expires: 1 }, { expireAfterSeconds: 0 })), so inserted sessions with the expires date are automatically deleted; run this index creation once at startup (or guard it) to avoid repeated index creation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/reset-password/route.ts`:
- Around line 11-13: The reset-password route currently only checks presence of
token and password in the handler and allows any password value; add a password
policy check before proceeding to hashing by validating password length (e.g.,
enforce PASSWORD_MIN_LENGTH) and/or reusing the shared
validatePassword(password) utility if available; if the password fails
validation return NextResponse.json({ error: "Password does not meet minimum
requirements." }, { status: 400 }) and only continue with hashing and token use
(the token, password variables and the route handler) when validation passes.
In `@app/forgot-password/page.tsx`:
- Around line 65-67: The JSX paragraph node with className "mt-2 text-sm
text-muted" contains an unescaped apostrophe in "we'll" which violates
react/no-unescaped-entities; update the text to escape the apostrophe (e.g.
replace we'll with we'll or use {"we'll"}) inside that <p> element in
page.tsx so lint/CI passes.
In `@app/HomeWrapper.tsx`:
- Around line 14-16: The redirect currently leaks PII by appending
session.user.email in router.replace("/set-password?email=" +
session.user.email); instead change the redirect to
router.replace("/set-password") (or equivalent) so no email is included, and
update the /set-password page/API to read the authenticated user from the
session (using session?.user or your auth helper) to resolve identity and
prefill/validate without any email query parameter; remove any code that reads
the email from query string and rely on session?.user?.email instead.
In `@app/login/LoginClient.tsx`:
- Around line 39-53: The client-side fetch to "/api/rate-limit" is incrementing
the quota and so is lib/auth.ts's sendVerificationRequest, causing
double-debits; change the client to perform a non-incrementing check (e.g., call
a new query-only route or include a flag like { checkOnly: true } in the POST
body) instead of incrementing, or remove the client-side rate check entirely and
rely on sendVerificationRequest to enforce limits; update the LoginClient.tsx
fetch call (the block that runs before signIn("email", ...)) to use the
non-incrementing check so sendVerificationRequest remains the single
incrementer.
In `@app/set-password/SetPasswordClient.tsx`:
- Around line 9-10: The page currently reads email from useSearchParams in
SetPasswordClient.tsx and the API (app/api/set-password/route.ts) accepts that
client-supplied email to claim a StagingUser; stop trusting the email query
param: remove usage of searchParams.get("email") in SetPasswordClient.tsx and
send only a single opaque, unguessable token (e.g., stagingToken or stagingId)
from the URL to the backend; update app/api/set-password/route.ts to look up the
StagingUser by that server-validated token (and expiry) and derive the email
from the StagingUser row instead of the request body/query, rejecting requests
without a valid token or expired staging row.
In `@lib/auth.ts`:
- Around line 140-152: The google-redirect cookie set in cookieStore.set (the
calls that use key "google-redirect" and values like `set-password:${email}` and
`login:${email}`) must be marked secure in production; update both
cookieStore.set invocations to include a secure flag (e.g., secure:
process.env.NODE_ENV === "production" or a config boolean) so the cookie is only
sent over HTTPS in prod while remaining compatible in dev.
In `@lib/mongodb.ts`:
- Around line 9-11: The cached global promise global._mongoClientPromise is left
rejected if MongoClient.connect() fails, causing all subsequent callers to use
the failed promise; change the initialization so the connect attempt clears the
cache on error (e.g., set global._mongoClientPromise =
client.connect().catch(err => { global._mongoClientPromise = undefined; throw
err; })) so that failed connect() calls reset global._mongoClientPromise and
allow retries; locate this logic around the MongoClient instantiation and the
client.connect() call.
In `@lib/ratelimit.ts`:
- Around line 5-7: The rate-limiter currently relies on the TTL index on
windowStart and can hit duplicate-key E11000 on upsert; change the logic that
reads/creates the bucket to detect stale buckets by comparing the stored
windowStart to now (older than 1 hour) and, if stale, explicitly reset the
document's count and windowStart (instead of relying on TTL deletion) before
performing increments/upserts. Update the code that accesses the schema fields
email, count, and windowStart (and the upsert path) to: 1) fetch the existing
document by email, 2) if found and windowStart + 1 hour < now then set count = 0
and windowStart = now (persisting this update), and 3) continue with the normal
increment—this avoids the race with TTL and prevents duplicate-key errors
(E11000) during upsert.
In `@middleware.ts`:
- Around line 1-2: The import for NextRequest is from the wrong module; update
the import in middleware.ts to import both NextResponse and NextRequest from
"next/server" instead of importing NextRequest from "next/request" so that the
existing usages of NextRequest and NextResponse continue to work across Next.js
versions.
In `@models/Log.ts`:
- Around line 3-9: There are two conflicting mongoose model definitions for
"Log" causing import-order dependent behavior; keep this file as the single
source by exporting the Log model built from LogSchema and remove the duplicate
model/schema in lib/logger.ts, then update lib/logger.ts to import the existing
Log model (referencing the exported Log model from this file) and reuse it for
all logging operations so mongoose.models.Log is not re-registered elsewhere.
---
Duplicate comments:
In `@app/api/forgot-password/route.ts`:
- Around line 39-40: Remove the sensitive debug log that prints the
password-reset link: do not log resetUrl or token (the console.log("Reset URL:",
resetUrl) call). Either delete that console.log or replace it with a
non-sensitive message (e.g., log that a reset email was sent without including
resetUrl or token) inside the same route handler where resetUrl and token are
created.
In `@app/api/login/route.ts`:
- Around line 45-55: Do not rely on req.url to decide cookie security; instead
derive the protocol from trusted proxy headers (e.g. check
req.headers.get("x-forwarded-proto") first and fall back to req.url's protocol)
and use that to set useSecureCookies and cookieName before calling cookies() and
cookieStore.set; update the logic around useSecureCookies, cookieName, and
cookieStore.set to treat the cookie as secure when the resolved protocol equals
"https" so the "__Secure-..." prefix and secure flag are always correct behind
proxies.
In `@app/api/register/route.ts`:
- Around line 48-64: The insert path in the registration route currently returns
a generic 500 on any error; make the catch block around
db.collection("users").insertOne(...) detect duplicate-key errors (e.g.
MongoServerError with code 11000 or an E11000 message) and handle them
deterministically: log a clear duplicate-key message including the conflicting
email/unique field and return a specific response (409 Conflict or a consistent
duplicate-account JSON) instead of the generic 500; leave other errors to be
logged and returned as 500 as before.
- Around line 11-15: In the register route handler in app/api/register/route.ts,
ensure we record rejected signup attempts with missing fields by calling the
existing logEvent (or audit logging) function before returning the 400 response;
gather whatever metadata is available (e.g., provided email or "unknown", client
IP, and which fields were missing), await logEvent if it returns a promise, pass
a clear event name like "signup_rejected" and a reason "missing_fields", then
return the NextResponse.json({ error: "Missing required fields" }, { status: 400
}) as before so the rejection is logged for abuse monitoring.
In `@app/api/reset-password/route.ts`:
- Around line 17-40: Replace the separate token lookup and later deletion with
an atomic consume: use PasswordResetToken.findOneAndDelete(...) (instead of
PasswordResetToken.findOne) to both validate and remove the token in one step,
store the returned document (e.g., deletedRecord.value) and treat a missing
return as an invalid/expired token; then perform the user update using
db.collection("users").updateOne({ email: deletedRecord.value.email }, { $set: {
password: hashed } }) and check the update result’s matchedCount/modifiedCount
to ensure a user was actually updated, returning the same error response if no
user was updated.
In `@app/api/set-password/route.ts`:
- Around line 9-18: The current POST handler reads email/password from req.json
and uses StagingUser.findOne({ email: canonical, ... }) allowing anyone who
knows an email to claim a staging row; change the flow to require and validate a
server-issued identifier (session-bound user id or a one-time secret token)
instead of trusting the client email alone: update the handler that calls
StagingUser.findOne to look up by a server-validated session user id or by a
one-time token field (e.g., stagingToken) passed by the client and matched
against the stored token on the StagingUser row, reject requests that lack/ fail
token or session validation, and only then allow creating the account and
clearing the staging entry—do not permit lookup/claiming by email alone.
- Around line 39-59: The current flow uses separate findOne/insertOne/deleteOne
calls (existing, db.collection("users").insertOne, StagingUser.deleteOne) which
is racy; wrap the staging-consume and real-user insert in a single MongoDB
transaction (start a session and call session.withTransaction) so you
read/verify the staging record, insert into db.collection("users"), and delete
the StagingUser row atomically; alternatively, atomically consume the staging
row with StagingUser.findOneAndDelete (ensuring it returns the staged document)
and then insert the user while checking for duplicate-key errors (unique index
on email) and handling retries—update the code paths that call existing,
db.collection("users").insertOne, and StagingUser.deleteOne in route.ts to use
one of these atomic patterns.
In `@lib/rateLimit.ts`:
- Around line 4-7: The current upsert matches on both email and windowStart,
which lets a stale document (pending TTL removal) block a fresh bucket; change
the logic to match only on email and perform an atomic conditional update that
either resets the bucket when windowStart is stale or increments count if still
inside the window. Concretely, replace the upsert that uses windowStart in the
query with a findOneAndUpdate on { email } using an update pipeline (or explicit
two-branch update) that checks windowStart vs. now and: if stale -> set count=1
and windowStart=now; else -> increment count; keep upsert:true and return the
updated document. Update code paths that reference RateLimitSchema / email /
windowStart / count to use this new atomic conditional upsert so TTL race
conditions no longer block first requests of a new window.
In `@models/PasswordResetToken.ts`:
- Around line 3-6: PasswordResetTokenSchema currently stores plaintext token;
change the schema field from token to tokenHash (keep expiresAt index) and
remove any plaintext token storage so the DB only contains the token hash, then
update the forgot-password handler to generate the reset token, hash it (e.g.,
crypto.createHash('sha256').update(token).digest('hex') or bcrypt) and persist
tokenHash via PasswordResetTokenSchema, and update the reset-password handler to
hash the incoming presented token the same way and query by tokenHash (and
validate expiresAt) instead of matching plaintext token; also preserve
uniqueness/indexing on tokenHash and update any usages referencing the old token
property.
---
Nitpick comments:
In `@app/api/login/route.ts`:
- Around line 39-43: Create a TTL index on the "sessions" collection so
documents with the expires field are auto-removed: ensure during DB
initialization (or before calling db.collection("sessions").insertOne) you call
createIndex on the "sessions" collection targeting the expires field with
expireAfterSeconds: 0 (e.g., db.collection("sessions").createIndex({ expires: 1
}, { expireAfterSeconds: 0 })), so inserted sessions with the expires date are
automatically deleted; run this index creation once at startup (or guard it) to
avoid repeated index creation.
In `@app/register/RegisterClient.tsx`:
- Around line 132-139: The password input in RegisterClient uses minLength={6},
which conflicts with ResetPasswordClient's 8-character minimum; update the
registration password constraint to enforce the same minimum as the reset flow
by changing the password input's minLength and any related client-side
validation in RegisterClient (e.g., the password state handling and setPassword
usage) to require at least 8 characters so both RegisterClient and
ResetPasswordClient enforce the same policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0f9e567-1af2-432f-abf3-ec91cf2fb877
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
.gitignoreapp/HomeWrapper.tsxapp/access-denied/page.tsxapp/api/auth/[...nextauth]/route.tsapp/api/forgot-password/route.tsapp/api/login/route.tsapp/api/rate-limit/route.tsapp/api/register/route.tsapp/api/reset-password/route.tsapp/api/set-password/route.tsapp/forgot-password/page.tsxapp/layout.tsxapp/login/LoginClient.tsxapp/login/page.tsxapp/page.tsxapp/providers.tsxapp/register/RegisterClient.tsxapp/register/page.tsxapp/reset-password/ResetPasswordClient.tsxapp/reset-password/page.tsxapp/set-password/SetPasswordClient.tsxapp/set-password/page.tsxcomponents/Navbar.tsxlib/auth.tslib/logger.tslib/mongodb.tslib/rateLimit.tslib/ratelimit.tsmiddleware.tsmodels/Log.tsmodels/PasswordResetToken.tsmodels/StagingUser.tspackage.jsonpnpm-workspace.yaml
| if (!token || !password) { | ||
| return NextResponse.json({ error: "Missing fields." }, { status: 400 }) | ||
| } |
There was a problem hiding this comment.
Enforce password policy in reset endpoint.
On Line 11, this route accepts any password value; that allows weaker passwords than registration rules. Add minimum-length validation (and ideally shared policy validation) before hashing.
Proposed patch
- if (!token || !password) {
+ if (!token || !password) {
return NextResponse.json({ error: "Missing fields." }, { status: 400 })
}
+ if (typeof password !== "string" || password.length < 6) {
+ return NextResponse.json(
+ { error: "Password must be at least 6 characters long." },
+ { status: 400 }
+ )
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/reset-password/route.ts` around lines 11 - 13, The reset-password
route currently only checks presence of token and password in the handler and
allows any password value; add a password policy check before proceeding to
hashing by validating password length (e.g., enforce PASSWORD_MIN_LENGTH) and/or
reusing the shared validatePassword(password) utility if available; if the
password fails validation return NextResponse.json({ error: "Password does not
meet minimum requirements." }, { status: 400 }) and only continue with hashing
and token use (the token, password variables and the route handler) when
validation passes.
| if (session?.user?.needsPassword) { | ||
| router.replace("/set-password?email=" + session.user.email) | ||
| } |
There was a problem hiding this comment.
Do not put user email in the redirect URL.
On Line 15, appending session.user.email to the query string exposes PII in logs/history/referrers. Redirect to /set-password without email and resolve identity from the authenticated session on that page/API.
Proposed patch
- if (session?.user?.needsPassword) {
- router.replace("/set-password?email=" + session.user.email)
- }
+ if (session?.user?.needsPassword) {
+ router.replace("/set-password")
+ }📝 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.
| if (session?.user?.needsPassword) { | |
| router.replace("/set-password?email=" + session.user.email) | |
| } | |
| if (session?.user?.needsPassword) { | |
| router.replace("/set-password") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/HomeWrapper.tsx` around lines 14 - 16, The redirect currently leaks PII
by appending session.user.email in router.replace("/set-password?email=" +
session.user.email); instead change the redirect to
router.replace("/set-password") (or equivalent) so no email is included, and
update the /set-password page/API to read the authenticated user from the
session (using session?.user or your auth helper) to resolve identity and
prefill/validate without any email query parameter; remove any code that reads
the email from query string and rely on session?.user?.email instead.
| if (!password) { | ||
| const res = await fetch("/api/rate-limit", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ email }), | ||
| }) | ||
|
|
||
| const data = await res.json() | ||
|
|
||
| if (!res.ok || !data.allowed) { | ||
| setError(data.error || "Too many requests. Please try again later.") | ||
| return | ||
| } | ||
|
|
||
| await signIn("email", { email, callbackUrl: "/" }) |
There was a problem hiding this comment.
This preflight burns the magic-link quota twice.
/api/rate-limit already increments the bucket, and lib/auth.ts increments it again in sendVerificationRequest. With the current checkRateLimit() implementation, users will hit the limit after only a couple of successful emails instead of the intended 5/hour.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/login/LoginClient.tsx` around lines 39 - 53, The client-side fetch to
"/api/rate-limit" is incrementing the quota and so is lib/auth.ts's
sendVerificationRequest, causing double-debits; change the client to perform a
non-incrementing check (e.g., call a new query-only route or include a flag like
{ checkOnly: true } in the POST body) instead of incrementing, or remove the
client-side rate check entirely and rely on sendVerificationRequest to enforce
limits; update the LoginClient.tsx fetch call (the block that runs before
signIn("email", ...)) to use the non-incrementing check so
sendVerificationRequest remains the single incrementer.
| cookieStore.set("google-redirect", `set-password:${email}`, { | ||
| httpOnly: true, | ||
| sameSite: "lax", | ||
| path: "/", | ||
| maxAge: 60, | ||
| }) | ||
| } else { | ||
| cookieStore.set("google-redirect", `login:${email}`, { | ||
| httpOnly: true, | ||
| sameSite: "lax", | ||
| path: "/", | ||
| maxAge: 60, | ||
| }) |
There was a problem hiding this comment.
Mark the google-redirect cookie as secure in production.
This cookie carries auth-flow state plus the user email. Leaving it non-secure means it can be sent over plain HTTP on misconfigured deployments and behaves inconsistently compared with the session cookie.
🔧 Suggested fix
cookieStore.set("google-redirect", `set-password:${email}`, {
httpOnly: true,
sameSite: "lax",
+ secure: process.env.NODE_ENV === "production",
path: "/",
maxAge: 60,
})
} else {
cookieStore.set("google-redirect", `login:${email}`, {
httpOnly: true,
sameSite: "lax",
+ secure: process.env.NODE_ENV === "production",
path: "/",
maxAge: 60,
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/auth.ts` around lines 140 - 152, The google-redirect cookie set in
cookieStore.set (the calls that use key "google-redirect" and values like
`set-password:${email}` and `login:${email}`) must be marked secure in
production; update both cookieStore.set invocations to include a secure flag
(e.g., secure: process.env.NODE_ENV === "production" or a config boolean) so the
cookie is only sent over HTTPS in prod while remaining compatible in dev.
| if (!global._mongoClientPromise) { | ||
| const client = new MongoClient(uri) | ||
| global._mongoClientPromise = client.connect() |
There was a problem hiding this comment.
Reset the cached promise when connect() fails.
If the first connection attempt rejects, global._mongoClientPromise stays rejected and every later request fails until the process restarts. Clear the cache on failure so a later request can retry.
🛠️ Suggested fix
if (!global._mongoClientPromise) {
const client = new MongoClient(uri)
- global._mongoClientPromise = client.connect()
+ global._mongoClientPromise = client.connect().catch((error) => {
+ global._mongoClientPromise = undefined
+ throw error
+ })
}📝 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.
| if (!global._mongoClientPromise) { | |
| const client = new MongoClient(uri) | |
| global._mongoClientPromise = client.connect() | |
| if (!global._mongoClientPromise) { | |
| const client = new MongoClient(uri) | |
| global._mongoClientPromise = client.connect().catch((error) => { | |
| global._mongoClientPromise = undefined | |
| throw error | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mongodb.ts` around lines 9 - 11, The cached global promise
global._mongoClientPromise is left rejected if MongoClient.connect() fails,
causing all subsequent callers to use the failed promise; change the
initialization so the connect attempt clears the cache on error (e.g., set
global._mongoClientPromise = client.connect().catch(err => {
global._mongoClientPromise = undefined; throw err; })) so that failed connect()
calls reset global._mongoClientPromise and allow retries; locate this logic
around the MongoClient instantiation and the client.connect() call.
| const LogSchema = new mongoose.Schema({ | ||
| email: String, | ||
| type: String, | ||
| timestamp: { type: Date, default: Date.now }, | ||
| }) | ||
|
|
||
| export default mongoose.models.Log || mongoose.model("Log", LogSchema) No newline at end of file |
There was a problem hiding this comment.
Make this the single Log schema and have lib/logger.ts reuse it.
lib/logger.ts registers the same "Log" model name with different fields. Because both modules do mongoose.models.Log || mongoose.model(...), whichever file loads first wins, so log retention and validation currently depend on import order.
🧩 Suggested direction
const LogSchema = new mongoose.Schema({
- email: String,
- type: String,
- timestamp: { type: Date, default: Date.now },
+ email: { type: String, default: "unknown" },
+ type: { type: String, required: true },
+ timestamp: { type: Date, default: Date.now, expires: "30d" },
})Then remove the duplicate schema/model definition from lib/logger.ts and import this model there instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@models/Log.ts` around lines 3 - 9, There are two conflicting mongoose model
definitions for "Log" causing import-order dependent behavior; keep this file as
the single source by exporting the Log model built from LogSchema and remove the
duplicate model/schema in lib/logger.ts, then update lib/logger.ts to import the
existing Log model (referencing the exported Log model from this file) and reuse
it for all logging operations so mongoose.models.Log is not re-registered
elsewhere.
…ing the domain restriction, along with other mentioned issues.
MrImmortal09
left a comment
There was a problem hiding this comment.
@Freny07 address all the critcal issues.
Bhai kuch acche source se ye chize dhang se copy kr dete.
Looks like if you are fixing one mistake with ai another one gets created.
Tired by giving these many reviews.
Description
This is the core privacy guarantee: only people with IIITL credentials can enter the platform. All alumni and students have @iiitl.ac.in emails issued during their time at IIITL, along with set password for users using Google Login and link for login sent to email of the respective users as well as rate limiting to 5 times per email per hour and allowing user to stay logged in for 30 days
Closes #6
Type of Change
Checklist
feat/,fix/,docs/, etc.)pnpm lintand there are no errorspnpm dev)Screenshots / Screen Recording
Summary by CodeRabbit
New Features
Chores