Merge branch 'remove-grack-lfs' into 'master'

Remove Grack::Auth: part 2 (LFS)

Deprecate Grack::Auth and handle LFS in Rails controllers under the Project namespace.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14501

See merge request !5369

app/controllers/projects/git_http_client_controller.rb

+# This file should be identical in GitLab Community Edition and Enterprise Edition
+
+class Projects::GitHttpClientController < Projects::ApplicationController
+  include ActionController::HttpAuthentication::Basic
+  include KerberosSpnegoHelper
+
+  attr_reader :user
+
+  # Git clients will not know what authenticity token to send along
+  skip_before_action :verify_authenticity_token
+  skip_before_action :repository
+  before_action :authenticate_user
+  before_action :ensure_project_found!
+
+  private
+
+  def authenticate_user
+    if project && project.public? && download_request?
+      return # Allow access
+    end
+
+    if allow_basic_auth? && basic_auth_provided?
+      login, password = user_name_and_password(request)
+      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
+
+      if auth_result.type == :ci && download_request?
+        @ci = true
+      elsif auth_result.type == :oauth && !download_request?
+        # Not allowed
+      else
+        @user = auth_result.user
+      end
+
+      if ci? || user
+        return # Allow access
+      end
+    elsif allow_kerberos_spnego_auth? && spnego_provided?
+      @user = find_kerberos_user
+
+      if user
+        send_final_spnego_response
+        return # Allow access
+      end
+    end
+
+    send_challenges
+    render plain: "HTTP Basic: Access denied\n", status: 401
+  end
+
+  def basic_auth_provided?
+    has_basic_credentials?(request)
+  end
+
+  def send_challenges
+    challenges = []
+    challenges << 'Basic realm="GitLab"' if allow_basic_auth?
+    challenges << spnego_challenge if allow_kerberos_spnego_auth?
+    headers['Www-Authenticate'] = challenges.join("\n") if challenges.any?
+  end
+
+  def ensure_project_found!
+    render_not_found if project.blank?
+  end
+
+  def project
+    return @project if defined?(@project)
+
+    project_id, _ = project_id_with_suffix
+    if project_id.blank?
+      @project = nil
+    else
+      @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}")
+    end
+  end
+
+  # This method returns two values so that we can parse
+  # params[:project_id] (untrusted input!) in exactly one place.
+  def project_id_with_suffix
+    id = params[:project_id] || ''
+
+    %w[.wiki.git .git].each do |suffix|
+      if id.end_with?(suffix)
+        # Be careful to only remove the suffix from the end of 'id'.
+        # Accidentally removing it from the middle is how security
+        # vulnerabilities happen!
+        return [id.slice(0, id.length - suffix.length), suffix]
+      end
+    end
+
+    # Something is wrong with params[:project_id]; do not pass it on.
+    [nil, nil]
+  end
+
+  def repository
+    _, suffix = project_id_with_suffix
+    if suffix == '.wiki.git'
+      project.wiki.repository
+    else
+      project.repository
+    end
+  end
+
+  def render_not_found
+    render plain: 'Not Found', status: :not_found
+  end
+
+  def ci?
+    @ci.present?
+  end
+end

app/controllers/projects/git_http_controller.rb

 # This file should be identical in GitLab Community Edition and Enterprise Edition
 
-class Projects::GitHttpController < Projects::ApplicationController
-  include ActionController::HttpAuthentication::Basic
-  include KerberosSpnegoHelper
-
-  attr_reader :user
-
-  # Git clients will not know what authenticity token to send along
-  skip_before_action :verify_authenticity_token
-  skip_before_action :repository
-  before_action :authenticate_user
-  before_action :ensure_project_found!
-
+class Projects::GitHttpController < Projects::GitHttpClientController
   # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
   # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
   def info_refs
@@ -46,81 +35,8 @@ class Projects::GitHttpController < Projects::ApplicationController
 
   private
 
