Merge branch 'ci-lfs-fetch' into 'master'

Allow to fetch LFS from CI

## What does this MR do?

This adds support for fetching LFS object from CI jobs (mostly it's made for supporting GitLab CI).

## What is left?

- [x] Write tests covering a new authorization mechanism

cc @grzesiek @marin

See merge request !4465

CHANGELOG

@@ -76,6 +76,7 @@ v 8.9.0 (unreleased)
   - Pipelines can be canceled only when there are running builds
   - Allow authentication using personal access tokens
   - Use downcased path to container repository as this is expected path by Docker
+  - Allow to use CI token to fetch LFS objects
   - Custom notification settings
   - Projects pending deletion will render a 404 page
   - Measure queue duration between gitlab-workhorse and Rails

lib/gitlab/backend/grack_auth.rb

@@ -31,7 +31,7 @@ module Grack
 
       auth!
 
-      lfs_response = Gitlab::Lfs::Router.new(project, @user, @request).try_call
+      lfs_response = Gitlab::Lfs::Router.new(project, @user, @ci, @request).try_call
       return lfs_response unless lfs_response.nil?
 
       if @user.nil? && !@ci

lib/gitlab/lfs/response.rb

@@ -2,10 +2,11 @@ module Gitlab
   module Lfs
     class Response
 
-      def initialize(project, user, request)
+      def initialize(project, user, ci, request)
         @origin_project = project
         @project = storage_project(project)
         @user = user
+        @ci = ci
         @env = request.env
         @request = request
       end
@@ -189,7 +190,7 @@ module Gitlab
         return render_not_enabled unless Gitlab.config.lfs.enabled
 
         unless @project.public?
-          return render_unauthorized unless @user
+          return render_unauthorized unless @user || @ci
           return render_forbidden unless user_can_fetch?
         end
 
@@ -210,7 +211,7 @@ module Gitlab
 
       def user_can_fetch?
         # Check user access against the project they used to initiate the pull
-        @user.can?(:download_code, @origin_project)
+        @ci || @user.can?(:download_code, @origin_project)
       end
 
       def user_can_push?

lib/gitlab/lfs/router.rb

 module Gitlab
   module Lfs
     class Router
-      def initialize(project, user, request)
+      attr_reader :project, :user, :ci, :request
+
+      def initialize(project, user, ci, request)
         @project = project
         @user = user
+        @ci = ci
         @env = request.env
         @request = request
       end
@@ -80,7 +83,7 @@ module Gitlab
       def lfs
         return unless @project
 
-        Gitlab::Lfs::Response.new(@project, @user, @request)
+        Gitlab::Lfs::Response.new(@project, @user, @ci, @request)
       end
 
       def sanitize_tmp_filename(name)

spec/lib/gitlab/lfs/lfs_router_spec.rb

@@ -17,12 +17,15 @@ describe Gitlab::Lfs::Router, lib: true do
     }
   end
 
-  let(:lfs_router_auth) { new_lfs_router(project, user) }
-  let(:lfs_router_noauth) { new_lfs_router(project, nil) }
-  let(:lfs_router_public_auth) { new_lfs_router(public_project, user) }
-  let(:lfs_router_public_noauth) { new_lfs_router(public_project, nil) }
-  let(:lfs_router_forked_noauth) { new_lfs_router(forked_project, nil) }
-  let(:lfs_router_forked_auth) { new_lfs_router(forked_project, user_two) }
+  let(:lfs_router_auth) { new_lfs_router(project, user: user) }
+  let(:lfs_router_ci_auth) { new_lfs_router(project, ci: true) }
+  let(:lfs_router_noauth) { new_lfs_router(project) }
+  let(:lfs_router_public_auth) { new_lfs_router(public_project, user: user) }
+  let(:lfs_router_public_ci_auth) { new_lfs_router(public_project, ci: true) }
+  let(:lfs_router_public_noauth) { new_lfs_router(public_project) }
+  let(:lfs_router_forked_noauth) { new_lfs_router(forked_project) }
+  let(:lfs_router_forked_auth) { new_lfs_router(forked_project, user: user_two) }
+  let(:lfs_router_forked_ci_auth) { new_lfs_router(forked_project, ci: true) }
 
   let(:sample_oid) { "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a17f80" }
   let(:sample_size) { 499013 }
@@ -80,6 +83,7 @@ describe Gitlab::Lfs::Router, lib: true do
 
       context 'with required headers' do
         before do
+          project.lfs_objects << lfs_object
           env['HTTP_X_SENDFILE_TYPE'] = "X-Sendfile"
         end
 
@@ -91,7 +95,6 @@ describe Gitlab::Lfs::Router, lib: true do
 
         context 'when user has project access' do
           before do
-            project.lfs_objects << lfs_object
             project.team << [user, :master]
           end
 
@@ -104,6 +107,17 @@ describe Gitlab::Lfs::Router, lib: true do
             expect(lfs_router_auth.try_call[1]['X-Sendfile']).to eq(lfs_object.file.path)
           end
         end
+
+        context 'when CI is authorized' do
+          it "responds with status 200" do
+            expect(lfs_router_ci_auth.try_call.first).to eq(200)
+          end
+
+          it "responds with the file location" do
+            expect(lfs_router_ci_auth.try_call[1]['Content-Type']).to eq("application/octet-stream")
+            expect(lfs_router_ci_auth.try_call[1]['X-Sendfile']).to eq(lfs_object.file.path)
+          end
+        end
       end
 
       context 'without required headers' do
@@ -134,7 +148,6 @@ describe Gitlab::Lfs::Router, lib: true do
     end
 
     describe 'download' do
-      describe 'when user is authenticated' do
       before do
         body = { 'operation' => 'download',
                  'objects' => [
@@ -145,20 +158,14 @@ describe Gitlab::Lfs::Router, lib: true do
         env['rack.input'] = StringIO.new(body)
       end
 
-        describe 'when user has download access' do
-          before do
-            @auth = authorize(user)
-            env["HTTP_AUTHORIZATION"] = @auth
-            project.team << [user, :reporter]
-          end
-
+      shared_examples 'an authorized requests' do
         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
+            response = router.try_call
             expect(response.first).to eq(200)
             response_body = ActiveSupport::JSON.decode(response.last.first)
 
@@ -168,7 +175,7 @@ describe Gitlab::Lfs::Router, lib: true do
                 'actions' => {
                   'download' => {
                     'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}",
-                      'header' => { 'Authorization' => @auth }
+                    'header' => { 'Authorization' => auth }
                   }
                 }
               }])
@@ -181,7 +188,7 @@ describe Gitlab::Lfs::Router, lib: true do
           end
 
           it 'responds with status 200 and error message' do
-              response = lfs_router_auth.try_call
+            response = router.try_call
             expect(response.first).to eq(200)
             response_body = ActiveSupport::JSON.decode(response.last.first)
 
@@ -208,7 +215,7 @@ describe Gitlab::Lfs::Router, lib: true do
           end
 
           it "responds with status 200 and error message" do
-              response = lfs_router_auth.try_call
+            response = router.try_call
             expect(response.first).to eq(200)
             response_body = ActiveSupport::JSON.decode(response.last.first)
 
@@ -240,7 +247,7 @@ describe Gitlab::Lfs::Router, lib: true do
           end
 
           it "responds with status 200 with upload hypermedia link for the new object" do
-              response = lfs_router_auth.try_call
+            response = router.try_call
             expect(response.first).to eq(200)
             response_body = ActiveSupport::JSON.decode(response.last.first)
 
@@ -257,7 +264,7 @@ describe Gitlab::Lfs::Router, lib: true do
                 'actions' => {
                   'download' => {
                     'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}",
-                      'header' => { 'Authorization' => @auth }
+                    'header' => { 'Authorization' => auth }
                   }
                 }
               }])
@@ -265,24 +272,29 @@ describe Gitlab::Lfs::Router, lib: true do
         end
       end
 
-        context 'when user does is not member of the project' do
+      context 'when user is authenticated' do
+        let(:auth) { authorize(user) }
+
         before do
