upload: Redirect unauthorized anonymous requests to login.

Note that this also redirects rate-limited anonymous requests to the
login page, as we do not currently differentiate the cases.
This commit is contained in:
Alex Vandiver 2023-11-21 21:23:22 +00:00 committed by Tim Abbott
parent f9884af114
commit 82960d9bc2
3 changed files with 18 additions and 9 deletions

View File

@ -216,9 +216,10 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
# without being logged in.
self.logout()
self.assertEqual(self.client_get(url_only_url).getvalue(), b"zulip!")
# The original url shouldn't work when logged out:
# The original url shouldn't work when logged out -- it redirects to the login page
result = self.client_get(url)
self.assertEqual(result.status_code, 403)
self.assertEqual(result.status_code, 302)
self.assertTrue(result.headers["Location"].endswith(f"/login/?next={url}"))
def test_serve_file_unauthed(self) -> None:
self.login("hamlet")
@ -237,7 +238,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
self.logout()
response = self.client_get(url)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, 302)
self.assertTrue(response.headers["Location"].endswith(f"/login/?next={url}"))
# Allow file access for web-public stream
self.login("hamlet")
@ -253,7 +255,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
# Deny file access since rate limited
with ratelimit_rule(86400, 0, domain="spectator_attachment_access_by_file"):
response = self.client_get(url)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, 302)
self.assertTrue(response.headers["Location"].endswith(f"/login/?next={url}"))
# Check that the /download/ variant works as well
download_url = url.replace("/user_uploads/", "/user_uploads/download/")
@ -262,7 +265,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
self.assertEqual(response.status_code, 200)
with ratelimit_rule(86400, 0, domain="spectator_attachment_access_by_file"):
response = self.client_get(download_url)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, 302)
self.assertTrue(response.headers["Location"].endswith(f"/login/?next={download_url}"))
# Deny random file access
response = self.client_get(
@ -323,8 +327,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
self.logout()
response = self.client_get(url)
self.assertEqual(response.status_code, 403)
self.assert_in_response("<p>You are not authorized to view this file.</p>", response)
self.assertEqual(response.status_code, 302)
self.assertTrue(response.headers["Location"].endswith(f"/login/?next={url}"))
def test_image_download_unauthed(self) -> None:
"""
@ -687,7 +691,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
# Verify that cross-realm access to files for spectators is denied.
self.logout()
response = self.client_get(url, subdomain=test_subdomain)
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, 302)
self.assertTrue(response.headers["Location"].endswith(f"/login/?next={url}"))
def test_file_download_authorization_invite_only(self) -> None:
hamlet = self.example_user("hamlet")

View File

@ -238,7 +238,8 @@ class S3Test(ZulipTestCase):
# The original url shouldn't work when logged out:
with self.settings(DEVELOPMENT=False):
result = self.client_get(url)
self.assertEqual(result.status_code, 403)
self.assertEqual(result.status_code, 302)
self.assertTrue(result.headers["Location"].endswith(f"/login/?next={url}"))
hamlet = self.example_user("hamlet")
self.subscribe(hamlet, "Denmark")

View File

@ -25,6 +25,7 @@ from django.utils.http import content_disposition_header
from django.utils.translation import gettext as _
from zerver.context_processors import get_valid_realm_from_request
from zerver.decorator import zulip_redirect_to_login
from zerver.lib.exceptions import JsonableError
from zerver.lib.response import json_success
from zerver.lib.storage import static_path
@ -215,6 +216,8 @@ def serve_file(
if not is_authorized:
if preferred_accept(request, ["text/html", "image/png"]) == "image/png":
response = serve_image_error(403, "images/errors/image-no-auth.png")
elif isinstance(maybe_user_profile, AnonymousUser):
response = zulip_redirect_to_login(request)
else:
response = HttpResponseForbidden(_("<p>You are not authorized to view this file.</p>"))
patch_vary_headers(response, ("Accept",))