Support session token authentication for direct uploads

[rbarbosa]

Context

I'm building a native iOS client that uses magic link (session token) authentication. Image and file uploads are core functionality, but since the direct upload endpoint only bypasses CSRF for bearer token auth, session-based clients get 422 errors. This has been a real blocker for me.

Summary

• The Active Storage direct upload endpoint only skips CSRF protection for bearer token auth, causing 422 errors for native mobile apps using magic link (session-based) authentication
• Broadens the skip_forgery_protection condition to also bypass CSRF for authenticated JSON requests
• Adds test coverage for session-token direct uploads and cross-account authorization

Security considerations

This is safe because:

• Still requires valid authentication (session or bearer token)
• Only applies to JSON requests (web UI remains CSRF-protected)
• CSRF attacks target browser ambient authority — mobile JSON API requests aren't vulnerable

Test plan

• Existing bearer token tests pass
• New test: session token + JSON → 201 success
• New test: session token + wrong account → 403 forbidden

[flavorjones]

The security check can be ignored -- will go away if this branch is rebased onto main.

[rbarbosa]

Thanks @flavorjones! Let me know if anything else is needed.

[Copilot]

Pull request overview

This PR updates ActiveStorage direct upload CSRF behavior so native/mobile clients using session-token (magic link) authentication can successfully create direct uploads via JSON, and adds integration tests to cover this flow and cross-account authorization.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

• Broaden skip_forgery_protection for ActiveStorage::DirectUploadsController to include authenticated JSON requests (in addition to bearer token auth).
• Add direct-upload tests for session-token JSON requests and cross-account access.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
config/initializers/active_storage.rb	Expands the CSRF-skip condition for direct uploads to support session-authenticated JSON clients.
test/controllers/active_storage/direct_uploads_controller_test.rb	Adds integration tests for session-token direct upload success and forbidden cross-account behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[robzolkos]

I may be missing something, but I think this is very close and mostly needs a CSRF-enabled test to show the session-auth path is really covered.

The bit I’m unsure about is that this skip_forgery_protection condition depends on authenticated?, and I think that only gets established during require_authentication. My read is that the Active Storage forgery callback runs before that, so I’m not yet convinced a session-backed JSON direct upload will skip CSRF in practice.

Could you please add a test variant with forgery protection explicitly enabled? There’s a good example of the setup/teardown pattern in test/controllers/concerns/request_forgery_protection_test.rb, and the existing sign_in_as helper lives in test/test_helpers/session_test_helper.rb. If that passes for the signed-in session upload flow, this would increase confidence that this works as intended.

[rbarbosa]

I may be missing something, but I think this is very close and mostly needs a CSRF-enabled test to show the session-auth path is really covered.

The bit I’m unsure about is that this skip_forgery_protection condition depends on authenticated?, and I think that only gets established during require_authentication. My read is that the Active Storage forgery callback runs before that, so I’m not yet convinced a session-backed JSON direct upload will skip CSRF in practice.

Could you please add a test variant with forgery protection explicitly enabled? There’s a good example of the setup/teardown pattern in test/controllers/concerns/request_forgery_protection_test.rb, and the existing sign_in_as helper lives in test/test_helpers/session_test_helper.rb. If that passes for the signed-in session upload flow, this would increase confidence that this works as intended.

Hey @robzolkos, thank you for taking the time to review and pointing me in the right direction. Let me know if there are any other cases or concerns not yet covered with the update I've pushed.

[robzolkos]

@rbarbosa looks good! Thank you. cc @flavorjones

[rbarbosa]

Hey @flavorjones, just checking in — any concerns on your end? Happy to address anything if needed.

[flavorjones]

Rebased onto master to try to get CI green.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[flavorjones]

@rbarbosa @robzolkos I added a couple of commits to:

1. add a negative auth test on the active storage controller
2. use the existing RequestForgeryProtection concern instead of skip_forgery_protection

I'm mostly interested in whether you agree (2) is an improvement. It switches the controller to use the :header_only (Sec-Fetch-Site) CSRF strategy already used by ApplicationController.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[rbarbosa]

Thank you both for taking the time to review and improve this 🙏 .
I agree — using the existing RequestForgeryProtection concern is a nice improvement: the end result is cleaner and more consistent with the rest of the app.