Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable session in ActiveStorage blobs and representations proxy controllers #48869

Merged
merged 2 commits into from Aug 3, 2023
Merged

Disable session in ActiveStorage blobs and representations proxy controllers #48869

merged 2 commits into from Aug 3, 2023

Conversation

brunoprietog
Copy link
Contributor

Motivation / Background

When configuring ActiveStorage with resolve_model_to_route in rails_storage_proxy, you would expect a CDN such as CloudFlare to cache the files. However, in the case of CloudFlare, by default, anything that has session cookies will not cache them. To work around this you can use custom rules or patch these ActiveStorage proxy controllers.

Detail

Session now is disabled in ActiveStorage::Blobs::ProxyController and ActiveStorage::Representations::ProxyController, including the new concern ActiveStorage::DisableSession in both controllers that does the work.

This way it will not be necessary to create custom rules and works by default.

Thanks to @fleck for the idea in this comment.

Fixes #44136

Additional information

I'm not sure if or how to test this, because there is no way to access the session in ActionDispatch::IntegrationTest.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

…roller

This allows CDNs such as CloudFlare to cache files delivered by proxy by default

Fix #44136
@brenogazzola
Copy link
Contributor

Beyond CDN compatibility, this will also prevent a footgun for developers who are starting to handle on their own infra and want to use something like nginx object cache, where they cache the file but forget to remove the Set-Cookie header from the response doing so.

This is a problem because rails uses said header to store its session, which might contain sensitive data. Worst case scenario, it might contain the ID of the logged user, which means that when the file is served, the user will be automatically switched to a different account.

@rafaelfranca rafaelfranca merged commit 83e5988 into rails:main Aug 3, 2023
9 checks passed
@brunoprietog brunoprietog deleted the disable-session-active-storage-proxy-controllers branch August 4, 2023 07:51
tenderlove pushed a commit that referenced this pull request Feb 21, 2024
…orage-proxy-controllers

Disable session in ActiveStorage blobs and representations proxy controllers

[CVE-2024-26144]
tenderlove pushed a commit that referenced this pull request Feb 21, 2024
…orage-proxy-controllers

Disable session in ActiveStorage blobs and representations proxy controllers

[CVE-2024-26144]
tenderlove added a commit that referenced this pull request Feb 21, 2024
* 6-1-sec:
  Preparing for 6.1.7.7 release
  update changelog
  Merge pull request #48869 from brunoprietog/disable-session-active-storage-proxy-controllers
tenderlove added a commit that referenced this pull request Feb 21, 2024
* 7-0-sec:
  Preparing for 7.0.8.1 release
  update changelog
  fix XSS vulnerability when using translation
  Merge pull request #48869 from brunoprietog/disable-session-active-storage-proxy-controllers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delivering assets through CloudFlare requires monkey-patch
3 participants