mirror of
https://github.com/basecamp/once-campfire.git
synced 2026-09-01 19:31:32 +09:00
Require authentication for Active Storage direct uploads (#267)
Active Storage mounts its direct-upload write endpoints -- POST /rails/active_storage/direct_uploads and the disk-service PUT at /rails/active_storage/disk/:token -- on framework controllers that inherit from ActiveStorage::BaseController, so they never pass through ApplicationController's require_authentication. Anyone who can read the public login page can lift a CSRF token and Rails session cookie, POST to the metadata endpoint, and receive a signed disk PUT URL without holding a Campfire session_token. That is enough to allocate ActiveStorage::Blob rows and persist bytes to disk anonymously. The blobs stay unattached (no message can be created without an account) and nothing purges them, so an unauthenticated caller can grow storage without bound. Because the recommended self-host layout co-locates uploaded files and the SQLite database on one /rails/storage volume, that growth eventually makes database writes fail -- blocking login and messaging until an administrator frees space and purges the blobs. Campfire uploads attachments through MessagesController as a normal multipart POST and does not use direct uploads at all, so these endpoints have no legitimate anonymous caller. Require a valid Campfire session before the metadata endpoint allocates a blob or the disk endpoint accepts an upload; both return 401 to anonymous callers. Serving (disk#show, representations, blob redirects) is unchanged.
This commit is contained in:
@@ -0,0 +1,9 @@
|
||||
module ActiveStorageAuthentication
|
||||
extend ActiveSupport::Concern
|
||||
include Authentication::SessionLookup
|
||||
|
||||
private
|
||||
def require_active_storage_authentication
|
||||
head :unauthorized unless find_session_by_cookie
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,15 @@
|
||||
# Active Storage mounts its direct-upload write endpoints
|
||||
# (POST /rails/active_storage/direct_uploads and the disk-service PUT) on
|
||||
# framework controllers that inherit from ActiveStorage::BaseController, so
|
||||
# they never pass through ApplicationController's require_authentication.
|
||||
# Campfire uploads attachments through MessagesController instead and does not
|
||||
# use direct uploads at all, leaving these endpoints reachable by anyone who
|
||||
# can read the public login page. Require a valid Campfire session before an
|
||||
# anonymous caller can allocate a Blob or persist bytes to disk.
|
||||
Rails.application.config.to_prepare do
|
||||
ActiveStorage::DirectUploadsController.include ActiveStorageAuthentication
|
||||
ActiveStorage::DirectUploadsController.before_action :require_active_storage_authentication
|
||||
|
||||
ActiveStorage::DiskController.include ActiveStorageAuthentication
|
||||
ActiveStorage::DiskController.before_action :require_active_storage_authentication, only: :update
|
||||
end
|
||||
@@ -0,0 +1,73 @@
|
||||
require "test_helper"
|
||||
|
||||
class ActiveStorageAuthenticationTest < ActionDispatch::IntegrationTest
|
||||
setup do
|
||||
host! "once.campfire.test"
|
||||
end
|
||||
|
||||
test "direct upload metadata endpoint rejects anonymous callers" do
|
||||
get new_session_url
|
||||
assert_response :success
|
||||
|
||||
assert_no_difference -> { ActiveStorage::Blob.count } do
|
||||
post rails_direct_uploads_url, params: blob_params, as: :json
|
||||
end
|
||||
|
||||
assert_response :unauthorized
|
||||
end
|
||||
|
||||
test "direct upload metadata endpoint allows authenticated users" do
|
||||
sign_in :david
|
||||
|
||||
assert_difference -> { ActiveStorage::Blob.count }, 1 do
|
||||
post rails_direct_uploads_url, params: blob_params, as: :json
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
test "disk service upload endpoint rejects anonymous callers" do
|
||||
sign_in :david
|
||||
post rails_direct_uploads_url, params: blob_params, as: :json
|
||||
assert_response :success
|
||||
upload_path = URI.parse(response.parsed_body.dig("direct_upload", "url")).request_uri
|
||||
|
||||
anonymous = open_session
|
||||
anonymous.host! "once.campfire.test"
|
||||
anonymous.put upload_path,
|
||||
params: attachment_bytes,
|
||||
headers: { "Content-Type" => "application/octet-stream" }
|
||||
|
||||
assert_equal 401, anonymous.status
|
||||
end
|
||||
|
||||
test "disk service download endpoint stays public" do
|
||||
ActiveStorage::Current.url_options = { host: "once.campfire.test", protocol: "https" }
|
||||
blob = ActiveStorage::Blob.create_and_upload! \
|
||||
io: StringIO.new(attachment_bytes), filename: "hi.txt", content_type: "text/plain"
|
||||
download_path = URI.parse(blob.url).request_uri
|
||||
|
||||
anonymous = open_session
|
||||
anonymous.host! "once.campfire.test"
|
||||
anonymous.get download_path
|
||||
|
||||
assert_equal 200, anonymous.status
|
||||
assert_equal attachment_bytes, anonymous.response.body
|
||||
ensure
|
||||
blob&.purge
|
||||
end
|
||||
|
||||
private
|
||||
def attachment_bytes
|
||||
"hello!"
|
||||
end
|
||||
|
||||
def blob_params
|
||||
{ blob: {
|
||||
filename: "quota.bin",
|
||||
byte_size: attachment_bytes.bytesize,
|
||||
checksum: Digest::MD5.base64digest(attachment_bytes),
|
||||
content_type: "application/octet-stream"
|
||||
} }
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user