Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove invalid upload files #11851

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
43a5fc5
Remove orphaned blobs
alecslupu Oct 27, 2023
5d6ab94
Add Rake task to cleanup old unattached blobs
alecslupu Oct 27, 2023
bd7c937
Refactor task
alecslupu Oct 27, 2023
dcdba03
Add job work to remove unattached blobs after a period of time
alecslupu Nov 8, 2023
87ec9b4
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Nov 8, 2023
f6323cc
Running linters
alecslupu Nov 8, 2023
1f0d755
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Nov 10, 2023
d01bfce
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Mar 8, 2024
84af5a6
Running linters
alecslupu Mar 8, 2024
b51842c
Fix spec
alecslupu Mar 8, 2024
e83826a
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Mar 11, 2024
b9ace94
Fix spelling
alecslupu Mar 11, 2024
dd16d30
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Apr 15, 2024
342751e
Remove the "decidim_core.clean_attachments" initializer and the Clean…
alecslupu Apr 15, 2024
3490ff8
Change the rake task so it prevents deleting blobs that are less than…
alecslupu Apr 15, 2024
63586fa
Change the Releases Notes so the task is run two times with one hour …
alecslupu Apr 15, 2024
2f38d9c
DirectUploadsController validations
alecslupu Apr 16, 2024
7bccc7e
Add specs
alecslupu Apr 16, 2024
2c961cd
Update RELEASE_NOTES.md
alecslupu Apr 16, 2024
8a9342b
Merge branch 'develop' into fix/attachment-validation-cleanup
alecslupu Apr 16, 2024
a544e43
Apply review recommendation
alecslupu Apr 16, 2024
31a384b
Fix specs
alecslupu Apr 17, 2024
dd28ea8
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Jun 9, 2024
e03b5ba
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Jun 12, 2024
74724a2
Convert Blob field to Blob type
alecslupu Jun 12, 2024
7ea1bbe
Remove CSV from allowed types
alecslupu Jun 12, 2024
ee2f2aa
A bit more security
alecslupu Jun 12, 2024
edc60c1
Fix specs
alecslupu Jun 13, 2024
b89acbd
More fixes
alecslupu Jun 13, 2024
5fd8c00
Fix Validator upload
alecslupu Jun 13, 2024
0aa5de1
lint
alecslupu Jun 13, 2024
fc978f1
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Jul 17, 2024
93642ac
Add new tests
alecslupu Jul 17, 2024
8a0d7bb
Merge branch 'develop' into fix/attachment-validation-cleanup
alecslupu Aug 14, 2024
6763dc6
Merge branch 'develop' of github.com:decidim/decidim into fix/attachm…
alecslupu Sep 5, 2024
a600e6f
Merge branch 'fix/attachment-validation-cleanup' of github.com:decidi…
alecslupu Sep 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,19 @@ bundle remove spring spring-watcher-listen

You can read more about this change on PR [#13235](https://github.com/decidim/decidim/pull/13235).

### 3.2. [[TITLE OF THE ACTION]]
### 3.2. 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.3. [[TITLE OF THE ACTION]]

You can read more about this change on PR [#XXXX](https://github.com/decidim/decidim/pull/XXXX).

Expand Down
11 changes: 10 additions & 1 deletion decidim-core/app/commands/decidim/validate_upload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
83 changes: 83 additions & 0 deletions decidim-core/app/controllers/concerns/decidim/direct_upload.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion decidim-core/app/forms/decidim/upload_validation_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions decidim-core/lib/decidim/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,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
#
Expand Down
6 changes: 6 additions & 0 deletions decidim-core/lib/decidim/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,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
Expand Down
8 changes: 6 additions & 2 deletions decidim-core/lib/decidim/organization_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,16 @@ 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" => {
"default" => %w(
image/*
application/pdf
application/rtf
text/markdown
text/plain
),
"admin" => %w(
Expand All @@ -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" => {
Expand Down
12 changes: 12 additions & 0 deletions decidim-core/lib/tasks/decidim_attachments.rake
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions decidim-core/spec/commands/decidim/validate_upload_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading
Loading