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

Jump to bottom

Vendor rack-protection/authenticity_token middleware #4588

Merged
merged 3 commits into from Jun 3, 2020
Merged

Vendor rack-protection/authenticity_token middleware #4588

merged 3 commits into from Jun 3, 2020

Conversation

seuros
Copy link
Collaborator

@seuros seuros commented Jun 3, 2020

Closes #4587

@seuros seuros requested a review from mperham June 3, 2020 00:44
@seuros seuros force-pushed the vendor-rackprotection branch 2 times, most recently from 17a0c57 to 9591b9c Compare June 3, 2020 00:51
mperham
mperham requested changes Jun 3, 2020
View reviewed changes
Gemfile.lock Outdated Show resolved Hide resolved
@seuros seuros force-pushed the vendor-rackprotection branch 2 times, most recently from 7d793a2 to d56903a Compare June 3, 2020 01:13
@mperham
Copy link
Collaborator

mperham commented Jun 3, 2020

Well done! I wonder if we could remove Rack and just have Sidekiq::Web quack like a Rack app... 🤔

@natematykiewicz
Copy link
Contributor

Well done! I wonder if we could remove Rack and just have Sidekiq::Web quack like a Rack app... 🤔

A safer option is probably to add a soft dependency on Rack. Like how Rails does with various database drivers.

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L3-L4

Basically, you still require a specific Rack version if they attempt to use mount the Web UI.

@mperham mperham merged commit 111d9d5 into master Jun 3, 2020
@mperham mperham deleted the vendor-rackprotection branch June 3, 2020 17:22
@luisgreg99 luisgreg99 mentioned this pull request Jul 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mperham mperham mperham requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove dependency on rack-protection?
3 participants
@seuros @mperham @natematykiewicz