Merge branch 'git-refactoring' into 'master'

Git Refactoring

5 files changed, 101 insertions, 84 deletions

diff --git a/Procfile b/Procfile
index 9003369c938..18df7e78f9b 100644
--- a/Procfile
+++ b/Procfile
@@ -1,2 +1,2 @@
-web: bundle exec unicorn_rails -p $PORT -E development
+web: bundle exec unicorn_rails -p $PORT -E development -c config/unicorn_development.rb
 worker: bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default,gitlab_shell
diff --git a/config/unicorn_development.rb b/config/unicorn_development.rb
new file mode 100644
index 00000000000..94a7061451d
--- /dev/null
+++ b/config/unicorn_development.rb
@@ -0,0 +1,2 @@
+worker_processes 2
+timeout 30
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 69aad3748b3..bcf97574673 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -1,16 +1,12 @@
 module API
   # Internal access API
   class Internal < Grape::API
-
-    DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
-    PUSH_COMMANDS = %w{ git-receive-pack }
-
     namespace 'internal' do
-      #
-      # Check if ssh key has access to project code
+      # Check if git command is allowed to project
       #
       # Params:
-      #   key_id - SSH Key id
+      #   key_id - ssh key id for Git over SSH
+      #   user_id - user id for Git over HTTP
       #   project - project path with namespace
       #   action - git action (git-upload-pack or git-receive-pack)
       #   ref - branch name
@@ -22,43 +18,25 @@ module API
         # the wiki repository as well.
         project_path = params[:project]
         project_path.gsub!(/\.wiki/,'') if project_path =~ /\.wiki/
-
-        key = Key.find(params[:key_id])
         project = Project.find_with_namespace(project_path)
-        git_cmd = params[:action]
         return false unless project
 
-
-        if key.is_a? DeployKey
-          key.projects.include?(project) && DOWNLOAD_COMMANDS.include?(git_cmd)
-        else
-          user = key.user
-
-          return false if user.blocked?
-
-          if Gitlab.config.ldap.enabled
-            if user.ldap_user?
-              # Check if LDAP user exists and match LDAP user_filter
-              unless Gitlab::LDAP::Access.new.allowed?(user)
-                return false
-              end
-            end
-          end
-
-          action = case git_cmd
-                   when *DOWNLOAD_COMMANDS
-                     then :download_code
-                   when *PUSH_COMMANDS
-                     then
-                     if project.protected_branch?(params[:ref])
-                       :push_code_to_protected_branches
-                     else
-                       :push_code
-                     end
-                   end
-
-          user.can?(action, project)
-        end
+        actor = if params[:key_id]
+                  Key.find(params[:key_id])
+                elsif params[:user_id]
+                  User.find(params[:user_id])
+                end
+
+        return false unless actor
+
+        Gitlab::GitAccess.new.allowed?(
+          actor,
+          params[:action],
+          project,
+          params[:ref],
+          params[:oldrev],
+          params[:newrev]
+        )
       end
 
       #
diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb
index 60c03ce1c04..ee99bd5f7de 100644
--- a/lib/gitlab/backend/grack_auth.rb
+++ b/lib/gitlab/backend/grack_auth.rb
@@ -5,7 +5,7 @@ module Grack
   class Auth < Rack::Auth::Basic
     include Helpers
 
-    attr_accessor :user, :project, :ref, :env
+    attr_accessor :user, :project, :env
 
     def call(env)
       @env = env
@@ -80,24 +80,11 @@ module Grack
     def authorize_request(service)
       case service
       when 'git-upload-pack'
-        can?(user, :download_code, project)
-      when'git-receive-pack'
-        refs.each do |ref|
-          action = if project.protected_branch?(ref)
-                     :push_code_to_protected_branches
-                   else
-                     :push_code
-                   end
-
-          return false unless can?(user, action, project)
-        end
-
-        # Never let git-receive-pack trough unauthenticated; it's
-        # harmless but git < 1.8 doesn't like it
-        return false if user.nil?
-        true
+        # Serve only upload request.
+        # Authorization on push will be serverd by update hook in repository
+        Gitlab::GitAccess.new.download_allowed?(user, project)
       else
-        false
+        true
       end
     end
 
@@ -114,29 +101,5 @@ module Grack
     def project
       @project ||= project_by_path(@request.path_info)
     end
-
-    def refs
-      @refs ||= parse_refs
-    end
-
-    def parse_refs
-      input = if @env["HTTP_CONTENT_ENCODING"] =~ /gzip/
-                Zlib::GzipReader.new(@request.body).read
-              else
-                @request.body.read
-              end
-
-      # Need to reset seek point
-      @request.body.rewind
-
-      # Parse refs
-      refs = input.force_encoding('ascii-8bit').scan(/refs\/heads\/([\/\w\.-]+)/n).flatten.compact
-
-      # Cleanup grabare from refs
-      # if push to multiple branches
-      refs.map do |ref|
-        ref.gsub(/00.*/, "")
-      end
-    end
   end
 end
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
new file mode 100644
index 00000000000..5fb5505743f
--- /dev/null
+++ b/lib/gitlab/git_access.rb
@@ -0,0 +1,74 @@
+module Gitlab
+  class GitAccess
+    DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
+    PUSH_COMMANDS = %w{ git-receive-pack }
+
+    attr_reader :params, :project, :git_cmd, :user
+
+    def allowed?(actor, cmd, project, ref = nil, oldrev = nil, newrev = nil)
+      case cmd
+      when *DOWNLOAD_COMMANDS
+        if actor.is_a? User
+          download_allowed?(actor, project)
+        elsif actor.is_a? DeployKey
+          actor.projects.include?(project)
+        elsif actor.is_a? Key
+          download_allowed?(actor.user, project)
+        else
+          raise 'Wrong actor'
+        end
+      when *PUSH_COMMANDS
+        if actor.is_a? User
+          push_allowed?(actor, project, ref, oldrev, newrev)
+        elsif actor.is_a? DeployKey
+          # Deploy key not allowed to push
+          return false
+        elsif actor.is_a? Key
+          push_allowed?(actor.user, project, ref, oldrev, newrev)
+        else
+          raise 'Wrong actor'
+        end
+      else
+        false
+      end
+    end
+
+    def download_allowed?(user, project)
+      if user_allowed?(user)
+        user.can?(:download_code, project)
+      else
+        false
+      end
+    end
+
+    def push_allowed?(user, project, ref, oldrev, newrev)
+      if user_allowed?(user)
+        action = if project.protected_branch?(ref)
+                   :push_code_to_protected_branches
+                 else
+                   :push_code
+                 end
+        user.can?(action, project)
+      else
+        false
+      end
+    end
+
+    private
+
+    def user_allowed?(user)
+      return false if user.blocked?
+
+      if Gitlab.config.ldap.enabled
+        if user.ldap_user?
+          # Check if LDAP user exists and match LDAP user_filter
+          unless Gitlab::LDAP::Access.new.allowed?(user)
+            return false
+          end
+        end
+      end
+
+      true
+    end
+  end
+end