Creating a user or a group times out and kills the worker

geonode/groups/models.py

@@ -302,7 +302,9 @@ def _handle_perms(self, role=None):
         from geonode.security.utils import AdvancedSecurityWorkflowManager
         from geonode.security.registry import permissions_registry
 
-        permissions_registry.delete_resource_permissions_cache(instance=self.group.group)
+        permissions_registry.delete_resource_permissions_cache(
+            instance=self.group.group, users=[self.user], group_clear_cache=False
+        )
         if not AdvancedSecurityWorkflowManager.is_auto_publishing_workflow():
             AdvancedSecurityWorkflowManager.set_group_member_permissions(self.user, self.group, role)
 

geonode/groups/tests.py

@@ -19,6 +19,8 @@
 import json
 import logging
 
+from unittest.mock import patch
+
 from django.urls import reverse
 from django.conf import settings
 from django.test import override_settings
@@ -838,6 +840,26 @@ def test_removing_user_from_auth_group_deletes_member(self):
         member_exists = GroupMember.objects.filter(user=self.test_user, group=self.bar).exists()
         self.assertFalse(member_exists)
 
+    def test_adding_user_to_auth_group_does_not_recompute_member_perms(self):
+        """Syncing GroupMember from auth.Group must not run the per-resource perms recomputation."""
+        with patch("geonode.groups.models.GroupMember._handle_perms") as _handle_perms:
+            self.test_user.groups.add(self.bar.group)
+
+        self.assertTrue(GroupMember.objects.filter(user=self.test_user, group=self.bar).exists())
+        _handle_perms.assert_not_called()
+
+    def test_adding_user_to_multiple_auth_groups_does_not_duplicate_member(self):
+        """Each m2m sync must not create a second GroupMember for a group already synced."""
+        foo = GroupProfile.objects.create(slug="sync_foo", title="sync_foo")
+        try:
+            self.test_user.groups.add(self.bar.group)
+            self.test_user.groups.add(foo.group)
+
+            self.assertEqual(GroupMember.objects.filter(user=self.test_user, group=self.bar).count(), 1)
+            self.assertEqual(GroupMember.objects.filter(user=self.test_user, group=foo).count(), 1)
+        finally:
+            foo.delete()
+
     def test_join_syncs_to_auth_group(self):
         self.bar.join(self.test_user, role=GroupMember.MEMBER)
 

geonode/people/signals.py

@@ -193,12 +193,14 @@ def sync_group_members(sender, instance, action, reverse, **kwargs):
     user_groups_ids = set(instance.groups.values_list("id", flat=True))
     group_profiles = GroupProfile.objects.filter(group_id__in=user_groups_ids)
 
-    for group_profile in group_profiles:
-        GroupMember.objects.get_or_create(
-            group=group_profile,
-            user=instance,
-            defaults={"role": GroupMember.MEMBER},
-        )
+    # bulk_create skips GroupMember.save(), the auth group is already assigned and
+    # _handle_perms would recompute perms for every resource/user of the group
+    GroupMember.objects.bulk_create(
+        [
+            GroupMember(group=group_profile, user=instance, role=GroupMember.MEMBER)
+            for group_profile in group_profiles.exclude(groupmember__user=instance)
+        ]
+    )
 
     stale_members = GroupMember.objects.filter(user=instance).exclude(group__group_id__in=user_groups_ids)
     for member in stale_members:

geonode/security/registry.py

