Merge branch '22465-rack-attack-authenticate-runner-requests' into 'master'

Authenticate runner requests in Rack::Attack

See merge request gitlab-org/gitlab!21311

changelogs/unreleased/22465-rack-attack-authenticate-runner-requests.yml

+---
+title: Authenticate runner requests in Rack::Attack
+merge_request: 21311
+author:
+type: fixed

config/initializers/rack_attack_new.rb

@@ -113,11 +113,15 @@ class Rack::Attack
 
   class Request
     def unauthenticated?
-      !authenticated_user_id([:api, :rss, :ics])
+      !(authenticated_user_id([:api, :rss, :ics]) || authenticated_runner_id)
     end
 
     def authenticated_user_id(request_formats)
-      Gitlab::Auth::RequestAuthenticator.new(self).user(request_formats)&.id
+      request_authenticator.user(request_formats)&.id
+    end
+
+    def authenticated_runner_id
+      request_authenticator.runner&.id
     end
 
     def api_request?
@@ -150,6 +154,10 @@ class Rack::Attack
 
     private
 
+    def request_authenticator
+      @request_authenticator ||= Gitlab::Auth::RequestAuthenticator.new(self)
+    end
+
     def protected_paths
       Gitlab::CurrentSettings.current_application_settings.protected_paths
     end

ee/lib/ee/gitlab/auth/user_auth_finders.rb → ee/lib/ee/gitlab/auth/auth_finders.rb

@@ -3,7 +3,7 @@
 module EE
   module Gitlab
     module Auth
-      module UserAuthFinders
+      module AuthFinders
         extend ActiveSupport::Concern
         extend ::Gitlab::Utils::Override
 

ee/spec/lib/gitlab/auth/user_auth_finders_spec.rb → ee/spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Auth::UserAuthFinders do
+describe Gitlab::Auth::AuthFinders do
   include described_class
 
   let(:user) { create(:user) }
@@ -98,7 +98,7 @@ describe Gitlab::Auth::UserAuthFinders do
       let(:token) { pat.token }
 
       before do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = pat.token
+        env[described_class::PRIVATE_TOKEN_HEADER] = pat.token
       end
 
       it { is_expected.to eq user }
@@ -124,7 +124,7 @@ describe Gitlab::Auth::UserAuthFinders do
 
     context 'when the job token is in the headers' do
       def set_token(token)
-        env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = token
+        env[described_class::JOB_TOKEN_HEADER] = token
       end
 
       it_behaves_like 'find user from job token'
@@ -133,7 +133,7 @@ describe Gitlab::Auth::UserAuthFinders do
 
     context 'when the job token is in the params' do
       def set_token(token)
-        set_param(Gitlab::Auth::UserAuthFinders::JOB_TOKEN_PARAM, token)
+        set_param(described_class::JOB_TOKEN_PARAM, token)
       end
 
       it_behaves_like 'find user from job token'

ee/spec/requests/api/helpers_spec.rb

@@ -41,13 +41,13 @@ describe API::Helpers do
         let(:route_authentication_setting) { { job_token_allowed: true } }
 
         it "returns a 401 response for an invalid token" do
-          env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = 'invalid token'
+          env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'invalid token'
 
           expect { current_user }.to raise_error /401/
         end
 
         it "returns a 403 response for a user without access" do
-          env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = job.token
+          env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token
           allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
 
           expect { current_user }.to raise_error /403/
@@ -55,13 +55,13 @@ describe API::Helpers do
 
         it 'returns a 403 response for a user who is blocked' do
           user.block!
-          env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = job.token
+          env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token
 
           expect { current_user }.to raise_error /403/
         end
 
         it "sets current_user" do
-          env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = job.token
+          env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token
 
           expect(current_user).to eq(user)
         end
@@ -71,7 +71,7 @@ describe API::Helpers do
         let(:route_authentication_setting) { { job_token_allowed: false } }
 
         it "sets current_user to nil" do
-          env[Gitlab::Auth::UserAuthFinders::JOB_TOKEN_HEADER] = job.token
+          env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = job.token
           allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true)
 
           expect(current_user).to be_nil

lib/api/api_guard.rb

@@ -44,7 +44,7 @@ module API
     # Helper Methods for Grape Endpoint
     module HelperMethods
       prepend_if_ee('EE::API::APIGuard::HelperMethods') # rubocop: disable Cop/InjectEnterpriseEditionModule
-      include Gitlab::Auth::UserAuthFinders
+      include Gitlab::Auth::AuthFinders
 
       def find_current_user!
         user = find_user_from_sources

lib/gitlab/auth/user_auth_finders.rb → lib/gitlab/auth/auth_finders.rb

