Disallow duplicate project members (#2945)

* Dissalow adding member twice

* Add UI check for duplicate members

Author: r4victor

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<IProps> = ({ 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<TFormValues>({
         defaultValues: { members: members ?? [] },
@@ -84,6 +85,17 @@ export const ProjectMembers: React.FC<IProps> = ({ 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) {

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(

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