fix: permissions and tests for public project access

Author: haydnli-shopify

src/dstack/_internal/server/routers/projects.py

@@ -16,8 +16,9 @@
 )
 from dstack._internal.server.security.permissions import (
     Authenticated,
+    ProjectAdmin,
     ProjectManager,
-    ProjectManagerOrPublicJoin,
+    ProjectManagerOrPublicProject,
     ProjectManagerOrSelfLeave,
     ProjectMemberOrPublicAccess,
 )
@@ -105,7 +106,7 @@ async def set_project_members(
 async def add_project_members(
     body: AddProjectMemberRequest,
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManagerOrPublicJoin()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManagerOrPublicProject()),
 ) -> Project:
     user, project = user_project
     await projects.add_project_members(
@@ -143,7 +144,7 @@ async def remove_project_members(
 async def update_project_visibility(
     body: UpdateProjectVisibilityRequest,
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()),
 ) -> Project:
     user, project = user_project
     await projects.update_project_visibility(

src/dstack/_internal/server/schemas/projects.py

@@ -1,4 +1,4 @@
-from typing import Annotated, List, Optional
+from typing import Annotated, List
 
 from pydantic import Field
 
@@ -8,7 +8,7 @@
 
 class CreateProjectRequest(CoreModel):
     project_name: str
-    is_public: Optional[bool] = False
+    is_public: bool = False
 
 
 class UpdateProjectVisibilityRequest(CoreModel):
@@ -32,7 +32,6 @@ class SetProjectMembersRequest(CoreModel):
 
 
 class AddProjectMemberRequest(CoreModel):
-    # Always accept a list of members for cleaner API design
     members: List[MemberSetting]
 
 

src/dstack/_internal/server/security/permissions.py

@@ -58,7 +58,7 @@ async def __call__(
             raise error_invalid_token()
         project = await get_project_model_by_name(session=session, project_name=project_name)
         if project is None:
-            raise error_forbidden()
+            raise error_not_found()
         if user.global_role == GlobalRole.ADMIN:
             return user, project
         project_role = get_user_project_role(user=user, project=project)
@@ -110,9 +110,10 @@ async def __call__(
 
 class ProjectMemberOrPublicAccess:
     """
-    Allows access to project details for:
-    1. Project members (existing behavior)
-    2. Any authenticated user if the project is public
+    Allows access to project for:
+    - Global admins
+    - Project members
+    - Any authenticated user if the project is public
     """
 
     async def __call__(
@@ -130,28 +131,25 @@ async def __call__(
         if project is None:
             raise error_not_found()
 
-        # Global admins always have access
         if user.global_role == GlobalRole.ADMIN:
             return user, project
 
-        # Check if user is a project member
         project_role = get_user_project_role(user=user, project=project)
         if project_role is not None:
             return user, project
 
-        # If not a member, check if project is public
         if project.is_public:
             return user, project
 
-        # Neither member nor public project
         raise error_forbidden()
 
 
-class ProjectManagerOrPublicJoin:
+class ProjectManagerOrPublicProject:
     """
-    Allows:
-    1. Project managers to add any members
-    2. Any authenticated user to join public projects themselves
+    Allows access to project for:
+    - Global admins
+    - Project managers and admins
+    - Any authenticated user if the project is public
     """
 
     async def __call__(
@@ -167,16 +165,13 @@ async def __call__(
         if project is None:
             raise error_not_found()
 
-        # Global admin can always manage projects
         if user.global_role == GlobalRole.ADMIN:
             return user, project
 
-        # Project managers can add members
         project_role = get_user_project_role(user=user, project=project)
         if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]:
             return user, project
 
-        # For public projects, any authenticated user can join (will be validated in service layer)
         if project.is_public:
             return user, project
 
@@ -185,9 +180,10 @@ async def __call__(
 
 class ProjectManagerOrSelfLeave:
     """
-    Allows:
-    1. Project managers to remove any members
-    2. Any project member to leave (remove themselves)
+    Allows access to project for:
+    - Global admins
+    - Project managers (can remove any members)
+    - Project members (can remove themselves)
     """
 
     async def __call__(
@@ -199,15 +195,14 @@ async def __call__(
         user = await log_in_with_token(session=session, token=token.credentials)
         if user is None:
             raise error_invalid_token()
+
         project = await get_project_model_by_name(session=session, project_name=project_name)
         if project is None:
             raise error_not_found()
 
-        # Global admin can always manage projects
         if user.global_role == GlobalRole.ADMIN:
             return user, project
 
-        # Any project member can access (managers can remove others, members can leave)
         project_role = get_user_project_role(user=user, project=project)
         if project_role is not None:
             return user, project

src/tests/_internal/server/routers/test_projects.py

@@ -1801,33 +1801,6 @@ async def test_project_admin_can_update_visibility(
         assert response.status_code == 200
         assert response.json()["is_public"] == False
 
-    @pytest.mark.asyncio
-    @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True)
-    async def test_project_manager_can_update_visibility(
-        self, test_db, session: AsyncSession, client: AsyncClient
-    ):
-        # Setup project with admin and manager
-        admin_user = await create_user(session=session, name="admin", global_role=GlobalRole.USER)
-        manager_user = await create_user(
-            session=session, name="manager", global_role=GlobalRole.USER
-        )
-        project = await create_project(session=session, owner=admin_user, is_public=False)
-        await add_project_member(
-            session=session, project=project, user=admin_user, project_role=ProjectRole.ADMIN
-        )
-        await add_project_member(
-            session=session, project=project, user=manager_user, project_role=ProjectRole.MANAGER
-        )
-
-        # Manager should be able to update visibility
-        response = await client.post(
-            f"/api/projects/{project.name}/update_visibility",
-            headers=get_auth_headers(manager_user.token),
-            json={"is_public": True},
-        )
-        assert response.status_code == 200
-        assert response.json()["is_public"] == True
-
     @pytest.mark.asyncio
     @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True)
     async def test_regular_user_cannot_update_visibility(
@@ -1900,3 +1873,29 @@ async def test_global_admin_can_update_any_project_visibility(
         )
         assert response.status_code == 200
         assert response.json()["is_public"] == True
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True)
+    async def test_project_manager_cannot_update_visibility(
+        self, test_db, session: AsyncSession, client: AsyncClient
+    ):
+        # Setup project with admin and manager
+        admin_user = await create_user(session=session, name="admin", global_role=GlobalRole.USER)
+        manager_user = await create_user(
+            session=session, name="manager", global_role=GlobalRole.USER
+        )
+        project = await create_project(session=session, owner=admin_user, is_public=False)
+        await add_project_member(
+            session=session, project=project, user=admin_user, project_role=ProjectRole.ADMIN
+        )
+        await add_project_member(
+            session=session, project=project, user=manager_user, project_role=ProjectRole.MANAGER
+        )
+
+        # Manager should not be able to update visibility
+        response = await client.post(
+            f"/api/projects/{project.name}/update_visibility",
+            headers=get_auth_headers(manager_user.token),
+            json={"is_public": True},
+        )
+        assert response.status_code == 403