-  def authenticate_user
-    if project && project.public? && upload_pack?
-      return # Allow access
-    end
-
-    if allow_basic_auth? && basic_auth_provided?
-      login, password = user_name_and_password(request)
-      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
-
-      if auth_result.type == :ci && upload_pack?
-        @ci = true
-      elsif auth_result.type == :oauth && !upload_pack?
-        # Not allowed
-      else
-        @user = auth_result.user
-      end
-
-      if ci? || user
-        return # Allow access
-      end
-    elsif allow_kerberos_spnego_auth? && spnego_provided?
-      @user = find_kerberos_user
-
-      if user
-        send_final_spnego_response
-        return # Allow access
-      end
-    end
-
-    send_challenges
-    render plain: "HTTP Basic: Access denied\n", status: 401
-  end
-
-  def basic_auth_provided?
-    has_basic_credentials?(request)
-  end
-
-  def send_challenges
-    challenges = []
-    challenges << 'Basic realm="GitLab"' if allow_basic_auth?
-    challenges << spnego_challenge if allow_kerberos_spnego_auth?
-    headers['Www-Authenticate'] = challenges.join("\n") if challenges.any?
-  end
-
-  def ensure_project_found!
-    render_not_found if project.blank?
-  end
-
-  def project
-    return @project if defined?(@project)
-
-    project_id, _ = project_id_with_suffix
-    if project_id.blank?
-      @project = nil
-    else
-      @project = Project.find_with_namespace("#{params[:namespace_id]}/#{project_id}")
-    end
-  end
-
-  # This method returns two values so that we can parse
-  # params[:project_id] (untrusted input!) in exactly one place.
-  def project_id_with_suffix
-    id = params[:project_id] || ''
-
-    %w[.wiki.git .git].each do |suffix|
-      if id.end_with?(suffix)
-        # Be careful to only remove the suffix from the end of 'id'.
-        # Accidentally removing it from the middle is how security
-        # vulnerabilities happen!
-        return [id.slice(0, id.length - suffix.length), suffix]
-      end
-    end
-
-    # Something is wrong with params[:project_id]; do not pass it on.
-    [nil, nil]
+  def download_request?
+    upload_pack?
   end
 
   def upload_pack?
@@ -143,19 +59,6 @@ class Projects::GitHttpController < Projects::ApplicationController
     render json: Gitlab::Workhorse.git_http_ok(repository, user)
   end
 
-  def repository
-    _, suffix = project_id_with_suffix
-    if suffix == '.wiki.git'
-      project.wiki.repository
-    else
-      project.repository
-    end
-  end
-
-  def render_not_found
-    render plain: 'Not Found', status: :not_found
-  end
-
   def render_http_not_allowed
     render plain: access_check.message, status: :forbidden
   end
@@ -169,10 +72,6 @@ class Projects::GitHttpController < Projects::ApplicationController
     end
   end
 
-  def ci?
-    @ci.present?
-  end
-
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 

app/controllers/projects/lfs_api_controller.rb

+class Projects::LfsApiController < Projects::GitHttpClientController
+  include LfsHelper
+
+  before_action :require_lfs_enabled!
+  before_action :lfs_check_access!, except: [:deprecated]
+
+  def batch
+    unless objects.present?
+      render_lfs_not_found
+      return
+    end
+
+    if download_request?
+      render json: { objects: download_objects! }
+    elsif upload_request?
+      render json: { objects: upload_objects! }
+    else
+      raise "Never reached"
+    end
+  end
+
+  def deprecated
+    render(
+      json: {
+        message: 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      status: 501
+    )
+  end
+
+  private
+
+  def objects
+    @objects ||= (params[:objects] || []).to_a
+  end
+
+  def existing_oids
+    @existing_oids ||= begin
+      storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
+    end
+  end
+
+  def download_objects!
+    objects.each do |object|
+      if existing_oids.include?(object[:oid])
+        object[:actions] = download_actions(object)
+      else
+        object[:error] = {
+          code: 404,
+          message: "Object does not exist on the server or you don't have permissions to access it",
+        }
+      end
+    end
+    objects
+  end
+
+  def upload_objects!
+    objects.each do |object|
+      object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid])
+    end
+    objects
+  end
+
+  def download_actions(object)
+    {
+      download: {
+        href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}",
+        header: {
+          Authorization: request.headers['Authorization']
+        }.compact
+      }
+    }
+  end
+
+  def upload_actions(object)
+    {
+      upload: {
+        href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}",
+        header: {
+          Authorization: request.headers['Authorization']
+        }.compact
+      }
+    }
+  end
+
+  def download_request?
+    params[:operation] == 'download'
+  end
+
+  def upload_request?
+    params[:operation] == 'upload'
+  end
+end

app/controllers/projects/lfs_storage_controller.rb

