From e00abbe1b73c5345708bb80085060d0f5e80a9e2 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Mon, 14 Sep 2020 13:32:25 +0200 Subject: [PATCH] DEV: Clean up S3 specs, stubs, and helpers Extracted commonly used spec helpers into spec/support/uploads_helpers.rb, removed unused stubs and let definitions. Makes it easier to write new S3-related specs without copy and pasting setup steps from other specs. --- spec/components/cooked_post_processor_spec.rb | 38 ++++------ spec/components/email/sender_spec.rb | 21 +----- spec/components/file_store/base_store_spec.rb | 11 +-- spec/components/file_store/s3_store_spec.rb | 69 ++++--------------- spec/components/post_creator_spec.rb | 14 +--- spec/components/pretty_text_spec.rb | 5 +- spec/components/s3_helper_spec.rb | 11 ++- spec/components/s3_inventory_spec.rb | 4 +- spec/components/stylesheet/importer_spec.rb | 3 +- spec/components/svg_sprite/svg_sprite_spec.rb | 6 +- spec/components/url_helper_spec.rb | 13 ++-- .../helpers/user_notifications_helper_spec.rb | 9 +-- .../correct_missing_dualstack_urls_spec.rb | 27 ++++---- spec/jobs/ensure_s3_uploads_existence_spec.rb | 4 +- spec/jobs/pull_hotlinked_images_spec.rb | 39 ++++------- spec/jobs/update_s3_inventory_spec.rb | 13 ++-- .../jobs/vacate_legacy_prefix_backups_spec.rb | 4 +- .../backup_restore/uploads_restorer_spec.rb | 5 +- spec/lib/shrink_uploaded_image_spec.rb | 11 +-- .../lib/topic_upload_security_manager_spec.rb | 44 ++++++------ spec/lib/upload_creator_spec.rb | 29 +++----- spec/lib/upload_security_spec.rb | 5 +- spec/models/optimized_image_spec.rb | 13 ++-- spec/models/post_spec.rb | 49 ++++++------- spec/models/topic_link_click_spec.rb | 4 +- spec/models/upload_spec.rb | 19 +---- spec/multisite/post_spec.rb | 10 ++- spec/multisite/s3_store_spec.rb | 30 ++------ spec/rails_helper.rb | 1 + spec/requests/session_controller_spec.rb | 11 ++- spec/requests/static_controller_spec.rb | 4 +- .../uploads_controller_multisite_spec.rb | 6 +- spec/requests/uploads_controller_spec.rb | 50 ++++---------- spec/requests/user_avatars_controller_spec.rb | 13 ++-- spec/serializers/upload_serializer_spec.rb | 16 +---- spec/services/inline_uploads_spec.rb | 5 +- spec/support/uploads_helpers.rb | 27 ++++++++ spec/tasks/uploads_spec.rb | 38 +++++----- 38 files changed, 237 insertions(+), 444 deletions(-) create mode 100644 spec/support/uploads_helpers.rb diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 143536b7f50..dc5842f0e3c 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -4,17 +4,6 @@ require "rails_helper" require "cooked_post_processor" require "file_store/s3_store" -def s3_setup - Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com") - - SiteSetting.s3_upload_bucket = "some-bucket-on-s3" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.s3_cdn_url = "https://s3.cdn.com" - SiteSetting.enable_s3_uploads = true - SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|" -end - describe CookedPostProcessor do fab!(:upload) { Fabricate(:upload) } let(:upload_path) { Discourse.store.upload_path } @@ -483,17 +472,16 @@ describe CookedPostProcessor do context "s3_uploads" do let(:upload) { Fabricate(:secure_upload_s3) } + before do - s3_setup + setup_s3 + SiteSetting.s3_cdn_url = "https://s3.cdn.com" + SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|" + stored_path = Discourse.store.get_path_for_upload(upload) upload.update_column(:url, "#{SiteSetting.Upload.absolute_base_url}/#{stored_path}") - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/optimized/1X/#{upload.sha1}_2_#{optimized_size}.#{upload.extension}" - ) - stub_request(:get, /#{SiteSetting.s3_upload_bucket}\.s3\.amazonaws\.com/) + stub_upload(upload) SiteSetting.login_required = true SiteSetting.secure_media = true @@ -1049,11 +1037,11 @@ describe CookedPostProcessor do context "when the post is with_secure_media and the upload is secure and secure media is enabled" do before do + setup_s3 upload.update(secure: true) + SiteSetting.login_required = true - s3_setup SiteSetting.secure_media = true - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") end it "does not use the direct URL, uses the cooked URL instead (because of the private ACL preventing w/h fetch)" do @@ -1269,7 +1257,11 @@ describe CookedPostProcessor do context "s3_uploads" do before do - s3_setup + Rails.configuration.action_controller.stubs(:asset_host).returns("https://local.cdn.com") + + setup_s3 + SiteSetting.s3_cdn_url = "https://s3.cdn.com" + SiteSetting.authorized_extensions = "png|jpg|gif|mov|ogg|" uploaded_file = file_from_fixtures("smallest.png") upload_sha1 = Digest::SHA1.hexdigest(File.read(uploaded_file)) @@ -1546,9 +1538,7 @@ describe CookedPostProcessor do end it "doesn't disable download_remote_images_to_local if site uses S3" do - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + setup_s3 cpp.disable_if_low_on_disk_space expect(SiteSetting.download_remote_images_to_local).to eq(true) diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb index 38d4f270d2b..54b30ceca83 100644 --- a/spec/components/email/sender_spec.rb +++ b/spec/components/email/sender_spec.rb @@ -408,25 +408,10 @@ describe Email::Sender do end context "when secure media enabled" do - def enable_s3_uploads - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image.sha1}.#{image.extension}?acl" - ) - store = FileStore::S3Store.new - s3_helper = store.instance_variable_get(:@s3_helper) - client = Aws::S3::Client.new(stub_responses: true) - s3_helper.stubs(:s3_client).returns(client) - Discourse.stubs(:store).returns(store) - end - before do - enable_s3_uploads + setup_s3 + stub_s3_store + SiteSetting.secure_media = true SiteSetting.login_required = true SiteSetting.email_total_attachment_size_limit_kb = 14_000 diff --git a/spec/components/file_store/base_store_spec.rb b/spec/components/file_store/base_store_spec.rb index 708235d80cf..4771b28f7bb 100644 --- a/spec/components/file_store/base_store_spec.rb +++ b/spec/components/file_store/base_store_spec.rb @@ -47,12 +47,7 @@ RSpec.describe FileStore::BaseStore do describe '#download' do before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.s3_region = "us-east-1" - + setup_s3 stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world") end @@ -78,7 +73,7 @@ RSpec.describe FileStore::BaseStore do end it "should return the file when s3 cdn enabled" do - SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com" + SiteSetting.s3_cdn_url = "https://cdn.s3.#{SiteSetting.s3_region}.amazonaws.com" stub_request(:get, Discourse.store.cdn_url(upload_s3.url)).to_return(status: 200, body: "Hello world") file = store.download(upload_s3) @@ -90,7 +85,7 @@ RSpec.describe FileStore::BaseStore do SiteSetting.login_required = true SiteSetting.secure_media = true - stub_request(:head, "https://s3-upload-bucket.s3.amazonaws.com/") + stub_request(:head, "https://s3-upload-bucket.s3.#{SiteSetting.s3_region}.amazonaws.com/") signed_url = Discourse.store.signed_url_for_path(upload_s3.url) stub_request(:get, signed_url).to_return(status: 200, body: "Hello world") diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index d5bbcebf1ee..e9e4f7c3dc7 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -5,42 +5,26 @@ require 'file_store/s3_store' require 'file_store/local_store' describe FileStore::S3Store do - let(:store) { FileStore::S3Store.new } let(:s3_helper) { store.instance_variable_get(:@s3_helper) } - fab!(:upload) { Fabricate(:upload) } - let(:uploaded_file) { file_from_fixtures("logo.png") } + let(:client) { Aws::S3::Client.new(stub_responses: true) } + let(:resource) { Aws::S3::Resource.new(client: client) } + let(:s3_bucket) { resource.bucket("s3-upload-bucket") } + let(:s3_object) { stub } fab!(:optimized_image) { Fabricate(:optimized_image) } let(:optimized_image_file) { file_from_fixtures("logo.png") } - - before(:each) do - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + let(:uploaded_file) { file_from_fixtures("logo.png") } + fab!(:upload) do + Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string')) end - shared_context 's3 helpers' do - fab!(:upload) do - Fabricate(:upload, sha1: Digest::SHA1.hexdigest('secreet image string')) - end - - let(:store) { FileStore::S3Store.new } - let(:client) { Aws::S3::Client.new(stub_responses: true) } - let(:resource) { Aws::S3::Resource.new(client: client) } - let(:s3_bucket) { resource.bucket("s3-upload-bucket") } - let(:s3_helper) { store.instance_variable_get(:@s3_helper) } - - before do - SiteSetting.s3_region = 'us-west-1' - end + before do + setup_s3 + SiteSetting.s3_region = 'us-west-1' end context 'uploading to s3' do - include_context "s3 helpers" - - let(:s3_object) { stub } let(:etag) { "etag" } describe "#store_upload" do @@ -159,8 +143,6 @@ describe FileStore::S3Store do end context 'copying files in S3' do - include_context "s3 helpers" - describe '#copy_file' do it "copies the from in S3 with the right paths" do s3_helper.expects(:s3_bucket).returns(s3_bucket) @@ -186,8 +168,6 @@ describe FileStore::S3Store do end context 'removal from s3' do - include_context "s3 helpers" - describe "#remove_upload" do it "removes the file from s3 with the right paths" do store.expects(:get_depth_for).with(upload.id).returns(0) @@ -243,8 +223,6 @@ describe FileStore::S3Store do end describe "#remove_optimized_image" do - fab!(:optimized_image) { Fabricate(:optimized_image, upload: upload) } - let(:image_path) do FileStore::BaseStore.new.get_path_for_optimized_image(optimized_image) end @@ -303,23 +281,22 @@ describe FileStore::S3Store do end describe ".has_been_uploaded?" do - it "doesn't crash for invalid URLs" do expect(store.has_been_uploaded?("https://site.discourse.com/#bad#6")).to eq(false) end it "doesn't crash if URL contains non-ascii characters" do - expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/漢1337.png")).to eq(true) + expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/漢1337.png")).to eq(true) expect(store.has_been_uploaded?("//s3-upload-bucket.s3.amazonaws.com/漢1337.png")).to eq(false) end it "identifies S3 uploads" do - expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/1337.png")).to eq(true) + expect(store.has_been_uploaded?("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/1337.png")).to eq(true) end it "does not match other s3 urls" do expect(store.has_been_uploaded?("//s3-upload-bucket.s3.amazonaws.com/1337.png")).to eq(false) - expect(store.has_been_uploaded?("//s3-upload-bucket.s3-us-east-1.amazonaws.com/1337.png")).to eq(false) + expect(store.has_been_uploaded?("//s3-upload-bucket.s3-us-west-1.amazonaws.com/1337.png")).to eq(false) expect(store.has_been_uploaded?("//s3.amazonaws.com/s3-upload-bucket/1337.png")).to eq(false) expect(store.has_been_uploaded?("//s4_upload_bucket.s3.amazonaws.com/1337.png")).to eq(false) end @@ -328,7 +305,7 @@ describe FileStore::S3Store do describe ".absolute_base_url" do it "returns a lowercase schemaless absolute url" do - expect(store.absolute_base_url).to eq("//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com") + expect(store.absolute_base_url).to eq("//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com") end it "uses the proper endpoint" do @@ -353,12 +330,10 @@ describe FileStore::S3Store do end describe ".purge_tombstone" do - it "updates tombstone lifecycle" do s3_helper.expects(:update_tombstone_lifecycle) store.purge_tombstone(1.day) end - end describe ".path_for" do @@ -380,9 +355,6 @@ describe FileStore::S3Store do end context 'update ACL' do - include_context "s3 helpers" - let(:s3_object) { stub } - before do SiteSetting.authorized_extensions = "pdf|png" end @@ -411,7 +383,6 @@ describe FileStore::S3Store do end describe '.cdn_url' do - it 'supports subfolder' do SiteSetting.s3_upload_bucket = 's3-upload-bucket/livechat' SiteSetting.s3_cdn_url = 'https://rainbow.com' @@ -420,25 +391,19 @@ describe FileStore::S3Store do # subfolder should not leak into uploads set_subfolder "/community" - url = "//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/livechat/original/gif.png" + url = "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/livechat/original/gif.png" expect(store.cdn_url(url)).to eq("https://rainbow.com/original/gif.png") end end describe ".download_url" do - include_context "s3 helpers" - let(:s3_object) { stub } - it "returns correct short URL with dl=1 param" do expect(store.download_url(upload)).to eq("#{upload.short_path}?dl=1") end end describe ".url_for" do - include_context "s3 helpers" - let(:s3_object) { stub } - it "returns signed URL with content disposition when requesting to download image" do s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once s3_bucket.expects(:object).with("original/1X/#{upload.sha1}.png").returns(s3_object) @@ -454,9 +419,6 @@ describe FileStore::S3Store do end describe ".signed_url_for_path" do - include_context "s3 helpers" - let(:s3_object) { stub } - it "returns signed URL for a given path" do s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once s3_bucket.expects(:object).with("special/optimized/file.png").returns(s3_object) @@ -469,5 +431,4 @@ describe FileStore::S3Store do expect(store.signed_url_for_path("special/optimized/file.png")).not_to eq(upload.url) end end - end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index b5616e81900..a624437b102 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -1591,20 +1591,10 @@ describe PostCreator do fab!(:public_topic) { Fabricate(:topic) } before do - SiteSetting.enable_s3_uploads = true + setup_s3 SiteSetting.authorized_extensions = "png|jpg|gif|mp4" - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.s3_region = "us-east-1" SiteSetting.secure_media = true - - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image_upload.sha1}.#{image_upload.extension}?acl" - ) + stub_upload(image_upload) end it "links post uploads" do diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 78c480af070..50cd6ac7498 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -902,11 +902,8 @@ describe PrettyText do describe "#strip_secure_media" do before do - SiteSetting.s3_upload_bucket = "some-bucket-on-s3" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" + setup_s3 SiteSetting.s3_cdn_url = "https://s3.cdn.com" - SiteSetting.enable_s3_uploads = true SiteSetting.secure_media = true SiteSetting.login_required = true end diff --git a/spec/components/s3_helper_spec.rb b/spec/components/s3_helper_spec.rb index 9a8697b460d..9d6473885b1 100644 --- a/spec/components/s3_helper_spec.rb +++ b/spec/components/s3_helper_spec.rb @@ -6,11 +6,8 @@ require "rails_helper" describe "S3Helper" do let(:client) { Aws::S3::Client.new(stub_responses: true) } - before(:each) do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "abc" - SiteSetting.s3_secret_access_key = "def" - SiteSetting.s3_region = "us-east-1" + before do + setup_s3 @lifecycle = <<~XML @@ -41,10 +38,10 @@ describe "S3Helper" do stub_request(:get, "http://169.254.169.254/latest/meta-data/iam/security-credentials/"). to_return(status: 404, body: "", headers: {}) - stub_request(:get, "https://bob.s3.amazonaws.com/?lifecycle"). + stub_request(:get, "https://bob.s3.#{SiteSetting.s3_region}.amazonaws.com/?lifecycle"). to_return(status: 200, body: @lifecycle, headers: {}) - stub_request(:put, "https://bob.s3.amazonaws.com/?lifecycle"). + stub_request(:put, "https://bob.s3.#{SiteSetting.s3_region}.amazonaws.com/?lifecycle"). with do |req| hash = Hash.from_xml(req.body.to_s) diff --git a/spec/components/s3_inventory_spec.rb b/spec/components/s3_inventory_spec.rb index 9143afff4a7..405a9e05b0d 100644 --- a/spec/components/s3_inventory_spec.rb +++ b/spec/components/s3_inventory_spec.rb @@ -12,9 +12,7 @@ describe "S3Inventory" do let(:csv_filename) { "#{Rails.root}/spec/fixtures/csv/s3_inventory.csv" } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "abc" - SiteSetting.s3_secret_access_key = "def" + setup_s3 SiteSetting.enable_s3_inventory = true client.stub_responses(:list_objects, -> (context) { diff --git a/spec/components/stylesheet/importer_spec.rb b/spec/components/stylesheet/importer_spec.rb index 23d538a8cc2..34128a24bf9 100644 --- a/spec/components/stylesheet/importer_spec.rb +++ b/spec/components/stylesheet/importer_spec.rb @@ -23,13 +23,12 @@ describe Stylesheet::Importer do end it "applies S3 CDN to background category images" do + setup_s3 SiteSetting.s3_use_iam_profile = true SiteSetting.s3_upload_bucket = 'test' SiteSetting.s3_region = 'ap-southeast-2' SiteSetting.s3_cdn_url = "https://s3.cdn" - SiteSetting.enable_s3_uploads = true - background = Fabricate(:upload_s3) category = Fabricate(:category, uploaded_background: background) diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb index 298a06dd7b3..abb39b1a3f8 100644 --- a/spec/components/svg_sprite/svg_sprite_spec.rb +++ b/spec/components/svg_sprite/svg_sprite_spec.rb @@ -164,11 +164,7 @@ describe SvgSprite do let(:upload_s3) { Fabricate(:upload_s3) } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3bucket" - SiteSetting.s3_access_key_id = "s3_access_key_id" - SiteSetting.s3_secret_access_key = "s3_secret_access_key" - + setup_s3 stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world") end diff --git a/spec/components/url_helper_spec.rb b/spec/components/url_helper_spec.rb index 3c533de935c..25c5dbdc99f 100644 --- a/spec/components/url_helper_spec.rb +++ b/spec/components/url_helper_spec.rb @@ -156,16 +156,15 @@ describe UrlHelper do end describe "#cook_url" do - let(:url) { "//s3bucket.s3.dualstack.us-east-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" } + let(:url) { "//s3bucket.s3.dualstack.us-west-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" } before do - FileStore::S3Store.any_instance.stubs(:has_been_uploaded?).returns(true) - Rails.configuration.action_controller.asset_host = "https://test.some-cdn.com/dev" - SiteSetting.enable_s3_uploads = true + setup_s3 SiteSetting.s3_upload_bucket = "s3bucket" - SiteSetting.s3_access_key_id = "s3_access_key_id" - SiteSetting.s3_secret_access_key = "s3_secret_access_key" SiteSetting.login_required = true + Rails.configuration.action_controller.asset_host = "https://test.some-cdn.com/dev" + + FileStore::S3Store.any_instance.stubs(:has_been_uploaded?).returns(true) end def cooked @@ -187,7 +186,7 @@ describe UrlHelper do it "returns the local_cdn_url" do expect(cooked).to eq( - "//s3bucket.s3.dualstack.us-east-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" + "//s3bucket.s3.dualstack.us-west-1.amazonaws.com/dev/original/3X/2/e/2e6f2ef81b6910ea592cd6d21ee897cd51cf72e4.jpeg" ) end end diff --git a/spec/helpers/user_notifications_helper_spec.rb b/spec/helpers/user_notifications_helper_spec.rb index 21e0a181fec..746a9b71f6c 100644 --- a/spec/helpers/user_notifications_helper_spec.rb +++ b/spec/helpers/user_notifications_helper_spec.rb @@ -163,16 +163,13 @@ describe UserNotificationsHelper do let(:upload) { Fabricate(:upload_s3, sha1: "somesha1") } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" + setup_s3 SiteSetting.logo = upload end it 'should return the right URL' do expect(helper.logo_url).to eq( - "http://s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/original/1X/somesha1.png" + "http://s3-upload-bucket.s3.dualstack.#{SiteSetting.s3_region}.amazonaws.com/original/1X/somesha1.png" ) end @@ -181,7 +178,7 @@ describe UserNotificationsHelper do GlobalSetting.stubs(:cdn_url).returns('https://some.cdn.com/cluster') expect(helper.logo_url).to eq( - "http://s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/original/1X/somesha1.png" + "http://s3-upload-bucket.s3.dualstack.#{SiteSetting.s3_region}.amazonaws.com/original/1X/somesha1.png" ) end end diff --git a/spec/jobs/correct_missing_dualstack_urls_spec.rb b/spec/jobs/correct_missing_dualstack_urls_spec.rb index c41f12c9893..d82961db712 100644 --- a/spec/jobs/correct_missing_dualstack_urls_spec.rb +++ b/spec/jobs/correct_missing_dualstack_urls_spec.rb @@ -3,33 +3,30 @@ require 'rails_helper' describe Jobs::CorrectMissingDualstackUrls do - it 'corrects the urls' do - - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + setup_s3 + SiteSetting.s3_region = "us-east-1" + SiteSetting.s3_upload_bucket = "custom-bucket" # we will only correct for our base_url, random urls will be left alone - expect(Discourse.store.absolute_base_url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com') + expect(Discourse.store.absolute_base_url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com') current_upload = Upload.create!( - url: '//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png', + url: '//custom-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png', original_filename: 'a.png', filesize: 100, user_id: -1, ) bad_upload = Upload.create!( - url: '//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png', + url: '//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png', original_filename: 'a.png', filesize: 100, user_id: -1, ) current_optimized = OptimizedImage.create!( - url: '//s3-upload-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png', + url: '//custom-bucket.s3-us-east-1.amazonaws.com/somewhere/a.png', filesize: 100, upload_id: current_upload.id, width: 100, @@ -39,7 +36,7 @@ describe Jobs::CorrectMissingDualstackUrls do ) bad_optimized = OptimizedImage.create!( - url: '//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png', + url: '//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png', filesize: 100, upload_id: current_upload.id, width: 110, @@ -51,15 +48,15 @@ describe Jobs::CorrectMissingDualstackUrls do Jobs::CorrectMissingDualstackUrls.new.execute_onceoff(nil) bad_upload.reload - expect(bad_upload.url).to eq('//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png') + expect(bad_upload.url).to eq('//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png') current_upload.reload - expect(current_upload.url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png') + expect(current_upload.url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png') bad_optimized.reload - expect(bad_optimized.url).to eq('//s3-upload-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png') + expect(bad_optimized.url).to eq('//custom-bucket.s3-us-west-1.amazonaws.com/somewhere/a.png') current_optimized.reload - expect(current_optimized.url).to eq('//s3-upload-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png') + expect(current_optimized.url).to eq('//custom-bucket.s3.dualstack.us-east-1.amazonaws.com/somewhere/a.png') end end diff --git a/spec/jobs/ensure_s3_uploads_existence_spec.rb b/spec/jobs/ensure_s3_uploads_existence_spec.rb index baaf1a461e7..d2d67f7e548 100644 --- a/spec/jobs/ensure_s3_uploads_existence_spec.rb +++ b/spec/jobs/ensure_s3_uploads_existence_spec.rb @@ -5,9 +5,7 @@ require 'rails_helper' RSpec.describe Jobs::EnsureS3UploadsExistence do context "S3 inventory enabled" do before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "abc" - SiteSetting.s3_secret_access_key = "def" + setup_s3 SiteSetting.enable_s3_inventory = true end diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index ab947c5dc9b..adb7a56d7eb 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -186,7 +186,9 @@ describe Jobs::PullHotlinkedImages do context "when secure media enabled for an upload that has already been downloaded and exists" do it "doesnt redownload the secure upload" do - enable_secure_media + setup_s3 + SiteSetting.secure_media = true + upload = Fabricate(:secure_upload_s3, secure: true) stub_s3(upload) url = Upload.secure_media_url_from_upload_url(upload.url) @@ -199,7 +201,9 @@ describe Jobs::PullHotlinkedImages do context "when the upload original_sha1 is missing" do it "redownloads the upload" do - enable_secure_media + setup_s3 + SiteSetting.secure_media = true + upload = Fabricate(:upload_s3, secure: true) stub_s3(upload) Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url) @@ -218,7 +222,9 @@ describe Jobs::PullHotlinkedImages do context "when the upload access_control_post is different to the current post" do it "redownloads the upload" do - enable_secure_media + setup_s3 + SiteSetting.secure_media = true + upload = Fabricate(:secure_upload_s3, secure: true) stub_s3(upload) Upload.stubs(:signed_url_from_secure_media_url).returns(upload.url) @@ -395,7 +401,9 @@ describe Jobs::PullHotlinkedImages do context "when secure media enabled" do it 'should return false for secure-media-upload url' do - enable_secure_media + setup_s3 + SiteSetting.secure_media = true + upload = Fabricate(:upload_s3, secure: true) stub_s3(upload) url = Upload.secure_media_url_from_upload_url(upload.url) @@ -415,12 +423,9 @@ describe Jobs::PullHotlinkedImages do end it "returns false for emoji when app and S3 CDNs configured" do - set_cdn_url "https://mydomain.cdn/test" - SiteSetting.s3_upload_bucket = "some-bucket-on-s3" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" + setup_s3 SiteSetting.s3_cdn_url = "https://s3.cdn.com" - SiteSetting.enable_s3_uploads = true + set_cdn_url "https://mydomain.cdn/test" src = UrlHelper.cook_url(Emoji.url_for("testemoji.png")) expect(subject.should_download_image?(src)).to eq(false) @@ -503,22 +508,8 @@ describe Jobs::PullHotlinkedImages do end end - def enable_secure_media - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" - SiteSetting.secure_media = true - end - def stub_s3(upload) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl" - ) + stub_upload(upload) stub_request(:get, "https:" + upload.url).to_return(status: 200, body: file_from_fixtures("smallest.png")) - # stub_request(:get, /#{SiteSetting.s3_upload_bucket}\.s3\.amazonaws\.com/) end end diff --git a/spec/jobs/update_s3_inventory_spec.rb b/spec/jobs/update_s3_inventory_spec.rb index 175170150fb..d03e404a73a 100644 --- a/spec/jobs/update_s3_inventory_spec.rb +++ b/spec/jobs/update_s3_inventory_spec.rb @@ -5,9 +5,8 @@ require "file_store/s3_store" describe Jobs::UpdateS3Inventory do before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "abc" - SiteSetting.s3_secret_access_key = "def" + setup_s3 + SiteSetting.s3_upload_bucket = "special-bucket" SiteSetting.enable_s3_inventory = true store = FileStore::S3Store.new @@ -21,17 +20,17 @@ describe Jobs::UpdateS3Inventory do path = File.join(S3Inventory::INVENTORY_PREFIX, S3Inventory::INVENTORY_VERSION) @client.expects(:put_bucket_policy).with( - bucket: "bucket", - policy: %Q|{"Version":"2012-10-17","Statement":[{"Sid":"InventoryAndAnalyticsPolicy","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Action":["s3:PutObject"],"Resource":["arn:aws:s3:::bucket/#{path}/*"],"Condition":{"ArnLike":{"aws:SourceArn":"arn:aws:s3:::bucket"},"StringEquals":{"s3:x-amz-acl":"bucket-owner-full-control"}}}]}| + bucket: "special-bucket", + policy: %Q|{"Version":"2012-10-17","Statement":[{"Sid":"InventoryAndAnalyticsPolicy","Effect":"Allow","Principal":{"Service":"s3.amazonaws.com"},"Action":["s3:PutObject"],"Resource":["arn:aws:s3:::special-bucket/#{path}/*"],"Condition":{"ArnLike":{"aws:SourceArn":"arn:aws:s3:::special-bucket"},"StringEquals":{"s3:x-amz-acl":"bucket-owner-full-control"}}}]}| ) @client.expects(:put_bucket_inventory_configuration) @client.expects(:put_bucket_inventory_configuration).with( - bucket: "bucket", + bucket: "special-bucket", id: id, inventory_configuration: { destination: { s3_bucket_destination: { - bucket: "arn:aws:s3:::bucket", + bucket: "arn:aws:s3:::special-bucket", prefix: path, format: "CSV" } diff --git a/spec/jobs/vacate_legacy_prefix_backups_spec.rb b/spec/jobs/vacate_legacy_prefix_backups_spec.rb index dc759792b50..4de6325ba88 100644 --- a/spec/jobs/vacate_legacy_prefix_backups_spec.rb +++ b/spec/jobs/vacate_legacy_prefix_backups_spec.rb @@ -18,9 +18,7 @@ describe Jobs::VacateLegacyPrefixBackups, type: :multisite do end before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "abc" - SiteSetting.s3_secret_access_key = "def" + setup_s3 SiteSetting.s3_backup_bucket = bucket_name SiteSetting.backup_location = BackupLocationSiteSetting::S3 end diff --git a/spec/lib/backup_restore/uploads_restorer_spec.rb b/spec/lib/backup_restore/uploads_restorer_spec.rb index fba41193fee..3a3729bed2d 100644 --- a/spec/lib/backup_restore/uploads_restorer_spec.rb +++ b/spec/lib/backup_restore/uploads_restorer_spec.rb @@ -369,10 +369,7 @@ describe BackupRestore::UploadsRestorer do context "currently stored on S3" do before do - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + setup_s3 end let!(:store_class) { FileStore::S3Store } diff --git a/spec/lib/shrink_uploaded_image_spec.rb b/spec/lib/shrink_uploaded_image_spec.rb index 8c8e47165ab..d343d639391 100644 --- a/spec/lib/shrink_uploaded_image_spec.rb +++ b/spec/lib/shrink_uploaded_image_spec.rb @@ -72,15 +72,8 @@ describe ShrinkUploadedImage do let(:upload) { Fabricate(:s3_image_upload, width: 200, height: 200) } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "fakeid7974664" - SiteSetting.s3_secret_access_key = "fakesecretid7974664" - - store = FileStore::S3Store.new - s3_helper = store.instance_variable_get(:@s3_helper) - client = Aws::S3::Client.new(stub_responses: true) - s3_helper.stubs(:s3_client).returns(client) - Discourse.stubs(:store).returns(store) + setup_s3 + stub_s3_store end it "resizes the image" do diff --git a/spec/lib/topic_upload_security_manager_spec.rb b/spec/lib/topic_upload_security_manager_spec.rb index 5947ceecabf..6179140ae70 100644 --- a/spec/lib/topic_upload_security_manager_spec.rb +++ b/spec/lib/topic_upload_security_manager_spec.rb @@ -30,7 +30,12 @@ describe TopicUploadSecurityManager do let(:category) { Fabricate(:private_category, group: group) } context "when secure media is enabled" do - before { enable_secure_media } + before do + setup_s3 + SiteSetting.secure_media = true + + [upload, upload2, upload3].each { |upl| stub_upload(upl) } + end it "does not change any upload statuses or update ACLs or rebake" do expect_upload_status_not_to_change @@ -57,7 +62,12 @@ describe TopicUploadSecurityManager do let(:topic) { Fabricate(:private_message_topic, category: category, user: user) } context "when secure media is enabled" do - before { enable_secure_media } + before do + setup_s3 + SiteSetting.secure_media = true + + [upload, upload2, upload3].each { |upl| stub_upload(upl) } + end it "does not change any upload statuses or update ACLs or rebake" do expect_upload_status_not_to_change @@ -82,7 +92,12 @@ describe TopicUploadSecurityManager do context "when the topic is public" do context "when secure media is enabled" do - before { enable_secure_media } + before do + setup_s3 + SiteSetting.secure_media = true + + [upload, upload2, upload3].each { |upl| stub_upload(upl) } + end context "when login required is enabled" do before do @@ -111,7 +126,10 @@ describe TopicUploadSecurityManager do let!(:upload3) { Fabricate(:upload) } before do - enable_secure_media + setup_s3 + SiteSetting.secure_media = true + + [upload, upload2, upload3].each { |upl| stub_upload(upl) } end context "when this is the first post the upload has appeared in" do @@ -155,24 +173,6 @@ describe TopicUploadSecurityManager do end end - def enable_secure_media - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" - SiteSetting.secure_media = true - - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - # because the ACLs will be changing... - [upload, upload2, upload3].each do |upl| - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upl.sha1}.#{upl.extension}?acl" - ) - end - end - def expect_upload_status_not_to_change Post.any_instance.expects(:rebake!).never subject.run diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb index 492bb44e440..49fbbab8381 100644 --- a/spec/lib/upload_creator_spec.rb +++ b/spec/lib/upload_creator_spec.rb @@ -193,7 +193,9 @@ RSpec.describe UploadCreator do let(:opts) { { type: "composer" } } before do - enable_s3_uploads + setup_s3 + stub_s3_store + SiteSetting.secure_media = true SiteSetting.authorized_extensions = 'pdf|svg|jpg' end @@ -221,7 +223,8 @@ RSpec.describe UploadCreator do let(:opts) { { type: "composer" } } before do - enable_s3_uploads + setup_s3 + stub_s3_store end it 'should store the file and return etag' do @@ -280,7 +283,9 @@ RSpec.describe UploadCreator do context "when SiteSetting.secure_media is enabled" do before do - enable_s3_uploads + setup_s3 + stub_s3_store + SiteSetting.secure_media = true end @@ -298,7 +303,9 @@ RSpec.describe UploadCreator do context "when SiteSetting.secure_media enabled" do before do - enable_s3_uploads + setup_s3 + stub_s3_store + SiteSetting.secure_media = true end @@ -434,18 +441,4 @@ RSpec.describe UploadCreator do end end end - - def enable_s3_uploads - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.s3_region = 'us-west-1' - SiteSetting.enable_s3_uploads = true - - store = FileStore::S3Store.new - s3_helper = store.instance_variable_get(:@s3_helper) - client = Aws::S3::Client.new(stub_responses: true) - s3_helper.stubs(:s3_client).returns(client) - Discourse.stubs(:store).returns(store) - end end diff --git a/spec/lib/upload_security_spec.rb b/spec/lib/upload_security_spec.rb index d154528f391..698e5b12759 100644 --- a/spec/lib/upload_security_spec.rb +++ b/spec/lib/upload_security_spec.rb @@ -12,10 +12,7 @@ RSpec.describe UploadSecurity do context "when secure media is enabled" do before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" + setup_s3 SiteSetting.secure_media = true end diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index a922e822284..c66293343c8 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -293,11 +293,7 @@ describe OptimizedImage do describe "external store" do before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.s3_region = "us-east-1" + setup_s3 end context "when we have a bad file returned" do @@ -318,8 +314,7 @@ describe OptimizedImage do before do stub_request(:head, "http://#{s3_upload.url}").to_return(status: 200) stub_request(:get, "http://#{s3_upload.url}").to_return(status: 200, body: file_from_fixtures("logo.png")) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - stub_request(:put, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com#{optimized_path}") + stub_request(:put, "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com#{optimized_path}") .to_return(status: 200, headers: { "ETag" => "someetag" }) end @@ -330,14 +325,14 @@ describe OptimizedImage do expect(oi.extension).to eq(".png") expect(oi.width).to eq(100) expect(oi.height).to eq(200) - expect(oi.url).to eq("//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com#{optimized_path}") + expect(oi.url).to eq("//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com#{optimized_path}") expect(oi.filesize).to be > 0 oi.filesize = nil stub_request( :get, - "http://#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com#{optimized_path}" + "http://#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com#{optimized_path}" ).to_return(status: 200, body: file_from_fixtures("resized.png")) expect(oi.filesize).to be > 0 diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 6819e8b3172..fd3277b8581 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -164,7 +164,11 @@ describe Post do end context "when secure media is enabled" do - before { enable_secure_media_and_s3 } + before do + setup_s3 + SiteSetting.authorized_extensions = "pdf|png|jpg|csv" + SiteSetting.secure_media = true + end context "if login_required" do before { SiteSetting.login_required = true } @@ -1426,7 +1430,11 @@ describe Post do end context "when secure media is enabled" do - before { enable_secure_media_and_s3 } + before do + setup_s3 + SiteSetting.authorized_extensions = "pdf|png|jpg|csv" + SiteSetting.secure_media = true + end it "sets the access_control_post_id on uploads in the post that don't already have the value set" do other_post = Fabricate(:post) @@ -1464,19 +1472,14 @@ describe Post do end before do - enable_secure_media_and_s3 + setup_s3 + SiteSetting.authorized_extensions = "pdf|png|jpg|csv" + SiteSetting.secure_media = true + attachment_upload.update!(original_filename: "hello.csv") - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{attachment_upload.sha1}.#{attachment_upload.extension}?acl" - ) - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{image_upload.sha1}.#{image_upload.extension}?acl" - ) + stub_upload(attachment_upload) + stub_upload(image_upload) end it "marks image and attachment uploads as secure in PMs when secure_media is ON" do @@ -1643,7 +1646,10 @@ describe Post do end it "correctly identifies secure uploads" do - enable_secure_media_and_s3 + setup_s3 + SiteSetting.authorized_extensions = "pdf|png|jpg|csv" + SiteSetting.secure_media = true + upload1 = Fabricate(:upload_s3, secure: true) upload2 = Fabricate(:upload_s3, secure: true) @@ -1690,11 +1696,7 @@ describe Post do end it "should skip external urls with upload url in query string" do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com" + setup_s3 urls = [] upload = Fabricate(:upload_s3) @@ -1703,13 +1705,4 @@ describe Post do expect(urls).to be_empty end end - - def enable_secure_media_and_s3 - SiteSetting.authorized_extensions = "pdf|png|jpg|csv" - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.secure_media = true - end end diff --git a/spec/models/topic_link_click_spec.rb b/spec/models/topic_link_click_spec.rb index a4d6771db65..543d92b6188 100644 --- a/spec/models/topic_link_click_spec.rb +++ b/spec/models/topic_link_click_spec.rb @@ -166,10 +166,8 @@ describe TopicLinkClick do context "s3 cdns" do it "works with s3 urls" do + setup_s3 SiteSetting.s3_cdn_url = "https://discourse-s3-cdn.global.ssl.fastly.net" - SiteSetting.s3_access_key_id = 'X' - SiteSetting.s3_secret_access_key = 'X' - SiteSetting.enable_s3_uploads = true post = Fabricate(:post, topic: @topic, raw: "[test](//test.localhost/uploads/default/my-test-link)") TopicLink.extract_from(post) diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 89a0a337924..10ddea17389 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -179,10 +179,7 @@ describe Upload do let(:path) { upload.url.sub(SiteSetting.Upload.s3_base_url, '') } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" + setup_s3 end it "should return the right upload when using base url (not CDN) for s3" do @@ -407,22 +404,12 @@ describe Upload do end def enable_secure_media - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" + setup_s3 SiteSetting.secure_media = true - - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl" - ) + stub_upload(upload) end context '.destroy' do - it "can correctly clear information when destroying an upload" do upload = Fabricate(:upload) user = Fabricate(:user) diff --git a/spec/multisite/post_spec.rb b/spec/multisite/post_spec.rb index ef1112de8b6..f555110c9cf 100644 --- a/spec/multisite/post_spec.rb +++ b/spec/multisite/post_spec.rb @@ -8,13 +8,11 @@ RSpec.describe 'Multisite Post', type: :multisite do let(:upload2) { Fabricate(:upload_s3) } let(:upload3) { Fabricate(:upload_s3) } - it "correctly identifies all upload urls" do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" - SiteSetting.s3_cdn_url = "https://cdn.s3.amazonaws.com" + before do + setup_s3 + end + it "correctly identifies all upload urls" do upload3.url.sub!(RailsMultisite::ConnectionManagement.current_db, "secondsite") upload3.save! diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb index f36a3a4bed4..f9049a1d858 100644 --- a/spec/multisite/s3_store_spec.rb +++ b/spec/multisite/s3_store_spec.rb @@ -15,10 +15,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do context 'uploading to s3' do before(:each) do - SiteSetting.s3_upload_bucket = "some-really-cool-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + setup_s3 end describe "#store_upload" do @@ -79,7 +76,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do test_multisite_connection('default') do upload = build_upload expect(store.store_upload(uploaded_file, upload)).to eq( - "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" + "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" ) expect(upload.etag).to eq("ETag") end @@ -88,7 +85,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do upload_path = Discourse.store.upload_path upload = build_upload expect(store.store_upload(uploaded_file, upload)).to eq( - "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" + "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/c530c06cf89c410c0355d7852644a73fc3ec8c04.png" ) expect(upload.etag).to eq("ETag") end @@ -98,11 +95,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do context 'removal from s3' do before do - SiteSetting.s3_region = 'us-west-1' - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true + setup_s3 end describe "#remove_upload" do @@ -180,10 +173,8 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do let(:s3_object) { stub } before(:each) do + setup_s3 SiteSetting.s3_upload_bucket = "some-really-cool-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true SiteSetting.authorized_extensions = "pdf|png|jpg|gif" end @@ -202,7 +193,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do s3_object.expects(:presigned_url).with(:get, expires_in: S3Helper::DOWNLOAD_URL_EXPIRES_AFTER_SECONDS) expect(store.store_upload(uploaded_file, upload)).to eq( - "//some-really-cool-bucket.s3.dualstack.us-east-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.pdf" + "//some-really-cool-bucket.s3.dualstack.us-west-1.amazonaws.com/#{upload_path}/original/1X/#{upload.sha1}.pdf" ) expect(store.url_for(upload)).not_to eq(upload.url) @@ -270,18 +261,11 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do describe "#has_been_uploaded?" do before do - SiteSetting.s3_region = 'us-west-1' + setup_s3 SiteSetting.s3_upload_bucket = "s3-upload-bucket/test" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true end let(:store) { FileStore::S3Store.new } - let(:client) { Aws::S3::Client.new(stub_responses: true) } - let(:resource) { Aws::S3::Resource.new(client: client) } - let(:s3_bucket) { resource.bucket(SiteSetting.s3_upload_bucket) } - let(:s3_helper) { store.s3_helper } it "returns false for blank urls and bad urls" do expect(store.has_been_uploaded?("")).to eq(false) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ba28a0c2961..f5dd33af0e6 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -173,6 +173,7 @@ RSpec.configure do |config| config.include WebauthnIntegrationHelpers config.include SiteSettingsHelpers config.include SidekiqHelpers + config.include UploadsHelpers config.mock_framework = :mocha config.order = 'random' config.infer_spec_type_from_file_location! diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index a4542c0cc5a..6ec3f4c73d9 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -1092,23 +1092,20 @@ RSpec.describe SessionController do it 'handles non local content correctly' do SiteSetting.avatar_sizes = "100|49" - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "XXX" - SiteSetting.s3_secret_access_key = "XXX" - SiteSetting.s3_upload_bucket = "test" + setup_s3 SiteSetting.s3_cdn_url = "http://cdn.com" - stub_request(:any, /test.s3.dualstack.us-east-1.amazonaws.com/).to_return(status: 200, body: "", headers: { referer: "fgdfds" }) + stub_request(:any, /s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/).to_return(status: 200, body: "", headers: { referer: "fgdfds" }) @user.create_user_avatar! - upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something") + upload = Fabricate(:upload, url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/something") Fabricate(:optimized_image, sha1: SecureRandom.hex << "A" * 8, upload: upload, width: 98, height: 98, - url: "//test.s3.amazonaws.com/something/else" + url: "//s3-upload-bucket.s3.amazonaws.com/something/else" ) @user.update_columns(uploaded_avatar_id: upload.id) diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 8444b20ed44..967185f567c 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -52,9 +52,7 @@ describe StaticController do end before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = 'X' - SiteSetting.s3_secret_access_key = 'X' + setup_s3 end it 'can proxy a favicon correctly' do diff --git a/spec/requests/uploads_controller_multisite_spec.rb b/spec/requests/uploads_controller_multisite_spec.rb index 0865dd6752c..8c073b09a37 100644 --- a/spec/requests/uploads_controller_multisite_spec.rb +++ b/spec/requests/uploads_controller_multisite_spec.rb @@ -10,17 +10,15 @@ describe UploadsController do let(:upload) { Fabricate(:upload_s3) } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "fakeid7974664" - SiteSetting.s3_secret_access_key = "fakesecretid7974664" + setup_s3 end context "when upload is secure and secure media enabled" do before do SiteSetting.secure_media = true upload.update(secure: true) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") end + context "when running on a multisite connection", type: :multisite do it "redirects to the signed_url_for_path with the multisite DB name in the url" do sign_in(user) diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 4c3f7dcb07e..02b4c035946 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -240,9 +240,7 @@ describe UploadsController do fab!(:upload) { upload_file("small.pdf", "pdf") } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "fakeid7974664" - SiteSetting.s3_secret_access_key = "fakesecretid7974664" + setup_s3 end it "returns 404 " do @@ -383,9 +381,7 @@ describe UploadsController do let(:upload) { Fabricate(:upload_s3) } before do - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "fakeid7974664" - SiteSetting.s3_secret_access_key = "fakesecretid7974664" + setup_s3 end it "should redirect to the s3 URL" do @@ -398,7 +394,6 @@ describe UploadsController do before do SiteSetting.secure_media = true upload.update(secure: true) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") end it "redirects to the signed_url_for_path" do @@ -448,22 +443,9 @@ describe UploadsController do let(:upload) { Fabricate(:upload_s3) } let(:secure_url) { upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") } - def sign_in_and_stub_head - sign_in(user) - stub_head - end - - def stub_head - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - end - before do + setup_s3 SiteSetting.authorized_extensions = "*" - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "fakeid7974664" - SiteSetting.s3_secret_access_key = "fakesecretid7974664" - SiteSetting.s3_region = "us-east-1" SiteSetting.secure_media = true end @@ -473,8 +455,7 @@ describe UploadsController do end it "should return signed url for legitimate request" do - sign_in_and_stub_head - + sign_in(user) get secure_url expect(response.status).to eq(302) @@ -494,7 +475,7 @@ describe UploadsController do context "when the upload cannot be found from the URL" do it "returns a 404" do - sign_in_and_stub_head + sign_in(user) upload.update(sha1: 'test') get secure_url @@ -507,13 +488,13 @@ describe UploadsController do let!(:private_category) { Fabricate(:private_category, group: Fabricate(:group)) } before do - sign_in_and_stub_head + sign_in(user) upload.update(access_control_post_id: post.id) end context "when the user has access to the post via guardian" do it "should return signed url for legitimate request" do - sign_in_and_stub_head + sign_in(user) get secure_url expect(response.status).to eq(302) expect(response.redirect_url).to match("Amz-Expires") @@ -526,7 +507,7 @@ describe UploadsController do end it "returns a 403" do - sign_in_and_stub_head + sign_in(user) get secure_url expect(response.status).to eq(403) end @@ -538,7 +519,7 @@ describe UploadsController do upload.update(original_filename: 'test.pdf') end it "redirects to the signed_url_for_path" do - sign_in_and_stub_head + sign_in(user) get secure_url expect(response.status).to eq(302) expect(response.redirect_url).to match("Amz-Expires") @@ -554,7 +535,7 @@ describe UploadsController do end it "returns a 403" do - sign_in_and_stub_head + sign_in(user) get secure_url expect(response.status).to eq(403) end @@ -564,8 +545,8 @@ describe UploadsController do before do SiteSetting.prevent_anons_from_downloading_files = true end + it "returns a 404" do - stub_head delete "/session/#{user.username}.json" get secure_url expect(response.status).to eq(404) @@ -586,8 +567,6 @@ describe UploadsController do it "should redirect to the regular show route" do secure_url = upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") sign_in(user) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - get secure_url expect(response.status).to eq(302) @@ -603,8 +582,6 @@ describe UploadsController do it "should redirect to the presigned URL still otherwise we will get a 403" do secure_url = upload.url.sub(SiteSetting.Upload.absolute_base_url, "/secure-media-uploads") sign_in(user) - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - get secure_url expect(response.status).to eq(302) @@ -632,11 +609,8 @@ describe UploadsController do let(:upload) { Fabricate(:upload_s3, secure: true) } before do + setup_s3 SiteSetting.authorized_extensions = "pdf|png" - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.enable_s3_uploads = true SiteSetting.secure_media = true end diff --git a/spec/requests/user_avatars_controller_spec.rb b/spec/requests/user_avatars_controller_spec.rb index 1ff23e62391..6a3d7fb07d9 100644 --- a/spec/requests/user_avatars_controller_spec.rb +++ b/spec/requests/user_avatars_controller_spec.rb @@ -69,25 +69,22 @@ describe UserAvatarsController do end it 'handles non local content correctly' do + setup_s3 SiteSetting.avatar_sizes = "100|49" - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "XXX" - SiteSetting.s3_secret_access_key = "XXX" - SiteSetting.s3_upload_bucket = "test" - SiteSetting.s3_cdn_url = "http://cdn.com" SiteSetting.unicode_usernames = true + SiteSetting.s3_cdn_url = "http://cdn.com" - stub_request(:get, "http://cdn.com/something/else").to_return(body: 'image') + stub_request(:get, "#{SiteSetting.s3_cdn_url}/something/else").to_return(body: 'image') set_cdn_url("http://awesome.com/boom") - upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something") + upload = Fabricate(:upload, url: "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/something") optimized_image = Fabricate(:optimized_image, sha1: SecureRandom.hex << "A" * 8, upload: upload, width: 98, height: 98, - url: "//test.s3.dualstack.us-east-1.amazonaws.com/something/else", + url: "//#{SiteSetting.s3_upload_bucket}.s3.dualstack.us-west-1.amazonaws.com/something/else", version: OptimizedImage::VERSION ) diff --git a/spec/serializers/upload_serializer_spec.rb b/spec/serializers/upload_serializer_spec.rb index 785f4f2c89c..c2a1cf6e048 100644 --- a/spec/serializers/upload_serializer_spec.rb +++ b/spec/serializers/upload_serializer_spec.rb @@ -29,7 +29,7 @@ RSpec.describe UploadSerializer do context "when secure media is enabled" do before do - enable_s3_uploads + setup_s3 SiteSetting.secure_media = true end @@ -39,18 +39,4 @@ RSpec.describe UploadSerializer do end end end - - def enable_s3_uploads - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "s3-access-key-id" - SiteSetting.s3_secret_access_key = "s3-secret-access-key" - SiteSetting.s3_region = 'us-west-1' - SiteSetting.enable_s3_uploads = true - - store = FileStore::S3Store.new - s3_helper = store.instance_variable_get(:@s3_helper) - client = Aws::S3::Client.new(stub_responses: true) - s3_helper.stubs(:s3_client).returns(client) - Discourse.stubs(:store).returns(store) - end end diff --git a/spec/services/inline_uploads_spec.rb b/spec/services/inline_uploads_spec.rb index 76fb36cf9a0..c6e4835b65c 100644 --- a/spec/services/inline_uploads_spec.rb +++ b/spec/services/inline_uploads_spec.rb @@ -654,10 +654,7 @@ RSpec.describe InlineUploads do before do upload3 - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secret key" + setup_s3 SiteSetting.s3_cdn_url = "https://s3.cdn.com" end diff --git a/spec/support/uploads_helpers.rb b/spec/support/uploads_helpers.rb new file mode 100644 index 00000000000..583c26e0c6d --- /dev/null +++ b/spec/support/uploads_helpers.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module UploadsHelpers + def setup_s3 + SiteSetting.enable_s3_uploads = true + + SiteSetting.s3_region = 'us-west-1' + SiteSetting.s3_upload_bucket = "s3-upload-bucket" + + SiteSetting.s3_access_key_id = "some key" + SiteSetting.s3_secret_access_key = "some secrets3_region key" + + stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com/") + end + + def stub_upload(upload) + url = "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl" + stub_request(:put, url) + end + + def stub_s3_store + store = FileStore::S3Store.new + client = Aws::S3::Client.new(stub_responses: true) + store.s3_helper.stubs(:s3_client).returns(client) + Discourse.stubs(:store).returns(store) + end +end diff --git a/spec/tasks/uploads_spec.rb b/spec/tasks/uploads_spec.rb index 8cc08383e89..bf817624129 100644 --- a/spec/tasks/uploads_spec.rb +++ b/spec/tasks/uploads_spec.rb @@ -50,7 +50,8 @@ RSpec.describe "tasks/uploads" do context "when store is external" do before do - enable_s3_uploads(uploads) + setup_s3 + uploads.each { |upload| stub_upload(upload) } end context "when secure media is enabled" do @@ -125,7 +126,11 @@ RSpec.describe "tasks/uploads" do upload_to_mark_not_secure = Fabricate(:upload_s3, secure: true) post_for_upload = Fabricate(:post) PostUpload.create(post: post_for_upload, upload: upload_to_mark_not_secure) - enable_s3_uploads(uploads.concat([upload_to_mark_not_secure])) + + setup_s3 + uploads.each { |upload| stub_upload(upload) } + stub_upload(upload_to_mark_not_secure) + invoke_task expect(upload_to_mark_not_secure.reload.secure).to eq(false) end @@ -154,7 +159,10 @@ RSpec.describe "tasks/uploads" do before do global_setting :s3_bucket, 'file-uploads/folder' global_setting :s3_region, 'us-east-1' - enable_s3_uploads(uploads) + + setup_s3 + uploads.each { |upload| stub_upload(upload) } + upload1.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload1.base62_sha1}.png" upload1.save! upload2.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload2.base62_sha1}.png" @@ -206,7 +214,9 @@ RSpec.describe "tasks/uploads" do before do global_setting :s3_bucket, 'file-uploads/folder' global_setting :s3_region, 'us-east-1' - enable_s3_uploads(uploads) + setup_s3 + uploads.each { |upload| stub_upload(upload) } + upload1.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload1.base62_sha1}.png" upload1.save! upload2.url = "//#{SiteSetting.s3_upload_bucket}.amazonaws.com/original/1X/#{upload2.base62_sha1}.png" @@ -251,7 +261,9 @@ RSpec.describe "tasks/uploads" do end before do - enable_s3_uploads(uploads) + setup_s3 + uploads.each { |upload| stub_upload(upload) } + SiteSetting.secure_media = true PostUpload.create(post: post1, upload: upload1) PostUpload.create(post: post1, upload: upload2) @@ -300,20 +312,4 @@ RSpec.describe "tasks/uploads" do invoke_task end end - - def enable_s3_uploads(uploads) - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_upload_bucket = "s3-upload-bucket" - SiteSetting.s3_access_key_id = "some key" - SiteSetting.s3_secret_access_key = "some secrets3_region key" - - stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/") - - uploads.each do |upload| - stub_request( - :put, - "https://#{SiteSetting.s3_upload_bucket}.s3.amazonaws.com/original/1X/#{upload.sha1}.#{upload.extension}?acl" - ) - end - end end