diff --git a/ami/users/api/views.py b/ami/users/api/views.py index aac43b5f9..bafcacaf8 100644 --- a/ami/users/api/views.py +++ b/ami/users/api/views.py @@ -15,7 +15,7 @@ UserProjectMembershipListSerializer, UserProjectMembershipSerializer, ) -from ami.users.roles import Role +from ami.users.roles import BasicMember, Role from ami.users.signals import manage_project_membership logger = logging.getLogger(__name__) @@ -50,7 +50,6 @@ def get_serializer_context(self): def perform_create(self, serializer): project = self.get_active_project() - # user = serializer._validated_user role_cls = serializer._validated_role_cls with transaction.atomic(): membership = serializer.save(project=project) @@ -60,12 +59,11 @@ def perform_create(self, serializer): # The membership is already created above, so we don't need the signal to modify it m2m_changed.disconnect(manage_project_membership, sender=Group.user_set.through) try: - # unassign all existing roles for this project + # Unassign all roles, assign the chosen role, then BasicMember for r in Role.__subclasses__(): r.unassign_user(user, project) - - # assign new role role_cls.assign_user(user, project) + BasicMember.assign_user(user, project) finally: # Reconnect signal m2m_changed.connect(manage_project_membership, sender=Group.user_set.through) @@ -86,10 +84,11 @@ def perform_update(self, serializer): membership.user = user membership.save() + # Unassign all roles, assign the chosen role, then BasicMember for r in Role.__subclasses__(): r.unassign_user(user, project) - role_cls.assign_user(user, project) + BasicMember.assign_user(user, project) finally: # Reconnect signal m2m_changed.connect(manage_project_membership, sender=Group.user_set.through) @@ -103,7 +102,7 @@ def perform_destroy(self, instance): m2m_changed.disconnect(manage_project_membership, sender=Group.user_set.through) try: with transaction.atomic(): - # remove roles for this project + # Revoke all roles (including BasicMember) before deleting membership for r in Role.__subclasses__(): r.unassign_user(user, project) diff --git a/ami/users/tests/test_membership_management_api.py b/ami/users/tests/test_membership_management_api.py index a3e9010e5..9527b8327 100644 --- a/ami/users/tests/test_membership_management_api.py +++ b/ami/users/tests/test_membership_management_api.py @@ -4,7 +4,7 @@ from ami.main.models import Project, UserProjectMembership from ami.tests.fixtures.main import setup_test_project from ami.users.models import User -from ami.users.roles import BasicMember, ProjectManager, Role +from ami.users.roles import BasicMember, Identifier, ProjectManager, Researcher, Role, create_roles_for_project class TestUserProjectMembershipAPI(APITestCase): @@ -243,3 +243,64 @@ def test_create_membership_missing_role_id(self): payload = {"email": self.user2.email} resp = self.client.post(self.members_url, payload, format="json") self.assertEqual(resp.status_code, 400) + + +class TestMembersApiDraftProjectAccess(APITestCase): + """ + Verify that members added via the Members API can access draft project details. + Regression tests for the BasicMember manual-assign fix. + """ + + def setUp(self): + self.project, _ = setup_test_project() + self.project.draft = True + self.project.save() + create_roles_for_project(self.project) + + self.superuser = User.objects.create_superuser(email="super@insectai.org", password="x") + self.user_basic = User.objects.create_user(email="basic@insectai.org") + self.user_identifier = User.objects.create_user(email="identifier@insectai.org") + self.user_researcher = User.objects.create_user(email="researcher@insectai.org") + self.user_project_manager = User.objects.create_user(email="manager@insectai.org") + self.outsider = User.objects.create_user(email="outsider@insectai.org") + + self.members_url = f"/api/v2/projects/{self.project.pk}/members/" + self.detail_url = f"/api/v2/projects/{self.project.pk}/" + + def _add_member_and_assert_can_access_draft(self, user, role_id: str) -> None: + """Add user as role via API, then assert they can GET draft project details.""" + self.client.force_authenticate(self.superuser) + resp = self.client.post( + self.members_url, + {"email": user.email, "role_id": role_id}, + format="json", + ) + self.assertEqual(resp.status_code, 201, f"Failed to add {role_id}: {resp.json()}") + self.client.force_authenticate(user) + detail_resp = self.client.get(self.detail_url) + self.assertEqual( + detail_resp.status_code, + 200, + f"{role_id} member should access draft project, got {detail_resp.status_code}", + ) + + def test_member_added_via_api_can_access_draft_project_basic_member(self): + self._add_member_and_assert_can_access_draft(self.user_basic, BasicMember.__name__) + + def test_member_added_via_api_can_access_draft_project_identifier(self): + self._add_member_and_assert_can_access_draft(self.user_identifier, Identifier.__name__) + + def test_member_added_via_api_can_access_draft_project_researcher(self): + self._add_member_and_assert_can_access_draft(self.user_researcher, Researcher.__name__) + + def test_member_added_via_api_can_access_draft_project_manager(self): + self._add_member_and_assert_can_access_draft(self.user_project_manager, ProjectManager.__name__) + + def test_non_member_cannot_access_draft_project(self): + self.client.force_authenticate(self.outsider) + detail_resp = self.client.get(self.detail_url) + self.assertIn( + detail_resp.status_code, + (403, 404), + "Non-member should not access draft project", + )