From 265647bf8a063e7fd12886ae3fdb96534197ae8f Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Tue, 5 Aug 2025 16:16:56 +0500 Subject: [PATCH 1/2] Dissalow adding member twice --- .../_internal/server/services/projects.py | 20 +++++-- .../_internal/server/routers/test_projects.py | 60 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/dstack/_internal/server/services/projects.py b/src/dstack/_internal/server/services/projects.py index 0a245690dd..992e1be046 100644 --- a/src/dstack/_internal/server/services/projects.py +++ b/src/dstack/_internal/server/services/projects.py @@ -197,6 +197,10 @@ async def set_project_members( project: ProjectModel, members: List[MemberSetting], ): + usernames = {m.username for m in members} + if len(usernames) != len(members): + raise ServerClientError("Cannot add same user multiple times") + project = await get_project_model_by_name_or_error( session=session, project_name=project.name, @@ -245,6 +249,10 @@ async def add_project_members( members: List[MemberSetting], ): """Add multiple members to a project.""" + usernames = {m.username for m in members} + if len(usernames) != len(members): + raise ServerClientError("Cannot add same user multiple times") + project = await get_project_model_by_name_or_error( session=session, project_name=project.name, @@ -259,7 +267,10 @@ async def add_project_members( ) if not is_self_join_to_public: - if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + if user.global_role != GlobalRole.ADMIN and requesting_user_role not in [ + ProjectRole.ADMIN, + ProjectRole.MANAGER, + ]: raise ForbiddenError("Access denied: insufficient permissions to add members") if user.global_role != GlobalRole.ADMIN and requesting_user_role == ProjectRole.MANAGER: @@ -272,8 +283,6 @@ async def add_project_members( if members[0].project_role != ProjectRole.USER: raise ForbiddenError("Access denied: can only join public projects as user role") - usernames = [member.username for member in members] - res = await session.execute( select(UserModel).where((UserModel.name.in_(usernames)) | (UserModel.email.in_(usernames))) ) @@ -628,7 +637,10 @@ async def remove_project_members( ) if not is_self_leave: - if requesting_user_role not in [ProjectRole.ADMIN, ProjectRole.MANAGER]: + if user.global_role != GlobalRole.ADMIN and requesting_user_role not in [ + ProjectRole.ADMIN, + ProjectRole.MANAGER, + ]: raise ForbiddenError("Access denied: insufficient permissions to remove members") res = await session.execute( diff --git a/src/tests/_internal/server/routers/test_projects.py b/src/tests/_internal/server/routers/test_projects.py index b0364b4235..d53e8a84fe 100644 --- a/src/tests/_internal/server/routers/test_projects.py +++ b/src/tests/_internal/server/routers/test_projects.py @@ -989,6 +989,37 @@ async def test_global_admin_manager_can_set_project_admins( members = res.scalars().all() assert len(members) == 2 + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_cannot_set_same_user_twice( + self, test_db, session: AsyncSession, client: AsyncClient + ): + project = await create_project(session=session) + user = await create_user(session=session, global_role=GlobalRole.ADMIN) + user1 = await create_user(session=session, name="user1") + members = [ + { + "username": user1.name, + "project_role": ProjectRole.ADMIN, + }, + { + "username": user1.name, + "project_role": ProjectRole.ADMIN, + }, + ] + body = {"members": members} + response = await client.post( + f"/api/projects/{project.name}/set_members", + headers=get_auth_headers(user.token), + json=body, + ) + assert response.status_code == 400 + res = await session.execute(select(MemberModel)) + members = res.scalars().all() + assert len(members) == 0 + + +class TestAddProjectMembers: @pytest.mark.asyncio @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) async def test_add_member_errors_on_nonexistent_user( @@ -1053,6 +1084,35 @@ async def test_add_member_manager_cannot_add_admin_without_global_admin( assert response.status_code == 403 + @pytest.mark.asyncio + @pytest.mark.parametrize("test_db", ["sqlite", "postgres"], indirect=True) + async def test_cannot_add_same_user_twice( + self, test_db, session: AsyncSession, client: AsyncClient + ): + project = await create_project(session=session) + user = await create_user(session=session, global_role=GlobalRole.ADMIN) + user1 = await create_user(session=session, name="user1") + members = [ + { + "username": user1.name, + "project_role": ProjectRole.ADMIN, + }, + { + "username": user1.name, + "project_role": ProjectRole.ADMIN, + }, + ] + body = {"members": members} + response = await client.post( + f"/api/projects/{project.name}/add_members", + headers=get_auth_headers(user.token), + json=body, + ) + assert response.status_code == 400, response.json() + res = await session.execute(select(MemberModel)) + members = res.scalars().all() + assert len(members) == 0 + class TestUpdateProjectVisibility: @pytest.mark.asyncio From 357dfd654037eb5360ff21d7168085f888be944d Mon Sep 17 00:00:00 2001 From: Victor Skvortsov Date: Tue, 5 Aug 2025 16:28:16 +0500 Subject: [PATCH 2/2] Add UI check for duplicate members --- frontend/src/pages/Project/Members/index.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frontend/src/pages/Project/Members/index.tsx b/frontend/src/pages/Project/Members/index.tsx index 8009f9fb67..007c64b9d7 100644 --- a/frontend/src/pages/Project/Members/index.tsx +++ b/frontend/src/pages/Project/Members/index.tsx @@ -14,7 +14,7 @@ import { Table, } from 'components'; -import { useAppSelector, useCollection } from 'hooks'; +import { useAppSelector, useCollection, useNotifications } from 'hooks'; import { ROUTES } from 'routes'; import { useGetUserListQuery } from 'services/user'; @@ -35,6 +35,7 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r const { data: usersData } = useGetUserListQuery(); const userData = useAppSelector(selectUserData); const { handleJoinProject, handleLeaveProject, isMemberActionLoading } = useProjectMemberActions(); + const [pushNotification] = useNotifications(); const { handleSubmit, control, getValues, setValue } = useForm({ defaultValues: { members: members ?? [] }, @@ -84,6 +85,17 @@ export const ProjectMembers: React.FC = ({ members, loading, onChange, r ]; const addMemberHandler = (username: string) => { + const existingMembers = getValues('members'); + const isDuplicate = existingMembers.some((member) => member.user.username === username); + + if (isDuplicate) { + pushNotification({ + type: 'error', + content: `User "${username}" is already a member of this project`, + }); + return; + } + const selectedUser = usersData?.find((u) => u.username === username); if (selectedUser) {