mirror of https://github.com/zulip/zulip.git
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.
This commit is contained in:
parent
c77ed52fa9
commit
c9ca4e68e5
|
@ -125,6 +125,7 @@ def set_up_streams_for_new_human_user(
|
||||||
user_profile: UserProfile,
|
user_profile: UserProfile,
|
||||||
prereg_user: Optional[PreregistrationUser] = None,
|
prereg_user: Optional[PreregistrationUser] = None,
|
||||||
default_stream_groups: Sequence[DefaultStreamGroup] = [],
|
default_stream_groups: Sequence[DefaultStreamGroup] = [],
|
||||||
|
add_initial_stream_subscriptions: bool = True,
|
||||||
) -> None:
|
) -> None:
|
||||||
realm = user_profile.realm
|
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
|
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
|
if add_initial_stream_subscriptions:
|
||||||
# happens when user directly signs up without any invitation), we add the
|
# If the Preregistration object didn't explicitly list some streams (it
|
||||||
# default streams for the realm. Note that we are fine with "slim" Stream
|
# happens when user directly signs up without any invitation), we add the
|
||||||
# objects for calling bulk_add_subscriptions and add_new_user_history,
|
# default streams for the realm. Note that we are fine with "slim" Stream
|
||||||
# which we verify in StreamSetupTest tests that check query counts.
|
# objects for calling bulk_add_subscriptions and add_new_user_history,
|
||||||
if len(streams) == 0 and not user_was_invited:
|
# which we verify in StreamSetupTest tests that check query counts.
|
||||||
streams = get_slim_realm_default_streams(realm.id)
|
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:
|
for default_stream_group in default_stream_groups:
|
||||||
default_stream_group_streams = default_stream_group.streams.all()
|
default_stream_group_streams = default_stream_group.streams.all()
|
||||||
for stream in default_stream_group_streams:
|
for stream in default_stream_group_streams:
|
||||||
if stream not in streams:
|
if stream not in streams:
|
||||||
streams.append(stream)
|
streams.append(stream)
|
||||||
|
else:
|
||||||
|
streams = []
|
||||||
|
|
||||||
bulk_add_subscriptions(
|
bulk_add_subscriptions(
|
||||||
realm,
|
realm,
|
||||||
|
@ -234,6 +238,7 @@ def process_new_human_user(
|
||||||
prereg_user: Optional[PreregistrationUser] = None,
|
prereg_user: Optional[PreregistrationUser] = None,
|
||||||
default_stream_groups: Sequence[DefaultStreamGroup] = [],
|
default_stream_groups: Sequence[DefaultStreamGroup] = [],
|
||||||
realm_creation: bool = False,
|
realm_creation: bool = False,
|
||||||
|
add_initial_stream_subscriptions: bool = True,
|
||||||
) -> None:
|
) -> None:
|
||||||
# subscribe to default/invitation streams and
|
# subscribe to default/invitation streams and
|
||||||
# fill in some recent historical messages
|
# fill in some recent historical messages
|
||||||
|
@ -241,6 +246,7 @@ def process_new_human_user(
|
||||||
user_profile=user_profile,
|
user_profile=user_profile,
|
||||||
prereg_user=prereg_user,
|
prereg_user=prereg_user,
|
||||||
default_stream_groups=default_stream_groups,
|
default_stream_groups=default_stream_groups,
|
||||||
|
add_initial_stream_subscriptions=add_initial_stream_subscriptions,
|
||||||
)
|
)
|
||||||
|
|
||||||
realm = user_profile.realm
|
realm = user_profile.realm
|
||||||
|
@ -461,6 +467,7 @@ def do_create_user(
|
||||||
acting_user: Optional[UserProfile],
|
acting_user: Optional[UserProfile],
|
||||||
enable_marketing_emails: bool = True,
|
enable_marketing_emails: bool = True,
|
||||||
email_address_visibility: Optional[int] = None,
|
email_address_visibility: Optional[int] = None,
|
||||||
|
add_initial_stream_subscriptions: bool = True,
|
||||||
) -> UserProfile:
|
) -> UserProfile:
|
||||||
with transaction.atomic():
|
with transaction.atomic():
|
||||||
user_profile = create_user(
|
user_profile = create_user(
|
||||||
|
@ -569,6 +576,7 @@ def do_create_user(
|
||||||
prereg_user=prereg_user,
|
prereg_user=prereg_user,
|
||||||
default_stream_groups=default_stream_groups,
|
default_stream_groups=default_stream_groups,
|
||||||
realm_creation=realm_creation,
|
realm_creation=realm_creation,
|
||||||
|
add_initial_stream_subscriptions=add_initial_stream_subscriptions,
|
||||||
)
|
)
|
||||||
|
|
||||||
if realm_creation:
|
if realm_creation:
|
||||||
|
|
|
@ -296,6 +296,13 @@ class ZulipSCIMUser(SCIMUser):
|
||||||
|
|
||||||
if self.is_new_user():
|
if self.is_new_user():
|
||||||
assert full_name_new_value is not None
|
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(
|
self.obj = do_create_user(
|
||||||
email_new_value,
|
email_new_value,
|
||||||
password,
|
password,
|
||||||
|
@ -303,6 +310,7 @@ class ZulipSCIMUser(SCIMUser):
|
||||||
full_name_new_value,
|
full_name_new_value,
|
||||||
role=role_new_value,
|
role=role_new_value,
|
||||||
tos_version=UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN,
|
tos_version=UserProfile.TOS_VERSION_BEFORE_FIRST_LOGIN,
|
||||||
|
add_initial_stream_subscriptions=add_initial_stream_subscriptions,
|
||||||
acting_user=None,
|
acting_user=None,
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
|
@ -9,6 +9,7 @@ from typing_extensions import override
|
||||||
|
|
||||||
from zerver.actions.user_settings import do_change_full_name
|
from zerver.actions.user_settings import do_change_full_name
|
||||||
from zerver.lib.scim import ZulipSCIMUser
|
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.lib.test_classes import ZulipTestCase
|
||||||
from zerver.models import UserProfile
|
from zerver.models import UserProfile
|
||||||
from zerver.models.realms import get_realm
|
from zerver.models.realms import get_realm
|
||||||
|
@ -389,6 +390,39 @@ class TestSCIMUser(SCIMTestCase):
|
||||||
expected_response_schema = self.generate_user_schema(new_user)
|
expected_response_schema = self.generate_user_schema(new_user)
|
||||||
self.assertEqual(output_data, expected_response_schema)
|
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:
|
def test_post_with_no_name_formatted_included_config(self) -> None:
|
||||||
# A payload for creating a new user with the specified account details.
|
# A payload for creating a new user with the specified account details.
|
||||||
payload = {
|
payload = {
|
||||||
|
|
|
@ -37,7 +37,8 @@ class OIDCIdPConfigDict(TypedDict, total=False):
|
||||||
auto_signup: bool
|
auto_signup: bool
|
||||||
|
|
||||||
|
|
||||||
class SCIMConfigDict(TypedDict):
|
class SCIMConfigDict(TypedDict, total=False):
|
||||||
bearer_token: str
|
bearer_token: str
|
||||||
scim_client_name: str
|
scim_client_name: str
|
||||||
name_formatted_included: bool
|
name_formatted_included: bool
|
||||||
|
create_guests_without_streams: bool
|
||||||
|
|
Loading…
Reference in New Issue