attachments: Add is_web_public field.

This commit adds the is_web_public field in the AbstractAttachment
class. This is useful when validating user access to the attachment,
as otherwise we would have to make a query in the db to check if
that attachment was sent in a message in a web-public stream or not.
This commit is contained in:
Clara Dantas
2020-07-31 22:17:21 -03:00
committed by Tim Abbott
parent f9651a1e98
commit 05bf72a75c
5 changed files with 52 additions and 5 deletions

View File

@@ -5602,8 +5602,11 @@ def do_claim_attachments(message: Message, potential_path_ids: List[str]) -> boo
for path_id in potential_path_ids:
user_profile = message.sender
is_message_realm_public = False
is_message_web_public = False
if message.is_stream_message():
is_message_realm_public = Stream.objects.get(id=message.recipient.type_id).is_public()
stream = Stream.objects.get(id=message.recipient.type_id)
is_message_realm_public = stream.is_public()
is_message_web_public = stream.is_web_public
if not validate_attachment_request(user_profile, path_id):
# Technically, there are 2 cases here:
@@ -5622,7 +5625,10 @@ def do_claim_attachments(message: Message, potential_path_ids: List[str]) -> boo
continue
claimed = True
attachment = claim_attachment(user_profile, path_id, message, is_message_realm_public)
attachment = claim_attachment(user_profile,
path_id, message,
is_message_realm_public,
is_message_web_public)
notify_attachment_update(user_profile, "update", attachment.to_dict())
return claimed

View File

@@ -879,9 +879,11 @@ def upload_message_file(uploaded_file_name: str, uploaded_file_size: int,
def claim_attachment(user_profile: UserProfile,
path_id: str,
message: Message,
is_message_realm_public: bool) -> Attachment:
is_message_realm_public: bool,
is_message_web_public: bool=False) -> Attachment:
attachment = Attachment.objects.get(path_id=path_id)
attachment.messages.add(message)
attachment.is_web_public = attachment.is_web_public or is_message_web_public
attachment.is_realm_public = attachment.is_realm_public or is_message_realm_public
attachment.save()
return attachment

View File

@@ -0,0 +1,23 @@
# Generated by Django 2.2.14 on 2020-07-31 21:15
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0299_subscription_role'),
]
operations = [
migrations.AddField(
model_name='archivedattachment',
name='is_web_public',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='attachment',
name='is_web_public',
field=models.BooleanField(default=False),
),
]

View File

@@ -2141,12 +2141,18 @@ class AbstractAttachment(models.Model):
# Size of the uploaded file, in bytes
size: int = models.IntegerField()
# The two fields below lets us avoid looking up the corresponding
# messages/streams to check permissions before serving these files.
# Whether this attachment has been posted to a public stream, and
# thus should be available to all non-guest users in the
# organization (even if they weren't a recipient of a message
# linking to it). This lets us avoid looking up the corresponding
# messages/streams to check permissions before serving these files.
# linking to it).
is_realm_public: bool = models.BooleanField(default=False)
# Whether this attachment has been posted to a web-public stream,
# and thus should be available to everyone on the internet, even
# if the person isn't logged in.
is_web_public: bool = models.BooleanField(default=False)
class Meta:
abstract = True

View File

@@ -403,18 +403,28 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
and 'but lacks permission' in warn_log.output[0])
self.assertEqual(Attachment.objects.get(path_id=d1_path_id).messages.count(), 1)
self.assertFalse(Attachment.objects.get(path_id=d1_path_id).is_realm_public)
self.assertFalse(Attachment.objects.get(path_id=d1_path_id).is_web_public)
# Then, have the owner PM it to another user, giving that other user access.
body = f"Second message ...[zulip.txt](http://{host}/user_uploads/" + d1_path_id + ")"
self.send_personal_message(self.example_user("hamlet"), self.example_user("othello"), body)
self.assertEqual(Attachment.objects.get(path_id=d1_path_id).messages.count(), 2)
self.assertFalse(Attachment.objects.get(path_id=d1_path_id).is_realm_public)
self.assertFalse(Attachment.objects.get(path_id=d1_path_id).is_web_public)
# Then, have that new recipient user publish it.
body = f"Third message ...[zulip.txt](http://{host}/user_uploads/" + d1_path_id + ")"
self.send_stream_message(self.example_user("othello"), "Denmark", body, "test")
self.assertEqual(Attachment.objects.get(path_id=d1_path_id).messages.count(), 3)
self.assertTrue(Attachment.objects.get(path_id=d1_path_id).is_realm_public)
self.assertFalse(Attachment.objects.get(path_id=d1_path_id).is_web_public)
# Finally send to Rome, the web-public stream, and confirm it's now web-public
body = f"Fourth message ...[zulip.txt](http://{host}/user_uploads/" + d1_path_id + ")"
self.send_stream_message(self.example_user("othello"), "Rome", body, "test")
self.assertEqual(Attachment.objects.get(path_id=d1_path_id).messages.count(), 4)
self.assertTrue(Attachment.objects.get(path_id=d1_path_id).is_realm_public)
self.assertTrue(Attachment.objects.get(path_id=d1_path_id).is_web_public)
def test_check_attachment_reference_update(self) -> None:
f1 = StringIO("file1")