users: Allow spectators to view user avatars.

If realm is web_public, spectators can now view avatar of other
users.

There is a special exception we had to introduce in rest model to
allow `/avatar` type of urls for `anonymous` access, because they
don't have the /api/v1 prefix.

Fixes #19838.

zerver/lib/rest.py

@@ -147,7 +147,10 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
             target_function = authenticated_rest_api_view(
                 allow_webhook_access="allow_incoming_webhooks" in view_flags,
             )(target_function)
-        elif request.path.startswith("/json") and "allow_anonymous_user_web" in view_flags:
+        elif (
+            request.path.startswith(("/json", "/avatar"))
+            and "allow_anonymous_user_web" in view_flags
+        ):
             # For endpoints that support anonymous web access, we do that.
             # TODO: Allow /api calls when this is stable enough.
             auth_kwargs = dict(allow_unauthenticated=True)

zerver/tests/test_upload.py

@@ -1047,28 +1047,44 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
 
         self.logout()
 
-        # Test /avatar/<email_or_id> endpoint with HTTP basic auth.
+        with self.settings(WEB_PUBLIC_STREAMS_ENABLED=False):
-        response = self.api_get(hamlet, "/avatar/cordelia@zulip.com", {"foo": "bar"})
+            # Test /avatar/<email_or_id> endpoint with HTTP basic auth.
-        redirect_url = response["Location"]
+            response = self.api_get(hamlet, "/avatar/cordelia@zulip.com", {"foo": "bar"})
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + "&foo=bar"))
+            redirect_url = response["Location"]
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + "&foo=bar"))
 
-        response = self.api_get(hamlet, f"/avatar/{cordelia.id}", {"foo": "bar"})
+            response = self.api_get(hamlet, f"/avatar/{cordelia.id}", {"foo": "bar"})
-        redirect_url = response["Location"]
+            redirect_url = response["Location"]
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + "&foo=bar"))
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + "&foo=bar"))
 
-        # Test cross_realm_bot avatar access using email.
+            # Test cross_realm_bot avatar access using email.
-        response = self.api_get(hamlet, "/avatar/welcome-bot@zulip.com", {"foo": "bar"})
+            response = self.api_get(hamlet, "/avatar/welcome-bot@zulip.com", {"foo": "bar"})
-        redirect_url = response["Location"]
+            redirect_url = response["Location"]
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + "&foo=bar"))
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + "&foo=bar"))
 
-        # Test cross_realm_bot avatar access using id.
+            # Test cross_realm_bot avatar access using id.
-        response = self.api_get(hamlet, f"/avatar/{cross_realm_bot.id}", {"foo": "bar"})
+            response = self.api_get(hamlet, f"/avatar/{cross_realm_bot.id}", {"foo": "bar"})
-        redirect_url = response["Location"]
+            redirect_url = response["Location"]
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + "&foo=bar"))
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + "&foo=bar"))
 
+            # Without spectators enabled, no unauthenticated access.
+            response = self.client_get("/avatar/cordelia@zulip.com", {"foo": "bar"})
+            self.assert_json_error(
+                response,
+                "Not logged in: API authentication or user session required",
+                status_code=401,
+            )
+
+        # Allow unauthenticated/spectator requests by ID.
+        response = self.client_get(f"/avatar/{cordelia.id}", {"foo": "bar"})
+        self.assertEqual(302, response.status_code)
+
+        # Disallow unauthenticated/spectator requests by email.
         response = self.client_get("/avatar/cordelia@zulip.com", {"foo": "bar"})
         self.assert_json_error(
-            response, "Not logged in: API authentication or user session required", status_code=401
+            response,
+            "Not logged in: API authentication or user session required",
+            status_code=401,
         )
 
     def test_get_user_avatar_medium(self) -> None:
@@ -1090,18 +1106,34 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
 
         self.logout()
 
-        # Test /avatar/<email_or_id>/medium endpoint with HTTP basic auth.
+        with self.settings(WEB_PUBLIC_STREAMS_ENABLED=False):
-        response = self.api_get(hamlet, "/avatar/cordelia@zulip.com/medium", {"foo": "bar"})
+            # Test /avatar/<email_or_id>/medium endpoint with HTTP basic auth.
-        redirect_url = response["Location"]
+            response = self.api_get(hamlet, "/avatar/cordelia@zulip.com/medium", {"foo": "bar"})
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + "&foo=bar"))
+            redirect_url = response["Location"]
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + "&foo=bar"))
 
