user_groups: Add server level setting disallow anonymous groups for settings.

This commit adds a server level setting which controls whether the setting
can be set to anonymous user groups. We only allow it in the tests for
now because the UI can only handle named user groups.
This commit is contained in:
Sahil Batra 2024-05-30 09:15:38 +05:30 committed by Tim Abbott
parent 32391c3d06
commit d3ea6520dc
7 changed files with 122 additions and 3 deletions

View File

@ -4,6 +4,13 @@ Settings defining permissions in Zulip are increasingly represented
using [user groups](/help/user-groups), which offer much more flexible using [user groups](/help/user-groups), which offer much more flexible
configuration than the older [roles](/api/roles-and-permissions) system. configuration than the older [roles](/api/roles-and-permissions) system.
!!! warn ""
This API feature is under development, and currently only values that
correspond to a single named user group are permitted in
production environments, pending the web application UI supporting
displaying more complex values correctly.
In the API, these settings are represented using a **group-setting In the API, these settings are represented using a **group-setting
value**, which can take two forms: value**, which can take two forms:

View File

@ -2,6 +2,7 @@ from contextlib import contextmanager
from dataclasses import dataclass from dataclasses import dataclass
from typing import Collection, Dict, Iterable, Iterator, List, Mapping, Optional, TypedDict, Union from typing import Collection, Dict, Iterable, Iterator, List, Mapping, Optional, TypedDict, Union
from django.conf import settings
from django.db import connection, transaction from django.db import connection, transaction
from django.db.models import F, Prefetch, QuerySet from django.db.models import F, Prefetch, QuerySet
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
@ -712,6 +713,7 @@ def get_server_supported_permission_settings() -> ServerSupportedPermissionSetti
def parse_group_setting_value( def parse_group_setting_value(
setting_value: Union[int, AnonymousSettingGroupDict], setting_value: Union[int, AnonymousSettingGroupDict],
setting_name: str,
) -> Union[int, AnonymousSettingGroupDict]: ) -> Union[int, AnonymousSettingGroupDict]:
if isinstance(setting_value, int): if isinstance(setting_value, int):
return setting_value return setting_value
@ -719,6 +721,13 @@ def parse_group_setting_value(
if len(setting_value.direct_members) == 0 and len(setting_value.direct_subgroups) == 1: if len(setting_value.direct_members) == 0 and len(setting_value.direct_subgroups) == 1:
return setting_value.direct_subgroups[0] return setting_value.direct_subgroups[0]
if not settings.ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS:
raise JsonableError(
_("{setting_name} can only be set to a single named user group.").format(
setting_name=setting_name
)
)
return setting_value return setting_value

View File

@ -538,6 +538,50 @@ class UserGroupAPITestCase(UserGroupTestCase):
result = self.client_post("/json/user_groups/create", info=params) result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_error(result, "Invalid user group ID: 1111") self.assert_json_error(result, "Invalid user group ID: 1111")
with self.settings(ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS=False):
params = {
"name": "frontend",
"members": orjson.dumps([hamlet.id]).decode(),
"description": "Frontend team",
"can_mention_group": orjson.dumps(
{
"direct_members": [othello.id],
"direct_subgroups": [moderators_group.id],
}
).decode(),
}
result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_error(
result, "can_mention_group can only be set to a single named user group."
)
params = {
"name": "frontend",
"members": orjson.dumps([hamlet.id]).decode(),
"description": "Frontend team",
"can_mention_group": orjson.dumps(
{
"direct_members": [],
"direct_subgroups": [moderators_group.id],
}
).decode(),
}
result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_success(result)
frontend_group = NamedUserGroup.objects.get(name="frontend", realm=hamlet.realm)
self.assertEqual(frontend_group.can_mention_group_id, moderators_group.id)
params = {
"name": "devops",
"members": orjson.dumps([hamlet.id]).decode(),
"description": "Devops team",
"can_mention_group": orjson.dumps(moderators_group.id).decode(),
}
result = self.client_post("/json/user_groups/create", info=params)
self.assert_json_success(result)
devops_group = NamedUserGroup.objects.get(name="devops", realm=hamlet.realm)
self.assertEqual(devops_group.can_mention_group_id, moderators_group.id)
def test_user_group_get(self) -> None: def test_user_group_get(self) -> None:
# Test success # Test success
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
@ -791,6 +835,50 @@ class UserGroupAPITestCase(UserGroupTestCase):
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_error(result, "Invalid user group ID: 1111") self.assert_json_error(result, "Invalid user group ID: 1111")
# Test case when ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS is False.
with self.settings(ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS=False):
params = {
"can_mention_group": orjson.dumps(
{
"new": {
"direct_members": [othello.id],
"direct_subgroups": [moderators_group.id, marketing_group.id],
}
}
).decode()
}
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_error(
result, "can_mention_group can only be set to a single named user group."
)
params = {
"can_mention_group": orjson.dumps(
{
"new": {
"direct_members": [],
"direct_subgroups": [moderators_group.id],
}
}
).decode()
}
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_success(result)
support_group = NamedUserGroup.objects.get(name="support", realm=hamlet.realm)
self.assertEqual(support_group.can_mention_group_id, moderators_group.id)
params = {
"can_mention_group": orjson.dumps(
{
"new": marketing_group.id,
}
).decode()
}
result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params)
self.assert_json_success(result)
support_group = NamedUserGroup.objects.get(name="support", realm=hamlet.realm)
self.assertEqual(support_group.can_mention_group_id, marketing_group.id)
def test_user_group_update_to_already_existing_name(self) -> None: def test_user_group_update_to_already_existing_name(self) -> None:
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
self.login_user(hamlet) self.login_user(hamlet)