@@ -17,8 +17,8 @@ module Gitlab
       end
     end
 
-    module UserAuthFinders
-      prepend_if_ee('::EE::Gitlab::Auth::UserAuthFinders') # rubocop: disable Cop/InjectEnterpriseEditionModule
+    module AuthFinders
+      prepend_if_ee('::EE::Gitlab::Auth::AuthFinders') # rubocop: disable Cop/InjectEnterpriseEditionModule
 
       include Gitlab::Utils::StrongMemoize
 
@@ -26,6 +26,7 @@ module Gitlab
       PRIVATE_TOKEN_PARAM = :private_token
       JOB_TOKEN_HEADER = "HTTP_JOB_TOKEN".freeze
       JOB_TOKEN_PARAM = :job_token
+      RUNNER_TOKEN_PARAM = :token
 
       # Check the Rails session for valid authentication details
       def find_user_from_warden
@@ -85,6 +86,15 @@ module Gitlab
         access_token.user || raise(UnauthorizedError)
       end
 
+      def find_runner_from_token
+        return unless api_request?
+
+        token = current_request.params[RUNNER_TOKEN_PARAM].presence
+        return unless token
+
+        ::Ci::Runner.find_by_token(token) || raise(UnauthorizedError)
+      end
+
       def validate_access_token!(scopes: [])
         return unless access_token
 
@@ -201,7 +211,7 @@ module Gitlab
       end
 
       def api_request?
-        current_request.path.starts_with?("/api/")
+        current_request.path.starts_with?('/api/')
       end
 
       def archive_request?

lib/gitlab/auth/request_authenticator.rb

@@ -5,7 +5,7 @@
 module Gitlab
   module Auth
     class RequestAuthenticator
-      include UserAuthFinders
+      include AuthFinders
 
       attr_reader :request
 
@@ -23,6 +23,12 @@ module Gitlab
         find_user_from_warden
       end
 
+      def runner
+        find_runner_from_token
+      rescue Gitlab::Auth::AuthenticationError
+        nil
+      end
+
       def find_sessionless_user(request_format)
         find_user_from_web_access_token(request_format) ||
           find_user_from_feed_token(request_format) ||

spec/lib/gitlab/auth/user_auth_finders_spec.rb → spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Auth::UserAuthFinders do
+describe Gitlab::Auth::AuthFinders do
   include described_class
 
   let(:user) { create(:user) }
