Allow localhost or 127.0.0.1 to access api/health_checks without health-check-token (#8231)

* This change allows access to api/health_checks if you are coming from localhost
or 127.0.0.1. Pretty sweet huh?

The reason for this change is because I want to easily get to these healthcheck
end points from localhost without having to get the health-check-token beforehand.

Buttttt.... this addition has some issues. You can spoof the host header pretty
easily:

https://daniel.haxx.se/blog/2018/04/05/curl-another-host/

I thought about doing `return if !request.ssl? && request.local?`

to check of the request was over https or not. Any localhost healthcheck we do
directly to puma will be over http. Checking for TLS seemed like a logical next
check, but since we force https on the edge and not necessarily from the edge to
Puma the Host header can still be spoofed to get at these end points.

More info on host header attacks and what Rails 6 is doing about it:

https://github.com/rails/rails/issues/29893

While I do think it is a good idea to protect these healthcheck end points from
the outside world, I do want to be able to get to them easily locally. WWYD?

* Fix Health Check spec by stubbing request

Co-authored-by: mstruve <mollylbs@gmail.com>
This commit is contained in:
Joe Doss 2020-06-02 10:29:04 -05:00 committed by GitHub
parent f2632a3f6b
commit 6406b0727b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 3 additions and 0 deletions

View file

@ -34,6 +34,8 @@ module Api
private
def authenticate_with_token
return if request.local?
key = request.headers["health-check-token"]
return if key == SiteConfig.health_check_token

View file

@ -8,6 +8,7 @@ RSpec.describe "HealthCheck", type: :request do
context "without a token" do
it "returns an unauthorized request" do
allow_any_instance_of(ActionDispatch::Request).to receive(:remote_addr).and_return("0.0.0.0") # rubocop:disable RSpec/AnyInstance
get app_api_health_checks_path
expect(response.status).to eq(401)
end