View File

@ -70,7 +70,9 @@ def add_user_group(
continue continue
if request_settings_dict[setting_name] is not None: if request_settings_dict[setting_name] is not None:
setting_value = parse_group_setting_value(request_settings_dict[setting_name]) setting_value = parse_group_setting_value(
request_settings_dict[setting_name], setting_name
)
setting_value_group = access_user_group_for_setting( setting_value_group = access_user_group_for_setting(
setting_value, setting_value,
user_profile, user_profile,
@ -130,11 +132,13 @@ def edit_user_group(
continue continue
setting_value = request_settings_dict[setting_name] setting_value = request_settings_dict[setting_name]
new_setting_value = parse_group_setting_value(setting_value.new) new_setting_value = parse_group_setting_value(setting_value.new, setting_name)
expected_current_setting_value = None expected_current_setting_value = None
if setting_value.old is not None: if setting_value.old is not None:
expected_current_setting_value = parse_group_setting_value(setting_value.old) expected_current_setting_value = parse_group_setting_value(
setting_value.old, setting_name
)
current_value = getattr(user_group, setting_name) current_value = getattr(user_group, setting_name)
current_setting_api_value = get_group_setting_value_for_api(current_value) current_setting_api_value = get_group_setting_value_for_api(current_value)

View File

@ -635,3 +635,9 @@ CAN_ACCESS_ALL_USERS_GROUP_LIMITS_PRESENCE = False
SIGNED_ACCESS_TOKEN_VALIDITY_IN_SECONDS = 60 SIGNED_ACCESS_TOKEN_VALIDITY_IN_SECONDS = 60
CUSTOM_AUTHENTICATION_WRAPPER_FUNCTION: Optional[Callable[..., Any]] = None CUSTOM_AUTHENTICATION_WRAPPER_FUNCTION: Optional[Callable[..., Any]] = None
# Whether we allow settings to be set to a collection of users and
# groups as described in api_docs/group-setting-values.md. Set to
# False in production, as we can only handle named user groups in the
# web app settings UI.
ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS = False

View File

@ -207,3 +207,6 @@ SCIM_CONFIG: Dict[str, SCIMConfigDict] = {
SELF_HOSTING_MANAGEMENT_SUBDOMAIN = "selfhosting" SELF_HOSTING_MANAGEMENT_SUBDOMAIN = "selfhosting"
DEVELOPMENT_DISABLE_PUSH_BOUNCER_DOMAIN_CHECK = True DEVELOPMENT_DISABLE_PUSH_BOUNCER_DOMAIN_CHECK = True
PUSH_NOTIFICATION_BOUNCER_URL = f"http://push.{EXTERNAL_HOST}" PUSH_NOTIFICATION_BOUNCER_URL = f"http://push.{EXTERNAL_HOST}"
# Breaks the UI if used, but enabled for development environment testing.
ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS = True

View File

@ -268,3 +268,5 @@ SCIM_CONFIG: Dict[str, SCIMConfigDict] = {
"name_formatted_included": True, "name_formatted_included": True,
} }
} }
ALLOW_ANONYMOUS_GROUP_VALUED_SETTINGS = True