Adds owner of a group as member of that group

gateway/.envs/example/django.prod-example.env

@@ -127,7 +127,7 @@ SDS_SHORT_INSTITUTION_NAME=SU
 SDS_PROGRAMMATIC_SITE_NAME=streeling
 
 # Fully qualified domain name for SDK and (future) federation communications; unique
-# across all deployments (e.g. `merrimack.haystack.nd.edu`, `sds.crc.nd.edu`).
+# across all deployments (e.g. `merrimack.haystack.mit.edu`, `sds.crc.nd.edu`).
 SDS_SITE_FQDN=streeling.example.com
 
 # Brand image path for navbar and page backgrounds

gateway/sds_gateway/api_methods/migrations/0020_group_owner_as_member_and_individual_share_field.py

@@ -0,0 +1,41 @@
+"""Add each ShareGroup owner as a member of their own group; add is_individual_share to UserSharePermission.
+
+Existing UserSharePermission rows default to True (individual share), preserving all
+current access. Permissions created purely through a group share are written with
+False so that revoking the group also revokes the permission.
+"""
+
+from django.db import migrations
+from django.db import models
+
+
+def add_owners_as_members(apps, schema_editor):
+    ShareGroup = apps.get_model("api_methods", "ShareGroup")
+    for group in ShareGroup.objects.filter(is_deleted=False).select_related("owner"):
+        group.members.add(group.owner)
+
+
+def remove_owners_as_members(apps, schema_editor):
+    """Reverse: remove owners who are members solely because of this migration.
+
+    Best-effort — also removes owners added manually before this migration, which
+    is an acceptable trade-off for a reversible migration.
+    """
+    ShareGroup = apps.get_model("api_methods", "ShareGroup")
+    for group in ShareGroup.objects.filter(is_deleted=False).select_related("owner"):
+        group.members.remove(group.owner)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("api_methods", "0019_capture_datasets_file_captures_file_datasets_and_more"),
+    ]
+
+    operations = [
+        migrations.RunPython(add_owners_as_members, remove_owners_as_members),
+        migrations.AddField(
+            model_name="usersharepermission",
+            name="is_individual_share",
+            field=models.BooleanField(default=True),
+        ),
+    ]

gateway/sds_gateway/api_methods/migrations/max_migration.txt

@@ -1 +1 @@
-0019_capture_datasets_file_captures_file_datasets_and_more
+0020_group_owner_as_member_and_individual_share_field

gateway/sds_gateway/api_methods/models.py

@@ -920,6 +920,11 @@ class UserSharePermission(BaseModel):
     # Whether this share permission is currently active
     is_enabled = models.BooleanField(default=True)
 
