Merge branch 'mk-328692-refactor-find-for-git-client-go-class-part' into 'master'

Refactor go middleware class to stop referencing auth::result.actor directly

See merge request gitlab-org/gitlab!67039

lib/gitlab/auth/result.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module Auth
     Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
+      self::EMPTY = self.new(nil, nil, nil, nil).freeze
+
       def ci?(for_project)
         type == :ci &&
           project &&
@@ -29,6 +31,20 @@ module Gitlab
       def deploy_token
         actor.is_a?(DeployToken) ? actor : nil
       end
+
+      def can?(action)
+        actor&.can?(action)
+      end
+
+      def can_perform_action_on_project?(action, given_project)
+        Ability.allowed?(actor, action, given_project)
+      end
+
+      def authentication_abilities_include?(ability)
+        return false if authentication_abilities.blank?
+
+        authentication_abilities.include?(ability)
+      end
     end
   end
 end

lib/gitlab/middleware/go.rb

@@ -127,23 +127,25 @@ module Gitlab
 
       def project_for_paths(paths, request)
         project = Project.where_full_path_in(paths).first
-        return unless Ability.allowed?(current_user(request, project), :read_project, project)
+
+        return unless authentication_result(request, project).can_perform_action_on_project?(:read_project, project)
 
         project
       end
 
-      def current_user(request, project)
-        return unless has_basic_credentials?(request)
+      def authentication_result(request, project)
+        empty_result = Gitlab::Auth::Result::EMPTY
+        return empty_result unless has_basic_credentials?(request)
 
         login, password = user_name_and_password(request)
         auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
-        return unless auth_result.success?
+        return empty_result unless auth_result.success?
 
-        return unless auth_result.actor&.can?(:access_git)
+        return empty_result unless auth_result.can?(:access_git)
 
-        return unless auth_result.authentication_abilities.include?(:read_project)
+        return empty_result unless auth_result.authentication_abilities_include?(:read_project)
 
-        auth_result.actor
+        auth_result
       end
     end
   end

spec/lib/gitlab/auth/result_spec.rb

@@ -3,10 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Auth::Result do
+  let_it_be(:actor) { create(:user) }
+
   subject { described_class.new(actor, nil, nil, []) }
 
   context 'when actor is User' do
-    let(:actor) { create(:user) }
+    let_it_be(:actor) { create(:user) }
 
     it 'returns auth_user' do
       expect(subject.auth_user).to eq(actor)
@@ -18,7 +20,7 @@ RSpec.describe Gitlab::Auth::Result do
   end
 
   context 'when actor is Deploy token' do
-    let(:actor) { create(:deploy_token) }
+    let_it_be(:actor) { create(:deploy_token) }
 
     it 'returns deploy token' do
       expect(subject.deploy_token).to eq(actor)
@@ -28,4 +30,50 @@ RSpec.describe Gitlab::Auth::Result do
       expect(subject.auth_user).to be_nil
     end
   end
+
+  describe '#authentication_abilities_include?' do
+    context 'when authentication abilities are empty' do
+      it 'returns false' do
+        expect(subject.authentication_abilities_include?(:read_code)).to be_falsey
+      end
+    end
+
+    context 'when authentication abilities are not empty' do
+      subject { described_class.new(actor, nil, nil, [:push_code]) }
+
+      it 'returns false when ability is not allowed' do
+        expect(subject.authentication_abilities_include?(:read_code)).to be_falsey
+      end
+
+      it 'returns true when ability is allowed' do
+        expect(subject.authentication_abilities_include?(:push_code)).to be_truthy
+      end
+    end
+  end
+
+  describe '#can_perform_action_on_project?' do
+    let(:project) { double }
+
+    it 'returns if actor can do perform given action on given project' do
+      expect(Ability).to receive(:allowed?).with(actor, :push_code, project).and_return(true)
+      expect(subject.can_perform_action_on_project?(:push_code, project)).to be_truthy
+    end
+
+    it 'returns if actor cannot do perform given action on given project' do
+      expect(Ability).to receive(:allowed?).with(actor, :push_code, project).and_return(false)
+      expect(subject.can_perform_action_on_project?(:push_code, project)).to be_falsey
+    end
+  end
+
+  describe '#can?' do
+    it 'returns if actor can do perform given action on given project' do
+      expect(actor).to receive(:can?).with(:push_code).and_return(true)
+      expect(subject.can?(:push_code)).to be_truthy
+    end
+
+    it 'returns if actor cannot do perform given action on given project' do
+      expect(actor).to receive(:can?).with(:push_code).and_return(false)
+      expect(subject.can?(:push_code)).to be_falsey
+    end
+  end
 end