diff --git a/.env.example b/.env.example index a3d38cf..22a086c 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,7 @@ R2_ENDPOINT=https://YOUR_ACCOUNT_ID.r2.cloudflarestorage.com # Public hostname for CDN URLs (used in generated links) CDN_HOST=cdn.hackclub.com -CDN_ASSETS_HOST=cdn.hackclub-assets.com +CDN_ASSETS_HOST=user-cdn.hackclub-assets.com # ============================================================================= # Hack Club OAuth # ============================================================================= @@ -47,6 +47,7 @@ ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT=your_key_derivation_salt # Cloudflare (cache purging on delete) # ============================================================================= CLOUDFLARE_ZONE_ID=your_zone_id +CLOUDFLARE_ASSETS_ZONE_ID=your_assets_zone_id CLOUDFLARE_API_TOKEN=your_api_token # ============================================================================= diff --git a/README.md b/README.md index 4228371..26c3654 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,9 @@ See `.env.example` for the full list. Key variables: | `R2_ENDPOINT` | R2 endpoint URL | | `CDN_HOST` | Public hostname for CDN URLs | | `CDN_ASSETS_HOST` | Public R2 bucket hostname | +| `CLOUDFLARE_ZONE_ID` | Zone containing `CDN_HOST`, for cache purging on delete | +| `CLOUDFLARE_ASSETS_ZONE_ID` | Zone containing `CDN_ASSETS_HOST` (a *different* zone) | +| `CLOUDFLARE_API_TOKEN` | Token with cache-purge permission on both zones | | `HACKCLUB_CLIENT_ID` | OAuth client ID from Hack Club Auth | | `HACKCLUB_CLIENT_SECRET` | OAuth client secret | | `LOCKBOX_MASTER_KEY` | 64-char hex key for encrypting API keys | @@ -89,7 +92,10 @@ See `.env.example` for the full list. Key variables: | Domain | Points to | |--------|-----------| | `cdn.hackclub.com` | Rails app (Heroku/Fly/etc.) | -| `cdn.hackclub-assets.com` | R2 bucket (custom domain in R2 settings) | +| `user-cdn.hackclub-assets.com` | R2 bucket (custom domain in R2 settings) | + +The two hostnames are in separate Cloudflare zones. Deleting an upload purges +the edge cache in both; see `app/jobs/purge_cloudflare_cache_job.rb`. ## API diff --git a/app/jobs/purge_cloudflare_cache_job.rb b/app/jobs/purge_cloudflare_cache_job.rb index 0bf4572..7adf59a 100644 --- a/app/jobs/purge_cloudflare_cache_job.rb +++ b/app/jobs/purge_cloudflare_cache_job.rb @@ -1,27 +1,83 @@ # frozen_string_literal: true class PurgeCloudflareCacheJob < ApplicationJob + class ConfigurationError < StandardError; end + + class PurgeFailedError < StandardError; end + queue_as :default + retry_on Faraday::Error, wait: :polynomially_longer, attempts: 5 + retry_on PurgeFailedError, wait: :polynomially_longer, attempts: 5 + def perform(urls) - zone_id = ENV["CLOUDFLARE_ZONE_ID"] - api_token = ENV["CLOUDFLARE_API_TOKEN"] + urls = Array(urls).map { |url| url.to_s.strip }.reject(&:blank?).uniq + return if urls.empty? - return unless zone_id.present? && api_token.present? + api_token = ENV["CLOUDFLARE_API_TOKEN"].presence + if api_token.nil? + return if Rails.env.local? - conn = Faraday.new(url: "https://api.cloudflare.com") do |f| - f.request :json - f.response :json - f.adapter :net_http + raise ConfigurationError, + "CLOUDFLARE_API_TOKEN is not set!! cache purging will fail!" + end + + zone_ids = zone_ids_by_host + + urls.group_by { |url| URI.parse(url).host }.each do |host, host_urls| + zone_id = zone_ids[host] + + if zone_id.blank? + raise ConfigurationError, + "No Cloudflare zone id configured for #{host}, so #{host_urls.size} " \ + "URL(s) cannot be purged (configured hosts: #{zone_ids.keys.join(', ')})" + end + + host_urls.each_slice(30) do |slice| + purge!(zone_id: zone_id, files: slice, api_token: api_token) + end + end + end + + private + + # Host => zone id. Both zones are required: purging only one leaves the other + # serving the deleted file. + def zone_ids_by_host + mapping = { + CDNHost.host => ENV["CLOUDFLARE_ZONE_ID"].presence, + CDNHost.assets_host => ENV["CLOUDFLARE_ASSETS_ZONE_ID"].presence + } + + configured = mapping.values.compact + if mapping.size > 1 && configured.uniq.size < configured.size + raise ConfigurationError, + "CLOUDFLARE_ZONE_ID and CLOUDFLARE_ASSETS_ZONE_ID are set to the same zone, " \ + "but #{mapping.keys.join(' and ')} are in different Cloudflare zones" end - response = conn.post("/client/v4/zones/#{zone_id}/purge_cache") do |req| + mapping + end + + def purge!(zone_id:, files:, api_token:) + response = connection.post("/client/v4/zones/#{zone_id}/purge_cache") do |req| req.headers["Authorization"] = "Bearer #{api_token}" - req.body = { files: Array(urls) } + req.body = { files: files } end - unless response.success? - Rails.logger.error("Cloudflare cache purge failed: #{response.body}") + body = response.body + return if response.success? && body.is_a?(Hash) && body["success"] + + raise PurgeFailedError, + "Cloudflare cache purge failed for zone #{zone_id} " \ + "(HTTP #{response.status}, #{files.size} URL(s)): #{body.inspect}" + end + + def connection + @connection ||= Faraday.new(url: "https://api.cloudflare.com") do |f| + f.request :json + f.response :json + f.adapter :net_http end end end diff --git a/app/models/cdn_host.rb b/app/models/cdn_host.rb index 2db86a3..c301504 100644 --- a/app/models/cdn_host.rb +++ b/app/models/cdn_host.rb @@ -6,13 +6,21 @@ # "https://https://cdn.hackclub.com". module CDNHost DEFAULT_HOST = "cdn.hackclub.com" + DEFAULT_ASSETS_HOST = "user-cdn.hackclub-assets.com" module_function def host - raw = ENV["CDN_HOST"].presence || DEFAULT_HOST - raw.sub(%r{\Ahttps?://}i, "").sub(%r{/+\z}, "") + normalize(ENV["CDN_HOST"].presence || DEFAULT_HOST) + end + + def assets_host + normalize(ENV["CDN_ASSETS_HOST"].presence || DEFAULT_ASSETS_HOST) end def base_url = "https://#{host}" + + def assets_base_url = "https://#{assets_host}" + + def normalize(raw) = raw.to_s.sub(%r{\Ahttps?://}i, "").sub(%r{/+\z}, "") end diff --git a/app/models/upload.rb b/app/models/upload.rb index a386800..1de57be 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -69,10 +69,12 @@ def human_file_size ActiveSupport::NumberHelper.number_to_human_size(byte_size) end - # Direct URL to public R2 bucket + # Direct URL to public R2 bucket. The key holds the raw filename, which can + # contain spaces or "#", so escape it the same way the route helper escapes + # cdn_url - otherwise "#" starts a fragment and both the redirect and the + # cache purge silently target the wrong URL. def assets_url - host = ENV.fetch("CDN_ASSETS_HOST", "cdn.hackclub-assets.com") - "https://#{host}/#{blob.key}" + "#{CDNHost.assets_base_url}/#{ActionDispatch::Journey::Router::Utils.escape_path(blob.key)}" end # Get CDN URL (uses external uploads controller) @@ -228,5 +230,7 @@ def purge_blob Rails.logger.info("Blob #{blob.key} already deleted from S3, skipping purge") end - def purge_cdn_cache = PurgeCloudflareCacheJob.perform_later(assets_url) + # Both hosts cache the file: the assets host caches the bytes and the CDN host + # caches the redirect that points at them. Purge both or the delete is partial. + def purge_cdn_cache = PurgeCloudflareCacheJob.perform_later([ assets_url, cdn_url ]) end diff --git a/test/jobs/purge_cloudflare_cache_job_test.rb b/test/jobs/purge_cloudflare_cache_job_test.rb new file mode 100644 index 0000000..50b991c --- /dev/null +++ b/test/jobs/purge_cloudflare_cache_job_test.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require "test_helper" + +class PurgeCloudflareCacheJobTest < ActiveSupport::TestCase + ENV_KEYS = %w[ + CLOUDFLARE_API_TOKEN CLOUDFLARE_ZONE_ID CLOUDFLARE_ASSETS_ZONE_ID + CDN_HOST CDN_ASSETS_HOST + ].freeze + + CDN_URL = "https://cdn.hackclub.com/an-id/file.png" + ASSETS_URL = "https://user-cdn.hackclub-assets.com/an-id/file.png" + + CONFIGURED = { + "CLOUDFLARE_API_TOKEN" => "token", + "CLOUDFLARE_ZONE_ID" => "cdn-zone", + "CLOUDFLARE_ASSETS_ZONE_ID" => "assets-zone" + }.freeze + + # Clears every var this job reads so a developer's .env can't change results. + def with_env(values) + previous = ENV_KEYS.index_with { |key| ENV[key] } + ENV_KEYS.each { |key| ENV.delete(key) } + values.each { |key, value| ENV[key.to_s] = value } + yield + ensure + ENV_KEYS.each { |key| previous[key].nil? ? ENV.delete(key) : ENV[key] = previous[key] } + end + + # The test environment is "local", where a missing token is allowed to no-op. + def with_rails_env(name) + previous = Rails.env + Rails.env = name + yield + ensure + Rails.env = previous + end + + # Returns [job, requests] where the job's Faraday connection is stubbed and + # every purge call is appended to requests. + def stubbed_job(status: 200, body: { "success" => true, "errors" => [] }) + requests = [] + + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.post(%r{\A/client/v4/zones/[^/]+/purge_cache\z}) do |env| + requests << { + zone: env.url.path[%r{/zones/([^/]+)/}, 1], + files: JSON.parse(env.body).fetch("files"), + authorization: env.request_headers["Authorization"] + } + [ status, { "Content-Type" => "application/json" }, body.to_json ] + end + end + + connection = Faraday.new do |f| + f.request :json + f.response :json + f.adapter :test, stubs + end + + job = PurgeCloudflareCacheJob.new + job.define_singleton_method(:connection) { connection } + [ job, requests ] + end + + test "purges each host in its own zone" do + with_env(CONFIGURED) do + job, requests = stubbed_job + job.perform([ ASSETS_URL, CDN_URL ]) + + assert_equal 2, requests.size + assert_equal [ ASSETS_URL ], requests.find { |r| r[:zone] == "assets-zone" }[:files] + assert_equal [ CDN_URL ], requests.find { |r| r[:zone] == "cdn-zone" }[:files] + assert_equal [ "Bearer token" ], requests.map { |r| r[:authorization] }.uniq + end + end + + test "respects CDN_HOST and CDN_ASSETS_HOST overrides" do + with_env(CONFIGURED.merge( + "CDN_HOST" => "cdn.example.com", + "CDN_ASSETS_HOST" => "assets.example.net" + )) do + job, requests = stubbed_job + job.perform([ "https://assets.example.net/key", "https://cdn.example.com/an-id/f.png" ]) + + assert_equal %w[assets-zone cdn-zone], requests.map { |r| r[:zone] }.sort + end + end + + test "does nothing when there are no urls" do + with_env(CONFIGURED) do + job, requests = stubbed_job + job.perform([]) + job.perform(nil) + + assert_empty requests + end + end + + # Matches the each_slice size in the job; Cloudflare's per-request limit is 30 + # URLs outside Enterprise. + BATCH_SIZE = 30 + + test "deduplicates urls and batches them per request" do + with_env(CONFIGURED) do + job, requests = stubbed_job + urls = Array.new(BATCH_SIZE + 5) do |i| + "https://user-cdn.hackclub-assets.com/id-#{i}/f.png" + end + job.perform(urls + [ urls.first ]) + + assert_equal 2, requests.size + assert_equal BATCH_SIZE, requests.first[:files].size + assert_equal 5, requests.second[:files].size + end + end + + test "raises when the assets zone is not configured" do + with_env(CONFIGURED.except("CLOUDFLARE_ASSETS_ZONE_ID")) do + job, = stubbed_job + + error = assert_raises(PurgeCloudflareCacheJob::ConfigurationError) do + job.perform([ ASSETS_URL ]) + end + assert_match "user-cdn.hackclub-assets.com", error.message + end + end + + test "raises when a url belongs to neither configured host" do + with_env(CONFIGURED) do + job, = stubbed_job + + assert_raises(PurgeCloudflareCacheJob::ConfigurationError) do + job.perform([ "https://somewhere-else.example.com/key" ]) + end + end + end + + test "raises when both zone ids are the same" do + with_env(CONFIGURED.merge("CLOUDFLARE_ASSETS_ZONE_ID" => "cdn-zone")) do + job, requests = stubbed_job + + error = assert_raises(PurgeCloudflareCacheJob::ConfigurationError) do + job.perform([ ASSETS_URL ]) + end + assert_match "same zone", error.message + assert_empty requests + end + end + + test "raises outside local environments when the api token is missing" do + with_env(CONFIGURED.except("CLOUDFLARE_API_TOKEN")) do + with_rails_env("production") do + job, requests = stubbed_job + + assert_raises(PurgeCloudflareCacheJob::ConfigurationError) do + job.perform([ ASSETS_URL ]) + end + assert_empty requests + end + end + end + + test "skips quietly in local environments when the api token is missing" do + with_env(CONFIGURED.except("CLOUDFLARE_API_TOKEN")) do + job, requests = stubbed_job + + assert_nil job.perform([ ASSETS_URL ]) + assert_empty requests + end + end + + # Cloudflare answers 200 with success:false for a rejected purge, which the + # previous implementation treated as a win. + test "raises when cloudflare reports failure in a 200 response" do + with_env(CONFIGURED) do + job, = stubbed_job(body: { + "success" => false, + "errors" => [ { "code" => 1012, "message" => "Unable to purge" } ] + }) + + error = assert_raises(PurgeCloudflareCacheJob::PurgeFailedError) do + job.perform([ ASSETS_URL ]) + end + assert_match "1012", error.message + end + end + + test "raises when cloudflare rejects the request" do + with_env(CONFIGURED) do + job, = stubbed_job(status: 403, body: { "success" => false }) + + error = assert_raises(PurgeCloudflareCacheJob::PurgeFailedError) do + job.perform([ ASSETS_URL ]) + end + assert_match "403", error.message + end + end +end