Add some upload specs

app/controllers/projects/git_http_controller.rb

@@ -5,10 +5,12 @@ class Projects::GitHttpController < Projects::ApplicationController
     
   def git_rpc
     if upload_pack? && upload_pack_allowed?
-      render_ok and return
+      render_ok
+    elsif receive_pack? && receive_pack_allowed?
+      render_ok
+    else
+      render_not_found
     end
-    
-    render_not_found
   end
   
   %i{info_refs git_receive_pack git_upload_pack}.each do |method|
@@ -30,7 +32,7 @@ class Projects::GitHttpController < Projects::ApplicationController
   end
 
   def project_found?
-    render_not_found if project.nil?
+    render_not_found if project.blank?
   end
 
   def ci_request?(login, password)
@@ -124,13 +126,21 @@ class Projects::GitHttpController < Projects::ApplicationController
   end
 
   def upload_pack?
-    if action_name == 'info_refs'
-      params[:service] == 'git-upload-pack'
-    else
-      action_name == 'git_upload_pack'
-    end
+    rpc == 'git-upload-pack'
   end
 
+  def receive_pack?
+    rpc == 'git-receive-pack'
+  end
+
+  def rpc
+    if action_name == 'info_refs'
+      params[:service]
+    else
+      action_name.gsub('_', '-')
+    end
+  end
+    
   def render_ok
     render json: {
       'GL_ID' => Gitlab::ShellEnv.gl_id(@user),
@@ -164,4 +174,16 @@ class Projects::GitHttpController < Projects::ApplicationController
       false
     end
   end
+
+  def receive_pack_allowed?
+    if !Gitlab.config.gitlab_shell.receive_pack
+      false
+    elsif user
+      # Skip user authorization on upload request.
+      # It will be done by the pre-receive hook in the repository.
+      true
+    else
+      false
+    end
+  end
 end

spec/requests/git_http_spec.rb

@@ -61,12 +61,38 @@ describe 'Git HTTP requests', lib: true do
         project.update_attribute(:visibility_level, Project::PUBLIC)
       end
 
-      it "responds with status 200" do
+      it "downloads get status 200" do
         download(path, env) do |response|
           expect(response.status).to eq(200)
         end
       end
 
+      it "uploads get status 401" do
+        upload(path, env) do |response|
+          expect(response.status).to eq(401)
+        end
+      end
+      
+      context "with correct credentials" do
+        let(:env) { { user: user.username, password: user.password } }
+
+        it "uploads get status 200 (because Git hooks do the real check)" do
+          upload(path, env) do |response|
+            expect(response.status).to eq(200)
+          end
+        end
+        
+        context 'but git-receive-pack is disabled' do
+          it "responds with status 404" do
+            allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false)
+  
+            upload(path, env) do |response|
+              expect(response.status).to eq(404)
+            end
+          end
+        end
+      end
+
       context 'but git-upload-pack is disabled' do
         it "responds with status 404" do
           allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false)
@@ -133,13 +159,19 @@ describe 'Git HTTP requests', lib: true do
             end
 
             context "when the user isn't blocked" do
-              it "responds with status 200" do
+              it "downloads status 200" do
                 expect(Rack::Attack::Allow2Ban).to receive(:reset)
 
                 clone_get(path, env)
 
                 expect(response.status).to eq(200)
               end
+              
+              it "uploads get status 200" do
+                upload(path, env) do |response|
+                  expect(response.status).to eq(200)
+                end      
+              end
             end
 
             context "when blank password attempts follow a valid login" do
@@ -174,11 +206,17 @@ describe 'Git HTTP requests', lib: true do
           end
 
           context "when the user doesn't have access to the project" do
-            it "responds with status 404" do
+            it "downloads get status 404" do
               download(path, user: user.username, password: user.password) do |response|
                 expect(response.status).to eq(404)
               end
             end
+            
+            it "uploads get status 200 (because Git hooks do the real check)" do
+              upload(path, user: user.username, password: user.password) do |response|
+                expect(response.status).to eq(200)
+              end
+            end
           end
         end
       end
@@ -196,6 +234,7 @@ describe 'Git HTTP requests', lib: true do
       end
     end
   end
+
   def clone_get(project, options={})
     get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password))
   end
@@ -204,6 +243,14 @@ describe 'Git HTTP requests', lib: true do
     post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password))
   end
 
+  def push_get(project, options={})
+    get "/#{project}/info/refs", { service: 'git-receive-pack' }, auth_env(*options.values_at(:user, :password))
+  end
+
+  def push_post(project, options={})
+    post "/#{project}/git-receive-pack", {}, auth_env(*options.values_at(:user, :password))
+  end
+
   def download(project, user: nil, password: nil)
     args = [project, {user: user, password: password}]
 
@@ -214,6 +261,16 @@ describe 'Git HTTP requests', lib: true do
     yield response
   end
 
+  def upload(project, user: nil, password: nil)
+    args = [project, {user: user, password: password}]
+
+    push_get *args
+    yield response
+
+    push_post *args
+    yield response
+  end
+
   def auth_env(user, password)
     if user && password
       { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) }