-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Replace origin_whitelist with permitted_origins #1625
Replace origin_whitelist with permitted_origins #1625
Conversation
@jkowens done :) |
Awesome, thanks! Hopefully we can fix the build in the near term, just to confirm this stays green. I don't think it should be too hard to remove Thin, but of course those are famous last words 😅 |
@jkowens rebased on top of master, tests pass! :D |
|
||
if options.key? :origin_whitelist | ||
warn "Rack::Protection origin_whitelist option is deprecated and will be removed, " \ | ||
"use origin_whitelist instead.\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhymes just realized this warning message needs tweaked 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahahaha great catch!
@namusyaka can we go ahead and merge this in for the 2.1 release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit
expect(send(method.downcase, '/', {}, 'HTTP_ORIGIN' => 'http://malicious.com')).not_to be_ok | ||
end | ||
|
||
it "accepts #{method} requests with whitelisted Origin" do | ||
mock_app do | ||
use Rack::Protection::HttpOrigin, :origin_whitelist => ['http://www.friend.com'] | ||
use Rack::Protection::HttpOrigin, :permitted_origins => ['http://www.friend.com'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perimitted_origins: ['http://www.friend.com']
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done :)
Thank you @namusyaka! |
permitted_origin
in lieu oforigin_whitelist
origin_whitelist
with a warningCloses #1620