From 8957771c07cd32f82546843f4d19c9fc1be15d5e Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 14:29:16 -0400 Subject: [PATCH 01/29] 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 ef9e0d864b72981612f54e2e3ece3575f649916c Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 15:50:33 -0400 Subject: [PATCH 02/29] 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 f91bf8602fd4ccd35bd4c32090c00ee778627aee Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 15:54:27 -0400 Subject: [PATCH 03/29] 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 f83e3c9c9ff30c1f77202f899bee333c53b80496 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 10 Jun 2025 17:12:36 -0400 Subject: [PATCH 04/29] 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 37fe42ce712be0c202d8beaf7c60549b18cc629b Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 11 Jun 2025 14:17:05 -0400 Subject: [PATCH 05/29] 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 846eb093ba3a0a3954eb41eeff7d3ac2dbded581 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 11 Jun 2025 14:57:26 -0400 Subject: [PATCH 06/29] 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 3d0ea5b7f3f5f3447993666b5840b2e13e15eef7 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:18:35 -0400 Subject: [PATCH 07/29] 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 52d9212a0df51d67c029c579cd9ec680b156243b Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:29:14 -0400 Subject: [PATCH 08/29] 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 e51598164151ccad163a5df1aae5fe18b5d2d6b5 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:42:53 -0400 Subject: [PATCH 09/29] 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 e3f6ea0575d192eec1df89fcab35e5b3754c9105 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 11:30:29 -0400 Subject: [PATCH 10/29] 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 36967fe26eb2e096eee271b60fe6a639efd7ae70 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Tue, 17 Jun 2025 10:27:37 -0400 Subject: [PATCH 11/29] 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 beaa33c8cd28138df59fd170f210fdb2ff6c836d Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Mon, 23 Jun 2025 10:09:18 -0400 Subject: [PATCH 12/29] 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 9e4efaf2fe76eb0c51a2e40c2cf190bdf16d77cc Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Mon, 23 Jun 2025 11:18:23 -0400 Subject: [PATCH 13/29] 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 b589cf4639fcc576f4f96091b40ff3a8d9fc65e3 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 25 Jun 2025 10:14:13 -0400 Subject: [PATCH 14/29] 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 f61f41699cbf56fb7f02246bcd834be1705c4744 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 25 Jun 2025 10:48:49 -0400 Subject: [PATCH 15/29] 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()) From 79920ee6a285a6ee613bd7693691e76d91518247 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 10:52:20 -0400 Subject: [PATCH 16/29] Add join/leave project UI functionality --- frontend/src/locale/en.json | 13 ++ .../pages/Project/Details/Settings/index.tsx | 52 ++++++- frontend/src/pages/Project/Form/index.tsx | 11 +- frontend/src/pages/Project/Members/index.tsx | 144 ++++++++++++++++-- frontend/src/pages/Project/Members/types.ts | 1 + frontend/src/router.tsx | 6 +- frontend/src/services/project.ts | 37 +++++ frontend/src/types/project.d.ts | 4 +- 8 files changed, 245 insertions(+), 23 deletions(-) diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index 7c8f8e6e05..97035dc925 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -172,6 +172,13 @@ "runs": "Runs", "tags": "Tags", "settings": "Settings", + "join": "Join Project", + "leave": "Leave Project", + "owner_cannot_leave": "Owner Cannot Leave", + "join_success": "Successfully joined the project", + "leave_success": "Successfully left the project", + "join_error": "Failed to join project", + "leave_error": "Failed to leave project", "card": { "backend": "Backend", "settings": "Settings" @@ -181,6 +188,8 @@ "project_name": "Project name", "owner": "Owner", "project_name_description": "Only latin characters, dashes, underscores, and digits", + "is_public": "Make project public", + "is_public_description": "Public projects can be accessed by any user without being a member", "backend": "Backend", "backend_config": "Backend config", "backend_config_description": "Specify the backend config in the YAML format. Click Info for examples.", @@ -189,6 +198,10 @@ "members_empty_message_title": "No members", "members_empty_message_text": "Select project's members", "update_members_success": "Members are updated", + "update_visibility_success": "Project visibility updated successfully", + "project_visibility": "Project Visibility", + "project_visibility_description": "Control who can access this project", + "make_project_public": "Make project public", "delete_project_confirm_title": "Delete project", "delete_project_confirm_message": "Are you sure you want to delete this project?", "delete_projects_confirm_title": "Delete projects", diff --git a/frontend/src/pages/Project/Details/Settings/index.tsx b/frontend/src/pages/Project/Details/Settings/index.tsx index 475f40b96c..d2cad1c902 100644 --- a/frontend/src/pages/Project/Details/Settings/index.tsx +++ b/frontend/src/pages/Project/Details/Settings/index.tsx @@ -17,18 +17,21 @@ import { SelectCSD, SpaceBetween, StatusIndicator, + Toggle, } from 'components'; import { HotspotIds } from 'layouts/AppLayout/TutorialPanel/constants'; import { useBreadcrumbs, useHelpPanel, useNotifications } from 'hooks'; import { riseRouterException } from 'libs'; import { ROUTES } from 'routes'; -import { useGetProjectQuery, useUpdateProjectMembersMutation } from 'services/project'; +import { useGetProjectQuery, useUpdateProjectMembersMutation, useUpdateProjectVisibilityMutation } from 'services/project'; import { useCheckAvailableProjectPermission } from 'pages/Project/hooks/useCheckAvailableProjectPermission'; import { useConfigProjectCliCommand } from 'pages/Project/hooks/useConfigProjectCliComand'; import { useDeleteProject } from 'pages/Project/hooks/useDeleteProject'; import { ProjectMembers } from 'pages/Project/Members'; +import { getProjectRoleByUserName } from 'pages/Project/utils'; +import { useGetUserDataQuery } from 'services/user'; import { useBackendsTable } from '../../Backends/hooks'; import { BackendsTable } from '../../Backends/Table'; @@ -51,7 +54,9 @@ export const ProjectSettings: React.FC = () => { const [pushNotification] = useNotifications(); const [updateProjectMembers] = useUpdateProjectMembersMutation(); + const [updateProjectVisibility] = useUpdateProjectVisibilityMutation(); const { deleteProject, isDeleting } = useDeleteProject(); + const { data: currentUser } = useGetUserDataQuery({}); const { data, isLoading, error } = useGetProjectQuery({ name: paramProjectName }); @@ -113,6 +118,26 @@ export const ProjectSettings: React.FC = () => { const debouncedMembersHandler = useCallback(debounce(changeMembersHandler, 1000), []); + const changeVisibilityHandler = (is_public: boolean) => { + updateProjectVisibility({ + project_name: paramProjectName, + is_public, + }) + .unwrap() + .then(() => { + pushNotification({ + type: 'success', + content: t('projects.edit.update_visibility_success'), + }); + }) + .catch((error) => { + pushNotification({ + type: 'error', + content: t('common.server_error', { error: error?.data?.detail?.msg }), + }); + }); + }; + const isDisabledButtons = useMemo(() => { return isDeleting || !data || !isAvailableDeletingPermission(data); }, [data, isDeleting, isAvailableDeletingPermission]); @@ -120,7 +145,12 @@ export const ProjectSettings: React.FC = () => { const deleteProjectHandler = () => { if (!data) return; - deleteProject(data).then(() => navigate(ROUTES.PROJECT.LIST)); + deleteProject(data) + .then(() => navigate(ROUTES.PROJECT.LIST)) + .catch((error) => { + // Error is already handled in useDeleteProject hook + console.error('Delete project failed:', error); + }); }; if (isLoadingPage) @@ -185,11 +215,29 @@ export const ProjectSettings: React.FC = () => { + {isAvailableProjectManaging && ( + {t('projects.edit.project_visibility')}}> + + + {t('projects.edit.project_visibility_description')} + + changeVisibilityHandler(detail.detail.checked)} + disabled={!isProjectManager(data)} + > + {t('projects.edit.make_project_public')} + + + + )} + {t('common.danger_zone')}}> diff --git a/frontend/src/pages/Project/Form/index.tsx b/frontend/src/pages/Project/Form/index.tsx index 5103676c8b..8edf454b28 100644 --- a/frontend/src/pages/Project/Form/index.tsx +++ b/frontend/src/pages/Project/Form/index.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; -import { Button, Container, FormInput, FormUI, Header, SpaceBetween } from 'components'; +import { Button, Container, FormCheckbox, FormInput, FormUI, Header, SpaceBetween } from 'components'; import { useNotifications } from 'hooks'; import { isResponseServerError, isResponseServerFormFieldError } from 'libs'; @@ -16,6 +16,7 @@ export const ProjectForm: React.FC = ({ initialValues, onCancel, loading const formMethods = useForm({ defaultValues: { + is_public: false, ...initialValues, }, }); @@ -81,6 +82,14 @@ export const ProjectForm: React.FC = ({ initialValues, onCancel, loading }, }} /> + + diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index b728d1962f..0b388f7f5e 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -1,12 +1,15 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import { useFieldArray, useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; +import { useNavigate } from 'react-router-dom'; -import { Button, FormSelect, Header, Link, ListEmptyMessage, Pagination, Table } from 'components'; +import { Button, FormSelect, Header, Link, ListEmptyMessage, Pagination, SpaceBetween, Table } from 'components'; -import { useCollection } from 'hooks'; +import { useAppSelector, useCollection, useNotifications } from 'hooks'; +import { selectUserData } from 'App/slice'; import { ROUTES } from 'routes'; import { useGetUserListQuery } from 'services/user'; +import { useAddProjectMemberMutation, useRemoveProjectMemberMutation } from 'services/project'; import { UserAutosuggest } from './UsersAutosuggest'; @@ -16,10 +19,15 @@ import { TRoleSelectOption } from 'pages/User/Form/types'; import styles from './styles.module.scss'; -export const ProjectMembers: React.FC = ({ members, loading, onChange, readonly, isAdmin }) => { +export const ProjectMembers: React.FC = ({ members, loading, onChange, readonly, isAdmin, project }) => { const { t } = useTranslation(); + const navigate = useNavigate(); + const [pushNotification] = useNotifications(); const [selectedItems, setSelectedItems] = useState([]); const { data: usersData } = useGetUserListQuery(); + const userData = useAppSelector(selectUserData); + const [addMember, { isLoading: isAdding }] = useAddProjectMemberMutation(); + const [removeMember, { isLoading: isRemoving }] = useRemoveProjectMemberMutation(); const { handleSubmit, control, getValues, setValue } = useForm({ defaultValues: { members: members ?? [] }, @@ -30,6 +38,67 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r name: 'members', }); + const currentUserRole = useMemo(() => { + if (!userData?.username) return null; + const member = members?.find(m => m.user.username === userData.username); + return member?.project_role || null; + }, [members, userData?.username]); + + const isProjectOwner = useMemo(() => { + return userData?.username === project?.owner.username; + }, [userData?.username, project?.owner.username]); + + const isMember = currentUserRole !== null; + const isMemberActionLoading = isAdding || isRemoving; + + const handleJoinProject = async () => { + if (!userData?.username || !project) return; + + try { + await addMember({ + project_name: project.project_name, + username: userData.username, + project_role: 'user', + }).unwrap(); + + pushNotification({ + type: 'success', + content: t('projects.join_success'), + }); + } catch (error) { + console.error('Failed to join project:', error); + pushNotification({ + type: 'error', + content: t('projects.join_error'), + }); + } + }; + + const handleLeaveProject = async () => { + if (!userData?.username || !project) return; + + try { + await removeMember({ + project_name: project.project_name, + username: userData.username, + }).unwrap(); + + pushNotification({ + type: 'success', + content: t('projects.leave_success'), + }); + + // Redirect to project list after successfully leaving + navigate(ROUTES.PROJECT.LIST); + } catch (error) { + console.error('Failed to leave project:', error); + pushNotification({ + type: 'error', + content: t('projects.leave_error'), + }); + } + }; + useEffect(() => { if (members) { setValue('members', members); @@ -60,7 +129,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r { label: t('roles.user'), value: 'user' }, ]; - const addMember = (username: string) => { + const addMemberHandler = (username: string) => { const selectedUser = usersData?.find((u) => u.username === username); if (selectedUser) { @@ -90,6 +159,61 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r onChangeHandler(); }; + const renderMemberActions = () => { + const actions = []; + + // Add management actions only if not readonly + if (!readonly) { + actions.push( + + ); + } + + // Add join/leave button if user is authenticated (available even in readonly mode) + if (userData?.username && project) { + if (!isMember) { + actions.unshift( + + ); + } else { + // Prevent owners and admins from leaving their projects + const canLeave = !isProjectOwner && currentUserRole !== 'admin'; + + actions.unshift( + + ); + } + } + + return actions.length > 0 ? {actions} : undefined; + }; + const COLUMN_DEFINITIONS = [ { id: 'name', @@ -153,13 +277,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r
- {t('common.delete')} - - ) - } + actions={renderMemberActions()} > {t('projects.edit.members.section_title')}
@@ -168,7 +286,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r readonly ? undefined : ( addMember(detail.value)} + onSelect={({ detail }) => addMemberHandler(detail.value)} optionsFilter={(options) => options.filter((o) => !fields.find((f) => f.user.username === o.value))} /> ) diff --git a/frontend/src/pages/Project/Members/types.ts b/frontend/src/pages/Project/Members/types.ts index 6b969d24ec..282791f850 100644 --- a/frontend/src/pages/Project/Members/types.ts +++ b/frontend/src/pages/Project/Members/types.ts @@ -4,6 +4,7 @@ export interface IProps { onChange: (users: IProjectMember[]) => void; readonly?: boolean; isAdmin?: boolean; + project?: IProject; } export type TProjectMemberWithIndex = IProjectMember & { index: number }; diff --git a/frontend/src/router.tsx b/frontend/src/router.tsx index 8812f71996..a953cbc6b3 100644 --- a/frontend/src/router.tsx +++ b/frontend/src/router.tsx @@ -63,25 +63,21 @@ export const router = createBrowserRouter([ element: , children: [ { - path: ROUTES.PROJECT.DETAILS.SETTINGS.TEMPLATE, + index: true, element: , }, - { path: ROUTES.PROJECT.BACKEND.ADD.TEMPLATE, element: , }, - { path: ROUTES.PROJECT.BACKEND.EDIT.TEMPLATE, element: , }, - { path: ROUTES.PROJECT.GATEWAY.ADD.TEMPLATE, element: , }, - { path: ROUTES.PROJECT.GATEWAY.EDIT.TEMPLATE, element: , diff --git a/frontend/src/services/project.ts b/frontend/src/services/project.ts index 712a95f950..932e403816 100644 --- a/frontend/src/services/project.ts +++ b/frontend/src/services/project.ts @@ -61,6 +61,40 @@ export const projectApi = createApi({ invalidatesTags: (result, error, params) => [{ type: 'Projects' as const, id: params?.project_name }], }), + addProjectMember: builder.mutation({ + query: ({ project_name, username, project_role = 'user' }) => ({ + url: API.PROJECTS.ADD_MEMBERS(project_name), + method: 'POST', + body: { + members: [{ username, project_role }], + }, + }), + + invalidatesTags: (result, error, params) => [{ type: 'Projects' as const, id: params?.project_name }], + }), + + removeProjectMember: builder.mutation({ + query: ({ project_name, username }) => ({ + url: API.PROJECTS.REMOVE_MEMBERS(project_name), + method: 'POST', + body: { + usernames: [username], + }, + }), + + invalidatesTags: (result, error, params) => [{ type: 'Projects' as const, id: params?.project_name }], + }), + + updateProjectVisibility: builder.mutation({ + query: ({ project_name, is_public }) => ({ + url: API.PROJECTS.UPDATE_VISIBILITY(project_name), + method: 'POST', + body: { is_public }, + }), + + invalidatesTags: (result, error, params) => [{ type: 'Projects' as const, id: params?.project_name }], + }), + deleteProjects: builder.mutation({ query: (projectNames) => ({ url: API.PROJECTS.DELETE(), @@ -109,6 +143,9 @@ export const { useGetProjectQuery, useCreateProjectMutation, useUpdateProjectMembersMutation, + useAddProjectMemberMutation, + useRemoveProjectMemberMutation, + useUpdateProjectVisibilityMutation, useDeleteProjectsMutation, useGetProjectLogsQuery, useGetProjectReposQuery, diff --git a/frontend/src/types/project.d.ts b/frontend/src/types/project.d.ts index 4418fa8810..41ceeafbf1 100644 --- a/frontend/src/types/project.d.ts +++ b/frontend/src/types/project.d.ts @@ -1,4 +1,3 @@ - declare type TProjectBackend = { name: string, config: IBackendAWS | IBackendAzure | IBackendGCP | IBackendLambda | IBackendLocal | IBackendDstack @@ -8,7 +7,8 @@ declare interface IProject { members: IProjectMember[], backends: TProjectBackend[] owner: IUser | {username: string}, - created_at: string + created_at: string, + is_public: boolean } declare interface IProjectMember { From 6df964e9be2c8f4eb19572bb9395cc68d15f3b30 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Thu, 12 Jun 2025 13:54:34 -0400 Subject: [PATCH 17/29] Add Join/Leave button to project header and restrict CLI section to members --- .../pages/Project/Details/Settings/index.tsx | 82 +++++++----- frontend/src/pages/Project/Details/index.tsx | 125 +++++++++++++++++- 2 files changed, 167 insertions(+), 40 deletions(-) diff --git a/frontend/src/pages/Project/Details/Settings/index.tsx b/frontend/src/pages/Project/Details/Settings/index.tsx index d2cad1c902..ab8136f061 100644 --- a/frontend/src/pages/Project/Details/Settings/index.tsx +++ b/frontend/src/pages/Project/Details/Settings/index.tsx @@ -61,13 +61,21 @@ export const ProjectSettings: React.FC = () => { const { data, isLoading, error } = useGetProjectQuery({ name: paramProjectName }); useEffect(() => { + // Only throw router exception for actual 404 errors, not permission errors + // For public projects, non-members should still be able to view project details // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore if (error?.status === 404) { riseRouterException(); } + // Don't throw exceptions for other errors (like 403) as they might be permission-related + // but the project might still be viewable if it's public }, [error]); + // Check if current user is a member of the project + const currentUserRole = data ? getProjectRoleByUserName(data, currentUser?.username ?? '') : null; + const isProjectMember = currentUserRole !== null; + const currentOwner = { label: data?.owner.username, value: data?.owner.username, @@ -164,42 +172,44 @@ export const ProjectSettings: React.FC = () => { <> {data && backendsData && gatewaysData && ( - openHelpPanel(CLI_INFO)} />}> - {t('projects.edit.cli')} - - } - > - - - Run the following commands to set up the CLI for this project - - -
- - {configCliCommand} - -
- {t('common.copied')}} - > -
- -
- - + {isProjectMember && ( + openHelpPanel(CLI_INFO)} />}> + {t('projects.edit.cli')} + + } + > + + + Run the following commands to set up the CLI for this project + + +
+ + {configCliCommand} + +
+ {t('common.copied')}} + > +
+ +
+ + + )} { + const { t } = useTranslation(); const params = useParams(); + const navigate = useNavigate(); const paramProjectName = params.projectName ?? ''; + const [pushNotification] = useNotifications(); + const userData = useAppSelector(selectUserData); + + const { data: project } = useGetProjectQuery({ name: paramProjectName }); + const [addMember, { isLoading: isAdding }] = useAddProjectMemberMutation(); + const [removeMember, { isLoading: isRemoving }] = useRemoveProjectMemberMutation(); + + const currentUserRole = useMemo(() => { + if (!userData?.username || !project) return null; + return getProjectRoleByUserName(project, userData.username); + }, [project, userData?.username]); + + const isProjectOwner = useMemo(() => { + return userData?.username === project?.owner.username; + }, [userData?.username, project?.owner.username]); + + const isMember = currentUserRole !== null; + const isMemberActionLoading = isAdding || isRemoving; + + const handleJoinProject = async () => { + if (!userData?.username || !project) return; + + try { + await addMember({ + project_name: project.project_name, + username: userData.username, + project_role: 'user', + }).unwrap(); + + pushNotification({ + type: 'success', + content: t('projects.join_success'), + }); + } catch (error) { + console.error('Failed to join project:', error); + pushNotification({ + type: 'error', + content: t('projects.join_error'), + }); + } + }; + + const handleLeaveProject = async () => { + if (!userData?.username || !project) return; + + try { + await removeMember({ + project_name: project.project_name, + username: userData.username, + }).unwrap(); + + pushNotification({ + type: 'success', + content: t('projects.leave_success'), + }); + + // Redirect to project list after successfully leaving + navigate(ROUTES.PROJECT.LIST); + } catch (error) { + console.error('Failed to leave project:', error); + pushNotification({ + type: 'error', + content: t('projects.leave_error'), + }); + } + }; + + const renderJoinLeaveButton = () => { + // Only show button if user is authenticated and project is loaded + if (!userData?.username || !project) return null; + + if (!isMember) { + return ( + + ); + } else { + // Prevent owners and admins from leaving their projects + const canLeave = !isProjectOwner && currentUserRole !== 'admin'; + + return ( + + ); + } + }; return ( - }> + + } + > ); From 8be83d4f25831b53a9459a3b8b72dbc503cdaa4a Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Wed, 18 Jun 2025 11:37:59 -0400 Subject: [PATCH 18/29] refactor: improve code style and extract member actions hook --- .../pages/Project/Details/Settings/index.tsx | 6 +- frontend/src/pages/Project/Details/index.tsx | 4 +- frontend/src/pages/Project/Form/index.tsx | 4 +- frontend/src/pages/Project/Members/index.tsx | 62 ++-------------- .../Project/hooks/useProjectMemberActions.ts | 70 +++++++++++++++++++ frontend/src/types/project.d.ts | 2 +- 6 files changed, 82 insertions(+), 66 deletions(-) create mode 100644 frontend/src/pages/Project/hooks/useProjectMemberActions.ts diff --git a/frontend/src/pages/Project/Details/Settings/index.tsx b/frontend/src/pages/Project/Details/Settings/index.tsx index ab8136f061..90f034ba42 100644 --- a/frontend/src/pages/Project/Details/Settings/index.tsx +++ b/frontend/src/pages/Project/Details/Settings/index.tsx @@ -126,10 +126,10 @@ export const ProjectSettings: React.FC = () => { const debouncedMembersHandler = useCallback(debounce(changeMembersHandler, 1000), []); - const changeVisibilityHandler = (is_public: boolean) => { + const changeVisibilityHandler = (isPublic: boolean) => { updateProjectVisibility({ project_name: paramProjectName, - is_public, + is_public: isPublic, }) .unwrap() .then(() => { @@ -232,7 +232,7 @@ export const ProjectSettings: React.FC = () => { {t('projects.edit.project_visibility_description')} changeVisibilityHandler(detail.detail.checked)} disabled={!isProjectManager(data)} > diff --git a/frontend/src/pages/Project/Details/index.tsx b/frontend/src/pages/Project/Details/index.tsx index 6e9f2c28da..f17114a2d0 100644 --- a/frontend/src/pages/Project/Details/index.tsx +++ b/frontend/src/pages/Project/Details/index.tsx @@ -27,9 +27,7 @@ export const ProjectDetails: React.FC = () => { return getProjectRoleByUserName(project, userData.username); }, [project, userData?.username]); - const isProjectOwner = useMemo(() => { - return userData?.username === project?.owner.username; - }, [userData?.username, project?.owner.username]); + const isProjectOwner = userData?.username === project?.owner.username; const isMember = currentUserRole !== null; const isMemberActionLoading = isAdding || isRemoving; diff --git a/frontend/src/pages/Project/Form/index.tsx b/frontend/src/pages/Project/Form/index.tsx index 8edf454b28..faf0365ed4 100644 --- a/frontend/src/pages/Project/Form/index.tsx +++ b/frontend/src/pages/Project/Form/index.tsx @@ -16,7 +16,7 @@ export const ProjectForm: React.FC = ({ initialValues, onCancel, loading const formMethods = useForm({ defaultValues: { - is_public: false, + isPublic: false, ...initialValues, }, }); @@ -87,7 +87,7 @@ export const ProjectForm: React.FC = ({ initialValues, onCancel, loading label={t('projects.edit.is_public')} description={t('projects.edit.is_public_description')} control={control} - name="is_public" + name="isPublic" disabled={loading} /> diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index 0b388f7f5e..ccbe5aca87 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -9,7 +9,7 @@ import { useAppSelector, useCollection, useNotifications } from 'hooks'; import { selectUserData } from 'App/slice'; import { ROUTES } from 'routes'; import { useGetUserListQuery } from 'services/user'; -import { useAddProjectMemberMutation, useRemoveProjectMemberMutation } from 'services/project'; +import { useProjectMemberActions } from '../hooks/useProjectMemberActions'; import { UserAutosuggest } from './UsersAutosuggest'; @@ -26,8 +26,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r const [selectedItems, setSelectedItems] = useState([]); const { data: usersData } = useGetUserListQuery(); const userData = useAppSelector(selectUserData); - const [addMember, { isLoading: isAdding }] = useAddProjectMemberMutation(); - const [removeMember, { isLoading: isRemoving }] = useRemoveProjectMemberMutation(); + const { handleJoinProject, handleLeaveProject, isMemberActionLoading } = useProjectMemberActions(); const { handleSubmit, control, getValues, setValue } = useForm({ defaultValues: { members: members ?? [] }, @@ -44,60 +43,9 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r return member?.project_role || null; }, [members, userData?.username]); - const isProjectOwner = useMemo(() => { - return userData?.username === project?.owner.username; - }, [userData?.username, project?.owner.username]); + const isProjectOwner = userData?.username === project?.owner.username; const isMember = currentUserRole !== null; - const isMemberActionLoading = isAdding || isRemoving; - - const handleJoinProject = async () => { - if (!userData?.username || !project) return; - - try { - await addMember({ - project_name: project.project_name, - username: userData.username, - project_role: 'user', - }).unwrap(); - - pushNotification({ - type: 'success', - content: t('projects.join_success'), - }); - } catch (error) { - console.error('Failed to join project:', error); - pushNotification({ - type: 'error', - content: t('projects.join_error'), - }); - } - }; - - const handleLeaveProject = async () => { - if (!userData?.username || !project) return; - - try { - await removeMember({ - project_name: project.project_name, - username: userData.username, - }).unwrap(); - - pushNotification({ - type: 'success', - content: t('projects.leave_success'), - }); - - // Redirect to project list after successfully leaving - navigate(ROUTES.PROJECT.LIST); - } catch (error) { - console.error('Failed to leave project:', error); - pushNotification({ - type: 'error', - content: t('projects.leave_error'), - }); - } - }; useEffect(() => { if (members) { @@ -182,7 +130,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r actions.unshift( - ); + if (canLeave) { + return ( + + ); + } } + + return null; }; return ( diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index 549724a678..9856eb57d1 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -141,21 +141,21 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r // Prevent owners and admins from leaving their projects const canLeave = !isProjectOwner && currentUserRole !== 'admin'; - actions.unshift( - - ); + } + + ); + } } } From 863f7023792ab52c5077262cf356d0276c06f2c5 Mon Sep 17 00:00:00 2001 From: Haydn Li Date: Mon, 23 Jun 2025 13:19:33 -0400 Subject: [PATCH 27/29] Hide Leave Project button for last admin instead of showing error --- .../ButtonWithConfirmation/index.tsx | 9 ++++- .../ButtonWithConfirmation/types.ts | 1 + .../pages/Project/Details/Settings/index.tsx | 17 ++++++--- frontend/src/pages/Project/Details/index.tsx | 37 ++++++++++--------- frontend/src/pages/Project/Members/index.tsx | 8 ++-- .../Project/hooks/useProjectMemberActions.ts | 20 +++++++++- .../_internal/server/routers/projects.py | 3 +- 7 files changed, 65 insertions(+), 30 deletions(-) diff --git a/frontend/src/components/ButtonWithConfirmation/index.tsx b/frontend/src/components/ButtonWithConfirmation/index.tsx index eeaa556a06..78c2793d9c 100644 --- a/frontend/src/components/ButtonWithConfirmation/index.tsx +++ b/frontend/src/components/ButtonWithConfirmation/index.tsx @@ -6,7 +6,13 @@ import { ConfirmationDialog } from '../ConfirmationDialog'; import { IProps } from './types'; -export const ButtonWithConfirmation: React.FC = ({ confirmTitle, confirmContent, onClick, ...props }) => { +export const ButtonWithConfirmation: React.FC = ({ + confirmTitle, + confirmContent, + onClick, + confirmButtonLabel, + ...props +}) => { const [showDeleteConfirm, setShowConfirmDelete] = useState(false); const toggleDeleteConfirm = () => { @@ -31,6 +37,7 @@ export const ButtonWithConfirmation: React.FC = ({ confirmTitle, confirm onConfirm={onConfirm} title={confirmTitle} content={content} + confirmButtonLabel={confirmButtonLabel} /> ); diff --git a/frontend/src/components/ButtonWithConfirmation/types.ts b/frontend/src/components/ButtonWithConfirmation/types.ts index a303a1916e..b796b4dfbb 100644 --- a/frontend/src/components/ButtonWithConfirmation/types.ts +++ b/frontend/src/components/ButtonWithConfirmation/types.ts @@ -5,5 +5,6 @@ import type { IProps as ButtonProps } from '../Button'; export interface IProps extends Omit { confirmTitle?: string; confirmContent?: ReactNode; + confirmButtonLabel?: string; onClick?: () => void; } diff --git a/frontend/src/pages/Project/Details/Settings/index.tsx b/frontend/src/pages/Project/Details/Settings/index.tsx index 8265951663..d858249ce2 100644 --- a/frontend/src/pages/Project/Details/Settings/index.tsx +++ b/frontend/src/pages/Project/Details/Settings/index.tsx @@ -75,12 +75,18 @@ export const ProjectSettings: React.FC = () => { }; const visibilityOptions = [ - { label: t('projects.edit.visibility.private'), value: 'private' }, - { label: t('projects.edit.visibility.public'), value: 'public' }, + { label: t('projects.edit.visibility.private') || '', value: 'private' }, + { label: t('projects.edit.visibility.public') || '', value: 'public' }, ]; const currentVisibility = data?.isPublic ? 'public' : 'private'; - const selectedVisibility = visibilityOptions.find(option => option.value === currentVisibility) || visibilityOptions[0]; + const [selectedVisibility, setSelectedVisibility] = useState( + data?.isPublic ? visibilityOptions[1] : visibilityOptions[0] + ); + + useEffect(() => { + setSelectedVisibility(data?.isPublic ? visibilityOptions[1] : visibilityOptions[0]); + }, [data]); const { data: backendsData, @@ -268,8 +274,9 @@ export const ProjectSettings: React.FC = () => { variant="danger-normal" disabled={!isProjectManager(data)} formAction="none" - onClick={() => changeVisibilityHandler(!data.isPublic)} + onClick={() => changeVisibilityHandler(selectedVisibility.value === 'public')} confirmTitle={t('projects.edit.update_visibility_confirm_title')} + confirmButtonLabel={t('projects.edit.change_visibility')} confirmContent={ @@ -277,9 +284,9 @@ export const ProjectSettings: React.FC = () => {
setSelectedVisibility(event.detail.selectedOption as { label: string; value: string })} expandToViewport={true} filteringType="auto" /> diff --git a/frontend/src/pages/Project/Details/index.tsx b/frontend/src/pages/Project/Details/index.tsx index bc3673bab5..3e6ff7ffdc 100644 --- a/frontend/src/pages/Project/Details/index.tsx +++ b/frontend/src/pages/Project/Details/index.tsx @@ -45,26 +45,29 @@ export const ProjectDetails: React.FC = () => { ); } else { - // Prevent owners and admins from leaving their projects - const canLeave = !isProjectOwner && currentUserRole !== 'admin'; + // Check if user is the last admin - if so, don't show leave button + const adminCount = project.members.filter(member => member.project_role === 'admin').length; + const isLastAdmin = currentUserRole === 'admin' && adminCount <= 1; - if (canLeave) { - return ( - - ); + if (isLastAdmin) { + // Don't show leave button for the last admin + return null; } + + // Allow leaving for all other members + return ( + + ); } - - return null; }; return ( diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index 9856eb57d1..b32effbb23 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -138,10 +138,12 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r ); } else { - // Prevent owners and admins from leaving their projects - const canLeave = !isProjectOwner && currentUserRole !== 'admin'; + // Check if user is the last admin - if so, don't show leave button + const adminCount = project.members.filter(member => member.project_role === 'admin').length; + const isLastAdmin = currentUserRole === 'admin' && adminCount <= 1; - if (canLeave) { + if (!isLastAdmin) { + // Only show leave button if user is not the last admin actions.unshift( - ); - } else { - // Check if user is the last admin - if so, don't show leave button - const adminCount = project.members.filter(member => member.project_role === 'admin').length; - const isLastAdmin = currentUserRole === 'admin' && adminCount <= 1; - - if (isLastAdmin) { - // Don't show leave button for the last admin - return null; - } - - // Allow leaving for all other members - return ( - - ); - } - }; - return ( - - } - > + }> ); diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index b32effbb23..33eafc0813 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -3,7 +3,7 @@ import { useFieldArray, useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; import { useNavigate } from 'react-router-dom'; -import { Button, FormSelect, Header, Link, ListEmptyMessage, Pagination, SpaceBetween, Table } from 'components'; +import { Button, ButtonWithConfirmation, FormSelect, Header, Link, ListEmptyMessage, Pagination, SpaceBetween, Table } from 'components'; import { useAppSelector, useCollection, useNotifications } from 'hooks'; import { selectUserData } from 'App/slice'; @@ -132,7 +132,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r key="join" onClick={() => handleJoinProject(project.project_name, userData.username!)} disabled={isMemberActionLoading} - variant="primary" + variant="normal" > {isMemberActionLoading ? t('common.loading') : t('projects.join')} @@ -145,17 +145,20 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r if (!isLastAdmin) { // Only show leave button if user is not the last admin actions.unshift( - + ); } }