diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-02-12 03:28:01 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-02-12 03:28:01 +0000 |
commit | 63e16172e524d83e4fe04945e277bc21dda6d777 (patch) | |
tree | 032eb81a84675799500b5ecb54789fea90f3fcb6 | |
parent | f11e1bf914854e53217ece1ec6f8947f2894a1c7 (diff) | |
parent | 562d7eb4ecaa9ca35f970567c0f09cdb29d26521 (diff) | |
download | gitlab-shell-63e16172e524d83e4fe04945e277bc21dda6d777.tar.gz |
Merge branch 'internal-api-unavailable' into 'master'
Show nice error message when internal API is unreachable.
Closes #27.
Error message reads "Failed to authorize your Git request: internal API unreachable".
See merge request !54
-rwxr-xr-x | bin/check | 15 | ||||
-rw-r--r-- | lib/gitlab_access.rb | 21 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 86 | ||||
-rw-r--r-- | lib/gitlab_post_receive.rb | 10 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 23 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 49 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 22 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 21 |
8 files changed, 175 insertions, 72 deletions
@@ -8,13 +8,18 @@ require_relative '../lib/gitlab_net' # print "Check GitLab API access: " -resp = GitlabNet.new.check -if resp.code == "200" - print 'OK' -else - abort "FAILED. code: #{resp.code}" +begin + resp = GitlabNet.new.check + if resp.code == "200" + print 'OK' + else + abort "FAILED. code: #{resp.code}" + end +rescue GitlabNet::ApiUnreachableError + abort "FAILED: Failed to connect to internal API" end + puts "\nCheck directories and files: " config = GitlabConfig.new diff --git a/lib/gitlab_access.rb b/lib/gitlab_access.rb index 547b81d..22343fd 100644 --- a/lib/gitlab_access.rb +++ b/lib/gitlab_access.rb @@ -18,15 +18,20 @@ class GitlabAccess end def exec - status = api.check_access('git-receive-pack', @repo_name, @actor, @changes) - if status.allowed? - true - else - # reset GL_ID env since we stop git push here - ENV['GL_ID'] = nil - puts "GitLab: #{status.message}" - false + begin + status = api.check_access('git-receive-pack', @repo_name, @actor, @changes) + + return true if status.allowed? + + message = status.message + rescue GitlabNet::ApiUnreachableError + message = "Failed to authorize your Git request: internal API unreachable" end + + # reset GL_ID env since we stop git push here + ENV['GL_ID'] = nil + puts "GitLab: #{message}" + false end protected diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 9bfc0d5..6e76c98 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -7,6 +7,8 @@ require_relative 'gitlab_logger' require_relative 'gitlab_access' class GitlabNet + class ApiUnreachableError < StandardError; end + def check_access(cmd, repo, actor, changes) project_name = repo.gsub("'", "") project_name = project_name.gsub(/\.git\Z/, "") @@ -64,63 +66,65 @@ class GitlabNet "#{config.gitlab_url}/api/v3/internal" end - def http_client_for(url) - Net::HTTP.new(url.host, url.port).tap do |http| - if URI::HTTPS === url - http.use_ssl = true - http.cert_store = cert_store - http.verify_mode = OpenSSL::SSL::VERIFY_NONE if config.http_settings['self_signed_cert'] - end + def http_client_for(uri) + http = Net::HTTP.new(uri.host, uri.port) + + if uri.is_a?(URI::HTTPS) + http.use_ssl = true + http.cert_store = cert_store + http.verify_mode = OpenSSL::SSL::VERIFY_NONE if config.http_settings['self_signed_cert'] end + + http end - def http_request_for(url, method = :get) + def http_request_for(method, uri, params = {}) + request_klass = method == :get ? Net::HTTP::Get : Net::HTTP::Post + request = request_klass.new(uri.request_uri) + user = config.http_settings['user'] password = config.http_settings['password'] + request.basic_auth(user, password) if user && password + + request.set_form_data(params.merge(secret_token: secret_token)) + + request + end + + def request(method, url, params = {}) + $logger.debug "Performing #{method.to_s.upcase} #{url}" + + uri = URI.parse(url) + + http = http_client_for(uri) + request = http_request_for(method, uri, params) - if method == :get - Net::HTTP::Get.new(url.request_uri).tap { |r| r.basic_auth(user, password) if user && password } + begin + response = http.start { http.request(request) } + rescue + raise ApiUnreachableError + end + + if response.code == "200" + $logger.debug "Received response #{response.code} => <#{response.body}>." else - Net::HTTP::Post.new(url.request_uri).tap { |r| r.basic_auth(user, password) if user && password } + $logger.error "API call <#{method.to_s.upcase} #{url}> failed: #{response.code} => <#{response.body}>." end + + response end def get(url) - $logger.debug "Performing GET #{url}" - - url = URI.parse(url) - http = http_client_for url - request = http_request_for url - request.set_form_data(secret_token: secret_token) - - http.start { |http| http.request(request) }.tap do |resp| - if resp.code == "200" - $logger.debug { "Received response #{resp.code} => <#{resp.body}>." } - else - $logger.error { "API call <GET #{url}> failed: #{resp.code} => <#{resp.body}>." } - end - end + request(:get, url) end def post(url, params) - $logger.debug "Performing POST #{url}" - - url = URI.parse(url) - http = http_client_for(url) - request = http_request_for(url, :post) - request.set_form_data(params.merge(secret_token: secret_token)) - - http.start { |http| http.request(request) }.tap do |resp| - if resp.code == "200" - $logger.debug { "Received response #{resp.code} => <#{resp.body}>." } - else - $logger.error { "API call <POST #{url}> failed: #{resp.code} => <#{resp.body}>." } - end - end + request(:post, url, params) end def cert_store - @cert_store ||= OpenSSL::X509::Store.new.tap do |store| + @cert_store ||= begin + store = OpenSSL::X509::Store.new store.set_default_paths if ca_file = config.http_settings['ca_file'] @@ -130,6 +134,8 @@ class GitlabNet if ca_path = config.http_settings['ca_path'] store.add_path(ca_path) end + + store end end diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 7cd5535..98b935b 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -18,9 +18,13 @@ class GitlabPostReceive update_redis - if broadcast_message = GitlabNet.new.broadcast_message - puts - print_broadcast_message(broadcast_message["message"]) + begin + broadcast_message = GitlabNet.new.broadcast_message + if broadcast_message + puts + print_broadcast_message(broadcast_message["message"]) + end + rescue GitlabNet::ApiUnreachableError end end diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 95fad9e..9605136 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -21,12 +21,13 @@ class GitlabShell if git_cmds.include?(@git_cmd) ENV['GL_ID'] = @key_id - if validate_access + access = api.check_access(@git_cmd, @repo_name, @key_id, '_any') + if access.allowed? process_cmd else message = "gitlab-shell: Access denied for git command <#{@origin_cmd}> by #{log_username}." $logger.warn message - $stderr.puts "Access denied." + puts access.message end else raise DisallowedCommandError @@ -34,10 +35,13 @@ class GitlabShell else puts "Welcome to GitLab, #{username}!" end + rescue GitlabNet::ApiUnreachableError => ex + $logger.warn "gitlab-shell: Failed to connect to internal API" + puts "Failed to authorize your Git request: internal API unreachable" rescue DisallowedCommandError => ex message = "gitlab-shell: Attempt to execute disallowed command <#{@origin_cmd}> by #{log_username}." $logger.warn message - puts 'Not allowed command' + puts 'Disallowed command' end protected @@ -59,10 +63,6 @@ class GitlabShell exec_cmd(@git_cmd, repo_full_path) end - def validate_access - api.check_access(@git_cmd, @repo_name, @key_id, '_any').allowed? - end - # This method is not covered by Rspec because it ends the current Ruby process. def exec_cmd(*args) Kernel::exec({'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'GL_ID' => ENV['GL_ID']}, *args, unsetenv_others: true) @@ -73,11 +73,12 @@ class GitlabShell end def user - # Can't use "@user ||=" because that will keep hitting the API when @user is really nil! - if instance_variable_defined?('@user') - @user - else + return @user if defined?(@user) + + begin @user = api.discover(@key_id) + rescue GitlabNet::ApiUnreachableError + @user = nil end end diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index 13db4bd..4768c71 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -5,15 +5,56 @@ describe GitlabAccess do let(:repository_path) { "/home/git/repositories" } let(:repo_name) { 'dzaporozhets/gitlab-ci' } let(:repo_path) { File.join(repository_path, repo_name) + ".git" } - let(:gitlab_access) { GitlabAccess.new(repo_path, 'key-123', 'wow') } + let(:api) do + double(GitlabNet).tap do |api| + api.stub(check_access: GitAccessStatus.new(true)) + end + end + subject do + GitlabAccess.new(repo_path, 'key-123', 'wow').tap do |access| + access.stub(exec_cmd: :exec_called) + access.stub(api: api) + end + end before do GitlabConfig.any_instance.stub(repos_path: repository_path) end describe :initialize do - it { gitlab_access.repo_name.should == repo_name } - it { gitlab_access.repo_path.should == repo_path } - it { gitlab_access.changes.should == ['wow'] } + it { subject.repo_name.should == repo_name } + it { subject.repo_path.should == repo_path } + it { subject.changes.should == ['wow'] } + end + + describe "#exec" do + context "access is granted" do + + it "returns true" do + expect(subject.exec).to be_true + end + end + + context "access is denied" do + + before do + api.stub(check_access: GitAccessStatus.new(false)) + end + + it "returns false" do + expect(subject.exec).to be_false + end + end + + context "API connection fails" do + + before do + api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError) + end + + it "returns false" do + expect(subject.exec).to be_false + end + end end end diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 5e6e4de..17fb56d 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -26,6 +26,11 @@ describe GitlabNet, vcr: true do gitlab_net.check end end + + it "raises an exception if the connection fails" do + Net::HTTP.any_instance.stub(:request).and_raise(StandardError) + expect { gitlab_net.check }.to raise_error(GitlabNet::ApiUnreachableError) + end end describe :discover do @@ -42,6 +47,13 @@ describe GitlabNet, vcr: true do gitlab_net.discover('key-126') end end + + it "raises an exception if the connection fails" do + VCR.use_cassette("discover-ok") do + Net::HTTP.any_instance.stub(:request).and_raise(StandardError) + expect { gitlab_net.discover('key-126') }.to raise_error(GitlabNet::ApiUnreachableError) + end + end end describe :broadcast_message do @@ -110,6 +122,13 @@ describe GitlabNet, vcr: true do end end end + + it "raises an exception if the connection fails" do + Net::HTTP.any_instance.stub(:request).and_raise(StandardError) + expect { + gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes) + }.to raise_error(GitlabNet::ApiUnreachableError) + end end describe :host do @@ -139,12 +158,13 @@ describe GitlabNet, vcr: true do let(:user) { 'user' } let(:password) { 'password' } let(:url) { URI 'http://localhost/' } - subject { gitlab_net.send :http_request_for, url } + subject { gitlab_net.send :http_request_for, :get, url } before do gitlab_net.send(:config).http_settings.stub(:[]).with('user') { user } gitlab_net.send(:config).http_settings.stub(:[]).with('password') { password } get.should_receive(:basic_auth).with(user, password).once + get.should_receive(:set_form_data).with(hash_including(secret_token: 'a123')).once end it { should_not be_nil } diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 5df2391..4ca7984 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -135,6 +135,27 @@ describe GitlabShell do api.should_receive(:discover).with(key_id) end end + + context "failed connection" do + before { + ssh_cmd 'git-upload-pack gitlab-ci.git' + api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError) + } + after { subject.exec } + + it "should not process the command" do + subject.should_not_receive(:process_cmd) + end + + it "should not execute the command" do + subject.should_not_receive(:exec_cmd) + end + + it "should log the failed connection" do + message = "gitlab-shell: Failed to connect to internal API" + $logger.should_receive(:warn).with(message) + end + end end describe :validate_access do |