+    # Whether this permission was explicitly granted to the user individually,
+    # as opposed to being created solely via a group share. When True, revocation
+    # of every group still keeps the permission active.
+    is_individual_share = models.BooleanField(default=True)
+
     class Meta:
         unique_together = ["owner", "shared_with", "item_type", "item_uuid"]
         indexes = [
@@ -965,13 +970,8 @@ def has_access_through_groups(self) -> bool:
         return self.share_groups.filter(is_deleted=False).exists()
 
     def update_enabled_status(self) -> None:
-        """Update enabled status based on group access."""
-        if self.share_groups.exists():
-            # User has access through groups, keep enabled
-            self.is_enabled = True
-        else:
-            # No group access, disable
-            self.is_enabled = False
+        """Update enabled status based on individual and group access."""
+        self.is_enabled = self.is_individual_share or self.share_groups.exists()
         self.save()
 
     @classmethod

gateway/sds_gateway/api_methods/tests/test_permission_updates.py

@@ -238,6 +238,85 @@ def test_update_permission_missing_user(self):
         data = response.json()
         assert "not found" in data["error"]
 
+    def test_individual_share_preserved_after_group_revocation(self):
+        """
+        Individual shares must survive a group-access revocation.
+
+        Scenario:
+          1. The owner (U2) grants U1 and U2 personal (non-group) access to X.
+          2. The owner also shares X with group G (which contains U1 and U2).
+             Both users' existing permissions gain G in share_groups.
+          3. The owner revokes group G's access to X.
+          4. U1 and U2 must still have access through their individual shares.
+          5. The owner must still have access via dataset ownership.
+        """
+        # Step 1 - individual shares (no groups yet)
+        perm_user1 = UserSharePermission.objects.create(
+            owner=self.owner,
+            shared_with=self.user1,
+            item_type=ItemType.DATASET,
+            item_uuid=self.dataset.uuid,
+            permission_level=PermissionLevel.VIEWER,
+            is_enabled=True,
+        )
+        perm_user2 = UserSharePermission.objects.create(
+            owner=self.owner,
+            shared_with=self.user2,
+            item_type=ItemType.DATASET,
+            item_uuid=self.dataset.uuid,
+            permission_level=PermissionLevel.VIEWER,
+            is_enabled=True,
+        )
+        assert perm_user1.share_groups.count() == 0
+        assert perm_user2.share_groups.count() == 0
+
+        # Step 2 - share X with group G (self.group has user1 and user2 as members)
+        self.client.force_login(self.owner)
+        add_response = self.client.post(
+            reverse(
+                "users:share_item",
+                kwargs={"item_type": "dataset", "item_uuid": self.dataset.uuid},
+            ),
+            data={"user-search": f"group:{self.group.uuid}"},
+        )
+        assert add_response.status_code == status.HTTP_200_OK
+
+        perm_user1.refresh_from_db()
+        perm_user2.refresh_from_db()
+        assert self.group in perm_user1.share_groups.all()
+        assert self.group in perm_user2.share_groups.all()
+
+        # Step 3 - revoke group G's access
+        remove_response = self.client.post(
+            reverse(
+                "users:share_item",
+                kwargs={"item_type": "dataset", "item_uuid": self.dataset.uuid},
+            ),
+            data={"remove_users": json.dumps([f"group:{self.group.uuid}"])},
+        )
+        assert remove_response.status_code == status.HTTP_200_OK
+
+        # Step 4 - user1 and user2 keep access via their individual shares
+        perm_user1.refresh_from_db()
+        perm_user2.refresh_from_db()
+        assert perm_user1.is_enabled, (
+            "user1's individual share must survive group revocation"
+        )
+        assert perm_user2.is_enabled, (
+            "user2's individual share must survive group revocation"
+        )
+        assert UserSharePermission.user_can_view(
+            self.user1, self.dataset.uuid, ItemType.DATASET
+        )
+        assert UserSharePermission.user_can_view(
+            self.user2, self.dataset.uuid, ItemType.DATASET
+        )
+
+        # Step 5 - owner retains access via dataset ownership
+        assert UserSharePermission.user_can_view(
+            self.owner, self.dataset.uuid, ItemType.DATASET
+        )
+
     def test_update_group_permission_unauthorized(self):
         """Test that users cannot update group permissions they don't own."""
         # Create group owned by user1

gateway/sds_gateway/users/tests/test_group_sharing.py

@@ -260,7 +260,7 @@ def dataset(self, owner: User) -> Dataset:
     @pytest.fixture
     def share_group(self, owner: User, group_member: User) -> ShareGroup:
         group = ShareGroup.objects.create(name="Test Group", owner=owner)
-        group.members.add(group_member)
+        group.members.add(owner, group_member)
         return group
 
     def test_create_share_group(self, client: Client, owner: User) -> None:
@@ -278,9 +278,10 @@ def test_create_share_group(self, client: Client, owner: User) -> None:
         result = response.json()
         assert result["success"] is True
 
-        # Verify group was created
+        # Verify group was created and owner is a member
         group = ShareGroup.objects.get(name="New Test Group", owner=owner)
         assert group.is_deleted is False
+        assert owner in group.members.all()
 
     def test_add_member_to_group(
         self,
@@ -360,6 +361,59 @@ def test_remove_member_from_group(
         assert permission.is_enabled is False
         assert share_group not in permission.share_groups.all()
 
+    def test_cannot_remove_owner_from_group(
+        self,
+        client: Client,
+        owner: User,
+        group_member: User,
+    ) -> None:
+        """Test that the group owner cannot be removed from the group."""
+        group = ShareGroup.objects.create(name="Owner Test Group", owner=owner)
+        group.members.add(owner, group_member)
+
+        client.force_login(owner)
+        url = reverse("users:share_group_list")
+        data = {
+            "action": "remove_members",
+            "group_uuid": str(group.uuid),
+            "user_emails": owner.email,
+        }
+
+        response = client.post(url, data)
+        assert response.status_code == status.HTTP_200_OK
+        result = response.json()
+        # The removal is blocked; the owner should still be a member
+        assert owner in group.members.all()
+        assert result["errors"]
+
+    def test_cannot_remove_non_member_from_group(
+        self,
+        client: Client,
+        owner: User,
+        share_group: ShareGroup,
+    ) -> None:
+        """Test that removing a user who is not in the group produces an error."""
+        non_member = User.objects.create_user(
+            email="nonmember@example.com",
+            password=TEST_PASSWORD,
+            name="Non Member",
+            is_approved=True,
+        )
+
+        client.force_login(owner)
+        url = reverse("users:share_group_list")
+        data = {
+            "action": "remove_members",
+            "group_uuid": str(share_group.uuid),
+            "user_emails": non_member.email,
+        }
+
+        response = client.post(url, data)
+        assert response.status_code == status.HTTP_200_OK
+        result = response.json()
+        assert non_member not in share_group.members.all()
+        assert result["errors"]
+
     def test_delete_share_group(
         self,
         client: Client,
@@ -1080,21 +1134,12 @@ def test_individual_and_group_access_group_removed(
         group1: ShareGroup,
     ) -> None:
         """
-        Test that asset is not accessible to individual when group
-        is removed from permission.
-        """
-        # Share dataset both individually and through group
-        # Individual share
-        UserSharePermission.objects.create(
-            owner=owner,
-            shared_with=group_member,
-            item_type=ItemType.DATASET,
-            item_uuid=dataset.uuid,
-            is_enabled=True,
-            message="Individual share",
-        )
+        Test that asset is not accessible when a group-only share is revoked.
 
-        # Group share
+        A permission created solely via a group share (is_individual_share=False)
+        must be disabled when the group is removed from it.
+        """
+        # Group-only share — no prior individual share
         update_or_create_user_group_share_permissions(
             request_user=owner,
             group=group1,

gateway/sds_gateway/users/utils.py

@@ -149,13 +149,16 @@ def update_or_create_user_group_share_permissions(
             message=message,
             permission_level=permission_level,
             is_enabled=True,
+            is_individual_share=False,
         )
         permission.share_groups.add(group)
     else:
         existing_individual.is_enabled = True
         existing_individual.message = message
         existing_individual.permission_level = permission_level
         existing_individual.share_groups.add(group)
+        # is_individual_share is intentionally NOT changed here: if the user was
+        # already individually shared, that status must be preserved.
         existing_individual.save()
 
 

gateway/sds_gateway/users/views.py

@@ -526,13 +526,17 @@ def _add_individual_user_to_item(
             )
 
             if existing_permission:
-                if existing_permission.is_enabled:
+                if (
+                    existing_permission.is_enabled
+                    and existing_permission.is_individual_share
+                ):
                     return (
                         None,
                         f"{item_type.capitalize()} is already shared with {email}",
                     )
-                # Re-enable the existing disabled permission and update permission level
+                # Re-enable and mark as explicitly individually shared
                 existing_permission.is_enabled = True
+                existing_permission.is_individual_share = True
                 existing_permission.message = message
                 existing_permission.permission_level = permission_level
                 existing_permission.save()
@@ -547,6 +551,7 @@ def _add_individual_user_to_item(
                 message=message,
                 permission_level=permission_level,
                 is_enabled=True,
+                is_individual_share=True,
             )
         except User.DoesNotExist:
             return None, f"User with email {email} not found or not approved"
@@ -3628,6 +3633,7 @@ def _create_share_group(self, request: HttpRequest) -> JsonResponse:
 
         try:
             share_group = ShareGroup.objects.create(name=name, owner=request.user)
+            share_group.members.add(request.user)
 
             return JsonResponse(
                 {
@@ -3637,7 +3643,7 @@ def _create_share_group(self, request: HttpRequest) -> JsonResponse:
                         "uuid": str(share_group.uuid),
                         "name": share_group.name,
                         "created_at": share_group.created_at.isoformat(),
-                        "member_count": 0,
+                        "member_count": 1,
                     },
                 }
             )
@@ -3808,15 +3814,19 @@ def _process_user_removal(
             try:
                 user = User.objects.get(email=email)
 
-                if share_group.members.filter(id=user.id).exists():
-                    share_group.members.remove(user)
+                if not share_group.members.filter(id=user.id).exists():
+                    errors.append(f"User {email} is not a member of this group")
+                    continue
 
-                    # Update share permissions for this user
-                    self._update_user_share_permissions_on_removal(user, share_group)
+                if user == share_group.owner:
+                    errors.append(
+                        f"User {email} is the owner of this group and cannot be removed"
+                    )
+                    continue
 
-                    removed_users.append(email)
-                else:
-                    errors.append(f"User {email} is not a member of this group")
+                share_group.members.remove(user)
+                self._update_user_share_permissions_on_removal(user, share_group)
+                removed_users.append(email)
 
             except User.DoesNotExist:
                 errors.append(f"User with email {email} not found")