From 0ed5f7606318de0baa937738a61be1694c0c5b63 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Tue, 26 Oct 2021 12:45:16 +0530 Subject: [PATCH] settings: Add backend code for using user email_address_visibility setting. This commits update the code to use user-level email_address_visibility setting instead of realm-level to set or update the value of UserProfile.email field and to send the emails to clients. Major changes are - - UserProfile.email field is set while creating the user according to RealmUserDefault.email_address_visbility. - UserProfile.email field is updated according to change in the setting. - 'email_address_visibility' is added to person objects in user add event and in avatar change event. - client_gravatar can be different for different users when computing avatar_url for messages and user objects since email available to clients is dependent on user-level setting. - For bots, email_address_visibility is set to EVERYONE while creating them irrespective of realm-default value. - Test changes are basically setting user-level setting instead of realm setting and modifying the checks accordingly. --- analytics/tests/test_support_views.py | 13 ++- zerver/actions/create_realm.py | 17 +++- zerver/actions/create_user.py | 4 +- zerver/actions/user_settings.py | 26 +++++- zerver/lib/bulk_create.py | 10 ++- zerver/lib/cache.py | 1 + zerver/lib/create_user.py | 13 ++- zerver/lib/events.py | 41 ++++++---- zerver/lib/message.py | 10 +++ zerver/lib/test_helpers.py | 20 +++-- zerver/lib/users.py | 26 +++--- zerver/models.py | 5 +- zerver/openapi/zulip.yaml | 10 +-- zerver/tests/test_auth_backends.py | 9 +-- zerver/tests/test_bots.py | 10 ++- zerver/tests/test_custom_profile_data.py | 8 ++ zerver/tests/test_decorators.py | 13 ++- zerver/tests/test_email_change.py | 15 +--- zerver/tests/test_event_system.py | 50 ++++++++---- zerver/tests/test_events.py | 78 ++++++++++++------ zerver/tests/test_message_fetch.py | 25 +++--- zerver/tests/test_mirror_users.py | 1 + zerver/tests/test_outgoing_webhook_system.py | 1 + zerver/tests/test_signup.py | 5 ++ zerver/tests/test_users.py | 85 +++++++++++--------- zerver/views/message_fetch.py | 12 ++- zerver/views/users.py | 5 -- zilencer/management/commands/populate_db.py | 3 + 28 files changed, 331 insertions(+), 185 deletions(-) diff --git a/analytics/tests/test_support_views.py b/analytics/tests/test_support_views.py index a76872e2c4..f0b244440a 100644 --- a/analytics/tests/test_support_views.py +++ b/analytics/tests/test_support_views.py @@ -8,11 +8,8 @@ from django.utils.timezone import now as timezone_now from corporate.lib.stripe import add_months, update_sponsorship_status from corporate.models import Customer, CustomerPlan, LicenseLedger, get_customer_by_realm from zerver.actions.invites import do_create_multiuse_invite_link -from zerver.actions.realm_settings import ( - do_change_realm_org_type, - do_send_realm_reactivation_email, - do_set_realm_property, -) +from zerver.actions.realm_settings import do_change_realm_org_type, do_send_realm_reactivation_email +from zerver.actions.user_settings import do_change_user_setting from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import reset_emails_in_zulip_realm from zerver.models import ( @@ -218,10 +215,10 @@ class TestSupportEndpoint(ZulipTestCase): self.login("iago") - do_set_realm_property( - get_realm("zulip"), + do_change_user_setting( + self.example_user("hamlet"), "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_NOBODY, + UserProfile.EMAIL_ADDRESS_VISIBILITY_NOBODY, acting_user=None, ) diff --git a/zerver/actions/create_realm.py b/zerver/actions/create_realm.py index cdb6a48ba3..f8baebfc6c 100644 --- a/zerver/actions/create_realm.py +++ b/zerver/actions/create_realm.py @@ -95,8 +95,7 @@ def set_realm_permissions_based_on_org_type(realm: Realm) -> None: realm.org_type == Realm.ORG_TYPES["education_nonprofit"]["id"] or realm.org_type == Realm.ORG_TYPES["education"]["id"] ): - # Limit email address visibility and user creation to administrators. - realm.email_address_visibility = Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS + # Limit user creation to administrators. realm.invite_to_realm_policy = Realm.POLICY_ADMINS_ONLY # Restrict public stream creation to staff, but allow private # streams (useful for study groups, etc.). @@ -211,7 +210,19 @@ def do_create_realm( event_time=realm.date_created, ) - RealmUserDefault.objects.create(realm=realm) + realm_default_email_address_visibility = RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_EVERYONE + if ( + realm.org_type == Realm.ORG_TYPES["education_nonprofit"]["id"] + or realm.org_type == Realm.ORG_TYPES["education"]["id"] + ): + # Email address of users should be initially visible to admins only. + realm_default_email_address_visibility = ( + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_ADMINS + ) + + RealmUserDefault.objects.create( + realm=realm, email_address_visibility=realm_default_email_address_visibility + ) create_system_user_groups_for_realm(realm) diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py index 1f9d2cabe3..b1bb2ff8a8 100644 --- a/zerver/actions/create_user.py +++ b/zerver/actions/create_user.py @@ -305,9 +305,7 @@ def notify_created_user(user_profile: UserProfile) -> None: user_ids_with_real_email_access = [] user_ids_without_real_email_access = [] for user in active_users: - if can_access_delivery_email( - user, user_profile.id, user_profile.realm.email_address_visibility, user_row["is_bot"] - ): + if can_access_delivery_email(user, user_profile.id, user_row["email_address_visibility"]): user_ids_with_real_email_access.append(user.id) else: user_ids_without_real_email_access.append(user.id) diff --git a/zerver/actions/user_settings.py b/zerver/actions/user_settings.py index 736d5340a8..0ca4c022cc 100644 --- a/zerver/actions/user_settings.py +++ b/zerver/actions/user_settings.py @@ -13,8 +13,10 @@ from zerver.lib.avatar import avatar_url from zerver.lib.cache import ( cache_delete, delete_user_profile_caches, + flush_user_profile, user_profile_by_api_key_cache_key, ) +from zerver.lib.create_user import get_display_email_address from zerver.lib.i18n import get_language_name from zerver.lib.queue import queue_json_publish from zerver.lib.send_email import FromAddress, clear_scheduled_emails, send_email @@ -64,10 +66,10 @@ def send_delivery_email_update_events( for active_user in active_users: could_access_delivery_email_previously = can_access_delivery_email( - active_user, user_profile.id, old_visibility_setting, user_profile.is_bot + active_user, user_profile.id, old_visibility_setting ) can_access_delivery_email_now = can_access_delivery_email( - active_user, user_profile.id, visibility_setting, user_profile.is_bot + active_user, user_profile.id, visibility_setting ) if could_access_delivery_email_previously != can_access_delivery_email_now: @@ -502,6 +504,26 @@ def do_change_user_setting( ) ) + if setting_name == "email_address_visibility": + send_delivery_email_update_events( + user_profile, old_value, user_profile.email_address_visibility + ) + + if UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE not in [old_value, setting_value]: + # We use real email addresses on UserProfile.email only if + # EMAIL_ADDRESS_VISIBILITY_EVERYONE is configured, so + # changes between values that will not require changing + # that field, so we can save work and return here. + return + + user_profile.email = get_display_email_address(user_profile) + user_profile.save(update_fields=["email"]) + + transaction.on_commit(lambda: flush_user_profile(sender=UserProfile, instance=user_profile)) + + send_user_email_update_event(user_profile) + notify_avatar_url_change(user_profile) + if setting_name == "enable_drafts_synchronization" and setting_value is False: # Delete all of the drafts from the backend but don't send delete events # for them since all that's happened is that we stopped syncing changes, diff --git a/zerver/lib/bulk_create.py b/zerver/lib/bulk_create.py index cd99750ecd..10746c6c7d 100644 --- a/zerver/lib/bulk_create.py +++ b/zerver/lib/bulk_create.py @@ -34,7 +34,14 @@ def bulk_create_users( UserProfile.objects.filter(realm=realm).values_list("email", flat=True) ) users = sorted(user_raw for user_raw in users_raw if user_raw[0] not in existing_users) + realm_user_default = RealmUserDefault.objects.get(realm=realm) + if bot_type is None: + email_address_visibility = realm_user_default.email_address_visibility + else: + # There is no privacy motivation for limiting access to bot email addresses, + # so we hardcode them to EMAIL_ADDRESS_VISIBILITY_EVERYONE. + email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE # Now create user_profiles profiles_to_create: List[UserProfile] = [] @@ -51,6 +58,7 @@ def bulk_create_users( tos_version, timezone, tutorial_status=UserProfile.TUTORIAL_FINISHED, + email_address_visibility=email_address_visibility, ) if bot_type is None: @@ -67,7 +75,7 @@ def bulk_create_users( setattr(profile, settings_name, value) profiles_to_create.append(profile) - if realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: + if email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE: UserProfile.objects.bulk_create(profiles_to_create) else: for user_profile in profiles_to_create: diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index dfd5fa0bf4..4f31345dca 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -499,6 +499,7 @@ realm_user_dict_fields: List[str] = [ "delivery_email", "bot_type", "long_term_idle", + "email_address_visibility", ] diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index c7b841364e..77dc29fec5 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -91,6 +91,8 @@ def create_user_profile( tutorial_status: str = UserProfile.TUTORIAL_WAITING, force_id: Optional[int] = None, force_date_joined: Optional[datetime] = None, + *, + email_address_visibility: int, ) -> UserProfile: if force_date_joined is None: date_joined = timezone_now() @@ -120,6 +122,7 @@ def create_user_profile( onboarding_steps=orjson.dumps([]).decode(), default_language=default_language, delivery_email=email, + email_address_visibility=email_address_visibility, **extra_kwargs, ) if bot_type or not active: @@ -154,6 +157,14 @@ def create_user( force_date_joined: Optional[datetime] = None, enable_marketing_emails: Optional[bool] = None, ) -> UserProfile: + realm_user_default = RealmUserDefault.objects.get(realm=realm) + if bot_type is None: + email_address_visibility = realm_user_default.email_address_visibility + else: + # There is no privacy motivation for limiting access to bot email addresses, + # so we hardcode them to EMAIL_ADDRESS_VISIBILITY_EVERYONE. + email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE + user_profile = create_user_profile( realm, email, @@ -168,6 +179,7 @@ def create_user( default_language, force_id=force_id, force_date_joined=force_date_joined, + email_address_visibility=email_address_visibility, ) user_profile.avatar_source = avatar_source user_profile.timezone = timezone @@ -189,7 +201,6 @@ def create_user( # save is not required. copy_default_settings(source_profile, user_profile) elif bot_type is None: - realm_user_default = RealmUserDefault.objects.get(realm=realm) copy_default_settings(realm_user_default, user_profile) else: # This will be executed only for bots. diff --git a/zerver/lib/events.py b/zerver/lib/events.py index e231dc65a6..73fc49f84f 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -767,6 +767,14 @@ def apply_event( if event["op"] == "add": person = copy.deepcopy(person) + + if client_gravatar: + email_address_visibility = UserProfile.objects.get( + id=person_user_id + ).email_address_visibility + if email_address_visibility != UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE: + client_gravatar = False + if client_gravatar and person["avatar_url"].startswith("https://secure.gravatar.com/"): person["avatar_url"] = None person["is_active"] = True @@ -856,17 +864,27 @@ def apply_event( if not was_admin and now_admin: state["realm_bots"] = get_owned_bot_dicts(user_profile) - if ( - client_gravatar - and "avatar_url" in person - # Respect the client_gravatar setting in the `users` data. - and person["avatar_url"].startswith("https://secure.gravatar.com/") - ): - person["avatar_url"] = None - person["avatar_url_medium"] = None - if person_user_id in state["raw_users"]: p = state["raw_users"][person_user_id] + + if "avatar_url" in person: + # Respect the client_gravatar setting in the `users` data. + if client_gravatar: + email_address_visibility = UserProfile.objects.get( + id=person_user_id + ).email_address_visibility + if ( + email_address_visibility + != UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE + ): + client_gravatar = False + + if client_gravatar and person["avatar_url"].startswith( + "https://secure.gravatar.com/" + ): + person["avatar_url"] = None + person["avatar_url_medium"] = None + for field in p: if field in person: p[field] = person[field] @@ -1399,11 +1417,6 @@ def do_events_register( stream_typing_notifications = client_capabilities.get("stream_typing_notifications", False) user_settings_object = client_capabilities.get("user_settings_object", False) - if realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: - # If real email addresses are not available to the user, their - # clients cannot compute gravatars, so we force-set it to false. - client_gravatar = False - if fetch_event_types is not None: event_types_set: Optional[Set[str]] = set(fetch_event_types) elif event_types is not None: diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 8f5346768d..588711c8e6 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -373,6 +373,13 @@ class MessageDict: if not skip_copy: obj = copy.copy(obj) + if obj["sender_email_address_visibility"] != UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE: + # If email address of the sender is only available to administrators, + # clients cannot compute gravatars, so we force-set it to false. + # If we plumbed the current user's role, we could allow client_gravatar=True + # here if the current user's role has access to the target user's email address. + client_gravatar = False + MessageDict.set_sender_avatar(obj, client_gravatar) if apply_markdown: obj["content_type"] = "text/html" @@ -390,6 +397,7 @@ class MessageDict: del obj["recipient_type"] del obj["recipient_type_id"] del obj["sender_is_mirror_dummy"] + del obj["sender_email_address_visibility"] return obj @staticmethod @@ -600,6 +608,7 @@ class MessageDict: "avatar_source", "avatar_version", "is_mirror_dummy", + "email_address_visibility", ) rows = query_for_ids(query, sender_ids, "zerver_userprofile.id") @@ -616,6 +625,7 @@ class MessageDict: obj["sender_avatar_source"] = user_row["avatar_source"] obj["sender_avatar_version"] = user_row["avatar_version"] obj["sender_is_mirror_dummy"] = user_row["is_mirror_dummy"] + obj["sender_email_address_visibility"] = user_row["email_address_visibility"] @staticmethod def hydrate_recipient_info(obj: Dict[str, Any], display_recipient: DisplayRecipientT) -> None: diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 33c0edd4be..2dffbe7294 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -38,7 +38,8 @@ from django.urls import URLResolver from moto.s3 import mock_s3 from mypy_boto3_s3.service_resource import Bucket -from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.realm_settings import do_set_realm_user_default_setting +from zerver.actions.user_settings import do_change_user_setting from zerver.lib import cache from zerver.lib.avatar import avatar_url from zerver.lib.cache import get_cache_backend @@ -49,7 +50,7 @@ from zerver.lib.upload.s3 import S3UploadBackend from zerver.models import ( Client, Message, - Realm, + RealmUserDefault, Subscription, UserMessage, UserProfile, @@ -199,12 +200,21 @@ def stdout_suppressed() -> Iterator[IO[str]]: def reset_emails_in_zulip_realm() -> None: realm = get_realm("zulip") - do_set_realm_property( - realm, + realm_user_default = RealmUserDefault.objects.get(realm=realm) + do_set_realm_user_default_setting( + realm_user_default, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) + users = UserProfile.objects.filter(realm=realm) + for user in users: + do_change_user_setting( + user, + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + acting_user=None, + ) def get_test_image_file(filename: str) -> IO[bytes]: diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 193dfe1085..5f2a7087fc 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -399,23 +399,23 @@ def can_access_delivery_email( user_profile: UserProfile, target_user_id: int, email_address_visibility: int, - target_user_is_bot: bool, ) -> bool: if target_user_id == user_profile.id: return True - if target_user_is_bot: + # Bots always have email_address_visibility as EMAIL_ADDRESS_VISIBILITY_EVERYONE. + if email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE: return True - if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: - return True - - if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS: + if email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS: return user_profile.is_realm_admin - if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS: + if email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS: return user_profile.is_realm_admin or user_profile.is_moderator + if email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_MEMBERS: + return not user_profile.is_guest + return False @@ -492,7 +492,7 @@ def format_user_row( ) if acting_user is not None and can_access_delivery_email( - acting_user, row["id"], realm.email_address_visibility, row["is_bot"] + acting_user, row["id"], row["email_address_visibility"] ): result["delivery_email"] = row["delivery_email"] else: @@ -618,12 +618,15 @@ def get_raw_user_data( for row in user_dicts: if profiles_by_user_id is not None: custom_profile_field_data = profiles_by_user_id.get(row["id"], {}) - + client_gravatar_for_user = ( + client_gravatar + and row["email_address_visibility"] == UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE + ) result[row["id"]] = format_user_row( realm, acting_user=acting_user, row=row, - client_gravatar=client_gravatar, + client_gravatar=client_gravatar_for_user, user_avatar_url_field_optional=user_avatar_url_field_optional, custom_profile_field_data=custom_profile_field_data, ) @@ -655,8 +658,7 @@ def get_users_with_access_to_real_email(user_profile: UserProfile) -> List[int]: if can_access_delivery_email( user, user_profile.id, - user_profile.realm.email_address_visibility, - user_profile.is_bot, + user_profile.email_address_visibility, ): user_ids_with_real_email_access.append(user.id) diff --git a/zerver/models.py b/zerver/models.py index 80949f4752..32f3b57a43 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2077,9 +2077,8 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): # type return allowed_bot_types def email_address_is_realm_public(self) -> bool: - if self.realm.email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: - return True - if self.is_bot: + # Bots always have EMAIL_ADDRESS_VISIBILITY_EVERYONE. + if self.email_address_visibility == UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE: return True return False diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7cfc843326..8e824dee3f 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -570,7 +570,7 @@ paths: description: | When the Zulip display email address of a user changes, either due to the user's email address changing, or - due to changes in the organization's + due to changes in the user's [email address visibility][help-email-visibility]. [help-email-visibility]: /help/restrict-visibility-of-email-addresses @@ -7242,8 +7242,8 @@ paths: description: | The user's real email address. This value will be `None` if you cannot access this user's real email address. For bot users, this field is - always set to the real email of the bot, since everyone can access the - bot's email irrespective of `email_address_visibility` setting. + always set to the real email of the bot, because bot users always have + `email_address_visibility` as `EMAIL_ADDRESS_VISIBILITY_EVERYONE`. **Changes**: Prior to Zulip 7.0 (feature level 163), this field was present only when `email_address_visibility` was restricted and you had access to @@ -16942,8 +16942,8 @@ components: description: | The user's real email address. This value will be `None` if you cannot access user's real email address. For bot users, this field is always - set to the real email of the bot, since everyone can access the bot's - email irrespective of `email_address_visibility` setting. + set to the real email of the bot, because bot users always have + `email_address_visibility` as `EMAIL_ADDRESS_VISIBILITY_EVERYONE`. **Changes**: Prior to Zulip 7.0 (feature level 163), this field was present only when `email_address_visibility` was restricted and you had access to the diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 33fbdd0ff8..1af5ba5c60 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -61,7 +61,7 @@ from zerver.actions.realm_settings import ( do_reactivate_realm, do_set_realm_property, ) -from zerver.actions.user_settings import do_change_password +from zerver.actions.user_settings import do_change_password, do_change_user_setting from zerver.actions.users import change_user_is_active, do_deactivate_user from zerver.lib.avatar import avatar_url from zerver.lib.avatar_hash import user_avatar_path @@ -6117,11 +6117,10 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase): def test_update_with_hidden_emails(self) -> None: hamlet = self.example_user("hamlet") - realm = get_realm("zulip") - do_set_realm_property( - realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) hamlet.refresh_from_db() diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 970c40def1..bf0ae4d2cf 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -9,7 +9,7 @@ from django.test import override_settings from zulip_bots.custom_exceptions import ConfigValidationError from zerver.actions.bots import do_change_bot_owner -from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.realm_settings import do_set_realm_user_default_setting from zerver.actions.streams import do_change_stream_permission from zerver.actions.users import do_change_can_create_users, do_change_user_role, do_deactivate_user from zerver.lib.bot_config import ConfigError, get_bot_config @@ -19,6 +19,7 @@ from zerver.lib.test_classes import UploadSerializeMixin, ZulipTestCase from zerver.lib.test_helpers import avatar_disk_path, get_test_image_file from zerver.models import ( Realm, + RealmUserDefault, Service, UserProfile, get_bot_services, @@ -318,10 +319,11 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): # Test that we don't mangle the email field with # email_address_visibility limited to admins user = self.example_user("hamlet") - do_set_realm_property( - user.realm, + realm_user_default = RealmUserDefault.objects.get(realm=user.realm) + do_set_realm_user_default_setting( + realm_user_default, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) user.refresh_from_db() diff --git a/zerver/tests/test_custom_profile_data.py b/zerver/tests/test_custom_profile_data.py index 48a30487d0..65694190e6 100644 --- a/zerver/tests/test_custom_profile_data.py +++ b/zerver/tests/test_custom_profile_data.py @@ -9,6 +9,7 @@ from zerver.actions.custom_profile_fields import ( try_add_realm_custom_profile_field, try_reorder_realm_custom_profile_fields, ) +from zerver.actions.user_settings import do_change_user_setting from zerver.lib.external_accounts import DEFAULT_EXTERNAL_ACCOUNTS from zerver.lib.markdown import markdown_convert from zerver.lib.test_classes import ZulipTestCase @@ -16,6 +17,7 @@ from zerver.lib.types import ProfileDataElementUpdateDict, ProfileDataElementVal from zerver.models import ( CustomProfileField, CustomProfileFieldValue, + UserProfile, custom_profile_fields_for_realm, get_realm, ) @@ -979,6 +981,12 @@ class ListCustomProfileFieldTest(CustomProfileFieldTestCase): def test_get_custom_profile_fields_from_api_for_single_user(self) -> None: self.login("iago") + do_change_user_setting( + self.example_user("iago"), + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, + acting_user=None, + ) expected_keys = { "result", "msg", diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index 8d5703e7e0..a43da050fc 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -15,11 +15,8 @@ from django.utils.timezone import now as timezone_now from zerver.actions.create_realm import do_create_realm from zerver.actions.create_user import do_reactivate_user -from zerver.actions.realm_settings import ( - do_deactivate_realm, - do_reactivate_realm, - do_set_realm_property, -) +from zerver.actions.realm_settings import do_deactivate_realm, do_reactivate_realm +from zerver.actions.user_settings import do_change_user_setting from zerver.actions.users import change_user_is_active, do_deactivate_user from zerver.decorator import ( authenticate_notify, @@ -1239,10 +1236,10 @@ class FetchAPIKeyTest(ZulipTestCase): def test_fetch_api_key_email_address_visibility(self) -> None: user = self.example_user("cordelia") - do_set_realm_property( - user.realm, + do_change_user_setting( + user, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) diff --git a/zerver/tests/test_email_change.py b/zerver/tests/test_email_change.py index 4d4a165b8d..63e75d8203 100644 --- a/zerver/tests/test_email_change.py +++ b/zerver/tests/test_email_change.py @@ -15,12 +15,11 @@ from confirmation.models import ( ) from zerver.actions.create_user import do_reactivate_user from zerver.actions.realm_settings import do_deactivate_realm, do_set_realm_property -from zerver.actions.user_settings import do_start_email_change_process +from zerver.actions.user_settings import do_change_user_setting, do_start_email_change_process from zerver.actions.users import do_deactivate_user from zerver.lib.test_classes import ZulipTestCase from zerver.models import ( EmailChangeStatus, - Realm, UserProfile, get_realm, get_user, @@ -89,12 +88,6 @@ class EmailChangeTestCase(ZulipTestCase): def test_confirm_email_change(self) -> None: user_profile = self.example_user("hamlet") - do_set_realm_property( - user_profile.realm, - "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, - acting_user=None, - ) old_email = user_profile.delivery_email new_email = '"
  • hamlet-new
  • "@zulip.com' @@ -270,10 +263,10 @@ class EmailChangeTestCase(ZulipTestCase): def test_change_delivery_email_end_to_end_with_admins_visibility(self) -> None: user_profile = self.example_user("hamlet") - do_set_realm_property( - user_profile.realm, + do_change_user_setting( + user_profile, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py index e14f5b298d..5561dcce4c 100644 --- a/zerver/tests/test_event_system.py +++ b/zerver/tests/test_event_system.py @@ -13,18 +13,22 @@ from version import API_FEATURE_LEVEL, ZULIP_MERGE_BASE, ZULIP_VERSION from zerver.actions.custom_profile_fields import try_update_realm_custom_profile_field from zerver.actions.message_send import check_send_message from zerver.actions.presence import do_update_user_presence -from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.user_settings import do_change_user_setting from zerver.actions.users import do_change_user_role from zerver.lib.event_schema import check_restart_event from zerver.lib.events import fetch_initial_state_data from zerver.lib.exceptions import AccessDeniedError from zerver.lib.request import RequestVariableMissingError from zerver.lib.test_classes import ZulipTestCase -from zerver.lib.test_helpers import HostRequestMock, dummy_handler, stub_event_queue_user_events +from zerver.lib.test_helpers import ( + HostRequestMock, + dummy_handler, + reset_emails_in_zulip_realm, + stub_event_queue_user_events, +) from zerver.lib.users import get_api_key, get_raw_user_data from zerver.models import ( CustomProfileField, - Realm, UserMessage, UserPresence, UserProfile, @@ -466,6 +470,13 @@ class GetEventsTest(ZulipTestCase): self.assertEqual(message["content"], "

    hello

    ") self.assertIn("gravatar.com", message["avatar_url"]) + do_change_user_setting( + user_profile, + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + acting_user=None, + ) + message = get_message(apply_markdown=False, client_gravatar=True) self.assertEqual(message["display_recipient"], "Denmark") self.assertEqual(message["content"], "**hello**") @@ -605,13 +616,14 @@ class FetchInitialStateDataTest(ZulipTestCase): def test_delivery_email_presence_for_non_admins(self) -> None: user_profile = self.example_user("aaron") + hamlet = self.example_user("hamlet") self.assertFalse(user_profile.is_realm_admin) hamlet = self.example_user("hamlet") - do_set_realm_property( - user_profile.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) result = fetch_initial_state_data(user_profile) @@ -619,10 +631,10 @@ class FetchInitialStateDataTest(ZulipTestCase): (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id) self.assertEqual(hamlet_obj["delivery_email"], hamlet.delivery_email) - do_set_realm_property( - user_profile.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) result = fetch_initial_state_data(user_profile) @@ -632,13 +644,14 @@ class FetchInitialStateDataTest(ZulipTestCase): def test_delivery_email_presence_for_admins(self) -> None: user_profile = self.example_user("iago") + hamlet = self.example_user("hamlet") self.assertTrue(user_profile.is_realm_admin) hamlet = self.example_user("hamlet") - do_set_realm_property( - user_profile.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) result = fetch_initial_state_data(user_profile) @@ -646,15 +659,15 @@ class FetchInitialStateDataTest(ZulipTestCase): (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id) self.assertEqual(hamlet_obj["delivery_email"], hamlet.delivery_email) - do_set_realm_property( - user_profile.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) result = fetch_initial_state_data(user_profile) - for key, value in result["raw_users"].items(): - self.assertIn("delivery_email", value) + (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id) + self.assertIn("delivery_email", hamlet_obj) def test_user_avatar_url_field_optional(self) -> None: hamlet = self.example_user("hamlet") @@ -691,6 +704,8 @@ class FetchInitialStateDataTest(ZulipTestCase): and urlsplit(user_dict["avatar_url"]).hostname == "secure.gravatar.com" ] + reset_emails_in_zulip_realm() + # Test again with client_gravatar = True result = fetch_initial_state_data( user_profile=hamlet, @@ -949,6 +964,7 @@ class ClientDescriptorsTest(ZulipTestCase): sender_avatar_source=UserProfile.AVATAR_FROM_GRAVATAR, sender_avatar_version=1, sender_is_mirror_dummy=None, + sender_email_address_visibility=UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, recipient_type=None, recipient_type_id=None, ), diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index e964ce3f99..fa37b12de7 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -528,6 +528,13 @@ class NormalActionsTest(BaseAction): check_message("events[0]", events[0]) assert isinstance(events[0]["message"]["avatar_url"], str) + do_change_user_setting( + self.example_user("hamlet"), + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + acting_user=None, + ) + events = self.verify_action( lambda: self.send_stream_message( self.example_user("hamlet"), "Verona", "hello", capture_on_commit_callbacks=False @@ -1160,10 +1167,11 @@ class NormalActionsTest(BaseAction): check_user_group_add_members("events[2]", events[2]) def test_register_events_email_address_visibility(self) -> None: - do_set_realm_property( - self.user_profile.realm, + realm_user_default = RealmUserDefault.objects.get(realm=self.user_profile.realm) + do_set_realm_user_default_setting( + realm_user_default, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) @@ -1486,6 +1494,12 @@ class NormalActionsTest(BaseAction): assert isinstance(events[0]["person"]["avatar_url"], str) assert isinstance(events[0]["person"]["avatar_url_medium"], str) + do_change_user_setting( + self.user_profile, + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + acting_user=self.user_profile, + ) events = self.verify_action( lambda: do_change_avatar_fields( self.user_profile, UserProfile.AVATAR_FROM_GRAVATAR, acting_user=self.user_profile @@ -1501,11 +1515,11 @@ class NormalActionsTest(BaseAction): ) check_realm_user_update("events[0]", events[0], "full_name") - def test_change_user_delivery_email_email_address_visibility_admins(self) -> None: - do_set_realm_property( - self.user_profile.realm, + def test_change_user_delivery_email_email_address_visibilty_admins(self) -> None: + do_change_user_setting( + self.user_profile, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) # Important: We need to refresh from the database here so that @@ -1521,10 +1535,10 @@ class NormalActionsTest(BaseAction): assert isinstance(events[1]["person"]["avatar_url_medium"], str) def test_change_user_delivery_email_email_address_visibility_everyone(self) -> None: - do_set_realm_property( - self.user_profile.realm, + do_change_user_setting( + self.user_profile, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) # Important: We need to refresh from the database here so that @@ -2800,6 +2814,19 @@ class UserDisplayActionTest(BaseAction): raise AssertionError(f"No test created for {setting_name}") for value in values: + if setting_name == "email_address_visibility": + # When "email_address_visibility" setting is changed, there is at least + # one event with type "user_settings" sent to the modified user itself. + num_events = 1 + + old_value = getattr(self.user_profile, setting_name) + if UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE in [old_value, value]: + # In case when either the old value or new value of setting is + # UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, "email" field of + # UserProfile object is updated and thus two additional events, for + # changing email and avatar_url field, are sent. + num_events = 3 + events = self.verify_action( lambda: do_change_user_setting( self.user_profile, setting_name, value, acting_user=self.user_profile @@ -2839,37 +2866,38 @@ class UserDisplayActionTest(BaseAction): check_realm_user_update("events[2]", events[2], "timezone") def test_delivery_email_events_on_changing_email_address_visibility(self) -> None: + cordelia = self.example_user("cordelia") do_change_user_role(self.user_profile, UserProfile.ROLE_MODERATOR, acting_user=None) - do_set_realm_property( - self.user_profile.realm, + do_change_user_setting( + cordelia, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS, acting_user=None, ) events = self.verify_action( - lambda: do_set_realm_property( - self.user_profile.realm, + lambda: do_change_user_setting( + cordelia, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=self.user_profile, ), - num_events=10, + user_settings_object=True, ) - check_realm_user_update("events[1]", events[1], "delivery_email") - self.assertIsNone(events[1]["person"]["delivery_email"]) + check_realm_user_update("events[0]", events[0], "delivery_email") + self.assertIsNone(events[0]["person"]["delivery_email"]) events = self.verify_action( - lambda: do_set_realm_property( - self.user_profile.realm, + lambda: do_change_user_setting( + cordelia, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS, acting_user=self.user_profile, ), - num_events=10, + user_settings_object=True, ) - check_realm_user_update("events[1]", events[1], "delivery_email") - self.assertIsNotNone(events[1]["person"]["delivery_email"]) + check_realm_user_update("events[0]", events[0], "delivery_email") + self.assertEqual(events[0]["person"]["delivery_email"], cordelia.delivery_email) class SubscribeActionTest(BaseAction): diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py index cc4c5d9268..2cd9aac904 100644 --- a/zerver/tests/test_message_fetch.py +++ b/zerver/tests/test_message_fetch.py @@ -15,6 +15,7 @@ from analytics.models import RealmCount from zerver.actions.message_edit import do_update_message from zerver.actions.realm_settings import do_set_realm_property from zerver.actions.uploads import do_claim_attachments +from zerver.actions.user_settings import do_change_user_setting from zerver.actions.users import do_deactivate_user from zerver.lib.avatar import avatar_url from zerver.lib.exceptions import JsonableError @@ -1681,31 +1682,37 @@ class GetOldMessagesTest(ZulipTestCase): hamlet = self.example_user("hamlet") self.login_user(hamlet) - do_set_realm_property( - hamlet.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) self.send_personal_message(hamlet, self.example_user("iago")) - result = self.get_and_check_messages(dict(client_gravatar=orjson.dumps(False).decode())) + result = self.get_and_check_messages( + dict(anchor="newest", client_gravatar=orjson.dumps(False).decode()) + ) message = result["messages"][0] self.assertIn("gravatar.com", message["avatar_url"]) - result = self.get_and_check_messages(dict(client_gravatar=orjson.dumps(True).decode())) + result = self.get_and_check_messages( + dict(anchor="newest", client_gravatar=orjson.dumps(True).decode()) + ) message = result["messages"][0] self.assertEqual(message["avatar_url"], None) # Now verify client_gravatar doesn't run with EMAIL_ADDRESS_VISIBILITY_ADMINS - do_set_realm_property( - hamlet.realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) - result = self.get_and_check_messages(dict(client_gravatar=orjson.dumps(True).decode())) + result = self.get_and_check_messages( + dict(anchor="newest", client_gravatar=orjson.dumps(True).decode()) + ) message = result["messages"][0] self.assertIn("gravatar.com", message["avatar_url"]) diff --git a/zerver/tests/test_mirror_users.py b/zerver/tests/test_mirror_users.py index 55b497e629..c81cdec32b 100644 --- a/zerver/tests/test_mirror_users.py +++ b/zerver/tests/test_mirror_users.py @@ -169,6 +169,7 @@ class MirroredMessageUsersTest(ZulipTestCase): kwargs["bot_owner"] = None kwargs["tos_version"] = None kwargs["timezone"] = timezone_now() + kwargs["email_address_visibility"] = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE create_user_profile(**kwargs).save() raise IntegrityError diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index 274d043c0e..ad29222a17 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -66,6 +66,7 @@ class DoRestCallTests(ZulipTestCase): "sender_full_name": bot_user.full_name, "sender_avatar_source": UserProfile.AVATAR_FROM_GRAVATAR, "sender_avatar_version": 1, + "sender_email_address_visibility": UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, "recipient_type": "stream", "recipient_type_id": 999, "sender_is_mirror_dummy": False, diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index e6a4794804..19635a5917 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -4593,6 +4593,7 @@ class UserSignUpTest(InviteUserBase): hamlet_in_zulip.high_contrast_mode = True hamlet_in_zulip.enter_sends = True hamlet_in_zulip.tutorial_status = UserProfile.TUTORIAL_FINISHED + hamlet_in_zulip.email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE hamlet_in_zulip.save() result = self.client_post("/accounts/home/", {"email": email}, subdomain=subdomain) @@ -4632,6 +4633,7 @@ class UserSignUpTest(InviteUserBase): hamlet_in_zulip.high_contrast_mode = True hamlet_in_zulip.enter_sends = True hamlet_in_zulip.tutorial_status = UserProfile.TUTORIAL_FINISHED + hamlet_in_zulip.email_address_visibility = UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE hamlet_in_zulip.save() # Now we'll be making requests to another subdomain, so we need to logout @@ -4676,6 +4678,9 @@ class UserSignUpTest(InviteUserBase): self.assertEqual(hamlet_in_lear.enter_sends, True) self.assertEqual(hamlet_in_lear.enable_stream_audible_notifications, False) self.assertEqual(hamlet_in_lear.tutorial_status, UserProfile.TUTORIAL_FINISHED) + self.assertEqual( + hamlet_in_lear.email_address_visibility, UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE + ) zulip_path_id = avatar_disk_path(hamlet_in_zulip) lear_path_id = avatar_disk_path(hamlet_in_lear) diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 5b1b88683a..16f020e0ed 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -17,7 +17,7 @@ from zerver.actions.invites import do_create_multiuse_invite_link, do_invite_use from zerver.actions.message_send import RecipientInfoResult, get_recipient_info from zerver.actions.muted_users import do_mute_user from zerver.actions.realm_settings import do_set_realm_property -from zerver.actions.user_settings import bulk_regenerate_api_keys +from zerver.actions.user_settings import bulk_regenerate_api_keys, do_change_user_setting from zerver.actions.users import ( change_user_is_active, do_change_can_create_users, @@ -54,7 +54,6 @@ from zerver.models import ( InvalidFakeEmailDomainError, Message, PreregistrationUser, - Realm, RealmDomain, RealmUserDefault, Recipient, @@ -328,31 +327,17 @@ class PermissionTest(ZulipTestCase): # Now, switch email address visibility, check client_gravatar # is automatically disabled for the user. with self.captureOnCommitCallbacks(execute=True): - do_set_realm_property( - user.realm, + do_change_user_setting( + user, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) result = self.client_get("/json/users", {"client_gravatar": "true"}) members = self.assert_json_success(result)["members"] hamlet = find_dict(members, "user_id", user.id) self.assertEqual(hamlet["email"], f"user{user.id}@zulip.testserver") - # Note that the Gravatar URL should still be computed from the - # `delivery_email`; otherwise, we won't be able to serve the - # user's Gravatar. self.assertEqual(hamlet["avatar_url"], get_gravatar_url(user.delivery_email, 1)) - self.assertEqual(hamlet["delivery_email"], user.delivery_email) - - # Also verify the /events code path. This is a bit hacky, but - # basically we want to verify client_gravatar is being - # overridden. - with mock.patch( - "zerver.lib.events.request_event_queue", return_value=None - ) as mock_request_event_queue: - with self.assertRaises(JsonableError): - do_events_register(user, user.realm, get_client("website"), client_gravatar=True) - self.assertEqual(mock_request_event_queue.call_args_list[0][0][3], False) # client_gravatar is still turned off for admins. In theory, # it doesn't need to be, but client-side changes would be @@ -834,8 +819,11 @@ class QueryCountTest(ZulipTestCase): class BulkCreateUserTest(ZulipTestCase): def test_create_users(self) -> None: realm = get_realm("zulip") - realm.email_address_visibility = Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS - realm.save() + realm_user_default = RealmUserDefault.objects.get(realm=realm) + realm_user_default.email_address_visibility = ( + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_ADMINS + ) + realm_user_default.save() name_list = [ ("Fred Flintstone", "fred@zulip.com"), @@ -855,8 +843,10 @@ class BulkCreateUserTest(ZulipTestCase): self.assertEqual(lisa.is_bot, False) self.assertEqual(lisa.bot_type, None) - realm.email_address_visibility = Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE - realm.save() + realm_user_default.email_address_visibility = ( + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_EVERYONE + ) + realm_user_default.save() name_list = [ ("Bono", "bono@zulip.com"), @@ -2132,7 +2122,9 @@ class GetProfileTest(ZulipTestCase): def test_get_all_profiles_avatar_urls(self) -> None: hamlet = self.example_user("hamlet") - result = self.api_get(hamlet, "/api/v1/users") + result = self.api_get( + hamlet, "/api/v1/users", {"client_gravatar": orjson.dumps(False).decode()} + ) response_dict = self.assert_json_success(result) (my_user,) = (user for user in response_dict["members"] if user["email"] == hamlet.email) @@ -2145,11 +2137,10 @@ class GetProfileTest(ZulipTestCase): def test_user_email_according_to_email_address_visibility_setting(self) -> None: hamlet = self.example_user("hamlet") - realm = hamlet.realm - do_set_realm_property( - realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_NOBODY, + UserProfile.EMAIL_ADDRESS_VISIBILITY_NOBODY, acting_user=None, ) @@ -2160,10 +2151,10 @@ class GetProfileTest(ZulipTestCase): self.assertEqual(result["user"].get("delivery_email"), None) self.assertEqual(result["user"].get("email"), f"user{hamlet.id}@zulip.testserver") - do_set_realm_property( - realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, acting_user=None, ) @@ -2180,10 +2171,10 @@ class GetProfileTest(ZulipTestCase): self.assertEqual(result["user"].get("delivery_email"), None) self.assertEqual(result["user"].get("email"), f"user{hamlet.id}@zulip.testserver") - do_set_realm_property( - realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS, acting_user=None, ) @@ -2200,10 +2191,30 @@ class GetProfileTest(ZulipTestCase): self.assertEqual(result["user"].get("delivery_email"), None) self.assertEqual(result["user"].get("email"), f"user{hamlet.id}@zulip.testserver") - do_set_realm_property( - realm, + do_change_user_setting( + hamlet, "email_address_visibility", - Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MEMBERS, + acting_user=None, + ) + + # Check that normal user can access email when setting is set to + # EMAIL_ADDRESS_VISIBILITY_MEMBERS. + result = orjson.loads(self.client_get(f"/json/users/{hamlet.id}").content) + self.assertEqual(result["user"].get("delivery_email"), hamlet.delivery_email) + self.assertEqual(result["user"].get("email"), f"user{hamlet.id}@zulip.testserver") + + # Check that guest cannot access email when setting is set to + # EMAIL_ADDRESS_VISIBILITY_MEMBERS. + self.login("polonius") + result = orjson.loads(self.client_get(f"/json/users/{hamlet.id}").content) + self.assertEqual(result["user"].get("delivery_email"), None) + self.assertEqual(result["user"].get("email"), f"user{hamlet.id}@zulip.testserver") + + do_change_user_setting( + hamlet, + "email_address_visibility", + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, acting_user=None, ) diff --git a/zerver/views/message_fetch.py b/zerver/views/message_fetch.py index 5549566601..266b5185da 100644 --- a/zerver/views/message_fetch.py +++ b/zerver/views/message_fetch.py @@ -25,7 +25,7 @@ from zerver.lib.sqlalchemy_utils import get_sqlalchemy_connection from zerver.lib.topic import DB_TOPIC_NAME, MATCH_TOPIC, topic_column_sa from zerver.lib.utils import statsd from zerver.lib.validator import check_bool, check_int, check_list, to_non_negative_int -from zerver.models import Realm, UserMessage, UserProfile +from zerver.models import UserMessage, UserProfile MAX_MESSAGES_PER_FETCH = 5000 @@ -137,12 +137,10 @@ def get_messages_backend( assert realm is not None - if ( - is_web_public_query - or realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE - ): - # If email addresses are only available to administrators, - # clients cannot compute gravatars, so we force-set it to false. + if is_web_public_query: + # client_gravatar here is just the user-requested value. "finalize_payload" function + # is responsible for sending avatar_url based on each individual sender's + # email_address_visibility setting. client_gravatar = False if narrow is not None: diff --git a/zerver/views/users.py b/zerver/views/users.py index 2508a8cc22..84ab329cfe 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -89,7 +89,6 @@ from zerver.models import ( EmailContainsPlusError, InvalidFakeEmailDomainError, Message, - Realm, Service, Stream, UserProfile, @@ -637,10 +636,6 @@ def get_user_data( compress very poorly compared to other data. """ realm = user_profile.realm - if realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: - # If email addresses are only available to administrators, - # clients cannot compute gravatars, so we force-set it to false. - client_gravatar = False members = get_raw_user_data( realm, diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index b31773716d..ae015cbd2c 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -333,6 +333,9 @@ class Command(BaseCommand): realm_user_default = RealmUserDefault.objects.get(realm=zulip_realm) realm_user_default.enter_sends = True + realm_user_default.email_address_visibility = ( + RealmUserDefault.EMAIL_ADDRESS_VISIBILITY_ADMINS + ) realm_user_default.save() if options["test_suite"]: