Merge branch 'lfs-batch-download' into 'master'

Add support for batch download operation

- Drops Accept for all download requests,
- Allows to do batch download for public projects and non-authorized users
- Returns 501 for legacy API with message to upgrade the client

/cc @marin @jacobvosmaer @yorickpeterse 

See merge request !1842

3 files changed, 419 insertions, 283 deletions

diff --git a/lib/gitlab/lfs/response.rb b/lib/gitlab/lfs/response.rb
index 4202c786466..aceef53ba60 100644
--- a/lib/gitlab/lfs/response.rb
+++ b/lib/gitlab/lfs/response.rb
@@ -10,23 +10,9 @@ module Gitlab
         @request = request
       end
 
-      # Return a response for a download request
-      # Can be a response to:
-      # Request from a user to get the file
-      # Request from gitlab-workhorse which file to serve to the user
-      def render_download_hypermedia_response(oid)
-        render_response_to_download do
-          if check_download_accept_header?
-            render_lfs_download_hypermedia(oid)
-          else
-            render_not_found
-          end
-        end
-      end
-
       def render_download_object_response(oid)
         render_response_to_download do
-          if check_download_sendfile_header? && check_download_accept_header?
+          if check_download_sendfile_header?
             render_lfs_sendfile(oid)
           else
             render_not_found
@@ -34,20 +20,15 @@ module Gitlab
         end
       end
 
-      def render_lfs_api_auth
-        render_response_to_push do
-          request_body = JSON.parse(@request.body.read)
-          return render_not_found if request_body.empty? || request_body['objects'].empty?
-
-          response = build_response(request_body['objects'])
-          [
-            200,
-            {
-              "Content-Type" => "application/json; charset=utf-8",
-              "Cache-Control" => "private",
-            },
-            [JSON.dump(response)]
-          ]
+      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
 
@@ -71,13 +52,24 @@ module Gitlab
         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 0.6.0 and up.',
+            'documentation_url' => "#{Gitlab.config.gitlab.url}/help",
+          })]
+        ]
+      end
+
       private
 
       def render_not_enabled
         [
           501,
           {
-            "Content-Type" => "application/vnd.git-lfs+json",
+            "Content-Type" => "application/json; charset=utf-8",
           },
           [JSON.dump({
             'message' => 'Git LFS is not enabled on this GitLab server, contact your admin.',
@@ -142,18 +134,35 @@ module Gitlab
         end
       end
 
-      def render_lfs_download_hypermedia(oid)
-        return render_not_found unless oid.present?
+      def render_batch_upload(body)
+        return render_not_found if body.empty? || body['objects'].nil?
 
-        lfs_object = object_for_download(oid)
-        if lfs_object
+        render_response_to_push do
+          response = build_upload_batch_response(body['objects'])
           [
             200,
-            { "Content-Type" => "application/vnd.git-lfs+json" },
-            [JSON.dump(download_hypermedia(oid))]
+            {
+              "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)]
           ]
-        else
-          render_not_found
         end
       end
 
@@ -199,10 +208,6 @@ module Gitlab
         @env['HTTP_X_SENDFILE_TYPE'].to_s == "X-Sendfile"
       end
 
-      def check_download_accept_header?
-        @env['HTTP_ACCEPT'].to_s == "application/vnd.git-lfs+json; charset=utf-8"
-      end
-
       def user_can_fetch?
         # Check user access against the project they used to initiate the pull
         @user.can?(:download_code, @origin_project)
@@ -266,42 +271,56 @@ module Gitlab
         @project.lfs_objects.where(oid: objects_oids).pluck(:oid).to_set
       end
 
-      def build_response(objects)
+      def build_upload_batch_response(objects)
         selected_objects = select_existing_objects(objects)
 
-        upload_hypermedia(objects, selected_objects)
+        upload_hypermedia_links(objects, selected_objects)
       end
 
-      def download_hypermedia(oid)
-        {
-         '_links' => {
-           'download' =>
-             {
-              'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{oid}",
-              'header' => {
-                'Accept' => "application/vnd.git-lfs+json; charset=utf-8",
-                'Authorization' => @env['HTTP_AUTHORIZATION']
-              }.compact
-            }
-          }
-        }
+      def build_download_batch_response(objects)
+        selected_objects = select_existing_objects(objects)
+
+        download_hypermedia_links(objects, selected_objects)
       end
 
-      def upload_hypermedia(all_objects, existing_objects)
+      def download_hypermedia_links(all_objects, existing_objects)
         all_objects.each do |object|
-          object['_links'] = hypermedia_links(object) unless existing_objects.include?(object['oid'])
+          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 hypermedia_links(object)
-        {
-          "upload" => {
-            'href' => "#{@origin_project.http_url_to_repo}/gitlab-lfs/objects/#{object['oid']}/#{object['size']}",
-            'header' => { 'Authorization' => @env['HTTP_AUTHORIZATION'] }
-          }.compact
-        }
+      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
diff --git a/lib/gitlab/lfs/router.rb b/lib/gitlab/lfs/router.rb
index 4809e834984..78d02891102 100644
--- a/lib/gitlab/lfs/router.rb
+++ b/lib/gitlab/lfs/router.rb
@@ -34,7 +34,7 @@ module Gitlab
 
         case path_match[1]
         when "info/lfs"
-          lfs.render_download_hypermedia_response(oid)
+          lfs.render_unsupported_deprecated_api
         when "gitlab-lfs"
           lfs.render_download_object_response(oid)
         else
@@ -48,7 +48,9 @@ module Gitlab
 
         # Check for Batch API
         if post_path[0].ends_with?("/info/lfs/objects/batch")
-          lfs.render_lfs_api_auth
+          lfs.render_batch_operation_response
+        elsif post_path[0].ends_with?("/info/lfs/objects")
+          lfs.render_unsupported_deprecated_api
         else
           nil
         end
diff --git a/spec/lib/gitlab/lfs/lfs_router_spec.rb b/spec/lib/gitlab/lfs/lfs_router_spec.rb
index cebcb5bc887..92598559aea 100644
--- a/spec/lib/gitlab/lfs/lfs_router_spec.rb
+++ b/spec/lib/gitlab/lfs/lfs_router_spec.rb
@@ -26,113 +26,52 @@ describe Gitlab::Lfs::Router do
 
   let(:sample_oid) { "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a17f80" }
   let(:sample_size) { 499013 }
+  let(:respond_with_deprecated) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 0.6.0 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
+  let(:respond_with_disabled) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
 
   describe 'when lfs is disabled' do
     before do
       allow(Gitlab.config.lfs).to receive(:enabled).and_return(false)
-      env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/#{sample_oid}"
+      env['REQUEST_METHOD'] = 'POST'
+      body = {
+                'objects' => [
+                  { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                    'size' => 1575078
+                  },
+                  { 'oid' => sample_oid,
+                    'size' => sample_size
+                  }
+                ],
+                'operation' => 'upload'
+              }.to_json
+      env['rack.input'] = StringIO.new(body)
+      env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/batch"
     end
 
     it 'responds with 501' do
-      respond_with_disabled = [ 501,
-                                { "Content-Type"=>"application/vnd.git-lfs+json" },
-                                ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]
-                              ]
       expect(lfs_router_auth.try_call).to match_array(respond_with_disabled)
     end
   end
 
-  describe 'when fetching lfs object' do
+  describe 'when fetching lfs object using deprecated API' do
     before do
       enable_lfs
-      env['HTTP_ACCEPT'] = "application/vnd.git-lfs+json; charset=utf-8"
       env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/#{sample_oid}"
     end
 
-    describe 'when user is authenticated' do
-      context 'and user has project download access' do
-        before do
-          @auth = authorize(user)
-          env["HTTP_AUTHORIZATION"] = @auth
-          project.lfs_objects << lfs_object
-          project.team << [user, :master]
-        end
-
-        it "responds with status 200" do
-          expect(lfs_router_auth.try_call.first).to eq(200)
-        end
-
-        it "responds with download hypermedia" do
-          json_response = ActiveSupport::JSON.decode(lfs_router_auth.try_call.last.first)
-
-          expect(json_response['_links']['download']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}")
-          expect(json_response['_links']['download']['header']).to eq("Authorization" => @auth, "Accept" => "application/vnd.git-lfs+json; charset=utf-8")
-        end
-      end
-
-      context 'and user does not have project access' do
-        it "responds with status 403" do
-          expect(lfs_router_auth.try_call.first).to eq(403)
-        end
-      end
-    end
-
-    describe 'when user is unauthenticated' do
-      context 'and user does not have download access' do
-        it "responds with status 401" do
-          expect(lfs_router_noauth.try_call.first).to eq(401)
-        end
-      end
-
-      context 'and user has download access' do
-        before do
-          project.team << [user, :master]
-        end
-
-        it "responds with status 401" do
-          expect(lfs_router_noauth.try_call.first).to eq(401)
-        end
-      end
+    it 'responds with 501' do
+      expect(lfs_router_auth.try_call).to match_array(respond_with_deprecated)
     end
+  end
 
-    describe 'and project is public' do
-      context 'and project has access to the lfs object' do
-        before do
-          public_project.lfs_objects << lfs_object
-        end
-
-        context 'and user is authenticated' do
-          it "responds with status 200 and sends download hypermedia" do
-            expect(lfs_router_public_auth.try_call.first).to eq(200)
-            json_response = ActiveSupport::JSON.decode(lfs_router_public_auth.try_call.last.first)
-
-            expect(json_response['_links']['download']['href']).to eq("#{Gitlab.config.gitlab.url}/#{public_project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}")
-            expect(json_response['_links']['download']['header']).to eq("Accept" => "application/vnd.git-lfs+json; charset=utf-8")
-          end
-        end
-
-        context 'and user is unauthenticated' do
-          it "responds with status 200 and sends download hypermedia" do
-            expect(lfs_router_public_noauth.try_call.first).to eq(200)
-            json_response = ActiveSupport::JSON.decode(lfs_router_public_noauth.try_call.last.first)
-
-            expect(json_response['_links']['download']['href']).to eq("#{Gitlab.config.gitlab.url}/#{public_project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}")
-            expect(json_response['_links']['download']['header']).to eq("Accept" => "application/vnd.git-lfs+json; charset=utf-8")
-          end
-        end
-      end
-
-      context 'and project does not have access to the lfs object' do
-        it "responds with status 404" do
-          expect(lfs_router_public_auth.try_call.first).to eq(404)
-        end
-      end
+  describe 'when fetching lfs object' do
+    before do
+      enable_lfs
+      env['HTTP_ACCEPT'] = "application/vnd.git-lfs+json; charset=utf-8"
+      env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}"
     end
 
     describe 'and request comes from gitlab-workhorse' do
-      before do
-        env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}"
-      end
       context 'without user being authorized' do
         it "responds with status 401" do
           expect(lfs_router_noauth.try_call.first).to eq(401)
@@ -173,200 +112,376 @@ describe Gitlab::Lfs::Router do
         end
       end
     end
+  end
 
-    describe 'from a forked public project' do
-      before do
-        env['HTTP_ACCEPT'] = "application/vnd.git-lfs+json; charset=utf-8"
-        env["PATH_INFO"] = "#{forked_project.repository.path_with_namespace}.git/info/lfs/objects/#{sample_oid}"
-      end
+  describe 'when handling lfs request using deprecated API' do
+    before do
+      enable_lfs
+      env['REQUEST_METHOD'] = 'POST'
+      env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects"
+    end
+
+    it 'responds with 501' do
+      expect(lfs_router_auth.try_call).to match_array(respond_with_deprecated)
+    end
+  end
+
+  describe 'when handling lfs batch request' do
+    before do
+      enable_lfs
+      env['REQUEST_METHOD'] = 'POST'
+      env['PATH_INFO'] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/batch"
+    end
+
+    describe 'download' do
+      describe 'when user is authenticated' do
+        before do
+          body = { 'operation' => 'download',
+                   'objects' => [
+                     { 'oid' => sample_oid,
+                       'size' => sample_size
+                     }]
+          }.to_json
+          env['rack.input'] = StringIO.new(body)
+        end
 
-      context "when fetching a lfs object" do
-        context "and user has project download access" do
+        describe 'when user has download access' do
           before do
-            public_project.lfs_objects << lfs_object
+            @auth = authorize(user)
+            env["HTTP_AUTHORIZATION"] = @auth
+            project.team << [user, :reporter]
           end
 
-          it "can download the lfs object" do
-            expect(lfs_router_forked_auth.try_call.first).to eq(200)
-            json_response = ActiveSupport::JSON.decode(lfs_router_forked_auth.try_call.last.first)
+          context 'when downloading an lfs object that is assigned to our project' do
+            before do
+              project.lfs_objects << lfs_object
+            end
+
+            it 'responds with status 200 and href to download' do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+              response_body = ActiveSupport::JSON.decode(response.last.first)
 
-            expect(json_response['_links']['download']['href']).to eq("#{Gitlab.config.gitlab.url}/#{forked_project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}")
-            expect(json_response['_links']['download']['header']).to eq("Accept" => "application/vnd.git-lfs+json; charset=utf-8")
+              expect(response_body).to eq('objects' => [
+                { 'oid' => sample_oid,
+                  'size' => sample_size,
+                  'actions' => {
+                    'download' => {
+                      'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}",
+                      'header' => { 'Authorization' => @auth }
+                    }
+                  }
+                }])
+            end
           end
-        end
 
-        context "and user is not authenticated but project is public" do
-          before do
-            public_project.lfs_objects << lfs_object
+          context 'when downloading an lfs object that is assigned to other project' do
+            before do
+              public_project.lfs_objects << lfs_object
+            end
+
+            it 'responds with status 200 and error message' do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+              response_body = ActiveSupport::JSON.decode(response.last.first)
+
+              expect(response_body).to eq('objects' => [
+                { 'oid' => sample_oid,
+                  'size' => sample_size,
+                  'error' => {
+                    'code' => 404,
+                    'message' => "Object does not exist on the server or you don't have permissions to access it",
+                  }
+                }])
+            end
           end
 
-          it "can download the lfs object" do
-            expect(lfs_router_forked_auth.try_call.first).to eq(200)
+          context 'when downloading a lfs object that does not exist' do
+            before do
+              body = { 'operation' => 'download',
+                       'objects' => [
+                         { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                           'size' => 1575078
+                         }]
+              }.to_json
+              env['rack.input'] = StringIO.new(body)
+            end
+
+            it "responds with status 200 and error message" do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+              response_body = ActiveSupport::JSON.decode(response.last.first)
+
+              expect(response_body).to eq('objects' => [
+                { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                  'size' => 1575078,
+                  'error' => {
+                    'code' => 404,
+                    'message' => "Object does not exist on the server or you don't have permissions to access it",
+                  }
+                }])
+            end
+          end
+
+          context 'when downloading one new and one existing lfs object' do
+            before do
+              body = { 'operation' => 'download',
+                       'objects' => [
+                         { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                           'size' => 1575078
+                         },
+                         { 'oid' => sample_oid,
+                           'size' => sample_size
+                         }
+                       ]
+              }.to_json
+              env['rack.input'] = StringIO.new(body)
+              project.lfs_objects << lfs_object
+            end
+
+            it "responds with status 200 with upload hypermedia link for the new object" do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+              response_body = ActiveSupport::JSON.decode(response.last.first)
+
+              expect(response_body).to eq('objects' => [
+                { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                  'size' => 1575078,
+                  'error' => {
+                    'code' => 404,
+                    'message' => "Object does not exist on the server or you don't have permissions to access it",
+                  }
+                },
+                { 'oid' => sample_oid,
+                  'size' => sample_size,
+                  'actions' => {
+                    'download' => {
+                      'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}",
+                      'header' => { 'Authorization' => @auth }
+                    }
+                  }
+                }])
+            end
           end
         end
 
-        context "and user has project download access" do
+        context 'when user does is not member of the project' do
           before do
-            env["PATH_INFO"] = "#{forked_project.repository.path_with_namespace}.git/info/lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897"
-            @auth = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
+            @auth = authorize(user)
             env["HTTP_AUTHORIZATION"] = @auth
-            lfs_object_two = create(:lfs_object, :with_file, oid: "91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", size: 1575078)
-            public_project.lfs_objects << lfs_object_two
+            project.team << [user, :guest]
           end
 
-          it "can get a lfs object that is not in the forked project" do
-            expect(lfs_router_forked_auth.try_call.first).to eq(200)
-
-            json_response = ActiveSupport::JSON.decode(lfs_router_forked_auth.try_call.last.first)
-            expect(json_response['_links']['download']['href']).to eq("#{Gitlab.config.gitlab.url}/#{forked_project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
-            expect(json_response['_links']['download']['header']).to eq("Accept" => "application/vnd.git-lfs+json; charset=utf-8", "Authorization" => @auth)
+          it 'responds with 403' do
+            expect(lfs_router_auth.try_call.first).to eq(403)
           end
         end
 
-        context "and user has project download access" do
+        context 'when user does not have download access' do
           before do
-            env["PATH_INFO"] = "#{forked_project.repository.path_with_namespace}.git/info/lfs/objects/267c8b1d876743971e3a9978405818ff5ca731c4c870b06507619cd9b1847b6b"
-            lfs_object_three = create(:lfs_object, :with_file, oid: "267c8b1d876743971e3a9978405818ff5ca731c4c870b06507619cd9b1847b6b", size: 127192524)
-            project.lfs_objects << lfs_object_three
+            @auth = authorize(user)
+            env["HTTP_AUTHORIZATION"] = @auth
+            project.team << [user, :guest]
           end
 
-          it "cannot get a lfs object that is not in the project" do
-            expect(lfs_router_forked_auth.try_call.first).to eq(404)
+          it 'responds with 403' do
+            expect(lfs_router_auth.try_call.first).to eq(403)
           end
         end
       end
-    end
-  end
-
-  describe 'when initiating pushing of the lfs object' do
-    before do
-      enable_lfs
-      env['REQUEST_METHOD'] = 'POST'
-      env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/batch"
-    end
-
-    describe 'when user is authenticated' do
-      before do
-        body = { 'objects' => [{
-                   'oid' => sample_oid,
-                   'size' => sample_size
-                  }]
-                }.to_json
-        env['rack.input'] = StringIO.new(body)
-      end
 
-      describe 'when user has project push access' do
+      context 'when user is not authenticated' do
         before do
-          @auth = authorize(user)
-          env["HTTP_AUTHORIZATION"] = @auth
-          project.team << [user, :master]
+          body = { 'operation' => 'download',
+                   'objects' => [
+                     { 'oid' => sample_oid,
+                       'size' => sample_size
+                     }],
+
+          }.to_json
+          env['rack.input'] = StringIO.new(body)
         end
 
-        context 'when pushing an lfs object that already exists' do
+        describe 'is accessing public project' do
           before do
             public_project.lfs_objects << lfs_object
           end
 
-          it "responds with status 200 and links the object to the project" do
-            response_body = lfs_router_auth.try_call.last
-            response = ActiveSupport::JSON.decode(response_body.first)
+          it 'responds with status 200 and href to download' do
+            response = lfs_router_public_noauth.try_call
+            expect(response.first).to eq(200)
+            response_body = ActiveSupport::JSON.decode(response.last.first)
 
-            expect(response['objects']).to be_kind_of(Array)
-            expect(response['objects'].first['oid']).to eq(sample_oid)
-            expect(response['objects'].first['size']).to eq(sample_size)
-            expect(lfs_object.projects.pluck(:id)).to_not include(project.id)
-            expect(lfs_object.projects.pluck(:id)).to include(public_project.id)
-            expect(response['objects'].first).to have_key('_links')
+            expect(response_body).to eq('objects' => [
+              { 'oid' => sample_oid,
+                'size' => sample_size,
+                'actions' => {
+                  'download' => {
+                    'href' => "#{public_project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}",
+                    'header' => {}
+                  }
+                }
+              }])
           end
         end
 
-        context 'when pushing a lfs object that does not exist' do
+        describe 'is accessing non-public project' do
           before do
-            body = {
-              'objects' => [{
-                'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
-                'size' => 1575078
-                }]
-                }.to_json
-            env['rack.input'] = StringIO.new(body)
+            project.lfs_objects << lfs_object
           end
 
-          it "responds with status 200 and upload hypermedia link" do
-            response = lfs_router_auth.try_call
-            expect(response.first).to eq(200)
-
-            response_body = ActiveSupport::JSON.decode(response.last.first)
-            expect(response_body['objects']).to be_kind_of(Array)
-            expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
-            expect(response_body['objects'].first['size']).to eq(1575078)
-            expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
-            expect(response_body['objects'].first['_links']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078")
-            expect(response_body['objects'].first['_links']['upload']['header']).to eq("Authorization" => @auth)
+          it 'responds with authorization required' do
+            expect(lfs_router_noauth.try_call.first).to eq(401)
           end
         end
+      end
+    end
 
-        context 'when pushing one new and one existing lfs object' do
+    describe 'upload' do
+      describe 'when user is authenticated' do
+        before do
+          body = { 'operation' => 'upload',
+                   'objects' => [
+                     { 'oid' => sample_oid,
+                       'size' => sample_size
+                     }]
+          }.to_json
+          env['rack.input'] = StringIO.new(body)
+        end
+
+        describe 'when user has project push access' do
           before do
-            body = {
-              'objects' => [
-                { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
-                  'size' => 1575078
-                },
-                { 'oid' => sample_oid,
-                  'size' => sample_size
-                }
-              ]
-            }.to_json
-            env['rack.input'] = StringIO.new(body)
-            public_project.lfs_objects << lfs_object
+            @auth = authorize(user)
+            env["HTTP_AUTHORIZATION"] = @auth
+            project.team << [user, :developer]
           end
 
-          it "responds with status 200 with upload hypermedia link for the new object" do
-            response = lfs_router_auth.try_call
-            expect(response.first).to eq(200)
+          context 'when pushing an lfs object that already exists' do
+            before do
+              public_project.lfs_objects << lfs_object
+            end
 
-            response_body = ActiveSupport::JSON.decode(response.last.first)
-            expect(response_body['objects']).to be_kind_of(Array)
+            it "responds with status 200 and links the object to the project" do
+              response_body = lfs_router_auth.try_call.last
+              response = ActiveSupport::JSON.decode(response_body.first)
 
+              expect(response['objects']).to be_kind_of(Array)
+              expect(response['objects'].first['oid']).to eq(sample_oid)
+              expect(response['objects'].first['size']).to eq(sample_size)
+              expect(lfs_object.projects.pluck(:id)).to_not include(project.id)
+              expect(lfs_object.projects.pluck(:id)).to include(public_project.id)
+              expect(response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}")
+              expect(response['objects'].first['actions']['upload']['header']).to eq('Authorization' => @auth)
+            end
+          end
 
-            expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
-            expect(response_body['objects'].first['size']).to eq(1575078)
-            expect(response_body['objects'].first['_links']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078")
-            expect(response_body['objects'].first['_links']['upload']['header']).to eq("Authorization" => @auth)
+          context 'when pushing a lfs object that does not exist' do
+            before do
+              body = { 'operation' => 'upload',
+                       'objects' => [
+                         { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                           'size' => 1575078
+                         }]
+              }.to_json
+              env['rack.input'] = StringIO.new(body)
+            end
 
-            expect(response_body['objects'].last['oid']).to eq(sample_oid)
-            expect(response_body['objects'].last['size']).to eq(sample_size)
-            expect(lfs_object.projects.pluck(:id)).to_not include(project.id)
-            expect(lfs_object.projects.pluck(:id)).to include(public_project.id)
-            expect(response_body['objects'].last).to have_key('_links')
+            it "responds with status 200 and upload hypermedia link" do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+
+              response_body = ActiveSupport::JSON.decode(response.last.first)
+              expect(response_body['objects']).to be_kind_of(Array)
+              expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
+              expect(response_body['objects'].first['size']).to eq(1575078)
+              expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
+              expect(response_body['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078")
+              expect(response_body['objects'].first['actions']['upload']['header']).to eq('Authorization' => @auth)
+            end
+          end
+
+          context 'when pushing one new and one existing lfs object' do
+            before do
+              body = { 'operation' => 'upload',
+                       'objects' => [
+                         { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
+                           'size' => 1575078
+                         },
+                         { 'oid' => sample_oid,
+                           'size' => sample_size
+                         }
+                       ]
+              }.to_json
+              env['rack.input'] = StringIO.new(body)
+              project.lfs_objects << lfs_object
+            end
+
+            it "responds with status 200 with upload hypermedia link for the new object" do
+              response = lfs_router_auth.try_call
+              expect(response.first).to eq(200)
+
+              response_body = ActiveSupport::JSON.decode(response.last.first)
+              expect(response_body['objects']).to be_kind_of(Array)
+
+              expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
+              expect(response_body['objects'].first['size']).to eq(1575078)
+              expect(response_body['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078")
+              expect(response_body['objects'].first['actions']['upload']['header']).to eq("Authorization" => @auth)
+
+              expect(response_body['objects'].last['oid']).to eq(sample_oid)
+              expect(response_body['objects'].last['size']).to eq(sample_size)
+              expect(response_body['objects'].last).to_not have_key('actions')
+            end
           end
         end
-      end
 
-      context 'when user does not have push access' do
-        it 'responds with 403' do
-          expect(lfs_router_auth.try_call.first).to eq(403)
+        context 'when user does not have push access' do
+          it 'responds with 403' do
+            expect(lfs_router_auth.try_call.first).to eq(403)
+          end
         end
       end
-    end
 
-    context 'when user is not authenticated' do
-      context 'when user has push access' do
+      context 'when user is not authenticated' do
         before do
-          project.team << [user, :master]
+          env['rack.input'] = StringIO.new(
+            { 'objects' => [], 'operation' => 'upload' }.to_json
+          )
         end
 
-        it "responds with status 401" do
-          expect(lfs_router_public_noauth.try_call.first).to eq(401)
+        context 'when user has push access' do
+          before do
+            project.team << [user, :master]
+          end
+
+          it "responds with status 401" do
+            expect(lfs_router_public_noauth.try_call.first).to eq(401)
+          end
         end
-      end
 
-      context 'when user does not have push access' do
-        it "responds with status 401" do
-          expect(lfs_router_public_noauth.try_call.first).to eq(401)
+        context 'when user does not have push access' do
+          it "responds with status 401" do
+            expect(lfs_router_public_noauth.try_call.first).to eq(401)
+          end
         end
       end
     end
+
+    describe 'unsupported' do
+      before do
+        body = { 'operation' => 'other',
+                 'objects' => [
+                   { 'oid' => sample_oid,
+                     'size' => sample_size
+                   }]
+        }.to_json
+        env['rack.input'] = StringIO.new(body)
+      end
+
+      it 'responds with status 404' do
+        expect(lfs_router_public_noauth.try_call.first).to eq(404)
+      end
+    end
   end
 
   describe 'when pushing a lfs object' do