Merge pull request #2813 from robzolkos/fix/active-storage-bearer-downloads

Allow bearer auth for Active Storage download endpoints

app/controllers/concerns/authentication.rb

@@ -57,7 +57,7 @@ module Authentication
 
     def authenticate_by_bearer_token
       if request.authorization.to_s.include?("Bearer")
-        if request.format.json?
+        if bearer_token_authenticatable_request?
           authenticate_or_request_with_http_token do |token|
             if identity = Identity.find_by_permissable_access_token(token, method: request.method)
               Current.identity = identity
@@ -69,6 +69,10 @@ module Authentication
       end
     end
 
+    def bearer_token_authenticatable_request?
+      request.format.json?
+    end
+
     def request_authentication
       if Current.account.present?
         session[:return_to_after_authenticating] = request.url

lib/rails_ext/active_storage_authorization.rb

@@ -31,6 +31,10 @@ Rails.application.config.to_prepare do
     end
 
     private
+      def bearer_token_authenticatable_request?
+        true
+      end
+
       def publicly_accessible_blob?
         @blob.publicly_accessible?
       end

test/controllers/accounts/exports_controller_test.rb

@@ -103,6 +103,25 @@ class Account::ExportsControllerTest < ActionDispatch::IntegrationTest
     assert body["download_url"].present?
   end
 
+  test "show as JSON with bearer token returns a download URL that can be fetched" do
+    export = Account::Export.create!(account: Current.account, user: users(:jason))
+    export.build
+    sign_out
+    bearer_token = { "HTTP_AUTHORIZATION" => "Bearer #{identity_access_tokens(:jasons_api_token).token}" }
+
+    get account_export_path(export), as: :json, env: bearer_token
+    assert_response :success
+
+    body = @response.parsed_body
+    assert_equal export.id, body["id"]
+    assert_equal "completed", body["status"]
+    assert body["download_url"].present?
+
+    get URI(body["download_url"]).request_uri, env: bearer_token
+    assert_response :redirect
+    assert_match %r{rails/active_storage}, response.location
+  end
+
   test "show as JSON with pending export" do
     export = Account::Export.create!(account: Current.account, user: users(:jason))
 

test/integration/active_storage_authorization_test.rb

@@ -17,6 +17,15 @@ class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest
     assert_match %r{rails/active_storage}, response.location
   end
 
+  test "bearer token with board access can view blob" do
+    bearer_token = { "HTTP_AUTHORIZATION" => "Bearer #{identity_access_tokens(:davids_api_token).token}" }
+
+    get rails_blob_path(@blob, disposition: :inline), env: bearer_token
+
+    assert_response :redirect
+    assert_match %r{rails/active_storage}, response.location
+  end
+
   test "authenticated user without board access cannot view blob" do
     sign_in_as :mike
 
@@ -38,6 +47,15 @@ class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest
     assert_match %r{rails/active_storage/}, response.location
   end
 
+  test "bearer token with board access can view representation" do
+    bearer_token = { "HTTP_AUTHORIZATION" => "Bearer #{identity_access_tokens(:davids_api_token).token}" }
+
+    get rails_representation_path(@blob.representation(resize_to_limit: [ 100, 100 ])), env: bearer_token
+
+    assert_response :redirect
+    assert_match %r{rails/active_storage/}, response.location
+  end
+
   test "authenticated user without board access cannot view representation" do
     sign_in_as :mike
 
@@ -194,6 +212,16 @@ class ActiveStorageAuthorizationTest < ActionDispatch::IntegrationTest
     assert_match %r{rails/active_storage}, response.location
   end
 
+  test "export owner can download their export with bearer token" do
+    blob = create_export_blob_for(users(:david))
+    bearer_token = { "HTTP_AUTHORIZATION" => "Bearer #{identity_access_tokens(:davids_api_token).token}" }
+
+    get rails_blob_path(blob, disposition: :attachment), env: bearer_token
+
+    assert_response :redirect
+    assert_match %r{rails/active_storage}, response.location
+  end
+
   test "non-owner cannot download another user's export" do
     sign_in_as :jz