Merge branch '22465-rack-attack-authenticate-job-token-requests' into 'master'

Auth runner requests with job tokens in Rack Attack

See merge request gitlab-org/gitlab!21412

changelogs/unreleased/22465-rack-attack-authenticate-job-token-requests.yml

+---
+title: Authenticate API requests with job tokens for Rack::Attack
+merge_request: 21412
+author:
+type: fixed

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

@@ -105,41 +105,4 @@ describe Gitlab::Auth::AuthFinders do
       it { is_expected.to eq user }
     end
   end
-
-  describe '#find_user_from_job_token' do
-    let(:job) { create(:ci_build, user: user) }
-
-    subject { find_user_from_job_token }
-
-    shared_examples 'job token disabled' do
-      context 'when route is not allowed to be authenticated' do
-        let(:route_authentication_setting) { { job_token_allowed: false } }
-
-        it "sets current_user to nil" do
-          set_token(job.token)
-          allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true)
-
-          expect(subject).to be_nil
-        end
-      end
-    end
-
-    context 'when the job token is in the headers' do
-      def set_token(token)
-        env[described_class::JOB_TOKEN_HEADER] = token
-      end
-
-      it_behaves_like 'find user from job token'
-      it_behaves_like 'job token disabled'
-    end
-
-    context 'when the job token is in the params' do
-      def set_token(token)
-        set_param(described_class::JOB_TOKEN_PARAM, token)
-      end
-
-      it_behaves_like 'find user from job token'
-      it_behaves_like 'job token disabled'
-    end
-  end
 end

lib/gitlab/auth/auth_finders.rb

@@ -25,9 +25,10 @@ module Gitlab
 
       PRIVATE_TOKEN_HEADER = 'HTTP_PRIVATE_TOKEN'
       PRIVATE_TOKEN_PARAM = :private_token
-      JOB_TOKEN_HEADER = "HTTP_JOB_TOKEN".freeze
+      JOB_TOKEN_HEADER = 'HTTP_JOB_TOKEN'.freeze
       JOB_TOKEN_PARAM = :job_token
       RUNNER_TOKEN_PARAM = :token
+      RUNNER_JOB_TOKEN_PARAM = :token
 
       # Check the Rails session for valid authentication details
       def find_user_from_warden
@@ -57,11 +58,13 @@ module Gitlab
       def find_user_from_job_token
         return unless route_authentication_setting[:job_token_allowed]
 
