api: Improve handling of delivery_email in the GET /users/{email} API.

Limiting lookups by delivery_email to users with "everyone" email
visibility is overly simplistic. We can successfully do these lookups
whenever the requester has the permission to view the real email address
of the user they're looking up.

api_docs/changelog.md

@@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 10.0
 
+**Feature level 302**
+
+* [`GET /users/{email}`](/api/get-user-by-email): Changed the `email`
+  values by which users can successfully be looked up to match the
+  user email visibility setting's semantics better.
+
 **Feature level 301**
 
 * [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events),

version.py

@@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
 # new level means in api_docs/changelog.md, as well as "**Changes**"
 # entries in the endpoint's documentation in `zulip.yaml`.
 
-API_FEATURE_LEVEL = 301  # Last bumped for can_join_group setting.
+API_FEATURE_LEVEL = 302  # Last bumped for changes to {email} requirements for GET /users/{email}
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/create_user.py

@@ -1,3 +1,4 @@
+import re
 from datetime import datetime
 from email.headerregistry import Address
 
@@ -65,8 +66,29 @@ def copy_default_settings(
     copy_onboarding_steps(settings_source, target_profile)
 
 
+def get_dummy_email_address_for_display_regex(realm: Realm) -> str:
+    """
+    Returns a regex that matches the format of dummy email addresses we
+    generate for the .email of users with limit email_address_visibility.
+
+    The reason we need a regex is that we want something that we can use both
+    for generating the dummy email addresses and recognizing them together with extraction
+    of the user ID.
+    """
+
+    # We can't directly have (\d+) in the username passed to Address, because it gets
+    # mutated by the underlying logic for escaping special characters.
+    # So we use a trick by using $ as a placeholder which will be preserved, and then
+    # replace it with (\d+) to obtain our intended regex.
+    address_template = Address(username="user$", domain=get_fake_email_domain(realm.host)).addr_spec
+    regex = re.escape(address_template).replace(r"\$", r"(\d+)", 1)
+    return regex
+
+
 def get_display_email_address(user_profile: UserProfile) -> str:
     if not user_profile.email_address_is_realm_public():
+        # The format of the dummy email address created here needs to stay consistent
+        # with get_dummy_email_address_for_display_regex.
         return Address(
             username=f"user{user_profile.id}", domain=get_fake_email_domain(user_profile.realm.host)
         ).addr_spec

zerver/lib/users.py

@@ -18,6 +18,7 @@ from zulip_bots.custom_exceptions import ConfigValidationError
 
 from zerver.lib.avatar import avatar_url, get_avatar_field, get_avatar_for_inaccessible_user
 from zerver.lib.cache import cache_with_key, get_cross_realm_dicts_key
+from zerver.lib.create_user import get_dummy_email_address_for_display_regex
 from zerver.lib.exceptions import (
     JsonableError,
     OrganizationAdministratorRequiredError,
@@ -45,7 +46,6 @@ from zerver.models.users import (
     active_non_guest_user_ids,
     active_user_ids,
     get_realm_user_dicts,
-    get_user,
     get_user_by_id_in_realm_including_cross_realm,
     get_user_profile_by_id_in_realm,
     is_cross_realm_bot_email,
@@ -342,8 +342,76 @@ def access_user_by_email(
     allow_bots: bool = False,
     for_admin: bool,
 ) -> UserProfile:
+    """Fetch a user by email address. Endpoints using this function can be queried either with:
+
+    1) The real email address of the intended user, if the requester
+       believes that user exists and allows their email address to be
+       visible to the requester via their `email_address_visibility` setting.
+
+    2) The dummy email address (of the approximate shape
+       'user{user_id}@{realm_dummy_email_domain}') of the intended user. We
+       detect when the format of the provided email address matches
+       the format of our dummy email addresses and extract the user id
+       from it for a regular id-based lookup.
+
+       In particular, this mode is kept around for backwards
+       compatibility with the old behavior of the `GET /users/{email}`
+       endpoint, which required use of the dummy email address for
+       lookups of any user which didn't have
+       EMAIL_ADDRESS_VISIBILITY_EVERYONE set, regardless of how the
+       actual email_address_visibility setting related to the role of
+       the requester.
+
+       Note: If the realm.host value changes (e.g. due to the server moving to a new
+       domain), the required dummy email values passed here will need to be updated
+       accordingly to match the new value. This deviates from the original API behavior,
+       where the lookups were supposed to match the UserProfile.email value, which was
+       **not** updated for existing users even if the server moved domains. See
+       get_fake_email_domain for details of how the email domain for dummy email addresses
+       is determined.
+
+    The purpose of this is to be used at API endpoints that allow selecting the target user by
+    delivery_email, while preventing the endpoint from leaking information about user emails.
+    """
+
+    # First, check if the email is just the dummy email address format. In that case,
+    # we don't need to deal with email lookups or email address visibility restrictions
+    # and we simply get the user by id, extracted from the dummy address.
+    dummy_email_regex = get_dummy_email_address_for_display_regex(user_profile.realm)
+    match = re.match(dummy_email_regex, email)
+    if match:
+        target_id = int(match.group(1))
+        return access_user_by_id(
+            user_profile,
+            target_id,
+            allow_deactivated=allow_deactivated,
+            allow_bots=allow_bots,
+            for_admin=for_admin,
+        )
+
+    # Since the format doesn't match, we should treat it as a lookup
+    # for a real email address.
+    allowed_email_address_visibility_values = (
+        UserProfile.ROLE_TO_ACCESSIBLE_EMAIL_ADDRESS_VISIBILITY_IDS[user_profile.role]
+    )
+
     try:
-        target = get_user(email, user_profile.realm)
+        # Fetch the user from the subset of users which allow the
+        # requester to see their email address. We carefully do this
+        # with a single query to hopefully make timing attacks
+        # ineffective.
+        #
+        # Notably, we use the same select_related as access_user_by_id.
+        target = UserProfile.objects.select_related(
+            "realm",
+            "realm__can_access_all_users_group",
+            "realm__can_access_all_users_group__named_user_group",
+            "bot_owner",
+        ).get(
+            delivery_email__iexact=email.strip(),
+            realm=user_profile.realm,
+            email_address_visibility__in=allowed_email_address_visibility_values,
+        )
     except UserProfile.DoesNotExist:
         raise JsonableError(_("No such user"))
 

zerver/openapi/zulip.yaml

@@ -12369,9 +12369,17 @@ paths:
         Fetching by user ID is generally recommended when possible,
         as a user might [change their email address](/help/change-your-email-address)
         or change their [email address visibility](/help/configure-email-visibility),
-        either of which could change the value of their Zulip API email address.
+        either of which could change the client's ability to look them up by that
+        email address.
 
-        **Changes**: New in Zulip Server 4.0 (feature level 39).
+        **Changes**: Starting with Zulip 10.0 (feature level 302), the real email
+        address can be used in the `email` parameter and will fetch the target user's
+        data if and only if the target's email visibility setting permits the requester
+        to see the email address.
+        The dummy email addresses of the form `user{id}@{realm.host}` still work, and
+        will now work for **all** users, via identifying them by the embedded user ID.
+
+        New in Zulip Server 4.0 (feature level 39).
       x-curl-examples-parameters:
         oneOf:
           - type: include
@@ -12388,7 +12396,24 @@ paths:
         - name: email
           in: path
           description: |
-            The Zulip API email address of the user whose details you want to fetch.
+            The email address of the user to fetch. Two forms are supported:
+
+            - The real email address of the user (`delivery_email`). The lookup will
+              succeed if and only if the user exists and their email address visibility
+              setting permits the client to see the email address.
+
+            - The dummy Zulip API email address of the form `user{user_id}@{realm_host}`. This
+              is identical to simply [getting user by ID](/api/get-user). If the server or
+              realm change domains, the dummy email address used has to be adjustment to
+              match the new realm domain. This is legacy behavior for
+              backwards-compatibility, and will be removed in a future release.
+
+            **Changes**: Starting with Zulip 10.0 (feature level 302), lookups by real email
+            address match the semantics of the target's email visibility setting and dummy
+            email addresses work for all users, independently of their email visibility
+            setting.
+
+            Previously, lookups were done only using the Zulip API email addresses.
           schema:
             type: string
           example: iago@zulip.com

zerver/tests/test_users.py

@@ -2591,31 +2591,82 @@ class GetProfileTest(ZulipTestCase):
     def test_get_user_by_email(self) -> None:
         user = self.example_user("hamlet")
         self.login("hamlet")
-        result = orjson.loads(self.client_get(f"/json/users/{user.email}").content)
+        result = self.client_get(f"/json/users/{user.email}")
+        data = result.json()
 
-        self.assertEqual(result["user"]["email"], user.email)
+        self.assertEqual(data["user"]["email"], user.email)
 
-        self.assertEqual(result["user"]["full_name"], user.full_name)
+        self.assertEqual(data["user"]["full_name"], user.full_name)
-        self.assertIn("user_id", result["user"])
+        self.assertIn("user_id", data["user"])
-        self.assertNotIn("profile_data", result["user"])
+        self.assertNotIn("profile_data", data["user"])
-        self.assertFalse(result["user"]["is_bot"])
+        self.assertFalse(data["user"]["is_bot"])
-        self.assertFalse(result["user"]["is_admin"])
+        self.assertFalse(data["user"]["is_admin"])
-        self.assertFalse(result["user"]["is_owner"])
+        self.assertFalse(data["user"]["is_owner"])
 
-        result = orjson.loads(
+        result = self.client_get(
-            self.client_get(
             f"/json/users/{user.email}", {"include_custom_profile_fields": "true"}
-            ).content
         )
-        self.assertIn("profile_data", result["user"])
+        data = result.json()
+        self.assertIn("profile_data", data["user"])
 
         result = self.client_get("/json/users/invalid")
         self.assert_json_error(result, "No such user")
 
         bot = self.example_user("default_bot")
-        result = orjson.loads(self.client_get(f"/json/users/{bot.email}").content)
+        result = self.client_get(f"/json/users/{bot.email}")
-        self.assertEqual(result["user"]["email"], bot.email)
+        data = result.json()
-        self.assertTrue(result["user"]["is_bot"])
+        self.assertEqual(data["user"]["email"], bot.email)
+        self.assertTrue(data["user"]["is_bot"])
+
+        iago = self.example_user("iago")
+        # Change iago's email address visibility so that hamlet can't see it.
+        do_change_user_setting(
+            iago,
+            "email_address_visibility",
+            UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS,
+            acting_user=None,
+        )
+        # Lookup by delivery email should fail, since hamlet can't access it.
+        result = self.client_get(f"/json/users/{iago.delivery_email}")
+        self.assert_json_error(result, "No such user")
+
+        # Lookup by the externally visible .email succeeds.
+        result = self.client_get(f"/json/users/{iago.email}")
+        data = result.json()
+        self.assertEqual(data["user"]["email"], iago.email)
+        self.assertEqual(data["user"]["delivery_email"], None)
+
+        # Allow members to see iago's email address, thus giving hamlet access.
+        do_change_user_setting(
+            iago,
+            "email_address_visibility",
+            UserProfile.EMAIL_ADDRESS_VISIBILITY_MEMBERS,
+            acting_user=None,
+        )
+        result = self.client_get(f"/json/users/{iago.delivery_email}")
+        data = result.json()
+        self.assertEqual(data["user"]["email"], iago.email)
+        self.assertEqual(data["user"]["delivery_email"], iago.delivery_email)
+
+        # Test the following edge case - when a user has EMAIL_ADDRESS_VISIBILITY_EVERYONE
+        # enabled, both their .email and .delivery_email will be set to the same, real
+        # email address.
+        # Querying for the user by the dummy email address should still work however,
+        # as the API understands the dummy email as a user ID. This is a nicer interface,
+        # as it allow clients not to worry about the implementation details of .email.
+        do_change_user_setting(
+            iago,
+            "email_address_visibility",
+            UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE,
+            acting_user=None,
+        )
+
+        dummy_email = f"user{iago.id}@{get_fake_email_domain(iago.realm.host)}"
+
+        result = self.client_get(f"/json/users/{dummy_email}")
+        data = result.json()
+        self.assertEqual(data["user"]["email"], iago.email)
+        self.assertEqual(data["user"]["delivery_email"], iago.delivery_email)
 
     def test_get_all_profiles_avatar_urls(self) -> None:
         hamlet = self.example_user("hamlet")