-            @auth = authorize(user)
-            env["HTTP_AUTHORIZATION"] = @auth
-            project.team << [user, :guest]
+          env["HTTP_AUTHORIZATION"] = auth
+          project.team << [user, role]
         end
 
+        it_behaves_like 'an authorized requests' do
+          let(:role) { :reporter }
+          let(:router) { lfs_router_auth }
+        end
+
+        context 'when user does is not member of the project' do
+          let(:role) { :guest }
+
           it 'responds with 403' do
             expect(lfs_router_auth.try_call.first).to eq(403)
           end
         end
 
         context 'when user does not have download access' do
-          before do
-            @auth = authorize(user)
-            env["HTTP_AUTHORIZATION"] = @auth
-            project.team << [user, :guest]
-          end
+          let(:role) { :guest }
 
           it 'responds with 403' do
             expect(lfs_router_auth.try_call.first).to eq(403)
@@ -290,18 +302,19 @@ describe Gitlab::Lfs::Router, lib: true do
         end
       end
 
-      context 'when user is not authenticated' do
+      context 'when CI is authorized' do
+        let(:auth) { 'gitlab-ci-token:password' }
+
         before do
-          body = { 'operation' => 'download',
-                   'objects' => [
-                     { 'oid' => sample_oid,
-                       'size' => sample_size
-                     }],
+          env["HTTP_AUTHORIZATION"] = auth
+        end
 
-          }.to_json
-          env['rack.input'] = StringIO.new(body)
+        it_behaves_like 'an authorized requests' do
+          let(:router) { lfs_router_ci_auth }
+        end
       end
 
+      context 'when user is not authenticated' do
         describe 'is accessing public project' do
           before do
             public_project.lfs_objects << lfs_object
@@ -338,7 +351,6 @@ describe Gitlab::Lfs::Router, lib: true do
     end
 
     describe 'upload' do
