Do not show LFS object when LFS is disabled

Do not display a 404, when a user tries to retrieve the raw content of
an LFS file (pointer) if the config option "lfs_enabled" is set to
false. Instead, display the LFS pointer file directly.

app/controllers/projects/raw_controller.rb

@@ -15,7 +15,7 @@ class Projects::RawController < Projects::ApplicationController
 
       return if cached_blob?
 
-      if @blob.lfs_pointer?
+      if @blob.lfs_pointer? && project.lfs_enabled?
         send_lfs_object
       else
         send_git_blob @repository, @blob

app/models/blob.rb

@@ -54,9 +54,13 @@ class Blob < SimpleDelegator
     UploaderHelper::VIDEO_EXT.include?(extname.downcase.delete('.'))
   end
 
-  def to_partial_path
+  def to_partial_path(project)
     if lfs_pointer?
-      'download'
+      if project.lfs_enabled?
+        'download'
+      else
+        'text'
+      end
     elsif image? || svg?
       'image'
     elsif text?

app/views/projects/blob/_blob.html.haml

@@ -33,4 +33,4 @@
           = number_to_human_size(blob_size(blob))
       .file-actions.hidden-xs
         = render "actions"
-    = render blob, blob: blob
+    = render blob.to_partial_path(@project), blob: blob

changelogs/unreleased/feature-custom-lfs.yml

+---
+title: Do not show LFS object when LFS is disabled
+merge_request: 9779
+author: Christopher Bartz

features/steps/project/source/browse_files.rb

@@ -337,6 +337,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   end
 
   step 'I click on "files/lfs/lfs_object.iso" file in repo' do
+    allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(true)
     visit namespace_project_tree_path(@project.namespace, @project, "lfs")
     click_link 'files'
     click_link "lfs"

spec/controllers/projects/raw_controller_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Projects::RawController do
   let(:public_project) { create(:project, :public, :repository) }
 
-  describe "#show" do
+  describe '#show' do
     context 'regular filename' do
       let(:id) { 'master/README.md' }
 
@@ -16,8 +16,8 @@ describe Projects::RawController do
         expect(response).to have_http_status(200)
         expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
         expect(response.header['Content-Disposition']).
-            to eq("inline")
-        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:")
+            to eq('inline')
+        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
       end
     end
 
@@ -32,7 +32,7 @@ describe Projects::RawController do
 
         expect(response).to have_http_status(200)
         expect(response.header['Content-Type']).to eq('image/jpeg')
-        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:")
+        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
       end
     end
 
@@ -40,32 +40,57 @@ describe Projects::RawController do
       let(:id) { 'be93687/files/lfs/lfs_object.iso' }
       let!(:lfs_object) { create(:lfs_object, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', size: '1575078') }
 
-      context 'when project has access' do
+      context 'when lfs is enabled' do
         before do
-          public_project.lfs_objects << lfs_object
-          allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true)
-          allow(controller).to receive(:send_file) { controller.head :ok }
+          allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(true)
         end
 
-        it 'serves the file' do
-          expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: "lfs_object.iso", disposition: 'attachment')
-          get(:show,
-              namespace_id: public_project.namespace.to_param,
-              project_id: public_project,
-              id: id)
+        context 'when project has access' do
+          before do
+            public_project.lfs_objects << lfs_object
+            allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true)
+            allow(controller).to receive(:send_file) { controller.head :ok }
+          end
 
-          expect(response).to have_http_status(200)
+          it 'serves the file' do
+            expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: 'lfs_object.iso', disposition: 'attachment')
+            get(:show,
+                namespace_id: public_project.namespace.to_param,
+                project_id: public_project,
+                id: id)
+
+            expect(response).to have_http_status(200)
+          end
+        end
+
+        context 'when project does not have access' do
+          it 'does not serve the file' do
+            get(:show,
+                namespace_id: public_project.namespace.to_param,
+                project_id: public_project,
+                id: id)
+
+            expect(response).to have_http_status(404)
+          end
         end
       end
 
-      context 'when project does not have access' do
-        it 'does not serve the file' do
+      context 'when lfs is not enabled' do
+        before do
+          allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false)
+        end
+
+        it 'delivers ASCII file' do
           get(:show,
               namespace_id: public_project.namespace.to_param,
               project_id: public_project,
               id: id)
 
-          expect(response).to have_http_status(404)
+          expect(response).to have_http_status(200)
+          expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
+          expect(response.header['Content-Disposition']).
+              to eq('inline')
+          expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
         end
       end
     end

spec/features/projects/files/browse_files_spec.rb

 require 'spec_helper'
 
-feature 'user checks git blame', feature: true do
+feature 'user browses project', feature: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
@@ -18,4 +18,16 @@ feature 'user checks git blame', feature: true do
     expect(page).to have_content "Dmitriy Zaporozhets"
     expect(page).to have_content "Initial commit"
   end
+
+  scenario 'can see raw content of LFS pointer with LFS disabled' do
+    allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false)
+    click_link 'files'
+    click_link 'lfs'
+    click_link 'lfs_object.iso'
+
+    expect(page).not_to have_content 'Download (1.5 MB)'
+    expect(page).to have_content 'version https://git-lfs.github.com/spec/v1'
+    expect(page).to have_content 'oid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897'
+    expect(page).to have_content 'size 1575078'
+  end
 end

spec/models/blob_spec.rb

@@ -70,6 +70,8 @@ describe Blob do
   end
 
   describe '#to_partial_path' do
+    let(:project) { double(lfs_enabled?: true) }
+
     def stubbed_blob(overrides = {})
       overrides.reverse_merge!(
         image?: false,
@@ -84,34 +86,35 @@ describe Blob do
       end
     end
 
-    it 'handles LFS pointers' do
-      blob = stubbed_blob(lfs_pointer?: true)
+    it 'handles LFS pointers with LFS enabled' do
+      blob = stubbed_blob(lfs_pointer?: true, text?: true)
+      expect(blob.to_partial_path(project)).to eq 'download'
+    end
 
-      expect(blob.to_partial_path).to eq 'download'
+    it 'handles LFS pointers with LFS disabled' do
+      blob = stubbed_blob(lfs_pointer?: true, text?: true)
+      project = double(lfs_enabled?: false)
+      expect(blob.to_partial_path(project)).to eq 'text'
     end
 
     it 'handles SVGs' do
       blob = stubbed_blob(text?: true, svg?: true)
-
-      expect(blob.to_partial_path).to eq 'image'
+      expect(blob.to_partial_path(project)).to eq 'image'
     end
 
     it 'handles images' do
       blob = stubbed_blob(image?: true)
-
-      expect(blob.to_partial_path).to eq 'image'
+      expect(blob.to_partial_path(project)).to eq 'image'
     end
 
     it 'handles text' do
       blob = stubbed_blob(text?: true)
-
-      expect(blob.to_partial_path).to eq 'text'
+      expect(blob.to_partial_path(project)).to eq 'text'
     end
 
     it 'defaults to download' do
       blob = stubbed_blob
-
-      expect(blob.to_partial_path).to eq 'download'
+      expect(blob.to_partial_path(project)).to eq 'download'
     end
   end