+class Projects::LfsStorageController < Projects::GitHttpClientController
+  include LfsHelper
+
+  before_action :require_lfs_enabled!
+  before_action :lfs_check_access!
+
+  def download
+    lfs_object = LfsObject.find_by_oid(oid)
+    unless lfs_object && lfs_object.file.exists?
+      render_lfs_not_found
+      return
+    end
+
+    send_file lfs_object.file.path, content_type: "application/octet-stream"
+  end
+
+  def upload_authorize
+    render(
+      json: {
+        StoreLFSPath: "#{Gitlab.config.lfs.storage_path}/tmp/upload",
+        LfsOid: oid,
+        LfsSize: size,
+      },
+      content_type: 'application/json; charset=utf-8'
+    )
+  end
+
+  def upload_finalize
+    unless tmp_filename
+      render_lfs_forbidden
+      return
+    end
+
+    if store_file(oid, size, tmp_filename)
+      head 200
+    else
+      render plain: 'Unprocessable entity', status: 422
+    end
+  end
+
+  private
+
+  def download_request?
+    action_name == 'download'
+  end
+
+  def upload_request?
+    %w[upload_authorize upload_finalize].include? action_name
+  end
+
+  def oid
+    params[:oid].to_s
+  end
+
+  def size
+    params[:size].to_i
+  end
+
+  def tmp_filename
+    name = request.headers['X-Gitlab-Lfs-Tmp']
+    return if name.include?('/')
+    return unless oid.present? && name.start_with?(oid)
+    name
+  end
+
+  def store_file(oid, size, tmp_file)
+    # Define tmp_file_path early because we use it in "ensure"
+    tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file)
+
+    object = LfsObject.find_or_create_by(oid: oid, size: size)
+    file_exists = object.file.exists? || move_tmp_file_to_storage(object, tmp_file_path)
+    file_exists && link_to_project(object)
+  ensure
+    FileUtils.rm_f(tmp_file_path)
+  end
+
+  def move_tmp_file_to_storage(object, path)
+    File.open(path) do |f|
+      object.file = f
+    end
+
+    object.file.store!
+    object.save
+  end
+
+  def link_to_project(object)
+    if object && !object.projects.exists?(storage_project.id)
+      object.projects << storage_project
+      object.save
+    end
+  end
+end

app/helpers/lfs_helper.rb

