Merge branch '44389-always-allow-http-for-ci-git-operations' into 'master'

Resolve "Allow HTTPS cloning by Runners if it is disabled for users"

Closes #44389

See merge request gitlab-org/gitlab-ce!18021

6 files changed, 50 insertions, 7 deletions

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index dd5e66f60e3..07249fe3182 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -7,6 +7,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   attr_reader :authentication_result, :redirected_path
 
   delegate :actor, :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
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 45910a9be44..1dcf837f78e 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -64,7 +64,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
     @access ||= access_klass.new(access_actor, project,
       'http', authentication_abilities: authentication_abilities,
               namespace_path: params[:namespace_id], project_path: project_path,
-              redirected_path: redirected_path)
+              redirected_path: redirected_path, auth_result_type: auth_result_type)
   end
 
   def access_actor
diff --git a/changelogs/unreleased/44389-always-allow-http-for-ci-git-operations.yml b/changelogs/unreleased/44389-always-allow-http-for-ci-git-operations.yml
new file mode 100644
index 00000000000..2e5a0302ee6
--- /dev/null
+++ b/changelogs/unreleased/44389-always-allow-http-for-ci-git-operations.yml
@@ -0,0 +1,5 @@
+---
+title: Allow HTTP(s) when git request is made by GitLab CI
+merge_request: 18021
+author:
+type: changed
diff --git a/doc/user/admin_area/settings/visibility_and_access_controls.md b/doc/user/admin_area/settings/visibility_and_access_controls.md
index 633f16a617c..3d38588a9ed 100644
--- a/doc/user/admin_area/settings/visibility_and_access_controls.md
+++ b/doc/user/admin_area/settings/visibility_and_access_controls.md
@@ -32,9 +32,15 @@ When you choose to allow only one of the protocols, a couple of things will happ
 On top of these UI restrictions, GitLab will deny all Git actions on the protocol
 not selected.
 
+CAUTION: **Important:**
+Starting with [GitLab 10.7][ce-18021], HTTP(s) protocol will be allowed for
+git clone/fetch requests done by GitLab Runner from CI/CD Jobs, even if
+_Only SSH_ was selected.
+
 > **Note:** Please keep in mind that disabling an access protocol does not actually
-  block access to the server itself. The ports used for the protocol, be it SSH or
-  HTTP, will still be accessible. What GitLab does is restrict access on the
-  application level.
+block access to the server itself. The ports used for the protocol, be it SSH or
+HTTP, will still be accessible. What GitLab does is restrict access on the
+application level.
 
 [ce-4696]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4696
+[ce-18021]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18021
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index ed0644f6cf1..6a01957184d 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -29,9 +29,9 @@ module Gitlab
     PUSH_COMMANDS = %w{ git-receive-pack }.freeze
     ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS
 
-    attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :project_path, :redirected_path
+    attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :project_path, :redirected_path, :auth_result_type
 
-    def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, project_path: nil, redirected_path: nil)
+    def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, project_path: nil, redirected_path: nil, auth_result_type: nil)
       @actor    = actor
       @project  = project
       @protocol = protocol
@@ -39,6 +39,7 @@ module Gitlab
       @namespace_path = namespace_path
       @project_path = project_path
       @redirected_path = redirected_path
+      @auth_result_type = auth_result_type
     end
 
     def check(cmd, changes)
@@ -78,6 +79,12 @@ module Gitlab
       authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code)
     end
 
+    def request_from_ci_build?
+      return false unless protocol == 'http'
+
+      auth_result_type == :build || auth_result_type == :ci
+    end
+
     def protocol_allowed?
       Gitlab::ProtocolAccess.allowed?(protocol)
     end
@@ -93,6 +100,8 @@ module Gitlab
     end
 
     def check_protocol!
+      return if request_from_ci_build?
+
       unless protocol_allowed?
         raise UnauthorizedError, "Git access over #{protocol.upcase} is not allowed"
       end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index f8f09d29c73..b845abab5ef 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -10,12 +10,13 @@ describe Gitlab::GitAccess do
   let(:protocol) { 'ssh' }
   let(:authentication_abilities) { %i[read_project download_code push_code] }
   let(:redirected_path) { nil }
+  let(:auth_result_type) { nil }
 
   let(:access) do
     described_class.new(actor, project,
       protocol, authentication_abilities: authentication_abilities,
                 namespace_path: namespace_path, project_path: project_path,
-                redirected_path: redirected_path)
+                redirected_path: redirected_path, auth_result_type: auth_result_type)
   end
 
   let(:changes) { '_any' }
@@ -45,6 +46,7 @@ describe Gitlab::GitAccess do
 
       before do
         disable_protocol('http')
+        project.add_master(user)
       end
 
       it 'blocks http push and pull' do
@@ -53,6 +55,26 @@ describe Gitlab::GitAccess do
           expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
         end
       end
+
+      context 'when request is made from CI' do
+        let(:auth_result_type) { :build }
+
+        it "doesn't block http pull" do
+          aggregate_failures do
+            expect { pull_access_check }.not_to raise_unauthorized('Git access over HTTP is not allowed')
+          end
+        end
+
+        context 'when legacy CI credentials are used' do
+          let(:auth_result_type) { :ci }
+
+          it "doesn't block http pull" do
+            aggregate_failures do
+              expect { pull_access_check }.not_to raise_unauthorized('Git access over HTTP is not allowed')
+            end
+          end
+        end
+      end
     end
   end