-        response = self.api_get(hamlet, f"/avatar/{cordelia.id}/medium", {"foo": "bar"})
+            response = self.api_get(hamlet, f"/avatar/{cordelia.id}/medium", {"foo": "bar"})
-        redirect_url = response["Location"]
+            redirect_url = response["Location"]
-        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + "&foo=bar"))
+            self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + "&foo=bar"))
 
+            # Without spectators enabled, no unauthenticated access.
+            response = self.client_get("/avatar/cordelia@zulip.com/medium", {"foo": "bar"})
+            self.assert_json_error(
+                response,
+                "Not logged in: API authentication or user session required",
+                status_code=401,
+            )
+
+        # Allow unauthenticated/spectator requests by ID.
+        response = self.client_get(f"/avatar/{cordelia.id}/medium", {"foo": "bar"})
+        self.assertEqual(302, response.status_code)
+
+        # Disallow unauthenticated/spectator requests by email.
         response = self.client_get("/avatar/cordelia@zulip.com/medium", {"foo": "bar"})
         self.assert_json_error(
-            response, "Not logged in: API authentication or user session required", status_code=401
+            response,
+            "Not logged in: API authentication or user session required",
+            status_code=401,
         )
 
     def test_non_valid_user_avatar(self) -> None:

zerver/views/users.py

@@ -1,10 +1,12 @@
 from typing import Any, Dict, List, Optional, Union
 
 from django.conf import settings
+from django.contrib.auth.models import AnonymousUser
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import redirect
 from django.utils.translation import gettext as _
 
+from zerver.context_processors import get_valid_realm_from_request
 from zerver.decorator import require_member_or_admin, require_realm_admin
 from zerver.forms import PASSWORD_TOO_WEAK_ERROR, CreateUserForm
 from zerver.lib.actions import (
@@ -32,6 +34,7 @@ from zerver.lib.email_validation import email_allowed_for_realm
 from zerver.lib.exceptions import (
     CannotDeactivateLastUserError,
     JsonableError,
+    MissingAuthenticationError,
     OrganizationOwnerRequired,
 )
 from zerver.lib.integrations import EMBEDDED_BOTS
@@ -219,7 +222,10 @@ def update_user_backend(
 
 
 def avatar(
-    request: HttpRequest, user_profile: UserProfile, email_or_id: str, medium: bool = False
+    request: HttpRequest,
+    maybe_user_profile: Union[UserProfile, AnonymousUser],
+    email_or_id: str,
+    medium: bool = False,
 ) -> HttpResponse:
     """Accepts an email address or user ID and returns the avatar"""
     is_email = False
@@ -228,8 +234,25 @@ def avatar(
     except ValueError:
         is_email = True
 
+    if not maybe_user_profile.is_authenticated:
+        # Allow anonynous access to avatars only if spectators are
+        # enabled in the organization.
+        realm = get_valid_realm_from_request(request)
+        # TODO: Replace with realm.allow_web_public_streams_access()
+        # when the method is available.
+        if not realm.has_web_public_streams():
+            raise MissingAuthenticationError()
+
+        # We only allow the ID format for accessing a user's avatar
+        # for spectators. This is mainly for defense in depth, since
+        # email_address_visibility should mean spectators only
+        # interact with fake email addresses anyway.
+        if is_email:
+            raise MissingAuthenticationError()
+    else:
+        realm = maybe_user_profile.realm
+
     try:
-        realm = user_profile.realm
         if is_email:
             avatar_user_profile = get_user_including_cross_realm(email_or_id, realm)
         else:

zproject/urls.py

@@ -669,9 +669,14 @@ urls += [
     rest_path("thumbnail", GET=(backend_serve_thumbnail, {"override_api_url_scheme"})),
     # Avatars have the same constraint because their URLs are included
     # in API data structures used by both the mobile and web clients.
-    rest_path("avatar/<email_or_id>", GET=(avatar, {"override_api_url_scheme"})),
     rest_path(
-        "avatar/<email_or_id>/medium", {"medium": True}, GET=(avatar, {"override_api_url_scheme"})
+        "avatar/<email_or_id>",
+        GET=(avatar, {"override_api_url_scheme", "allow_anonymous_user_web"}),
+    ),
+    rest_path(
+        "avatar/<email_or_id>/medium",
+        {"medium": True},
+        GET=(avatar, {"override_api_url_scheme", "allow_anonymous_user_web"}),
     ),
 ]