users: Always pass "delivery_email" field in user objects.

Previously, user objects contained delivery_email field
only when user had access to real email. Also, delivery_email
was not present if visibility setting is set to "everyone"
as email field was itself set to real email.

This commit changes the code to pass "delivery_email" field
always in the user objects with its value being "None" if
user does not have access to real email and real email otherwise.
The "delivery_email" field value is None for logged-out users.
For bots, the "delivery_email" is always set to real email
irrespective of email_address_visibility setting.

Also, since user has access to real email if visibility is set
to "everyone", "delivery_email" field is passed in that case
too.

There is no change in email field and it is same as before.

This commit also adds code to send event to update delivery_email
field when email_address_visibility setting changes to all the
users whose access to emails changes and also changes the code to
send event on changing delivery_email to users who have access
to email.

api_docs/changelog.md

@@ -20,6 +20,19 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 7.0
 
+**Feature level 163**
+
+* [`GET /users`](/api/get-users), [`GET /users/{user_id}`](/api/get-user),
+  [`GET /users/{email}`](/api/get-user-by-email),
+  [`GET /users/me`](/api/get-own-user) and [`GET /events`](/api/get-events):
+  The `delivery_email` field is always present in user objects, including the case
+  when `email_address_visibility` is set to `EMAIL_ADDRESS_VISIBILITY_EVERYONE,`
+  with the value being `None` if the requestor does not have access to the user's
+  real email. For bot users, the `delivery_email` field is always set to the real email.
+* [`GET /events`](/api/get-events): Event for updating `delivery_email`  is now sent to
+  all users who have access to it and is also sent when `email_address_visibility` setting
+  changes.
+
 **Feature level 162**
 
 * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events),

frontend_tests/node_tests/lib/events.js

@@ -547,6 +547,7 @@ exports.fixtures = {
             profile_data: {},
             timezone: "America/New_York",
             date_joined: "2020-01-01",
+            delivery_email: "test-delivery@example.com",
         },
     },
 

zerver/actions/create_user.py

@@ -306,7 +306,7 @@ def notify_created_user(user_profile: UserProfile) -> None:
     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, user_profile.id, user_profile.realm.email_address_visibility, user_row["is_bot"]
         ):
             user_ids_with_real_email_access.append(user.id)
         else:
@@ -322,7 +322,7 @@ def notify_created_user(user_profile: UserProfile) -> None:
         )
 
     if user_ids_without_real_email_access:
-        del person["delivery_email"]
+        person["delivery_email"] = None
         event = dict(type="realm_user", op="add", person=person)
         transaction.on_commit(
             lambda event=event: send_event(

zerver/actions/realm_settings.py

@@ -12,7 +12,11 @@ from confirmation.models import Confirmation, create_confirmation_link, generate
 from zerver.actions.custom_profile_fields import do_remove_realm_custom_profile_fields
 from zerver.actions.message_delete import do_delete_messages_by_sender
 from zerver.actions.user_groups import update_users_in_full_members_system_group
-from zerver.actions.user_settings import do_delete_avatar_image, send_user_email_update_event
+from zerver.actions.user_settings import (
+    do_delete_avatar_image,
+    send_delivery_email_update_events,
+    send_user_email_update_event,
+)
 from zerver.lib.cache import flush_user_profile
 from zerver.lib.create_user import get_display_email_address
 from zerver.lib.message import parse_message_time_limit_setting, update_first_visible_message_id
@@ -95,6 +99,11 @@ def do_set_realm_property(
     )
 
     if name == "email_address_visibility":
+        user_profiles = UserProfile.objects.filter(realm=realm, is_bot=False)
+
+        for user_profile in user_profiles:
+            send_delivery_email_update_events(user_profile, old_value, value)
+
         if Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE not in [old_value, value]:
             # We use real email addresses on UserProfile.email only if
             # EMAIL_ADDRESS_VISIBILITY_EVERYONE is configured, so
@@ -102,7 +111,6 @@ def do_set_realm_property(
             # that field, so we can save work and return here.
             return
 
-        user_profiles = UserProfile.objects.filter(realm=realm, is_bot=False)
         for user_profile in user_profiles:
             user_profile.email = get_display_email_address(user_profile)
         UserProfile.objects.bulk_update(user_profiles, ["email"])

zerver/actions/user_settings.py

@@ -20,7 +20,12 @@ from zerver.lib.queue import queue_json_publish
 from zerver.lib.send_email import FromAddress, clear_scheduled_emails, send_email
 from zerver.lib.timezone import canonicalize_timezone
 from zerver.lib.upload import delete_avatar_image
-from zerver.lib.users import check_bot_name_available, check_full_name
+from zerver.lib.users import (
+    can_access_delivery_email,
+    check_bot_name_available,
+    check_full_name,
+    get_users_with_access_to_real_email,
+)
 from zerver.lib.utils import generate_api_key
 from zerver.models import (
     Draft,
@@ -50,6 +55,49 @@ def send_user_email_update_event(user_profile: UserProfile) -> None:
     )
 
 
+def send_delivery_email_update_events(
+    user_profile: UserProfile, old_visibility_setting: int, visibility_setting: int
+) -> None:
+    active_users = user_profile.realm.get_active_users()
+    delivery_email_now_visible_user_ids = []
+    delivery_email_now_invisible_user_ids = []
+
+    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
+        )
+        can_access_delivery_email_now = can_access_delivery_email(
+            active_user, user_profile.id, visibility_setting, user_profile.is_bot
+        )
+
+        if could_access_delivery_email_previously != can_access_delivery_email_now:
+            if can_access_delivery_email_now:
+                delivery_email_now_visible_user_ids.append(active_user.id)
+            else:
+                delivery_email_now_invisible_user_ids.append(active_user.id)
+
+    if delivery_email_now_visible_user_ids:
+        person = dict(user_id=user_profile.id, delivery_email=user_profile.delivery_email)
+        event = dict(type="realm_user", op="update", person=person)
+        transaction.on_commit(
+            lambda event=event: send_event(
+                user_profile.realm,
+                event,
+                delivery_email_now_visible_user_ids,
+            )
+        )
+    if delivery_email_now_invisible_user_ids:
+        person = dict(user_id=user_profile.id, delivery_email=None)
+        event = dict(type="realm_user", op="update", person=person)
+        transaction.on_commit(
+            lambda event=event: send_event(
+                user_profile.realm,
+                event,
+                delivery_email_now_invisible_user_ids,
+            )
+        )
+
+
 @transaction.atomic(savepoint=False)
 def do_change_user_delivery_email(user_profile: UserProfile, new_email: str) -> None:
     delete_user_profile_caches([user_profile])
@@ -61,12 +109,14 @@ def do_change_user_delivery_email(user_profile: UserProfile, new_email: str) ->
     else:
         user_profile.save(update_fields=["delivery_email"])
 
-    # We notify just the target user (and eventually org admins, only
+    # We notify all the users who have access to delivery email.
-    # when email_address_visibility=EMAIL_ADDRESS_VISIBILITY_ADMINS)
-    # about their new delivery email, since that field is private.
     payload = dict(user_id=user_profile.id, delivery_email=new_email)
     event = dict(type="realm_user", op="update", person=payload)
-    transaction.on_commit(lambda: send_event(user_profile.realm, event, [user_profile.id]))
+    delivery_email_visible_user_ids = get_users_with_access_to_real_email(user_profile)
+
+    transaction.on_commit(
+        lambda: send_event(user_profile.realm, event, delivery_email_visible_user_ids)
+    )
 
     if user_profile.avatar_source == UserProfile.AVATAR_FROM_GRAVATAR:
         # If the user is using Gravatar to manage their email address,

zerver/lib/event_schema.py

@@ -1085,6 +1085,7 @@ realm_user_type = DictType(
         ("profile_data", StringDictType(dict)),
         ("timezone", str),
         ("date_joined", str),
+        ("delivery_email", OptionalType(str)),
     ]
 )
 
@@ -1157,7 +1158,7 @@ realm_user_person_types = dict(
         required_keys=[
             # vertical formatting
             ("user_id", int),
-            ("delivery_email", str),
+            ("delivery_email", OptionalType(str)),
         ],
     ),
     email=DictType(

zerver/lib/users.py

@@ -396,11 +396,20 @@ def validate_user_custom_profile_data(
 
 
 def can_access_delivery_email(
-    user_profile: UserProfile, target_user_id: int, email_address_visibility: int
+    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:
+        return True
+
+    if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE:
+        return True
+
     if email_address_visibility == Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS:
         return user_profile.is_realm_admin
 
@@ -483,9 +492,11 @@ def format_user_row(
         )
 
     if acting_user is not None and can_access_delivery_email(
-        acting_user, row["id"], realm.email_address_visibility
+        acting_user, row["id"], realm.email_address_visibility, row["is_bot"]
     ):
         result["delivery_email"] = row["delivery_email"]
+    else:
+        result["delivery_email"] = None
 
     if is_bot:
         result["bot_type"] = row["bot_type"]
@@ -635,3 +646,18 @@ def is_2fa_verified(user: UserProfile) -> bool:
     # is True before calling `is_2fa_verified`.
     assert settings.TWO_FACTOR_AUTHENTICATION_ENABLED
     return is_verified(user)
+
+
+def get_users_with_access_to_real_email(user_profile: UserProfile) -> List[int]:
+    active_users = user_profile.realm.get_active_users()
+    user_ids_with_real_email_access = []
+    for user in active_users:
+        if can_access_delivery_email(
+            user,
+            user_profile.id,
+            user_profile.realm.email_address_visibility,
+            user_profile.is_bot,
+        ):
+            user_ids_with_real_email_access.append(user.id)
+
+    return user_ids_with_real_email_access

zerver/openapi/zulip.yaml

@@ -504,9 +504,13 @@ paths:
                                     - type: object
                                       additionalProperties: false
                                       description: |
-                                        When the delivery email of a user changes.
+                                        When the value of the user's delivery email as visible to you changes,
+                                        either due to the email address changing or your access to the user's
+                                        email via their `email_address_visibility` policy changing.
 
-                                        Note: This event is only visible to admins.
+                                        **Changes**: Prior to Zulip 7.0 (feature level 163), this event was
+                                        sent only to the affected user, and this event would only be triggered
+                                        by changing the affected user's delivery email.
                                       properties:
                                         user_id:
                                           type: integer
@@ -514,8 +518,17 @@ paths:
                                             The ID of the user affected by this change.
                                         delivery_email:
                                           type: string
+                                          nullable: true
                                           description: |
                                             The new delivery email of the user.
+
+                                            This value can be `None` if user changes the
+                                            `email_address_visibility` value such that
+                                            you cannot access user's real email.
+
+                                            **Changes**: As of Zulip 7.0 (feature level 163),
+                                            `None` is a possible value as this event is also
+                                            sent on changing `email_address_visibility` setting.
                                     - type: object
                                       additionalProperties: false
                                       description: |
@@ -7225,11 +7238,20 @@ paths:
                         example: 1
                       delivery_email:
                         type: string
+                        nullable: true
                         description: |
-                          The user's real email address. This field is present only if
+                          The user's real email address. This value will be `None` if you cannot
-                          [email address visibility](/help/restrict-visibility-of-email-addresses) is
+                          access this user's real email address. For bot users, this field is
-                          limited and you are an administrator with access to real email addresses
+                          always set to the real email of the bot, since everyone can access the
-                          under the configured policy.
+                          bot's email irrespective of `email_address_visibility` setting.
+
+                          **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 user's real email. Now, this field is always present, including the case
+                          when `email_address_visibility` is set to `EMAIL_ADDRESS_VISIBILITY_EVERYONE`,
+                          with the value being `None` if you don't have access to the user's real email.
+                          For bot users, this field is now always set to the real email irrespective of
+                          `email_address_visibility` setting.
                       profile_data:
                         $ref: "#/components/schemas/profile_data"
                     example:
@@ -16814,7 +16836,8 @@ components:
         - additionalProperties: false
           properties:
             user_id: {}
-            delivery_email: {}
+            delivery_email:
+              nullable: true
             email: {}
             full_name: {}
             date_joined: {}
@@ -16845,11 +16868,20 @@ components:
             The unique ID of the user.
         delivery_email:
           type: string
+          nullable: true
           description: |
-            The user's real email address. This field is present only if
+            The user's real email address. This value will be `None` if you cannot
-            [email address visibility](/help/restrict-visibility-of-email-addresses) is
+            access user's real email address. For bot users, this field is always
-            limited and you are an administrator with access to real email addresses
+            set to the real email of the bot, since everyone can access the bot's
-            under the configured policy.
+            email irrespective of `email_address_visibility` setting.
+
+            **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
+            user's real email. Now, this field is always present, including the case when
+            `email_address_visibility` is set to `EMAIL_ADDRESS_VISIBILITY_EVERYONE`,
+            with the value being `None` if you don't have access the user's real email.
+            For bot users, this field is now always set to the real email irrespective of
+            `email_address_visibility` setting.
         email:
           type: string
           description: |

zerver/tests/test_bots.py

@@ -163,7 +163,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         self.login("hamlet")
         self.assert_num_bots_equal(0)
         events: List[Mapping[str, Any]] = []
-        with self.tornado_redirected_to_list(events, expected_num_events=5):
+        with self.tornado_redirected_to_list(events, expected_num_events=4):
             result = self.create_bot()
         self.assert_num_bots_equal(1)
 
@@ -329,7 +329,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         self.login_user(user)
         self.assert_num_bots_equal(0)
         events: List[Mapping[str, Any]] = []
-        with self.tornado_redirected_to_list(events, expected_num_events=5):
+        with self.tornado_redirected_to_list(events, expected_num_events=4):
             result = self.create_bot()
         self.assert_num_bots_equal(1)
 
@@ -427,7 +427,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
 
         self.assert_num_bots_equal(0)
         events: List[Mapping[str, Any]] = []
-        with self.tornado_redirected_to_list(events, expected_num_events=5):
+        with self.tornado_redirected_to_list(events, expected_num_events=4):
             result = self.create_bot(default_sending_stream="Denmark")
         self.assert_num_bots_equal(1)
         self.assertEqual(result["default_sending_stream"], "Denmark")
@@ -511,7 +511,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
 
         self.assert_num_bots_equal(0)
         events: List[Mapping[str, Any]] = []
-        with self.tornado_redirected_to_list(events, expected_num_events=5):
+        with self.tornado_redirected_to_list(events, expected_num_events=4):
             result = self.create_bot(default_events_register_stream="Denmark")
         self.assert_num_bots_equal(1)
         self.assertEqual(result["default_events_register_stream"], "Denmark")

zerver/tests/test_event_system.py

@@ -606,6 +606,7 @@ class FetchInitialStateDataTest(ZulipTestCase):
     def test_delivery_email_presence_for_non_admins(self) -> None:
         user_profile = self.example_user("aaron")
         self.assertFalse(user_profile.is_realm_admin)
+        hamlet = self.example_user("hamlet")
 
         do_set_realm_property(
             user_profile.realm,
@@ -615,11 +616,8 @@ class FetchInitialStateDataTest(ZulipTestCase):
         )
         result = fetch_initial_state_data(user_profile)
 
-        for key, value in result["raw_users"].items():
+        (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id)
-            if key == user_profile.id:
+        self.assertEqual(hamlet_obj["delivery_email"], hamlet.delivery_email)
-                self.assertEqual(value["delivery_email"], user_profile.delivery_email)
-            else:
-                self.assertNotIn("delivery_email", value)
 
         do_set_realm_property(
             user_profile.realm,
@@ -629,15 +627,13 @@ class FetchInitialStateDataTest(ZulipTestCase):
         )
         result = fetch_initial_state_data(user_profile)
 
-        for key, value in result["raw_users"].items():
+        (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id)
-            if key == user_profile.id:
+        self.assertIsNone(hamlet_obj["delivery_email"])
-                self.assertEqual(value["delivery_email"], user_profile.delivery_email)
-            else:
-                self.assertNotIn("delivery_email", value)
 
     def test_delivery_email_presence_for_admins(self) -> None:
         user_profile = self.example_user("iago")
         self.assertTrue(user_profile.is_realm_admin)
+        hamlet = self.example_user("hamlet")
 
         do_set_realm_property(
             user_profile.realm,
@@ -647,11 +643,8 @@ class FetchInitialStateDataTest(ZulipTestCase):
         )
         result = fetch_initial_state_data(user_profile)
 
-        for key, value in result["raw_users"].items():
+        (hamlet_obj,) = (value for key, value in result["raw_users"].items() if key == hamlet.id)
-            if key == user_profile.id:
+        self.assertEqual(hamlet_obj["delivery_email"], hamlet.delivery_email)
-                self.assertEqual(value["delivery_email"], user_profile.delivery_email)
-            else:
-                self.assertNotIn("delivery_email", value)
 
         do_set_realm_property(
             user_profile.realm,

zerver/tests/test_events.py

@@ -2635,8 +2635,11 @@ class RealmPropertyActionTest(BaseAction):
                 old_value,
                 val,
             ]:
-                # email update event is sent for each user.
+                # one event for updating "email_address_visibility" setting +
-                num_events = 11
+                # one delivery_email update event is sent for 9 users (all human
+                # users except the user themselves) + one email update event for
+                # each of the 10 human users in the realm.
+                num_events = 20
 
             events = self.verify_action(
                 lambda: do_set_realm_property(
@@ -2676,7 +2679,8 @@ class RealmPropertyActionTest(BaseAction):
                 old_value,
                 val,
             ]:
-                check_realm_user_update("events[1]", events[1], "email")
+                check_realm_user_update("events[1]", events[1], "delivery_email")
+                check_realm_user_update("events[10]", events[10], "email")
 
     def test_change_realm_property(self) -> None:
         for prop in Realm.property_types:
@@ -2832,6 +2836,39 @@ class UserDisplayActionTest(BaseAction):
             check_update_display_settings("events[1]", events[1])
             check_realm_user_update("events[2]", events[2], "timezone")
 
+    def test_delivery_email_events_on_changing_email_address_visibility(self) -> None:
+        do_change_user_role(self.user_profile, UserProfile.ROLE_MODERATOR, acting_user=None)
+        do_set_realm_property(
+            self.user_profile.realm,
+            "email_address_visibility",
+            Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS,
+            acting_user=None,
+        )
+
+        events = self.verify_action(
+            lambda: do_set_realm_property(
+                self.user_profile.realm,
+                "email_address_visibility",
+                Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS,
+                acting_user=self.user_profile,
+            ),
+            num_events=10,
+        )
+        check_realm_user_update("events[1]", events[1], "delivery_email")
+        self.assertIsNone(events[1]["person"]["delivery_email"])
+
+        events = self.verify_action(
+            lambda: do_set_realm_property(
+                self.user_profile.realm,
+                "email_address_visibility",
+                Realm.EMAIL_ADDRESS_VISIBILITY_MODERATORS,
+                acting_user=self.user_profile,
+            ),
+            num_events=10,
+        )
+        check_realm_user_update("events[1]", events[1], "delivery_email")
+        self.assertIsNotNone(events[1]["person"]["delivery_email"])
+
 
 class SubscribeActionTest(BaseAction):
     def test_subscribe_events(self) -> None:

zerver/tests/test_example.py

@@ -128,6 +128,7 @@ class TestFullStack(ZulipTestCase):
                 avatar_url=content["user"]["avatar_url"],
                 avatar_version=1,
                 date_joined=content["user"]["date_joined"],
+                delivery_email=None,
                 email=cordelia.email,
                 full_name=cordelia.full_name,
                 is_active=True,

zerver/tests/test_users.py

@@ -2211,17 +2211,17 @@ class GetProfileTest(ZulipTestCase):
         # is set to EMAIL_ADDRESS_VISIBILITY_EVERYONE.
         self.login("shiva")
         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("delivery_email"), hamlet.delivery_email)
         self.assertEqual(result["user"].get("email"), hamlet.delivery_email)
 
         self.login("cordelia")
         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("delivery_email"), hamlet.delivery_email)
         self.assertEqual(result["user"].get("email"), hamlet.delivery_email)
 
         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("delivery_email"), hamlet.delivery_email)
         self.assertEqual(result["user"].get("email"), hamlet.delivery_email)