From e0768a9bcb32e81fe18a77b21573969f45b47683 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Thu, 10 May 2018 16:13:05 +0200
Subject: [PATCH] Allow triggered builds git access

Allow builds that have been triggered by a user before terms were
enforced access to git. That way the builds can complete as usual.
---
 lib/gitlab/build_access.rb           | 12 +++++++
 lib/gitlab/git_access.rb             |  6 +++-
 spec/lib/gitlab/build_access_spec.rb | 23 ++++++++++++
 spec/lib/gitlab/git_access_spec.rb   | 16 +++++++++
 spec/requests/git_http_spec.rb       | 53 ++++++++++++++++++++++++++++
 5 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 lib/gitlab/build_access.rb
 create mode 100644 spec/lib/gitlab/build_access_spec.rb

diff --git a/lib/gitlab/build_access.rb b/lib/gitlab/build_access.rb
new file mode 100644
index 00000000000..08a8f846ca5
--- /dev/null
+++ b/lib/gitlab/build_access.rb
@@ -0,0 +1,12 @@
+module Gitlab
+  class BuildAccess < UserAccess
+    attr_accessor :user, :project
+
+    # This bypasses the `can?(:access_git)`-check we normally do in `UserAccess`
+    # for CI. That way if a user was able to trigger a pipeline, then the
+    # build is allowed to clone the project.
+    def can_access_git?
+      true
+    end
+  end
+end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 520b92a0363..db7c29be94b 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -105,7 +105,9 @@ module Gitlab
     end
 
     def check_active_user!
-      if user && !user_access.allowed?
+      return unless user
+
+      unless user_access.allowed?
         message = Gitlab::Auth::UserAccessDeniedReason.new(user).rejection_message
         raise UnauthorizedError, message
       end
@@ -338,6 +340,8 @@ module Gitlab
     def user_access
       @user_access ||= if ci?
                          CiAccess.new
+                       elsif user && request_from_ci_build?
+                         BuildAccess.new(user, project: project)
                        else
                          UserAccess.new(user, project: project)
                        end
diff --git a/spec/lib/gitlab/build_access_spec.rb b/spec/lib/gitlab/build_access_spec.rb
new file mode 100644
index 00000000000..08f50bf4fac
--- /dev/null
+++ b/spec/lib/gitlab/build_access_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Gitlab::BuildAccess do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  describe '#can_do_action' do
+    subject { described_class.new(user, project: project).can_do_action?(:download_code) }
+
+    context 'when the user can do an action on the project but cannot access git' do
+      before do
+        user.block!
+        project.add_developer(user)
+      end
+
+      it { is_expected.to be(true) }
+    end
+
+    context 'when the user cannot do an action on the project' do
+      it { is_expected.to be(false) }
+    end
+  end
+end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index f421a7135f3..317a932d5a6 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1114,6 +1114,22 @@ describe Gitlab::GitAccess do
 
       it_behaves_like 'access after accepting terms'
     end
+
+    describe 'when a ci build clones the project' do
+      let(:protocol) { 'http' }
+      let(:authentication_abilities) { [:build_download_code] }
+      let(:auth_result_type) { :build }
+
+      before do
+        project.add_developer(user)
+      end
+
+      it "doesn't block http pull" do
+        aggregate_failures do
+          expect { pull_access_check }.not_to raise_error
+        end
+      end
+    end
   end
 
   private
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 494db30e8e0..2514dab1714 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -1,6 +1,7 @@
 require "spec_helper"
 
 describe 'Git HTTP requests' do
+  include TermsHelper
   include GitHttpHelpers
   include WorkhorseHelpers
   include UserActivitiesHelpers
@@ -824,4 +825,56 @@ describe 'Git HTTP requests' do
       end
     end
   end
+
+  context 'when terms are enforced' do
+    let(:project) { create(:project, :repository) }
+    let(:user) { create(:user) }
+    let(:path) { "#{project.full_path}.git" }
+    let(:env) { { user: user.username, password: user.password } }
+
+    before do
+      project.add_master(user)
+      enforce_terms
+    end
+
+    it 'blocks git access when the user did not accept terms', :aggregate_failures do
+      clone_get(path, env) do |response|
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      download(path, env) do |response|
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      upload(path, env) do |response|
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'when the user accepted the terms' do
+      before do
+        accept_terms(user)
+      end
+
+      it 'allows clones' do
+        clone_get(path, env) do |response|
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      it_behaves_like 'pulls are allowed'
+      it_behaves_like 'pushes are allowed'
+    end
+
+    context 'from CI' do
+      let(:build) { create(:ci_build, :running) }
+      let(:env) { { user: 'gitlab-ci-token', password: build.token } }
+
+      before do
+        build.update!(user: user, project: project)
+      end
+
+      it_behaves_like 'pulls are allowed'
+    end
+  end
 end