diff --git a/lib/generators/thermite/install/active_storage/USAGE b/lib/generators/thermite/install/active_storage/USAGE index 42362bb..0b5ef5d 100644 --- a/lib/generators/thermite/install/active_storage/USAGE +++ b/lib/generators/thermite/install/active_storage/USAGE @@ -4,6 +4,12 @@ Description: Writes a Katalyst-flavoured config/storage.yml (Disk locally, S3 elsewhere) and points config.active_storage.service at :s3 for every non dev/test environment. + Also secures direct uploads: the POST /rails/active_storage/direct_uploads endpoint + is routed through an authenticated controller, and the disk service routes are + disabled unless storage is served from local disk. Authentication is app-specific, so + the controller ships locked down (every upload rejected) and the generated request + spec leaves the authenticated examples pending for you to complete. + To use the S3 service in deployed environments, add `aws-sdk-s3` to your Gemfile. Example: @@ -13,3 +19,10 @@ Example: Create config/storage.yml Install the Active Storage migrations Set the Active Storage service to :s3 (non dev/test environments) + Add app/controllers/active_storage/authenticated_direct_uploads_controller.rb + Add config/routes/overrides.rb and draw it from config/routes.rb + Add spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb + + After running, enable uploads for your authenticated users: + Permit your principals in #current_user in the authenticated direct uploads controller + Complete the pending examples in the generated request spec diff --git a/lib/generators/thermite/install/active_storage/active_storage_generator.rb b/lib/generators/thermite/install/active_storage/active_storage_generator.rb index b6abe6c..14e8389 100644 --- a/lib/generators/thermite/install/active_storage/active_storage_generator.rb +++ b/lib/generators/thermite/install/active_storage/active_storage_generator.rb @@ -15,6 +15,16 @@ def copy_storage_config template "config/storage.yml" end + # Lock the direct upload endpoint behind authentication and disable the disk + # service routes when storage isn't served from local disk. The authentication + # stubs are left for the app to complete (see the generated controller and spec). + def secure_direct_uploads + copy_file "app/controllers/active_storage/authenticated_direct_uploads_controller.rb" + copy_file "config/routes/overrides.rb" + copy_file "spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb" + draw_overrides + end + # update tops up an existing install; install lays the base for a new one. def install_migrations rails_command active_storage_installed? ? "active_storage:update" : "active_storage:install" @@ -31,6 +41,17 @@ def configure_services private + # Wire config/routes/overrides.rb into the app's router with `draw :overrides`, + # inserted just inside the routes block. Idempotent and a no-op without routes.rb. + def draw_overrides + routes = Pathname(destination_root).join("config/routes.rb") + return unless routes.exist? + return if routes.read.include?("draw :overrides") + + inject_into_file "config/routes.rb", " draw :overrides\n", + after: /^\s*Rails\.application\.routes\.draw do\n/, verbose: false + end + def environments Pathname(destination_root).join("config/environments").glob("*.rb") end diff --git a/lib/generators/thermite/install/active_storage/templates/app/controllers/active_storage/authenticated_direct_uploads_controller.rb b/lib/generators/thermite/install/active_storage/templates/app/controllers/active_storage/authenticated_direct_uploads_controller.rb new file mode 100644 index 0000000..48c9050 --- /dev/null +++ b/lib/generators/thermite/install/active_storage/templates/app/controllers/active_storage/authenticated_direct_uploads_controller.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module ActiveStorage + # Active Storage's default direct upload endpoint (POST /rails/active_storage/direct_uploads) + # mints a signed URL that lets the caller PUT a file straight into the storage service. + # Left unauthenticated, it is an open door for anyone to fill the asset bucket. + # @see https://guides.rubyonrails.org/active_storage_overview.html#authenticated-controllers + # + # This controller restricts the endpoint to authenticated principals and rate limits + # requests using #current_user to identify a database-backed principal for authorisation and + # request throttling. If no principal is returned requests fail with :unauthorized. + # @see config/routes/overrides.rb + class AuthenticatedDirectUploadsController < ActiveStorage::DirectUploadsController + before_action :authenticate_user! + rate_limit to: 20, within: 10.seconds, by: -> { current_user.to_gid } + + private + + # Reject any request we can't attribute to a #current_user. + def authenticate_user! + head(:unauthorized) if current_user.nil? + end + + # The authenticated principal for the current request, or nil when the request is anonymous. + def current_user + nil + end + end +end diff --git a/lib/generators/thermite/install/active_storage/templates/config/routes/overrides.rb b/lib/generators/thermite/install/active_storage/templates/config/routes/overrides.rb new file mode 100644 index 0000000..829a085 --- /dev/null +++ b/lib/generators/thermite/install/active_storage/templates/config/routes/overrides.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# Drawn from config/routes.rb via `draw :overrides`. + +# Only authenticated users can post files. +# @see app/controllers/active_storage/authenticated_direct_uploads_controller.rb +post "/rails/active_storage/direct_uploads", to: "active_storage/authenticated_direct_uploads#create" + +# Disable the disk service routes unless files are actually served from local disk. +unless %i[local test].include?(Rails.application.config.active_storage.service) + no_route = ->(req) { raise ActionController::RoutingError, "No route matches #{req['REQUEST_PATH']}" } + + get "/rails/active_storage/disk/:encoded_key/*filename", to: no_route + put "/rails/active_storage/disk/:encoded_token", to: no_route +end diff --git a/lib/generators/thermite/install/active_storage/templates/spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb b/lib/generators/thermite/install/active_storage/templates/spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb new file mode 100644 index 0000000..32cc8b9 --- /dev/null +++ b/lib/generators/thermite/install/active_storage/templates/spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe ActiveStorage::AuthenticatedDirectUploadsController do + let(:action) { post(rails_direct_uploads_path, params:) } + let(:file) { StringIO.new("example") } + let(:params) do + { + blob: { + filename: "example.txt", + content_type: "text/plain", + byte_size: file.string.bytesize, + checksum: OpenSSL::Digest::MD5.new(file.string).base64digest, + }, + } + end + + it "rejects uploads from anonymous users" do + action + + expect(response).to have_http_status(:unauthorized) + end + + context "as a signed-in admin", pending: "requires admin session setup" do + # include_context "with admin session" + + it "permits the upload" do + action + + expect(response).to have_http_status(:ok) + end + end + + context "as a signed-in user", pending: "requires user session setup" do + # before { sign_in(create(:user)) } + + it "permits the upload" do + action + + expect(response).to have_http_status(:ok) + end + end + + describe "rate limiting", pending: "requires a user session" do + # rate_limit captures the cache in a lambda at class-load time, so stub the call not the object. + let(:cache) { ActiveSupport::Cache::MemoryStore.new } + let(:rate_limit) { 20 } + + before do + allow(Rails.cache).to receive(:increment) do |name, amount = 1, **options| + cache.increment(name, amount, **options) + end + end + + # include_context "with admin session" + + it "throttles bursts of uploads with 429 Too Many Requests" do + aggregate_failures do + rate_limit.times do + post(rails_direct_uploads_path, params:, headers:) + expect(response).to have_http_status(:ok) + end + + post(rails_direct_uploads_path, params:, headers:) + expect(response).to have_http_status(:too_many_requests) + end + end + end +end diff --git a/spec/fixtures/app/config/routes.rb b/spec/fixtures/app/config/routes.rb new file mode 100644 index 0000000..ce1b4c4 --- /dev/null +++ b/spec/fixtures/app/config/routes.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +# Minimal stand-in for a real app's router so generators can inject routes. +Rails.application.routes.draw do + # Intentionally empty. +end diff --git a/spec/generators/thermite/install/active_storage_generator_spec.rb b/spec/generators/thermite/install/active_storage_generator_spec.rb index 0d94aac..16ba0a5 100644 --- a/spec/generators/thermite/install/active_storage_generator_spec.rb +++ b/spec/generators/thermite/install/active_storage_generator_spec.rb @@ -24,6 +24,29 @@ assert_file "config/environments/staging.rb", /config\.active_storage\.service = :s3/ end + it "installs the authenticated direct uploads controller, routes override, and spec" do + generator.invoke_all + + assert_file "app/controllers/active_storage/authenticated_direct_uploads_controller.rb", + /class AuthenticatedDirectUploadsController < ActiveStorage::DirectUploadsController/ + assert_file "config/routes/overrides.rb", %r{active_storage/authenticated_direct_uploads#create} + assert_file "spec/requests/active_storage/authenticated_direct_uploads_controller_spec.rb" + end + + it "draws the routes override from config/routes.rb" do + generator.invoke_all + + assert_file "config/routes.rb", /Rails\.application\.routes\.draw do\n draw :overrides\n/ + end + + it "doesn't draw the routes override twice when run again" do + generator.invoke_all + generator.invoke_all + + routes = File.read(File.join(destination_root, "config/routes.rb")) + expect(routes.scan("draw :overrides").size).to eq(1) + end + it "installs the base migration when Active Storage isn't installed yet" do generator.install_migrations