Merge branch '213929-move-job-auth-to-core' into 'master'

Move job token code to core

See merge request gitlab-org/gitlab!36510

ee/lib/ee/api/api_guard.rb

-# frozen_string_literal: true
-
-module EE
-  module API
-    module APIGuard
-      module HelperMethods
-        extend ::Gitlab::Utils::Override
-
-        override :find_user_from_sources
-        def find_user_from_sources
-          deploy_token_from_request ||
-            find_user_from_bearer_token ||
-            find_user_from_job_token ||
-            find_user_from_warden
-        end
-      end
-    end
-  end
-end

ee/lib/ee/api/helpers.rb

@@ -142,16 +142,6 @@ module EE
         params[::APIGuard::PRIVATE_TOKEN_PARAM] || env[::APIGuard::PRIVATE_TOKEN_HEADER]
       end
 
-      def job_token_authentication?
-        initial_current_user && @current_authenticated_job.present? # rubocop:disable Gitlab/ModuleWithInstanceVariables
-      end
-
-      # Returns the job associated with the token provided for
-      # authentication, if any
-      def current_authenticated_job
-        @current_authenticated_job
-      end
-
       def warden
         env['warden']
       end

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

@@ -7,11 +7,6 @@ module EE
         extend ActiveSupport::Concern
         extend ::Gitlab::Utils::Override
 
-        def find_user_from_bearer_token
-          find_user_from_job_bearer_token ||
-            find_user_from_access_token
-        end
-
         override :find_oauth_access_token
         def find_oauth_access_token
           return if scim_request?
@@ -19,31 +14,9 @@ module EE
           super
         end
 
-        override :validate_access_token!
-        def validate_access_token!(scopes: [])
-          # return early if we've already authenticated via a job token
-          @current_authenticated_job.present? || super # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        end
-
         def scim_request?
           current_request.path.starts_with?("/api/scim/")
         end
-
-        private
-
-        def find_user_from_job_bearer_token
-          return unless route_authentication_setting[:job_token_allowed]
-
-          token = parsed_oauth_token
-          return unless token
-
-          job = ::Ci::Build.find_by_token(token)
-          return unless job
-
-          @current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
-
-          job.user
-        end
       end
     end
   end

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

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Auth::AuthFinders do
-  include described_class
-
-  let(:user) { create(:user) }
-  let(:env) do
-    {
-      'rack.input' => ''
-    }
-  end
-  let(:request) { ActionDispatch::Request.new(env)}
-  let(:params) { request.params }
-
-  def set_param(key, value)
-    request.update_param(key, value)
-  end
-
-  shared_examples 'find user from job token' do
-    context 'when route is allowed to be authenticated' do
-      let(:route_authentication_setting) { { job_token_allowed: true } }
-
-      it "returns an Unauthorized exception for an invalid token" do
-        set_token('invalid token')
-
-        expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
-      end
-
-      it "return user if token is valid" do
-        set_token(job.token)
-
-        expect(subject).to eq(user)
-        expect(@current_authenticated_job).to eq job
-      end
-    end
-  end
-
-  describe '#validate_access_token!' do
-    subject { validate_access_token! }
-
-    context 'with a job token' do
-      let(:route_authentication_setting) { { job_token_allowed: true } }
-      let(:job) { create(:ci_build, user: user) }
-
-      before do
-        env['HTTP_AUTHORIZATION'] = "Bearer #{job.token}"
-        find_user_from_bearer_token
-      end
-
-      it 'does not raise an error' do
-        expect { subject }.not_to raise_error
-      end
-    end
-
-    context 'without a job token' do
-      let(:personal_access_token) { create(:personal_access_token, user: user) }
-
-      before do
-        personal_access_token.revoke!
-        allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token)
-      end
-
-      it 'delegates the logic to super' do
-        expect { subject }.to raise_error(Gitlab::Auth::RevokedError)
-      end
-    end
-  end
-
-  describe '#find_user_from_bearer_token' do
-    let(:job) { create(:ci_build, user: user) }
-
-    subject { find_user_from_bearer_token }
-
-    context 'when the token is passed as an oauth token' do
-      def set_token(token)
-        env['HTTP_AUTHORIZATION'] = "Bearer #{token}"
-      end
-
-      context 'with a job token' do
-        it_behaves_like 'find user from job token'
-      end
-
-      context 'with oauth token' do
-        let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
-        let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api').token }
-
-        before do
-          set_token(token)
-        end
-
-        it { is_expected.to eq user }
-      end
-    end
-
-    context 'with a personal access token' do
-      let(:pat) { create(:personal_access_token, user: user) }
-      let(:token) { pat.token }
-
-      before do
-        env[described_class::PRIVATE_TOKEN_HEADER] = pat.token
-      end
-
-      it { is_expected.to eq user }
-    end
-  end
-end

lib/api/api_guard.rb

