[WEB-2778] chore: private project join restriction #6082
Conversation
WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
web/core/layouts/auth-layout/project-wrapper.tsx (1)
Line range hint
165-178: Simplify the condition and improve error messagingThe empty state implementation looks good, but there are a few suggestions for improvement:
- The condition
!!hasPermissionToCurrentProject === falsecan be simplified- The error message could be more specific about private vs public projects to align with the PR objectives
- Consider adding error boundary protection for this component
Here's the suggested improvement:
- if (!loader && !projectExists && projectId && !!hasPermissionToCurrentProject === false) + if (!loader && !projectExists && projectId && !hasPermissionToCurrentProject) return ( <div className="grid h-screen place-items-center bg-custom-background-100"> <EmptyState - title="No such project exists" - description="Try creating a new project" + title="Project not found or access restricted" + description="This project may be private or doesn't exist. You can try creating a new project." image={emptyProject} primaryButton={{ text: "Create Project", onClick: () => { setTrackElement("Projects page empty state"); toggleCreateProjectModal(true); }, }} /> </div> );Also, consider wrapping the component with an error boundary:
import { ErrorBoundary } from "@/components/common"; export const ProjectAuthWrapper: FC<IProjectAuthWrapper> = observer((props) => { return ( <ErrorBoundary> {/* existing component code */} </ErrorBoundary> ); });apiserver/plane/app/views/project/base.py (1)
179-182: Security enhancement looks good, but consider public projectsThe additional filtering ensures that only active project members can access project details, which is a good security measure. However, consider making this filter conditional based on the project's visibility/network settings to maintain accessibility for public projects.
Consider updating the filter to handle public projects:
.filter( + Q(network=2) | # Public projects + Q( project_projectmember__member=self.request.user, project_projectmember__is_active=True, + ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apiserver/plane/app/views/project/base.py(1 hunks)web/core/layouts/auth-layout/project-wrapper.tsx(1 hunks)
🔇 Additional comments (2)
web/core/layouts/auth-layout/project-wrapper.tsx (1)
Line range hint 1-178: Verify permission handling for private projects
The implementation correctly checks permissions and shows appropriate UI states. However, let's verify if all permission-related code paths are properly tested.
Also, please ensure that:
- The
JoinProjectcomponent properly handles private project scenarios - Error messages clearly indicate when access is denied due to project privacy settings
- Permission checks are consistent with the backend changes in
ProjectViewSet
✅ Verification successful
Based on the search results, I can now verify the permission handling implementation. Let me generate a final response:
Permission handling implementation is comprehensive and correct
The implementation properly handles project access permissions through:
- Correct usage of
EUserPermissionsenum with proper hierarchy (GUEST < MEMBER < ADMIN) - Proper permission level checks using
EUserPermissionsLevel.PROJECT - Consistent permission validation using
allowPermissionshelper across components - Appropriate UI states:
- Loading spinner when checking permissions
JoinProjectcomponent for unauthorized access- Project not found state with proper messaging
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for comprehensive test coverage of permission-related scenarios
# Look for test files related to ProjectAuthWrapper
fd -e test.tsx -e spec.tsx "project.*wrapper"
# Check for permission-related test cases
rg -A 5 "test\(.*permission.*private.*project"
# Look for related permission constants
rg "EUserPermissions|EUserPermissionsLevel"
Length of output: 57293
apiserver/plane/app/views/project/base.py (1)
179-182: Consider API compatibility and caching implications
The new access restrictions might affect API clients and caching strategies:
- API clients should handle 404 responses appropriately when accessing restricted projects.
- Consider reviewing and updating any caching implementations to account for user-specific access restrictions.
Let's verify the API usage patterns:
…e into chore/project-restriction
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
apiserver/plane/app/views/project/invite.py (2)
139-144: Enhance error handling and improve error message clarity.The current implementation has several areas for improvement:
- No handling for non-existent workspace_member
- Generic error message doesn't explain the required role
+ if not workspace_member: + return Response( + {"error": "You are not a member of this workspace"}, + status=status.HTTP_403_FORBIDDEN, + ) + if workspace_member.role != 20: return Response( - {"error": "You do not have permission to join the project"}, + {"error": "Only workspace admins can join projects directly. Please request an invitation from a project admin."}, status=status.HTTP_403_FORBIDDEN, )
139-144: Add docstring to explain the permission model.The method lacks documentation about the role-based access control model being implemented.
def create(self, request, slug): + """ + Handle bulk project joining requests. + + This endpoint is restricted to workspace admins (role=20) only as part of + the private project access control system. Other members must be explicitly + invited to join projects. + + Args: + request: The HTTP request object containing project_ids in the data + slug: The workspace slug + + Returns: + Response: HTTP 201 if successful, HTTP 403 if permission denied + """ project_ids = request.data.get("project_ids", [])
chore:
Issue Link: WEB-2778
Summary by CodeRabbit
New Features
Bug Fixes