From f0c0eef8dc26fc1c84f89518fcf345db7d89b09f Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Fri, 28 Jun 2024 21:52:06 +0530
Subject: [PATCH] groups: Lock user group rows while using them for settings.

This is important to make sure that we handle cases when there
are two parallel requests - one for using a group for a setting
and one for deactivating the same group. This makes sure that
atleast one of the above task fails.
---
 zerver/lib/user_groups.py | 12 ++++++++----
 zerver/views/streams.py   | 19 ++++++++++---------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py
index 3bc330bbf0..23b1ef3a37 100644
--- a/zerver/lib/user_groups.py
+++ b/zerver/lib/user_groups.py
@@ -108,10 +108,10 @@ def has_user_group_access(
 
 
 def access_user_group_by_id(
-    user_group_id: int, user_profile: UserProfile, *, for_read: bool
+    user_group_id: int, user_profile: UserProfile, *, for_read: bool, for_setting: bool = False
 ) -> NamedUserGroup:
     try:
-        if for_read:
+        if for_read and not for_setting:
             user_group = NamedUserGroup.objects.get(id=user_group_id, realm=user_profile.realm)
         else:
             user_group = NamedUserGroup.objects.select_for_update().get(
@@ -277,7 +277,9 @@ def update_or_create_user_group_for_setting(
     member_users = user_ids_to_users(direct_members, realm)
     user_group.direct_members.set(member_users)
 
-    potential_subgroups = NamedUserGroup.objects.filter(realm=realm, id__in=direct_subgroups)
+    potential_subgroups = NamedUserGroup.objects.select_for_update().filter(
+        realm=realm, id__in=direct_subgroups
+    )
     group_ids_found = [group.id for group in potential_subgroups]
     group_ids_not_found = [
         group_id for group_id in direct_subgroups if group_id not in group_ids_found
@@ -301,7 +303,9 @@ def access_user_group_for_setting(
     current_setting_value: UserGroup | None = None,
 ) -> UserGroup:
     if isinstance(setting_user_group, int):
-        named_user_group = access_user_group_by_id(setting_user_group, user_profile, for_read=True)
+        named_user_group = access_user_group_by_id(
+            setting_user_group, user_profile, for_read=True, for_setting=True
+        )
         check_setting_configuration_for_system_groups(
             named_user_group, setting_name, permission_configuration
         )
diff --git a/zerver/views/streams.py b/zerver/views/streams.py
index 2510631c85..add9fe0880 100644
--- a/zerver/views/streams.py
+++ b/zerver/views/streams.py
@@ -392,15 +392,16 @@ def update_stream_backend(
                 raise JsonableError(_("Invalid channel ID"))
 
             user_group_id = request_settings_dict[setting_group_id_name]
-            user_group = access_user_group_for_setting(
-                user_group_id,
-                user_profile,
-                setting_name=setting_name,
-                permission_configuration=permission_configuration,
-            )
-            do_change_stream_group_based_setting(
-                stream, setting_name, user_group, acting_user=user_profile
-            )
+            with transaction.atomic(durable=True):
+                user_group = access_user_group_for_setting(
+                    user_group_id,
+                    user_profile,
+                    setting_name=setting_name,
+                    permission_configuration=permission_configuration,
+                )
+                do_change_stream_group_based_setting(
+                    stream, setting_name, user_group, acting_user=user_profile
+                )
 
     return json_success(request)