[Bug Fix] Enforce author ownership check for updating and deleting blogs - #22
[Bug Fix] Enforce author ownership check for updating and deleting blogs#22JyantiM wants to merge 2 commits into
Conversation
|
@jyantimalan is attempting to deploy a commit to the Aaditya's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesBlog ownership validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 2
🧹 Nitpick comments (1)
server/controllers/blogs.controller.js (1)
38-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both mutation endpoints.
Cover these cases for
updateBloganddeleteBlog:
- Blog author receives 200.
- Missing blog receives 404.
- Authenticated non-owner receives 403.
- Missing-author records receive a controlled response.
- Concurrent ownership changes do not permit stale mutations.
Assert the persisted blog state, not only the response status.
🤖 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 `@server/controllers/blogs.controller.js` around lines 38 - 67, Add regression tests for the updateBlog and deleteBlog controller endpoints covering successful author mutations (200), missing blogs (404), non-owner access (403), records without an author returning a controlled response, and concurrent ownership changes preventing stale mutations. Verify the database state after each mutation rather than asserting response status alone.
🤖 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 `@server/controllers/blogs.controller.js`:
- Around line 41-43: The authorization checks in updateBlog
(server/controllers/blogs.controller.js:41-43) and deleteBlog
(server/controllers/blogs.controller.js:62-64) must safely handle blogs without
an author; add a null-safe guard before calling toString while preserving the
unauthorized response. Migrate existing blog documents to populate author, and
update the blog schema in server/models/blogs.model.js to mark author as
required.
- Around line 45-49: Update updateBlog and deleteBlog to perform ownership
checks atomically by replacing _id-only mutations with owner-filtered
findOneAndUpdate and findOneAndDelete queries using the blog ID and
authenticated owner. In both methods, return a 404 when the mutation returns
null, while preserving the existing success responses for a returned blog or
successful deletion.
---
Nitpick comments:
In `@server/controllers/blogs.controller.js`:
- Around line 38-67: Add regression tests for the updateBlog and deleteBlog
controller endpoints covering successful author mutations (200), missing blogs
(404), non-owner access (403), records without an author returning a controlled
response, and concurrent ownership changes preventing stale mutations. Verify
the database state after each mutation rather than asserting response status
alone.
🪄 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 Plus
Run ID: 9de0d6e8-a09f-4686-baa6-9abec32a381c
📒 Files selected for processing (1)
server/controllers/blogs.controller.js
|
Hi! I've fully resolved the issue and incorporated the CodeRabbit AI feedback. However, the Vercel check is currently blocked could a team member please authorize the Vercel deployment so the checks can finish running? Thanks! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
Description
This is a Bug Fix.
The goal of this change is to secure the blog update and delete endpoints by ensuring that only the original author of a blog can modify or delete it. Previously, these routes only checked if the user was authenticated and if the blog existed. This meant any logged in user could potentially manipulate another user's blog.
I updated the logic to fetch the blog first and verify that the requesting user's ID matches the blog author's ID. If they do not match, the API now correctly returns a 403 Unauthorized error. This brings the security of these endpoints in line with the existing comment deletion logic.
Related Issues
Closes #21
PR Type & Checklist
Code & Testing
Security & Architecture (Crucial)
📸 Screenshots / Demos (If applicable)
Not applicable for this backend security fix.
Summary by CodeRabbit