-      describe 'when user is authenticated' do
       before do
         body = { 'operation' => 'upload',
                  'objects' => [
@@ -349,6 +361,7 @@ describe Gitlab::Lfs::Router, lib: true do
         env['rack.input'] = StringIO.new(body)
       end
 
+      describe 'when request is authenticated' do
         describe 'when user has project push access' do
           before do
             @auth = authorize(user)
@@ -440,15 +453,15 @@ describe Gitlab::Lfs::Router, lib: true do
             expect(lfs_router_auth.try_call.first).to eq(403)
           end
         end
-      end
 
-      context 'when user is not authenticated' do
-        before do
-          env['rack.input'] = StringIO.new(
-            { 'objects' => [], 'operation' => 'upload' }.to_json
-          )
+        context 'when CI is authorized' do
+          it 'responds with 401' do
+            expect(lfs_router_ci_auth.try_call.first).to eq(401)
+          end
+        end
       end
 
+      context 'when user is not authenticated' do
         context 'when user has push access' do
           before do
             project.team << [user, :master]
@@ -465,6 +478,18 @@ describe Gitlab::Lfs::Router, lib: true do
           end
         end
       end
+
+      context 'when CI is authorized' do
+        let(:auth) { 'gitlab-ci-token:password' }
+
+        before do
+          env["HTTP_AUTHORIZATION"] = auth
+        end
+
+        it "responds with status 403" do
+          expect(lfs_router_public_ci_auth.try_call.first).to eq(401)
+        end
+      end
     end
 
     describe 'unsupported' do
@@ -490,109 +515,80 @@ describe Gitlab::Lfs::Router, lib: true do
       env['REQUEST_METHOD'] = 'PUT'
     end
 
-    describe 'to one project' do
-      describe 'when user has push access to the project' do
-        before do
-          project.team << [user, :master]
-        end
-
-        describe 'when user is authenticated' do
+    shared_examples 'unauthorized' do
       context 'and request is sent by gitlab-workhorse to authorize the request' do
         before do
-              header_for_upload_authorize(project)
-            end
-
-            it 'responds with status 200, location of lfs store and object details' do
-              json_response = ActiveSupport::JSON.decode(lfs_router_auth.try_call.last.first)
-
-              expect(lfs_router_auth.try_call.first).to eq(200)
-              expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload")
-              expect(json_response['LfsOid']).to eq(sample_oid)
-              expect(json_response['LfsSize']).to eq(sample_size)
-            end
-          end
-
-          context 'and request is sent by gitlab-workhorse to finalize the upload' do
-            before do
-              headers_for_upload_finalize(project)
-            end
-
-            it 'responds with status 200 and lfs object is linked to the project' do
-              expect(lfs_router_auth.try_call.first).to eq(200)
-              expect(lfs_object.projects.pluck(:id)).to include(project.id)
-            end
-          end
-        end
-
-        describe 'when user is unauthenticated' do
-          let(:lfs_router_noauth) { new_lfs_router(project, nil) }
-
-          context 'and request is sent by gitlab-workhorse to authorize the request' do
-            before do
-              header_for_upload_authorize(project)
+          header_for_upload_authorize(router.project)
         end
 
         it 'responds with status 401' do
-              expect(lfs_router_noauth.try_call.first).to eq(401)
+          expect(router.try_call.first).to eq(401)
         end
       end
 
       context 'and request is sent by gitlab-workhorse to finalize the upload' do
         before do
-              headers_for_upload_finalize(project)
+          headers_for_upload_finalize(router.project)
         end
 
         it 'responds with status 401' do
-              expect(lfs_router_noauth.try_call.first).to eq(401)
+          expect(router.try_call.first).to eq(401)
         end
       end
 
       context 'and request is sent with a malformed headers' do
         before do
-              env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}"
+          env["PATH_INFO"] = "#{router.project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}"
           env["HTTP_X_GITLAB_LFS_TMP"] = "cat /etc/passwd"
         end
 
         it 'does not recognize it as a valid lfs command' do
-              expect(lfs_router_noauth.try_call).to eq(nil)
-            end
+          expect(router.try_call).to eq(nil)
         end
       end
     end
 
-      describe 'and user does not have push access' do
-        describe 'when user is authenticated' do
+    shared_examples 'forbidden' do
       context 'and request is sent by gitlab-workhorse to authorize the request' do
         before do
-              header_for_upload_authorize(project)
+          header_for_upload_authorize(router.project)
         end
 
         it 'responds with 403' do
-              expect(lfs_router_auth.try_call.first).to eq(403)
+          expect(router.try_call.first).to eq(403)
         end
       end
 
       context 'and request is sent by gitlab-workhorse to finalize the upload' do
         before do
-              headers_for_upload_finalize(project)
+          headers_for_upload_finalize(router.project)
         end
 
         it 'responds with 403' do
-              expect(lfs_router_auth.try_call.first).to eq(403)
+          expect(router.try_call.first).to eq(403)
         end
       end
     end
 
-        describe 'when user is unauthenticated' do
-          let(:lfs_router_noauth) { new_lfs_router(project, nil) }
+    describe 'to one project' do
+      describe 'when user is authenticated' do
+        describe 'when user has push access to the project' do
+          before do
+            project.team << [user, :developer]
+          end
 
           context 'and request is sent by gitlab-workhorse to authorize the request' do
             before do
               header_for_upload_authorize(project)
             end
 
-            it 'responds with 401' do
-              expect(lfs_router_noauth.try_call.first).to eq(401)
+            it 'responds with status 200, location of lfs store and object details' do
+              json_response = ActiveSupport::JSON.decode(lfs_router_auth.try_call.last.first)
+
+              expect(lfs_router_auth.try_call.first).to eq(200)
+              expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload")
+              expect(json_response['LfsOid']).to eq(sample_oid)
+              expect(json_response['LfsSize']).to eq(sample_size)
             end
           end
 
@@ -601,23 +597,42 @@ describe Gitlab::Lfs::Router, lib: true do
               headers_for_upload_finalize(project)
             end
 
-            it 'responds with 401' do
-              expect(lfs_router_noauth.try_call.first).to eq(401)
+            it 'responds with status 200 and lfs object is linked to the project' do
+              expect(lfs_router_auth.try_call.first).to eq(200)
+              expect(lfs_object.projects.pluck(:id)).to include(project.id)
             end
           end
         end
+
+        describe 'and user does not have push access' do
+          let(:router) { lfs_router_auth }
+
+          it_behaves_like 'forbidden'
         end
       end
 
-    describe "to a forked project" do
+      context 'when CI is authenticated' do
+        let(:router) { lfs_router_ci_auth }
+
+        it_behaves_like 'unauthorized'
+      end
+
+      context 'for unauthenticated' do
+        let(:router) { new_lfs_router(project) }
+
+        it_behaves_like 'unauthorized'
+      end
+    end
+
+    describe 'to a forked project' do
       let(:forked_project) { fork_project(public_project, user) }
 
+      describe 'when user is authenticated' do
         describe 'when user has push access to the project' do
           before do
-          forked_project.team << [user_two, :master]
+            forked_project.team << [user_two, :developer]
           end
 
-        describe 'when user is authenticated' do
           context 'and request is sent by gitlab-workhorse to authorize the request' do
             before do
               header_for_upload_authorize(forked_project)
@@ -645,78 +660,28 @@ describe Gitlab::Lfs::Router, lib: true do
           end
         end
 
-        describe 'when user is unauthenticated' do
-          context 'and request is sent by gitlab-workhorse to authorize the request' do
-            before do
-              header_for_upload_authorize(forked_project)
-            end
-
-            it 'responds with status 401' do
-              expect(lfs_router_forked_noauth.try_call.first).to eq(401)
-            end
-          end
-
-          context 'and request is sent by gitlab-workhorse to finalize the upload' do
-            before do
-              headers_for_upload_finalize(forked_project)
-            end
-
-            it 'responds with status 401' do
-              expect(lfs_router_forked_noauth.try_call.first).to eq(401)
-            end
-          end
-        end
-      end
-
         describe 'and user does not have push access' do
-        describe 'when user is authenticated' do
-          context 'and request is sent by gitlab-workhorse to authorize the request' do
-            before do
-              header_for_upload_authorize(forked_project)
-            end
-
-            it 'responds with 403' do
-              expect(lfs_router_forked_auth.try_call.first).to eq(403)
-            end
-          end
-
-          context 'and request is sent by gitlab-workhorse to finalize the upload' do
-            before do
-              headers_for_upload_finalize(forked_project)
-            end
+          let(:router) { lfs_router_forked_auth }
 
-            it 'responds with 403' do
-              expect(lfs_router_forked_auth.try_call.first).to eq(403)
-            end
+          it_behaves_like 'forbidden'
         end
       end
 
-        describe 'when user is unauthenticated' do
-          context 'and request is sent by gitlab-workhorse to authorize the request' do
-            before do
-              header_for_upload_authorize(forked_project)
-            end
+      context 'when CI is authenticated' do
+        let(:router) { lfs_router_forked_ci_auth }
 
-            it 'responds with 401' do
-              expect(lfs_router_forked_noauth.try_call.first).to eq(401)
-            end
+        it_behaves_like 'unauthorized'
       end
 
-          context 'and request is sent by gitlab-workhorse to finalize the upload' do
-            before do
-              headers_for_upload_finalize(forked_project)
-            end
+      context 'for unauthenticated' do
+        let(:router) { lfs_router_forked_noauth }
 
-            it 'responds with 401' do
-              expect(lfs_router_forked_noauth.try_call.first).to eq(401)
-            end
-          end
-        end
+        it_behaves_like 'unauthorized'
       end
 
       describe 'and second project not related to fork or a source project' do
         let(:second_project) { create(:project) }
-        let(:lfs_router_second_project) { new_lfs_router(second_project, user) }
+        let(:lfs_router_second_project) { new_lfs_router(second_project, user: user) }
 
         before do
           public_project.lfs_objects << lfs_object
@@ -745,8 +710,8 @@ describe Gitlab::Lfs::Router, lib: true do
     ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
   end
 
-  def new_lfs_router(project, user)
-    Gitlab::Lfs::Router.new(project, user, request)
+  def new_lfs_router(project, user: nil, ci: false)
+    Gitlab::Lfs::Router.new(project, user, ci, request)
   end
 
   def header_for_upload_authorize(project)