refactor: integrate Privy access token handling in delete and update#1629
Closed
pradipthaadhi wants to merge 1 commit into
Closed
Conversation
…task hooks * Updated useDeleteScheduledAction and useUpdateScheduledAction hooks to directly use getAccessToken from usePrivy for authentication. * Enhanced error handling to prompt users to sign in if the access token is not available. * Modified deleteTask and updateTask functions to accept the access token as a parameter for API calls, ensuring secure and authenticated requests.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughPrivy authentication integration has been added to task management operations. Hooks now retrieve access tokens via Changes
Sequence DiagramsequenceDiagram
participant Hook as useDeleteScheduledAction
participant Privy as usePrivy()
participant API as deleteTask()
participant Backend as Backend API
participant DB as Database/Storage
Hook->>Privy: getAccessToken()
Privy-->>Hook: accessToken (or null)
alt Token Available
Hook->>Hook: Call deleteTask(accessToken, {id})
API->>Backend: DELETE /tasks/{id}<br/>Header: Authorization Bearer
Backend->>DB: Delete task record
alt Success
DB-->>Backend: Record deleted
Backend-->>API: 200 OK
API-->>Hook: Success
else Not Found
Backend-->>API: 404 or error text
API->>DB: Fallback: deleteTaskFromDatabase()
API-->>Hook: Success
end
else No Token
Hook->>Hook: Show toast error
Hook-->>Hook: Throw error, abort
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
hooks/useUpdateScheduledAction.ts (1)
28-43:⚠️ Potential issue | 🟡 MinorDouble toast notification on authentication failure.
When
getAccessToken()returns null, line 30 shows a toast error, then the thrown error is caught at line 40, triggering another toast at line 42. The user sees two error messages for one failure.🔧 Proposed fix: early return after auth-specific toast
const accessToken = await getAccessToken(); if (!accessToken) { toast.error("Please sign in to update tasks"); - throw new Error("Please sign in to update tasks"); + setIsLoading(false); + return; }Alternatively, distinguish auth errors in the catch block to avoid duplicate toasts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useUpdateScheduledAction.ts` around lines 28 - 43, When getAccessToken() returns null the hook both shows toast.error("Please sign in to update tasks") and then throws, which is caught by the catch block and triggers a second generic toast; fix by preventing the catch from showing a duplicate toast for auth failures — either return early after showing the auth toast (in the block around getAccessToken()) or throw a specific AuthError from that branch and, in the catch for the function (use the same try/catch around updateTask/onSuccess), check error type/message before calling toast.error("Failed to update. Please try again.") so only non-auth errors produce the generic toast; refer to getAccessToken, updateTask, onSuccess, and toast to locate and implement the change.hooks/useDeleteScheduledAction.ts (1)
25-39:⚠️ Potential issue | 🟡 MinorSame double toast issue as in
useUpdateScheduledAction.Line 27 shows a toast, then the thrown error triggers another toast at line 38. This mirrors the same pattern flagged in the update hook.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDeleteScheduledAction.ts` around lines 25 - 39, The hook useDeleteScheduledAction currently shows a toast then throws when no accessToken, which combined with the catch toast causes duplicate toasts; update the function so that when getAccessToken() returns falsy you do not call toast.error("Please sign in to delete tasks") before throwing—just throw the Error to let the existing catch block handle user feedback (leave deleteTask(...), onSuccess, and the success toast unchanged); reference symbols: useDeleteScheduledAction, getAccessToken, deleteTask, onSuccess, toast.error.lib/tasks/deleteTask.ts (1)
20-26:⚠️ Potential issue | 🔴 CriticalSecurity vulnerability:
deleteTaskFromDatabasehas no authentication.The internal fallback deletion endpoint (
/api/scheduled-actions/delete) accepts requests without any authentication validation. WhiledeleteTaskcorrectly passes theaccessTokento the external Recoup API, the fallback path completely bypasses this check, allowing unauthenticated deletion of any task by directly calling the internal endpoint.This creates a critical security gap: an attacker could delete any task by making an authenticated request to
/api/scheduled-actions/deletewith a known task ID, regardless of whether they own it.Required fix: Add authentication to both the client function and server route:
Client-side: Forward access token to internal endpoint
-async function deleteTaskFromDatabase(taskId: string): Promise<void> { +async function deleteTaskFromDatabase(taskId: string, accessToken: string): Promise<void> { await fetch("/api/scheduled-actions/delete", { method: "DELETE", - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${accessToken}`, + }, body: JSON.stringify({ id: taskId }), }); }Update the call site at line 56:
- await deleteTaskFromDatabase(params.id); + await deleteTaskFromDatabase(params.id, accessToken);Server-side: Add token validation to the route
The DELETE handler in
app/api/scheduled-actions/delete/route.tsmust validate the Bearer token before allowing deletion. Currently it processes any request without authentication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/deleteTask.ts` around lines 20 - 26, The client function deleteTaskFromDatabase currently sends unauthenticated DELETEs to /api/scheduled-actions/delete; update deleteTaskFromDatabase to accept and forward the accessToken (e.g., add a parameter accessToken: string and include an Authorization: `Bearer ${accessToken}` header) so the internal endpoint receives the token, and update the caller deleteTask to pass its access token into deleteTaskFromDatabase. On the server side, modify the DELETE handler in app/api/scheduled-actions/delete/route.ts to validate the Bearer token from the Authorization header (reuse existing token validation logic used elsewhere) and reject requests with invalid/missing tokens before performing deletion.
🧹 Nitpick comments (2)
hooks/useUpdateScheduledAction.ts (1)
44-50: Query invalidation runs unconditionally infinallyblock.The invalidation at lines 46-49 executes even when the update fails. This triggers unnecessary refetches and network traffic on error scenarios. Consider moving invalidation to the success path only.
♻️ Proposed refactor
const updatedTask = await updateTask(accessToken, updates); onSuccess?.(updatedTask); toast.success(successMessage); + + queryClient.invalidateQueries({ + queryKey: ["scheduled-actions"], + exact: false, + }); return updatedTask; } catch (error) { console.error("Failed to update scheduled action:", error); toast.error("Failed to update. Please try again."); throw error; } finally { setIsLoading(false); - queryClient.invalidateQueries({ - queryKey: ["scheduled-actions"], - exact: false, - }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useUpdateScheduledAction.ts` around lines 44 - 50, The query invalidation for ["scheduled-actions"] is currently in the finally block and runs even on failures; move the queryClient.invalidateQueries call out of the finally and into the success path (after the awaited update call completes successfully) so it only runs on success, while keeping setIsLoading(false) in the finally; update the useUpdateScheduledAction flow to call queryClient.invalidateQueries({ queryKey: ["scheduled-actions"], exact: false }) (or equivalent) immediately after the successful update response and remove it from the finally block.hooks/useDeleteScheduledAction.ts (1)
13-46: Consider extracting shared auth-action pattern to reduce duplication.Both
useDeleteScheduledActionanduseUpdateScheduledActionshare nearly identical structures:
- Get access token via
getAccessToken()- Validate token with toast + throw pattern
- Execute API call
- Handle success callback + toast
- Error handling with console + toast
- Finally block with loading state + query invalidation
This is a good candidate for a shared utility or higher-order hook to keep the codebase DRY.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDeleteScheduledAction.ts` around lines 13 - 46, Both useDeleteScheduledAction and useUpdateScheduledAction duplicate the same auth+action+ui pattern (getAccessToken, toast+throw when missing, call API, run onSuccess+toast, catch+toast+console, finally setIsLoading + queryClient.invalidateQueries). Extract this into a shared helper (e.g., performAuthAction or withAuthAction) that accepts the async API call (deleteTask/updateTask), successMessage, and optional onSuccess callback; have the hook implementations call that helper and pass getAccessToken, toast handlers, and the ["scheduled-actions"] invalidate key so you remove the duplicated token-check, try/catch/finally, and invalidate logic from useDeleteScheduledAction and useUpdateScheduledAction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@hooks/useDeleteScheduledAction.ts`:
- Around line 25-39: The hook useDeleteScheduledAction currently shows a toast
then throws when no accessToken, which combined with the catch toast causes
duplicate toasts; update the function so that when getAccessToken() returns
falsy you do not call toast.error("Please sign in to delete tasks") before
throwing—just throw the Error to let the existing catch block handle user
feedback (leave deleteTask(...), onSuccess, and the success toast unchanged);
reference symbols: useDeleteScheduledAction, getAccessToken, deleteTask,
onSuccess, toast.error.
In `@hooks/useUpdateScheduledAction.ts`:
- Around line 28-43: When getAccessToken() returns null the hook both shows
toast.error("Please sign in to update tasks") and then throws, which is caught
by the catch block and triggers a second generic toast; fix by preventing the
catch from showing a duplicate toast for auth failures — either return early
after showing the auth toast (in the block around getAccessToken()) or throw a
specific AuthError from that branch and, in the catch for the function (use the
same try/catch around updateTask/onSuccess), check error type/message before
calling toast.error("Failed to update. Please try again.") so only non-auth
errors produce the generic toast; refer to getAccessToken, updateTask,
onSuccess, and toast to locate and implement the change.
In `@lib/tasks/deleteTask.ts`:
- Around line 20-26: The client function deleteTaskFromDatabase currently sends
unauthenticated DELETEs to /api/scheduled-actions/delete; update
deleteTaskFromDatabase to accept and forward the accessToken (e.g., add a
parameter accessToken: string and include an Authorization: `Bearer
${accessToken}` header) so the internal endpoint receives the token, and update
the caller deleteTask to pass its access token into deleteTaskFromDatabase. On
the server side, modify the DELETE handler in
app/api/scheduled-actions/delete/route.ts to validate the Bearer token from the
Authorization header (reuse existing token validation logic used elsewhere) and
reject requests with invalid/missing tokens before performing deletion.
---
Nitpick comments:
In `@hooks/useDeleteScheduledAction.ts`:
- Around line 13-46: Both useDeleteScheduledAction and useUpdateScheduledAction
duplicate the same auth+action+ui pattern (getAccessToken, toast+throw when
missing, call API, run onSuccess+toast, catch+toast+console, finally
setIsLoading + queryClient.invalidateQueries). Extract this into a shared helper
(e.g., performAuthAction or withAuthAction) that accepts the async API call
(deleteTask/updateTask), successMessage, and optional onSuccess callback; have
the hook implementations call that helper and pass getAccessToken, toast
handlers, and the ["scheduled-actions"] invalidate key so you remove the
duplicated token-check, try/catch/finally, and invalidate logic from
useDeleteScheduledAction and useUpdateScheduledAction.
In `@hooks/useUpdateScheduledAction.ts`:
- Around line 44-50: The query invalidation for ["scheduled-actions"] is
currently in the finally block and runs even on failures; move the
queryClient.invalidateQueries call out of the finally and into the success path
(after the awaited update call completes successfully) so it only runs on
success, while keeping setIsLoading(false) in the finally; update the
useUpdateScheduledAction flow to call queryClient.invalidateQueries({ queryKey:
["scheduled-actions"], exact: false }) (or equivalent) immediately after the
successful update response and remove it from the finally block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54d62621-2ba2-47a1-b5ef-85019f36cdd9
📒 Files selected for processing (4)
hooks/useDeleteScheduledAction.tshooks/useUpdateScheduledAction.tslib/tasks/deleteTask.tslib/tasks/updateTask.ts
sweetmantech
reviewed
Apr 8, 2026
| await deleteTaskFromDatabase(params.id); | ||
| return; | ||
| } | ||
| const responseText = await response.text(); |
Collaborator
There was a problem hiding this comment.
KISS - Why are you changing the error handling in this lib?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes