From b7a2d29b7bc509dbd4de6bd4121b2397150f5a61 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 10 Apr 2024 12:02:44 +1000 Subject: [PATCH] DEV: Introduce post_should_secure_uploads? plugin modifier (#26508) This modifier allows plugins to alter the outcome of `should_secure_uploads?` on a Post record, for cases when plugins need post-attached uploads to always be secure (or not secure) in specific scenarios. --- app/models/post.rb | 15 +++++++++++++++ lib/topic_upload_security_manager.rb | 15 ++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 1c7ce99d334..7445c8de9c0 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -565,11 +565,26 @@ class Post < ActiveRecord::Base ReviewableFlaggedPost.pending.find_by(target: self) end + # NOTE (martin): This is turning into hack city; when changing this also + # consider how it interacts with UploadSecurity and the uploads.rake tasks. def should_secure_uploads? return false if !SiteSetting.secure_uploads? topic_including_deleted = Topic.with_deleted.find_by(id: self.topic_id) return false if topic_including_deleted.blank? + # NOTE: This is to be used for plugins where adding a new public upload + # type that should not be secured via UploadSecurity.register_custom_public_type + # is not an option. This also is not taken into account in the secure upload + # rake tasks, and will more than likely change in future. + modifier_result = + DiscoursePluginRegistry.apply_modifier( + :post_should_secure_uploads?, + nil, + self, + topic_including_deleted, + ) + return modifier_result if !modifier_result.nil? + # NOTE: This is meant to be a stopgap solution to prevent secure uploads # in a single place (private messages) for sensitive admin data exports. # Ideally we would want a more comprehensive way of saying that certain diff --git a/lib/topic_upload_security_manager.rb b/lib/topic_upload_security_manager.rb index d1cc634d52b..1b0be86998a 100644 --- a/lib/topic_upload_security_manager.rb +++ b/lib/topic_upload_security_manager.rb @@ -21,6 +21,7 @@ class TopicUploadSecurityManager end def run + rebaked_posts = [] Rails.logger.debug("Updating upload security in topic #{@topic.id}") posts_owning_uploads.each do |post| Post.transaction do @@ -35,14 +36,18 @@ class TopicUploadSecurityManager upload.access_control_post = post upload.update_secure_status(source: "topic upload security") end - post.rebake! if secure_status_did_change + + if secure_status_did_change + post.rebake! + rebaked_posts << post + end Rails.logger.debug( "Security updated & rebake complete in topic #{@topic.id} - post #{post.id}", ) end end - return if !SiteSetting.secure_uploads + return rebaked_posts if !SiteSetting.secure_uploads # We only want to do this if secure uploads is enabled. If # the setting is turned on after a site has been running @@ -76,7 +81,10 @@ class TopicUploadSecurityManager end end - post.rebake! if secure_status_did_change + if secure_status_did_change + post.rebake! + rebaked_posts << post + end Rails.logger.debug( "Completed changing access control posts #{secure_status_did_change ? "and rebaking" : ""} in topic #{@topic.id} - post #{post.id}", ) @@ -84,6 +92,7 @@ class TopicUploadSecurityManager end Rails.logger.debug("Completed updating upload security in topic #{@topic.id}!") + rebaked_posts end private