FIX: Do not enable published page if secure media enabled (#11131)

There are issues around displaying images on published pages when secure media is enabled. This PR temporarily makes it appear as if published pages are enabled if secure media is also enabled.

app/controllers/published_pages_controller.rb

@@ -92,7 +92,9 @@ private
   end
 
   def ensure_publish_enabled
-    raise Discourse::NotFound unless SiteSetting.enable_page_publishing?
+    if !SiteSetting.enable_page_publishing? || SiteSetting.secure_media
+      raise Discourse::NotFound
+    end
   end
 
   def enforce_login_required!

app/serializers/topic_view_serializer.rb

@@ -275,7 +275,10 @@ class TopicViewSerializer < ApplicationSerializer
   end
 
   def include_published_page?
-    SiteSetting.enable_page_publishing? && scope.is_staff? && object.published_page.present?
+    SiteSetting.enable_page_publishing? &&
+      scope.is_staff? &&
+      object.published_page.present? &&
+      !SiteSetting.secure_media
   end
 
   def thumbnails

config/locales/server.en.yml

@@ -190,6 +190,7 @@ en:
       default_tags_already_selected: "You cannot select a tag used in another list."
       s3_upload_bucket_is_required: "You cannot enable uploads to S3 unless you've provided the 's3_upload_bucket'."
       enable_s3_uploads_is_required: "You cannot enable inventory to S3 unless you've enabled the S3 uploads."
+      page_publishing_requirements: "Page publishing cannot be enabled if secure media is enabled."
       s3_backup_requires_s3_settings: "You cannot use S3 as backup location unless you've provided the '%{setting_name}'."
       s3_bucket_reused: "You cannot use the same bucket for 's3_upload_bucket' and 's3_backup_bucket'. Choose a different bucket or use a different path for each bucket."
       secure_media_requirements: "S3 uploads must be enabled before enabling secure media."

lib/guardian.rb

@@ -523,7 +523,8 @@ class Guardian
   end
 
   def can_publish_page?(topic)
-    return false unless SiteSetting.enable_page_publishing?
+    return false if !SiteSetting.enable_page_publishing?
+    return false if SiteSetting.secure_media?
     return false if topic.blank?
     return false if topic.private_message?
     return false unless can_see_topic?(topic)

lib/site_settings/validations.rb

@@ -144,6 +144,10 @@ module SiteSettings::Validations
     validate_error :secure_media_requirements if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads
   end
 
+  def validate_enable_page_publishing(new_val)
+    validate_error :page_publishing_requirements if new_val == "t" && SiteSetting.secure_media?
+  end
+
   def validate_share_quote_buttons(new_val)
     validate_error :share_quote_facebook_requirements if new_val.include?("facebook") && SiteSetting.facebook_app_id.blank?
   end

spec/components/guardian_spec.rb

@@ -3756,6 +3756,20 @@ describe Guardian do
         post = Fabricate(:private_message_post, user: admin)
         expect(Guardian.new(admin).can_publish_page?(post.topic)).to eq(false)
       end
+
+      context "when secure_media is also enabled" do
+        before do
+          setup_s3
+          SiteSetting.secure_media = true
+        end
+
+        it "is false for everyone" do
+          expect(Guardian.new(moderator).can_publish_page?(topic)).to eq(false)
+          expect(Guardian.new(user).can_publish_page?(topic)).to eq(false)
+          expect(Guardian.new.can_publish_page?(topic)).to eq(false)
+          expect(Guardian.new(admin).can_publish_page?(topic)).to eq(false)
+        end
+      end
     end
   end
 end

spec/lib/site_settings/validations_spec.rb

@@ -205,6 +205,25 @@ describe SiteSettings::Validations do
       end
     end
 
+    describe "#validate_enable_page_publishing" do
+      context "when the new value is true" do
+        it "is ok" do
+          expect { subject.validate_enable_page_publishing("t") }.not_to raise_error
+        end
+
+        context "if secure media is enabled" do
+          let(:error_message) { I18n.t("errors.site_settings.page_publishing_requirements") }
+          before do
+            enable_secure_media
+          end
+
+          it "is not ok" do
+            expect { subject.validate_enable_page_publishing("t") }.to raise_error(Discourse::InvalidParameters, error_message)
+          end
+        end
+      end
+    end
+
     describe "#validate_secure_media" do
       let(:error_message) { I18n.t("errors.site_settings.secure_media_requirements") }
 

spec/requests/published_pages_controller_spec.rb

@@ -93,6 +93,18 @@ RSpec.describe PublishedPagesController do
           published_page.topic.tags = [Fabricate(:tag, name: "recipes")]
         end
 
+        context "when secure media is enabled" do
+          before do
+            setup_s3
+            SiteSetting.secure_media = true
+          end
+
+          it "returns 404" do
+            get published_page.path
+            expect(response.status).to eq(404)
+          end
+        end
+
         it "returns 200" do
           get published_page.path
           expect(response.status).to eq(200)

spec/serializers/topic_view_serializer_spec.rb

@@ -399,6 +399,18 @@ describe TopicViewSerializer do
           expect(json[:published_page]).to be_present
           expect(json[:published_page][:slug]).to eq(published_page.slug)
         end
+
+        context "secure media is enabled" do
+          before do
+            setup_s3
+            SiteSetting.secure_media = true
+          end
+
+          it "doesn't return the published page" do
+            json = serialize_topic(topic, admin)
+            expect(json[:published_page]).to be_blank
+          end
+        end
       end
     end
   end

spec/support/uploads_helpers.rb

@@ -13,6 +13,11 @@ module UploadsHelpers
     stub_request(:head, "https://#{SiteSetting.s3_upload_bucket}.s3.#{SiteSetting.s3_region}.amazonaws.com/")
   end
 
+  def enable_secure_media
+    setup_s3
+    SiteSetting.secure_media = true
+  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)