mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-07-08 00:50:22 +09:00
Require authentication for ActiveStorage direct-upload write endpoints
ActiveStorage's direct-upload endpoints ship unauthenticated by Rails default: ActiveStorage::DirectUploadsController and DiskController inherit from ActionController::Base, so they bypass the app's Authentication concern. That leaves the write path open — anyone could mint blob records and PUT bytes to local disk storage. Campfire never uses direct upload for legitimate attachments. Those flow through MessagesController#create (already authenticated), and Trix file drops are disabled in the composer. Gating the write path is therefore pure defense-in-depth with no functional cost. Require an authenticated session on the two write actions (DirectUploadsController#create and DiskController#update) by including the existing Authentication concern. Blob serving stays public — DiskController#show keeps its auth skip, and the Blobs/Representations controllers are untouched — so message attachments and the account logo keep loading. Because these controllers live in ActiveStorage::Engine and only see the engine's url helpers, also include the application route helpers so the concern can redirect to new_session_url on failure (302, write blocked).
This commit is contained in:
@@ -2,4 +2,20 @@ ActiveSupport.on_load(:active_storage_blob) do
|
||||
ActiveStorage::DiskController.after_action only: :show do
|
||||
response.set_header("Cache-Control", "max-age=3600, public")
|
||||
end
|
||||
|
||||
# Gate the ActiveStorage write path behind app authentication. These endpoints
|
||||
# ship unauthenticated by Rails default; Campfire never uses direct upload for
|
||||
# legit attachments (those go through MessagesController#create, and Trix file
|
||||
# drops are disabled in the composer). Requiring a session on the write actions
|
||||
# blocks anonymous blob writes and disk-fill while leaving blob serving public.
|
||||
#
|
||||
# ActiveStorage controllers live in ActiveStorage::Engine, so they see the
|
||||
# engine's url helpers, not the main app's. Include the application helpers
|
||||
# first so Authentication#request_authentication can redirect to new_session_url.
|
||||
ActiveStorage::DirectUploadsController.include Rails.application.routes.url_helpers
|
||||
ActiveStorage::DirectUploadsController.include Authentication # only action: #create
|
||||
|
||||
ActiveStorage::DiskController.include Rails.application.routes.url_helpers
|
||||
ActiveStorage::DiskController.include Authentication
|
||||
ActiveStorage::DiskController.skip_before_action :require_authentication, :deny_bots, only: :show
|
||||
end
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
require "test_helper"
|
||||
|
||||
class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
host! "once.campfire.test"
|
||||
end
|
||||
|
||||
test "create requires authentication" do
|
||||
assert_no_difference -> { ActiveStorage::Blob.count } do
|
||||
post rails_direct_uploads_url, params: { blob: blob_params }
|
||||
end
|
||||
|
||||
assert_redirected_to new_session_url
|
||||
end
|
||||
|
||||
test "create succeeds for an authenticated session" do
|
||||
sign_in :david
|
||||
|
||||
assert_difference -> { ActiveStorage::Blob.count }, 1 do
|
||||
post rails_direct_uploads_url, params: { blob: blob_params }
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
test "disk show stays reachable without authentication" do
|
||||
blob = ActiveStorage::Blob.create_and_upload! \
|
||||
io: StringIO.new("hello"), filename: "hello.txt", content_type: "text/plain"
|
||||
ActiveStorage::Current.url_options = { host: "once.campfire.test", protocol: "http" }
|
||||
|
||||
get blob.url
|
||||
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
private
|
||||
def blob_params
|
||||
content = "hello"
|
||||
{ filename: "hello.txt", byte_size: content.bytesize, content_type: "text/plain", \
|
||||
checksum: Digest::MD5.base64digest(content) }
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user