-        token = (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
-        return unless token.present?
+        token = current_request.params[JOB_TOKEN_PARAM].presence ||
+          current_request.params[RUNNER_JOB_TOKEN_PARAM].presence ||
+          current_request.env[JOB_TOKEN_HEADER].presence
+        return unless token
 
         job = ::Ci::Build.find_by_token(token)
-        raise ::Gitlab::Auth::UnauthorizedError unless job
+        raise UnauthorizedError unless job
 
         @current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
 

lib/gitlab/auth/request_authenticator.rb

@@ -33,7 +33,8 @@ module Gitlab
         find_user_from_web_access_token(request_format) ||
           find_user_from_feed_token(request_format) ||
           find_user_from_static_object_token(request_format) ||
-          find_user_from_basic_auth_job
+          find_user_from_basic_auth_job ||
+          find_user_from_job_token
       rescue Gitlab::Auth::AuthenticationError
         nil
       end
@@ -45,6 +46,14 @@ module Gitlab
       rescue Gitlab::Auth::AuthenticationError
         false
       end
+
+      private
+
+      def route_authentication_setting
+        @route_authentication_setting ||= {
+          job_token_allowed: api_request?
+        }
+      end
     end
   end
 end

spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -446,6 +446,93 @@ describe Gitlab::Auth::AuthFinders do
     end
   end
 
+  describe '#find_user_from_job_token' do
+    let(:job) { create(:ci_build, user: user) }
+    let(:route_authentication_setting) { { job_token_allowed: true } }
+
+    subject { find_user_from_job_token }
+
+    context 'when the job token is in the headers' do
+      it 'returns the user if valid job token' do
+        env[described_class::JOB_TOKEN_HEADER] = job.token
+
+        is_expected.to eq(user)
+        expect(@current_authenticated_job).to eq(job)
+      end
+
+      it 'returns nil without job token' do
+        env[described_class::JOB_TOKEN_HEADER] = ''
+
+        is_expected.to be_nil
+      end
+
+      it 'returns exception if invalid job token' do
+        env[described_class::JOB_TOKEN_HEADER] = 'invalid token'
+
+        expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+      end
+
+      context 'when route is not allowed to be authenticated' do
+        let(:route_authentication_setting) { { job_token_allowed: false } }
+
+        it 'sets current_user to nil' do
+          env[described_class::JOB_TOKEN_HEADER] = job.token
+
+          allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true)
+
+          is_expected.to be_nil
+        end
+      end
+    end
+
+    context 'when the job token is in the params' do
+      shared_examples 'job token params' do |token_key_name|
+        before do
+          set_param(token_key_name, token)
+        end
+
+        context 'with valid job token' do
+          let(:token) { job.token }
+
+          it 'returns the user' do
+            is_expected.to eq(user)
+            expect(@current_authenticated_job).to eq(job)
+          end
+        end
+
+        context 'with empty job token' do
+          let(:token) { '' }
+
+          it 'returns nil' do
+            is_expected.to be_nil
+          end
+        end
+
+        context 'with invalid job token' do
+          let(:token) { 'invalid token' }
+
+          it 'returns exception' do
+            expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+          end
+        end
+
+        context 'when route is not allowed to be authenticated' do
+          let(:route_authentication_setting) { { job_token_allowed: false } }
+          let(:token) { job.token }
+
+          it 'sets current_user to nil' do
+            allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true)
+
+            is_expected.to be_nil
+          end
+        end
+      end
+
+      it_behaves_like 'job token params', described_class::JOB_TOKEN_PARAM
+      it_behaves_like 'job token params', described_class::RUNNER_JOB_TOKEN_PARAM
+    end
+  end
+
   describe '#find_runner_from_token' do
     let(:runner) { create(:ci_runner) }
 

spec/lib/gitlab/auth/request_authenticator_spec.rb

@@ -42,6 +42,8 @@ describe Gitlab::Auth::RequestAuthenticator do
   describe '#find_sessionless_user' do
     let!(:access_token_user) { build(:user) }
     let!(:feed_token_user) { build(:user) }
+    let!(:static_object_token_user) { build(:user) }
+    let!(:job_token_user) { build(:user) }
 
     it 'returns access_token user first' do
       allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user)
@@ -56,6 +58,22 @@ describe Gitlab::Auth::RequestAuthenticator do
       expect(subject.find_sessionless_user([:api])).to eq feed_token_user
     end
 
+    it 'returns static_object_token user if no feed_token user found' do
+      allow_any_instance_of(described_class)
+        .to receive(:find_user_from_static_object_token)
+        .and_return(static_object_token_user)
+
+      expect(subject.find_sessionless_user([:api])).to eq static_object_token_user
+    end
+
+    it 'returns job_token user if no static_object_token user found' do
+      allow_any_instance_of(described_class)
+        .to receive(:find_user_from_job_token)
+        .and_return(job_token_user)
+
+      expect(subject.find_sessionless_user([:api])).to eq job_token_user
+    end
+
     it 'returns nil if no user found' do
       expect(subject.find_sessionless_user([:api])).to be_blank
     end
@@ -67,6 +85,39 @@ describe Gitlab::Auth::RequestAuthenticator do
     end
   end
 
+  describe '#find_user_from_job_token' do
+    let!(:user) { build(:user) }
+    let!(:job) { build(:ci_build, user: user) }
+
+    before do
+      env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'token'
+    end
+
+    context 'with API requests' do
+      before do
+        env['SCRIPT_NAME'] = '/api/endpoint'
+      end
+
+      it 'tries to find the user' do
+        expect(::Ci::Build).to receive(:find_by_token).and_return(job)
+
+        expect(subject.find_sessionless_user([:api])).to eq user
+      end
+    end
+
+    context 'without API requests' do
+      before do
+        env['SCRIPT_NAME'] = '/web/endpoint'
+      end
+
+      it 'does not search for job users' do
+        expect(::Ci::Build).not_to receive(:find_by_token)
+
+        expect(subject.find_sessionless_user([:api])).to be_nil
+      end
+    end
+  end
+
   describe '#runner' do
     let!(:runner) { build(:ci_runner) }