Permalink
Browse files

Improve logging when Origin header doesn't match

I came up against this while dealing with a misconfigured server. The
browser was setting the Origin header to "https://example.com", but the
Rails app returned "http://example.com" from request.base_url (because
it was failing to detect that HTTPS was used).

This caused verify_authenticity_token to fail, but the message in the
log was "Can't verify CSRF token", which is confusing because the
failure had nothing to do with the CSRF token sent in the request. This
made it very hard to identify the issue, so hopefully this will make it
more obvious for the next person.
  • Loading branch information...
jonleighton committed Apr 6, 2017
1 parent fd097cf commit a500b4796f86b05b3fece414f090a496d3cb4298
@@ -213,7 +213,11 @@ def verify_authenticity_token # :doc:
if !verified_request?
if logger && log_warning_on_csrf_failure
logger.warn "Can't verify CSRF token authenticity."
if valid_request_origin?
logger.warn "Can't verify CSRF token authenticity."
else
logger.warn "HTTP Origin header (#{request.origin}) didn't match request.base_url (#{request.base_url})"
end
end
handle_unverified_request
end
@@ -347,6 +347,10 @@ def test_should_allow_post_with_origin_checking_and_no_origin
end
def test_should_block_post_with_origin_checking_and_wrong_origin
old_logger = ActionController::Base.logger
logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new
ActionController::Base.logger = logger
forgery_protection_origin_check do
session[:_csrf_token] = @token
@controller.stub :form_authenticity_token, @token do
@@ -356,6 +360,13 @@ def test_should_block_post_with_origin_checking_and_wrong_origin
end
end
end
assert_match(
"HTTP Origin header (http://bad.host) didn't match request.base_url (http://test.host)",
logger.logged(:warn).last
)
ensure
ActionController::Base.logger = old_logger
end
def test_should_warn_on_missing_csrf_token

3 comments on commit a500b47

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Apr 6, 2017

Member

❤️ welcome back! You just fixed #25843 with this commit

Member

rafaelfranca replied Apr 6, 2017

❤️ welcome back! You just fixed #25843 with this commit

@deivid-rodriguez

This comment has been minimized.

Show comment
Hide comment
@deivid-rodriguez

deivid-rodriguez Apr 7, 2017

Contributor

This will be a fantastic aid for troubleshooting this kind of issues. Will this make it to 5.1.0 final?

Contributor

deivid-rodriguez replied Apr 7, 2017

This will be a fantastic aid for troubleshooting this kind of issues. Will this make it to 5.1.0 final?

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Apr 7, 2017

Member

Now it is 470c189

Member

rafaelfranca replied Apr 7, 2017

Now it is 470c189

Please sign in to comment.