From 0672c5a92e8be90da0cb79f277bb7aee82fdba8a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 20 Sep 2016 15:41:41 +0200 Subject: Post-merge improve of CI permissions --- app/controllers/projects/git_http_client_controller.rb | 6 +++--- app/models/ci/build.rb | 7 +++++-- lib/ci/mask_secret.rb | 6 +++--- spec/lib/ci/mask_secret_spec.rb | 12 +++++++++--- spec/lib/gitlab/git_access_spec.rb | 2 +- spec/requests/git_http_spec.rb | 6 +++--- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index cbfd3cab3dd..383e184d796 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -32,11 +32,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController return # Allow access end elsif allow_kerberos_spnego_auth? && spnego_provided? - user = find_kerberos_user + kerberos_user = find_kerberos_user - if user + if kerberos_user @authentication_result = Gitlab::Auth::Result.new( - user, nil, :kerberos, Gitlab::Auth.full_authentication_abilities) + kerberos_user, nil, :kerberos, Gitlab::Auth.full_authentication_abilities) send_final_spnego_response return # Allow access diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index dd984aef318..cb87b43f6be 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -493,8 +493,11 @@ module Ci end def hide_secrets(trace) - trace = Ci::MaskSecret.mask(trace, project.runners_token) if project - trace = Ci::MaskSecret.mask(trace, token) + return unless trace + + trace = trace.dup + Ci::MaskSecret.mask!(trace, project.runners_token) if project + Ci::MaskSecret.mask!(trace, token) trace end end diff --git a/lib/ci/mask_secret.rb b/lib/ci/mask_secret.rb index 3da04edde70..3388a642eb4 100644 --- a/lib/ci/mask_secret.rb +++ b/lib/ci/mask_secret.rb @@ -1,9 +1,9 @@ module Ci::MaskSecret class << self - def mask(value, token) - return value unless value.present? && token.present? + def mask!(value, token) + return unless value.present? && token.present? - value.gsub(token, 'x' * token.length) + value.gsub!(token, 'x' * token.length) end end end diff --git a/spec/lib/ci/mask_secret_spec.rb b/spec/lib/ci/mask_secret_spec.rb index 518de76911c..a6938533138 100644 --- a/spec/lib/ci/mask_secret_spec.rb +++ b/spec/lib/ci/mask_secret_spec.rb @@ -5,15 +5,21 @@ describe Ci::MaskSecret, lib: true do describe '#mask' do it 'masks exact number of characters' do - expect(subject.mask('token', 'oke')).to eq('txxxn') + expect(mask('token', 'oke')).to eq('txxxn') end it 'masks multiple occurrences' do - expect(subject.mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') + expect(mask('token token token', 'oke')).to eq('txxxn txxxn txxxn') end it 'does not mask if not found' do - expect(subject.mask('token', 'not')).to eq('token') + expect(mask('token', 'not')).to eq('token') + end + + def mask(value, token) + value = value.dup + subject.mask!(value, token) + value end end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ed43646330f..de68e32e5b4 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -343,7 +343,7 @@ describe Gitlab::GitAccess, lib: true do end context 'to private project' do - let(:project) { create(:project, :internal) } + let(:project) { create(:project) } it { expect(subject).not_to be_allowed } end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index e3922bec689..74516686921 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -335,7 +335,7 @@ describe 'Git HTTP requests', lib: true do project.team << [user, :reporter] end - shared_examples 'can download code only from own projects' do + shared_examples 'can download code only' do it 'downloads get status 200' do clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token @@ -353,7 +353,7 @@ describe 'Git HTTP requests', lib: true do context 'administrator' do let(:user) { create(:admin) } - it_behaves_like 'can download code only from own projects' + it_behaves_like 'can download code only' it 'downloads from other project get status 403' do clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token @@ -365,7 +365,7 @@ describe 'Git HTTP requests', lib: true do context 'regular user' do let(:user) { create(:user) } - it_behaves_like 'can download code only from own projects' + it_behaves_like 'can download code only' it 'downloads from other project get status 404' do clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token -- cgit v1.2.1