Remove the need to use health check token

in favor of whitelist that will be used to
control the access to monitoring resources

app/controllers/concerns/requires_health_token.rb → app/controllers/concerns/requires_whitelisted_monitoring_client.rb

-module RequiresHealthToken
+module RequiresWhitelistedMonitoringClient
   extend ActiveSupport::Concern
   included do
-    before_action :validate_health_check_access!
+    before_action :validate_ip_whitelisted!
   end
 
   private
 
-  def validate_health_check_access!
-    render_404 unless token_valid?
+  def validate_ip_whitelisted!
+    render_404 unless client_ip_whitelisted?
   end
 
-  def token_valid?
-    token = params[:token].presence || request.headers['TOKEN']
-    token.present? &&
-      ActiveSupport::SecurityUtils.variable_size_secure_compare(
-        token,
-        current_application_settings.health_check_access_token
-      )
+  def client_ip_whitelisted?
+    Settings.monitoring.ip_whitelist.any? {|e| e.include?(Gitlab::RequestContext.client_ip) }
   end
 
   def render_404

app/controllers/health_check_controller.rb

 class HealthCheckController < HealthCheck::HealthCheckController
-  include RequiresHealthToken
+  include RequiresWhitelistedMonitoringClient
 end

app/controllers/health_controller.rb

 class HealthController < ActionController::Base
   protect_from_forgery with: :exception
-  include RequiresHealthToken
+  include RequiresWhitelistedMonitoringClient
 
   CHECKS = [
     Gitlab::HealthChecks::DbCheck,

app/controllers/metrics_controller.rb

 class MetricsController < ActionController::Base
-  include RequiresHealthToken
+  include RequiresWhitelistedMonitoringClient
 
   protect_from_forgery with: :exception
 
   before_action :validate_prometheus_metrics
 
   def index
-    render text: metrics_service.metrics_text, content_type: 'text/plain; verssion=0.0.4'
+    render text: metrics_service.metrics_text, content_type: 'text/plain; version=0.0.4'
   end
 
   private

config/gitlab.yml.example

@@ -548,6 +548,12 @@ production: &base
     # unicorn_sampler_interval: 10
 
 
+  ## Monitoring
+  # Built in monitoring settings
+  monitoring:
+    # IP whitelist to access monitoring endpoints
+    access_whitelist: 127.0.0.0/8
+
   #
   # 5. Extra customization
   # ==========================

config/initializers/1_settings.rb

@@ -494,6 +494,13 @@ Settings.webpack.dev_server['enabled'] ||= false
 Settings.webpack.dev_server['host']    ||= 'localhost'
 Settings.webpack.dev_server['port']    ||= 3808
 
+#
+# Monitoring settings
+#
+Settings['monitoring'] ||= Settingslogic.new({})
+Settings.monitoring['ip_whitelist'] ||= %w{127.0.0.1/8}
+Settings.monitoring.ip_whitelist.map!(&IPAddr.method(:new))
+
 #
 # Prometheus metrics settings
 #

spec/controllers/health_check_controller_spec.rb

@@ -3,52 +3,57 @@ require 'spec_helper'
 describe HealthCheckController do
   include StubENV
 
-  let(:token) { current_application_settings.health_check_access_token }
   let(:json_response) { JSON.parse(response.body) }
   let(:xml_response) { Hash.from_xml(response.body)['hash'] }
+  let(:whitelisted_ip) { '127.0.0.1' }
+  let(:not_whitelisted_ip) { '127.0.0.2' }
 
   before do
+    allow(Settings.monitoring).to receive(:ip_whitelist).and_return([IPAddr.new(whitelisted_ip)])
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
   end
 
   describe 'GET #index' do
-    context 'when services are up but NO access token' do
+    context 'when services are up but accessed from outside whitelisted ips' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
       it 'returns a not found page' do
         get :index
         expect(response).to be_not_found
       end
     end
 
-    context 'when services are up and an access token is provided' do
-      it 'supports passing the token in the header' do
-        request.headers['TOKEN'] = token
-        get :index
-        expect(response).to be_success
-        expect(response.content_type).to eq 'text/plain'
+    context 'when services are up and accessed from whitelisted ips' do
+      let(:ip) { '127.0.0.1' }
+
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
       it 'supports successful plaintest response' do
-        get :index, token: token
+        get :index
         expect(response).to be_success
         expect(response.content_type).to eq 'text/plain'
       end
 
       it 'supports successful json response' do
-        get :index, token: token, format: :json
+        get :index, format: :json
         expect(response).to be_success
         expect(response.content_type).to eq 'application/json'
         expect(json_response['healthy']).to be true
       end
 
       it 'supports successful xml response' do
-        get :index, token: token, format: :xml
+        get :index, format: :xml
         expect(response).to be_success
         expect(response.content_type).to eq 'application/xml'
         expect(xml_response['healthy']).to be true
       end
 
       it 'supports successful responses for specific checks' do
-        get :index, token: token, checks: 'email', format: :json
+        get :index, checks: 'email', format: :json
         expect(response).to be_success
         expect(response.content_type).to eq 'application/json'
         expect(json_response['healthy']).to be true
@@ -62,29 +67,22 @@ describe HealthCheckController do
       end
     end
 
-    context 'when a service is down and an access token is provided' do
+    context 'when a service is down and an endpoint is accessed from whitelisted ip' do
       before do
         allow(HealthCheck::Utils).to receive(:process_checks).with(['standard']).and_return('The server is on fire')
         allow(HealthCheck::Utils).to receive(:process_checks).with(['email']).and_return('Email is on fire')
-      end
-
-      it 'supports passing the token in the header' do
-        request.headers['TOKEN'] = token
-        get :index
-        expect(response).to have_http_status(500)
-        expect(response.content_type).to eq 'text/plain'
-        expect(response.body).to include('The server is on fire')
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
       it 'supports failure plaintest response' do
-        get :index, token: token
+        get :index
         expect(response).to have_http_status(500)
         expect(response.content_type).to eq 'text/plain'
         expect(response.body).to include('The server is on fire')
       end
 
       it 'supports failure json response' do
-        get :index, token: token, format: :json
+        get :index, format: :json
         expect(response).to have_http_status(500)
         expect(response.content_type).to eq 'application/json'
         expect(json_response['healthy']).to be false
@@ -92,7 +90,7 @@ describe HealthCheckController do
       end
 
       it 'supports failure xml response' do
-        get :index, token: token, format: :xml
+        get :index, format: :xml
         expect(response).to have_http_status(500)
         expect(response.content_type).to eq 'application/xml'
         expect(xml_response['healthy']).to be false
@@ -100,7 +98,7 @@ describe HealthCheckController do
       end
 
       it 'supports failure responses for specific checks' do
-        get :index, token: token, checks: 'email', format: :json
+        get :index, checks: 'email', format: :json
         expect(response).to have_http_status(500)
         expect(response.content_type).to eq 'application/json'
         expect(json_response['healthy']).to be false

spec/controllers/health_controller_spec.rb

@@ -3,17 +3,19 @@ require 'spec_helper'
 describe HealthController do
   include StubENV
 
-  let(:token) { current_application_settings.health_check_access_token }
   let(:json_response) { JSON.parse(response.body) }
+  let(:whitelisted_ip) { '127.0.0.1' }
+  let(:not_whitelisted_ip) { '127.0.0.2' }
 
   before do
+    allow(Settings.monitoring).to receive(:ip_whitelist).and_return([IPAddr.new(whitelisted_ip)])
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
   end
 
   describe '#readiness' do
-    context 'authorization token provided' do
+    context 'accessed from whitelisted ip' do
       before do
-        request.headers['TOKEN'] = token
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
       it 'returns proper response' do
@@ -25,7 +27,11 @@ describe HealthController do
       end
     end
 
-    context 'without authorization token' do
+    context 'accessed from not whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
       it 'returns proper response' do
         get :readiness
         expect(response.status).to eq(404)
@@ -34,9 +40,9 @@ describe HealthController do
   end
 
   describe '#liveness' do
-    context 'authorization token provided' do
+    context 'accessed from whitelisted ip' do
       before do
-        request.headers['TOKEN'] = token
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
       it 'returns proper response' do
@@ -47,7 +53,11 @@ describe HealthController do
       end
     end
 
-    context 'without authorization token' do
+    context 'accessed from not whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
       it 'returns proper response' do
         get :liveness
         expect(response.status).to eq(404)

spec/controllers/metrics_controller_spec.rb

@@ -3,20 +3,22 @@ require 'spec_helper'
 describe MetricsController do
   include StubENV
 
-  let(:token) { current_application_settings.health_check_access_token }
   let(:json_response) { JSON.parse(response.body) }
   let(:metrics_multiproc_dir) { Dir.mktmpdir }
+  let(:whitelisted_ip) { '127.0.0.1' }
+  let(:not_whitelisted_ip) { '127.0.0.2' }
 
   before do
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
     stub_env('prometheus_multiproc_dir', metrics_multiproc_dir)
     allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true)
+    allow(Settings.monitoring).to receive(:ip_whitelist).and_return([IPAddr.new(whitelisted_ip)])
   end
 
   describe '#index' do
-    context 'authorization token provided' do
+    context 'accessed from whitelisted ip' do
       before do
-        request.headers['TOKEN'] = token
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
       it 'returns DB ping metrics' do
@@ -59,7 +61,11 @@ describe MetricsController do
       end
     end
 
-    context 'without authorization token' do
+    context 'accessed from not whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
       it 'returns proper response' do
         get :index