Merge branch '33949-remove-healthcheck-access-token' into 'master'

Remove the need to use health check token by adding ability to whitelist hosts

Closes #33949

See merge request !12612

3 files changed, 155 insertions, 49 deletions

diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb
index 58c16cc57e6..03da6287774 100644
--- a/spec/controllers/health_check_controller_spec.rb
+++ b/spec/controllers/health_check_controller_spec.rb
@@ -3,52 +3,79 @@ 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(:token) { current_application_settings.health_check_access_token }
+  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([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
+
+      context 'when services are accessed with token' 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'
+        end
+
+        it 'supports passing the token in query params' do
+          get :index, token: token
+
+          expect(response).to be_success
+          expect(response.content_type).to eq 'text/plain'
+        end
+      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
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
-      it 'supports successful plaintest response' do
-        get :index, token: token
+      it 'supports successful plaintext response' do
+        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
@@ -58,33 +85,29 @@ describe HealthCheckController do
     context 'when a service is down but NO access token' do
       it 'returns a not found page' do
         get :index
+
         expect(response).to be_not_found
       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')
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
       end
 
-      it 'supports passing the token in the header' do
-        request.headers['TOKEN'] = token
+      it 'supports failure plaintext response' do
         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 plaintest response' do
-        get :index, token: token
         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 +115,8 @@ 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 +124,8 @@ 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
diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb
index 241c36620d1..cc389e554ad 100644
--- a/spec/controllers/health_controller_spec.rb
+++ b/spec/controllers/health_controller_spec.rb
@@ -3,21 +3,25 @@ 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(:token) { current_application_settings.health_check_access_token }
+  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([whitelisted_ip])
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
   end
 
   describe '#readiness' do
-    context 'authorization token provided' do
-      before do
-        request.headers['TOKEN'] = token
-      end
+    shared_context 'endpoint responding with readiness data' do
+      let(:request_params) { {} }
+
+      subject { get :readiness, request_params }
+
+      it 'responds with readiness checks data' do
+        subject
 
-      it 'returns proper response' do
-        get :readiness
         expect(json_response['db_check']['status']).to eq('ok')
         expect(json_response['cache_check']['status']).to eq('ok')
         expect(json_response['queues_check']['status']).to eq('ok')
@@ -27,22 +31,50 @@ describe HealthController do
       end
     end
 
-    context 'without authorization token' do
-      it 'returns proper response' do
+    context 'accessed from whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
+      end
+
+      it_behaves_like 'endpoint responding with readiness data'
+    end
+
+    context 'accessed from not whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
+      it 'responds with resource not found' do
         get :readiness
+
         expect(response.status).to eq(404)
       end
+
+      context 'accessed with valid token' do
+        context 'token passed in request header' do
+          before do
+            request.headers['TOKEN'] = token
+          end
+
+          it_behaves_like 'endpoint responding with readiness data'
+        end
+      end
+
+      context 'token passed as URL param' do
+        it_behaves_like 'endpoint responding with readiness data' do
+          let(:request_params) { { token: token } }
+        end
+      end
     end
   end
 
   describe '#liveness' do
-    context 'authorization token provided' do
-      before do
-        request.headers['TOKEN'] = token
-      end
+    shared_context 'endpoint responding with liveness data' do
+      subject { get :liveness }
+
+      it 'responds with liveness checks data' do
+        subject
 
-      it 'returns proper response' do
-        get :liveness
         expect(json_response['db_check']['status']).to eq('ok')
         expect(json_response['cache_check']['status']).to eq('ok')
         expect(json_response['queues_check']['status']).to eq('ok')
@@ -51,11 +83,40 @@ describe HealthController do
       end
     end
 
-    context 'without authorization token' do
-      it 'returns proper response' do
+    context 'accessed from whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
+      end
+
+      it_behaves_like 'endpoint responding with liveness data'
+    end
+
+    context 'accessed from not whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip)
+      end
+
+      it 'responds with resource not found' do
         get :liveness
+
         expect(response.status).to eq(404)
       end
+
+      context 'accessed with valid token' do
+        context 'token passed in request header' do
+          before do
+            request.headers['TOKEN'] = token
+          end
+
+          it_behaves_like 'endpoint responding with liveness data'
+        end
+
+        context 'token passed as URL param' do
+          it_behaves_like 'endpoint responding with liveness data' do
+            subject { get :liveness, token: token }
+          end
+        end
+      end
     end
   end
 end
diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb
index c46dd92b454..86847c07c09 100644
--- a/spec/controllers/metrics_controller_spec.rb
+++ b/spec/controllers/metrics_controller_spec.rb
@@ -3,22 +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(:whitelisted_ip_range) { '10.0.0.0/24' }
+  let(:ip_in_whitelisted_range) { '10.0.0.1' }
+  let(:not_whitelisted_ip) { '10.0.1.1' }
 
   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([whitelisted_ip, whitelisted_ip_range])
   end
 
   describe '#index' do
-    context 'authorization token provided' do
-      before do
-        request.headers['TOKEN'] = token
-      end
-
+    shared_examples_for 'endpoint providing metrics' do
       it 'returns DB ping metrics' do
         get :index
 
@@ -83,7 +83,27 @@ describe MetricsController do
       end
     end
 
-    context 'without authorization token' do
+    context 'accessed from whitelisted ip' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip)
+      end
+
+      it_behaves_like 'endpoint providing metrics'
+    end
+
+    context 'accessed from ip in whitelisted range' do
+      before do
+        allow(Gitlab::RequestContext).to receive(:client_ip).and_return(ip_in_whitelisted_range)
+      end
+
+      it_behaves_like 'endpoint providing metrics'
+    end
+
+    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