From 12352f0f77bf54c2f07681d2e33e71a223f9ca28 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 20 Aug 2018 15:29:41 -0700 Subject: Send required object storage PUT headers in /uploads/authorize API As revealed in https://gitlab.com/gitlab-org/gitlab-ce/issues/49957, Rails generates a signed URL with a fixed HTTP header with `Content-Type: application/octet-stream`. However, if we change or remove that for some reason in Workhorse, this breaks the upload with a 403 Unauthorized because the signed URL is not valid. We can make this more robust by doing the following: 1. In the `/uploads/authorize` request, Rails can return a `StoreHeaders` key-value pair in the JSON response containing the required headers that the PUT request must include. 2. Use those HTTP headers if that value is present. 3. For backwards compatibility, if that key is not present, default to the old behavior of sending the fixed `Content-Type` header. See https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/297 as well. --- changelogs/unreleased/sh-send-put-headers-object-storage.yml | 5 +++++ lib/object_storage/direct_upload.rb | 3 ++- spec/lib/object_storage/direct_upload_spec.rb | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-send-put-headers-object-storage.yml diff --git a/changelogs/unreleased/sh-send-put-headers-object-storage.yml b/changelogs/unreleased/sh-send-put-headers-object-storage.yml new file mode 100644 index 00000000000..cbd8b6deb5b --- /dev/null +++ b/changelogs/unreleased/sh-send-put-headers-object-storage.yml @@ -0,0 +1,5 @@ +--- +title: Send back required object storage PUT headers in /uploads/authorize API +merge_request: 21319 +author: +type: changed diff --git a/lib/object_storage/direct_upload.rb b/lib/object_storage/direct_upload.rb index 61a69e7ffe4..7302ed4cad9 100644 --- a/lib/object_storage/direct_upload.rb +++ b/lib/object_storage/direct_upload.rb @@ -41,7 +41,8 @@ module ObjectStorage GetURL: get_url, StoreURL: store_url, DeleteURL: delete_url, - MultipartUpload: multipart_upload_hash + MultipartUpload: multipart_upload_hash, + StoreHeaders: upload_options }.compact end diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb index e0569218d78..fa1dc15303a 100644 --- a/spec/lib/object_storage/direct_upload_spec.rb +++ b/spec/lib/object_storage/direct_upload_spec.rb @@ -61,6 +61,7 @@ describe ObjectStorage::DirectUpload do expect(subject[:GetURL]).to start_with(storage_url) expect(subject[:StoreURL]).to start_with(storage_url) expect(subject[:DeleteURL]).to start_with(storage_url) + expect(subject[:StoreHeaders]).to eq({ 'Content-Type' => 'application/octet-stream' }) end end -- cgit v1.2.1