From 0672c5a92e8be90da0cb79f277bb7aee82fdba8a Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 20 Sep 2016 15:41:41 +0200
Subject: [PATCH] Post-merge improve of CI permissions
---
.../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
--
GitLab