Merge pull request #2760 from basecamp/cache-control-bypass

Don't set public Cache-Control on proxied non-public blobs

lib/rails_ext/active_storage_authorization.rb

@@ -40,6 +40,10 @@ Rails.application.config.to_prepare do
           head :forbidden
         end
       end
+
+      def http_cache_forever(public: false, &block)
+        super(public: public && publicly_accessible_blob?, &block)
+      end
   end
 
   ActiveStorage::Blobs::RedirectController.include ActiveStorage::Authorize

test/integration/active_storage_authorization_test.rb

@@ -150,6 +150,38 @@ class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest
     assert_match %r{rails/active_storage}, response.location
   end
 
+  test "proxy for non-public blob does not set public Cache-Control" do
+    sign_in_as :david
+
+    get rails_storage_proxy_path(@blob)
+    assert_response :success
+    assert_not_includes response.headers["Cache-Control"], "public"
+  end
+
+  test "proxy for publicly accessible blob sets public Cache-Control" do
+    blob = attach_avatar_to(users(:david))
+
+    get rails_storage_proxy_path(blob)
+    assert_response :success
+    assert_includes response.headers["Cache-Control"], "public"
+  end
+
+  test "representation proxy for non-public blob does not set public Cache-Control" do
+    sign_in_as :david
+
+    get rails_storage_proxy_path(@blob.representation(resize_to_limit: [ 100, 100 ]))
+    assert_response :success
+    assert_not_includes response.headers["Cache-Control"], "public"
+  end
+
+  test "representation proxy for publicly accessible blob sets public Cache-Control" do
+    blob = attach_avatar_to(users(:david))
+
+    get rails_storage_proxy_path(blob.representation(resize_to_fill: [ 256, 256 ]))
+    assert_response :success
+    assert_includes response.headers["Cache-Control"], "public"
+  end
+
   # Account exports
 
   test "export owner can download their export" do