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

11 files changed, 150 insertions, 17 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 58a9e39..31cda72 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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
 
diff --git a/VERSION b/VERSION
index fd2a018..944880f 100644
--- a/VERSION
+++ b/VERSION
@@ -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