From 4dc50b9f2224dc4d7af9891a755795afa1ed6b06 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 14:29:16 -0400 Subject: [PATCH 01/15] fix: retrack PR2 based on PR1 --- .../_internal/server/routers/projects.py | 43 ++ .../_internal/server/schemas/projects.py | 10 + .../_internal/server/security/permissions.py | 91 ++- .../_internal/server/services/projects.py | 188 ++++- src/dstack/api/server/_projects.py | 26 + .../_internal/server/routers/test_projects.py | 675 +++++++++++++++++- 6 files changed, 977 insertions(+), 56 deletions(-) diff --git a/src/dstack/_internal/server/routers/projects.py b/src/dstack/_internal/server/routers/projects.py index 8f5455a2f7..8354d49c35 100644 --- a/src/dstack/_internal/server/routers/projects.py +++ b/src/dstack/_internal/server/routers/projects.py @@ -7,13 +7,18 @@ from dstack._internal.server.db import get_session from dstack._internal.server.models import ProjectModel, UserModel from dstack._internal.server.schemas.projects import ( + AddProjectMemberRequest, CreateProjectRequest, DeleteProjectsRequest, + RemoveProjectMemberRequest, SetProjectMembersRequest, ) from dstack._internal.server.security.permissions import ( Authenticated, ProjectManager, + ProjectManagerOrPublicJoin, + ProjectManagerOrSelfLeave, + ProjectMember, ProjectMemberOrPublicAccess, ) from dstack._internal.server.services import projects @@ -92,3 +97,41 @@ async def set_project_members( ) await session.refresh(project) return projects.project_model_to_project(project) + + +@router.post( + "/{project_name}/add_members", +) +async def add_project_members( + body: AddProjectMemberRequest, + session: AsyncSession = Depends(get_session), + user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManagerOrPublicJoin()), +) -> Project: + user, project = user_project + await projects.add_project_members( + session=session, + user=user, + project=project, + members=body.members, + ) + await session.refresh(project) + return projects.project_model_to_project(project) + + +@router.post( + "/{project_name}/remove_members", +) +async def remove_project_members( + body: RemoveProjectMemberRequest, + session: AsyncSession = Depends(get_session), + user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManagerOrSelfLeave()), +) -> Project: + user, project = user_project + await projects.remove_project_members( + session=session, + user=user, + project=project, + usernames=body.usernames, + ) + await session.refresh(project) + return projects.project_model_to_project(project) diff --git a/src/dstack/_internal/server/schemas/projects.py b/src/dstack/_internal/server/schemas/projects.py index 3e5b99b772..ed5dc159b2 100644 --- a/src/dstack/_internal/server/schemas/projects.py +++ b/src/dstack/_internal/server/schemas/projects.py @@ -25,3 +25,13 @@ class MemberSetting(CoreModel): class SetProjectMembersRequest(CoreModel): members: List[MemberSetting] + + +class AddProjectMemberRequest(CoreModel): + # Always accept a list of members for cleaner API design + members: List[MemberSetting] + + +class RemoveProjectMemberRequest(CoreModel): + # Always accept a list of usernames for cleaner API design + usernames: List[str] diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 79080aa7bd..d5384ecc00 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -67,27 +67,6 @@ async def __call__( raise error_forbidden() -class ProjectManager: - async def __call__( - self, - project_name: str, - session: AsyncSession = Depends(get_session), - token: HTTPAuthorizationCredentials = Security(HTTPBearer()), - ) -> Tuple[UserModel, ProjectModel]: - 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_forbidden() - if user.global_role == GlobalRole.ADMIN: - return user, project - project_role = get_user_project_role(user=user, project=project) - if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: - return user, project - raise error_forbidden() - - class ProjectMember: async def __call__( self, @@ -135,6 +114,76 @@ async def __call__( raise error_forbidden() +class ProjectManagerOrPublicJoin: + """ + Allows: + 1. Project managers to add any members + 2. Any authenticated user to join public projects themselves + """ + async def __call__( + self, + project_name: str, + session: AsyncSession = Depends(get_session), + token: HTTPAuthorizationCredentials = Security(HTTPBearer()), + ) -> Tuple[UserModel, ProjectModel]: + 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 + + # 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 + + raise error_forbidden() + + +class ProjectManagerOrSelfLeave: + """ + Allows: + 1. Project managers to remove any members + 2. Any project member to leave (remove themselves) + """ + async def __call__( + self, + project_name: str, + session: AsyncSession = Depends(get_session), + token: HTTPAuthorizationCredentials = Security(HTTPBearer()), + ) -> Tuple[UserModel, ProjectModel]: + 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 + + # Project managers can remove members + project_role = get_user_project_role(user=user, project=project) + if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + return user, project + + # Any project member can leave (will be validated in service layer) + if project_role is not None: + return user, project + + raise error_forbidden() + + class OptionalServiceAccount: def __init__(self, token: Optional[str]) -> None: self._token = token diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 48fa3614eb..53d89367c0 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -1,6 +1,6 @@ import uuid from datetime import timezone -from typing import Awaitable, Callable, List, Optional, Tuple +from typing import Awaitable, Callable, List, Optional, Tuple, Union from sqlalchemy import delete, select, update from sqlalchemy import func as safunc @@ -74,8 +74,8 @@ async def list_user_accessible_projects( ) -> List[Project]: """ Returns all projects accessible to the user: - - For global admins: ALL projects in the system - - For regular users: Projects where user is a member + public projects where user is NOT a member + - Projects where user is a member (public or private) + - Public projects where user is NOT a member """ if user.global_role == GlobalRole.ADMIN: projects = await list_project_models(session=session) @@ -230,6 +230,99 @@ async def set_project_members( await session.commit() +async def add_project_members( + session: AsyncSession, + user: UserModel, + project: ProjectModel, + members: List[MemberSetting], +): + """Add multiple members to a project.""" + # reload with members + project = await get_project_model_by_name_or_error( + session=session, + project_name=project.name, + ) + requesting_user_role = get_user_project_role(user=user, project=project) + + # Check if this is a self-join to public project + is_self_join_to_public = ( + len(members) == 1 and + project.is_public and + (members[0].username == user.name or members[0].username == user.email) and + requesting_user_role is None # User is not already a member + ) + + # Check permissions: only managers/admins can add members, EXCEPT for self-join to public projects + if not is_self_join_to_public: + if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + raise ForbiddenError("Access denied: insufficient permissions to add members") + + # For project managers, check if they're trying to add admins + if user.global_role != GlobalRole.ADMIN and requesting_user_role == ProjectRole.MANAGER: + for member in members: + if member.project_role == ProjectRole.ADMIN: + raise ForbiddenError("Access denied: only global admins can add project admins") + else: + # For self-join to public project, only allow USER role + if members[0].project_role != ProjectRole.USER: + raise ForbiddenError("Access denied: can only join public projects as user role") + + # Collect all usernames to query + usernames = [member.username for member in members] + + # Find all users (by username or email) + res = await session.execute( + select(UserModel).where( + (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)) + ) + ) + users_found = res.scalars().all() + + # Create lookup maps for both username and email + username_to_user = {user.name: user for user in users_found} + email_to_user = {user.email: user for user in users_found if user.email} + + # Build a set of current member IDs so we can quickly check if users are already members + member_ids = {m.user_id for m in project.members} + # Build a map from user_id to member for efficient existing member updates + member_by_user_id = {m.user_id: m for m in project.members} + + # Get current max member_num + max_member_num = 0 + for member in project.members: + if member.member_num is not None and member.member_num > max_member_num: + max_member_num = member.member_num + + # Process each member to add + for member_setting in members: + user_to_add = username_to_user.get(member_setting.username) or email_to_user.get(member_setting.username) + if user_to_add is None: + # Error on adding non-existing users instead of silently skipping + raise ServerClientError(f"User not found: {member_setting.username}") + + # Check if user is already a member using our fast lookup set + if user_to_add.id in member_ids: + # Update existing member role if different using our efficient lookup + existing_member = member_by_user_id[user_to_add.id] + if existing_member.project_role != member_setting.project_role: + existing_member.project_role = member_setting.project_role + else: + # Add new member + max_member_num += 1 + await add_project_member( + session=session, + project=project, + user=user_to_add, + project_role=member_setting.project_role, + member_num=max_member_num, + commit=False, + ) + # Keep track of newly added members for subsequent iterations + member_ids.add(user_to_add.id) + + await session.commit() + + async def add_project_member( session: AsyncSession, project: ProjectModel, @@ -502,3 +595,92 @@ def _is_project_admin( if m.project_role == ProjectRole.ADMIN: return True return False + + +async def remove_project_members( + session: AsyncSession, + user: UserModel, + project: ProjectModel, + usernames: List[str], +): + """Remove multiple members from a project.""" + # reload with members + project = await get_project_model_by_name_or_error( + session=session, + project_name=project.name, + ) + requesting_user_role = get_user_project_role(user=user, project=project) + + # Check if this is a self-leave (user removing themselves) + is_self_leave = ( + len(usernames) == 1 and + (usernames[0] == user.name or usernames[0] == user.email) and + requesting_user_role is not None # User is actually a member + ) + + # Check basic permissions: only managers/admins can remove members, EXCEPT for self-leave + if not is_self_leave: + if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + raise ForbiddenError("Access denied: insufficient permissions to remove members") + + # Find all users to remove (by username or email) + res = await session.execute( + select(UserModel).where( + (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)) + ) + ) + users_found = res.scalars().all() + + # Create lookup maps + username_to_user = {user.name: user for user in users_found} + email_to_user = {user.email: user for user in users_found if user.email} + + # Build a set of member IDs for faster membership checks + member_user_ids = {m.user_id for m in project.members} + # Build a map from user_id to member for efficient member lookups + member_by_user_id = {m.user_id: m for m in project.members} + + # Find members to remove and validate permissions + members_to_remove = [] + admin_removals = 0 + + for username in usernames: + user_to_remove = username_to_user.get(username) or email_to_user.get(username) + if user_to_remove is None: + # Error on removing non-existing users instead of silently skipping + raise ServerClientError(f"User not found: {username}") + + # Check if user is actually a member before trying to remove them + if user_to_remove.id not in member_user_ids: + # Error on removing non-members instead of silently skipping + raise ServerClientError(f"User is not a member of this project: {username}") + + # Get the member to remove using our efficient lookup + member_to_remove = member_by_user_id[user_to_remove.id] + + # Check if trying to remove project admin + if member_to_remove.project_role == ProjectRole.ADMIN: + if is_self_leave: + # For self-leave, check if user is the last admin + total_admins = sum(1 for member in project.members if member.project_role == ProjectRole.ADMIN) + if total_admins <= 1: + raise ServerClientError("Cannot leave project: you are the last admin") + else: + # For manager/admin removing other admins, only global admins can do this + if user.global_role != GlobalRole.ADMIN: + raise ForbiddenError(f"Access denied: only global admins can remove project admins (user: {username})") + admin_removals += 1 + + members_to_remove.append(member_to_remove) + + # Check we're not removing all admins (for non-self-leave operations) + if not is_self_leave: + total_admins = sum(1 for member in project.members if member.project_role == ProjectRole.ADMIN) + if admin_removals >= total_admins: + raise ServerClientError("Cannot remove all project admins") + + # Remove all members + for member in members_to_remove: + await session.delete(member) + + await session.commit() diff --git a/src/dstack/api/server/_projects.py b/src/dstack/api/server/_projects.py index da6eccc4f4..47a0c37d70 100644 --- a/src/dstack/api/server/_projects.py +++ b/src/dstack/api/server/_projects.py @@ -3,10 +3,13 @@ from pydantic import parse_obj_as from dstack._internal.core.models.projects import Project +from dstack._internal.core.models.users import ProjectRole from dstack._internal.server.schemas.projects import ( + AddProjectMemberRequest, CreateProjectRequest, DeleteProjectsRequest, MemberSetting, + RemoveProjectMemberRequest, SetProjectMembersRequest, ) from dstack.api.server._group import APIClientGroup @@ -34,3 +37,26 @@ def set_members(self, project_name: str, members: List[MemberSetting]) -> Projec body = SetProjectMembersRequest(members=members) resp = self._request(f"/api/projects/{project_name}/set_members", body=body.json()) return parse_obj_as(Project.__response__, resp.json()) + + def add_member(self, project_name: str, username: str, project_role: ProjectRole) -> Project: + # Convert single user to list format + member_setting = MemberSetting(username=username, project_role=project_role) + body = AddProjectMemberRequest(members=[member_setting]) + resp = self._request(f"/api/projects/{project_name}/add_members", body=body.json()) + return parse_obj_as(Project.__response__, resp.json()) + + def add_members(self, project_name: str, members: List[MemberSetting]) -> Project: + body = AddProjectMemberRequest(members=members) + resp = self._request(f"/api/projects/{project_name}/add_members", body=body.json()) + return parse_obj_as(Project.__response__, resp.json()) + + def remove_member(self, project_name: str, username: str) -> Project: + # Convert single username to list format + body = RemoveProjectMemberRequest(usernames=[username]) + resp = self._request(f"/api/projects/{project_name}/remove_members", body=body.json()) + return parse_obj_as(Project.__response__, resp.json()) + + def remove_members(self, project_name: str, usernames: List[str]) -> Project: + body = RemoveProjectMemberRequest(usernames=usernames) + resp = self._request(f"/api/projects/{project_name}/remove_members", body=body.json()) + return parse_obj_as(Project.__response__, resp.json()) diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 620c2d1eec..81ccd844ba 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -75,10 +75,10 @@ async def test_returns_projects(self, test_db, session: AsyncSession, client: As "created_at": "2023-01-02T03:04:00+00:00", "backends": [], "members": [], - "is_public": False, } ] +<<<<<<< HEAD @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) async def test_returns_public_projects_to_non_members( @@ -201,6 +201,8 @@ async def test_member_sees_both_public_and_private_projects( assert "public_project" in project_names assert "private_project" in project_names +======= +>>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) class TestCreateProject: @pytest.mark.asyncio @@ -336,18 +338,16 @@ async def test_no_project_quota_for_global_admins( async def test_forbids_if_no_permission_to_create_projects( self, test_db, session: AsyncSession, client: AsyncClient ): - user = await create_user(session=session, name="owner", global_role=GlobalRole.USER) + user = await create_user(session=session, global_role=GlobalRole.USER) with default_permissions_context( - DefaultPermissions( - allow_non_admins_create_projects=False, - allow_non_admins_manage_ssh_fleets=True, - ) + DefaultPermissions(allow_non_admins_create_projects=False) ): response = await client.post( "/api/projects/create", headers=get_auth_headers(user.token), - json={"project_name": "test_project"}, + json={"project_name": "new_project"}, ) +<<<<<<< HEAD assert response.status_code == 403 @pytest.mark.asyncio @@ -436,6 +436,9 @@ async def test_creates_private_project_explicitly( res = await session.execute(select(ProjectModel).where(ProjectModel.name == project_name)) project = res.scalar_one() assert project.is_public is False +======= + assert response.status_code in [401, 403] +>>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) class TestDeleteProject: @@ -602,7 +605,6 @@ async def test_returns_project(self, test_db, session: AsyncSession, client: Asy }, } ], - "is_public": False, } @pytest.mark.asyncio @@ -994,37 +996,48 @@ async def test_global_admin_manager_can_set_project_admins( @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_non_manager_cannot_set_project_members( - self, test_db, session: AsyncSession, client: AsyncClient - ): - project = await create_project(session=session) - user = await create_user(session=session, global_role=GlobalRole.USER) - await add_project_member( - session=session, - project=project, - user=user, - project_role=ProjectRole.USER, + async def test_add_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Try to add non-existent user - should now error instead of silently skipping + body = {"members": [{"username": "nonexistent", "project_role": "user"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, ) - user1 = await create_user(session=session, name="user1") - members = [ - { - "username": user.name, - "project_role": ProjectRole.ADMIN, - }, - { - "username": user1.name, - "project_role": ProjectRole.ADMIN, - }, - ] - body = {"members": members} + + # Operation should fail with 400 error for non-existent user + assert response.status_code == 400 + response_json = response.json() + assert "User not found: nonexistent" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_manager_cannot_add_admin_without_global_admin(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with manager (not global admin) + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) + + # Create user to add + new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # Try to add admin + body = {"members": [{"username": "newuser", "project_role": "admin"}]} response = await client.post( - f"/api/projects/{project.name}/set_members", - headers=get_auth_headers(user.token), + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(manager.token), json=body, ) + assert response.status_code == 403 +<<<<<<< HEAD class TestListUserProjectsService: """Test the service-level functions for backward compatibility""" @@ -1134,3 +1147,601 @@ async def test_list_user_accessible_projects_returns_member_and_public_projects( project_names = [p.project_name for p in projects] assert "public_project" in project_names assert "private_project" in project_names +======= +class TestMemberManagement: + """Test class for add_member and remove_member endpoints""" + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + @pytest.mark.parametrize("endpoint", ["add_members", "remove_members"]) + async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClient, endpoint: str): + response = await client.post(f"/api/projects/test-project/{endpoint}") + assert response.status_code in [401, 403] + + # Add Member Tests + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_by_username(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user to add + new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # Add member + body = {"members": [{"username": "newuser", "project_role": "user"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that new user is in the members list + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "newuser" in member_usernames + + # Find the new member and check their role + new_member = next(m for m in response_data["members"] if m["user"]["username"] == "newuser") + assert new_member["project_role"] == "user" + + # Verify in database + res = await session.execute(select(MemberModel).where(MemberModel.user_id == new_user.id)) + member = res.scalar_one() + assert member.project_role == ProjectRole.USER + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_by_email(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user to add + new_user = await create_user( + session=session, + name="emailuser", + email="test@example.com", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + + # Add member by email + body = {"members": [{"username": "test@example.com", "project_role": "manager"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that new user is in the members list + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "emailuser" in member_usernames + + # Find the new member and check their role + new_member = next(m for m in response_data["members"] if m["user"]["username"] == "emailuser") + assert new_member["project_role"] == "manager" + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_updates_existing_role(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user and add as USER first + existing_user = await create_user(session=session, name="existing", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=existing_user, project_role=ProjectRole.USER) + + # Update to MANAGER + body = {"members": [{"username": "existing", "project_role": "manager"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Find the updated member and check their role + updated_member = next(m for m in response_data["members"] if m["user"]["username"] == "existing") + assert updated_member["project_role"] == "manager" + + # Verify in database + res = await session.execute(select(MemberModel).where(MemberModel.user_id == existing_user.id)) + member = res.scalar_one() + assert member.project_role == ProjectRole.MANAGER + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Try to add non-existent user - should now error instead of silently skipping + body = {"members": [{"username": "nonexistent", "project_role": "user"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + # Operation should fail with 400 error for non-existent user + assert response.status_code == 400 + response_json = response.json() + assert "User not found: nonexistent" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_member_manager_cannot_add_admin_without_global_admin(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with manager (not global admin) + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) + + # Create user to add + new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # Try to add admin + body = {"members": [{"username": "newuser", "project_role": "admin"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(manager.token), + json=body, + ) + + assert response.status_code == 403 + + # Remove Member Tests + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_by_username(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user to remove + user_to_remove = await create_user(session=session, name="removeuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) + + # Remove member + body = {"usernames": ["removeuser"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that user is NOT in the members list anymore + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "removeuser" not in member_usernames + + # Verify removed from database + res = await session.execute(select(MemberModel).where(MemberModel.user_id == user_to_remove.id)) + member = res.scalar_one_or_none() + assert member is None + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_by_email(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user to remove + user_to_remove = await create_user( + session=session, + name="emailremove", + email="remove@example.com", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) + + # Remove member by email + body = {"usernames": ["remove@example.com"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Try to remove non-existent user - should now error instead of silently skipping + body = {"usernames": ["nonexistent"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + # Operation should fail with 400 error for non-existent user + assert response.status_code == 400 + response_json = response.json() + assert "User not found: nonexistent" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_errors_on_non_members(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user who is NOT a member + non_member = await create_user(session=session, name="nonmember", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # Try to remove non-member - should now error instead of silently skipping + body = {"usernames": ["nonmember"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + # Operation should fail with 400 error for non-member + assert response.status_code == 400 + response_json = response.json() + assert "User is not a member of this project: nonmember" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_prevents_removing_last_admin(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with only one admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Try to remove the only admin + body = {"usernames": [admin.name]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 400 + response_json = response.json() + # Check for the new self-leave error message (since admin is removing themselves) + assert "Cannot leave project: you are the last admin" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_member_requires_manager_or_admin_permission(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with regular user + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user(session=session, name="regular", global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + other_user = await create_user(session=session, name="other", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) + await add_project_member(session=session, project=project, user=other_user, project_role=ProjectRole.USER) + + # Try to remove as regular user + body = {"usernames": ["other"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 403 + + # Batch Operations Tests + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_add_multiple_members_batch(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create multiple users to add + user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # Add multiple members at once + body = { + "members": [ + {"username": "user1", "project_role": "user"}, + {"username": "user2", "project_role": "manager"}, + {"username": "user3", "project_role": "user"} + ] + } + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that all new users are in the members list + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "user1" in member_usernames + assert "user2" in member_usernames + assert "user3" in member_usernames + + # Check roles + user1_member = next(m for m in response_data["members"] if m["user"]["username"] == "user1") + assert user1_member["project_role"] == "user" + + user2_member = next(m for m in response_data["members"] if m["user"]["username"] == "user2") + assert user2_member["project_role"] == "manager" + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_remove_multiple_members_batch(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project and admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create multiple users to remove + user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) + await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) + await add_project_member(session=session, project=project, user=user3, project_role=ProjectRole.USER) + + # Remove multiple members at once + body = {"usernames": ["user1", "user2", "user3"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that all users are NOT in the members list anymore + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "user1" not in member_usernames + assert "user2" not in member_usernames + assert "user3" not in member_usernames + + # Verify removed from database + res = await session.execute(select(MemberModel).where(MemberModel.user_id == user1.id)) + assert res.scalar_one_or_none() is None + + res = await session.execute(select(MemberModel).where(MemberModel.user_id == user2.id)) + assert res.scalar_one_or_none() is None + + # Join/Leave Functionality Tests + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_can_join_public_project(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup public project with admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Make project public + project.is_public = True + await session.commit() + + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user who wants to join + regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # User joins public project (should work) + body = {"members": [{"username": "joiner", "project_role": "user"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that user is now in the members list + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "joiner" in member_usernames + + # Find the new member and check their role is USER + new_member = next(m for m in response_data["members"] if m["user"]["username"] == "joiner") + assert new_member["project_role"] == "user" + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_cannot_join_private_project(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup private project with admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Project is private by default (is_public=False) + + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user who wants to join + regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # User tries to join private project (should fail) + body = {"members": [{"username": "joiner", "project_role": "user"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 403 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_cannot_join_public_project_as_admin(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup public project with admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project.is_public = True + await session.commit() + + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Create user who wants to join as admin + regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + # User tries to join public project as admin (should fail) + body = {"members": [{"username": "joiner", "project_role": "admin"}]} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 403 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_can_leave_project(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with admin and regular member + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user(session=session, name="leaver", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) + + # User leaves project (should work) + body = {"usernames": ["leaver"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that user is NOT in the members list anymore + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "leaver" not in member_usernames + + # Should only have admin left + assert len(response_data["members"]) == 1 + assert response_data["members"][0]["user"]["username"] == "admin" + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_can_leave_project_by_email(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with admin and regular member + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user( + session=session, + name="leaver", + email="leaver@example.com", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) + + # User leaves project by email (should work) + body = {"usernames": ["leaver@example.com"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(regular_user.token), + json=body, + ) + + assert response.status_code == 200 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_last_admin_cannot_leave_project(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with only one admin + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + + # Admin tries to leave (should fail as they're the last admin) + body = {"usernames": ["admin"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin.token), + json=body, + ) + + assert response.status_code == 400 + response_json = response.json() + assert "Cannot leave project: you are the last admin" in str(response_json) + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_admin_can_leave_if_other_admins_exist(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with two admins + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin1 = await create_user(session=session, name="admin1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin2 = await create_user(session=session, name="admin2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + await add_project_member(session=session, project=project, user=admin1, project_role=ProjectRole.ADMIN) + await add_project_member(session=session, project=project, user=admin2, project_role=ProjectRole.ADMIN) + + # One admin leaves (should work as there's another admin) + body = {"usernames": ["admin1"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(admin1.token), + json=body, + ) + + assert response.status_code == 200 + response_data = response.json() + + # Check that admin1 is NOT in the members list anymore + member_usernames = [member["user"]["username"] for member in response_data["members"]] + assert "admin1" not in member_usernames + assert "admin2" in member_usernames + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_user_cannot_leave_others_from_project(self, test_db, session: AsyncSession, client: AsyncClient): + # Setup project with admin and two regular users + project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) + await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) + + # user1 tries to remove user2 (should fail) + body = {"usernames": ["user2"]} + response = await client.post( + f"/api/projects/{project.name}/remove_members", + headers=get_auth_headers(user1.token), + json=body, + ) + + assert response.status_code == 403 +>>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) From 95e25fc54821eb8d31298c4377a454d857698912 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 15:50:33 -0400 Subject: [PATCH 02/15] fix: lint errors in test_projects.py --- .../_internal/server/routers/test_projects.py | 180 +++++++++--------- 1 file changed, 90 insertions(+), 90 deletions(-) diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 81ccd844ba..dcf22f60b5 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -1001,7 +1001,7 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Try to add non-existent user - should now error instead of silently skipping body = {"members": [{"username": "nonexistent", "project_role": "user"}]} response = await client.post( @@ -1009,7 +1009,7 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy headers=get_auth_headers(admin.token), json=body, ) - + # Operation should fail with 400 error for non-existent user assert response.status_code == 400 response_json = response.json() @@ -1022,10 +1022,10 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin(self, te project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) - + # Create user to add - new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Try to add admin body = {"members": [{"username": "newuser", "project_role": "admin"}]} response = await client.post( @@ -1033,7 +1033,7 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin(self, te headers=get_auth_headers(manager.token), json=body, ) - + assert response.status_code == 403 @@ -1168,7 +1168,7 @@ async def test_add_member_by_username(self, test_db, session: AsyncSession, clie await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) # Create user to add - new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) # Add member body = {"members": [{"username": "newuser", "project_role": "user"}]} @@ -1177,20 +1177,20 @@ async def test_add_member_by_username(self, test_db, session: AsyncSession, clie headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that new user is in the members list member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "newuser" in member_usernames - + # Find the new member and check their role new_member = next(m for m in response_data["members"] if m["user"]["username"] == "newuser") assert new_member["project_role"] == "user" - + # Verify in database - res = await session.execute(select(MemberModel).where(MemberModel.user_id == new_user.id)) + res = await session.execute(select(MemberModel).where(MemberModel.user_id == _new_user.id)) member = res.scalar_one() assert member.project_role == ProjectRole.USER @@ -1203,9 +1203,9 @@ async def test_add_member_by_email(self, test_db, session: AsyncSession, client: await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) # Create user to add - new_user = await create_user( + _new_user = await create_user( session=session, - name="emailuser", + name="emailuser", email="test@example.com", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) ) @@ -1220,11 +1220,11 @@ async def test_add_member_by_email(self, test_db, session: AsyncSession, client: assert response.status_code == 200 response_data = response.json() - + # Check that new user is in the members list member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "emailuser" in member_usernames - + # Find the new member and check their role new_member = next(m for m in response_data["members"] if m["user"]["username"] == "emailuser") assert new_member["project_role"] == "manager" @@ -1236,11 +1236,11 @@ async def test_add_member_updates_existing_role(self, test_db, session: AsyncSes project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user and add as USER first existing_user = await create_user(session=session, name="existing", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=existing_user, project_role=ProjectRole.USER) - + # Update to MANAGER body = {"members": [{"username": "existing", "project_role": "manager"}]} response = await client.post( @@ -1248,10 +1248,10 @@ async def test_add_member_updates_existing_role(self, test_db, session: AsyncSes headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Find the updated member and check their role updated_member = next(m for m in response_data["members"] if m["user"]["username"] == "existing") assert updated_member["project_role"] == "manager" @@ -1268,7 +1268,7 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Try to add non-existent user - should now error instead of silently skipping body = {"members": [{"username": "nonexistent", "project_role": "user"}]} response = await client.post( @@ -1276,7 +1276,7 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy headers=get_auth_headers(admin.token), json=body, ) - + # Operation should fail with 400 error for non-existent user assert response.status_code == 400 response_json = response.json() @@ -1289,10 +1289,10 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin(self, te project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) - + # Create user to add - new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Try to add admin body = {"members": [{"username": "newuser", "project_role": "admin"}]} response = await client.post( @@ -1300,7 +1300,7 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin(self, te headers=get_auth_headers(manager.token), json=body, ) - + assert response.status_code == 403 # Remove Member Tests @@ -1311,7 +1311,7 @@ async def test_remove_member_by_username(self, test_db, session: AsyncSession, c project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user to remove user_to_remove = await create_user(session=session, name="removeuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) @@ -1323,14 +1323,14 @@ async def test_remove_member_by_username(self, test_db, session: AsyncSession, c headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() # Check that user is NOT in the members list anymore member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "removeuser" not in member_usernames - + # Verify removed from database res = await session.execute(select(MemberModel).where(MemberModel.user_id == user_to_remove.id)) member = res.scalar_one_or_none() @@ -1343,16 +1343,16 @@ async def test_remove_member_by_email(self, test_db, session: AsyncSession, clie project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user to remove user_to_remove = await create_user( session=session, - name="emailremove", + name="emailremove", email="remove@example.com", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) ) await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) - + # Remove member by email body = {"usernames": ["remove@example.com"]} response = await client.post( @@ -1360,7 +1360,7 @@ async def test_remove_member_by_email(self, test_db, session: AsyncSession, clie headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 @pytest.mark.asyncio @@ -1370,7 +1370,7 @@ async def test_remove_member_errors_on_nonexistent_user(self, test_db, session: project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Try to remove non-existent user - should now error instead of silently skipping body = {"usernames": ["nonexistent"]} response = await client.post( @@ -1378,7 +1378,7 @@ async def test_remove_member_errors_on_nonexistent_user(self, test_db, session: headers=get_auth_headers(admin.token), json=body, ) - + # Operation should fail with 400 error for non-existent user assert response.status_code == 400 response_json = response.json() @@ -1393,8 +1393,8 @@ async def test_remove_member_errors_on_non_members(self, test_db, session: Async await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) # Create user who is NOT a member - non_member = await create_user(session=session, name="nonmember", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + _non_member = await create_user(session=session, name="nonmember", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Try to remove non-member - should now error instead of silently skipping body = {"usernames": ["nonmember"]} response = await client.post( @@ -1402,7 +1402,7 @@ async def test_remove_member_errors_on_non_members(self, test_db, session: Async headers=get_auth_headers(admin.token), json=body, ) - + # Operation should fail with 400 error for non-member assert response.status_code == 400 response_json = response.json() @@ -1415,7 +1415,7 @@ async def test_remove_member_prevents_removing_last_admin(self, test_db, session project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Try to remove the only admin body = {"usernames": [admin.name]} response = await client.post( @@ -1423,7 +1423,7 @@ async def test_remove_member_prevents_removing_last_admin(self, test_db, session headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 400 response_json = response.json() # Check for the new self-leave error message (since admin is removing themselves) @@ -1437,7 +1437,7 @@ async def test_remove_member_requires_manager_or_admin_permission(self, test_db, admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) regular_user = await create_user(session=session, name="regular", global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) other_user = await create_user(session=session, name="other", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) await add_project_member(session=session, project=project, user=other_user, project_role=ProjectRole.USER) @@ -1449,7 +1449,7 @@ async def test_remove_member_requires_manager_or_admin_permission(self, test_db, headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 403 # Batch Operations Tests @@ -1460,12 +1460,12 @@ async def test_add_multiple_members_batch(self, test_db, session: AsyncSession, project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create multiple users to add - user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + _user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + # Add multiple members at once body = { "members": [ @@ -1479,20 +1479,20 @@ async def test_add_multiple_members_batch(self, test_db, session: AsyncSession, headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that all new users are in the members list member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "user1" in member_usernames assert "user2" in member_usernames assert "user3" in member_usernames - + # Check roles user1_member = next(m for m in response_data["members"] if m["user"]["username"] == "user1") assert user1_member["project_role"] == "user" - + user2_member = next(m for m in response_data["members"] if m["user"]["username"] == "user2") assert user2_member["project_role"] == "manager" @@ -1508,11 +1508,11 @@ async def test_remove_multiple_members_batch(self, test_db, session: AsyncSessio user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) await add_project_member(session=session, project=project, user=user3, project_role=ProjectRole.USER) - + # Remove multiple members at once body = {"usernames": ["user1", "user2", "user3"]} response = await client.post( @@ -1520,20 +1520,20 @@ async def test_remove_multiple_members_batch(self, test_db, session: AsyncSessio headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that all users are NOT in the members list anymore member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "user1" not in member_usernames assert "user2" not in member_usernames assert "user3" not in member_usernames - + # Verify removed from database res = await session.execute(select(MemberModel).where(MemberModel.user_id == user1.id)) assert res.scalar_one_or_none() is None - + res = await session.execute(select(MemberModel).where(MemberModel.user_id == user2.id)) assert res.scalar_one_or_none() is None @@ -1546,13 +1546,13 @@ async def test_user_can_join_public_project(self, test_db, session: AsyncSession # Make project public project.is_public = True await session.commit() - + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user who wants to join regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + # User joins public project (should work) body = {"members": [{"username": "joiner", "project_role": "user"}]} response = await client.post( @@ -1560,14 +1560,14 @@ async def test_user_can_join_public_project(self, test_db, session: AsyncSession headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that user is now in the members list member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "joiner" in member_usernames - + # Find the new member and check their role is USER new_member = next(m for m in response_data["members"] if m["user"]["username"] == "joiner") assert new_member["project_role"] == "user" @@ -1578,13 +1578,13 @@ async def test_user_cannot_join_private_project(self, test_db, session: AsyncSes # Setup private project with admin project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) # Project is private by default (is_public=False) - + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user who wants to join regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + # User tries to join private project (should fail) body = {"members": [{"username": "joiner", "project_role": "user"}]} response = await client.post( @@ -1592,7 +1592,7 @@ async def test_user_cannot_join_private_project(self, test_db, session: AsyncSes headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 403 @pytest.mark.asyncio @@ -1602,13 +1602,13 @@ async def test_user_cannot_join_public_project_as_admin(self, test_db, session: project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) project.is_public = True await session.commit() - + admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Create user who wants to join as admin regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + # User tries to join public project as admin (should fail) body = {"members": [{"username": "joiner", "project_role": "admin"}]} response = await client.post( @@ -1616,7 +1616,7 @@ async def test_user_cannot_join_public_project_as_admin(self, test_db, session: headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 403 @pytest.mark.asyncio @@ -1626,10 +1626,10 @@ async def test_user_can_leave_project(self, test_db, session: AsyncSession, clie project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) regular_user = await create_user(session=session, name="leaver", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) - + # User leaves project (should work) body = {"usernames": ["leaver"]} response = await client.post( @@ -1637,14 +1637,14 @@ async def test_user_can_leave_project(self, test_db, session: AsyncSession, clie headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that user is NOT in the members list anymore member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "leaver" not in member_usernames - + # Should only have admin left assert len(response_data["members"]) == 1 assert response_data["members"][0]["user"]["username"] == "admin" @@ -1656,15 +1656,15 @@ async def test_user_can_leave_project_by_email(self, test_db, session: AsyncSess project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) regular_user = await create_user( - session=session, - name="leaver", + session=session, + name="leaver", email="leaver@example.com", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) ) - + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) - + # User leaves project by email (should work) body = {"usernames": ["leaver@example.com"]} response = await client.post( @@ -1672,7 +1672,7 @@ async def test_user_can_leave_project_by_email(self, test_db, session: AsyncSess headers=get_auth_headers(regular_user.token), json=body, ) - + assert response.status_code == 200 @pytest.mark.asyncio @@ -1682,7 +1682,7 @@ async def test_last_admin_cannot_leave_project(self, test_db, session: AsyncSess project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - + # Admin tries to leave (should fail as they're the last admin) body = {"usernames": ["admin"]} response = await client.post( @@ -1690,7 +1690,7 @@ async def test_last_admin_cannot_leave_project(self, test_db, session: AsyncSess headers=get_auth_headers(admin.token), json=body, ) - + assert response.status_code == 400 response_json = response.json() assert "Cannot leave project: you are the last admin" in str(response_json) @@ -1702,10 +1702,10 @@ async def test_admin_can_leave_if_other_admins_exist(self, test_db, session: Asy project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin1 = await create_user(session=session, name="admin1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) admin2 = await create_user(session=session, name="admin2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + await add_project_member(session=session, project=project, user=admin1, project_role=ProjectRole.ADMIN) await add_project_member(session=session, project=project, user=admin2, project_role=ProjectRole.ADMIN) - + # One admin leaves (should work as there's another admin) body = {"usernames": ["admin1"]} response = await client.post( @@ -1713,10 +1713,10 @@ async def test_admin_can_leave_if_other_admins_exist(self, test_db, session: Asy headers=get_auth_headers(admin1.token), json=body, ) - + assert response.status_code == 200 response_data = response.json() - + # Check that admin1 is NOT in the members list anymore member_usernames = [member["user"]["username"] for member in response_data["members"]] assert "admin1" not in member_usernames @@ -1730,11 +1730,11 @@ async def test_user_cannot_leave_others_from_project(self, test_db, session: Asy admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - + await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) - + # user1 tries to remove user2 (should fail) body = {"usernames": ["user2"]} response = await client.post( @@ -1742,6 +1742,6 @@ async def test_user_cannot_leave_others_from_project(self, test_db, session: Asy headers=get_auth_headers(user1.token), json=body, ) - + assert response.status_code == 403 >>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) From 1805c9a4c85bbc2b96c6c4b44b7027908a6c0775 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 15:54:27 -0400 Subject: [PATCH 03/15] fix: update test assertions to include is_public field --- src/tests/_internal/server/routers/test_projects.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index dcf22f60b5..30f8be30fe 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -75,6 +75,7 @@ async def test_returns_projects(self, test_db, session: AsyncSession, client: As "created_at": "2023-01-02T03:04:00+00:00", "backends": [], "members": [], + "is_public": False, } ] @@ -605,6 +606,7 @@ async def test_returns_project(self, test_db, session: AsyncSession, client: Asy }, } ], + "is_public": False, } @pytest.mark.asyncio From 4f3c123ce2e8f7f0c73d9aa9166f63bae0d04456 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 17:12:36 -0400 Subject: [PATCH 04/15] style: apply pre-commit formatting fixes --- .../_internal/server/security/permissions.py | 18 +- .../_internal/server/services/projects.py | 60 +- .../_internal/server/routers/test_projects.py | 607 ++++++++++++++---- 3 files changed, 512 insertions(+), 173 deletions(-) diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index d5384ecc00..7342fc336d 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -120,6 +120,7 @@ class ProjectManagerOrPublicJoin: 1. Project managers to add any members 2. Any authenticated user to join public projects themselves """ + async def __call__( self, project_name: str, @@ -132,20 +133,20 @@ async def __call__( 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 - + # 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 - + raise error_forbidden() @@ -155,6 +156,7 @@ class ProjectManagerOrSelfLeave: 1. Project managers to remove any members 2. Any project member to leave (remove themselves) """ + async def __call__( self, project_name: str, @@ -167,20 +169,20 @@ async def __call__( 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 - + # Project managers can remove members project_role = get_user_project_role(user=user, project=project) if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: return user, project - + # Any project member can leave (will be validated in service layer) if project_role is not None: return user, project - + raise error_forbidden() diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 53d89367c0..b4458e43f0 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -1,6 +1,6 @@ import uuid from datetime import timezone -from typing import Awaitable, Callable, List, Optional, Tuple, Union +from typing import Awaitable, Callable, List, Optional, Tuple from sqlalchemy import delete, select, update from sqlalchemy import func as safunc @@ -246,10 +246,10 @@ async def add_project_members( # Check if this is a self-join to public project is_self_join_to_public = ( - len(members) == 1 and - project.is_public and - (members[0].username == user.name or members[0].username == user.email) and - requesting_user_role is None # User is not already a member + len(members) == 1 + and project.is_public + and (members[0].username == user.name or members[0].username == user.email) + and requesting_user_role is None # User is not already a member ) # Check permissions: only managers/admins can add members, EXCEPT for self-join to public projects @@ -261,7 +261,9 @@ async def add_project_members( if user.global_role != GlobalRole.ADMIN and requesting_user_role == ProjectRole.MANAGER: for member in members: if member.project_role == ProjectRole.ADMIN: - raise ForbiddenError("Access denied: only global admins can add project admins") + raise ForbiddenError( + "Access denied: only global admins can add project admins" + ) else: # For self-join to public project, only allow USER role if members[0].project_role != ProjectRole.USER: @@ -269,24 +271,22 @@ async def add_project_members( # Collect all usernames to query usernames = [member.username for member in members] - + # Find all users (by username or email) res = await session.execute( - select(UserModel).where( - (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)) - ) + select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) ) users_found = res.scalars().all() - + # Create lookup maps for both username and email username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - + # Build a set of current member IDs so we can quickly check if users are already members member_ids = {m.user_id for m in project.members} # Build a map from user_id to member for efficient existing member updates member_by_user_id = {m.user_id: m for m in project.members} - + # Get current max member_num max_member_num = 0 for member in project.members: @@ -295,7 +295,9 @@ async def add_project_members( # Process each member to add for member_setting in members: - user_to_add = username_to_user.get(member_setting.username) or email_to_user.get(member_setting.username) + user_to_add = username_to_user.get(member_setting.username) or email_to_user.get( + member_setting.username + ) if user_to_add is None: # Error on adding non-existing users instead of silently skipping raise ServerClientError(f"User not found: {member_setting.username}") @@ -613,9 +615,9 @@ async def remove_project_members( # Check if this is a self-leave (user removing themselves) is_self_leave = ( - len(usernames) == 1 and - (usernames[0] == user.name or usernames[0] == user.email) and - requesting_user_role is not None # User is actually a member + len(usernames) == 1 + and (usernames[0] == user.name or usernames[0] == user.email) + and requesting_user_role is not None # User is actually a member ) # Check basic permissions: only managers/admins can remove members, EXCEPT for self-leave @@ -625,16 +627,14 @@ async def remove_project_members( # Find all users to remove (by username or email) res = await session.execute( - select(UserModel).where( - (UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames)) - ) + select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) ) users_found = res.scalars().all() - + # Create lookup maps username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - + # Build a set of member IDs for faster membership checks member_user_ids = {m.user_id for m in project.members} # Build a map from user_id to member for efficient member lookups @@ -643,7 +643,7 @@ async def remove_project_members( # Find members to remove and validate permissions members_to_remove = [] admin_removals = 0 - + for username in usernames: user_to_remove = username_to_user.get(username) or email_to_user.get(username) if user_to_remove is None: @@ -662,25 +662,31 @@ async def remove_project_members( if member_to_remove.project_role == ProjectRole.ADMIN: if is_self_leave: # For self-leave, check if user is the last admin - total_admins = sum(1 for member in project.members if member.project_role == ProjectRole.ADMIN) + total_admins = sum( + 1 for member in project.members if member.project_role == ProjectRole.ADMIN + ) if total_admins <= 1: raise ServerClientError("Cannot leave project: you are the last admin") else: # For manager/admin removing other admins, only global admins can do this if user.global_role != GlobalRole.ADMIN: - raise ForbiddenError(f"Access denied: only global admins can remove project admins (user: {username})") + raise ForbiddenError( + f"Access denied: only global admins can remove project admins (user: {username})" + ) admin_removals += 1 members_to_remove.append(member_to_remove) # Check we're not removing all admins (for non-self-leave operations) if not is_self_leave: - total_admins = sum(1 for member in project.members if member.project_role == ProjectRole.ADMIN) + total_admins = sum( + 1 for member in project.members if member.project_role == ProjectRole.ADMIN + ) if admin_removals >= total_admins: raise ServerClientError("Cannot remove all project admins") # Remove all members for member in members_to_remove: await session.delete(member) - + await session.commit() diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 30f8be30fe..c9e573dbdd 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -998,11 +998,19 @@ async def test_global_admin_manager_can_set_project_admins( @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_errors_on_nonexistent_user( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Try to add non-existent user - should now error instead of silently skipping body = {"members": [{"username": "nonexistent", "project_role": "user"}]} @@ -1019,14 +1027,28 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_manager_cannot_add_admin_without_global_admin(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_manager_cannot_add_admin_without_global_admin( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with manager (not global admin) - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + manager = await create_user( + session=session, + global_role=GlobalRole.USER, + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=manager, project_role=ProjectRole.MANAGER + ) # Create user to add - _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _new_user = await create_user( + session=session, + name="newuser", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # Try to add admin body = {"members": [{"username": "newuser", "project_role": "admin"}]} @@ -1156,21 +1178,35 @@ class TestMemberManagement: @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) @pytest.mark.parametrize("endpoint", ["add_members", "remove_members"]) - async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClient, endpoint: str): + async def test_returns_40x_if_not_authenticated( + self, test_db, client: AsyncClient, endpoint: str + ): response = await client.post(f"/api/projects/test-project/{endpoint}") assert response.status_code in [401, 403] # Add Member Tests @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_by_username(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_by_username( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user to add - _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _new_user = await create_user( + session=session, + name="newuser", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # Add member body = {"members": [{"username": "newuser", "project_role": "user"}]} @@ -1188,7 +1224,9 @@ async def test_add_member_by_username(self, test_db, session: AsyncSession, clie assert "newuser" in member_usernames # Find the new member and check their role - new_member = next(m for m in response_data["members"] if m["user"]["username"] == "newuser") + new_member = next( + m for m in response_data["members"] if m["user"]["username"] == "newuser" + ) assert new_member["project_role"] == "user" # Verify in database @@ -1200,16 +1238,22 @@ async def test_add_member_by_username(self, test_db, session: AsyncSession, clie @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) async def test_add_member_by_email(self, test_db, session: AsyncSession, client: AsyncClient): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user to add _new_user = await create_user( session=session, name="emailuser", email="test@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), ) # Add member by email @@ -1228,20 +1272,36 @@ async def test_add_member_by_email(self, test_db, session: AsyncSession, client: assert "emailuser" in member_usernames # Find the new member and check their role - new_member = next(m for m in response_data["members"] if m["user"]["username"] == "emailuser") + new_member = next( + m for m in response_data["members"] if m["user"]["username"] == "emailuser" + ) assert new_member["project_role"] == "manager" @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_updates_existing_role(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_updates_existing_role( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user and add as USER first - existing_user = await create_user(session=session, name="existing", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=existing_user, project_role=ProjectRole.USER) + existing_user = await create_user( + session=session, + name="existing", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=existing_user, project_role=ProjectRole.USER + ) # Update to MANAGER body = {"members": [{"username": "existing", "project_role": "manager"}]} @@ -1255,21 +1315,33 @@ async def test_add_member_updates_existing_role(self, test_db, session: AsyncSes response_data = response.json() # Find the updated member and check their role - updated_member = next(m for m in response_data["members"] if m["user"]["username"] == "existing") + updated_member = next( + m for m in response_data["members"] if m["user"]["username"] == "existing" + ) assert updated_member["project_role"] == "manager" # Verify in database - res = await session.execute(select(MemberModel).where(MemberModel.user_id == existing_user.id)) + res = await session.execute( + select(MemberModel).where(MemberModel.user_id == existing_user.id) + ) member = res.scalar_one() assert member.project_role == ProjectRole.MANAGER @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_errors_on_nonexistent_user( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Try to add non-existent user - should now error instead of silently skipping body = {"members": [{"username": "nonexistent", "project_role": "user"}]} @@ -1286,14 +1358,28 @@ async def test_add_member_errors_on_nonexistent_user(self, test_db, session: Asy @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_manager_cannot_add_admin_without_global_admin(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_member_manager_cannot_add_admin_without_global_admin( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with manager (not global admin) - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - manager = await create_user(session=session, global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=manager, project_role=ProjectRole.MANAGER) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + manager = await create_user( + session=session, + global_role=GlobalRole.USER, + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=manager, project_role=ProjectRole.MANAGER + ) # Create user to add - _new_user = await create_user(session=session, name="newuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _new_user = await create_user( + session=session, + name="newuser", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # Try to add admin body = {"members": [{"username": "newuser", "project_role": "admin"}]} @@ -1308,15 +1394,29 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin(self, te # Remove Member Tests @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_by_username(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_by_username( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user to remove - user_to_remove = await create_user(session=session, name="removeuser", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) + user_to_remove = await create_user( + session=session, + name="removeuser", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER + ) # Remove member body = {"usernames": ["removeuser"]} @@ -1334,26 +1434,38 @@ async def test_remove_member_by_username(self, test_db, session: AsyncSession, c assert "removeuser" not in member_usernames # Verify removed from database - res = await session.execute(select(MemberModel).where(MemberModel.user_id == user_to_remove.id)) + res = await session.execute( + select(MemberModel).where(MemberModel.user_id == user_to_remove.id) + ) member = res.scalar_one_or_none() assert member is None @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_by_email(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_by_email( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user to remove user_to_remove = await create_user( session=session, name="emailremove", email="remove@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER ) - await add_project_member(session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER) # Remove member by email body = {"usernames": ["remove@example.com"]} @@ -1367,11 +1479,19 @@ async def test_remove_member_by_email(self, test_db, session: AsyncSession, clie @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_errors_on_nonexistent_user(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_errors_on_nonexistent_user( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Try to remove non-existent user - should now error instead of silently skipping body = {"usernames": ["nonexistent"]} @@ -1388,14 +1508,26 @@ async def test_remove_member_errors_on_nonexistent_user(self, test_db, session: @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_errors_on_non_members(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_errors_on_non_members( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user who is NOT a member - _non_member = await create_user(session=session, name="nonmember", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _non_member = await create_user( + session=session, + name="nonmember", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # Try to remove non-member - should now error instead of silently skipping body = {"usernames": ["nonmember"]} @@ -1412,11 +1544,19 @@ async def test_remove_member_errors_on_non_members(self, test_db, session: Async @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_prevents_removing_last_admin(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_prevents_removing_last_admin( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with only one admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Try to remove the only admin body = {"usernames": [admin.name]} @@ -1433,16 +1573,39 @@ async def test_remove_member_prevents_removing_last_admin(self, test_db, session @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_requires_manager_or_admin_permission(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_member_requires_manager_or_admin_permission( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with regular user - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - regular_user = await create_user(session=session, name="regular", global_role=GlobalRole.USER, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - other_user = await create_user(session=session, name="other", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + regular_user = await create_user( + session=session, + name="regular", + global_role=GlobalRole.USER, + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + other_user = await create_user( + session=session, + name="other", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) - await add_project_member(session=session, project=project, user=other_user, project_role=ProjectRole.USER) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) + await add_project_member( + session=session, project=project, user=regular_user, project_role=ProjectRole.USER + ) + await add_project_member( + session=session, project=project, user=other_user, project_role=ProjectRole.USER + ) # Try to remove as regular user body = {"usernames": ["other"]} @@ -1457,23 +1620,43 @@ async def test_remove_member_requires_manager_or_admin_permission(self, test_db, # Batch Operations Tests @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_multiple_members_batch(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_add_multiple_members_batch( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create multiple users to add - _user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - _user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - _user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + _user1 = await create_user( + session=session, + name="user1", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + _user2 = await create_user( + session=session, + name="user2", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + _user3 = await create_user( + session=session, + name="user3", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # Add multiple members at once body = { "members": [ {"username": "user1", "project_role": "user"}, {"username": "user2", "project_role": "manager"}, - {"username": "user3", "project_role": "user"} + {"username": "user3", "project_role": "user"}, ] } response = await client.post( @@ -1492,28 +1675,58 @@ async def test_add_multiple_members_batch(self, test_db, session: AsyncSession, assert "user3" in member_usernames # Check roles - user1_member = next(m for m in response_data["members"] if m["user"]["username"] == "user1") + user1_member = next( + m for m in response_data["members"] if m["user"]["username"] == "user1" + ) assert user1_member["project_role"] == "user" - user2_member = next(m for m in response_data["members"] if m["user"]["username"] == "user2") + user2_member = next( + m for m in response_data["members"] if m["user"]["username"] == "user2" + ) assert user2_member["project_role"] == "manager" @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_multiple_members_batch(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_remove_multiple_members_batch( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project and admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create multiple users to remove - user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user3 = await create_user(session=session, name="user3", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + user1 = await create_user( + session=session, + name="user1", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + user2 = await create_user( + session=session, + name="user2", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + user3 = await create_user( + session=session, + name="user3", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) - await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) - await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) - await add_project_member(session=session, project=project, user=user3, project_role=ProjectRole.USER) + await add_project_member( + session=session, project=project, user=user1, project_role=ProjectRole.USER + ) + await add_project_member( + session=session, project=project, user=user2, project_role=ProjectRole.USER + ) + await add_project_member( + session=session, project=project, user=user3, project_role=ProjectRole.USER + ) # Remove multiple members at once body = {"usernames": ["user1", "user2", "user3"]} @@ -1542,18 +1755,32 @@ async def test_remove_multiple_members_batch(self, test_db, session: AsyncSessio # Join/Leave Functionality Tests @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_join_public_project(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_can_join_public_project( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup public project with admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) # Make project public project.is_public = True await session.commit() - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user who wants to join - regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user( + session=session, + name="joiner", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # User joins public project (should work) body = {"members": [{"username": "joiner", "project_role": "user"}]} @@ -1576,16 +1803,30 @@ async def test_user_can_join_public_project(self, test_db, session: AsyncSession @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_join_private_project(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_cannot_join_private_project( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup private project with admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) # Project is private by default (is_public=False) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user who wants to join - regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user( + session=session, + name="joiner", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # User tries to join private project (should fail) body = {"members": [{"username": "joiner", "project_role": "user"}]} @@ -1599,17 +1840,31 @@ async def test_user_cannot_join_private_project(self, test_db, session: AsyncSes @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_join_public_project_as_admin(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_cannot_join_public_project_as_admin( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup public project with admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) project.is_public = True await session.commit() - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Create user who wants to join as admin - regular_user = await create_user(session=session, name="joiner", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + regular_user = await create_user( + session=session, + name="joiner", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) # User tries to join public project as admin (should fail) body = {"members": [{"username": "joiner", "project_role": "admin"}]} @@ -1623,14 +1878,30 @@ async def test_user_cannot_join_public_project_as_admin(self, test_db, session: @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_leave_project(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_can_leave_project( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with admin and regular member - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - regular_user = await create_user(session=session, name="leaver", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + regular_user = await create_user( + session=session, + name="leaver", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) + await add_project_member( + session=session, project=project, user=regular_user, project_role=ProjectRole.USER + ) # User leaves project (should work) body = {"usernames": ["leaver"]} @@ -1653,19 +1924,31 @@ async def test_user_can_leave_project(self, test_db, session: AsyncSession, clie @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_leave_project_by_email(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_can_leave_project_by_email( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with admin and regular member - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) regular_user = await create_user( session=session, name="leaver", email="leaver@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), ) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - await add_project_member(session=session, project=project, user=regular_user, project_role=ProjectRole.USER) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) + await add_project_member( + session=session, project=project, user=regular_user, project_role=ProjectRole.USER + ) # User leaves project by email (should work) body = {"usernames": ["leaver@example.com"]} @@ -1679,11 +1962,21 @@ async def test_user_can_leave_project_by_email(self, test_db, session: AsyncSess @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_last_admin_cannot_leave_project(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_last_admin_cannot_leave_project( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with only one admin - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) # Admin tries to leave (should fail as they're the last admin) body = {"usernames": ["admin"]} @@ -1699,14 +1992,30 @@ async def test_last_admin_cannot_leave_project(self, test_db, session: AsyncSess @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_admin_can_leave_if_other_admins_exist(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_admin_can_leave_if_other_admins_exist( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with two admins - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin1 = await create_user(session=session, name="admin1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin2 = await create_user(session=session, name="admin2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin1 = await create_user( + session=session, + name="admin1", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + admin2 = await create_user( + session=session, + name="admin2", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) - await add_project_member(session=session, project=project, user=admin1, project_role=ProjectRole.ADMIN) - await add_project_member(session=session, project=project, user=admin2, project_role=ProjectRole.ADMIN) + await add_project_member( + session=session, project=project, user=admin1, project_role=ProjectRole.ADMIN + ) + await add_project_member( + session=session, project=project, user=admin2, project_role=ProjectRole.ADMIN + ) # One admin leaves (should work as there's another admin) body = {"usernames": ["admin1"]} @@ -1726,16 +2035,38 @@ async def test_admin_can_leave_if_other_admins_exist(self, test_db, session: Asy @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_leave_others_from_project(self, test_db, session: AsyncSession, client: AsyncClient): + async def test_user_cannot_leave_others_from_project( + self, test_db, session: AsyncSession, client: AsyncClient + ): # Setup project with admin and two regular users - project = await create_project(session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - admin = await create_user(session=session, name="admin", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user1 = await create_user(session=session, name="user1", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - user2 = await create_user(session=session, name="user2", created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc)) - - await add_project_member(session=session, project=project, user=admin, project_role=ProjectRole.ADMIN) - await add_project_member(session=session, project=project, user=user1, project_role=ProjectRole.USER) - await add_project_member(session=session, project=project, user=user2, project_role=ProjectRole.USER) + project = await create_project( + session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) + ) + admin = await create_user( + session=session, + name="admin", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + user1 = await create_user( + session=session, + name="user1", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + user2 = await create_user( + session=session, + name="user2", + created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), + ) + + await add_project_member( + session=session, project=project, user=admin, project_role=ProjectRole.ADMIN + ) + await add_project_member( + session=session, project=project, user=user1, project_role=ProjectRole.USER + ) + await add_project_member( + session=session, project=project, user=user2, project_role=ProjectRole.USER + ) # user1 tries to remove user2 (should fail) body = {"usernames": ["user2"]} From 001839982c6c3719cf5b408d3a81126c367848c3 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 11 Jun 2025 14:17:05 -0400 Subject: [PATCH 05/15] Add project visibility update API endpoint --- .../_internal/server/routers/projects.py | 20 +++ .../_internal/server/schemas/projects.py | 4 + .../_internal/server/security/permissions.py | 29 ++++ .../_internal/server/services/projects.py | 12 ++ .../_internal/server/routers/test_projects.py | 151 +++++++++++++++++- 5 files changed, 215 insertions(+), 1 deletion(-) diff --git a/src/dstack/_internal/server/routers/projects.py b/src/dstack/_internal/server/routers/projects.py index 8354d49c35..d5c619bcb7 100644 --- a/src/dstack/_internal/server/routers/projects.py +++ b/src/dstack/_internal/server/routers/projects.py @@ -12,6 +12,7 @@ DeleteProjectsRequest, RemoveProjectMemberRequest, SetProjectMembersRequest, + UpdateProjectVisibilityRequest, ) from dstack._internal.server.security.permissions import ( Authenticated, @@ -135,3 +136,22 @@ async def remove_project_members( ) await session.refresh(project) return projects.project_model_to_project(project) + + +@router.post( + "/{project_name}/update_visibility", +) +async def update_project_visibility( + body: UpdateProjectVisibilityRequest, + session: AsyncSession = Depends(get_session), + user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()), +) -> Project: + user, project = user_project + await projects.update_project_visibility( + session=session, + user=user, + project=project, + is_public=body.is_public, + ) + await session.refresh(project) + return projects.project_model_to_project(project) diff --git a/src/dstack/_internal/server/schemas/projects.py b/src/dstack/_internal/server/schemas/projects.py index ed5dc159b2..8318ae19f9 100644 --- a/src/dstack/_internal/server/schemas/projects.py +++ b/src/dstack/_internal/server/schemas/projects.py @@ -11,6 +11,10 @@ class CreateProjectRequest(CoreModel): is_public: bool = False +class UpdateProjectVisibilityRequest(CoreModel): + is_public: bool + + class DeleteProjectsRequest(CoreModel): projects_names: List[str] diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 7342fc336d..74983a897b 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -67,6 +67,35 @@ async def __call__( raise error_forbidden() +class ProjectManager: + """ + Allows project admins and managers to manage projects. + """ + async def __call__( + self, + project_name: str, + session: AsyncSession = Depends(get_session), + token: HTTPAuthorizationCredentials = Security(HTTPBearer()), + ) -> Tuple[UserModel, ProjectModel]: + 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 + + # Project managers and admins can manage projects + project_role = get_user_project_role(user=user, project=project) + if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + return user, project + + raise error_forbidden() + + class ProjectMember: async def __call__( self, diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index b4458e43f0..0a1dd59725 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -150,6 +150,18 @@ async def create_project( return project_model_to_project(project_model) +async def update_project_visibility( + session: AsyncSession, + user: UserModel, + project: ProjectModel, + is_public: bool, +): + """Update project visibility (public/private).""" + # Update the project visibility + project.is_public = is_public + await session.commit() + + async def delete_projects( session: AsyncSession, user: UserModel, diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index c9e573dbdd..3dd8ac65a4 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -49,7 +49,10 @@ async def test_returns_projects(self, test_db, session: AsyncSession, client: As created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), ) await add_project_member( - session=session, project=project, user=user, project_role=ProjectRole.ADMIN + session=session, + project=project, + user=user, + project_role=ProjectRole.ADMIN ) await create_backend( session=session, @@ -2077,4 +2080,150 @@ async def test_user_cannot_leave_others_from_project( ) assert response.status_code == 403 +<<<<<<< HEAD >>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) +======= + + +class TestUpdateProjectVisibility: + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClient): + response = await client.post("/api/projects/test/update_visibility") + assert response.status_code in [401, 403] + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_returns_404_if_project_does_not_exist( + self, test_db, session: AsyncSession, client: AsyncClient + ): + user = await create_user(session=session) + response = await client.post( + "/api/projects/nonexistent/update_visibility", + headers=get_auth_headers(user.token), + json={"is_public": True}, + ) + assert response.status_code == 404 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_project_admin_can_update_visibility( + self, test_db, session: AsyncSession, client: AsyncClient + ): + # Setup project with admin + admin_user = await create_user(session=session, name="admin", 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 + ) + + # Admin should be able to make project public + response = await client.post( + f"/api/projects/{project.name}/update_visibility", + headers=get_auth_headers(admin_user.token), + json={"is_public": True}, + ) + assert response.status_code == 200 + assert response.json()["is_public"] == True + + # Admin should be able to make project private again + response = await client.post( + f"/api/projects/{project.name}/update_visibility", + headers=get_auth_headers(admin_user.token), + json={"is_public": False}, + ) + 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( + self, test_db, session: AsyncSession, client: AsyncClient + ): + # Setup project with admin and regular user + admin_user = await create_user(session=session, name="admin", global_role=GlobalRole.USER) + regular_user = await create_user(session=session, name="user", 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=regular_user, project_role=ProjectRole.USER + ) + + # Regular user should not be able to update visibility + response = await client.post( + f"/api/projects/{project.name}/update_visibility", + headers=get_auth_headers(regular_user.token), + json={"is_public": True}, + ) + assert response.status_code == 403 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_non_member_cannot_update_visibility( + self, test_db, session: AsyncSession, client: AsyncClient + ): + # Setup project with admin and separate non-member user + admin_user = await create_user(session=session, name="admin", global_role=GlobalRole.USER) + non_member_user = await create_user(session=session, name="nonmember", 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 + ) + + # Non-member should not be able to update visibility + response = await client.post( + f"/api/projects/{project.name}/update_visibility", + headers=get_auth_headers(non_member_user.token), + json={"is_public": True}, + ) + assert response.status_code == 403 + + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_global_admin_can_update_any_project_visibility( + self, test_db, session: AsyncSession, client: AsyncClient + ): + # Setup project with regular owner and global admin + project_owner = await create_user(session=session, name="owner", global_role=GlobalRole.USER) + global_admin = await create_user(session=session, name="admin", global_role=GlobalRole.ADMIN) + project = await create_project(session=session, owner=project_owner, is_public=False) + await add_project_member( + session=session, project=project, user=project_owner, project_role=ProjectRole.ADMIN + ) + + # Global admin should be able to update any project's visibility + response = await client.post( + f"/api/projects/{project.name}/update_visibility", + headers=get_auth_headers(global_admin.token), + json={"is_public": True}, + ) + assert response.status_code == 200 + assert response.json()["is_public"] == True +>>>>>>> a6246f4d (Add project visibility update API endpoint) From f239325211affd786a442e4fb5ffe36e6a6952a3 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 11 Jun 2025 14:57:26 -0400 Subject: [PATCH 06/15] Fix user autocomplete: allow all users to see user list --- src/dstack/_internal/server/services/users.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 8504f2af75..6a4c7edea3 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -44,9 +44,8 @@ async def list_users_for_user( session: AsyncSession, user: UserModel, ) -> List[User]: - if user.global_role == GlobalRole.ADMIN: - return await list_all_users(session=session) - return [user_model_to_user(user)] + # Allow all authenticated users to see the user list for project collaboration + return await list_all_users(session=session) async def list_all_users( From 27cfa3d07399b31f829afa2de3385ae258227ec1 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:18:35 -0400 Subject: [PATCH 07/15] Fix project deletion permissions: allow project owners to delete their projects --- src/dstack/_internal/server/services/projects.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 0a1dd59725..5457f5d6b3 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -175,7 +175,9 @@ async def delete_projects( for project_name in projects_names: if project_name not in user_project_names: raise ForbiddenError() - for project in user_projects: + # Only check admin permissions for projects being deleted + projects_to_delete = [p for p in user_projects if p.name in projects_names] + for project in projects_to_delete: if not _is_project_admin(user=user, project=project): raise ForbiddenError() if all(name in projects_names for name in user_project_names): @@ -604,6 +606,11 @@ def _is_project_admin( user: UserModel, project: ProjectModel, ) -> bool: + # Check if user is the project owner + if user.id == project.owner_id: + return True + + # Check if user has admin role in project members for m in project.members: if user.id == m.user_id: if m.project_role == ProjectRole.ADMIN: From da13db35924a0c7ac1b12acd4af6b2197253efcf Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:29:14 -0400 Subject: [PATCH 08/15] Apply pre-commit formatting fixes to PR2 --- .../_internal/server/routers/projects.py | 1 - .../_internal/server/security/permissions.py | 7 ++++--- .../_internal/server/services/projects.py | 2 +- .../_internal/server/routers/test_projects.py | 21 ++++++++++++------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/dstack/_internal/server/routers/projects.py b/src/dstack/_internal/server/routers/projects.py index d5c619bcb7..f60c90b331 100644 --- a/src/dstack/_internal/server/routers/projects.py +++ b/src/dstack/_internal/server/routers/projects.py @@ -19,7 +19,6 @@ ProjectManager, ProjectManagerOrPublicJoin, ProjectManagerOrSelfLeave, - ProjectMember, ProjectMemberOrPublicAccess, ) from dstack._internal.server.services import projects diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 74983a897b..3a9cf0c72e 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -71,6 +71,7 @@ class ProjectManager: """ Allows project admins and managers to manage projects. """ + async def __call__( self, project_name: str, @@ -83,16 +84,16 @@ async def __call__( 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 - + # Project managers and admins can manage projects project_role = get_user_project_role(user=user, project=project) if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: return user, project - + raise error_forbidden() diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 5457f5d6b3..57392d3ac1 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -609,7 +609,7 @@ def _is_project_admin( # Check if user is the project owner if user.id == project.owner_id: return True - + # Check if user has admin role in project members for m in project.members: if user.id == m.user_id: diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 3dd8ac65a4..34191dd1b2 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -49,10 +49,7 @@ async def test_returns_projects(self, test_db, session: AsyncSession, client: As created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), ) await add_project_member( - session=session, - project=project, - user=user, - project_role=ProjectRole.ADMIN + session=session, project=project, user=user, project_role=ProjectRole.ADMIN ) await create_backend( session=session, @@ -2142,7 +2139,9 @@ async def test_project_manager_can_update_visibility( ): # 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) + 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 @@ -2191,7 +2190,9 @@ async def test_non_member_cannot_update_visibility( ): # Setup project with admin and separate non-member user admin_user = await create_user(session=session, name="admin", global_role=GlobalRole.USER) - non_member_user = await create_user(session=session, name="nonmember", global_role=GlobalRole.USER) + non_member_user = await create_user( + session=session, name="nonmember", 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 @@ -2211,8 +2212,12 @@ async def test_global_admin_can_update_any_project_visibility( self, test_db, session: AsyncSession, client: AsyncClient ): # Setup project with regular owner and global admin - project_owner = await create_user(session=session, name="owner", global_role=GlobalRole.USER) - global_admin = await create_user(session=session, name="admin", global_role=GlobalRole.ADMIN) + project_owner = await create_user( + session=session, name="owner", global_role=GlobalRole.USER + ) + global_admin = await create_user( + session=session, name="admin", global_role=GlobalRole.ADMIN + ) project = await create_project(session=session, owner=project_owner, is_public=False) await add_project_member( session=session, project=project, user=project_owner, project_role=ProjectRole.ADMIN From 2dbd7b7f3e7e6e4922eaca78aac579d7e89ea331 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:42:53 -0400 Subject: [PATCH 09/15] Fix project deletion permissions and member race conditions --- .../_internal/server/security/permissions.py | 6 +--- .../_internal/server/services/projects.py | 32 ++++++------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 3a9cf0c72e..237a883909 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -204,12 +204,8 @@ async def __call__( if user.global_role == GlobalRole.ADMIN: return user, project - # Project managers can remove members + # 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 in [ProjectRole.ADMIN, ProjectRole.MANAGER]: - return user, project - - # Any project member can leave (will be validated in service layer) if project_role is not None: return user, project diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 57392d3ac1..8372f94e13 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -296,45 +296,35 @@ async def add_project_members( username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - # Build a set of current member IDs so we can quickly check if users are already members - member_ids = {m.user_id for m in project.members} # Build a map from user_id to member for efficient existing member updates member_by_user_id = {m.user_id: m for m in project.members} - # Get current max member_num - max_member_num = 0 - for member in project.members: - if member.member_num is not None and member.member_num > max_member_num: - max_member_num = member.member_num - # Process each member to add for member_setting in members: user_to_add = username_to_user.get(member_setting.username) or email_to_user.get( member_setting.username ) if user_to_add is None: - # Error on adding non-existing users instead of silently skipping raise ServerClientError(f"User not found: {member_setting.username}") - # Check if user is already a member using our fast lookup set - if user_to_add.id in member_ids: - # Update existing member role if different using our efficient lookup + # Check if user is already a member + if user_to_add.id in member_by_user_id: + # Update existing member role if different existing_member = member_by_user_id[user_to_add.id] if existing_member.project_role != member_setting.project_role: existing_member.project_role = member_setting.project_role else: - # Add new member - max_member_num += 1 + # Add new member (let database handle member_num to avoid race conditions) await add_project_member( session=session, project=project, user=user_to_add, project_role=member_setting.project_role, - member_num=max_member_num, + member_num=None, # Let database auto-assign to avoid race conditions commit=False, ) - # Keep track of newly added members for subsequent iterations - member_ids.add(user_to_add.id) + # Update our local tracking for subsequent iterations + member_by_user_id[user_to_add.id] = None # Placeholder to track addition await session.commit() @@ -654,8 +644,6 @@ async def remove_project_members( username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - # Build a set of member IDs for faster membership checks - member_user_ids = {m.user_id for m in project.members} # Build a map from user_id to member for efficient member lookups member_by_user_id = {m.user_id: m for m in project.members} @@ -666,15 +654,13 @@ async def remove_project_members( for username in usernames: user_to_remove = username_to_user.get(username) or email_to_user.get(username) if user_to_remove is None: - # Error on removing non-existing users instead of silently skipping raise ServerClientError(f"User not found: {username}") # Check if user is actually a member before trying to remove them - if user_to_remove.id not in member_user_ids: - # Error on removing non-members instead of silently skipping + if user_to_remove.id not in member_by_user_id: raise ServerClientError(f"User is not a member of this project: {username}") - # Get the member to remove using our efficient lookup + # Get the member to remove member_to_remove = member_by_user_id[user_to_remove.id] # Check if trying to remove project admin From e540d636fd528f1a8e9ec07c4735db42cb79b327 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 11:30:29 -0400 Subject: [PATCH 10/15] Allow public project access to gateways endpoints --- src/dstack/_internal/server/routers/gateways.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/dstack/_internal/server/routers/gateways.py b/src/dstack/_internal/server/routers/gateways.py index 604519af0c..e0e0ad37d1 100644 --- a/src/dstack/_internal/server/routers/gateways.py +++ b/src/dstack/_internal/server/routers/gateways.py @@ -9,7 +9,10 @@ from dstack._internal.core.errors import ResourceNotExistsError from dstack._internal.server.db import get_session from dstack._internal.server.models import ProjectModel, UserModel -from dstack._internal.server.security.permissions import ProjectAdmin, ProjectMember +from dstack._internal.server.security.permissions import ( + ProjectAdmin, + ProjectMemberOrPublicAccess, +) from dstack._internal.server.utils.routers import get_base_api_additional_responses router = APIRouter( @@ -22,7 +25,7 @@ @router.post("/list") async def list_gateways( session: AsyncSession = Depends(get_session), - user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectMember()), + user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectMemberOrPublicAccess()), ) -> List[models.Gateway]: _, project = user_project return await gateways.list_project_gateways(session=session, project=project) @@ -32,7 +35,7 @@ async def list_gateways( async def get_gateway( body: schemas.GetGatewayRequest, session: AsyncSession = Depends(get_session), - user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectMember()), + user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectMemberOrPublicAccess()), ) -> models.Gateway: _, project = user_project gateway = await gateways.get_gateway_by_name(session=session, project=project, name=body.name) From 31fe3a5a43213be4be7d3de029a6faef905bcb95 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 17 Jun 2025 10:27:37 -0400 Subject: [PATCH 11/15] fix: address PR comments from r4victor --- .../_internal/server/routers/projects.py | 7 ++--- .../_internal/server/schemas/projects.py | 2 -- .../_internal/server/security/permissions.py | 11 +++++--- .../_internal/server/routers/test_projects.py | 27 ------------------- 4 files changed, 11 insertions(+), 36 deletions(-) diff --git a/src/dstack/_internal/server/routers/projects.py b/src/dstack/_internal/server/routers/projects.py index f60c90b331..c0920d060a 100644 --- a/src/dstack/_internal/server/routers/projects.py +++ b/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( diff --git a/src/dstack/_internal/server/schemas/projects.py b/src/dstack/_internal/server/schemas/projects.py index 8318ae19f9..b884687f11 100644 --- a/src/dstack/_internal/server/schemas/projects.py +++ b/src/dstack/_internal/server/schemas/projects.py @@ -32,10 +32,8 @@ class SetProjectMembersRequest(CoreModel): class AddProjectMemberRequest(CoreModel): - # Always accept a list of members for cleaner API design members: List[MemberSetting] class RemoveProjectMemberRequest(CoreModel): - # Always accept a list of usernames for cleaner API design usernames: List[str] diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 237a883909..9efc9146ed 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/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) @@ -144,13 +144,16 @@ async def __call__( raise error_forbidden() -class ProjectManagerOrPublicJoin: +class ProjectManagerOrPublicProject: """ Allows: - 1. Project managers to add any members - 2. Any authenticated user to join public projects themselves + 1. Project managers to perform member management operations + 2. Access to public projects for any authenticated user """ + def __init__(self): + self.project_manager = ProjectManager() + async def __call__( self, project_name: str, diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 34191dd1b2..62610033d8 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -2132,33 +2132,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( From 841c63a95ff4ca561235bf9f59b5dfdfb70dad30 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Mon, 23 Jun 2025 10:09:18 -0400 Subject: [PATCH 12/15] feat: generalize project update endpoint --- frontend/src/api.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/src/api.ts b/frontend/src/api.ts index f5d910ed71..226d5edde3 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -56,6 +56,9 @@ export const API = { DETAILS: (name: IProject['project_name']) => `${API.PROJECTS.BASE()}/${name}`, DETAILS_INFO: (name: IProject['project_name']) => `${API.PROJECTS.DETAILS(name)}/get`, SET_MEMBERS: (name: IProject['project_name']) => `${API.PROJECTS.DETAILS(name)}/set_members`, + ADD_MEMBERS: (name: IProject['project_name']) => `${API.PROJECTS.DETAILS(name)}/add_members`, + REMOVE_MEMBERS: (name: IProject['project_name']) => `${API.PROJECTS.DETAILS(name)}/remove_members`, + UPDATE: (name: IProject['project_name']) => `${API.PROJECTS.DETAILS(name)}/update`, // Repos REPOS: (projectName: IProject['project_name']) => `${API.BASE()}/project/${projectName}/repos`, From 2d5ce9e7150c01c14c851f6cae2288e4c8d2606f Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Mon, 23 Jun 2025 11:18:23 -0400 Subject: [PATCH 13/15] refactor: rename /update_visibility to /update for more generic project updates --- .../_internal/server/routers/projects.py | 10 +- .../_internal/server/schemas/projects.py | 2 +- .../_internal/server/services/projects.py | 2 +- .../_internal/server/routers/test_projects.py | 1045 +---------------- 4 files changed, 15 insertions(+), 1044 deletions(-) diff --git a/src/dstack/_internal/server/routers/projects.py b/src/dstack/_internal/server/routers/projects.py index c0920d060a..1d967c6c8d 100644 --- a/src/dstack/_internal/server/routers/projects.py +++ b/src/dstack/_internal/server/routers/projects.py @@ -12,7 +12,7 @@ DeleteProjectsRequest, RemoveProjectMemberRequest, SetProjectMembersRequest, - UpdateProjectVisibilityRequest, + UpdateProjectRequest, ) from dstack._internal.server.security.permissions import ( Authenticated, @@ -139,15 +139,15 @@ async def remove_project_members( @router.post( - "/{project_name}/update_visibility", + "/{project_name}/update", ) -async def update_project_visibility( - body: UpdateProjectVisibilityRequest, +async def update_project( + body: UpdateProjectRequest, session: AsyncSession = Depends(get_session), user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()), ) -> Project: user, project = user_project - await projects.update_project_visibility( + await projects.update_project( session=session, user=user, project=project, diff --git a/src/dstack/_internal/server/schemas/projects.py b/src/dstack/_internal/server/schemas/projects.py index b884687f11..355bb3a770 100644 --- a/src/dstack/_internal/server/schemas/projects.py +++ b/src/dstack/_internal/server/schemas/projects.py @@ -11,7 +11,7 @@ class CreateProjectRequest(CoreModel): is_public: bool = False -class UpdateProjectVisibilityRequest(CoreModel): +class UpdateProjectRequest(CoreModel): is_public: bool diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 8372f94e13..b823f5456f 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -150,7 +150,7 @@ async def create_project( return project_model_to_project(project_model) -async def update_project_visibility( +async def update_project( session: AsyncSession, user: UserModel, project: ProjectModel, diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index 62610033d8..b0364b4235 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -79,7 +79,6 @@ async def test_returns_projects(self, test_db, session: AsyncSession, client: As } ] -<<<<<<< HEAD @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) async def test_returns_public_projects_to_non_members( @@ -202,8 +201,6 @@ async def test_member_sees_both_public_and_private_projects( assert "public_project" in project_names assert "private_project" in project_names -======= ->>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) class TestCreateProject: @pytest.mark.asyncio @@ -348,8 +345,7 @@ async def test_forbids_if_no_permission_to_create_projects( headers=get_auth_headers(user.token), json={"project_name": "new_project"}, ) -<<<<<<< HEAD - assert response.status_code == 403 + assert response.status_code == 403 @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) @@ -437,9 +433,6 @@ async def test_creates_private_project_explicitly( res = await session.execute(select(ProjectModel).where(ProjectModel.name == project_name)) project = res.scalar_one() assert project.is_public is False -======= - assert response.status_code in [401, 403] ->>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) class TestDeleteProject: @@ -1061,1032 +1054,11 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin( assert response.status_code == 403 -<<<<<<< HEAD -class TestListUserProjectsService: - """Test the service-level functions for backward compatibility""" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_list_user_projects_only_returns_member_projects( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Create project owner - owner = await create_user( - session=session, - name="owner", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - global_role=GlobalRole.USER, - ) - - # Create a different user who is not a member - non_member = await create_user( - session=session, - name="non_member", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - global_role=GlobalRole.USER, - ) - - # Create a public project - public_project = await create_project( - session=session, - owner=owner, - name="public_project", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - is_public=True, - ) - - # Add owner as admin - await add_project_member( - session=session, project=public_project, user=owner, project_role=ProjectRole.ADMIN - ) - - # Test: list_user_projects should NOT return public projects for non-members - from dstack._internal.server.services.projects import list_user_projects - - projects = await list_user_projects(session=session, user=non_member) - assert len(projects) == 0 # Non-member should see NO projects - - # Test: list_user_projects should return projects where user IS a member - projects = await list_user_projects(session=session, user=owner) - assert len(projects) == 1 - assert projects[0].project_name == "public_project" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_list_user_accessible_projects_returns_member_and_public_projects( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Create project owner - owner = await create_user( - session=session, - name="owner", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - global_role=GlobalRole.USER, - ) - - # Create a different user who is not a member - non_member = await create_user( - session=session, - name="non_member", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - global_role=GlobalRole.USER, - ) - - # Create a public project - public_project = await create_project( - session=session, - owner=owner, - name="public_project", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - is_public=True, - ) - - # Create a private project - private_project = await create_project( - session=session, - owner=owner, - name="private_project", - created_at=datetime(2023, 1, 2, 3, 5, tzinfo=timezone.utc), - is_public=False, - ) - - # Add owner as admin to both projects - await add_project_member( - session=session, project=public_project, user=owner, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=private_project, user=owner, project_role=ProjectRole.ADMIN - ) - - # Test: list_user_accessible_projects should return public projects for non-members - from dstack._internal.server.services.projects import list_user_accessible_projects - - projects = await list_user_accessible_projects(session=session, user=non_member) - assert len(projects) == 1 # Should see only the public project - assert projects[0].project_name == "public_project" - - # Test: list_user_accessible_projects should return ALL projects for members - projects = await list_user_accessible_projects(session=session, user=owner) - assert len(projects) == 2 # Should see both projects - project_names = [p.project_name for p in projects] - assert "public_project" in project_names - assert "private_project" in project_names -======= -class TestMemberManagement: - """Test class for add_member and remove_member endpoints""" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - @pytest.mark.parametrize("endpoint", ["add_members", "remove_members"]) - async def test_returns_40x_if_not_authenticated( - self, test_db, client: AsyncClient, endpoint: str - ): - response = await client.post(f"/api/projects/test-project/{endpoint}") - assert response.status_code in [401, 403] - - # Add Member Tests - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_by_username( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user to add - _new_user = await create_user( - session=session, - name="newuser", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # Add member - body = {"members": [{"username": "newuser", "project_role": "user"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that new user is in the members list - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "newuser" in member_usernames - - # Find the new member and check their role - new_member = next( - m for m in response_data["members"] if m["user"]["username"] == "newuser" - ) - assert new_member["project_role"] == "user" - - # Verify in database - res = await session.execute(select(MemberModel).where(MemberModel.user_id == _new_user.id)) - member = res.scalar_one() - assert member.project_role == ProjectRole.USER - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_by_email(self, test_db, session: AsyncSession, client: AsyncClient): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user to add - _new_user = await create_user( - session=session, - name="emailuser", - email="test@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # Add member by email - body = {"members": [{"username": "test@example.com", "project_role": "manager"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that new user is in the members list - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "emailuser" in member_usernames - - # Find the new member and check their role - new_member = next( - m for m in response_data["members"] if m["user"]["username"] == "emailuser" - ) - assert new_member["project_role"] == "manager" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_updates_existing_role( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user and add as USER first - existing_user = await create_user( - session=session, - name="existing", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=existing_user, project_role=ProjectRole.USER - ) - - # Update to MANAGER - body = {"members": [{"username": "existing", "project_role": "manager"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Find the updated member and check their role - updated_member = next( - m for m in response_data["members"] if m["user"]["username"] == "existing" - ) - assert updated_member["project_role"] == "manager" - - # Verify in database - res = await session.execute( - select(MemberModel).where(MemberModel.user_id == existing_user.id) - ) - member = res.scalar_one() - assert member.project_role == ProjectRole.MANAGER - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_errors_on_nonexistent_user( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Try to add non-existent user - should now error instead of silently skipping - body = {"members": [{"username": "nonexistent", "project_role": "user"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - # Operation should fail with 400 error for non-existent user - assert response.status_code == 400 - response_json = response.json() - assert "User not found: nonexistent" in str(response_json) - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_member_manager_cannot_add_admin_without_global_admin( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with manager (not global admin) - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - manager = await create_user( - session=session, - global_role=GlobalRole.USER, - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=manager, project_role=ProjectRole.MANAGER - ) - - # Create user to add - _new_user = await create_user( - session=session, - name="newuser", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # Try to add admin - body = {"members": [{"username": "newuser", "project_role": "admin"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(manager.token), - json=body, - ) - - assert response.status_code == 403 - - # Remove Member Tests - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_by_username( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user to remove - user_to_remove = await create_user( - session=session, - name="removeuser", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER - ) - - # Remove member - body = {"usernames": ["removeuser"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that user is NOT in the members list anymore - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "removeuser" not in member_usernames - - # Verify removed from database - res = await session.execute( - select(MemberModel).where(MemberModel.user_id == user_to_remove.id) - ) - member = res.scalar_one_or_none() - assert member is None - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_by_email( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user to remove - user_to_remove = await create_user( - session=session, - name="emailremove", - email="remove@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=user_to_remove, project_role=ProjectRole.USER - ) - - # Remove member by email - body = {"usernames": ["remove@example.com"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_errors_on_nonexistent_user( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Try to remove non-existent user - should now error instead of silently skipping - body = {"usernames": ["nonexistent"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - # Operation should fail with 400 error for non-existent user - assert response.status_code == 400 - response_json = response.json() - assert "User not found: nonexistent" in str(response_json) - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_errors_on_non_members( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user who is NOT a member - _non_member = await create_user( - session=session, - name="nonmember", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # Try to remove non-member - should now error instead of silently skipping - body = {"usernames": ["nonmember"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - # Operation should fail with 400 error for non-member - assert response.status_code == 400 - response_json = response.json() - assert "User is not a member of this project: nonmember" in str(response_json) - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_prevents_removing_last_admin( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with only one admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Try to remove the only admin - body = {"usernames": [admin.name]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 400 - response_json = response.json() - # Check for the new self-leave error message (since admin is removing themselves) - assert "Cannot leave project: you are the last admin" in str(response_json) - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_member_requires_manager_or_admin_permission( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with regular user - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - regular_user = await create_user( - session=session, - name="regular", - global_role=GlobalRole.USER, - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - other_user = await create_user( - session=session, - name="other", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=project, user=regular_user, project_role=ProjectRole.USER - ) - await add_project_member( - session=session, project=project, user=other_user, project_role=ProjectRole.USER - ) - - # Try to remove as regular user - body = {"usernames": ["other"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 403 - - # Batch Operations Tests - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_add_multiple_members_batch( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create multiple users to add - _user1 = await create_user( - session=session, - name="user1", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - _user2 = await create_user( - session=session, - name="user2", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - _user3 = await create_user( - session=session, - name="user3", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # Add multiple members at once - body = { - "members": [ - {"username": "user1", "project_role": "user"}, - {"username": "user2", "project_role": "manager"}, - {"username": "user3", "project_role": "user"}, - ] - } - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that all new users are in the members list - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "user1" in member_usernames - assert "user2" in member_usernames - assert "user3" in member_usernames - - # Check roles - user1_member = next( - m for m in response_data["members"] if m["user"]["username"] == "user1" - ) - assert user1_member["project_role"] == "user" - - user2_member = next( - m for m in response_data["members"] if m["user"]["username"] == "user2" - ) - assert user2_member["project_role"] == "manager" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_remove_multiple_members_batch( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project and admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create multiple users to remove - user1 = await create_user( - session=session, - name="user1", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - user2 = await create_user( - session=session, - name="user2", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - user3 = await create_user( - session=session, - name="user3", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=user1, project_role=ProjectRole.USER - ) - await add_project_member( - session=session, project=project, user=user2, project_role=ProjectRole.USER - ) - await add_project_member( - session=session, project=project, user=user3, project_role=ProjectRole.USER - ) - - # Remove multiple members at once - body = {"usernames": ["user1", "user2", "user3"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that all users are NOT in the members list anymore - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "user1" not in member_usernames - assert "user2" not in member_usernames - assert "user3" not in member_usernames - - # Verify removed from database - res = await session.execute(select(MemberModel).where(MemberModel.user_id == user1.id)) - assert res.scalar_one_or_none() is None - - res = await session.execute(select(MemberModel).where(MemberModel.user_id == user2.id)) - assert res.scalar_one_or_none() is None - - # Join/Leave Functionality Tests - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_join_public_project( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup public project with admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - # Make project public - project.is_public = True - await session.commit() - - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user who wants to join - regular_user = await create_user( - session=session, - name="joiner", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # User joins public project (should work) - body = {"members": [{"username": "joiner", "project_role": "user"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that user is now in the members list - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "joiner" in member_usernames - - # Find the new member and check their role is USER - new_member = next(m for m in response_data["members"] if m["user"]["username"] == "joiner") - assert new_member["project_role"] == "user" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_join_private_project( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup private project with admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - # Project is private by default (is_public=False) - - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user who wants to join - regular_user = await create_user( - session=session, - name="joiner", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # User tries to join private project (should fail) - body = {"members": [{"username": "joiner", "project_role": "user"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 403 - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_join_public_project_as_admin( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup public project with admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - project.is_public = True - await session.commit() - - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Create user who wants to join as admin - regular_user = await create_user( - session=session, - name="joiner", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - # User tries to join public project as admin (should fail) - body = {"members": [{"username": "joiner", "project_role": "admin"}]} - response = await client.post( - f"/api/projects/{project.name}/add_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 403 - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_leave_project( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with admin and regular member - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - regular_user = await create_user( - session=session, - name="leaver", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=project, user=regular_user, project_role=ProjectRole.USER - ) - - # User leaves project (should work) - body = {"usernames": ["leaver"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that user is NOT in the members list anymore - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "leaver" not in member_usernames - - # Should only have admin left - assert len(response_data["members"]) == 1 - assert response_data["members"][0]["user"]["username"] == "admin" - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_can_leave_project_by_email( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with admin and regular member - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - regular_user = await create_user( - session=session, - name="leaver", - email="leaver@example.com", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=project, user=regular_user, project_role=ProjectRole.USER - ) - - # User leaves project by email (should work) - body = {"usernames": ["leaver@example.com"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(regular_user.token), - json=body, - ) - - assert response.status_code == 200 - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_last_admin_cannot_leave_project( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with only one admin - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - - # Admin tries to leave (should fail as they're the last admin) - body = {"usernames": ["admin"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin.token), - json=body, - ) - - assert response.status_code == 400 - response_json = response.json() - assert "Cannot leave project: you are the last admin" in str(response_json) - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_admin_can_leave_if_other_admins_exist( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with two admins - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin1 = await create_user( - session=session, - name="admin1", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - admin2 = await create_user( - session=session, - name="admin2", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=admin1, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=project, user=admin2, project_role=ProjectRole.ADMIN - ) - - # One admin leaves (should work as there's another admin) - body = {"usernames": ["admin1"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(admin1.token), - json=body, - ) - - assert response.status_code == 200 - response_data = response.json() - - # Check that admin1 is NOT in the members list anymore - member_usernames = [member["user"]["username"] for member in response_data["members"]] - assert "admin1" not in member_usernames - assert "admin2" in member_usernames - - @pytest.mark.asyncio - @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) - async def test_user_cannot_leave_others_from_project( - self, test_db, session: AsyncSession, client: AsyncClient - ): - # Setup project with admin and two regular users - project = await create_project( - session=session, created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc) - ) - admin = await create_user( - session=session, - name="admin", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - user1 = await create_user( - session=session, - name="user1", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - user2 = await create_user( - session=session, - name="user2", - created_at=datetime(2023, 1, 2, 3, 4, tzinfo=timezone.utc), - ) - - await add_project_member( - session=session, project=project, user=admin, project_role=ProjectRole.ADMIN - ) - await add_project_member( - session=session, project=project, user=user1, project_role=ProjectRole.USER - ) - await add_project_member( - session=session, project=project, user=user2, project_role=ProjectRole.USER - ) - - # user1 tries to remove user2 (should fail) - body = {"usernames": ["user2"]} - response = await client.post( - f"/api/projects/{project.name}/remove_members", - headers=get_auth_headers(user1.token), - json=body, - ) - - assert response.status_code == 403 -<<<<<<< HEAD ->>>>>>> 7fc1b408 (fix: retrack PR2 based on PR1) -======= - - class TestUpdateProjectVisibility: @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) async def test_returns_40x_if_not_authenticated(self, test_db, client: AsyncClient): - response = await client.post("/api/projects/test/update_visibility") + response = await client.post("/api/projects/test/update") assert response.status_code in [401, 403] @pytest.mark.asyncio @@ -2096,7 +1068,7 @@ async def test_returns_404_if_project_does_not_exist( ): user = await create_user(session=session) response = await client.post( - "/api/projects/nonexistent/update_visibility", + "/api/projects/nonexistent/update", headers=get_auth_headers(user.token), json={"is_public": True}, ) @@ -2116,7 +1088,7 @@ async def test_project_admin_can_update_visibility( # Admin should be able to make project public response = await client.post( - f"/api/projects/{project.name}/update_visibility", + f"/api/projects/{project.name}/update", headers=get_auth_headers(admin_user.token), json={"is_public": True}, ) @@ -2125,7 +1097,7 @@ async def test_project_admin_can_update_visibility( # Admin should be able to make project private again response = await client.post( - f"/api/projects/{project.name}/update_visibility", + f"/api/projects/{project.name}/update", headers=get_auth_headers(admin_user.token), json={"is_public": False}, ) @@ -2150,7 +1122,7 @@ async def test_regular_user_cannot_update_visibility( # Regular user should not be able to update visibility response = await client.post( - f"/api/projects/{project.name}/update_visibility", + f"/api/projects/{project.name}/update", headers=get_auth_headers(regular_user.token), json={"is_public": True}, ) @@ -2173,7 +1145,7 @@ async def test_non_member_cannot_update_visibility( # Non-member should not be able to update visibility response = await client.post( - f"/api/projects/{project.name}/update_visibility", + f"/api/projects/{project.name}/update", headers=get_auth_headers(non_member_user.token), json={"is_public": True}, ) @@ -2198,10 +1170,9 @@ async def test_global_admin_can_update_any_project_visibility( # Global admin should be able to update any project's visibility response = await client.post( - f"/api/projects/{project.name}/update_visibility", + f"/api/projects/{project.name}/update", headers=get_auth_headers(global_admin.token), json={"is_public": True}, ) assert response.status_code == 200 assert response.json()["is_public"] == True ->>>>>>> a6246f4d (Add project visibility update API endpoint) From 9d2dd98a44a3e3a815f73a9c558c6e08f162e78e Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 25 Jun 2025 10:14:13 -0400 Subject: [PATCH 14/15] Remove redundant comments that duplicate code logic --- .../_internal/server/security/permissions.py | 7 ---- .../_internal/server/services/projects.py | 36 +++---------------- src/dstack/_internal/server/services/users.py | 1 - 3 files changed, 4 insertions(+), 40 deletions(-) diff --git a/src/dstack/_internal/server/security/permissions.py b/src/dstack/_internal/server/security/permissions.py index 9efc9146ed..0ecddf1d9e 100644 --- a/src/dstack/_internal/server/security/permissions.py +++ b/src/dstack/_internal/server/security/permissions.py @@ -85,11 +85,9 @@ 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 and admins can manage projects project_role = get_user_project_role(user=user, project=project) if project_role in [ProjectRole.ADMIN, ProjectRole.MANAGER]: return user, project @@ -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 @@ -203,11 +198,9 @@ 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 - # 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 diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index b823f5456f..6310293925 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -157,7 +157,6 @@ async def update_project( is_public: bool, ): """Update project visibility (public/private).""" - # Update the project visibility project.is_public = is_public await session.commit() @@ -175,7 +174,6 @@ async def delete_projects( for project_name in projects_names: if project_name not in user_project_names: raise ForbiddenError() - # Only check admin permissions for projects being deleted projects_to_delete = [p for p in user_projects if p.name in projects_names] for project in projects_to_delete: if not _is_project_admin(user=user, project=project): @@ -201,7 +199,6 @@ async def set_project_members( project: ProjectModel, members: List[MemberSetting], ): - # reload with members project = await get_project_model_by_name_or_error( session=session, project_name=project.name, @@ -226,7 +223,6 @@ async def set_project_members( select(UserModel).where((UserModel.name.in_(names)) | (UserModel.email.in_(names))) ) users = res.scalars().all() - # Create lookup maps for both username and email username_to_user = {user.name: user for user in users} email_to_user = {user.email: user for user in users if user.email} for i, member in enumerate(members): @@ -251,7 +247,6 @@ async def add_project_members( members: List[MemberSetting], ): """Add multiple members to a project.""" - # reload with members project = await get_project_model_by_name_or_error( session=session, project_name=project.name, @@ -263,7 +258,7 @@ async def add_project_members( len(members) == 1 and project.is_public and (members[0].username == user.name or members[0].username == user.email) - and requesting_user_role is None # User is not already a member + and requesting_user_role is None ) # Check permissions: only managers/admins can add members, EXCEPT for self-join to public projects @@ -283,23 +278,18 @@ async def add_project_members( if members[0].project_role != ProjectRole.USER: raise ForbiddenError("Access denied: can only join public projects as user role") - # Collect all usernames to query usernames = [member.username for member in members] - # Find all users (by username or email) res = await session.execute( select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) ) users_found = res.scalars().all() - # Create lookup maps for both username and email username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - # Build a map from user_id to member for efficient existing member updates member_by_user_id = {m.user_id: m for m in project.members} - # Process each member to add for member_setting in members: user_to_add = username_to_user.get(member_setting.username) or email_to_user.get( member_setting.username @@ -307,24 +297,20 @@ async def add_project_members( if user_to_add is None: raise ServerClientError(f"User not found: {member_setting.username}") - # Check if user is already a member if user_to_add.id in member_by_user_id: - # Update existing member role if different existing_member = member_by_user_id[user_to_add.id] if existing_member.project_role != member_setting.project_role: existing_member.project_role = member_setting.project_role else: - # Add new member (let database handle member_num to avoid race conditions) await add_project_member( session=session, project=project, user=user_to_add, project_role=member_setting.project_role, - member_num=None, # Let database auto-assign to avoid race conditions + member_num=None, commit=False, ) - # Update our local tracking for subsequent iterations - member_by_user_id[user_to_add.id] = None # Placeholder to track addition + member_by_user_id[user_to_add.id] = None await session.commit() @@ -596,11 +582,9 @@ def _is_project_admin( user: UserModel, project: ProjectModel, ) -> bool: - # Check if user is the project owner if user.id == project.owner_id: return True - # Check if user has admin role in project members for m in project.members: if user.id == m.user_id: if m.project_role == ProjectRole.ADMIN: @@ -615,7 +599,6 @@ async def remove_project_members( usernames: List[str], ): """Remove multiple members from a project.""" - # reload with members project = await get_project_model_by_name_or_error( session=session, project_name=project.name, @@ -626,7 +609,7 @@ async def remove_project_members( is_self_leave = ( len(usernames) == 1 and (usernames[0] == user.name or usernames[0] == user.email) - and requesting_user_role is not None # User is actually a member + and requesting_user_role is not None ) # Check basic permissions: only managers/admins can remove members, EXCEPT for self-leave @@ -634,20 +617,16 @@ async def remove_project_members( if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: raise ForbiddenError("Access denied: insufficient permissions to remove members") - # Find all users to remove (by username or email) res = await session.execute( select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) ) users_found = res.scalars().all() - # Create lookup maps username_to_user = {user.name: user for user in users_found} email_to_user = {user.email: user for user in users_found if user.email} - # Build a map from user_id to member for efficient member lookups member_by_user_id = {m.user_id: m for m in project.members} - # Find members to remove and validate permissions members_to_remove = [] admin_removals = 0 @@ -656,24 +635,19 @@ async def remove_project_members( if user_to_remove is None: raise ServerClientError(f"User not found: {username}") - # Check if user is actually a member before trying to remove them if user_to_remove.id not in member_by_user_id: raise ServerClientError(f"User is not a member of this project: {username}") - # Get the member to remove member_to_remove = member_by_user_id[user_to_remove.id] - # Check if trying to remove project admin if member_to_remove.project_role == ProjectRole.ADMIN: if is_self_leave: - # For self-leave, check if user is the last admin total_admins = sum( 1 for member in project.members if member.project_role == ProjectRole.ADMIN ) if total_admins <= 1: raise ServerClientError("Cannot leave project: you are the last admin") else: - # For manager/admin removing other admins, only global admins can do this if user.global_role != GlobalRole.ADMIN: raise ForbiddenError( f"Access denied: only global admins can remove project admins (user: {username})" @@ -682,7 +656,6 @@ async def remove_project_members( members_to_remove.append(member_to_remove) - # Check we're not removing all admins (for non-self-leave operations) if not is_self_leave: total_admins = sum( 1 for member in project.members if member.project_role == ProjectRole.ADMIN @@ -690,7 +663,6 @@ async def remove_project_members( if admin_removals >= total_admins: raise ServerClientError("Cannot remove all project admins") - # Remove all members for member in members_to_remove: await session.delete(member) diff --git a/src/dstack/_internal/server/services/users.py b/src/dstack/_internal/server/services/users.py index 6a4c7edea3..7aaf4b9799 100644 --- a/src/dstack/_internal/server/services/users.py +++ b/src/dstack/_internal/server/services/users.py @@ -44,7 +44,6 @@ async def list_users_for_user( session: AsyncSession, user: UserModel, ) -> List[User]: - # Allow all authenticated users to see the user list for project collaboration return await list_all_users(session=session) From 256a721f77c0c621487773ae6162c2a2ea345d81 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 25 Jun 2025 10:48:49 -0400 Subject: [PATCH 15/15] Remove only redundant comments added in PR2 --- src/dstack/_internal/server/services/projects.py | 6 ------ src/dstack/api/server/_projects.py | 2 -- 2 files changed, 8 deletions(-) diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 6310293925..1e46979dcf 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -253,7 +253,6 @@ async def add_project_members( ) requesting_user_role = get_user_project_role(user=user, project=project) - # Check if this is a self-join to public project is_self_join_to_public = ( len(members) == 1 and project.is_public @@ -261,12 +260,10 @@ async def add_project_members( and requesting_user_role is None ) - # Check permissions: only managers/admins can add members, EXCEPT for self-join to public projects if not is_self_join_to_public: if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: raise ForbiddenError("Access denied: insufficient permissions to add members") - # For project managers, check if they're trying to add admins if user.global_role != GlobalRole.ADMIN and requesting_user_role == ProjectRole.MANAGER: for member in members: if member.project_role == ProjectRole.ADMIN: @@ -274,7 +271,6 @@ async def add_project_members( "Access denied: only global admins can add project admins" ) else: - # For self-join to public project, only allow USER role if members[0].project_role != ProjectRole.USER: raise ForbiddenError("Access denied: can only join public projects as user role") @@ -605,14 +601,12 @@ async def remove_project_members( ) requesting_user_role = get_user_project_role(user=user, project=project) - # Check if this is a self-leave (user removing themselves) is_self_leave = ( len(usernames) == 1 and (usernames[0] == user.name or usernames[0] == user.email) and requesting_user_role is not None ) - # Check basic permissions: only managers/admins can remove members, EXCEPT for self-leave if not is_self_leave: if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: raise ForbiddenError("Access denied: insufficient permissions to remove members") diff --git a/src/dstack/api/server/_projects.py b/src/dstack/api/server/_projects.py index 47a0c37d70..0fb47c9ab5 100644 --- a/src/dstack/api/server/_projects.py +++ b/src/dstack/api/server/_projects.py @@ -39,7 +39,6 @@ def set_members(self, project_name: str, members: List[MemberSetting]) -> Projec return parse_obj_as(Project.__response__, resp.json()) def add_member(self, project_name: str, username: str, project_role: ProjectRole) -> Project: - # Convert single user to list format member_setting = MemberSetting(username=username, project_role=project_role) body = AddProjectMemberRequest(members=[member_setting]) resp = self._request(f"/api/projects/{project_name}/add_members", body=body.json()) @@ -51,7 +50,6 @@ def add_members(self, project_name: str, members: List[MemberSetting]) -> Projec return parse_obj_as(Project.__response__, resp.json()) def remove_member(self, project_name: str, username: str) -> Project: - # Convert single username to list format body = RemoveProjectMemberRequest(usernames=[username]) resp = self._request(f"/api/projects/{project_name}/remove_members", body=body.json()) return parse_obj_as(Project.__response__, resp.json())