diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-10 16:13:05 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-11 15:20:11 +0200 |
commit | e0768a9bcb32e81fe18a77b21573969f45b47683 (patch) | |
tree | 00e91e901cad4c4ae0a616cdf7b555b5773e3aad | |
parent | d801dd177483a8375f1656654ca3638c18550204 (diff) | |
download | gitlab-ce-e0768a9bcb32e81fe18a77b21573969f45b47683.tar.gz |
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.
-rw-r--r-- | lib/gitlab/build_access.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/git_access.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/build_access_spec.rb | 23 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 16 | ||||
-rw-r--r-- | spec/requests/git_http_spec.rb | 53 |
5 files changed, 109 insertions, 1 deletions
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 |