paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-01 12:33:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Add authorization check before serving files."

Browse Source 
This reverts commit e985b57259.

This commit will break production when we next do a release, because
we haven't done a migration to create Attachment objects for
previously uploaded files.
This commit is contained in:
Tim Abbott
2016-06-27 12:09:56 -07:00
parent f7e87bc1f0
commit b2a24e0306
3 changed files with 3 additions and 182 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
zerver/models.py
View File

@@ -1205,22 +1205,6 @@ 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]

161
zerver/tests/test_upload.py
View File

@@ -11,9 +11,8 @@ 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, Stream, Realm, get_realm
get_old_unclaimed_attachments
from zerver.lib.actions import do_delete_old_unclaimed_attachments
from zilencer.models import Deployment
import ujson
from six.moves import urllib
@@ -131,48 +130,9 @@ 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!")
@@ -229,123 +189,6 @@ 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()

8
zerver/views/upload.py
View File

@@ -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, validate_attachment_request
from zerver.models import UserProfile
from django.conf import settings
def serve_s3(request, user_profile, realm_id_str, filename, redir):
@@ -54,12 +54,6 @@ 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)