Skip to content

Commit

Permalink
Merge pull request #4357 from samvera/rename-white-list
Browse files Browse the repository at this point in the history
Renaming method from whitelisted to registered
  • Loading branch information
jcoyne authored Jun 8, 2020
2 parents c7b95e5 + ad5e645 commit 0fb9b48
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 22 deletions.
6 changes: 3 additions & 3 deletions app/actors/hyrax/actors/create_with_remote_files_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ def update(env)

private

def whitelisted_ingest_dirs
Hyrax.config.whitelisted_ingest_dirs
def registered_ingest_dirs
Hyrax.config.registered_ingest_dirs
end

# @param uri [URI] the uri fo the resource to import
def validate_remote_url(uri)
if uri.scheme == 'file'
path = File.absolute_path(CGI.unescape(uri.path))
whitelisted_ingest_dirs.any? do |dir|
registered_ingest_dirs.any? do |dir|
path.start_with?(dir) && path.length > dir.length
end
else
Expand Down
8 changes: 7 additions & 1 deletion app/models/content_block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,18 @@ class ContentBlock < ActiveRecord::Base
# `for` is a reserved word in Ruby.
def self.for(key)
key = key.respond_to?(:to_sym) ? key.to_sym : key
raise ArgumentError, "#{key} is not a ContentBlock name" unless whitelisted?(key)
raise ArgumentError, "#{key} is not a ContentBlock name" unless registered?(key)
ContentBlock.public_send(NAME_REGISTRY[key])
end

class << self
# @deprecated
def whitelisted?(key)
Deprecation.warn(self, "Samvera is deprecating '#{self}.whitelisted?' in Hyrax 3.0. Use #{self}.registered? instead.")
registered?(key)
end

def registered?(key)
NAME_REGISTRY.include?(key)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/generators/hyrax/templates/config/initializers/hyrax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@
config.browse_everything = nil
end

## Whitelist all directories which can be used to ingest from the local file
## Register all directories which can be used to ingest from the local file
# system.
#
# Any file, and only those, that is anywhere under one of the specified
Expand All @@ -275,7 +275,7 @@
# ingest files from the file system that are not part of the BrowseEverything
# mount point.
#
# config.whitelisted_ingest_dirs = []
# config.registered_ingest_dirs = []

##
# Set the system-wide virus scanner
Expand Down
22 changes: 18 additions & 4 deletions lib/hyrax/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,25 @@ def virus_scanner
end
end

# @!attribute [w] whitelisted_ingest_dirs
# List of directories which can be used for local file system ingestion.
attr_writer :whitelisted_ingest_dirs
# @deprecated
def whitelisted_ingest_dirs
@whitelisted_ingest_dirs ||= \
Deprecation.warn(self, "Samvera is deprecating #{self.class}#whitelisted_ingest_dirs " \
"in Hyrax 3.0. Instead use #{self.class}#registered_ingest_dirs.")
registered_ingest_dirs
end

# @deprecated
def whitelisted_ingest_dirs=(input)
Deprecation.warn(self, "Samvera is deprecating #{self.class}#whitelisted_ingest_dirs= " \
"in Hyrax 3.0. Instead use #{self.class}#registered_ingest_dirs=.")
self.registered_ingest_dirs = input
end

# @!attribute [w] registered_ingest_dirs
# List of directories which can be used for local file system ingestion.
attr_writer :registered_ingest_dirs
def registered_ingest_dirs
@registered_ingest_dirs ||= \
if defined? BrowseEverything
file_system_dirs = Array.wrap(BrowseEverything.config['file_system'].try(:[], :home)).compact
# Include the Rails tmp directory for cases where the BrowseEverything provider is required to download the file to a temporary directory first
Expand Down
10 changes: 5 additions & 5 deletions spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@
end

before do
allow(Hyrax.config).to receive(:whitelisted_ingest_dirs).and_return(["/local/file/"])
allow(Hyrax.config).to receive(:registered_ingest_dirs).and_return(["/local/file/"])
end

it "attaches files" do
expect(IngestLocalFileJob).to receive(:perform_later).with(FileSet, "/local/file/here.txt", user)
expect(actor.create(environment)).to be true
end

context "with files from non-whitelisted directories" do
context "with files from non-registered directories" do
let(:file) { "file:///local/otherdir/test.txt" }

it "doesn't attach files" do
Expand All @@ -114,16 +114,16 @@

describe "#validate_remote_url" do
before do
allow(Hyrax.config).to receive(:whitelisted_ingest_dirs).and_return(['/test/', '/local/file/'])
allow(Hyrax.config).to receive(:registered_ingest_dirs).and_return(['/test/', '/local/file/'])
end

it "accepts file: urls in whitelisted directories" do
it "accepts file: urls in registered directories" do
expect(actor.send(:validate_remote_url, URI('file:///local/file/test.txt'))).to be true
expect(actor.send(:validate_remote_url, URI('file:///local/file/subdirectory/test.txt'))).to be true
expect(actor.send(:validate_remote_url, URI('file:///test/test.txt'))).to be true
end

it "rejects file: urls outside whitelisted directories" do
it "rejects file: urls outside registered directories" do
expect(actor.send(:validate_remote_url, URI('file:///tmp/test.txt'))).to be false
expect(actor.send(:validate_remote_url, URI('file:///test/../tmp/test.txt'))).to be false
expect(actor.send(:validate_remote_url, URI('file:///test/'))).to be false
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/hyrax/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@
it { is_expected.to respond_to(:translate_id_to_uri) }
it { is_expected.to respond_to(:translate_uri_to_id) }
it { is_expected.to respond_to(:upload_path) }
it { is_expected.to respond_to(:whitelisted_ingest_dirs) }
it { is_expected.to respond_to(:whitelisted_ingest_dirs=) }
it { is_expected.to respond_to(:registered_ingest_dirs) }
it { is_expected.to respond_to(:registered_ingest_dirs=) }
it { is_expected.to respond_to(:work_requires_files?) }

describe "#whitelisted_ingest_dirs" do
describe "#registered_ingest_dirs" do
it "provides the Rails tmp directory for temporary downloads for cloud files" do
expect(configuration.whitelisted_ingest_dirs).to include(Rails.root.join('tmp').to_s)
expect(configuration.registered_ingest_dirs).to include(Rails.root.join('tmp').to_s)
end
end
end
6 changes: 3 additions & 3 deletions spec/models/content_block_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
expect { described_class.for(nil) }.to raise_error(ArgumentError)
end
end
context 'with a non-whitelisted value' do
context 'with a non-registered value' do
it 'raises an ArgumentError' do
expect { described_class.for(:destroy_all) }.to raise_error(ArgumentError)
end
end
context 'with a whitelisted value as a symbol' do
context 'with a registered value as a symbol' do
subject { described_class.for(:about) }

it 'returns a new instance' do
Expand All @@ -19,7 +19,7 @@
expect(subject).to be_persisted
end
end
context 'with a whitelisted value as a string' do
context 'with a registered value as a string' do
subject { described_class.for('about') }

it 'returns a new instance' do
Expand Down

0 comments on commit 0fb9b48

Please sign in to comment.