diff --git a/app/controllers/object_files_controller.rb b/app/controllers/object_files_controller.rb index fdf262450..f32bbf075 100644 --- a/app/controllers/object_files_controller.rb +++ b/app/controllers/object_files_controller.rb @@ -1,16 +1,14 @@ class ObjectFilesController < ApplicationController - - before_filter :authenticate_user! + before_action :authenticate_user! def show @fobj = Hydrus::Item.find(params[:id]) @fobj.current_user = current_user authorize! :read, @fobj # only users who are authorized to view this object and download the files - filename = params[:filename] - object_file = Hydrus::ObjectFile.new(:pid=>DruidTools::Druid.new(@fobj.pid).druid) - file_location = File.join(object_file.file.store_dir,filename) + filename = params[:filename] + object_file = Hydrus::ObjectFile.new(pid: DruidTools::Druid.new(@fobj.pid).druid) + file_location = File.join(object_file.file.store_dir, filename) file = File.new(file_location) send_file file end - end diff --git a/app/models/hydrus/contentable.rb b/app/models/hydrus/contentable.rb index 116fed90c..9d09a183d 100644 --- a/app/models/hydrus/contentable.rb +++ b/app/models/hydrus/contentable.rb @@ -67,8 +67,8 @@ def parent_directory end def base_file_directory - file_object = Hydrus::ObjectFile.new(:pid=>DruidTools::Druid.new(pid).druid) - file_location = File.join(file_object.file.base_dir) + file_object = Hydrus::ObjectFile.new(pid: DruidTools::Druid.new(pid).druid) + File.join(file_object.file.base_dir) end def content_directory diff --git a/app/models/hydrus/object_file.rb b/app/models/hydrus/object_file.rb index 857aa9d15..c5feb08e5 100644 --- a/app/models/hydrus/object_file.rb +++ b/app/models/hydrus/object_file.rb @@ -7,10 +7,10 @@ class Hydrus::ObjectFile < ActiveRecord::Base def size file.size end - + # Override the URL supplied by CarrierWave def url - Rails.application.routes.url_helpers.file_upload_path(:id=>pid,:filename=>filename) + Rails.application.routes.url_helpers.file_upload_path(id: pid, filename: filename) end def current_path diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 5126aab69..7f6df08e2 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -47,7 +47,7 @@ def delete_tmp_dir(new_file) def base_dir File.join(Rails.root, Settings.hydrus.file_upload_path, DruidTools::Druid.new(model.pid).path) end - + # Set the directory where uploaded files will be stored. def store_dir File.join(base_dir, 'content') @@ -55,7 +55,7 @@ def store_dir # temp directory where files are stored before they are uploaded def cache_dir - File.join(Rails.root,'tmp') + File.join(Rails.root, Settings.hydrus.file_upload_path, 'tmp') end # Provide a default URL as a default if there hasn't been a file uploaded: diff --git a/config/environment.rb b/config/environment.rb index 264d23f18..f0b909956 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -5,7 +5,6 @@ Rails.application.initialize! Hydrus::Application.configure do - # file attributes by mimetype, including defaults, to use when generating content metadata config.cm_file_attributes = { 'default' => { publish: 'yes', preserve: 'yes', shelve: 'yes' } diff --git a/config/routes.rb b/config/routes.rb index 9326d55d1..6221c9420 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -59,6 +59,6 @@ get '/file/:id/:filename' => 'object_files#show', :as => 'file_upload' end - get '/404', :to => "exceptions#render_404" - get '/500', :to => "exceptions#render_500" + get '/404', to: 'exceptions#render_404' + get '/500', to: 'exceptions#render_500' end diff --git a/lib/tasks/fixtures.rake b/lib/tasks/fixtures.rake index f05f46ca3..45aa6548a 100644 --- a/lib/tasks/fixtures.rake +++ b/lib/tasks/fixtures.rake @@ -97,7 +97,7 @@ namespace :hydrus do end end - desc "reload test uploaded files to upload directory" + desc 'reload test uploaded files to upload directory' task :refresh_upload_files do # Copies fixture files from source control to the app's public area: # source: spec/fixtures/files/DRUID/* @@ -119,7 +119,7 @@ namespace :hydrus do end end - desc "clear uploaded files upload directory" + desc 'clear uploaded files upload directory' task :clear_upload_files do puts 'clearing upload files directory' require File.expand_path('config/environment') diff --git a/spec/controllers/object_files_controller_spec.rb b/spec/controllers/object_files_controller_spec.rb deleted file mode 100644 index adf53a483..000000000 --- a/spec/controllers/object_files_controller_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'spec_helper' - -describe ObjectFilesController, :type => :controller do - -end diff --git a/spec/features/home_page_spec.rb b/spec/features/home_page_spec.rb index 09355ee50..e03713cb3 100644 --- a/spec/features/home_page_spec.rb +++ b/spec/features/home_page_spec.rb @@ -69,7 +69,7 @@ it 'breadcrumbs should not be displayed' do # Logged out - logout + sign_out visit root_path expect(page).not_to have_css(@breadcrumbs) end diff --git a/spec/features/item_create_spec.rb b/spec/features/item_create_spec.rb index 6760837c8..599998c13 100644 --- a/spec/features/item_create_spec.rb +++ b/spec/features/item_create_spec.rb @@ -125,7 +125,6 @@ f.file = Tempfile.new('mock_HydrusObjectFile_') f.save click_button(@buttons[:save]) - # confirm validation message is shown and publish button is not available expect(find(@div_alert)).to have_content(@notices[:save]) expect(find(@div_alert)).to have_content('Contributors must be entered') diff --git a/spec/features/models/object_file_spec.rb b/spec/features/models/object_file_spec.rb index 904869960..9efa4ae39 100644 --- a/spec/features/models/object_file_spec.rb +++ b/spec/features/models/object_file_spec.rb @@ -10,9 +10,9 @@ it 'finds four files associated with the first item and it grabs the url of a given file' do expect(files.size).to eq(4) f = files[0] - exp_url = '/uploads/bb/123/bb/1234/bb123bb1234/content/pinocchio.htm' + exp_url = '/file/druid:bb123bb1234/pinocchio.htm' expect(f.url).to eq(exp_url) - expect(f.current_path).to eq("#{Rails.root}/public#{exp_url}") + expect(f.current_path).to eq("#{Rails.root}/uploads/bb/123/bb/1234/bb123bb1234/content/pinocchio.htm") expect(files[1].filename).to eq(%q{pinocchio characters tc in file name.pdf}) expect(files[1].size).to be > 0 end @@ -24,7 +24,6 @@ files = @hi.files expect(files.size).to eq(4) file = files.first - file_url = file.url full_file_path = file.current_path expect(File.exists?(full_file_path)).to be_truthy diff --git a/spec/features/object_files_controller_spec.rb b/spec/features/object_files_controller_spec.rb new file mode 100644 index 000000000..eea21658f --- /dev/null +++ b/spec/features/object_files_controller_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe('Object Files Download', type: :request, integration: true) do + fixtures :object_files + let(:archivist1) { create :archivist1 } + let(:archivist99) { create :archivist99 } + + before :each do + @druid = 'druid:bb123bb1234' + @file = Hydrus::ObjectFile.find(3) # this txt file defined in the fixtures belongs to archivist1 + end + + it 'allows the owner of the file to download it' do + sign_in(archivist1) # owner of the item can download the file + visit @file.url + expect(page.status_code).to eq(200) + expect(page.response_headers['Content-Type']).to eq('text/plain') + logout(archivist1) + end + + it 'redirects to home page with not authorized message when accessing a file URL when no access is allowed' do + sign_in(archivist99) # no view access on the item, no access + visit @file.url + expect(page.response_headers['Content-Type']).not_to eq('text/plain') + expect(current_path).to eq(root_path) + expect(find('#flash-notices div.alert')).to have_content('You are not authorized to access this page.') + logout(archivist99) + visit @file.url + expect(current_path).to eq(root_path) # logged out, still can't get the file + end +end diff --git a/spec/models/hydrus/contentable_spec.rb b/spec/models/hydrus/contentable_spec.rb index ab2dce42b..463503720 100644 --- a/spec/models/hydrus/contentable_spec.rb +++ b/spec/models/hydrus/contentable_spec.rb @@ -4,7 +4,7 @@ before(:each) do @go = Hydrus::GenericObject.new @pid = 'oo000oo9999' - @base_dir = File.join(Rails.root,Settings.hydrus.file_upload_path,'./oo/000/oo/9999/oo000oo9999') + @base_dir = File.join(Rails.root, Settings.hydrus.file_upload_path, './oo/000/oo/9999/oo000oo9999') allow(@go).to receive(:pid).and_return(@pid) end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 9338b9fe8..7024781b7 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -220,7 +220,7 @@ def create_new_item(opts = {}) # Helper method to restore a file to the uploads directory # after it was deleted in a integration test. def restore_upload_file(object_file) - src = File.join('spec/fixtures/files', DruidTools::Druid.new(object_file.pid).id, object_file.filename) - dst = object_file.current_path - FileUtils.cp(src, dst) + src = File.join('spec/fixtures/files', DruidTools::Druid.new(object_file.pid).id, object_file.filename) + dst = object_file.current_path + FileUtils.cp(src, dst) end