diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-07-06 15:24:22 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-07-06 15:24:22 +0000 |
commit | dd6eed168d75030a028da4f1ba5b5f1483de395a (patch) | |
tree | ebcf8c577cf09061803be5dddf60a0abda04450f | |
parent | 5afdd3f1cc3f12d4e919edb554c755b227675d06 (diff) | |
parent | 02b8071c60f26318d660864450ca869cc8e6b7cf (diff) | |
download | gitlab-shell-dd6eed168d75030a028da4f1ba5b5f1483de395a.tar.gz |
Merge branch 'add-allowed-protocols-support' into 'master'
Allow GitLab Shell to check for allowed access based on the used Git protocol.
Needed for gitlab-org/gitlab-ce!4696 and gitlab-org/gitlab-ce#18601
See merge request !62
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | VERSION | 2 | ||||
-rwxr-xr-x | hooks/pre-receive | 3 | ||||
-rw-r--r-- | lib/gitlab_access.rb | 7 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 3 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 6 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 3 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 50 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/http-access-disabled.yml | 44 | ||||
-rw-r--r-- | spec/vcr_cassettes/ssh-access-disabled.yml | 44 |
11 files changed, 150 insertions, 17 deletions
@@ -1,3 +1,6 @@ +v3.2.0 + - Allow GitLab Shell to check for allowed access based on the used Git protocol + v3.1.0 - Refactor repository paths handling to allow multiple git mount points @@ -1 +1 @@ -3.1.0 +3.2.0 diff --git a/hooks/pre-receive b/hooks/pre-receive index 1f8a9d5..a4b2e32 100755 --- a/hooks/pre-receive +++ b/hooks/pre-receive @@ -5,12 +5,13 @@ refs = $stdin.read key_id = ENV['GL_ID'] +protocol = ENV['GL_PROTOCOL'] repo_path = Dir.pwd require_relative '../lib/gitlab_custom_hook' require_relative '../lib/gitlab_access' -if GitlabAccess.new(repo_path, key_id, refs).exec && +if GitlabAccess.new(repo_path, key_id, refs, protocol).exec && GitlabCustomHook.new.pre_receive(refs, repo_path) exit 0 else diff --git a/lib/gitlab_access.rb b/lib/gitlab_access.rb index 10afeef..04806b2 100644 --- a/lib/gitlab_access.rb +++ b/lib/gitlab_access.rb @@ -9,18 +9,19 @@ class GitlabAccess include NamesHelper - attr_reader :config, :repo_path, :repo_name, :changes + attr_reader :config, :repo_path, :repo_name, :changes, :protocol - def initialize(repo_path, actor, changes) + def initialize(repo_path, actor, changes, protocol) @config = GitlabConfig.new @repo_path = repo_path.strip @actor = actor @repo_name = extract_repo_name(@repo_path.dup) @changes = changes.lines + @protocol = protocol end def exec - status = api.check_access('git-receive-pack', @repo_name, @actor, @changes) + status = api.check_access('git-receive-pack', @repo_name, @actor, @changes, @protocol) raise AccessDeniedError, status.message unless status.allowed? diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index dd9a4b0..e10a07a 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -14,7 +14,7 @@ class GitlabNet CHECK_TIMEOUT = 5 READ_TIMEOUT = 300 - def check_access(cmd, repo, actor, changes) + def check_access(cmd, repo, actor, changes, protocol) project_name = repo.gsub("'", "") project_name = project_name.gsub(/\.git\Z/, "") project_name = project_name.gsub(/\A\//, "") @@ -24,6 +24,7 @@ class GitlabNet action: cmd, changes: changes, project: project_name, + protocol: protocol } if actor =~ /\Akey\-\d+\Z/ diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index c5d5c02..d9812ce 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -8,6 +8,7 @@ class GitlabShell class InvalidRepositoryPathError < StandardError; end GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-annex-shell git-lfs-authenticate).freeze + GL_PROTOCOL = 'ssh'.freeze attr_accessor :key_id, :repo_name, :git_cmd attr_reader :repo_path @@ -85,7 +86,7 @@ class GitlabShell end def verify_access - status = api.check_access(@git_access, @repo_name, @key_id, '_any') + status = api.check_access(@git_access, @repo_name, @key_id, '_any', GL_PROTOCOL) raise AccessDeniedError, status.message unless status.allowed? @@ -131,7 +132,8 @@ class GitlabShell 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'LANG' => ENV['LANG'], - 'GL_ID' => @key_id + 'GL_ID' => @key_id, + 'GL_PROTOCOL' => GL_PROTOCOL } if @config.git_annex_enabled? diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index 98848ae..2781aa9 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -11,7 +11,7 @@ describe GitlabAccess do end end subject do - GitlabAccess.new(repo_path, 'key-123', 'wow').tap do |access| + GitlabAccess.new(repo_path, 'key-123', 'wow', 'ssh').tap do |access| access.stub(exec_cmd: :exec_called) access.stub(api: api) end @@ -25,6 +25,7 @@ describe GitlabAccess do it { subject.repo_name.should == repo_name } it { subject.repo_path.should == repo_path } it { subject.changes.should == ['wow'] } + it { subject.protocol.should == 'ssh' } end describe "#exec" do diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index 0643868..2bbf98b 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -110,7 +110,7 @@ describe GitlabNet, vcr: true 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 = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes, 'ssh') access.allowed?.should be_true end end @@ -118,36 +118,72 @@ describe GitlabNet, vcr: true do it 'adds the secret_token to the 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.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes, 'ssh') 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 = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'key-126', changes, 'ssh') access.allowed?.should be_true end end end + context 'ssh access has been disabled' do + it 'should deny pull access for dev.gitlab.org' do + VCR.use_cassette('ssh-access-disabled') do + access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'ssh') + access.allowed?.should be_false + access.message.should eq 'Git access over SSH is not allowed' + end + end + + it 'should deny pull access for dev.gitlab.org' do + VCR.use_cassette('ssh-access-disabled') do + access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'ssh') + access.allowed?.should be_false + access.message.should eq 'Git access over SSH is not allowed' + end + end + end + + context 'http access has been disabled' do + it 'should deny pull access for dev.gitlab.org' do + VCR.use_cassette('http-access-disabled') do + access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'http') + access.allowed?.should be_false + access.message.should eq 'Git access over HTTP is not allowed' + end + end + + it 'should deny pull access for dev.gitlab.org' do + VCR.use_cassette('http-access-disabled') do + access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'http') + access.allowed?.should be_false + access.message.should eq 'Git access over HTTP is not allowed' + end + end + end + 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 = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'ssh') access.allowed?.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 = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'key-2', changes, 'ssh') access.allowed?.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 = gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes, 'ssh') access.allowed?.should be_false end end @@ -156,7 +192,7 @@ describe GitlabNet, vcr: true do 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) + gitlab_net.check_access('git-upload-pack', 'gitlab/gitlabhq.git', 'user-1', changes, 'ssh') }.to raise_error(GitlabNet::ApiUnreachableError) end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 79fa49b..0b0a817 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -242,7 +242,7 @@ describe GitlabShell do after { subject.exec(ssh_cmd) } it "should call api.check_access" do - api.should_receive(:check_access).with('git-upload-pack', 'gitlab-ci.git', key_id, '_any') + api.should_receive(:check_access).with('git-upload-pack', 'gitlab-ci.git', key_id, '_any', 'ssh') end it "should disallow access and log the attempt if check_access returns false status" do diff --git a/spec/vcr_cassettes/http-access-disabled.yml b/spec/vcr_cassettes/http-access-disabled.yml new file mode 100644 index 0000000..36e27a9 --- /dev/null +++ b/spec/vcr_cassettes/http-access-disabled.yml @@ -0,0 +1,44 @@ +--- +http_interactions: +- request: + method: post + uri: https://dev.gitlab.org/api/v3/internal/allowed + body: + encoding: US-ASCII + string: action=git-receive-pack&changes=0000000000000000000000000000000000000000+92d0970eefd7acb6d548878925ce2208cfe2d2ec+refs%2Fheads%2Fbranch4&project=gitlab%2Fgitlabhq&protocol=http&key_id=2&secret_token=a123 + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Content-Type: + - application/x-www-form-urlencoded + response: + status: + code: 200 + message: OK + headers: + Cache-Control: + - no-cache + Content-Length: + - '30' + Content-Type: + - application/json + Date: + - Wed, 22 Jun 2016 01:03:41 GMT + Status: + - 200 OK + Vary: + - Origin + X-Request-Id: + - 55b7af2c-3559-41d2-b301-9b86ad1d8fac + X-Runtime: + - '2.280895' + body: + encoding: UTF-8 + string: '{"status": false, "message":"Git access over HTTP is not allowed"}' + http_version: + recorded_at: Wed, 22 Jun 2016 01:03:41 GMT +recorded_with: VCR 2.4.0
\ No newline at end of file diff --git a/spec/vcr_cassettes/ssh-access-disabled.yml b/spec/vcr_cassettes/ssh-access-disabled.yml new file mode 100644 index 0000000..656d0aa --- /dev/null +++ b/spec/vcr_cassettes/ssh-access-disabled.yml @@ -0,0 +1,44 @@ +--- +http_interactions: +- request: + method: post + uri: https://dev.gitlab.org/api/v3/internal/allowed + body: + encoding: US-ASCII + string: action=git-receive-pack&changes=0000000000000000000000000000000000000000+92d0970eefd7acb6d548878925ce2208cfe2d2ec+refs%2Fheads%2Fbranch4&project=gitlab%2Fgitlabhq&protocol=ssh&key_id=2&secret_token=a123 + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + Content-Type: + - application/x-www-form-urlencoded + response: + status: + code: 200 + message: OK + headers: + Cache-Control: + - no-cache + Content-Length: + - '30' + Content-Type: + - application/json + Date: + - Wed, 22 Jun 2016 01:01:41 GMT + Status: + - 200 OK + Vary: + - Origin + X-Request-Id: + - 55b7af2c-3559-41d2-b301-9b86ad1d8fac + X-Runtime: + - '2.280895' + body: + encoding: UTF-8 + string: '{"status": false, "message":"Git access over SSH is not allowed"}' + http_version: + recorded_at: Wed, 22 Jun 2016 01:01:41 GMT +recorded_with: VCR 2.4.0 |