github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 09:12:45 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: ACL for OptimizedImage was using wrong path on multisite (#20784)

Browse Source 
When setting the ACL for optimized images after setting the
ACL for the linked upload (e.g. via the SyncACLForUploads job),
we were using the optimized image path as the S3 key. This worked
for single sites, however it would fail silently for multisite
sites since the path would be incorrect, because the Discourse.store.upload_path
was not included.

For example, something like this:

somecluster1/optimized/2X/1/3478534853498753984_2_1380x300.png

Instead of:

somecluster1/uploads/somesite1/2X/1/3478534853498753984_2_1380x300.png

The silent failure is still intentional, since we don't want to
break other things because of ACL updates, but now we will update
the ACL correctly for optimized images on multisite sites.
This commit is contained in:
Martin Brennan 2023-03-24 10:16:53 +10:00 committed by GitHub
parent a6166c5b32
commit 97f8f88cfe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
lib/file_store/s3_store.rb
View File

@ -309,25 +309,29 @@ module FileStore
key = get_upload_key(upload)
update_ACL(key, upload.secure?)
# if we do find_each when the images have already been preloaded with
# If we do find_each when the images have already been preloaded with
# includes(:optimized_images), then the optimized_images are fetched
# from the database again, negating the preloading if this operation
# is done on a large amount of uploads at once (see Jobs::SyncAclsForUploads)
if optimized_images_preloaded
upload.optimized_images.each do |optimized_image|
optimized_image_key = get_path_for_optimized_image(optimized_image)
update_ACL(optimized_image_key, upload.secure?)
update_optimized_image_acl(optimized_image, secure: upload.secure)
end
else
upload.optimized_images.find_each do |optimized_image|
optimized_image_key = get_path_for_optimized_image(optimized_image)
update_ACL(optimized_image_key, upload.secure?)
update_optimized_image_acl(optimized_image, secure: upload.secure)
end
end
true
end
def update_optimized_image_acl(optimized_image, secure: false)
optimized_image_key = get_path_for_optimized_image(optimized_image)
optimized_image_key.prepend(File.join(upload_path, "/")) if Rails.configuration.multisite
update_ACL(optimized_image_key, secure)
end
def download_file(upload, destination_path)
s3_helper.download_file(get_upload_key(upload), destination_path)
end

33
spec/lib/file_store/s3_store_spec.rb
View File

@ -461,12 +461,7 @@ RSpec.describe FileStore::S3Store do
it "sets acl to public by default" do
s3_helper.expects(:s3_bucket).returns(s3_bucket)
s3_bucket
.expects(:object)
.with(regexp_matches(%r{original/\d+X.*/#{upload.sha1}\.pdf}))
.returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "public-read").returns(s3_object)
expect_upload_acl_update(upload, "public-read")
expect(store.update_upload_ACL(upload)).to be_truthy
end
@ -474,14 +469,34 @@ RSpec.describe FileStore::S3Store do
it "sets acl to private when upload is marked secure" do
upload.update!(secure: true)
s3_helper.expects(:s3_bucket).returns(s3_bucket)
expect_upload_acl_update(upload, "private")
expect(store.update_upload_ACL(upload)).to be_truthy
end
describe "optimized images" do
it "sets acl to public by default" do
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
expect_upload_acl_update(upload, "public-read")
optimized_image = Fabricate(:optimized_image, upload: upload)
path = Discourse.store.get_path_for_optimized_image(optimized_image)
stub_optimized_image = stub
s3_bucket.expects(:object).with(path).returns(stub_optimized_image)
stub_optimized_image.expects(:acl).returns(stub_optimized_image)
stub_optimized_image.expects(:put).with(acl: "public-read").returns(stub_optimized_image)
expect(store.update_upload_ACL(upload)).to be_truthy
end
end
def expect_upload_acl_update(upload, acl)
s3_bucket
.expects(:object)
.with(regexp_matches(%r{original/\d+X.*/#{upload.sha1}\.pdf}))
.returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "private").returns(s3_object)
expect(store.update_upload_ACL(upload)).to be_truthy
s3_object.expects(:put).with(acl: acl).returns(s3_object)
end
end
end

74
spec/multisite/s3_store_spec.rb
View File

@ -8,8 +8,14 @@ RSpec.describe "Multisite s3 uploads", type: :multisite do
let(:upload_sha1) { Digest::SHA1.hexdigest(File.read(uploaded_file)) }
let(:upload_path) { Discourse.store.upload_path }
def build_upload
Fabricate.build(:upload, sha1: upload_sha1, id: 1, original_filename: original_filename)
def build_upload(secure: false)
Fabricate.build(
:upload,
sha1: upload_sha1,
id: 1,
original_filename: original_filename,
secure: secure,
)
end
describe "uploading to s3" do
@ -266,36 +272,68 @@ RSpec.describe "Multisite s3 uploads", type: :multisite do
describe "#update_upload_ACL" do
it "updates correct file for default and second multisite db" do
test_multisite_connection("default") do
upload = build_upload
upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true)
upload = build_upload(secure: true)
upload.update!(original_filename: "small.pdf", extension: "pdf")
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
s3_bucket
.expects(:object)
.with("#{upload_path}/original/1X/#{upload.sha1}.pdf")
.returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "private").returns(s3_object)
expect_upload_acl_update(upload, upload_path)
expect(store.update_upload_ACL(upload)).to be_truthy
end
test_multisite_connection("second") do
upload_path = Discourse.store.upload_path
upload = build_upload
upload.update!(original_filename: "small.pdf", extension: "pdf", secure: true)
upload = build_upload(secure: true)
upload.update!(original_filename: "small.pdf", extension: "pdf")
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
s3_bucket
.expects(:object)
.with("#{upload_path}/original/1X/#{upload.sha1}.pdf")
.returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "private").returns(s3_object)
expect_upload_acl_update(upload, upload_path)
expect(store.update_upload_ACL(upload)).to be_truthy
end
end
describe "optimized images" do
it "updates correct file for default and second multisite DB" do
test_multisite_connection("default") do
upload = build_upload(secure: true)
upload_path = Discourse.store.upload_path
optimized_image = Fabricate(:optimized_image, upload: upload)
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
expect_upload_acl_update(upload, upload_path)
expect_optimized_image_acl_update(optimized_image, upload_path)
expect(store.update_upload_ACL(upload)).to be_truthy
end
test_multisite_connection("second") do
upload = build_upload(secure: true)
upload_path = Discourse.store.upload_path
optimized_image = Fabricate(:optimized_image, upload: upload)
s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
expect_upload_acl_update(upload, upload_path)
expect_optimized_image_acl_update(optimized_image, upload_path)
expect(store.update_upload_ACL(upload)).to be_truthy
end
end
end
def expect_upload_acl_update(upload, upload_path)
s3_bucket
.expects(:object)
.with("#{upload_path}/original/1X/#{upload.sha1}.#{upload.extension}")
.returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "private").returns(s3_object)
end
def expect_optimized_image_acl_update(optimized_image, upload_path)
path = Discourse.store.get_path_for_optimized_image(optimized_image)
s3_bucket.expects(:object).with("#{upload_path}/#{path}").returns(s3_object)
s3_object.expects(:acl).returns(s3_object)
s3_object.expects(:put).with(acl: "private").returns(s3_object)
end
end
end