From e6cc4e90f985633615c4b0077016dd9e155ceecc Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Mon, 22 Nov 2021 15:15:25 -0600
Subject: [PATCH] Refactor git http controllers to rely on auth results
 differently

Auth results can return an ambiguous actor, either user or
deploy token. Refactor to explicitly get user or deploy token
rather than accessing the actor directly.
---
 .../repositories/git_http_client_controller.rb      | 13 +++++++++----
 .../ee/repositories/git_http_controller.rb          |  1 +
 .../repositories/git_http_controller_spec.rb        |  8 ++++++++
 .../git_http_controller_shared_examples.rb          | 10 ++++++----
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/app/controllers/repositories/git_http_client_controller.rb b/app/controllers/repositories/git_http_client_controller.rb
index b3adda8c633d..c002c9b83f9c 100644
--- a/app/controllers/repositories/git_http_client_controller.rb
+++ b/app/controllers/repositories/git_http_client_controller.rb
@@ -8,12 +8,9 @@ class GitHttpClientController < Repositories::ApplicationController
 
     attr_reader :authentication_result, :redirected_path
 
-    delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true
+    delegate :authentication_abilities, to: :authentication_result, allow_nil: true
     delegate :type, to: :authentication_result, allow_nil: true, prefix: :auth_result
 
-    alias_method :user, :actor
-    alias_method :authenticated_user, :actor
-
     # Git clients will not know what authenticity token to send along
     skip_around_action :set_session_storage
     skip_before_action :verify_authenticity_token
@@ -22,8 +19,16 @@ class GitHttpClientController < Repositories::ApplicationController
 
     feature_category :source_code_management
 
+    def authenticated_user
+      authentication_result&.user || authentication_result&.deploy_token
+    end
+
     private
 
+    def user
+      authenticated_user
+    end
+
     def download_request?
       raise NotImplementedError
     end
diff --git a/ee/app/controllers/ee/repositories/git_http_controller.rb b/ee/app/controllers/ee/repositories/git_http_controller.rb
index 695b8359dfa1..903fec8f0c74 100644
--- a/ee/app/controllers/ee/repositories/git_http_controller.rb
+++ b/ee/app/controllers/ee/repositories/git_http_controller.rb
@@ -30,6 +30,7 @@ def git_receive_pack
 
       private
 
+      override :user
       def user
         super || geo_push_user&.user
       end
diff --git a/spec/controllers/repositories/git_http_controller_spec.rb b/spec/controllers/repositories/git_http_controller_spec.rb
index b5cd14154a31..4a6e745cd63e 100644
--- a/spec/controllers/repositories/git_http_controller_spec.rb
+++ b/spec/controllers/repositories/git_http_controller_spec.rb
@@ -90,6 +90,14 @@ def send_request
         end
       end
     end
+
+    context 'when the user is a deploy token' do
+      it_behaves_like Repositories::GitHttpController do
+        let(:container) { project }
+        let(:user) { create(:deploy_token, :project, projects: [project]) }
+        let(:access_checker_class) { Gitlab::GitAccess }
+      end
+    end
   end
 
   context 'when repository container is a project wiki' do
diff --git a/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb b/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
index 00a0fb7e4c54..3a7588a5cc93 100644
--- a/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/repositories/git_http_controller_shared_examples.rb
@@ -50,7 +50,8 @@
 
     context 'with authorized user' do
       before do
-        request.headers.merge! auth_env(user.username, user.password, nil)
+        password = user.try(:password) || user.try(:token)
+        request.headers.merge! auth_env(user.username, password, nil)
       end
 
       it 'returns 200' do
@@ -71,9 +72,10 @@
         it 'adds user info to the logs' do
           get :info_refs, params: params
 
-          expect(log_data).to include('username' => user.username,
-                                      'user_id' => user.id,
-                                      'meta.user' => user.username)
+          user_log_data = { 'username' => user.username, 'user_id' => user.id }
+          user_log_data['meta.user'] = user.username if user.is_a?(User)
+
+          expect(log_data).to include(user_log_data)
         end
       end
     end
-- 
GitLab