Commit f8453da5 authored by Jacob Vosmaer's avatar Jacob Vosmaer

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 11311a95, reversing
changes made to 45444597.
parent 11311a95
v2.4.0
- Show error message when git access is rejected
v2.2.0
- Support for custom hooks (Drew Blessing and Jose Kahan)
......
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
......
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
......@@ -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)
......
......@@ -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.
......
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
......
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)
......
......@@ -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
......@@ -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
......@@ -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
......@@ -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
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment