Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion frontend/src/pages/Project/Members/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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 ?? [] },
Expand Down Expand Up @@ -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) {
Expand Down
20 changes: 16 additions & 4 deletions src/dstack/_internal/server/services/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand All @@ -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)))
)
Expand Down Expand Up @@ -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(
Expand Down
60 changes: 60 additions & 0 deletions src/tests/_internal/server/routers/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
Loading