@@ -423,10 +423,11 @@ def delete_resource_permissions_cache(self, instance, user_clear_cache=True, gro
             self._clear_cache_keys(cache_keys if cache_keys else [])
 
         elif isinstance(instance, Group):
-            group_users = instance.user_set.all()
-            resource_pks_with_perms = [
-                resource.pk for resource in get_objects_for_group(instance, ["base.view_resourcebase"], any_perm=True)
-            ]
+            # membership changes only affect the given users, not the whole group
+            group_users = kwargs.get("users") or instance.user_set.all()
+            resource_pks_with_perms = list(
+                get_objects_for_group(instance, ["base.view_resourcebase"], any_perm=True).values_list("pk", flat=True)
+            )
 
             cache_keys = self._get_cache_key(
                 resource_pks=resource_pks_with_perms,
@@ -437,8 +438,7 @@ def delete_resource_permissions_cache(self, instance, user_clear_cache=True, gro
             self._clear_cache_keys(cache_keys)
 
         elif isinstance(instance, Profile):
-            resources = get_objects_for_user(instance, "base.view_resourcebase")
-            resource_pks = [resource.pk for resource in resources]
+            resource_pks = list(get_objects_for_user(instance, "base.view_resourcebase").values_list("pk", flat=True))
 
             cache_keys = self._get_cache_key(
                 resource_pks=resource_pks,
@@ -595,8 +595,8 @@ def _clear_cache_keys(self, cache_keys):
             else:
                 raise TypeError(f"Expected str or list, got {type(cache_keys)}")
 
-    def _user_identifier(self, user):
-        if user.is_anonymous or user.username == "AnonymousUser" or user == get_anonymous_user():
+    def _user_identifier(self, user, anonymous_user=None):
+        if user.is_anonymous or user.username == "AnonymousUser" or user == (anonymous_user or get_anonymous_user()):
             return "anonymous"
         return f"user:{user.pk}"
 
@@ -614,10 +614,12 @@ def _get_cache_key(self, resource_pks, users=None, groups=None, remove_all_cache
             remove_all_cache: If True, includes the __ALL__ cache key
         """
         cache_keys = []
+        # resolve the anonymous user once, get_anonymous_user hits the DB for each call
+        anonymous_user = get_anonymous_user() if users else None
+        user_identifiers = [self._user_identifier(user, anonymous_user) for user in users or []]
         for pk in resource_pks:
-            if users:
-                for user in users:
-                    cache_keys.append(f"resource_perms:{pk}:{self._user_identifier(user)}")
+            for identifier in user_identifiers:
+                cache_keys.append(f"resource_perms:{pk}:{identifier}")
 
             if groups:
                 for group in groups:

geonode/security/tests.py

@@ -3663,6 +3663,37 @@ def test_multiple_resources_independent_caching(self):
         self.assertIsNotNone(cache.get(cache_key_r1))
         self.assertIsNotNone(cache.get(cache_key_r2))
 
+    def test_cache_key_resolves_user_identifier_once_per_user(self):
+        """Cache key generation must not query the anonymous user for every resource/user pair."""
+        resource_pks = [resource.pk for resource in self.resources]
+        users = [self.admin_user, self.test_user, self.test_user_owner]
+
+        with patch("geonode.security.registry.get_anonymous_user", wraps=get_anonymous_user) as _get_anonymous_user:
+            cache_keys = permissions_registry._get_cache_key(resource_pks, users=users)
+
+        self.assertEqual(_get_anonymous_user.call_count, 1)
+        self.assertEqual(len(cache_keys), len(resource_pks) * len(users))
+        self.assertIn(f"resource_perms:{resource_pks[-1]}:user:{self.test_user.pk}", cache_keys)
+
+    def test_joining_group_clears_only_member_cache(self):
+        """Joining a group must clear only the joining user's cache, not the other members' or the group's."""
+        test_resource = self.resources[0]
+        member_key = f"resource_perms:{test_resource.pk}:user:{self.admin_user.pk}"
+        joining_key = f"resource_perms:{test_resource.pk}:user:{self.test_user.pk}"
+        group_key = f"resource_perms:{test_resource.pk}:group:{self.test_group.pk}"
+
+        permissions_registry.get_perms(instance=test_resource, user=self.admin_user, use_cache=True)
+        permissions_registry.get_perms(instance=test_resource, user=self.test_user, use_cache=True)
+        permissions_registry.get_perms(instance=test_resource, group=self.test_group, use_cache=True)
+        try:
+            self.test_group_profile.join(self.test_user, role=GroupMember.MEMBER)
+
+            self.assertIsNone(cache.get(joining_key))
+            self.assertIsNotNone(cache.get(member_key))
+            self.assertIsNotNone(cache.get(group_key))
+        finally:
+            self.test_group_profile.leave(self.test_user)
+
     def test_cache_key_generation_consistency(self):
         """
         Test that the _get_cache_key method generates consistent and correct cache keys