@@ -43,7 +43,6 @@ module API
 
     # Helper Methods for Grape Endpoint
     module HelperMethods
-      prepend_if_ee('EE::API::APIGuard::HelperMethods') # rubocop: disable Cop/InjectEnterpriseEditionModule
       include Gitlab::Auth::AuthFinders
 
       def access_token
@@ -66,7 +65,7 @@ module API
 
       def find_user_from_sources
         deploy_token_from_request ||
-          find_user_from_access_token ||
+          find_user_from_bearer_token ||
           find_user_from_job_token ||
           find_user_from_warden
       end

lib/api/helpers.rb

@@ -41,6 +41,16 @@ module API
       end
     end
 
+    def job_token_authentication?
+      initial_current_user && @current_authenticated_job.present? # rubocop:disable Gitlab/ModuleWithInstanceVariables
+    end
+
+    # Returns the job associated with the token provided for
+    # authentication, if any
+    def current_authenticated_job
+      @current_authenticated_job
+    end
+
     # rubocop:disable Gitlab/ModuleWithInstanceVariables
     # We can't rewrite this with StrongMemoize because `sudo!` would
     # actually write to `@current_user`, and `sudo?` would immediately

lib/gitlab/auth/auth_finders.rb

@@ -54,6 +54,11 @@ module Gitlab
         User.find_by_feed_token(token) || raise(UnauthorizedError)
       end
 
+      def find_user_from_bearer_token
+        find_user_from_job_bearer_token ||
+          find_user_from_access_token
+      end
+
       def find_user_from_job_token
         return unless route_authentication_setting[:job_token_allowed]
         return find_user_from_basic_auth_job if route_authentication_setting[:job_token_allowed] == :basic_auth
@@ -136,6 +141,9 @@ module Gitlab
       end
 
       def validate_access_token!(scopes: [])
+        # return early if we've already authenticated via a job token
+        return if @current_authenticated_job.present? # rubocop:disable Gitlab/ModuleWithInstanceVariables
+
         # return early if we've already authenticated via a deploy token
         return if @current_authenticated_deploy_token.present? # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
@@ -155,6 +163,20 @@ module Gitlab
 
       private
 
+      def find_user_from_job_bearer_token
+        return unless route_authentication_setting[:job_token_allowed]
+
+        token = parsed_oauth_token
+        return unless token
+
+        job = ::Ci::Build.find_by_token(token)
+        return unless job
+
+        @current_authenticated_job = job # rubocop:disable Gitlab/ModuleWithInstanceVariables
+
+        job.user
+      end
+
       def route_authentication_setting
         return {} unless respond_to?(:route_setting)
 

spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -26,6 +26,63 @@ RSpec.describe Gitlab::Auth::AuthFinders do
     env.merge!(basic_auth_header(username, password))
   end
 
+  shared_examples 'find user from job token' do
+    context 'when route is allowed to be authenticated' do
+      let(:route_authentication_setting) { { job_token_allowed: true } }
+
+      it "returns an Unauthorized exception for an invalid token" do
+        set_token('invalid token')
+
+        expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError)
+      end
+
+      it "return user if token is valid" do
+        set_token(job.token)
+
+        expect(subject).to eq(user)
+        expect(@current_authenticated_job).to eq job
+      end
+    end
+  end
+
+  describe '#find_user_from_bearer_token' do
+    let(:job) { create(:ci_build, user: user) }
+
+    subject { find_user_from_bearer_token }
+
+    context 'when the token is passed as an oauth token' do
+      def set_token(token)
+        env['HTTP_AUTHORIZATION'] = "Bearer #{token}"
+      end
+
+      context 'with a job token' do
+        it_behaves_like 'find user from job token'
+      end
+
+      context 'with oauth token' do
+        let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
+        let(:token) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api').token }
+
+        before do
+          set_token(token)
+        end
+
+        it { is_expected.to eq user }
+      end
+    end
+
+    context 'with a personal access token' do
+      let(:pat) { create(:personal_access_token, user: user) }
+      let(:token) { pat.token }
+
+      before do
+        env[described_class::PRIVATE_TOKEN_HEADER] = pat.token
+      end
+
+      it { is_expected.to eq user }
+    end
+  end
+
   describe '#find_user_from_warden' do
     context 'with CSRF token' do
       before do
@@ -522,8 +579,24 @@ RSpec.describe Gitlab::Auth::AuthFinders do
   end
 
   describe '#validate_access_token!' do
+    subject { validate_access_token! }
+
     let(:personal_access_token) { create(:personal_access_token, user: user) }
 
+    context 'with a job token' do
+      let(:route_authentication_setting) { { job_token_allowed: true } }
+      let(:job) { create(:ci_build, user: user) }
+
+      before do
+        env['HTTP_AUTHORIZATION'] = "Bearer #{job.token}"
+        find_user_from_bearer_token
+      end
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+
     it 'returns nil if no access_token present' do
       expect(validate_access_token!).to be_nil
     end