Remove invalid upload files

RELEASE_NOTES.md

@@ -116,7 +116,21 @@ bin/rails decidim:upgrade:fix_orphan_categorizations
 
 You can read more about this change on PR [\#12143](https://github.com/decidim/decidim/pull/12143).
 
-### 3.5. [[TITLE OF THE ACTION]]
+### 3.5. 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.
+
+For a consistent behavior we are recommending to run this task 2 times at least 1 hour apart.
+
+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.6. [[TITLE OF THE ACTION]]
 
 You can read more about this change on PR [\#XXXX](https://github.com/decidim/decidim/pull/XXXX).
 

decidim-core/app/commands/decidim/validate_upload.rb

@@ -7,9 +7,19 @@ 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
+      blob = ActiveStorage::Blob.find_signed(@form.blob)
+      blob.purge if blob.present?
+    end
   end
 end

decidim-core/app/controllers/decidim/direct_uploads_controller.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module Decidim
+  class DirectUploadsController < ActiveStorage::DirectUploadsController
+    include Decidim::NeedsOrganization
+    skip_before_action :verify_organization
+
+    before_action :check_organization!
+    before_action :check_authenticated!
+    before_action :validate_direct_upload
+
+    protected
+
+    def verify_organization; end
+
+    def validate_direct_upload
+      maximum_allowed_size = current_organization.settings.upload_maximum_file_size
+
+      extension = File.extname(blob_args[:filename]).delete(".")
+
+      Rails.logger.debug content_types.inspect
+
+      head :unprocessable_entity unless maximum_allowed_size.try(:to_i) >= blob_args[:byte_size].try(:to_i)
+      head :unprocessable_entity unless content_types.any? { |pattern| pattern.match?(blob_args[:content_type]) }
+      head :unprocessable_entity unless content_types.any? { |pattern| pattern.match?(MiniMime.lookup_by_extension(extension).content_type) }
+      head :unprocessable_entity unless allowed_extensions.any? { |pattern| pattern.match?(extension) }
+    rescue NoMethodError
+      head :unprocessable_entity
+    end
+
+    def check_organization!
+      head :unauthorized if current_organization.blank?
+    end
+
+    def check_authenticated!
+      head :unauthorized if current_user.blank?
+    end
+
+    def allowed_extensions
+      if URI.parse(request.referer).path.starts_with?("/admin")
+        current_organization.settings.upload_allowed_file_extensions_admin
+      else
+        current_organization.settings.upload_allowed_file_extensions
+      end
+    end
+
+    def content_types
+      if URI.parse(request.referer).path.starts_with?("/admin")
+        current_organization.settings.upload_allowed_content_types_admin
+      else
+        current_organization.settings.upload_allowed_content_types
+      end
+    end
+  end
+end

decidim-core/lib/decidim/core/engine.rb

@@ -266,6 +266,14 @@ 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|
+        Rails.application.routes.prepend do
+          scope ActiveStorage.routes_prefix do
+            post "/direct_uploads" => "decidim/direct_uploads#create", :as => :decidim_direct_uploads
+          end
+        end
+      end
+
       initializer "decidim_core.locales" do |app|
         app.config.i18n.fallbacks = true
       end

decidim-core/lib/tasks/decidim_attachments.rake

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+namespace :decidim do
+  namespace :attachments do
+    desc "Cleanup the orphaned blobs attachments"
+    task cleanup: :environment do
+      ActiveStorage::Blob.unattached.where(ActiveStorage::Blob.arel_table[:created_at].lteq(1.hour.ago)).find_each(&:purge_later)
+    end
+  end
+end

decidim-core/spec/commands/decidim/validate_upload_spec.rb

@@ -9,25 +9,38 @@ module Decidim
       let(:form) do
         double(
           invalid?: invalid,
+          blob:,
           errors:
         )
       end
       let(:invalid) { false }
       let(:errors) { [] }
+      let(:blob) { "foobar" }
 
       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
         let(:invalid) { true }
         let(:errors) { ["File too dummy"] }
 
         it "broadcasts invalid" do
+          allow(ActiveStorage::Blob).to receive(:find_signed).with(blob).and_return(double(purge: true))
           expect { command.call }.to broadcast(:invalid, errors)
         end
+
+        it "removes the invalid file" do
+          expect(ActiveStorage::Blob).to receive(:find_signed)
+          command.call
+        end
       end
     end
   end

decidim-core/spec/controllers/decidim/direct_uploads_controller_spec.rb

@@ -0,0 +1,145 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module Decidim
+  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