diff options
author | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2014-11-20 11:06:27 +0100 |
---|---|---|
committer | Jacob Vosmaer <contact@jacobvosmaer.nl> | 2014-11-20 11:06:27 +0100 |
commit | f8453da5868dd7a23d0f2f3da7a45e33c441d1db (patch) | |
tree | 4287dcaab5a9b0e8be14e82104a109ca9349b81a | |
parent | 11311a95545f967a5736cd16ab5fc37f7e658519 (diff) | |
download | gitlab-shell-f8453da5868dd7a23d0f2f3da7a45e33c441d1db.tar.gz |
Revert "Merge branch 'git_hook_messages'"
At least the following things were broken:
- missing require for 'gitlab_access_status' in lib/gitlab_net.rb
- gitlabhq master internal API returns 'true' or 'false', gitlab-shell
expects JSON
This reverts commit 11311a95545f967a5736cd16ab5fc37f7e658519, reversing
changes made to 45444597aef3e434571de2491934ae92357ad231.
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rw-r--r-- | lib/gitlab_access.rb | 10 | ||||
-rw-r--r-- | lib/gitlab_access_status.rb | 20 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 8 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 2 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 27 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 11 | ||||
-rw-r--r-- | spec/vcr_cassettes/allowed-pull.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/allowed-push.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/denied-pull.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/denied-push-with-user.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/denied-push.yml | 2 |
13 files changed, 31 insertions, 62 deletions
@@ -1,6 +1,3 @@ -v2.4.0 - - Show error message when git access is rejected - v2.2.0 - Support for custom hooks (Drew Blessing and Jose Kahan) @@ -1 +1 @@ -2.4.0 +2.3.1 diff --git a/lib/gitlab_access.rb b/lib/gitlab_access.rb index 547b81d..78d353c 100644 --- a/lib/gitlab_access.rb +++ b/lib/gitlab_access.rb @@ -1,6 +1,5 @@ require_relative 'gitlab_init' require_relative 'gitlab_net' -require_relative 'gitlab_access_status' require_relative 'names_helper' require 'json' @@ -18,14 +17,13 @@ class GitlabAccess end def exec - status = api.check_access('git-receive-pack', @repo_name, @actor, @changes) - if status.allowed? - true + if api.allowed?('git-receive-pack', @repo_name, @actor, @changes) + return true else # reset GL_ID env since we stop git push here ENV['GL_ID'] = nil - puts "GitLab: #{status.message}" - false + puts "GitLab: You are not allowed to access some of the refs!" + return false end end diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb deleted file mode 100644 index 597fcbb..0000000 --- a/lib/gitlab_access_status.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'json' - -class GitAccessStatus - attr_accessor :status, :message - alias_method :allowed?, :status - - def initialize(status, message = '') - @status = status - @message = message - end - - def self.create_from_json(json) - values = JSON.parse(json) - self.new(values["status"], values["message"]) - end - - def to_json - {status: @status, message: @message}.to_json - end -end
\ No newline at end of file diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 1f27398..e6478ef 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -6,7 +6,7 @@ require_relative 'gitlab_config' require_relative 'gitlab_logger' class GitlabNet - def check_access(cmd, repo, actor, changes) + def allowed?(cmd, repo, actor, changes) project_name = repo.gsub("'", "") project_name = project_name.gsub(/\.git\Z/, "") project_name = project_name.gsub(/\A\//, "") @@ -26,11 +26,7 @@ class GitlabNet url = "#{host}/allowed" resp = post(url, params) - if resp.code == '200' - GitAccessStatus.create_from_json(resp.body) - else - GitAccessStatus.new(false, "API is not accesible") - end + !!(resp.code == '200' && resp.body == 'true') end def discover(key) diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 95fad9e..e2cb2cc 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -60,7 +60,7 @@ class GitlabShell end def validate_access - api.check_access(@git_cmd, @repo_name, @key_id, '_any').allowed? + api.allowed?(@git_cmd, @repo_name, @key_id, '_any') end # This method is not covered by Rspec because it ends the current Ruby process. diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index d431ac7..9ccabe0 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -1,6 +1,5 @@ require_relative 'spec_helper' require_relative '../lib/gitlab_net' -require_relative '../lib/gitlab_access_status' describe GitlabNet, vcr: true do @@ -44,26 +43,26 @@ describe GitlabNet, vcr: true do end end - describe :check_access do + describe :allowed? do context 'ssh key with access to project' do it 'should allow pull access for dev.gitlab.org' do VCR.use_cassette("allowed-pull") do - access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes) - access.allowed?.should be_true + access = gitlab_net.allowed?('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes) + access.should be_true end end - it 'adds the secret_token to the request' do + it 'adds the secret_token theo request' do VCR.use_cassette("allowed-pull") do Net::HTTP::Post.any_instance.should_receive(:set_form_data).with(hash_including(secret_token: 'a123')) - gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes) + gitlab_net.allowed?('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes) end end it 'should allow push access for dev.gitlab.org' do VCR.use_cassette("allowed-push") do - access = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'key-126', changes) - access.allowed?.should be_true + access = gitlab_net.allowed?('git-upload-pack', 'gitlab/gitlabhq.git', 'key-126', changes) + access.should be_true end end end @@ -71,22 +70,22 @@ describe GitlabNet, vcr: true do context 'ssh key without access to project' do it 'should deny pull access for dev.gitlab.org' do VCR.use_cassette("denied-pull") do - access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes) - access.allowed?.should be_false + access = gitlab_net.allowed?('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes) + access.should be_false end end it 'should deny push access for dev.gitlab.org' do VCR.use_cassette("denied-push") do - access = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'key-2', changes) - access.allowed?.should be_false + access = gitlab_net.allowed?('git-upload-pack', 'gitlab/gitlabhq.git', 'key-2', changes) + access.should be_false end end it 'should deny push access for dev.gitlab.org (with user)' do VCR.use_cassette("denied-push-with-user") do - access = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes) - access.allowed?.should be_false + access = gitlab_net.allowed?('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes) + access.should be_false end end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 5df2391..4741303 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -1,6 +1,5 @@ require_relative 'spec_helper' require_relative '../lib/gitlab_shell' -require_relative '../lib/gitlab_access_status' describe GitlabShell do subject do @@ -13,7 +12,7 @@ describe GitlabShell do let(:api) do double(GitlabNet).tap do |api| api.stub(discover: { 'name' => 'John Doe' }) - api.stub(check_access: GitAccessStatus.new(true)) + api.stub(allowed?: true) end end let(:key_id) { "key-#{rand(100) + 100}" } @@ -141,13 +140,13 @@ describe GitlabShell do before { ssh_cmd 'git-upload-pack gitlab-ci.git' } after { subject.exec } - it "should call api.check_access" do - api.should_receive(:check_access). + it "should call api.allowed?" do + api.should_receive(:allowed?). with('git-upload-pack', 'gitlab-ci.git', key_id, '_any') end - it "should disallow access and log the attempt if check_access returns false status" do - api.stub(check_access: GitAccessStatus.new(false)) + it "should disallow access and log the attempt if allowed? returns false" do + api.stub(allowed?: false) message = "gitlab-shell: Access denied for git command <git-upload-pack gitlab-ci.git> " message << "by user with key #{key_id}." $logger.should_receive(:warn).with(message) diff --git a/spec/vcr_cassettes/allowed-pull.yml b/spec/vcr_cassettes/allowed-pull.yml index 5a10ec9..337b00f 100644 --- a/spec/vcr_cassettes/allowed-pull.yml +++ b/spec/vcr_cassettes/allowed-pull.yml @@ -42,7 +42,7 @@ http_interactions: - '0.089741' body: encoding: UTF-8 - string: '{"status": "true"}' + string: 'true' http_version: recorded_at: Wed, 03 Sep 2014 11:27:36 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/allowed-push.yml b/spec/vcr_cassettes/allowed-push.yml index a75c2db..cb757bf 100644 --- a/spec/vcr_cassettes/allowed-push.yml +++ b/spec/vcr_cassettes/allowed-push.yml @@ -42,7 +42,7 @@ http_interactions: - '0.833195' body: encoding: UTF-8 - string: '{"status": "true"}' + string: 'true' http_version: recorded_at: Wed, 03 Sep 2014 11:27:37 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/denied-pull.yml b/spec/vcr_cassettes/denied-pull.yml index 8535b4e..9941e70 100644 --- a/spec/vcr_cassettes/denied-pull.yml +++ b/spec/vcr_cassettes/denied-pull.yml @@ -40,7 +40,7 @@ http_interactions: - '0.028027' body: encoding: UTF-8 - string: '{"status": false, "message":"404 Not found"}' + string: '{"message":"404 Not found"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:38 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/denied-push-with-user.yml b/spec/vcr_cassettes/denied-push-with-user.yml index 101a868..4694797 100644 --- a/spec/vcr_cassettes/denied-push-with-user.yml +++ b/spec/vcr_cassettes/denied-push-with-user.yml @@ -42,7 +42,7 @@ http_interactions: - '0.019640' body: encoding: UTF-8 - string: '{"status": false}' + string: 'false' http_version: recorded_at: Wed, 03 Sep 2014 11:27:39 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/denied-push.yml b/spec/vcr_cassettes/denied-push.yml index 53ccc57..fc0a309 100644 --- a/spec/vcr_cassettes/denied-push.yml +++ b/spec/vcr_cassettes/denied-push.yml @@ -40,7 +40,7 @@ http_interactions: - '0.015198' body: encoding: UTF-8 - string: '{"status": false, "message":"404 Not found"}' + string: '{"message":"404 Not found"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:38 GMT recorded_with: VCR 2.4.0 |