feat: add secure account deletion functionality#571
Open
adityayadav176 wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR implements secure account deletion functionality allowing users to permanently delete their accounts. It adds backend password verification, cascading deletion of user data (including messages), session invalidation, and a frontend modal-based UI with password confirmation and CSRF protection. ChangesAccount Deletion Feature
Sequence DiagramsequenceDiagram
participant SettingsPage as Frontend:<br/>SettingsPage
participant Router as Backend:<br/>Router
participant Controller as Backend:<br/>deleteProfile
participant Database as Database:<br/>User/Message
SettingsPage->>Router: DELETE /delete-account<br/>(password, XSRF-TOKEN)
Router->>Controller: protectRoute passes req.userId
Controller->>Database: Find user by req.userId
Controller->>Controller: bcrypt.compare password
Controller->>Database: Delete messages (sender OR receiver)
Controller->>Database: Delete user document
Controller->>Router: Clear jwt & XSRF-TOKEN cookies
Controller->>SettingsPage: 200 success JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
Author
|
Hi @UTKARSHH20 , I have successfully implemented the secure Account Deletion functionality. Changes Implemented
This introduces a significant security-focused feature and improves user account management. Kindly review the implementation and, if it meets the project requirements, please consider marking it as Critical and adding the appropriate GSSoC'26 labels. Closes #515 Thank you! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/.env.example`:
- Line 13: The EMAIL_PASSWORD value in the .env example is a real-looking
secret; replace the current value for the EMAIL_PASSWORD entry with a
non-sensitive placeholder (e.g., EMAIL_PASSWORD=your_email_password_here or
EMAIL_PASSWORD=CHANGE_ME) so the example no longer contains secret-looking
credentials and clearly signals that users must supply their own secret.
In `@backend/src/controllers/user.controller.js`:
- Around line 71-80: Wrap the two destructive operations (Message.deleteMany and
User.findByIdAndDelete) in a single transactional boundary or convert to a
two-phase delete; for a transaction use Mongoose sessions: obtain a session via
mongoose.startSession(), call session.withTransaction(async () => { await
Message.deleteMany({ $or: [{ senderId: userId }, { receiverId: userId }] }, {
session }); await User.findByIdAndDelete(userId, { session }); }); and ensure
you end the session and propagate errors; alternatively implement a two-phase
soft-delete by marking User as "deleting" (e.g., userController sets
user.deleting=true) and have a background job perform the actual
Message.deleteMany and final User removal to avoid partial application.
- Around line 79-80: The delete flow currently calls
User.findByIdAndDelete(userId) and never removes the Cloudinary asset; update
the deleteProfile flow to first fetch the user (e.g., via User.findById or
findByIdAndUpdate with return) read the profilePicture field (upload.secure_url
or ideally upload.public_id), call cloudinary.uploader.destroy(public_id) (or
parse the public_id from secure_url if public_id is not stored), await
successful destroy (handle errors/no-op if no image), then proceed to remove the
user with User.findByIdAndDelete(userId); ensure you reference
User.findByIdAndDelete, profilePicture (or upload.public_id), and
cloudinary.uploader.destroy in the change.
- Around line 51-62: In deleteProfile, guard against missing password input and
passwordless (OAuth) accounts before calling bcrypt.compare by validating
req.body.password is a non-empty string and that user.password exists; return a
400 with a clear message if either check fails. Wrap the deletes in a MongoDB
session/transaction (use startSession and session.withTransaction) and perform
Message.deleteMany(...) and User.findByIdAndDelete(...) inside that transaction
to avoid partial deletion. Also, if user.profilePicture (or its publicId)
exists, call the Cloudinary deletion (e.g.,
cloudinary.uploader.destroy(publicId)) as part of the same transactional flow or
immediately after successful user deletion to remove the stored profile picture.
Ensure to end/abort the session on errors and surface appropriate error
responses.
In `@frontend/pages/SettingsPage.jsx`:
- Line 12: Replace the raw axios import and hardcoded URL usage in
SettingsPage.jsx with the shared axios client and the relative endpoint path;
import the shared client (e.g., apiClient from the existing axios wrapper)
instead of `import axios from "axios"`, and update the account-deletion calls
that use `http://localhost:5001/...` (including the logic around the delete
account handler at the block around lines ~107-125) to call
apiClient.post('/users/delete-account') so XSRF and credentials logic defined in
the shared client are reused. Ensure you update all occurrences in this file to
use the shared client and the `/users/delete-account` path.
- Around line 90-91: Add an in-flight isDeleting boolean state (e.g., const
[isDeleting, setIsDeleting] = useState(false)) in SettingsPage.jsx and use it to
prevent duplicate requests: in the delete-confirm handler (the function that
sends the DELETE request from the account delete modal) early-return if
isDeleting is true, setIsDeleting(true) before the async fetch/axios call, and
setIsDeleting(false) in a finally block so state is cleared on both success and
error. Disable the modal action buttons and the "Delete Permanently" button when
isDeleting is true and change the button label to a progress state like
"Deleting…" while in-flight; apply the same guard/disable behavior to the other
delete-related handlers/components referenced in the file (the modal confirm
handler and the other delete button locations).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8039d030-2311-4c38-a3eb-11215b32250a
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
backend/.env.examplebackend/package.jsonbackend/src/controllers/user.controller.jsbackend/src/routes/user.route.jsfrontend/pages/SettingsPage.jsx
| VAPID_PRIVATE_KEY=your_vapid_private_key | ||
| VAPID_SUBJECT=mailto:your@email.com | ||
| EMAIL=mailto:your@email.com | ||
| EMAIL_PASSWORD=vzktivbgeaqcdczr |
There was a problem hiding this comment.
Replace the example password with a placeholder.
vzktivbgeaqcdczr looks like a real app password, not a dummy value. Keeping secret-looking credentials in .env.example normalizes committing secrets and risks accidental reuse or leakage.
Suggested change
-EMAIL_PASSWORD=vzktivbgeaqcdczr
+EMAIL_PASSWORD=your_email_app_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.
Suggested change
| EMAIL_PASSWORD=vzktivbgeaqcdczr | |
| EMAIL_PASSWORD=your_email_app_password |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 13-13: [UnorderedKey] The EMAIL_PASSWORD key should go before the GOOGLE_CLIENT_ID key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/.env.example` at line 13, The EMAIL_PASSWORD value in the .env
example is a real-looking secret; replace the current value for the
EMAIL_PASSWORD entry with a non-sensitive placeholder (e.g.,
EMAIL_PASSWORD=your_email_password_here or EMAIL_PASSWORD=CHANGE_ME) so the
example no longer contains secret-looking credentials and clearly signals that
users must supply their own secret.
Comment on lines
+51
to
+62
| const { password } = req.body; | ||
|
|
||
| const user = await User.findById(userId); | ||
|
|
||
| if (!user) { | ||
| return res.status(404).json({ | ||
| success: false, | ||
| message: "User not found", | ||
| }); | ||
| } | ||
|
|
||
| const isMatch = await bcrypt.compare(password, user.password); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "User schema fields related to local-vs-OAuth auth:"
sed -n '1,80p' backend/src/models/user.model.js
echo
echo "All bcrypt.compare call sites:"
rg -n -C2 'bcrypt\.compare\s*\(' backend/src
echo
echo "Any Google/OAuth-related auth paths:"
rg -n -C2 'googleId|google' backend/srcRepository: UTKARSHH20/fullstack-chat-app
Length of output: 7661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate deleteProfile in user.controller.js:"
rg -n "deleteProfile" backend/src/controllers/user.controller.js
echo
echo "Show deleteProfile implementation (around the reported lines 51-62):"
sed -n '1,140p' backend/src/controllers/user.controller.js | nl -ba | sed -n '35,120p'
echo
echo "Search for profile image / Cloudinary cleanup across backend/src:"
rg -n "cloudinary|profilePicture|XSRF-TOKEN|jwt|delete.*profile|remove.*image" backend/src -S
echo
echo "Search for message deletion logic (senderId/receiverId) specifically:"
rg -n "senderId|receiverId|deleteMany" backend/src/controllers backend/src/models -SRepository: UTKARSHH20/fullstack-chat-app
Length of output: 313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate deleteProfile in user.controller.js:"
rg -n "export const deleteProfile" backend/src/controllers/user.controller.js
echo
echo "Show deleteProfile implementation (user.controller.js, lines 1-220):"
sed -n '1,220p' backend/src/controllers/user.controller.js | cat -n | sed -n '35,190p'
echo
echo "Search for profile image / Cloudinary cleanup across backend/src:"
rg -n "cloudinary|profilePicture|XSRF-TOKEN|jwt|delete.*profile|remove.*image|destroy.*image" backend/src -S
echo
echo "Search for message deletion logic (senderId/receiverId) specifically:"
rg -n "senderId|receiverId|deleteMany" backend/src/controllers backend/src/models -SRepository: UTKARSHH20/fullstack-chat-app
Length of output: 14302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect full user.model.js for any hooks (up to 240 lines):"
sed -n '1,240p' backend/src/models/user.model.js | cat -n
echo
echo "Search for Cloudinary destroy usage related to profilePicture/user deletion:"
rg -n "profilePicture.*destroy|destroy.*profilePicture|destroy.*user|on\\(\"delete|pre\\(\"delete|post\\(\"delete" backend/src -SRepository: UTKARSHH20/fullstack-chat-app
Length of output: 1871
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for profile-picture update/delete and Cloudinary destroy in auth/user-related controllers:"
rg -n "updateProfilePicture|profilePicture.*(upload|destroy|delete)|destroy\\(" backend/src/controllers backend/src/services backend/src/lib -S
echo
echo "Search for any 'delete user' helpers or routes besides deleteProfile:"
rg -n "deleteProfile|findByIdAndDelete\\(|remove\\(\\)|deleteOne\\(|deleteMany\\(\\)" backend/src -SRepository: UTKARSHH20/fullstack-chat-app
Length of output: 1425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Show delete-account route middleware stack:"
sed -n '1,120p' backend/src/routes/user.route.js | cat -n
echo
echo "Show validateLogin / validateSignup behavior (if present):"
rg -n "export const validateLogin|validateLogin|export const validateSignup|validateSignup" backend/src/middleware/validate.js backend/src/middleware -S
sed -n '1,220p' backend/src/middleware/validate.js | cat -n
echo
echo "Inspect auth.controller.js login section for how bcrypt.compare inputs are handled:"
rg -n "export const login|bcrypt\\.compare" backend/src/controllers/auth.controller.js
sed -n '40,140p' backend/src/controllers/auth.controller.js | cat -n
echo
echo "Inspect updateProfilePicture to see what Cloudinary fields are stored (secure_url vs public_id):"
sed -n '160,260p' backend/src/controllers/auth.controller.js | cat -nRepository: UTKARSHH20/fullstack-chat-app
Length of output: 11395
Guard password confirmation input and passwordless accounts before bcrypt.compare in deleteProfile.
deleteProfilecallsbcrypt.compare(password, user.password)without validating thatpasswordis a string or thatuser.passwordis present;User.passworddefaults tonullfor OAuth accounts, and this path is currently missing the same guard used bylogin.deleteProfileperformsMessage.deleteMany(...)andUser.findByIdAndDelete(...)as separate operations; use a MongoDB transaction/session to avoid partial deletion.- No Cloudinary/profile-picture deletion runs in
deleteProfile(message attachment cleanup exists elsewhere, but userprofilePictureis not destroyed here).
Suggested change
export const deleteProfile = catchAsync(async (req, res) => {
const userId = req.userId;
const { password } = req.body;
const user = await User.findById(userId);
if (!user) {
return res.status(404).json({
success: false,
message: "User not found",
});
}
+
+ if (typeof password !== "string" || password.trim() === "") {
+ return res.status(400).json({
+ success: false,
+ message: "Password is required",
+ });
+ }
+
+ if (!user.password) {
+ return res.status(400).json({
+ success: false,
+ message: "This account does not support password confirmation",
+ });
+ }
const isMatch = await bcrypt.compare(password, user.password);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/user.controller.js` around lines 51 - 62, In
deleteProfile, guard against missing password input and passwordless (OAuth)
accounts before calling bcrypt.compare by validating req.body.password is a
non-empty string and that user.password exists; return a 400 with a clear
message if either check fails. Wrap the deletes in a MongoDB session/transaction
(use startSession and session.withTransaction) and perform
Message.deleteMany(...) and User.findByIdAndDelete(...) inside that transaction
to avoid partial deletion. Also, if user.profilePicture (or its publicId)
exists, call the Cloudinary deletion (e.g.,
cloudinary.uploader.destroy(publicId)) as part of the same transactional flow or
immediately after successful user deletion to remove the stored profile picture.
Ensure to end/abort the session on errors and surface appropriate error
responses.
Comment on lines
+71
to
+80
| // Delete user's messages | ||
| await Message.deleteMany({ | ||
| $or: [ | ||
| { senderId: userId }, | ||
| { receiverId: userId }, | ||
| ], | ||
| }); | ||
|
|
||
| // Delete user | ||
| await User.findByIdAndDelete(userId); |
There was a problem hiding this comment.
Make the destructive deletes atomic.
If message deletion succeeds and user deletion fails, this endpoint leaves the account alive with its history already gone. Account removal needs a single transactional boundary, or a two-phase delete state, so it cannot partially apply.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/user.controller.js` around lines 71 - 80, Wrap the
two destructive operations (Message.deleteMany and User.findByIdAndDelete) in a
single transactional boundary or convert to a two-phase delete; for a
transaction use Mongoose sessions: obtain a session via mongoose.startSession(),
call session.withTransaction(async () => { await Message.deleteMany({ $or: [{
senderId: userId }, { receiverId: userId }] }, { session }); await
User.findByIdAndDelete(userId, { session }); }); and ensure you end the session
and propagate errors; alternatively implement a two-phase soft-delete by marking
User as "deleting" (e.g., userController sets user.deleting=true) and have a
background job perform the actual Message.deleteMany and final User removal to
avoid partial application.
Comment on lines
+79
to
+80
| // Delete user | ||
| await User.findByIdAndDelete(userId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "Image/profile fields on the user model:"
rg -n -C2 'profile|avatar|image|publicId|cloudinary' backend/src/models/user.model.js
echo
echo "Cloudinary delete/destroy usage:"
rg -n -C2 'cloudinary|uploader\.destroy|destroy\s*\(' backend/src
echo
echo "User delete hooks or cleanup middleware:"
rg -n -C2 'pre\(|post\(|findOneAndDelete|findByIdAndDelete|deleteOne' backend/src/models backend/srcRepository: UTKARSHH20/fullstack-chat-app
Length of output: 8064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the user model + delete controller code
echo "=== backend/src/models/user.model.js ==="
ls -la backend/src/models/user.model.js
nl -ba backend/src/models/user.model.js | sed -n '1,220p'
echo
echo "=== backend/src/controllers/user.controller.js (around deletion) ==="
nl -ba backend/src/controllers/user.controller.js | sed -n '1,220p'Repository: UTKARSHH20/fullstack-chat-app
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for Cloudinary deletion/destroy logic that might be used during user deletion
echo "=== Cloudinary destroy usage anywhere ==="
rg -n --glob='backend/src/**/*.js' "cloudinary\.uploader\.destroy|destroy\s*\(" backend/src | head -n 200
echo
echo "=== Any helper that extracts Cloudinary public IDs ==="
rg -n --glob='backend/src/**/*.js' "extractPublicId" backend/src | head -n 200
echo
echo "=== User deletion flow entry points ==="
rg -n --glob='backend/src/**/*.js' "findByIdAndDelete\(userId\)|deleteUser|/delete|delete.*user" backend/src/controllers backend/src/routes | head -n 200Repository: UTKARSHH20/fullstack-chat-app
Length of output: 1210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== User controller: find deleteProfile + updateProfilePicture ==="
rg -n "function\s+deleteProfile|const\s+deleteProfile|deleteProfile\s*=" backend/src/controllers/user.controller.js
rg -n "function\s+updateProfilePicture|const\s+updateProfilePicture|updateProfilePicture\s*=" backend/src/controllers/user.controller.js
rg -n "cloudinary|uploader\.destroy|destroy\s*\(" backend/src/controllers/user.controller.js
echo
echo "=== Print user.controller.js (top + around deleteProfile) ==="
wc -l backend/src/controllers/user.controller.js
cat -n backend/src/controllers/user.controller.js | head -n 220
echo
# best-effort: print around lines ~60-120 (where the delete snippet was)
cat -n backend/src/controllers/user.controller.js | sed -n '50,130p'
echo
echo "=== Print user.model.js ==="
wc -l backend/src/models/user.model.js
cat -n backend/src/models/user.model.js | sed -n '1,260p'Repository: UTKARSHH20/fullstack-chat-app
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for extractPublicId usage besides message.controller.js ==="
rg -n --glob='backend/src/**/*.js' "extractPublicId\s*\(" backend/src | head -n 200
echo
echo "=== Search for profilePicture update/delete logic (cloudinary destroy) ==="
rg -n --glob='backend/src/**/*.js' "profilePicture" backend/src/controllers backend/src/models | head -n 200
echo
echo "=== Search for Mongoose hooks in user.model.js ==="
rg -n "pre\s*\(|post\s*\(|schema\.(pre|post)|hooks" backend/src/models/user.model.jsRepository: UTKARSHH20/fullstack-chat-app
Length of output: 2370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== backend/src/controllers/user.controller.js (full, with line numbers via cat -n) ==="
wc -l backend/src/controllers/user.controller.js
cat -n backend/src/controllers/user.controller.js | sed -n '1,220p'
echo
echo "=== backend/src/models/user.model.js (with line numbers) ==="
wc -l backend/src/models/user.model.js
cat -n backend/src/models/user.model.js | sed -n '1,260p'
echo
echo "=== Cloudinary usage specifically in user.controller.js ==="
rg -n --glob='backend/src/controllers/user.controller.js' "cloudinary\.uploader|uploader\.destroy|uploader\.|destroy\s*\(" backend/src/controllers/user.controller.js || trueRepository: UTKARSHH20/fullstack-chat-app
Length of output: 4946
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "updateProfilePicture" backend/src/controllers/auth.controller.js
rg -n "profilePicture" backend/src/controllers/auth.controller.js | head -n 50
sed -n '150,260p' backend/src/controllers/auth.controller.jsRepository: UTKARSHH20/fullstack-chat-app
Length of output: 4660
Add Cloudinary profile-image cleanup before deleting the user document
The deleteProfile flow deletes the user via await User.findByIdAndDelete(userId) without any Cloudinary uploader.destroy call. The user model has no Mongoose delete hooks, and profilePicture is stored as upload.secure_url, so the current delete path can’t clean up the underlying Cloudinary asset (it will orphan it).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/controllers/user.controller.js` around lines 79 - 80, The delete
flow currently calls User.findByIdAndDelete(userId) and never removes the
Cloudinary asset; update the deleteProfile flow to first fetch the user (e.g.,
via User.findById or findByIdAndUpdate with return) read the profilePicture
field (upload.secure_url or ideally upload.public_id), call
cloudinary.uploader.destroy(public_id) (or parse the public_id from secure_url
if public_id is not stored), await successful destroy (handle errors/no-op if no
image), then proceed to remove the user with User.findByIdAndDelete(userId);
ensure you reference User.findByIdAndDelete, profilePicture (or
upload.public_id), and cloudinary.uploader.destroy in the change.
| import useAuthStore from "../src/store/useAuthStore" | ||
| import StatusMoodSelector from "../components/StatusMoodSelector" | ||
| import toast from "react-hot-toast" | ||
| import axios from "axios" |
There was a problem hiding this comment.
Use the shared axios client for this flow.
Hardcoding http://localhost:5001/... makes account deletion fail outside local development, and this duplicates XSRF/credentials logic that frontend/lib/axios.js already centralizes. Call the shared client with /users/delete-account instead.
Suggested change
-import axios from "axios"
+import axiosInstance from "../lib/axios"
...
const handleDeleteAccount = async () => {
try {
- const csrfToken = document.cookie
- .split("; ")
- .find((row) => row.startsWith("XSRF-TOKEN="))
- ?.split("=")[1];
-
- const res = await axios.delete(
- "http://localhost:5001/api/users/delete-account",
- {
- data: {
- password: deletePassword,
- },
- withCredentials: true,
- headers: {
- "x-xsrf-token": csrfToken,
- },
- }
- );
+ const res = await axiosInstance.delete("/users/delete-account", {
+ data: { password: deletePassword },
+ });Also applies to: 107-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/pages/SettingsPage.jsx` at line 12, Replace the raw axios import and
hardcoded URL usage in SettingsPage.jsx with the shared axios client and the
relative endpoint path; import the shared client (e.g., apiClient from the
existing axios wrapper) instead of `import axios from "axios"`, and update the
account-deletion calls that use `http://localhost:5001/...` (including the logic
around the delete account handler at the block around lines ~107-125) to call
apiClient.post('/users/delete-account') so XSRF and credentials logic defined in
the shared client are reused. Ensure you update all occurrences in this file to
use the shared client and the `/users/delete-account` path.
Comment on lines
+90
to
+91
| const [showDeleteModal, setShowDeleteModal] = useState(false); | ||
| const [deletePassword, setDeletePassword] = useState(""); |
There was a problem hiding this comment.
Prevent duplicate delete requests and expose an in-flight state.
Delete Permanently stays clickable while the DELETE is running, so double-clicks can fire concurrent destructive requests and turn a success path into a later error toast. Track isDeleting, disable the modal actions, and switch the button label to a progress state.
Suggested change
const [showDeleteModal, setShowDeleteModal] = useState(false);
const [deletePassword, setDeletePassword] = useState("");
+const [isDeleting, setIsDeleting] = useState(false);
const handleDeleteAccount = async () => {
+ if (isDeleting) return;
+ setIsDeleting(true);
try {
const res = await axiosInstance.delete("/users/delete-account", {
data: {
password: deletePassword,
},
});
toast.success(res.data.message);
setShowDeleteModal(false);
setDeletePassword("");
window.location.href = "/login";
} catch (error) {
toast.error(
error.response?.data?.message ||
"Failed to delete account"
);
+ } finally {
+ setIsDeleting(false);
}
};
...
<button
className="btn btn-outline"
+ disabled={isDeleting}
onClick={() => {
setShowDeleteModal(false);
setDeletePassword("");
}}
>
Cancel
</button>
<button
className="btn btn-error"
+ disabled={isDeleting || !deletePassword.trim()}
onClick={handleDeleteAccount}
>
- Delete Permanently
+ {isDeleting ? "Deleting..." : "Delete Permanently"}
</button>Also applies to: 107-139, 457-462
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/pages/SettingsPage.jsx` around lines 90 - 91, Add an in-flight
isDeleting boolean state (e.g., const [isDeleting, setIsDeleting] =
useState(false)) in SettingsPage.jsx and use it to prevent duplicate requests:
in the delete-confirm handler (the function that sends the DELETE request from
the account delete modal) early-return if isDeleting is true,
setIsDeleting(true) before the async fetch/axios call, and setIsDeleting(false)
in a finally block so state is cleared on both success and error. Disable the
modal action buttons and the "Delete Permanently" button when isDeleting is true
and change the button label to a progress state like "Deleting…" while
in-flight; apply the same guard/disable behavior to the other delete-related
handlers/components referenced in the file (the modal confirm handler and the
other delete button locations).
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
1 participant
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.
Description
Summary
Implemented a secure Account Deletion feature that allows authenticated users to permanently delete their accounts after password verification.
Changes Made
Security Enhancements
Testing
step 1
step2
step3 Click the delete account btn and enter your password for confirmation and delete your account
Closes #515
Summary by CodeRabbit