diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 4f6416f02257..d832ecae2db3 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -168,7 +168,19 @@ We have changed the behavior of the footer pages and topics links: You can read more about this change on PR [\#12592](https://github.com/decidim/decidim/pull/12592). -### 3.7. [[TITLE OF THE ACTION]] +### 3.7. Clean up orphaned attachment blobs + +We have added a new task that helps you clean the orphaned attachment blobs. This task will remove all the attachment blobs that have been created for more than 1 hour and are not yet referenced by any attachment record. This helps cleaning your filesystem of unused files. + +You can run the task with the following command: + +```bash +bundle exec decidim:attachments:cleanup +``` + +You can see more details about this change on PR [\#11851](https://github.com/decidim/decidim/pull/11851) + +### 3.8. [[TITLE OF THE ACTION]] You can read more about this change on PR [\#XXXX](https://github.com/decidim/decidim/pull/XXXX). diff --git a/decidim-core/app/commands/decidim/validate_upload.rb b/decidim-core/app/commands/decidim/validate_upload.rb index 6d590f0fb822..149ce190e31d 100644 --- a/decidim-core/app/commands/decidim/validate_upload.rb +++ b/decidim-core/app/commands/decidim/validate_upload.rb @@ -7,9 +7,18 @@ def initialize(form) end def call - return broadcast(:invalid, @form.errors) if @form.invalid? + if @form.invalid? + remove_invalid_file + return broadcast(:invalid, @form.errors) + end broadcast(:ok) end + + private + + def remove_invalid_file + @form.blob.purge_later if @form.blob.present? + end end end diff --git a/decidim-core/app/controllers/concerns/decidim/direct_upload.rb b/decidim-core/app/controllers/concerns/decidim/direct_upload.rb new file mode 100644 index 000000000000..a951b06f1b22 --- /dev/null +++ b/decidim-core/app/controllers/concerns/decidim/direct_upload.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Decidim + module DirectUpload + extend ActiveSupport::Concern + + included do + include Decidim::NeedsOrganization + skip_before_action :verify_organization + + before_action :check_organization!, + :check_authenticated!, + :check_user_belongs_to_organization, + :validate_direct_upload + end + + protected + + def verify_organization; end + + def validate_direct_upload + return if current_admin.present? + + head :unprocessable_entity unless [ + maximum_allowed_size.try(:to_i) >= blob_args[:byte_size].try(:to_i), + content_types.any? { |pattern| pattern.match?(blob_args[:content_type]) }, + content_types.any? { |pattern| pattern.match?(MiniMime.lookup_by_extension(extension)&.content_type) }, + allowed_extensions.any? { |pattern| pattern.match?(extension) } + ].all? + rescue NoMethodError + head :unprocessable_entity + end + + def extension + File.extname(blob_args[:filename]).delete(".") + end + + def maximum_allowed_size + current_organization.settings.upload_maximum_file_size + end + + def check_organization! + head :unauthorized if current_organization.blank? && current_admin.blank? + end + + def check_authenticated! + head :unauthorized if current_user.blank? && current_admin.blank? + end + + def check_user_belongs_to_organization + return if current_admin.present? + + head :unauthorized unless current_organization == current_user.organization + end + + def allowed_extensions + if user_has_elevated_role? + current_organization.settings.upload_allowed_file_extensions_admin + else + current_organization.settings.upload_allowed_file_extensions + end + end + + def content_types + if user_has_elevated_role? + current_organization.settings.upload_allowed_content_types_admin + else + current_organization.settings.upload_allowed_content_types + end + end + + private + + def user_has_elevated_role? + [ + current_user&.admin?, + defined?(Decidim::Assemblies::AssembliesWithUserRole) && Decidim::Assemblies::AssembliesWithUserRole.for(current_user).any?, + defined?(Decidim::Conferences::ConferencesWithUserRole) && Decidim::Conferences::ConferencesWithUserRole.for(current_user).any?, + defined?(Decidim::ParticipatoryProcessesWithUserRole) && Decidim::ParticipatoryProcessesWithUserRole.for(current_user).any? + ].any? + end + end +end diff --git a/decidim-core/app/forms/decidim/upload_validation_form.rb b/decidim-core/app/forms/decidim/upload_validation_form.rb index e4741c58191c..1bc2ae09b736 100644 --- a/decidim-core/app/forms/decidim/upload_validation_form.rb +++ b/decidim-core/app/forms/decidim/upload_validation_form.rb @@ -10,7 +10,7 @@ class UploadValidationForm < Decidim::Form # Property is named as attribute in upload modal and passthru validator, but # it cannot be named as attribute here. attribute :property, String - attribute :blob, String + attribute :blob, Decidim::Attributes::Blob attribute :form_class, String validates :resource_class, presence: true diff --git a/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js b/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js index ff1b752bf451..1940f6e26b3c 100644 --- a/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js +++ b/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js @@ -77,6 +77,9 @@ export default class UploadModal { uploader.upload.create((error, blob) => { if (error) { uploader.errors = [error] + this.uploadItems.replaceChild(this.createUploadItem(file, [error], { value: 100 }), item); + this.updateDropZone(); + } else { // attach the file hash to submit the form, when the file has been uploaded file.hiddenField = blob.signed_id diff --git a/decidim-core/lib/decidim/core.rb b/decidim-core/lib/decidim/core.rb index b8dd35bfb2e4..2b7432785e18 100644 --- a/decidim-core/lib/decidim/core.rb +++ b/decidim-core/lib/decidim/core.rb @@ -566,6 +566,12 @@ def self.reset_all_column_information {} end + # This config parameter is used to set the default amount of time after which the unattached blobs + # are cleaned up. The default value is 60 minutes. + config_accessor :clean_up_unattached_blobs_after do + 60.minutes + end + # Public: Registers a global engine. This method is intended to be used # by component engines that also offer unscoped functionality # diff --git a/decidim-core/lib/decidim/core/engine.rb b/decidim-core/lib/decidim/core/engine.rb index 1aed6cd63a18..6bd622b1901e 100644 --- a/decidim-core/lib/decidim/core/engine.rb +++ b/decidim-core/lib/decidim/core/engine.rb @@ -268,6 +268,12 @@ class Engine < ::Rails::Engine app.config.exceptions_app = Decidim::Core::Engine.routes end + initializer "decidim_core.direct_uploader_paths", after: "decidim_core.exceptions_app" do |_app| + config.to_prepare do + ActiveStorage::DirectUploadsController.include Decidim::DirectUpload + end + end + initializer "decidim_core.locales" do |app| app.config.i18n.fallbacks = true end diff --git a/decidim-core/lib/decidim/organization_settings.rb b/decidim-core/lib/decidim/organization_settings.rb index 4d749bd1a6d6..fdcf575a51f3 100644 --- a/decidim-core/lib/decidim/organization_settings.rb +++ b/decidim-core/lib/decidim/organization_settings.rb @@ -104,8 +104,8 @@ def defaults_hash { "upload" => { "allowed_file_extensions" => { - "default" => %w(jpg jpeg png webp pdf rtf txt), - "admin" => %w(jpg jpeg png webp pdf doc docx xls xlsx ppt pptx ppx rtf txt odt ott odf otg ods ots), + "default" => %w(jpg jpeg png webp pdf rtf txt md), + "admin" => %w(jpg jpeg png webp pdf doc docx xls xlsx ppt pptx ppx rtf txt odt ott odf otg ods ots csv json md), "image" => %w(jpg jpeg png webp) }, "allowed_content_types" => { @@ -113,6 +113,7 @@ def defaults_hash image/* application/pdf application/rtf + text/markdown text/plain ), "admin" => %w( @@ -125,7 +126,10 @@ def defaults_hash application/vnd.oasis.opendocument application/pdf application/rtf + application/json + text/markdown text/plain + text/csv ) }, "maximum_file_size" => { diff --git a/decidim-core/lib/tasks/decidim_attachments.rake b/decidim-core/lib/tasks/decidim_attachments.rake new file mode 100644 index 000000000000..920d6e518cec --- /dev/null +++ b/decidim-core/lib/tasks/decidim_attachments.rake @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +namespace :decidim do + namespace :attachments do + desc "Cleanup the orphaned blobs attachments" + task cleanup: :environment do + return if Decidim.clean_up_unattached_blobs_after.to_i.zero? + + ActiveStorage::Blob.unattached.where(created_at: ..Decidim.clean_up_unattached_blobs_after.ago).find_each(batch_size: 100, &:purge_later) + end + end +end diff --git a/decidim-core/spec/commands/decidim/validate_upload_spec.rb b/decidim-core/spec/commands/decidim/validate_upload_spec.rb index 2e61b7a21b38..06edda7fd298 100644 --- a/decidim-core/spec/commands/decidim/validate_upload_spec.rb +++ b/decidim-core/spec/commands/decidim/validate_upload_spec.rb @@ -9,16 +9,24 @@ module Decidim let(:form) do double( invalid?: invalid, + blob:, errors: ) end let(:invalid) { false } let(:errors) { [] } + let(:io) { Decidim::Dev.test_file("city.jpeg", "image/jpeg") } + let(:blob) { ActiveStorage::Blob.create_and_upload!(io:, filename: "city.jpeg") } describe "when form is valid" do it "broadcasts ok" do expect { command.call }.to broadcast(:ok) end + + it "ignores file removal" do + expect(ActiveStorage::Blob).not_to receive(:find_signed).with(blob) + command.call + end end describe "when the form is not valid" do @@ -28,6 +36,11 @@ module Decidim it "broadcasts invalid" do expect { command.call }.to broadcast(:invalid, errors) end + + it "removes the invalid file" do + expect(blob).to receive(:purge_later) + command.call + end end end end diff --git a/decidim-core/spec/controllers/active_storage/direct_uploads_controller_spec.rb b/decidim-core/spec/controllers/active_storage/direct_uploads_controller_spec.rb new file mode 100644 index 000000000000..7bf6cd118542 --- /dev/null +++ b/decidim-core/spec/controllers/active_storage/direct_uploads_controller_spec.rb @@ -0,0 +1,145 @@ +# frozen_string_literal: true + +require "spec_helper" + +module ActiveStorage + describe DirectUploadsController do + describe "POST #create" do + let(:checksum) { OpenSSL::Digest.base64digest("MD5", "Hello") } + + let(:blob) do + { + filename: "hello.txt", + byte_size: 6, + checksum:, + content_type: "text/plain" + } + end + + let(:extensions) { %w(txt) } + let(:content_types) { %w(text/plain) } + let(:file_size) { { "default" => 5, "avatar" => 2 } } + + let(:file_upload_settings) do + { + "allowed_file_extensions" => { + "default" => extensions, + "admin" => extensions, + "image" => extensions + }, + "allowed_content_types" => { + "default" => content_types, + "admin" => content_types + }, + "maximum_file_size" => file_size + } + end + + let(:organization) { create(:organization, file_upload_settings:, favicon: nil, official_img_footer: nil) } + let!(:user) { create(:user, :confirmed, organization:, avatar: nil) } + + let(:params) do + { + blob:, + direct_upload: blob + } + end + + context "when the user is not logged in" do + before do + request.env["decidim.current_organization"] = organization + end + + it "returns unauthorized" do + post(:create, params:) + + expect(response).to have_http_status(:unauthorized) + end + end + + context "when the organization does not exists" do + before do + request.env["decidim.current_organization"] = nil + request.headers["HTTP_REFERER"] = "http://#{organization.host}" + sign_in user + end + + it "returns unauthorized" do + post(:create, params:) + + expect(response).to have_http_status(:unauthorized) + end + end + + context "when the upload is successful" do + before do + request.env["decidim.current_organization"] = organization + request.headers["HTTP_REFERER"] = "http://#{organization.host}" + sign_in user + end + + it "returns success" do + post(:create, params:) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("content_type") + end + end + + context "when the attachment is not allowed" do + before do + request.env["decidim.current_organization"] = organization + request.headers["HTTP_REFERER"] = "http://#{organization.host}" + sign_in user + end + + context "when content_type is not allowed" do + let(:content_types) { %w(image/jpeg) } + + it "returns renders unprocessable entity" do + post(:create, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context "when extension is not allowed" do + let(:extensions) { %w(jpeg) } + + it "returns renders unprocessable entity" do + post(:create, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context "when size is not allowed" do + let(:file_size) { { "default" => 0, "avatar" => 0 } } + + it "returns renders unprocessable entity" do + post(:create, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context "when extension not matching content_type" do + let(:blob) do + { + filename: "hello.pdf", + byte_size: 6, + checksum:, + content_type: "text/plain" + } + end + + it "returns renders unprocessable entity" do + post(:create, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + end + end +end diff --git a/decidim-core/spec/lib/organization_settings_spec.rb b/decidim-core/spec/lib/organization_settings_spec.rb index 7d04cb502456..cb282d1af7d1 100644 --- a/decidim-core/spec/lib/organization_settings_spec.rb +++ b/decidim-core/spec/lib/organization_settings_spec.rb @@ -11,8 +11,8 @@ module Decidim let(:default_settings) do { "allowed_file_extensions" => { - "default" => %w(jpg jpeg png webp pdf rtf txt), - "admin" => %w(jpg jpeg png webp pdf doc docx xls xlsx ppt pptx ppx rtf txt odt ott odf otg ods ots), + "default" => %w(jpg jpeg png webp pdf rtf txt md), + "admin" => %w(jpg jpeg png webp pdf doc docx xls xlsx ppt pptx ppx rtf txt odt ott odf otg ods ots csv json md), "image" => %w(jpg jpeg png webp) }, "allowed_content_types" => { @@ -20,6 +20,7 @@ module Decidim image/* application/pdf application/rtf + text/markdown text/plain ), "admin" => %w( @@ -32,7 +33,10 @@ module Decidim application/vnd.oasis.opendocument application/pdf application/rtf + application/json + text/markdown text/plain + text/csv ) }, "maximum_file_size" => { diff --git a/decidim-dev/lib/decidim/dev/assets/invalid_extension.log b/decidim-dev/lib/decidim/dev/assets/invalid_extension.log new file mode 100644 index 000000000000..87b10bd8e638 --- /dev/null +++ b/decidim-dev/lib/decidim/dev/assets/invalid_extension.log @@ -0,0 +1 @@ +FooBar diff --git a/decidim-forms/spec/commands/decidim/forms/answer_questionnaire_spec.rb b/decidim-forms/spec/commands/decidim/forms/answer_questionnaire_spec.rb index a07a5c51a43c..5b82380919f2 100644 --- a/decidim-forms/spec/commands/decidim/forms/answer_questionnaire_spec.rb +++ b/decidim-forms/spec/commands/decidim/forms/answer_questionnaire_spec.rb @@ -168,7 +168,7 @@ def tokenize(id) }, { title: "CSV document", - file: upload_test_file(Decidim::Dev.asset("verify_user_groups.csv"), content_type: "text/csv") + file: upload_test_file(Decidim::Dev.asset("invalid_extension.log"), content_type: "text/plain") } ] end diff --git a/decidim-generators/lib/decidim/generators/app_templates/initializer.rb b/decidim-generators/lib/decidim/generators/app_templates/initializer.rb index 05f25811d203..4f4d9d952bfa 100644 --- a/decidim-generators/lib/decidim/generators/app_templates/initializer.rb +++ b/decidim-generators/lib/decidim/generators/app_templates/initializer.rb @@ -382,6 +382,10 @@ # Read more: https://docs.decidim.org/en/develop/configure/initializer#_content_security_policy config.content_security_policies_extra = {} + # This config parameter is used to set the default amount of time after which the unattached blobs + # are cleaned up. The default value is 60 minutes. + # config.clean_up_unattached_blobs_after = 60.minutes + # Admin admin password configurations Rails.application.secrets.dig(:decidim, :admin_password, :strong).tap do |strong_pw| # When the strong password is not configured, default to true diff --git a/decidim-initiatives/spec/commands/decidim/initiatives/update_initiative_spec.rb b/decidim-initiatives/spec/commands/decidim/initiatives/update_initiative_spec.rb index 9fc91f80d674..0f622b3121a8 100644 --- a/decidim-initiatives/spec/commands/decidim/initiatives/update_initiative_spec.rb +++ b/decidim-initiatives/spec/commands/decidim/initiatives/update_initiative_spec.rb @@ -165,7 +165,7 @@ module Initiatives let(:uploaded_files) do [ upload_test_file(Decidim::Dev.test_file("city.jpeg", "image/jpeg")), - upload_test_file(Decidim::Dev.test_file("verify_user_groups.csv", "text/csv")) + upload_test_file(Decidim::Dev.test_file("invalid_extension.log", "text/plain")) ] end diff --git a/decidim-meetings/spec/system/meeting_registrations_spec.rb b/decidim-meetings/spec/system/meeting_registrations_spec.rb index 90cd57f1f94a..e44ebc13a239 100644 --- a/decidim-meetings/spec/system/meeting_registrations_spec.rb +++ b/decidim-meetings/spec/system/meeting_registrations_spec.rb @@ -301,13 +301,9 @@ def questionnaire_public_path it "shows errors for invalid file" do visit questionnaire_public_path - dynamically_attach_file("questionnaire_responses_0_add_documents", Decidim::Dev.asset("verify_user_groups.csv")) + dynamically_attach_file("questionnaire_responses_0_add_documents", Decidim::Dev.asset("verify_user_groups.csv"), keep_modal_open: true) - expect(page).to have_field("public_participation", checked: false) - find_by_id("questionnaire_tos_agreement").set(true) - accept_confirm { click_on "Submit" } - - expect(page).to have_content("Needs to be reattached") + expect(page).to have_content("Validation error!") end context "and the announcement for the meeting is configured" do diff --git a/decidim-proposals/spec/system/edit_proposal_spec.rb b/decidim-proposals/spec/system/edit_proposal_spec.rb index 0b6a570499c7..40a2234f5e40 100644 --- a/decidim-proposals/spec/system/edit_proposal_spec.rb +++ b/decidim-proposals/spec/system/edit_proposal_spec.rb @@ -52,10 +52,10 @@ it "shows validation error when format is not accepted" do click_on "Edit proposal" - dynamically_attach_file(:proposal_documents, Decidim::Dev.asset("participatory_text.md"), keep_modal_open: true) do + dynamically_attach_file(:proposal_documents, Decidim::Dev.asset("dummy-dummies-example.xlsx"), keep_modal_open: true) do expect(page).to have_content("Accepted formats: #{Decidim::OrganizationSettings.for(organization).upload_allowed_file_extensions_image.join(", ")}") end - expect(page).to have_content("only files with the following extensions are allowed: jpeg, jpg, pdf, png, rtf, txt") + expect(page).to have_content("Validation error!") end context "with a file and photo" do