From c9ca4e68e525c8e5d4e1019e45b7a3105b21e41e Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 2 Apr 2024 00:56:05 +0200 Subject: [PATCH] scim: Add config option to disable initial streams for guests. When an organization (without open ability for anyone to join) invites a guest user, the invitation prompts allows them to choose whether the guest should be added to default streams or not. This is useful, because since we don't have per-role default streams configs, they may want default streams to be for full Members. SCIM provisioning doesn't have this control, since a newly provisioned user gets created via a direct do_create_user call, thus adding them to the organization's default streams, with no workaround possible aside of just getting rid of default streams in the organization. To make provisioning guests in such an organization usable, we add a simple config option to create them with no streams. It's configured by adding ``` "create_guests_without_streams": True ``` to the config dict in settings.SCIM_CONFIG. --- zerver/actions/create_user.py | 32 ++++++++++++++++++++------------ zerver/lib/scim.py | 8 ++++++++ zerver/tests/test_scim.py | 34 ++++++++++++++++++++++++++++++++++ zproject/settings_types.py | 3 ++- 4 files changed, 64 insertions(+), 13 deletions(-) diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py index 364f3cc79f..69f6d86b97 100644 --- a/zerver/actions/create_user.py +++ b/zerver/actions/create_user.py @@ -125,6 +125,7 @@ def set_up_streams_for_new_human_user( user_profile: UserProfile, prereg_user: Optional[PreregistrationUser] = None, default_stream_groups: Sequence[DefaultStreamGroup] = [], + add_initial_stream_subscriptions: bool = True, ) -> None: realm = user_profile.realm @@ -142,19 +143,22 @@ def set_up_streams_for_new_human_user( prereg_user.referred_by is not None or prereg_user.multiuse_invite is not None ) - # If the Preregistration object didn't explicitly list some streams (it - # happens when user directly signs up without any invitation), we add the - # default streams for the realm. Note that we are fine with "slim" Stream - # objects for calling bulk_add_subscriptions and add_new_user_history, - # which we verify in StreamSetupTest tests that check query counts. - if len(streams) == 0 and not user_was_invited: - streams = get_slim_realm_default_streams(realm.id) + if add_initial_stream_subscriptions: + # If the Preregistration object didn't explicitly list some streams (it + # happens when user directly signs up without any invitation), we add the + # default streams for the realm. Note that we are fine with "slim" Stream + # objects for calling bulk_add_subscriptions and add_new_user_history, + # which we verify in StreamSetupTest tests that check query counts. + if len(streams) == 0 and not user_was_invited: + streams = get_slim_realm_default_streams(realm.id) - for default_stream_group in default_stream_groups: - default_stream_group_streams = default_stream_group.streams.all() - for stream in default_stream_group_streams: - if stream not in streams: - streams.append(stream) + for default_stream_group in default_stream_groups: + default_stream_group_streams = default_stream_group.streams.all() + for stream in default_stream_group_streams: + if stream not in streams: + streams.append(stream) + else: + streams = [] bulk_add_subscriptions( realm, @@ -234,6 +238,7 @@ def process_new_human_user( prereg_user: Optional[PreregistrationUser] = None, default_stream_groups: Sequence[DefaultStreamGroup] = [], realm_creation: bool = False, + add_initial_stream_subscriptions: bool = True, ) -> None: # subscribe to default/invitation streams and # fill in some recent historical messages @@ -241,6 +246,7 @@ def process_new_human_user( user_profile=user_profile, prereg_user=prereg_user, default_stream_groups=default_stream_groups, + add_initial_stream_subscriptions=add_initial_stream_subscriptions, ) realm = user_profile.realm @@ -461,6 +467,7 @@ def do_create_user( acting_user: Optional[UserProfile], enable_marketing_emails: bool = True, email_address_visibility: Optional[int] = None, + add_initial_stream_subscriptions: bool = True, ) -> UserProfile: with transaction.atomic(): user_profile = create_user( @@ -569,6 +576,7 @@ def do_create_user( prereg_user=prereg_user, default_stream_groups=default_stream_groups, realm_creation=realm_creation, + add_initial_stream_subscriptions=add_initial_stream_subscriptions, ) if realm_creation: diff --git a/zerver/lib/scim.py b/zerver/lib/scim.py index 24971c1bc8..0d4d2d5823 100644 --- a/zerver/lib/scim.py +++ b/zerver/lib/scim.py @@ -296,6 +296,13 @@ class ZulipSCIMUser(SCIMUser): if self.is_new_user(): assert full_name_new_value is not None + add_initial_stream_subscriptions = True + if ( + self.config.get("create_guests_without_streams", False) + and role_new_value == UserProfile.ROLE_GUEST + ): + add_initial_stream_subscriptions = False + self.obj = do_create_user( email_new_value, password, @@ -303,6 +310,7 @@ class ZulipSCIMUser(SCIMUser): full_name_new_value, role=role_new_value, tos_version=UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN, + add_initial_stream_subscriptions=add_initial_stream_subscriptions, acting_user=None, ) return diff --git a/zerver/tests/test_scim.py b/zerver/tests/test_scim.py index 600c191de5..af69e98b88 100644 --- a/zerver/tests/test_scim.py +++ b/zerver/tests/test_scim.py @@ -9,6 +9,7 @@ from typing_extensions import override from zerver.actions.user_settings import do_change_full_name from zerver.lib.scim import ZulipSCIMUser +from zerver.lib.stream_subscription import get_subscribed_stream_ids_for_user from zerver.lib.test_classes import ZulipTestCase from zerver.models import UserProfile from zerver.models.realms import get_realm @@ -389,6 +390,39 @@ class TestSCIMUser(SCIMTestCase): expected_response_schema = self.generate_user_schema(new_user) self.assertEqual(output_data, expected_response_schema) + def test_post_create_guest_user_without_streams(self) -> None: + @contextmanager + def mock_create_guests_without_streams() -> Iterator[None]: + config_dict = copy.deepcopy(settings.SCIM_CONFIG) + config_dict["zulip"]["create_guests_without_streams"] = True + with self.settings(SCIM_CONFIG=config_dict): + yield + + payload = { + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName": "newuser@zulip.com", + "name": {"formatted": "New User", "givenName": "New", "familyName": "User"}, + "active": True, + "role": "guest", + } + with mock_create_guests_without_streams(): + result = self.client_post( + "/scim/v2/Users", payload, content_type="application/json", **self.scim_headers() + ) + + self.assertEqual(result.status_code, 201) + output_data = orjson.loads(result.content) + + new_user = UserProfile.objects.last() + assert new_user is not None + self.assertEqual(new_user.delivery_email, "newuser@zulip.com") + self.assertEqual(new_user.role, UserProfile.ROLE_GUEST) + + expected_response_schema = self.generate_user_schema(new_user) + self.assertEqual(output_data, expected_response_schema) + + self.assertEqual(list(get_subscribed_stream_ids_for_user(new_user)), []) + def test_post_with_no_name_formatted_included_config(self) -> None: # A payload for creating a new user with the specified account details. payload = { diff --git a/zproject/settings_types.py b/zproject/settings_types.py index 39cc271a6b..6855a41769 100644 --- a/zproject/settings_types.py +++ b/zproject/settings_types.py @@ -37,7 +37,8 @@ class OIDCIdPConfigDict(TypedDict, total=False): auto_signup: bool -class SCIMConfigDict(TypedDict): +class SCIMConfigDict(TypedDict, total=False): bearer_token: str scim_client_name: str name_formatted_included: bool + create_guests_without_streams: bool