+module LfsHelper
+  def require_lfs_enabled!
+    return if Gitlab.config.lfs.enabled
+
+    render(
+      json: {
+        message: 'Git LFS is not enabled on this GitLab server, contact your admin.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      status: 501
+    )
+  end
+
+  def lfs_check_access!
+    return if download_request? && lfs_download_access?
+    return if upload_request? && lfs_upload_access?
+
+    if project.public? || (user && user.can?(:read_project, project))
+      render_lfs_forbidden
+    else
+      render_lfs_not_found
+    end
+  end
+
+  def lfs_download_access?
+    project.public? || ci? || (user && user.can?(:download_code, project))
+  end
+
+  def lfs_upload_access?
+    user && user.can?(:push_code, project)
+  end
+
+  def render_lfs_forbidden
+    render(
+      json: {
+        message: 'Access forbidden. Check your access level.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      content_type: "application/vnd.git-lfs+json",
+      status: 403
+    )
+  end
+
+  def render_lfs_not_found
+    render(
+      json: {
+        message: 'Not found.',
+        documentation_url: "#{Gitlab.config.gitlab.url}/help",
+      },
+      content_type: "application/vnd.git-lfs+json",
+      status: 404
+    )
+  end
+
+  def storage_project
+    @storage_project ||= begin
+      result = project
+
+      loop do
+        break unless result.forked?
+        result = result.forked_from_project
+      end
+
+      result
+    end
+  end
+end

config/initializers/5_backend.rb

-# GIT over HTTP
-require_dependency Rails.root.join('lib/gitlab/backend/grack_auth')
-
 # GIT over SSH
 require_dependency Rails.root.join('lib/gitlab/backend/shell')
 

config/initializers/mime_types.rb

@@ -12,3 +12,10 @@ Mime::Type.register_alias "text/html",  :md
 Mime::Type.register "video/mp4",  :mp4, [], [:m4v, :mov]
 Mime::Type.register "video/webm", :webm
 Mime::Type.register "video/ogg",  :ogv
+
+middlewares = Gitlab::Application.config.middleware
+middlewares.swap(ActionDispatch::ParamsParser, ActionDispatch::ParamsParser, {
+  Mime::Type.lookup('application/vnd.git-lfs+json') => lambda do |body|
+    ActiveSupport::JSON.decode(body)
+  end
+})

config/routes.rb

@@ -84,9 +84,6 @@ Rails.application.routes.draw do
   # Health check
   get 'health_check(/:checks)' => 'health_check#index', as: :health_check
 
-  # Enable Grack support (for LFS only)
-  mount Grack::AuthSpawner, at: '/', constraints: lambda { |request| /[-\/\w\.]+\.git\/(info\/lfs|gitlab-lfs)/.match(request.path_info) }, via: [:get, :post, :put]
-
   # Help
   get 'help'           => 'help#index'
   get 'help/shortcuts' => 'help#shortcuts'
@@ -482,11 +479,26 @@ Rails.application.routes.draw do
       end
 
       scope module: :projects do
-        # Git HTTP clients ('git clone' etc.)
         scope constraints: { id: /.+\.git/, format: nil } do
+          # Git HTTP clients ('git clone' etc.)
           get '/info/refs', to: 'git_http#info_refs'
           post '/git-upload-pack', to: 'git_http#git_upload_pack'
           post '/git-receive-pack', to: 'git_http#git_receive_pack'
+
+          # Git LFS API (metadata)
+          post '/info/lfs/objects/batch', to: 'lfs_api#batch'
+          post '/info/lfs/objects', to: 'lfs_api#deprecated'
+          get '/info/lfs/objects/*oid', to: 'lfs_api#deprecated'
+
+          # GitLab LFS object storage
+          scope constraints: { oid: /[a-f0-9]{64}/ } do
+            get '/gitlab-lfs/objects/*oid', to: 'lfs_storage#download'
+
+            scope constraints: { size: /[0-9]+/ } do
+              put '/gitlab-lfs/objects/*oid/*size/authorize', to: 'lfs_storage#upload_authorize'
+              put '/gitlab-lfs/objects/*oid/*size', to: 'lfs_storage#upload_finalize'
+            end
+          end
         end
 
         # Allow /info/refs, /info/refs?service=git-upload-pack, and

lib/gitlab/backend/grack_auth.rb

-module Grack
-  class AuthSpawner
-    def self.call(env)
-      # Avoid issues with instance variables in Grack::Auth persisting across
-      # requests by creating a new instance for each request.
-      Auth.new({}).call(env)
-    end
-  end
-
-  class Auth < Rack::Auth::Basic
-    attr_accessor :user, :project, :env
-
-    def call(env)
-      @env = env
-      @request = Rack::Request.new(env)
-      @auth = Request.new(env)
-
-      @ci = false
-
-      # Need this patch due to the rails mount
-      # Need this if under RELATIVE_URL_ROOT
-      unless Gitlab.config.gitlab.relative_url_root.empty?
-        # If website is mounted using relative_url_root need to remove it first
-        @env['PATH_INFO'] = @request.path.sub(Gitlab.config.gitlab.relative_url_root, '')
-      else
-        @env['PATH_INFO'] = @request.path
-      end
-
-      @env['SCRIPT_NAME'] = ""
-
-      auth!
-
-      lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call
-      return lfs_response unless lfs_response.nil?
-
-      if @user.nil? && !@ci
-        unauthorized
-      else
-        render_not_found
-      end
-    end
-
-    private
-
-    def auth!
-      return unless @auth.provided?
-
-      return bad_request unless @auth.basic?
-
-      # Authentication with username and password
-      login, password = @auth.credentials
-
-      # Allow authentication for GitLab CI service
-      # if valid token passed
-      if ci_request?(login, password)
-        @ci = true
-        return
-      end
-
-      @user = authenticate_user(login, password)
-    end
-
-    def ci_request?(login, password)
-      matched_login = /(?<s>^[a-zA-Z]*-ci)-token$/.match(login)
-
-      if project && matched_login.present?
-        underscored_service = matched_login['s'].underscore
-
-        if underscored_service == 'gitlab_ci'
-          return project && project.valid_build_token?(password)
-        elsif Service.available_services_names.include?(underscored_service)
-          service_method = "#{underscored_service}_service"
-          service = project.send(service_method)
-
-          return service && service.activated? && service.valid_token?(password)
-        end
-      end
-
-      false
-    end
-
-    def oauth_access_token_check(login, password)
-      if login == "oauth2" && git_cmd == 'git-upload-pack' && password.present?
-        token = Doorkeeper::AccessToken.by_token(password)
-        token && token.accessible? && User.find_by(id: token.resource_owner_id)
-      end
-    end
-
-    def authenticate_user(login, password)
-      user = Gitlab::Auth.find_with_user_password(login, password)
-
-      unless user
-        user = oauth_access_token_check(login, password)
-      end
-
-      # If the user authenticated successfully, we reset the auth failure count
-      # from Rack::Attack for that IP. A client may attempt to authenticate
-      # with a username and blank password first, and only after it receives
-      # a 401 error does it present a password. Resetting the count prevents
-      # false positives from occurring.
-      #
-      # Otherwise, we let Rack::Attack know there was a failed authentication
-      # attempt from this IP. This information is stored in the Rails cache
-      # (Redis) and will be used by the Rack::Attack middleware to decide
-      # whether to block requests from this IP.
-      config = Gitlab.config.rack_attack.git_basic_auth
-
-      if config.enabled
-        if user
-          # A successful login will reset the auth failure count from this IP
-          Rack::Attack::Allow2Ban.reset(@request.ip, config)
-        else
-          banned = Rack::Attack::Allow2Ban.filter(@request.ip, config) do
-            # Unless the IP is whitelisted, return true so that Allow2Ban
-            # increments the counter (stored in Rails.cache) for the IP
-            if config.ip_whitelist.include?(@request.ip)
-              false
-            else
-              true
-            end
-          end
-
-          if banned
-            Rails.logger.info "IP #{@request.ip} failed to login " \
-              "as #{login} but has been temporarily banned from Git auth"
-          end
-        end
-      end
-
-      user
-    end
-
-    def git_cmd
-      if @request.get?
-        @request.params['service']
-      elsif @request.post?
-        File.basename(@request.path)
-      else
-        nil
-      end
-    end
-
-    def project
-      return @project if defined?(@project)
-
-      @project = project_by_path(@request.path_info)
-    end
-
-    def project_by_path(path)
-      if m = /^([\w\.\/-]+)\.git/.match(path).to_a
-        path_with_namespace = m.last
-        path_with_namespace.gsub!(/\.wiki$/, '')
-
-        path_with_namespace[0] = '' if path_with_namespace.start_with?('/')
-        Project.find_with_namespace(path_with_namespace)
-      end
-    end
-
-    def render_not_found
-      [404, { "Content-Type" => "text/plain" }, ["Not Found"]]
-    end
-  end
-end

lib/gitlab/lfs/response.rb

-module Gitlab
-  module Lfs
-    class Response
-      def initialize(project, user, ci, request)
-        @origin_project = project
-        @project = storage_project(project)
-        @user = user
-        @ci = ci
-        @env = request.env
-        @request = request
-      end
-
-      def render_download_object_response(oid)
-        render_response_to_download do
-          if check_download_sendfile_header?
-            render_lfs_sendfile(oid)
-          else
-            render_not_found
-          end
-        end
-      end
-
-      def render_batch_operation_response
-        request_body = JSON.parse(@request.body.read)
-        case request_body["operation"]
-        when "download"
-          render_batch_download(request_body)
-        when "upload"
-          render_batch_upload(request_body)
-        else
-          render_not_found
-        end
-      end
-
-      def render_storage_upload_authorize_response(oid, size)
-        render_response_to_push do
-          [
-            200,
-            { "Content-Type" => "application/json; charset=utf-8" },
-            [JSON.dump({
-              'StoreLFSPath' => "#{Gitlab.config.lfs.storage_path}/tmp/upload",
-              'LfsOid' => oid,
-              'LfsSize' => size
-            })]
-          ]
-        end
-      end
-
-      def render_storage_upload_store_response(oid, size, tmp_file_name)
-        return render_forbidden unless tmp_file_name
-
-        render_response_to_push do
-          render_lfs_upload_ok(oid, size, tmp_file_name)
-        end
-      end
-
-      def render_unsupported_deprecated_api
-        [
-          501,
-          { "Content-Type" => "application/json; charset=utf-8" },
-          [JSON.dump({
-            'message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      private
-
-      def render_not_enabled
-        [
-          501,
-          {
-            "Content-Type" => "application/json; charset=utf-8",
-          },
-          [JSON.dump({
-            'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_unauthorized
-        [
-          401,
-          {
-            'Content-Type' => 'text/plain'
-          },
-          ['Unauthorized']
-        ]
-      end
-
-      def render_not_found
-        [
-          404,
-          {
-            "Content-Type" => "application/vnd.git-lfs+json"
-          },
-          [JSON.dump({
-            'message' => 'Not found.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_forbidden
-        [
-          403,
-          {
-            "Content-Type" => "application/vnd.git-lfs+json"
-          },
-          [JSON.dump({
-            'message' => 'Access forbidden. Check your access level.',
-            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
-          })]
-        ]
-      end
-
-      def render_lfs_sendfile(oid)
-        return render_not_found unless oid.present?
-
-        lfs_object = object_for_download(oid)
-
-        if lfs_object && lfs_object.file.exists?
-          [
-            200,
-            {
-              # GitLab-workhorse will forward Content-Type header
-              "Content-Type" => "application/octet-stream",
-              "X-Sendfile" => lfs_object.file.path
-            },
-            []
-          ]
-        else
-          render_not_found
-        end
-      end
-
-      def render_batch_upload(body)
-        return render_not_found if body.empty? || body['objects'].nil?
-
-        render_response_to_push do
-          response = build_upload_batch_response(body['objects'])
-          [
-            200,
-            {
-              "Content-Type" => "application/json; charset=utf-8",
-              "Cache-Control" => "private",
-            },
-            [JSON.dump(response)]
-          ]
-        end
-      end
-
-      def render_batch_download(body)
-        return render_not_found if body.empty? || body['objects'].nil?
-
-        render_response_to_download do
-          response = build_download_batch_response(body['objects'])
-          [
-            200,
-            {
-              "Content-Type" => "application/json; charset=utf-8",
-              "Cache-Control" => "private",
-            },
-            [JSON.dump(response)]
-          ]
-        end
-      end
-
-      def render_lfs_upload_ok(oid, size, tmp_file)
-        if store_file(oid, size, tmp_file)
-          [
-            200,
-            {
-              'Content-Type' => 'text/plain',
-              'Content-Length' => 0
-            },
-            []
-          ]
-        else
-          [
-            422,
-            { 'Content-Type' => 'text/plain' },
-            ["Unprocessable entity"]
-          ]
-        end
-      end
-
-      def render_response_to_download
-        return render_not_enabled unless Gitlab.config.lfs.enabled
-
-        unless @project.public?
-          return render_unauthorized unless @user || @ci
-          return render_forbidden unless user_can_fetch?
-        end
-
-        yield
-      end
-
-      def render_response_to_push
-        return render_not_enabled unless Gitlab.config.lfs.enabled
-        return render_unauthorized unless @user
-        return render_forbidden unless user_can_push?
-
-        yield
-      end
-
-      def check_download_sendfile_header?
-        @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile"
-      end
-
-      def user_can_fetch?
-        # Check user access against the project they used to initiate the pull
-        @ci || @user.can?(:download_code, @origin_project)
-      end
-
-      def user_can_push?
-        # Check user access against the project they used to initiate the push
-        @user.can?(:push_code, @origin_project)
-      end
-
-      def storage_project(project)
-        if project.forked?
-          storage_project(project.forked_from_project)
-        else
-          project
-        end
-      end
-
-      def store_file(oid, size, tmp_file)
-        tmp_file_path = File.join("#{Gitlab.config.lfs.storage_path}/tmp/upload", tmp_file)
-
-        object = LfsObject.find_or_create_by(oid: oid, size: size)
-        if object.file.exists?
-          success = true
-        else
-          success = move_tmp_file_to_storage(object, tmp_file_path)
-        end
-
-        if success
-          success = link_to_project(object)
-        end
-
-        success
-      ensure
-        # Ensure that the tmp file is removed
-        FileUtils.rm_f(tmp_file_path)
-      end
-
-      def object_for_download(oid)
-        @project.lfs_objects.find_by(oid: oid)
-      end
-
-      def move_tmp_file_to_storage(object, path)
-        File.open(path) do |f|
-          object.file = f
-        end
-
-        object.file.store!
-        object.save
-      end
-
-      def link_to_project(object)
-        if object && !object.projects.exists?(@project.id)
-          object.projects << @project
-          object.save
-        end
-      end
-
-      def select_existing_objects(objects)
-        objects_oids = objects.map { |o| o['oid'] }
-        @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set
-      end
-
-      def build_upload_batch_response(objects)
-        selected_objects = select_existing_objects(objects)
-
-        upload_hypermedia_links(objects, selected_objects)
-      end
-
-      def build_download_batch_response(objects)
-        selected_objects = select_existing_objects(objects)
-
-        download_hypermedia_links(objects, selected_objects)
-      end
-
-      def download_hypermedia_links(all_objects, existing_objects)
-        all_objects.each do |object|
-          if existing_objects.include?(object['oid'])
-            object['actions'] = {
-              'download' => {
-                'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}",
-                'header' => {
-                  'Authorization' => @env['HTTP_AUTHORIZATION']
-                }.compact
-              }
-            }
-          else
-            object['error'] = {
-              'code' => 404,
-              'message' => "Object does not exist on the server or you don't have permissions to access it",
-            }
-          end
-        end
-
-        { 'objects' => all_objects }
-      end
-
-      def upload_hypermedia_links(all_objects, existing_objects)
-        all_objects.each do |object|
-          # generate actions only for non-existing objects
-          next if existing_objects.include?(object['oid'])
-
-          object['actions'] = {
-            'upload' => {
-              'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}",
-              'header' => {
-                'Authorization' => @env['HTTP_AUTHORIZATION']
-              }.compact
-            }
-          }
-        end
-
-        { 'objects' => all_objects }
-      end
-    end
-  end
-end

lib/gitlab/lfs/router.rb

-module Gitlab
-  module Lfs
-    class Router
-      attr_reader :project, :user, :ci, :request
-
-      def initialize(project, user, ci, request)
-        @project = project
-        @user = user
-        @ci = ci
-        @env = request.env
-        @request = request
-      end
-
-      def try_call
-        return unless @request && @request.path.present?
-
-        case @request.request_method
-        when 'GET'
-          get_response
-        when 'POST'
-          post_response
-        when 'PUT'
-          put_response
-        else
-          nil
-        end
-      end
-
-      private
-
-      def get_response
-        path_match = @request.path.match(/\/(info\/lfs|gitlab-lfs)\/objects\/([0-9a-f]{64})$/)
-        return nil unless path_match
-
-        oid = path_match[2]
-        return nil unless oid
-
-        case path_match[1]
-        when "info/lfs"
-          lfs.render_unsupported_deprecated_api
-        when "gitlab-lfs"
-          lfs.render_download_object_response(oid)
-        else
-          nil
-        end
-      end
-
-      def post_response
-        post_path = @request.path.match(/\/info\/lfs\/objects(\/batch)?$/)
-        return nil unless post_path
-
-        # Check for Batch API
-        if post_path[0].ends_with?("/info/lfs/objects/batch")
-          lfs.render_batch_operation_response
-        elsif post_path[0].ends_with?("/info/lfs/objects")
-          lfs.render_unsupported_deprecated_api
-        else
-          nil
-        end
-      end
-
-      def put_response
-        object_match = @request.path.match(/\/gitlab-lfs\/objects\/([0-9a-f]{64})\/([0-9]+)(|\/authorize){1}$/)
-        return nil if object_match.nil?
-
-        oid = object_match[1]
-        size = object_match[2].try(:to_i)
-        return nil if oid.nil? || size.nil?
-
-        # GitLab-workhorse requests
-        # 1. Try to authorize the request
-        # 2. send a request with a header containing the name of the temporary file
-        if object_match[3] && object_match[3] == '/authorize'
-          lfs.render_storage_upload_authorize_response(oid, size)
-        else
-          tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP'])
-          lfs.render_storage_upload_store_response(oid, size, tmp_file_name)
-        end
-      end
-
-      def lfs
-        return unless @project
-
-        Gitlab::Lfs::Response.new(@project, @user, @ci, @request)
-      end
-
-      def sanitize_tmp_filename(name)
-        if name.present?
-          name.gsub!(/^.*(\\|\/)/, '')
-          name = name.match(/[0-9a-f]{73}/)
-          name[0] if name
-        else
-          nil
-        end
-      end
-    end
-  end
-end

spec/requests/lfs_http_spec.rb

 require 'spec_helper'
 
-describe Gitlab::Lfs::Router do
+describe 'Git LFS API and storage' do
   let(:user) { create(:user) }
   let!(:lfs_object) { create(:lfs_object, :with_file) }
 
@@ -31,10 +31,11 @@ describe Gitlab::Lfs::Router do
         'operation' => 'upload'
       }
     end
+    let(:authorization) { authorize_user }
 
     before do
       allow(Gitlab.config.lfs).to receive(:enabled).and_return(false)
-      post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
+      post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
     end
 
     it 'responds with 501' do
@@ -71,8 +72,9 @@ describe Gitlab::Lfs::Router do
     end
 
     context 'when handling lfs request using deprecated API' do
+      let(:authorization) { authorize_user }
       before do
-        post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers
+        post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers
       end
 
       it_behaves_like 'a deprecated'
@@ -118,8 +120,8 @@ describe Gitlab::Lfs::Router do
               project.lfs_objects << lfs_object
             end
 
-            it 'responds with status 403' do
-              expect(response).to have_http_status(403)
+            it 'responds with status 404' do
+              expect(response).to have_http_status(404)
             end
           end
 
@@ -147,8 +149,8 @@ describe Gitlab::Lfs::Router do
       context 'without required headers' do
         let(:authorization) { authorize_user }
 
-        it 'responds with status 403' do
-          expect(response).to have_http_status(403)
+        it 'responds with status 404' do
+          expect(response).to have_http_status(404)
         end
       end
     end
@@ -162,7 +164,7 @@ describe Gitlab::Lfs::Router do
       enable_lfs
       update_lfs_permissions
       update_user_permissions
-      post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
+      post_lfs_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers
     end
 
     describe 'download' do
@@ -304,10 +306,10 @@ describe Gitlab::Lfs::Router do
         end
 
         context 'when user does is not member of the project' do
-          let(:role) { :guest }
+          let(:update_user_permissions) { nil }
 
-          it 'responds with 403' do
-            expect(response).to have_http_status(403)
+          it 'responds with 404' do
+            expect(response).to have_http_status(404)
           end
         end
 
@@ -510,6 +512,7 @@ describe Gitlab::Lfs::Router do
 
     describe 'unsupported' do
       let(:project) { create(:empty_project) }
+      let(:authorization) { authorize_user }
       let(:body) do
         { 'operation' => 'other',
           'objects' => [
@@ -553,11 +556,11 @@ describe Gitlab::Lfs::Router do
 
       context 'and request is sent with a malformed headers' do
         before do
-          put_finalize('cat /etc/passwd')
+          put_finalize('/etc/passwd')
         end
 
         it 'does not recognize it as a valid lfs command' do
-          expect(response).to have_http_status(403)
+          expect(response).to have_http_status(401)
         end
       end
     end
@@ -582,6 +585,16 @@ describe Gitlab::Lfs::Router do
           expect(response).to have_http_status(403)
         end
       end
+
+      context 'and request is sent with a malformed headers' do
+        before do
+          put_finalize('/etc/passwd')
+        end
+
+        it 'does not recognize it as a valid lfs command' do
+          expect(response).to have_http_status(403)
+        end
+      end
     end
 
     describe 'to one project' do
@@ -624,9 +637,25 @@ describe Gitlab::Lfs::Router do
               expect(lfs_object.projects.pluck(:id)).to include(project.id)
             end
           end
+
+          context 'invalid tempfiles' do
+            it 'rejects slashes in the tempfile name (path traversal' do
+              put_finalize('foo/bar')
+              expect(response).to have_http_status(403)
+            end
+
+            it 'rejects tempfile names that do not start with the oid' do
+              put_finalize("foo#{sample_oid}")
+              expect(response).to have_http_status(403)
+            end
+          end
         end
 
         describe 'and user does not have push access' do
+          before do
+            project.team << [user, :reporter]
+          end
+
           it_behaves_like 'forbidden'
         end
       end
@@ -758,8 +787,8 @@ describe Gitlab::Lfs::Router do
     Projects::ForkService.new(project, user, {}).execute
   end
 
-  def post_json(url, body = nil, headers = nil)
-    post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/json'))
+  def post_lfs_json(url, body = nil, headers = nil)
+    post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/vnd.git-lfs+json'))
   end
 
   def json_response