diff --git a/.gitignore b/.gitignore index 6b90aed9bf..d9073f40c3 100644 --- a/.gitignore +++ b/.gitignore @@ -89,7 +89,6 @@ Gemfile.lock # Local Gemfile for developing without sharing dependencies .gemfile -*.txt *.orig diff --git a/resources/app/models/refinery/resource.rb b/resources/app/models/refinery/resource.rb index 05defeaf8e..5882d6e122 100644 --- a/resources/app/models/refinery/resource.rb +++ b/resources/app/models/refinery/resource.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'dragonfly' module Refinery @@ -7,14 +9,18 @@ class Resource < Refinery::Core::BaseModel extend Mobility translates :resource_title - dragonfly_accessor :file, :app => :refinery_resources + dragonfly_accessor :file, app: :refinery_resources - validates :file, :presence => true + validates :file, presence: true validates_with FileSizeValidator + validates_property :mime_type, + of: :file, + in: ::Refinery::Resources.whitelisted_mime_types, + message: :incorrect_format - delegate :ext, :size, :mime_type, :url, :to => :file + delegate :ext, :size, :mime_type, :url, to: :file - before_destroy :cached_mime_type, :prepend => true + before_destroy :cached_mime_type, prepend: true def cached_mime_type @cached_mime_type ||= mime_type @@ -22,18 +28,18 @@ def cached_mime_type # used for searching def type_of_content - cached_mime_type.split("/").join(" ") + cached_mime_type.split('/').join(' ') end # Returns a titleized version of the filename # my_file.pdf returns My File def title - resource_title.presence || CGI::unescape(file_name.to_s).gsub(/\.\w+$/, '').titleize + resource_title.presence || CGI.unescape(file_name.to_s).gsub(/\.\w+$/, '').titleize end def update_index - return if self.aai_config.disable_auto_indexing - copy = self.dup.tap{ |r| r.file_uid = r.file_uid_was} + return if aai_config.disable_auto_indexing + copy = dup.tap { |r| r.file_uid = r.file_uid_was } self.class.index_remove(copy) self.class.index_add(self) end @@ -47,9 +53,9 @@ def per_page(dialog = false) def create_resources(params) resources = [] - if params.present? and params[:file].is_a?(Array) + if params.present? && params[:file].is_a?(Array) params[:file].each do |resource| - resources << create({:file => resource}.merge(params.except(:file).to_h)) + resources << create({ file: resource }.merge(params.except(:file).to_h)) end else resources << create(params) diff --git a/resources/config/locales/en.yml b/resources/config/locales/en.yml index b6689991f1..904ec0854b 100644 --- a/resources/config/locales/en.yml +++ b/resources/config/locales/en.yml @@ -36,4 +36,6 @@ en: models: refinery/resource: blank: You must specify file for upload - too_big: File should be smaller than %{size} bytes in size + incorrect_format: "File type is not allowed. Your file must be a MP4, MPEG, WMV, AVI, WAV, + GIF, JPEG, PNG, SVG, TIFF, PSD, CSV, PDF, TXT, RAR, ZIP, XLS, PPT or a DOC" + too_big: File should be smaller than %{size} bytes in size \ No newline at end of file diff --git a/resources/config/locales/fr.yml b/resources/config/locales/fr.yml index 3524b3ae52..064847f643 100644 --- a/resources/config/locales/fr.yml +++ b/resources/config/locales/fr.yml @@ -36,4 +36,6 @@ fr: models: refinery/resource: blank: Vous devez spécifier un fichier à télécharger - too_big: Le poids maximal des fichiers est de %{size} megaoctets + incorrect_format: "Type de fichier non autorisé. Votre fichier doit être un MP4, MPEG, WMV, AVI, WAV, + GIF, JPEG, PNG, SVG, TIFF, PSD, CSV, PDF, TXT, RAR, ZIP, XLS, PPT ou un DOC" + too_big: Le poids maximal des fichiers est de %{size} megaoctets \ No newline at end of file diff --git a/resources/lib/generators/refinery/resources/templates/config/initializers/refinery/resources.rb.erb b/resources/lib/generators/refinery/resources/templates/config/initializers/refinery/resources.rb.erb index 13e5ab74c8..ec526170d6 100644 --- a/resources/lib/generators/refinery/resources/templates/config/initializers/refinery/resources.rb.erb +++ b/resources/lib/generators/refinery/resources/templates/config/initializers/refinery/resources.rb.erb @@ -9,6 +9,9 @@ Refinery::Resources.configure do |config| # Configure how many resources per page should be displayed in the list of resources in the admin area # config.pages_per_admin_index = <%= Refinery::Resources.pages_per_admin_index.inspect %> + # Configure white-listed mime types for validation + # config.whitelisted_mime_types = <%= Refinery::Resources.whitelisted_mime_types.inspect %> + # Configure Dragonfly. # Refer to config/initializers/refinery/dragonfly.rb for the full list of dragonfly configurations which can be used. # This includes all dragonfly config for Dragonfly v 1.1.1 diff --git a/resources/lib/refinery/resources/configuration.rb b/resources/lib/refinery/resources/configuration.rb index eb1c15390d..5f0f8d0419 100644 --- a/resources/lib/refinery/resources/configuration.rb +++ b/resources/lib/refinery/resources/configuration.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + module Refinery module Resources - extend Refinery::Dragonfly::ExtensionConfiguration include ActiveSupport::Configurable - config_accessor :max_file_size, :pages_per_dialog, :pages_per_admin_index, :content_disposition + config_accessor :max_file_size, :pages_per_dialog, :pages_per_admin_index, + :content_disposition, :whitelisted_mime_types self.content_disposition = :attachment self.max_file_size = 52_428_800 @@ -13,6 +15,40 @@ module Resources self.dragonfly_name = :refinery_resources - end -end + self.whitelisted_mime_types = %w[ + audio/mp4 + audio/mpeg + audio/wav + audio/x-wav + + image/gif + image/jpeg + image/png + image/svg+xml + image/tiff + image/x-psd + + video/mp4 + video/mpeg + video/quicktime + video/x-msvideo + video/x-ms-wmv + text/csv + text/plain + + application/pdf + application/rtf + application/x-rar + application/zip + + application/vnd.ms-excel + application/vnd.ms-powerpoint + application/vnd.msword + + application/vnd.openxmlformats-officedocument.presentationml.presentation + application/vnd.openxmlformats-officedocument.spreadsheetml.sheet + application/vnd.openxmlformats-officedocument.wordprocessingml.document + ] + end +end \ No newline at end of file diff --git a/resources/spec/factories/resource.rb b/resources/spec/factories/resource.rb index 24c0421533..5518ed66e4 100644 --- a/resources/spec/factories/resource.rb +++ b/resources/spec/factories/resource.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + FactoryBot.define do - factory :resource, :class => Refinery::Resource do - file Refinery.roots('refinery/resources').join("spec/fixtures/refinery_is_awesome.txt") + factory :resource, class: Refinery::Resource do + file Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table.pdf') end end diff --git a/resources/spec/features/refinery/admin/resources_spec.rb b/resources/spec/features/refinery/admin/resources_spec.rb index 1e31548163..5dc96092cd 100644 --- a/resources/spec/features/refinery/admin/resources_spec.rb +++ b/resources/spec/features/refinery/admin/resources_spec.rb @@ -1,116 +1,135 @@ -# Encoding: UTF-8 -require "spec_helper" +# frozen_string_literal: true + +require 'spec_helper' module Refinery module Admin - describe "Resources", :type => :feature do + describe 'Resources', type: :feature do refinery_login - context "when no files" do - it "invites to upload file" do + context 'when no files' do + it 'invites to upload file' do visit refinery.admin_resources_path - expect(page).to have_content(%q{There are no files yet. Click "Upload new file" to add your first file.}) + expect(page).to have_content('There are no files yet. Click "Upload new file" to add your first file.') end end - it "shows upload file link" do + it 'shows upload file link' do visit refinery.admin_resources_path - expect(page).to have_content("Upload new file") - expect(page).to have_selector("a[href*='/refinery/resources/new']") + expect(page).to have_content('Upload new file') + expect(page).to have_selector('a[href*="/refinery/resources/new"]') end - context "new/create" do - it "uploads file", :js => true do - visit refinery.admin_resources_path - find('a', text: 'Upload new file').click + context 'new/create' do + let(:uploading_a_file) do + lambda do + visit refinery.admin_resources_path + find('a', text: 'Upload new file').click - expect(page).to have_selector 'iframe#dialog_iframe' + expect(page).to have_selector 'iframe#dialog_iframe' - page.within_frame('dialog_iframe') do - attach_file "resource_file", Refinery.roots('refinery/resources'). - join("spec/fixtures/refinery_is_awesome.txt") - click_button ::I18n.t('save', :scope => 'refinery.admin.form_actions') + page.within_frame('dialog_iframe') do + attach_file 'resource_file', file_path + click_button ::I18n.t('save', scope: 'refinery.admin.form_actions') + end end + end - expect(page).to have_content("Refinery Is Awesome") - expect(Refinery::Resource.count).to eq(1) + context 'when the file mime_type is acceptable' do + let(:file_path) { Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table.pdf') } + + it 'the file is uploaded', js: true do + expect(uploading_a_file).to change(Refinery::Resource, :count).by(1) + end end - describe "max file size" do + context 'when the file mime_type is not acceptable' do + let(:file_path) { Refinery.roots('refinery/resources').join('spec/fixtures/refinery_is_secure.html') } + + it 'the file is rejected', js: true do + expect(uploading_a_file).to_not change(Refinery::Resource, :count) + + page.within_frame('dialog_iframe') do + expect(page).to have_content(::I18n.t('incorrect_format', scope: 'activerecord.errors.models.refinery/resource')) + end + end + end + + describe 'max file size' do before do allow(Refinery::Resources).to receive(:max_file_size).and_return('1224') end - context "in english" do + context 'in english' do before do allow(Refinery::I18n).to receive(:current_locale).and_return(:en) end - it "is shown" do + it 'is shown' do visit refinery.admin_resources_path - click_link "Upload new file" + click_link 'Upload new file' within('#file') do - expect(page).to have_selector("a[tooltip='The maximum file size is 1.2 KB.']") + expect(page).to have_selector('a[tooltip="The maximum file size is 1.2 KB."]') end end end - context "in danish" do + context 'in danish' do before do allow(Refinery::I18n).to receive(:current_locale).and_return(:da) end - it "is shown" do + it 'is shown' do visit refinery.admin_resources_path - click_link "Tilføj en ny fil" - within "#file" do - expect(page).to have_selector("a[tooltip='Filen må maksimalt fylde 1,2 KB.']") + click_link 'Tilføj en ny fil' + within '#file' do + expect(page).to have_selector('a[tooltip="Filen må maksimalt fylde 1,2 KB."]') end end end end end - context "edit/update" do + context 'edit/update' do let!(:resource) { FactoryBot.create(:resource) } - it "updates file" do + it 'updates file' do visit refinery.admin_resources_path - expect(page).to have_content("Refinery Is Awesome") + expect(page).to have_content('Cape Town Tide Table') expect(page).to have_selector("a[href='/refinery/resources/#{resource.id}/edit']") - click_link "Edit this file" + click_link 'Edit this file' - expect(page).to have_content("Refinery Is Awesome or replace it with this one...") + expect(page).to have_content('Cape Town Tide Table or replace it with this one...') expect(page).to have_selector("a[href*='/refinery/resources']") - attach_file "resource_file", Refinery.roots('refinery/resources').join("spec/fixtures/refinery_is_awesome2.txt") - click_button "Save" + attach_file 'resource_file', Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table2.pdf') + click_button 'Save' - expect(page).to have_content("Refinery Is Awesome2") + expect(page).to have_content('Cape Town Tide Table2') expect(Refinery::Resource.count).to eq(1) end - describe "translate" do + describe 'translate' do before do - allow(Refinery::I18n).to receive(:frontend_locales).and_return([:en, :fr]) + allow(Refinery::I18n).to receive(:frontend_locales).and_return(%i[en fr]) end - it "can have a second locale added to it" do + it 'can have a second locale added to it' do visit refinery.admin_resources_path - expect(page).to have_content("Refinery Is Awesome") + expect(page).to have_content('Cape Town Tide Table') expect(page).to have_selector("a[href='/refinery/resources/#{resource.id}/edit']") - click_link "Edit this file" + click_link 'Edit this file' - within "#switch_locale_picker" do - click_link "FR" + within '#switch_locale_picker' do + click_link 'FR' end - fill_in "Title", :with => "Premier fichier" - click_button "Save" + fill_in 'Title', with: 'Premier fichier' + click_button 'Save' expect(page).to have_content("'Premier fichier' was successfully updated.") expect(Resource::Translation.count).to eq(1) @@ -118,53 +137,52 @@ module Admin end end - context "destroy" do + context 'destroy' do let!(:resource) { FactoryBot.create(:resource) } - it "removes file" do + it 'removes file' do visit refinery.admin_resources_path expect(page).to have_selector("a[href='/refinery/resources/#{resource.id}']") - click_link "Remove this file forever" + click_link 'Remove this file forever' - expect(page).to have_content("'Refinery Is Awesome' was successfully removed.") + expect(page).to have_content("'Cape Town Tide Table' was successfully removed.") expect(Refinery::Resource.count).to eq(0) end end - context "download" do + context 'download' do let!(:resource) { FactoryBot.create(:resource) } - it "succeeds" do + it 'succeeds' do visit refinery.admin_resources_path - click_link "Download this file" + click_link 'Download this file' - expect(page).to have_content("http://www.refineryhq.com/") + expect(page.body[0, 4]).to eq('%PDF') end context 'when the extension is mounted with a named space' do before do Rails.application.routes.draw do - mount Refinery::Core::Engine, :at => "/about" + mount Refinery::Core::Engine, at: '/about' end Rails.application.routes_reloader.reload! end after do Rails.application.routes.draw do - mount Refinery::Core::Engine, :at => "/" + mount Refinery::Core::Engine, at: '/' end end - it "succeeds" do + it 'succeeds' do visit refinery.admin_resources_path - click_link "Download this file" + click_link 'Download this file' - expect(page).to have_content("http://www.refineryhq.com/") + expect(page.body[0, 4]).to eq('%PDF') end - end end end diff --git a/resources/spec/fixtures/cape-town-tide-table.pdf b/resources/spec/fixtures/cape-town-tide-table.pdf new file mode 100644 index 0000000000..032a8a86c3 Binary files /dev/null and b/resources/spec/fixtures/cape-town-tide-table.pdf differ diff --git a/resources/spec/fixtures/cape-town-tide-table2.pdf b/resources/spec/fixtures/cape-town-tide-table2.pdf new file mode 100644 index 0000000000..032a8a86c3 Binary files /dev/null and b/resources/spec/fixtures/cape-town-tide-table2.pdf differ diff --git a/resources/spec/fixtures/refinery_is_awesome.txt b/resources/spec/fixtures/refinery_is_awesome.txt deleted file mode 100644 index 3ba158285d..0000000000 --- a/resources/spec/fixtures/refinery_is_awesome.txt +++ /dev/null @@ -1 +0,0 @@ -http://www.refineryhq.com/ diff --git a/resources/spec/fixtures/refinery_is_awesome2.txt b/resources/spec/fixtures/refinery_is_awesome2.txt deleted file mode 100644 index 69d6068eee..0000000000 --- a/resources/spec/fixtures/refinery_is_awesome2.txt +++ /dev/null @@ -1 +0,0 @@ -http://refinerycms.com \ No newline at end of file diff --git a/resources/spec/fixtures/refinery_is_secure.html b/resources/spec/fixtures/refinery_is_secure.html new file mode 100644 index 0000000000..780b652bf1 --- /dev/null +++ b/resources/spec/fixtures/refinery_is_secure.html @@ -0,0 +1 @@ +https://www.refinerycms.com/ \ No newline at end of file diff --git a/resources/spec/models/refinery/resource_spec.rb b/resources/spec/models/refinery/resource_spec.rb index 0e0327d241..2774c81709 100644 --- a/resources/spec/models/refinery/resource_spec.rb +++ b/resources/spec/models/refinery/resource_spec.rb @@ -1,61 +1,64 @@ +# frozen_string_literal: true + require 'spec_helper' module Refinery - describe Resource, :type => :model do + describe Resource, type: :model do let(:resource) { FactoryBot.create(:resource) } - let(:titled_resource) { FactoryBot.create(:resource, resource_title: 'Resource Title')} + let(:titled_resource) { FactoryBot.create(:resource, resource_title: 'Resource Title') } - context "with valid attributes" do - it "should create successfully" do + context 'with valid attributes' do + it 'should create successfully' do expect(resource.errors).to be_empty end end - context "resource url" do - it "should respond to .url" do + context 'resource url' do + it 'should respond to .url' do expect(resource).to respond_to(:url) end - it "should not support thumbnailing like images do" do + it 'should not support thumbnailing like images do' do expect(resource).not_to respond_to(:thumbnail) end - it "should contain its filename at the end" do + it 'should contain its filename at the end' do expect(resource.url.split('/').last).to match(/\A#{resource.file_name}/) end - context "when Dragonfly.verify_urls is true" do + context 'when Dragonfly.verify_urls is true' do before do allow(Refinery::Resources).to receive(:dragonfly_verify_urls).and_return(true) ::Refinery::Dragonfly.configure!(Refinery::Resources) end - it "returns a url with an SHA parameter" do + it 'returns a url with an SHA parameter' do expect(resource.url).to match(/\?sha=[\da-fA-F]{16}\z/) end end - context "when Dragonfly.verify_urls is false" do + context 'when Dragonfly.verify_urls is false' do before do allow(Refinery::Resources).to receive(:dragonfly_verify_urls).and_return(false) ::Refinery::Dragonfly.configure!(Refinery::Resources) end - it "returns a url without an SHA parameter" do + + it 'returns a url without an SHA parameter' do expect(resource.url).not_to match(/\?sha=[\da-fA-F]{16}\z/) end end end - describe "#type_of_content" do - it "returns formated mime type" do - expect(resource.type_of_content).to eq("text plain") + describe '#type_of_content' do + it 'returns formated mime type' do + expect(resource.type_of_content).to eq('application pdf') end end - describe "#title" do + describe '#title' do context 'when a specific title has not been given' do - it "returns a titleized version of the filename" do - expect(resource.title).to eq("Refinery Is Awesome") + it 'returns a titleized version of the filename' do + expect(resource.title).to eq('Cape Town Tide Table') end end context 'when a specific title has been given' do @@ -65,93 +68,113 @@ module Refinery end end - describe ".per_page" do - context "dialog is true" do - it "returns resource count specified by Resources.pages_per_dialog option" do + describe '.per_page' do + context 'dialog is true' do + it 'returns resource count specified by Resources.pages_per_dialog option' do expect(Resource.per_page(true)).to eq(Resources.pages_per_dialog) end end - context "dialog is false" do - it "returns resource count specified by Resources.pages_per_admin_index constant" do + context 'dialog is false' do + it 'returns resource count specified by Resources.pages_per_admin_index constant' do expect(Resource.per_page).to eq(Resources.pages_per_admin_index) end end end - describe ".create_resources" do - let(:file) { Refinery.roots('refinery/resources').join("spec/fixtures/refinery_is_awesome.txt") } + describe '.create_resources' do + let(:file) { Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table.pdf') } - context "only one resource uploaded" do - it "returns an array containing one resource" do - expect(Resource.create_resources(:file => file).size).to eq(1) + context 'only one resource uploaded' do + it 'returns an array containing one resource' do + expect(Resource.create_resources(file: file).size).to eq(1) end end - context "many resources uploaded at once" do - it "returns an array containing all those resources" do - expect(Resource.create_resources(:file => [file, file, file]).size).to eq(3) + context 'many resources uploaded at once' do + it 'returns an array containing all those resources' do + expect(Resource.create_resources(file: [file, file, file]).size).to eq(3) end end - specify "each returned array item should be an instance of resource" do - Resource.create_resources(:file => [file, file, file]).each do |r| - expect(r).to be_an_instance_of(Resource) + specify 'each returned array item should be an instance of resource' do + Resource.create_resources(file: [file, file, file]).each do |resource| + expect(resource).to be_an_instance_of(Resource) end end - specify "each returned array item should be passed form parameters" do - params = {:file => [file, file, file], :fake_param => 'blah'} + specify 'each returned array item should be passed form parameters' do + params = { file: [file, file, file], fake_param: 'blah' } - expect(Resource).to receive(:create).exactly(3).times.with({:file => file, :fake_param => 'blah'}) + expect(Resource).to receive(:create).exactly(3).times.with(file: file, fake_param: 'blah') Resource.create_resources(params) end end - describe "validations" do - describe "valid #file" do + describe 'validations' do + describe 'valid #file' do + before do + @file = Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table.pdf') + Resources.max_file_size = (File.read(@file).size + 1000) + end + + it 'should be valid when size does not exceed .max_file_size' do + expect(Resource.new(file: @file)).to be_valid + end + end + + describe 'wrong mime_type #file' do before do - @file = Refinery.roots('refinery/resources').join("spec/fixtures/refinery_is_awesome.txt") + @file = Refinery.roots('refinery/resources').join('spec/fixtures/refinery_is_secure.html') Resources.max_file_size = (File.read(@file).size + 10) + @resource = Resource.new(file: @file) + end + + it 'should not be valid when mime_type is not in .whitelisted_mime_types' do + expect(@resource).not_to be_valid end - it "should be valid when size does not exceed .max_file_size" do - expect(Resource.new(:file => @file)).to be_valid + it 'should contain an error message' do + @resource.valid? + expect(@resource.errors).not_to be_empty + expect(@resource.errors[:file]).to eq(Array(::I18n.t( + 'incorrect_format', scope: 'activerecord.errors.models.refinery/resource' + ))) end end - describe "too large #file" do + describe 'too large #file' do before do - @file = Refinery.roots('refinery/resources').join("spec/fixtures/refinery_is_awesome.txt") + @file = Refinery.roots('refinery/resources').join('spec/fixtures/cape-town-tide-table.pdf') Resources.max_file_size = (File.read(@file).size - 10) - @resource = Resource.new(:file => @file) + @resource = Resource.new(file: @file) end - it "should not be valid when size exceeds .max_file_size" do + it 'should not be valid when size exceeds .max_file_size' do expect(@resource).not_to be_valid end - it "should contain an error message" do + it 'should contain an error message' do @resource.valid? expect(@resource.errors).not_to be_empty expect(@resource.errors[:file]).to eq(Array(::I18n.t( - 'too_big', :scope => 'activerecord.errors.models.refinery/resource', - :size => Resources.max_file_size - ))) + 'too_big', scope: 'activerecord.errors.models.refinery/resource', + size: Resources.max_file_size + ))) end end - describe "invalid argument for #file" do + describe 'invalid argument for #file' do before do @resource = Resource.new end - it "has an error message" do + it 'has an error message' do @resource.valid? expect(@resource.errors).not_to be_empty expect(@resource.errors[:file]).to eq(Array(::I18n.t( - 'blank', :scope => 'activerecord.errors.models.refinery/resource' - ))) + 'blank', scope: 'activerecord.errors.models.refinery/resource' + ))) end end end