diff --git a/lib/backup_restore/s3_backup_store.rb b/lib/backup_restore/s3_backup_store.rb index 71c368405df..cb2fcd72dea 100644 --- a/lib/backup_restore/s3_backup_store.rb +++ b/lib/backup_restore/s3_backup_store.rb @@ -83,14 +83,7 @@ module BackupRestore end def ensure_cors! - rule = { - allowed_headers: ["*"], - allowed_methods: ["PUT"], - allowed_origins: [Discourse.base_url_no_prefix], - max_age_seconds: 3000 - } - - @s3_helper.ensure_cors!([rule]) + @s3_helper.ensure_cors!([S3CorsRulesets::BACKUP_DIRECT_UPLOAD]) end def cleanup_allowed? diff --git a/lib/s3_cors_rulesets.rb b/lib/s3_cors_rulesets.rb new file mode 100644 index 00000000000..88f76f33527 --- /dev/null +++ b/lib/s3_cors_rulesets.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class S3CorsRulesets + ASSETS = { + allowed_headers: ["Authorization"], + allowed_methods: ["GET", "HEAD"], + allowed_origins: ["*"], + max_age_seconds: 3000 + }.freeze + + BACKUP_DIRECT_UPLOAD = { + allowed_headers: ["*"], + allowed_methods: ["PUT"], + allowed_origins: [Discourse.base_url_no_prefix], + max_age_seconds: 3000 + }.freeze +end diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index f5a57732953..253269b5d9a 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -140,12 +140,7 @@ class S3Helper end unless rule - rules = [{ - allowed_headers: ["Authorization"], - allowed_methods: ["GET", "HEAD"], - allowed_origins: ["*"], - max_age_seconds: 3000 - }] if rules.nil? + rules = [S3CorsRulesets::ASSETS] if rules.nil? s3_resource.client.put_bucket_cors( bucket: @s3_bucket_name, diff --git a/spec/components/s3_helper_spec.rb b/spec/components/s3_helper_spec.rb index daf229fc6fe..39df6ffeeca 100644 --- a/spec/components/s3_helper_spec.rb +++ b/spec/components/s3_helper_spec.rb @@ -129,4 +129,41 @@ describe "S3Helper" do expect(response.second).to eq("etag") end end + + describe "#ensure_cors" do + let(:s3_helper) { S3Helper.new("test-bucket", "", client: client) } + + it "does nothing if !s3_install_cors_rule" do + SiteSetting.s3_install_cors_rule = false + s3_helper.expects(:s3_resource).never + s3_helper.ensure_cors! + end + + it "creates the assets rule if no rule exists" do + s3_helper.s3_client.stub_responses(:get_bucket_cors, Aws::S3::Errors::NoSuchCORSConfiguration.new("", {})) + s3_helper.s3_client.expects(:put_bucket_cors).with( + bucket: s3_helper.s3_bucket_name, + cors_configuration: { + cors_rules: [S3CorsRulesets::ASSETS] + } + ) + s3_helper.ensure_cors! + end + + it "does nothing if a rule already exists" do + s3_helper.s3_client.stub_responses(:get_bucket_cors, { + cors_rules: [S3CorsRulesets::ASSETS] + }) + s3_helper.s3_client.expects(:put_bucket_cors).never + s3_helper.ensure_cors! + end + + it "does not apply the passed in rules if a different rule already exists" do + s3_helper.s3_client.stub_responses(:get_bucket_cors, { + cors_rules: [S3CorsRulesets::ASSETS] + }) + s3_helper.s3_client.expects(:put_bucket_cors).never + s3_helper.ensure_cors!([S3CorsRulesets::BACKUP_DIRECT_UPLOAD]) + end + end end