Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR implements an email verification token resend feature, adds Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as ResendHandler
participant DB as Database
participant Email as EmailService
Client->>Handler: POST /verify { token }
Handler->>DB: find_token_by_value(token)
alt token not found
Handler-->>Client: 404 Not Found
else token found
Handler->>DB: find_user_by_id(user_id)
alt user not found
Handler-->>Client: 404 Not Found
else user found
Handler->>DB: update_verification_token(user_id)
DB-->>Handler: new_token (24h expiry)
Handler->>Email: send_verification_email(email, new_token)
alt email sent
Email-->>Handler: success
Handler-->>Client: 200 OK (resend confirmed)
else email failed
Email-->>Handler: error
Handler-->>Client: 500 / email error
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 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)
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-server/src/features/auth/handlers.rs`:
- Line 174: Remove the debug stderr print that exposes full user data: delete or
replace the eprint!("RESULT: {:#?}", user); in auth handlers (handlers.rs) and
instead log only non-sensitive identifiers (e.g., user.id or a redacted subset)
or use a secure logger that redacts PII in the function where `user` is printed;
ensure no full `user` struct is formatted to stderr or logs.
- Around line 167-186: The code rotates the verification token via
update_verification_token(db, user.get("id")) before attempting to send the
email, which can lock the user out if send_verification_email fails; change the
flow so you only persist the new token after the email send succeeds (call
update_verification_token after send_verification_email), or if you must
pre-create the token, capture the old token and on email send failure call a
restore_verification_token(db, user.get("id"), old_token) helper to revert to
the previous token and return the email error; update calls around
find_token_by_value, update_verification_token, and send_verification_email
accordingly.
- Around line 160-164: The resend_token handler currently accepts the token via
Query(ResendTokenData) which exposes sensitive data in URLs; change the handler
to accept the token from the request body instead (e.g., Json<ResendTokenData>
or a dedicated ResendTokenBody) by replacing Query(params) with the appropriate
Json/Extract type in the signature, update how you read token (from params.token
-> body.token), and adjust any route/tests that call resend_token to send a JSON
body; keep State(AppState) and return types the same.
In `@rust-server/src/features/auth/queries.rs`:
- Around line 87-95: The UPDATE currently always returns Ok(new_token) after
running sqlx::query(...).bind(...).execute(db).await, which can return zero rows
if no token row exists for that user_id; change this to capture the execute
result into a variable (e.g. let res =
sqlx::query(...).execute(db).await.map_err(AppError::Sql)?), check
res.rows_affected(), and if it is 0 return an appropriate error (e.g.
AppError::NotFound or a specific token-not-persisted error) instead of
Ok(new_token); only return Ok(new_token) when rows_affected() > 0.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8559a87a-90f2-4217-b855-564768d91743
📒 Files selected for processing (6)
rust-server/src/db/queries.rsrust-server/src/errors.rsrust-server/src/features/auth/handlers.rsrust-server/src/features/auth/queries.rsrust-server/src/features/auth/routes.rsrust-server/src/features/auth/types.rs
| let result = find_token_by_value(db, token) | ||
| .await? | ||
| .ok_or_else(|| AppError::not_found("Token not found")); | ||
|
|
||
| let user = find_user_by_id(db, result?.get("user_id")) | ||
| .await? | ||
| .ok_or_else(|| AppError::not_found("User not found"))?; | ||
| eprint!("RESULT: {:#?}", user); | ||
|
|
||
| let new_token = update_verification_token(db, user.get("id")).await?; | ||
|
|
||
| if !state.config.app_env.is_test() | ||
| && let Err(email_err) = state | ||
| .send_verification_email(user.get("email"), &new_token) | ||
| .await | ||
| { | ||
| tracing::error!("Failed to send verification email: {:#?}", email_err); | ||
| return Err(email_err); | ||
| } |
There was a problem hiding this comment.
Token rotation before email delivery can lock users out after transient mail failures.
Line 176 updates the token first; if Line 181–182 email send fails, the previously delivered token is invalidated and the user may no longer have a usable token to retry resend.
Suggested direction
- let new_token = update_verification_token(db, user.get("id")).await?;
+ // Keep previous token data for compensation if email send fails.
+ let old_token: String = result?.get("token");
+ let old_expires_at: DateTime<Utc> = result?.get("expires_at");
+ let new_token = update_verification_token(db, user.get("id")).await?;
@@
if !state.config.app_env.is_test()
&& let Err(email_err) = state
.email
.send_verification_email(user.get("email"), &new_token)
.await
{
+ // restore previous token state so the last delivered link remains usable
+ restore_verification_token(db, user.get("id"), &old_token, old_expires_at).await?;
tracing::error!("Failed to send verification email: {:#?}", email_err);
return Err(email_err);
}If you want, I can draft a minimal restore_verification_token(...) query helper too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-server/src/features/auth/handlers.rs` around lines 167 - 186, The code
rotates the verification token via update_verification_token(db, user.get("id"))
before attempting to send the email, which can lock the user out if
send_verification_email fails; change the flow so you only persist the new token
after the email send succeeds (call update_verification_token after
send_verification_email), or if you must pre-create the token, capture the old
token and on email send failure call a restore_verification_token(db,
user.get("id"), old_token) helper to revert to the previous token and return the
email error; update calls around find_token_by_value, update_verification_token,
and send_verification_email accordingly.
| sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3") | ||
| .bind(&new_token) | ||
| .bind(new_expires_at) | ||
| .bind(user_id) | ||
| .execute(db) | ||
| .await | ||
| .map_err(AppError::Sql)?; | ||
|
|
||
| Ok(new_token) |
There was a problem hiding this comment.
Check rows_affected to avoid returning non-persisted tokens.
Right now, Line 87–95 returns Ok(new_token) even when no token row exists for that user_id. That can send a token that will never verify.
Suggested fix
- sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
+ let result = sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3")
.bind(&new_token)
.bind(new_expires_at)
.bind(user_id)
.execute(db)
.await
.map_err(AppError::Sql)?;
+ if result.rows_affected() != 1 {
+ return Err(AppError::not_found("Verification token not found for user"));
+ }
+
Ok(new_token)📝 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.
| sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3") | |
| .bind(&new_token) | |
| .bind(new_expires_at) | |
| .bind(user_id) | |
| .execute(db) | |
| .await | |
| .map_err(AppError::Sql)?; | |
| Ok(new_token) | |
| let result = sqlx::query("UPDATE tokens SET token = $1, expires_at = $2 WHERE user_id = $3") | |
| .bind(&new_token) | |
| .bind(new_expires_at) | |
| .bind(user_id) | |
| .execute(db) | |
| .await | |
| .map_err(AppError::Sql)?; | |
| if result.rows_affected() != 1 { | |
| return Err(AppError::not_found("Verification token not found for user")); | |
| } | |
| Ok(new_token) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust-server/src/features/auth/queries.rs` around lines 87 - 95, The UPDATE
currently always returns Ok(new_token) after running
sqlx::query(...).bind(...).execute(db).await, which can return zero rows if no
token row exists for that user_id; change this to capture the execute result
into a variable (e.g. let res =
sqlx::query(...).execute(db).await.map_err(AppError::Sql)?), check
res.rows_affected(), and if it is 0 return an appropriate error (e.g.
AppError::NotFound or a specific token-not-persisted error) instead of
Ok(new_token); only return Ok(new_token) when rows_affected() > 0.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
rust-server/src/features/auth/handlers.rs (1)
178-187:⚠️ Potential issue | 🔴 CriticalDo not invalidate the last working link before email delivery succeeds.
update_verification_token(...)runs beforesend_verification_email(...). If mail delivery fails here, the previously issued token is already unusable and the user is stuck without any valid verification link. Persist the new token only after a successful send, or restore the old token on failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-server/src/features/auth/handlers.rs` around lines 178 - 187, Currently update_verification_token(db, user.get("id")) is called before sending email, which invalidates the previous token if send_verification_email fails; change the flow so you generate a new token but do not persist it before sending (or alternatively read and save the old token, attempt state.email.send_verification_email(user.get("email"), &new_token).await first, and only call update_verification_token(...) after the send succeeds), and if you must persist before send, ensure you restore the previous token on any send error; refer to update_verification_token, state.email.send_verification_email, and user.get("id")/user.get("email") to locate the relevant code to reorder or add rollback 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 `@client/src/app/`(auth)/verify/page.tsx:
- Around line 10-21: The VerifyPage should normalize the incoming token by
trimming whitespace before treating the link as valid: in VerifyPage (the
exported async function receiving VerifyPageProps and searchParams) obtain and
trim params.token, reject if the trimmed value is empty (so links like
?token=%20%20 are treated as invalid) and pass the trimmed token into
<VerifyContent token={...}>; update any local variable names accordingly so
downstream code receives the normalized token.
In `@client/src/app/`(auth)/verify/VerifyContent.tsx:
- Around line 19-24: The "Resend verification email" Button in VerifyContent
does nothing; update the VerifyContent component to attach an onClick handler
that POSTs to the existing resend endpoint (POST /api/auth/verify) when
res.status === 410, using the same payload or token used by the initial flow,
and wire up basic UI feedback: set a loading state while the fetch/mutation
runs, disable the Button during the request, and surface success or error
messages from the response (or rethrow to global error handling). Locate the JSX
in VerifyContent.tsx where the Button is rendered and implement the async
handler, ensuring you reference the Button element and the POST /api/auth/verify
endpoint and update component state (e.g., loading, message) accordingly.
---
Duplicate comments:
In `@rust-server/src/features/auth/handlers.rs`:
- Around line 178-187: Currently update_verification_token(db, user.get("id"))
is called before sending email, which invalidates the previous token if
send_verification_email fails; change the flow so you generate a new token but
do not persist it before sending (or alternatively read and save the old token,
attempt state.email.send_verification_email(user.get("email"), &new_token).await
first, and only call update_verification_token(...) after the send succeeds),
and if you must persist before send, ensure you restore the previous token on
any send error; refer to update_verification_token,
state.email.send_verification_email, and user.get("id")/user.get("email") to
locate the relevant code to reorder or add rollback 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: ASSERTIVE
Plan: Pro
Run ID: 6d9bc84e-8a0e-4d3d-87d6-c8afbb53d35e
📒 Files selected for processing (6)
client/src/app/(auth)/layout.cssclient/src/app/(auth)/verify/VerifyContent.tsxclient/src/app/(auth)/verify/page.tsxclient/src/lib/api/auth.tsclient/src/lib/types/apiResponse.tsrust-server/src/features/auth/handlers.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/`(auth)/verify/page.tsx:
- Around line 5-7: The searchParams type and handling for token is too narrow
and can crash when Next.js supplies repeated query keys; update the type from
token?: string to token?: string | string[] and, before calling .trim() (where
token is used in the verify page logic), coerce token to a single string by
checking Array.isArray(token) and taking token = token[0] (or undefined if
empty), then safely call token = token?.trim(); adjust any downstream
null/undefined checks to use the normalized token variable so
invalid/multi-value tokens fall through to the invalid-link fallback instead of
throwing.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 18150c5b-a2b5-40ab-8eaf-cbcd0b07a060
📒 Files selected for processing (1)
client/src/app/(auth)/verify/page.tsx
| searchParams: Promise<{ | ||
| token?: string; | ||
| }>; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Next.js App Router page props, what is the exact runtime/type shape of searchParamsvalues for repeated query keys (e.g.?token=a&token=b)? Is it string | string[] | undefined?
💡 Result:
In Next.js App Router page components (server components), the searchParams prop has the runtime/type shape { [key: string]: string | string[] | undefined }. For repeated query keys like ?token=a&token=b, the value of searchParams.token is string[] containing ['a', 'b']. This matches the proposed type string | string[] | undefined for individual values (per key). In Next.js 15+, searchParams is a Promise<{ [key: string]: string | string[] | undefined }>, which must be awaited in async page components. This behavior follows standard URLSearchParams parsing where duplicate keys produce arrays, as confirmed across multiple sources including GitHub discussions, Stack Overflow answers with code examples, and Next.js tutorial type definitions.
Citations:
- 1: Why `params` and `searchParams` type is different with docs? vercel/next.js#57463
- 2: https://stackoverflow.com/questions/79124951/type-error-in-next-js-route-type-params-id-string-does-not-satis
- 3: https://stackoverflow.com/questions/79178658/how-to-get-url-params-in-next-js-15-on-the-server-side
- 4: https://www.owolf.com/blog/extracting-url-parameters-in-nextjs-15-a-practical-guide
- 5: https://tribhuvancode.medium.com/how-to-use-searchparams-in-next-js-server-components-2025-fix-ffc7510f2477
🏁 Script executed:
cat -n client/src/app/(auth)/verify/page.tsxRepository: reijjo/tarpit2.0
Length of output: 191
🏁 Script executed:
cat -n "client/src/app/(auth)/verify/page.tsx"Repository: reijjo/tarpit2.0
Length of output: 771
Handle multi-value token query params before calling .trim()—a runtime trap in Next.js.
Here's the situation: Next.js searchParams can deliver repeated query keys as arrays. When a user hits ?token=a&token=b, params.token becomes ['a', 'b']. Your current code calls .trim() directly on line 12, which will throw a TypeError at runtime because arrays don't have a .trim() method. You'll serve a 500 error instead of the invalid-link fallback.
The type definition on line 6 is too narrow. It says token?: string, but it should be token?: string | string[] to match what Next.js actually delivers at runtime.
The fix: Before trimming, detect whether token is an array and take the first element:
Suggested fix
type VerifyPageProps = {
searchParams: Promise<{
- token?: string;
+ token?: string | string[];
}>;
};
export default async function VerifyPage({ searchParams }: VerifyPageProps) {
const params = await searchParams;
- const token = params.token?.trim();
+ const rawToken = params.token;
+ const token = Array.isArray(rawToken)
+ ? rawToken[0]?.trim()
+ : rawToken?.trim();
if (!token) {
return <p>Invalid verification link</p>;
}This pattern is your insurance policy: you handle the edge case gracefully, type accurately, and keep the happy path clear.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/`(auth)/verify/page.tsx around lines 5 - 7, The searchParams
type and handling for token is too narrow and can crash when Next.js supplies
repeated query keys; update the type from token?: string to token?: string |
string[] and, before calling .trim() (where token is used in the verify page
logic), coerce token to a single string by checking Array.isArray(token) and
taking token = token[0] (or undefined if empty), then safely call token =
token?.trim(); adjust any downstream null/undefined checks to use the normalized
token variable so invalid/multi-value tokens fall through to the invalid-link
fallback instead of throwing.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style