diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index c56f432a1f1..be7d5c187fe 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -38,18 +38,28 @@ class Projects::RawController < Projects::ApplicationController type = get_blob_type send_data( - @blob.data, - type: type, - disposition: 'inline' - ) + @blob.data, + type: type, + disposition: 'inline' + ) end def send_lfs_object - lfs_object = LfsObject.find_by_oid(@blob.lfs_oid) - return nil unless lfs_object && lfs_object.file.exists? + lfs_object = find_lfs_object - if lfs_object.projects.exists?(lfs_object.storage_project(@project).id) + if lfs_object && lfs_object.project_allowed_access?(@project) send_file lfs_object.file.path, filename: @blob.name, disposition: 'attachment' + else + render_404 + end + end + + def find_lfs_object + lfs_object = LfsObject.find_by_oid(@blob.lfs_oid) + if lfs_object && lfs_object.file.exists? + lfs_object + else + nil end end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index df5f5fae23c..fa1b2522051 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -30,7 +30,7 @@ module BlobHelper nil end - if blob && blob.text? + if blob && blob.text? && !blob.lfs_pointer? text = 'Edit' after = options[:after] || '' from_mr = options[:from_merge_request_id] @@ -71,4 +71,16 @@ module BlobHelper def blob_icon(mode, name) icon("#{file_type_icon_class('file', mode, name)} fw") end + + def viewable?(blob) + blob.text? && !blob.lfs_pointer? + end + + def blob_size(blob) + if blob.lfs_pointer? + blob.lfs_size + else + blob.size + end + end end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 03a49e119b8..6afa1aacc5b 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -54,6 +54,10 @@ module TreeHelper ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) end + def can_delete_or_replace?(blob) + allowed_tree_edit? && !blob.lfs_pointer? + end + def tree_breadcrumbs(tree, max_links = 2) if @path.present? part_path = "" diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb index a243c7b77cc..18657c3e1c8 100644 --- a/app/models/lfs_object.rb +++ b/app/models/lfs_object.rb @@ -13,4 +13,8 @@ class LfsObject < ActiveRecord::Base project end end + + def project_allowed_access?(project) + projects.exists?(storage_project(project).id) + end end diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml index 15cd8f056f5..c31f6442f12 100644 --- a/app/views/projects/blob/_actions.html.haml +++ b/app/views/projects/blob/_actions.html.haml @@ -1,9 +1,9 @@ .btn-group.tree-btn-group - = edit_blob_link(@project, @ref, @path) unless @blob.lfs_pointer? + = edit_blob_link(@project, @ref, @path) = link_to 'Raw', namespace_project_raw_path(@project.namespace, @project, @id), class: 'btn btn-sm', target: '_blank' -# only show normal/blame view links for text files - - if @blob.text? && !@blob.lfs_pointer? + - if viewable?(@blob) - if current_page? namespace_project_blame_path(@project.namespace, @project, @id) = link_to 'Normal View', namespace_project_blob_path(@project.namespace, @project, @id), class: 'btn btn-sm' @@ -16,7 +16,7 @@ = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project, tree_join(@commit.sha, @path)), class: 'btn btn-sm' -- if allowed_tree_edit? && !@blob.lfs_pointer? +- if can_delete_or_replace?(@blob) .btn-group{ role: "group" } %button.btn.btn-default{ 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } Replace %button.btn.btn-remove{ 'data-target' => '#modal-remove-blob', 'data-toggle' => 'modal' } Delete diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index bb9e1c63413..2a3315da3db 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -29,7 +29,7 @@ %strong = blob.name %small - = number_to_human_size(blob.size) unless blob.lfs_pointer? + = number_to_human_size(blob_size(blob)) .file-actions.hidden-xs = render "actions" - if blob.lfs_pointer? diff --git a/app/views/projects/blob/_download.html.haml b/app/views/projects/blob/_download.html.haml index 39ec6f693e2..7908fcae3de 100644 --- a/app/views/projects/blob/_download.html.haml +++ b/app/views/projects/blob/_download.html.haml @@ -4,8 +4,4 @@ %h1.light %i.fa.fa-download %h4 - - if blob.lfs_pointer? - - size = blob.lfs_size - - else - - size = blob.size - Download (#{number_to_human_size size}) + Download (#{number_to_human_size blob_size(blob)}) diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index b7276868ce6..09d6fc18e3e 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -6,7 +6,7 @@ %div#tree-holder.tree-holder = render 'blob', blob: @blob -- if allowed_tree_edit? +- if can_delete_or_replace?(@blob) = render 'projects/blob/remove' - title = "Replace #{@blob.name}" diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index ec1c665716e..f6ba64d31b5 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -25,7 +25,7 @@ = "#{diff_file.diff.a_mode} → #{diff_file.diff.b_mode}" .diff-controls - - if blob.text? && !blob.lfs_pointer? + - if viewable?(blob) = link_to '#', class: 'js-toggle-diff-comments btn btn-sm active has_tooltip', title: "Toggle comments for this file" do %i.fa.fa-comments   @@ -40,7 +40,7 @@ .diff-content.diff-wrap-lines -# Skipp all non non-supported blobs - return unless blob.respond_to?('text?') - - if blob.text? && !blob.lfs_pointer? + - if viewable?(blob) - if diff_view == 'parallel' = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i - else diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature index e545ea63ca8..37f99b37619 100644 --- a/features/project/source/browse_files.feature +++ b/features/project/source/browse_files.feature @@ -221,3 +221,9 @@ Feature: Project Source Browse Files Given I switch ref to fix And I visit the fix tree Then I see the commit data for a directory with a leading dot + + Scenario: I browse LFS object + Given I click on "files/lfs/lfs_object.iso" file in repo + Then I should see download link and object size + And I should not see lfs pointer details + And I should see buttons for allowed commands diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index ceab23b9a4d..2edbca9b7fe 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -305,6 +305,32 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps expect(page).not_to have_content('Loading commit data...') end + step 'I click on "files/lfs/lfs_object.iso" file in repo' do + click_link 'files' + click_link "lfs" + click_link "lfs_object.iso" + end + + step 'I should see download link and object size' do + expect(page).to have_content 'Download (1.5 MB)' + end + + step 'I should not see lfs pointer details' do + expect(page).not_to have_content 'version https://git-lfs.github.com/spec/v1' + expect(page).not_to have_content 'oid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' + expect(page).not_to have_content 'size 1575078' + end + + step 'I should see buttons for allowed commands' do + expect(page).to have_content 'Raw' + expect(page).to have_content 'History' + expect(page).to have_content 'Permalink' + expect(page).not_to have_content 'Edit' + expect(page).not_to have_content 'Blame' + expect(page).not_to have_content 'Delete' + expect(page).not_to have_content 'Replace' + end + private def set_new_content diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index c114f342021..2329c246daa 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -33,5 +33,37 @@ describe Projects::RawController do expect(response.header['Content-Type']).to eq('image/jpeg') end end + + context 'lfs object' do + let(:id) { 'master/files/lfs/lfs_object.iso' } + let!(:lfs_object) { create(:lfs_object, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', size: '1575078') } + + context 'when project has access' do + before do + public_project.lfs_objects << lfs_object + end + + it 'serves the file' do + get(:show, + namespace_id: public_project.namespace.to_param, + project_id: public_project.to_param, + id: id) + + expect(response.status).to eq(200) + expect(response.header['Content-Type']).to eq('application/octet-stream') + 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.to_param, + id: id) + + expect(response.status).to eq(404) + end + end + end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 787670e9297..47c67475cd0 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -12,7 +12,7 @@ module TestEnv 'fix' => '48f0be4', 'improve/awesome' => '5937ac0', 'markdown' => '0ed8c6c', - 'master' => '5937ac0', + 'master' => 'be93687', "'test'" => 'e56497b', }