Skip to content

Commit 9af8b50

Browse files
rbarbosaclaude
authored andcommitted
Address review: use resume_session and add forgery protection test
Use resume_session instead of authenticated? in the skip_forgery_protection condition so the session is established before the forgery check runs, regardless of callback ordering. Add a test with forgery protection explicitly enabled to prove the CSRF bypass works in production, not just in test mode where CSRF is disabled by default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0708f4c commit 9af8b50

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

config/initializers/active_storage.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ module ActiveStorageDirectUploadsControllerExtensions
5252
included do
5353
include Authentication
5454
include Authorization
55-
skip_forgery_protection if: -> { authenticate_by_bearer_token || (authenticated? && request.format.json?) }
55+
skip_forgery_protection if: -> { authenticate_by_bearer_token || (resume_session && request.format.json?) }
5656
end
5757
end
5858

test/controllers/active_storage/direct_uploads_controller_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe
4545
assert_includes response.parsed_body.keys, "direct_upload"
4646
end
4747

48+
test "create with session token skips forgery protection" do
49+
sign_in_as :david
50+
51+
with_forgery_protection do
52+
post rails_direct_uploads_path,
53+
params: @blob_params,
54+
as: :json
55+
56+
assert_response :success
57+
assert_includes response.parsed_body.keys, "direct_upload"
58+
end
59+
end
60+
4861
test "create with session token in another account is forbidden" do
4962
sign_in_as :david
5063

@@ -104,4 +117,12 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe
104117
def bearer_token_header(token)
105118
{ "Authorization" => "Bearer #{token}" }
106119
end
120+
121+
def with_forgery_protection
122+
original = ActionController::Base.allow_forgery_protection
123+
ActionController::Base.allow_forgery_protection = true
124+
yield
125+
ensure
126+
ActionController::Base.allow_forgery_protection = original
127+
end
107128
end

0 commit comments

Comments
 (0)