Fix project invitation authorization#9301
Conversation
|
Gaurav Singhal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughAdds explicit ChangesProjectInvitationsViewset Admin Permission Enforcement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (1)
apps/api/plane/tests/contract/app/test_project_invitation_app.py (1)
42-48: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd contract coverage for the new
destroypermission gate.Line 42 to Line 48 validate
list/retrieveonly;destroywas also changed in this PR and should be asserted as403for cross-workspace users.Suggested test extension
list_response = session_client.get(list_url) detail_response = session_client.get(detail_url) + delete_response = session_client.delete(detail_url) assert list_response.status_code == status.HTTP_403_FORBIDDEN assert detail_response.status_code == status.HTTP_403_FORBIDDEN + assert delete_response.status_code == status.HTTP_403_FORBIDDEN assert b"secret-project-invite-token" not in list_response.content assert b"secret-project-invite-token" not in detail_response.content🤖 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 `@apps/api/plane/tests/contract/app/test_project_invitation_app.py` around lines 42 - 48, The test currently only validates list and retrieve endpoints for 403 permission gates, but the destroy endpoint was also modified in this PR and needs contract coverage. Add a destroy_response by making a DELETE request to a destroy_url endpoint using session_client, similar to how list_response and detail_response are obtained. Assert that destroy_response returns status.HTTP_403_FORBIDDEN and verify that b"secret-project-invite-token" is not exposed in the destroy_response.content, matching the pattern of the existing assertions for list and detail endpoints.
🤖 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.
Nitpick comments:
In `@apps/api/plane/tests/contract/app/test_project_invitation_app.py`:
- Around line 42-48: The test currently only validates list and retrieve
endpoints for 403 permission gates, but the destroy endpoint was also modified
in this PR and needs contract coverage. Add a destroy_response by making a
DELETE request to a destroy_url endpoint using session_client, similar to how
list_response and detail_response are obtained. Assert that destroy_response
returns status.HTTP_403_FORBIDDEN and verify that b"secret-project-invite-token"
is not exposed in the destroy_response.content, matching the pattern of the
existing assertions for list and detail endpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73e501e0-fb15-4711-849a-f3449c7d1163
📒 Files selected for processing (2)
apps/api/plane/app/views/project/invite.pyapps/api/plane/tests/contract/app/test_project_invitation_app.py
Finding
Niro
TC-B92EA42Afound that an authenticated user from another workspace could list and retrieve invitation records for a private project, including invitee emails, roles, and raw invitation tokens.Original PoC
As a user who belongs only to another workspace, request:
GET /api/workspaces/<victim_slug>/projects/<victim_project_id>/invitations/GET /api/workspaces/<victim_slug>/projects/<victim_project_id>/invitations/<invitation_id>/Before this fix the API returned
200 OKand serialized project invitation data.Red test on unfixed code
Green test on fixed code
Fix
Add explicit project-admin permission gates to project invitation
list,retrieve, anddestroy, matching the existingcreategate. Added a regression test that verifies a user from another workspace receives 403 and cannot obtain the invitation token.Pentested and fixed by Niro
Summary by CodeRabbit
Bug Fixes
Tests