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.
This commit is contained in:
Sahil Batra
2024-06-28 21:52:06 +05:30
committed by Tim Abbott
parent 38053e9c7c
commit f0c0eef8dc
2 changed files with 18 additions and 13 deletions

View File

@@ -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
)

View File

@@ -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)