diff --git a/zerver/models.py b/zerver/models.py index 336adfa80b..799ed2f23d 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1205,6 +1205,22 @@ class Attachment(ModelReprMixin, models.Model): # type: () -> text_type return u"/user_uploads/%s" % (self.path_id) +def validate_attachment_request(user_profile, path_id): + try: + attachment = Attachment.objects.get(path_id=path_id) + messages = attachment.messages.all() + + if user_profile == attachment.owner: + return True + elif attachment.is_realm_public and attachment.realm == user_profile.realm: + return True + elif UserMessage.objects.filter(user_profile=user_profile, message__in=messages).exists(): + return True + else: + return False + except Attachment.DoesNotExist: + return None + def get_attachments_by_owner_id(uid): # type: (int) -> Sequence[Attachment] # TODO: Change return type to QuerySet[Attachment] diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 52e6ebc0a2..a760a9c07c 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -11,8 +11,9 @@ from zerver.lib.upload import sanitize_name, S3UploadBackend, \ upload_message_image, delete_message_image, LocalUploadBackend import zerver.lib.upload from zerver.models import Attachment, Recipient, get_user_profile_by_email, \ - get_old_unclaimed_attachments + get_old_unclaimed_attachments, Stream, Realm, get_realm from zerver.lib.actions import do_delete_old_unclaimed_attachments +from zilencer.models import Deployment import ujson from six.moves import urllib @@ -130,9 +131,48 @@ class FileUploadTest(AuthedTestCase): self.send_message("hamlet@zulip.com", "Denmark", Recipient.STREAM, body, "test") self.assertIn('title="zulip.txt"', self.get_last_message().rendered_content) + def test_file_download_unauthed(self): + # type: () -> None + self.login("hamlet@zulip.com") + fp = StringIO("zulip!") + fp.name = "zulip.txt" + result = self.client.post("/json/upload_file", {'file': fp}) + json = ujson.loads(result.content) + uri = json["uri"] + + self.client.post('/accounts/logout/') + response = self.client.get(uri) + self.assert_json_error(response, "Not logged in: API authentication or user session required", + status_code=401) + + def test_removed_file_download(self): + # type: () -> None + ''' + Trying to download deleted files should return 404 error + ''' + self.login("hamlet@zulip.com") + fp = StringIO("zulip!") + fp.name = "zulip.txt" + result = self.client.post("/json/upload_file", {'file': fp}) + json = ujson.loads(result.content) + uri = json["uri"] + + destroy_uploads() + + response = self.client.get(uri) + self.assertEqual(response.status_code, 404) + + def test_non_existing_file_download(self): + # type: () -> None + ''' + Trying to download a file that was never uploaded will return a json_error + ''' + self.login("hamlet@zulip.com") + response = self.client.get("http://localhost:9991/user_uploads/1/ff/gg/abc.py") + self.assert_json_error(response, 'That file does not exist.', status_code=404) + def test_delete_old_unclaimed_attachments(self): # type: () -> None - # Upload some files and make them older than a weeek self.login("hamlet@zulip.com") d1 = StringIO("zulip!") @@ -189,6 +229,123 @@ class FileUploadTest(AuthedTestCase): self.assertEquals(Attachment.objects.get(path_id=d1_path_id).messages.count(), 2) + def test_cross_realm_file_access(self): + # type: () -> None + + def create_user(email): + username, domain = email.split('@') + self.register(username, 'test', domain=domain) + return get_user_profile_by_email(email) + + user1_email = 'user1@uploadtest.example.com' + user2_email = 'test-og-bot@zulip.com' + user3_email = 'other-user@uploadtest.example.com' + + settings.CROSS_REALM_BOT_EMAILS.add(user2_email) + settings.CROSS_REALM_BOT_EMAILS.add(user3_email) + dep = Deployment() + dep.base_api_url = "https://zulip.com/api/" + dep.base_site_url = "https://zulip.com/" + # We need to save the object before we can access + # the many-to-many relationship 'realms' + dep.save() + dep.realms = [get_realm("zulip.com")] + dep.save() + + r1 = Realm.objects.create(domain='uploadtest.example.com') + deployment = Deployment.objects.filter()[0] + deployment.realms.add(r1) + + create_user(user1_email) + create_user(user2_email) + create_user(user3_email) + + # Send a message from @zulip.com -> @uploadtest.example.com + self.login(user2_email, 'test') + fp = StringIO("zulip!") + fp.name = "zulip.txt" + result = self.client.post("/json/upload_file", {'file': fp}) + json = ujson.loads(result.content) + uri = json["uri"] + fp_path_id = re.sub('/user_uploads/', '', uri) + body = "First message ...[zulip.txt](http://localhost:9991/user_uploads/" + fp_path_id + ")" + self.send_message(user2_email, user1_email, Recipient.PERSONAL, body) + + self.login(user1_email, 'test') + response = self.client.get(uri) + self.assertEqual(response.status_code, 200) + data = "".join(response.streaming_content) + self.assertEquals("zulip!", data) + self.client.post('/accounts/logout/') + + # Confirm other cross-realm users can't read it. + self.login(user3_email, 'test') + response = self.client.get(uri) + self.assert_json_error(response, "You are not authorized to view this file.", status_code=403) + + def test_file_download_authorization_invite_only(self): + subscribed_users = ["hamlet@zulip.com", "iago@zulip.com"] + unsubscribed_users = ["othello@zulip.com", "prospero@zulip.com"] + for user in subscribed_users: + self.subscribe_to_stream(user, "test-subscribe") + + # Make the stream private + stream = Stream.objects.get(name='test-subscribe') + stream.invite_only = True + stream.save() + + self.login("hamlet@zulip.com") + fp = StringIO("zulip!") + fp.name = "zulip.txt" + result = self.client.post("/json/upload_file", {'file': fp}) + json = ujson.loads(result.content) + uri = json["uri"] + fp_path_id = re.sub('/user_uploads/', '', uri) + body = "First message ...[zulip.txt](http://localhost:9991/user_uploads/" + fp_path_id + ")" + self.send_message("hamlet@zulip.com", "test-subscribe", Recipient.STREAM, body, "test") + self.client.post('/accounts/logout/') + + # Subscribed user should be able to view file + for user in subscribed_users: + self.login(user) + response = self.client.get(uri) + self.assertEqual(response.status_code, 200) + data = "".join(response.streaming_content) + self.assertEquals("zulip!", data) + self.client.post('/accounts/logout/') + + # Unsubscribed user should not be able to view file + for user in unsubscribed_users: + self.login(user) + response = self.client.get(uri) + self.assert_json_error(response, "You are not authorized to view this file.", status_code=403) + self.client.post('/accounts/logout/') + + def test_file_download_authorization_public(self): + subscribed_users = ["hamlet@zulip.com", "iago@zulip.com"] + unsubscribed_users = ["othello@zulip.com", "prospero@zulip.com"] + for user in subscribed_users: + self.subscribe_to_stream(user, "test-subscribe") + + self.login("hamlet@zulip.com") + fp = StringIO("zulip!") + fp.name = "zulip.txt" + result = self.client.post("/json/upload_file", {'file': fp}) + json = ujson.loads(result.content) + uri = json["uri"] + fp_path_id = re.sub('/user_uploads/', '', uri) + body = "First message ...[zulip.txt](http://localhost:9991/user_uploads/" + fp_path_id + ")" + self.send_message("hamlet@zulip.com", "test-subscribe", Recipient.STREAM, body, "test") + self.client.post('/accounts/logout/') + + # Now all users should be able to access the files + for user in subscribed_users + unsubscribed_users: + self.login(user) + response = self.client.get(uri) + data = "".join(response.streaming_content) + self.assertEquals("zulip!", data) + self.client.post('/accounts/logout/') + def tearDown(self): # type: () -> None destroy_uploads() diff --git a/zerver/views/upload.py b/zerver/views/upload.py index cfcc693b91..06c158b2e1 100644 --- a/zerver/views/upload.py +++ b/zerver/views/upload.py @@ -11,7 +11,7 @@ from zerver.lib.response import json_success, json_error from zerver.lib.upload import upload_message_image_from_request, get_local_file_path, \ get_signed_upload_url, get_realm_for_filename from zerver.lib.validator import check_bool -from zerver.models import UserProfile +from zerver.models import UserProfile, validate_attachment_request from django.conf import settings def serve_s3(request, user_profile, realm_id_str, filename, redir): @@ -54,6 +54,12 @@ def serve_file_backend(request, user_profile, realm_id_str, filename, redir=REQ(validator=check_bool, default=True)): # type: (HttpRequest, UserProfile, str, str, bool) -> HttpResponse path_id = "%s/%s" % (realm_id_str, filename) + is_authorized = validate_attachment_request(user_profile, path_id) + + if is_authorized is None: + return json_error(_("That file does not exist."), status=404) + if not is_authorized: + return json_error(_("You are not authorized to view this file."), status=403) if settings.LOCAL_UPLOADS_DIR is not None: return serve_local(request, path_id)