Remove default Active Storage direct upload action

We're already using a custom controller to handle direct uploads.

Besides, as mentioned by one of Active Storage co-authors [1], the
Active Storage DirectUploadsController doesn't provide any
authentication or validation at all, meaning anyone could create blobs
in our database by posting to `/rails/active_storage/direct_uploads`.
The response there could be then used to upload any file (again, without
validation) to `/rails/active_storage/disk/`.

For now, we're monkey-patching the controllers in order to send
unauthorized responses, since we aren't using these routes. If we ever
enable direct uploads with Active Storage, we'll have to add some sort
of authentication.

Similar upload solutions like CKEditor don't have this issue since their
controllers inherit from `ApplicationController` (which includes
authorization rules), while Active Storage controllers inherit from
`ActionController::Base`.

[1] https://discuss.rubyonrails.org/t/activestorage-direct-uploads-safe-by-default-how-to-make-it-safe/74863/2
This commit is contained in:
Javi Martín
2021-09-23 00:38:19 +02:00
parent 259cd8e108
commit be9c272ce4
4 changed files with 44 additions and 0 deletions

View File

@@ -0,0 +1,11 @@
ActiveStorage::DirectUploadsController.class_eval do
def create
head :unauthorized
end
end
ActiveStorage::DiskController.class_eval do
def update
head :unauthorized
end
end

View File

@@ -0,0 +1,14 @@
require "rails_helper"
describe ActiveStorage::DirectUploadsController do
describe "POST create" do
it "doesn't allow anonymous users to upload files" do
blob_attributes = { filename: "logo.pdf", byte_size: 30000, checksum: SecureRandom.hex(32) }
post :create, params: { blob: blob_attributes }
expect(ActiveStorage::Blob.count).to eq 0
expect(response).to be_unauthorized
end
end
end

View File

@@ -0,0 +1,13 @@
require "rails_helper"
describe ActiveStorage::DiskController do
describe "PUT update" do
it "doesn't allow anonymous users to upload files" do
blob = create(:active_storage_blob)
put :update, params: { encoded_token: blob.signed_id }
expect(response).to be_unauthorized
end
end
end

View File

@@ -55,4 +55,10 @@ FactoryBot.define do
end end
initialize_with { new(attributes) } initialize_with { new(attributes) }
end end
factory :active_storage_blob, class: "ActiveStorage::Blob" do
filename { "sample.pdf" }
byte_size { 3000 }
checksum { SecureRandom.hex(32) }
end
end end