@@ -196,13 +196,13 @@ describe Gitlab::Auth::UserAuthFinders do
 
     context 'when validate_access_token! returns valid' do
       it 'returns user' do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[described_class::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect(find_user_from_access_token).to eq user
       end
 
       it 'returns exception if token has no user' do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[described_class::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         allow_any_instance_of(PersonalAccessToken).to receive(:user).and_return(nil)
 
         expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
@@ -228,7 +228,7 @@ describe Gitlab::Auth::UserAuthFinders do
     let(:personal_access_token) { create(:personal_access_token, user: user) }
 
     before do
-      env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+      env[described_class::PRIVATE_TOKEN_HEADER] = personal_access_token.token
     end
 
     it 'returns exception if token has no user' do
@@ -279,7 +279,7 @@ describe Gitlab::Auth::UserAuthFinders do
 
     context 'passed as header' do
       it 'returns token if valid personal_access_token' do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[described_class::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect(find_personal_access_token).to eq personal_access_token
       end
@@ -287,7 +287,7 @@ describe Gitlab::Auth::UserAuthFinders do
 
     context 'passed as param' do
       it 'returns token if valid personal_access_token' do
-        set_param(Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_PARAM, personal_access_token.token)
+        set_param(described_class::PRIVATE_TOKEN_PARAM, personal_access_token.token)
 
         expect(find_personal_access_token).to eq personal_access_token
       end
@@ -298,7 +298,7 @@ describe Gitlab::Auth::UserAuthFinders do
     end
 
     it 'returns exception if invalid personal_access_token' do
-      env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid_token'
+      env[described_class::PRIVATE_TOKEN_HEADER] = 'invalid_token'
 
       expect { find_personal_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
     end
@@ -379,4 +379,58 @@ describe Gitlab::Auth::UserAuthFinders do
       end
     end
   end
+
+  describe '#find_runner_from_token' do
+    let(:runner) { create(:ci_runner) }
+
+    context 'with API requests' do
+      before do
+        env['SCRIPT_NAME'] = '/api/endpoint'
+      end
+
+      it 'returns the runner if token is valid' do
+        set_param(:token, runner.token)
+
+        expect(find_runner_from_token).to eq(runner)
+      end
+
+      it 'returns nil if token is not present' do
+        expect(find_runner_from_token).to be_nil
+      end
+
+      it 'returns nil if token is blank' do
+        set_param(:token, '')
+
+        expect(find_runner_from_token).to be_nil
+      end
+
+      it 'returns exception if invalid token' do
+        set_param(:token, 'invalid_token')
+
+        expect { find_runner_from_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
+      end
+    end
+
+    context 'without API requests' do
+      before do
+        env['SCRIPT_NAME'] = 'url.ics'
+      end
+
+      it 'returns nil if token is valid' do
+        set_param(:token, runner.token)
+
+        expect(find_runner_from_token).to be_nil
+      end
+
+      it 'returns nil if token is blank' do
+        expect(find_runner_from_token).to be_nil
+      end
+
+      it 'returns nil if invalid token' do
+        set_param(:token, 'invalid_token')
+
+        expect(find_runner_from_token).to be_nil
+      end
+    end
+  end
 end

spec/lib/gitlab/auth/request_authenticator_spec.rb

@@ -66,4 +66,28 @@ describe Gitlab::Auth::RequestAuthenticator do
       expect(subject.find_sessionless_user([:api])).to be_blank
     end
   end
+
+  describe '#runner' do
+    let!(:runner) { build(:ci_runner) }
+
+    it 'returns the runner using #find_runner_from_token' do
+      expect_any_instance_of(described_class)
+        .to receive(:find_runner_from_token)
+        .and_return(runner)
+
+      expect(subject.runner).to eq runner
+    end
+
+    it 'returns nil if no runner is found' do
+      expect(subject.runner).to be_blank
+    end
+
+    it 'rescue Gitlab::Auth::AuthenticationError exceptions' do
+      expect_any_instance_of(described_class)
+        .to receive(:find_runner_from_token)
+        .and_raise(Gitlab::Auth::UnauthorizedError)
+
+      expect(subject.runner).to be_blank
+    end
+  end
 end

spec/requests/api/helpers_spec.rb

@@ -146,13 +146,13 @@ describe API::Helpers do
       let(:personal_access_token) { create(:personal_access_token, user: user) }
 
       it "returns a 401 response for an invalid token" do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid token'
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = 'invalid token'
 
         expect { current_user }.to raise_error /401/
       end
 
       it "returns a 403 response for a user without access" do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
 
         expect { current_user }.to raise_error /403/
@@ -160,7 +160,7 @@ describe API::Helpers do
 
       it 'returns a 403 response for a user who is blocked' do
         user.block!
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect { current_user }.to raise_error /403/
       end
@@ -168,7 +168,7 @@ describe API::Helpers do
       context 'when terms are enforced' do
         before do
           enforce_terms
-          env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+          env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         end
 
         it 'returns a 403 when a user has not accepted the terms' do
@@ -183,27 +183,27 @@ describe API::Helpers do
       end
 
       it "sets current_user" do
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         expect(current_user).to eq(user)
       end
 
       it "does not allow tokens without the appropriate scope" do
         personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect { current_user }.to raise_error Gitlab::Auth::InsufficientScopeError
       end
 
       it 'does not allow revoked tokens' do
         personal_access_token.revoke!
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect { current_user }.to raise_error Gitlab::Auth::RevokedError
       end
 
       it 'does not allow expired tokens' do
         personal_access_token.update!(expires_at: 1.day.ago)
-        env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
         expect { current_user }.to raise_error Gitlab::Auth::ExpiredError
       end
@@ -213,7 +213,7 @@ describe API::Helpers do
 
         before do
           stub_config_setting(impersonation_enabled: false)
-          env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+          env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         end
 
         it 'does not allow impersonation tokens' do
@@ -478,7 +478,7 @@ describe API::Helpers do
 
       context 'passed as param' do
         before do
-          set_param(Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_PARAM, token.token)
+          set_param(Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_PARAM, token.token)
         end
 
         it_behaves_like 'sudo'
@@ -486,7 +486,7 @@ describe API::Helpers do
 
       context 'passed as header' do
         before do
-          env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = token.token
+          env[Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER] = token.token
         end
 
         it_behaves_like 'sudo'

spec/requests/rack_attack_global_spec.rb

@@ -100,6 +100,18 @@ describe 'Rack Attack global throttles' do
         end
       end
 
+      context 'when the request is authenticated by a runner token' do
+        let(:request_jobs_url) { '/api/v4/jobs/request' }
+        let(:runner) { create(:ci_runner) }
+
+        it 'does not cont as unauthenticated' do
+          (1 + requests_per_period).times do
+            post request_jobs_url, params: { token: runner.token }
+            expect(response).to have_http_status 204
+          end
+        end
+      end
+
       it 'logs RackAttack info into structured logs' do
         requests_per_period.times do
           get url_that_does_not_require_authentication