mirror of
https://github.com/zulip/zulip.git
synced 2025-11-09 16:37:23 +00:00
tusd: Reject tusd terminations after we insert them into our database.
The tusd protocol allows DELETE requests ("terminations") at any
point, including after a file has successfully been uploaded. This
can allow tusd to remove a file from the bucket, out from under Zulip.
We use the new-in-2.7.0 pre-terminate hook to look up the file which
the client is requesting to terminate, and reject the termination if
it is a file that the Zulip database is already aware of.
This commit is contained in:
committed by
Tim Abbott
parent
21eff33875
commit
cf51013bb7
@@ -55,7 +55,7 @@ class Command(BaseCommand):
|
|||||||
"-behind-proxy",
|
"-behind-proxy",
|
||||||
f"-hooks-http={hooks_http}",
|
f"-hooks-http={hooks_http}",
|
||||||
"-hooks-http-forward-headers=Cookie,Authorization",
|
"-hooks-http-forward-headers=Cookie,Authorization",
|
||||||
"--hooks-enabled-events=pre-create,pre-finish",
|
"--hooks-enabled-events=pre-create,pre-finish,pre-terminate",
|
||||||
"-disable-download",
|
"-disable-download",
|
||||||
"--show-startup-logs=false",
|
"--show-startup-logs=false",
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -8,7 +8,12 @@ from typing_extensions import ParamSpec
|
|||||||
from zerver.lib.cache import cache_delete, get_realm_used_upload_space_cache_key
|
from zerver.lib.cache import cache_delete, get_realm_used_upload_space_cache_key
|
||||||
from zerver.lib.test_classes import ZulipTestCase
|
from zerver.lib.test_classes import ZulipTestCase
|
||||||
from zerver.lib.test_helpers import create_s3_buckets, find_key_by_email, use_s3_backend
|
from zerver.lib.test_helpers import create_s3_buckets, find_key_by_email, use_s3_backend
|
||||||
from zerver.lib.upload import sanitize_name, upload_backend, upload_message_attachment
|
from zerver.lib.upload import (
|
||||||
|
create_attachment,
|
||||||
|
sanitize_name,
|
||||||
|
upload_backend,
|
||||||
|
upload_message_attachment,
|
||||||
|
)
|
||||||
from zerver.lib.upload.s3 import S3UploadBackend
|
from zerver.lib.upload.s3 import S3UploadBackend
|
||||||
from zerver.lib.utils import assert_is_not_none
|
from zerver.lib.utils import assert_is_not_none
|
||||||
from zerver.models import Attachment, PreregistrationRealm, Realm
|
from zerver.models import Attachment, PreregistrationRealm, Realm
|
||||||
@@ -662,3 +667,76 @@ class TusdPreFinishTest(ZulipTestCase):
|
|||||||
|
|
||||||
response = bucket.Object(f"{path_id}.info").get()
|
response = bucket.Object(f"{path_id}.info").get()
|
||||||
self.assertEqual(response["ContentType"], "binary/octet-stream")
|
self.assertEqual(response["ContentType"], "binary/octet-stream")
|
||||||
|
|
||||||
|
|
||||||
|
class TusdPreTerminateTest(ZulipTestCase):
|
||||||
|
def request(self, info: TusUpload) -> TusHook:
|
||||||
|
return TusHook(
|
||||||
|
type="pre-terminate",
|
||||||
|
event=TusEvent(
|
||||||
|
upload=info,
|
||||||
|
http_request=TusHTTPRequest(
|
||||||
|
method="PATCH",
|
||||||
|
uri=f"/api/v1/tus/{info.id}",
|
||||||
|
remote_addr="12.34.56.78",
|
||||||
|
header={},
|
||||||
|
),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_tusd_pre_terminate_hook(self) -> None:
|
||||||
|
self.login("hamlet")
|
||||||
|
hamlet = self.example_user("hamlet")
|
||||||
|
|
||||||
|
# Act like tusd does -- put the file and its .info in place
|
||||||
|
path_id = upload_backend.generate_message_upload_path(
|
||||||
|
str(hamlet.realm.id), sanitize_name("zulip.txt")
|
||||||
|
)
|
||||||
|
upload_backend.upload_message_attachment(
|
||||||
|
path_id, "zulip.txt", "text/plain", b"zulip!", hamlet
|
||||||
|
)
|
||||||
|
|
||||||
|
info = TusUpload(
|
||||||
|
id=path_id,
|
||||||
|
size=len("zulip!"),
|
||||||
|
offset=0,
|
||||||
|
size_is_deferred=False,
|
||||||
|
meta_data={
|
||||||
|
"filename": "zulip.txt",
|
||||||
|
"filetype": "text/plain",
|
||||||
|
"name": "zulip.txt",
|
||||||
|
"type": "text/plain",
|
||||||
|
},
|
||||||
|
is_final=False,
|
||||||
|
is_partial=False,
|
||||||
|
partial_uploads=None,
|
||||||
|
storage=None,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Try to terminate the upload before it's in Attachments
|
||||||
|
result = self.client_post(
|
||||||
|
"/api/internal/tusd",
|
||||||
|
self.request(info).model_dump(),
|
||||||
|
content_type="application/json",
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
self.assertEqual(result.json(), {})
|
||||||
|
|
||||||
|
# Make the attachment
|
||||||
|
create_attachment(
|
||||||
|
"zulip.txt",
|
||||||
|
path_id,
|
||||||
|
"text/plain",
|
||||||
|
b"zulip!",
|
||||||
|
hamlet,
|
||||||
|
hamlet.realm,
|
||||||
|
)
|
||||||
|
|
||||||
|
# The terminate should get rejected now
|
||||||
|
result = self.client_post(
|
||||||
|
"/api/internal/tusd",
|
||||||
|
self.request(info).model_dump(),
|
||||||
|
content_type="application/json",
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
self.assertEqual(result.json(), {"RejectTermination": True})
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ from zerver.lib.upload import (
|
|||||||
sanitize_name,
|
sanitize_name,
|
||||||
upload_backend,
|
upload_backend,
|
||||||
)
|
)
|
||||||
from zerver.models import PreregistrationRealm, Realm, UserProfile
|
from zerver.models import ArchivedAttachment, Attachment, PreregistrationRealm, Realm, UserProfile
|
||||||
|
|
||||||
|
|
||||||
# See https://tus.github.io/tusd/advanced-topics/hooks/ for the spec
|
# See https://tus.github.io/tusd/advanced-topics/hooks/ for the spec
|
||||||
@@ -219,6 +219,22 @@ def handle_upload_pre_finish_hook(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def handle_upload_pre_terminate_hook(
|
||||||
|
request: HttpRequest, user_profile: UserProfile, data: TusUpload
|
||||||
|
) -> HttpResponse:
|
||||||
|
path_id = data.id.partition("+")[0]
|
||||||
|
|
||||||
|
if (
|
||||||
|
Attachment.objects.filter(path_id=path_id).exists()
|
||||||
|
or ArchivedAttachment.objects.filter(path_id=path_id).exists()
|
||||||
|
):
|
||||||
|
# Once we have it in our Attachments table (i.e. the
|
||||||
|
# pre-upload-finished hook has run), it is ours to manage and
|
||||||
|
# we no longer accept terminations.
|
||||||
|
return tusd_json_response({"RejectTermination": True})
|
||||||
|
return tusd_json_response({})
|
||||||
|
|
||||||
|
|
||||||
def authenticate_user(request: HttpRequest) -> UserProfile | AnonymousUser:
|
def authenticate_user(request: HttpRequest) -> UserProfile | AnonymousUser:
|
||||||
# This acts like the authenticated_rest_api_view wrapper, while
|
# This acts like the authenticated_rest_api_view wrapper, while
|
||||||
# allowing fallback to session-based request.user
|
# allowing fallback to session-based request.user
|
||||||
@@ -291,6 +307,8 @@ def handle_tusd_hook(
|
|||||||
return handle_upload_pre_create_hook(request, maybe_user, payload.event.upload)
|
return handle_upload_pre_create_hook(request, maybe_user, payload.event.upload)
|
||||||
elif hook_name == "pre-finish":
|
elif hook_name == "pre-finish":
|
||||||
return handle_upload_pre_finish_hook(request, maybe_user, payload.event.upload)
|
return handle_upload_pre_finish_hook(request, maybe_user, payload.event.upload)
|
||||||
|
elif hook_name == "pre-terminate":
|
||||||
|
return handle_upload_pre_terminate_hook(request, maybe_user, payload.event.upload)
|
||||||
else:
|
else:
|
||||||
return HttpResponseNotFound()
|
return HttpResponseNotFound()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user