avatar: Allow API authentication for /avatar/ routes.

This makes it feasibly for the mobile apps to correctly render user
avatars generated by the `!avatar()` syntax.

zerver/tests/test_upload.py

@@ -862,8 +862,10 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
             self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar'))
 
     def test_get_user_avatar(self) -> None:
-        self.login(self.example_email("hamlet"))
+        hamlet = self.example_email("hamlet")
+        self.login(hamlet)
         cordelia = self.example_user('cordelia')
+        cross_realm_bot = self.example_user('welcome_bot')
 
         cordelia.avatar_source = UserProfile.AVATAR_FROM_USER
         cordelia.save()
@@ -878,8 +880,34 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
         response = self.client_get("/avatar/")
         self.assertEqual(response.status_code, 404)
 
+        self.logout()
+
+        # Test /avatar/<email_or_id> endpoint with HTTP basic auth.
+        response = self.api_get(hamlet, "/avatar/cordelia@zulip.com?foo=bar")
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar'))
+
+        response = self.api_get(hamlet, "/avatar/%s?foo=bar" % (cordelia.id))
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia)) + '&foo=bar'))
+
+        # Test cross_realm_bot avatar access using email.
+        response = self.api_get(hamlet, "/avatar/welcome-bot@zulip.com?foo=bar")
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + '&foo=bar'))
+
+        # Test cross_realm_bot avatar access using id.
+        response = self.api_get(hamlet, "/avatar/%s?foo=bar" % (cross_realm_bot.id))
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cross_realm_bot)) + '&foo=bar'))
+
+        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)
+
     def test_get_user_avatar_medium(self) -> None:
-        self.login(self.example_email("hamlet"))
+        hamlet = self.example_email("hamlet")
+        self.login(hamlet)
         cordelia = self.example_user('cordelia')
 
         cordelia.avatar_source = UserProfile.AVATAR_FROM_USER
@@ -892,6 +920,21 @@ class AvatarTest(UploadSerializeMixin, ZulipTestCase):
         redirect_url = response['Location']
         self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + '&foo=bar'))
 
+        self.logout()
+
+        # Test /avatar/<email_or_id>/medium endpoint with HTTP basic auth.
+        response = self.api_get(hamlet, "/avatar/cordelia@zulip.com/medium?foo=bar")
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + '&foo=bar'))
+
+        response = self.api_get(hamlet, "/avatar/%s/medium?foo=bar" % (cordelia.id,))
+        redirect_url = response['Location']
+        self.assertTrue(redirect_url.endswith(str(avatar_url(cordelia, True)) + '&foo=bar'))
+
+        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)
+
     def test_non_valid_user_avatar(self) -> None:
 
         # It's debatable whether we should generate avatars for non-users,

zerver/views/users.py

@@ -34,9 +34,9 @@ from zerver.lib.users import check_valid_bot_type, check_bot_creation_policy, \
     access_bot_by_id, add_service, access_user_by_id
 from zerver.lib.utils import generate_api_key, generate_random_token
 from zerver.models import UserProfile, Stream, Message, email_allowed_for_realm, \
-    get_user_profile_by_id, get_user, Service, get_user_including_cross_realm, \
+    get_user, Service, get_user_including_cross_realm, \
     DomainNotAllowedForRealmError, DisposableEmailError, get_user_profile_by_id_in_realm, \
-    EmailContainsPlusError
+    EmailContainsPlusError, get_user_by_id_in_realm_including_cross_realm
 
 def deactivate_user_backend(request: HttpRequest, user_profile: UserProfile,
                             user_id: int) -> HttpResponse:
@@ -94,11 +94,8 @@ def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id
 
     return json_success()
 
-# TODO: Since eventually we want to support using the same email with
-# different organizations, we'll eventually want this to be a
-# logged-in endpoint so that we can access the realm_id.
-@zulip_login_required
-def avatar(request: HttpRequest, email_or_id: str, medium: bool=False) -> HttpResponse:
+def avatar(request: HttpRequest, user_profile: UserProfile,
+           email_or_id: str, medium: bool=False) -> HttpResponse:
     """Accepts an email address or user ID and returns the avatar"""
     is_email = False
     try:
@@ -107,11 +104,11 @@ def avatar(request: HttpRequest, email_or_id: str, medium: bool=False) -> HttpRe
         is_email = True
 
     try:
+        realm = user_profile.realm
         if is_email:
-            realm = request.user.realm
             avatar_user_profile = get_user_including_cross_realm(email_or_id, realm)
         else:
-            avatar_user_profile = get_user_profile_by_id(email_or_id)
+            avatar_user_profile = get_user_by_id_in_realm_including_cross_realm(int(email_or_id), realm)
         # If there is a valid user account passed in, use its avatar
         url = avatar_url(avatar_user_profile, medium=medium)
     except UserProfile.DoesNotExist:

zproject/urls.py

@@ -413,12 +413,6 @@ i18n_urls = [
         zerver.views.auth.show_deactivation_notice,
         name='zerver.views.auth.show_deactivation_notice'),
 
-    # Avatar
-    url(r'^avatar/(?P<email_or_id>[\S]+)/(?P<medium>[\S]+)?', zerver.views.users.avatar,
-        name='zerver.views.users.avatar'),
-    url(r'^avatar/(?P<email_or_id>[\S]+)', zerver.views.users.avatar,
-        name='zerver.views.users.avatar'),
-
     # Registration views, require a confirmation ID.
     url(r'^accounts/register/social/(\w+)$',
         zerver.views.auth.start_social_signup,
@@ -539,15 +533,26 @@ urls += [
 # easily support the mobile apps fetching uploaded files without
 # having to rewrite URLs, and is implemented using the
 # 'override_api_url_scheme' flag passed to rest_dispatch
-urls += url(r'^user_uploads/(?P<realm_id_str>(\d*|unk))/(?P<filename>.*)',
-            rest_dispatch,
-            {'GET': ('zerver.views.upload.serve_file_backend',
-                     {'override_api_url_scheme'})}),
-# This endpoint serves thumbnailed versions of images using thumbor;
-# it requires an exception for the same reason.
-urls += url(r'^thumbnail', rest_dispatch,
-            {'GET': ('zerver.views.thumbnail.backend_serve_thumbnail',
-                     {'override_api_url_scheme'})}),
+urls += [
+    url(r'^user_uploads/(?P<realm_id_str>(\d*|unk))/(?P<filename>.*)',
+        rest_dispatch,
+        {'GET': ('zerver.views.upload.serve_file_backend',
+                 {'override_api_url_scheme'})}),
+    # This endpoint serves thumbnailed versions of images using thumbor;
+    # it requires an exception for the same reason.
+    url(r'^thumbnail', rest_dispatch,
+        {'GET': ('zerver.views.thumbnail.backend_serve_thumbnail',
+                 {'override_api_url_scheme'})}),
+    # Avatars have the same constraint due to `!avatar` syntax.
+    url(r'^avatar/(?P<email_or_id>[\S]+)/(?P<medium>[\S]+)?',
+        rest_dispatch,
+        {'GET': ('zerver.views.users.avatar',
+                 {'override_api_url_scheme'})}),
+    url(r'^avatar/(?P<email_or_id>[\S]+)',
+        rest_dispatch,
+        {'GET': ('zerver.views.users.avatar',
+                 {'override_api_url_scheme'})}),
+]
 
 # This url serves as a way to recieve CSP violation reports from the users.
 # We use this endpoint to just log these reports.