From fe46e4eb35dd6728a8a5ebcee9cc05a4613effbf Mon Sep 17 00:00:00 2001 From: Justin DiPierro Date: Thu, 29 Sep 2016 12:46:54 -0400 Subject: [PATCH 1/2] Load Github::Shell's secret token from file on initialization instead of every request. --- CHANGELOG | 1 + .../initializers/gitlab_shell_secret_token.rb | 2 +- lib/api/helpers.rb | 2 +- lib/gitlab/backend/shell.rb | 46 +++++++++++++------ lib/tasks/gitlab/shell.rake | 2 +- spec/lib/gitlab/backend/shell_spec.rb | 8 ++-- 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3bbf580b67e..c442ac4e058 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,7 @@ v 8.13.0 (unreleased) - Prevent flash alert text from being obscured when container is fluid - Append issue template to existing description !6149 (Joseph Frazier) - Trending projects now only show public projects and the list of projects is cached for a day + - Memoize Gitlab Shell's secret token (!6599, Justin DiPierro) - Revoke button in Applications Settings underlines on hover. - Use higher size on Gitlab::Redis connection pool on Sidekiq servers - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) diff --git a/config/initializers/gitlab_shell_secret_token.rb b/config/initializers/gitlab_shell_secret_token.rb index 7454c33c9dd..529dcdd4644 100644 --- a/config/initializers/gitlab_shell_secret_token.rb +++ b/config/initializers/gitlab_shell_secret_token.rb @@ -1 +1 @@ -Gitlab::Shell.new.generate_and_link_secret_token +Gitlab::Shell.ensure_secret_token! diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 8b8c4eb4d46..e3b947adcc3 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -433,7 +433,7 @@ module API end def secret_token - File.read(Gitlab.config.gitlab_shell.secret_file).chomp + Gitlab::Shell.secret_token end def send_git_blob(repository, blob) diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 79eac66b364..d0060fbaca1 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -17,6 +17,18 @@ module Gitlab end class << self + def secret_token + @secret_token ||= begin + File.read(Gitlab.config.gitlab_shell.secret_file).chomp + end + end + + def ensure_secret_token! + return if File.exist?(File.join(Gitlab.config.gitlab_shell.path, '.gitlab_shell_secret')) + + generate_and_link_secret_token + end + def version_required @version_required ||= File.read(Rails.root. join('GITLAB_SHELL_VERSION')).strip @@ -25,6 +37,25 @@ module Gitlab def strip_key(key) key.split(/ /)[0, 2].join(' ') end + + private + + # Create (if necessary) and link the secret token file + def generate_and_link_secret_token + secret_file = Gitlab.config.gitlab_shell.secret_file + shell_path = Gitlab.config.gitlab_shell.path + + unless File.size?(secret_file) + # Generate a new token of 16 random hexadecimal characters and store it in secret_file. + token = SecureRandom.hex(16) + File.write(secret_file, token) + end + + link_path = File.join(shell_path, '.gitlab_shell_secret') + if File.exist?(shell_path) && !File.exist?(link_path) + FileUtils.symlink(secret_file, link_path) + end + end end # Init new repository @@ -201,21 +232,6 @@ module Gitlab File.exist?(full_path(storage, dir_name)) end - # Create (if necessary) and link the secret token file - def generate_and_link_secret_token - secret_file = Gitlab.config.gitlab_shell.secret_file - unless File.size?(secret_file) - # Generate a new token of 16 random hexadecimal characters and store it in secret_file. - token = SecureRandom.hex(16) - File.write(secret_file, token) - end - - link_path = File.join(gitlab_shell_path, '.gitlab_shell_secret') - if File.exist?(gitlab_shell_path) && !File.exist?(link_path) - FileUtils.symlink(secret_file, link_path) - end - end - protected def gitlab_shell_path diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index bb7eb852f1b..210899882b4 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -78,7 +78,7 @@ namespace :gitlab do f.puts "PATH=#{ENV['PATH']}" end - Gitlab::Shell.new.generate_and_link_secret_token + Gitlab::Shell.ensure_secret_token! end desc "GitLab | Setup gitlab-shell" diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index 07407f212aa..c9c7ef8f479 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -22,15 +22,14 @@ describe Gitlab::Shell, lib: true do it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") } - describe 'generate_and_link_secret_token' do + describe 'memoized secret_token' do let(:secret_file) { 'tmp/tests/.secret_shell_test' } let(:link_file) { 'tmp/tests/shell-secret-test/.gitlab_shell_secret' } before do - allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file) + allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') FileUtils.mkdir('tmp/tests/shell-secret-test') - gitlab_shell.generate_and_link_secret_token end after do @@ -39,7 +38,10 @@ describe Gitlab::Shell, lib: true do end it 'creates and links the secret token file' do + secret_token = Gitlab::Shell.secret_token + expect(File.exist?(secret_file)).to be(true) + expect(File.read(secret_file).chomp).to eq(secret_token) expect(File.symlink?(link_file)).to be(true) expect(File.readlink(link_file)).to eq(secret_file) end From 1c462cf7d6d61aebac9b909102f0e794cc9e409a Mon Sep 17 00:00:00 2001 From: Justin DiPierro Date: Thu, 6 Oct 2016 22:40:04 -0400 Subject: [PATCH 2/2] Call ensure_secret_token! in secret token test's before block since it would be called in an initializer. --- spec/lib/gitlab/backend/shell_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index c9c7ef8f479..f826d0d1b04 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -30,6 +30,7 @@ describe Gitlab::Shell, lib: true do allow(Gitlab.config.gitlab_shell).to receive(:secret_file).and_return(secret_file) allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-secret-test') FileUtils.mkdir('tmp/tests/shell-secret-test') + Gitlab::Shell.ensure_secret_token! end after do